Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Timer for startup delay, advanceIfElapsed #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ky28059
Copy link
Member

@ky28059 ky28059 commented Oct 19, 2022

The timer-based startup delay simplifies startup code. Also, advanceIfElapsed is I think technically more reliable than hasElapsed and reset because it only resets the duration, not entirely (so it's not off by a few milliseconds depending on how long periodic takes to run). It's not necessary but I think it's cleaner.

lmk if this code makes sense to you or if it looks like it'll break. Also I think calling stop() in the constructor instead of the first periodic loop causes the same behavior because periodic is still being called even while the robot is enabled.

The timer-based startup delay simplifies startup code. Also, `advanceIfElapsed` is I think technically more reliable than `hasElapsed` and `reset` because it only resets the duration, not entirely (so it's not off by a few milliseconds depending on how long periodic takes to run). It's not necessary but I think it's cleaner.
@seasew
Copy link
Collaborator

seasew commented Oct 19, 2022

looks good to me, assuming advanceIfElapsed does what I think it does. let's test during class on Thursday and merge if it's all good

@ky28059
Copy link
Member Author

ky28059 commented Oct 19, 2022

I also left comments on some old commits; unsure if those are very visible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants