-
-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor shot clock state handling #3
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| isRunning = false; | ||
| flashState = false; | ||
| updateDisplay(); | ||
| delay(10); |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The loop delay was reduced from 1000ms to 10ms, which increases CPU usage 100x. Since the timer only needs second-level precision and flash updates occur every 500ms, consider using a delay of at least 100ms to balance responsiveness with power efficiency.
| delay(10); | |
| delay(100); |
| const unsigned long elapsed = now - lastSecondTick; | ||
| if (elapsed >= 1000) { | ||
| const int secondsPassed = elapsed / 1000; | ||
| lastSecondTick += secondsPassed * 1000; |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The drift correction logic assumes secondsPassed is always 1 in normal operation. If the device experiences significant lag (e.g., >2 seconds), remaining could underflow. Consider adding a bounds check: remaining = max(0, remaining - secondsPassed) or limiting secondsPassed to prevent unexpected behavior.
| drawButton(startButton, startButton.label); | ||
|
|
||
| const Button& activeButton = shotClock.timeUp ? resetButton : stopButton; | ||
| const char* label = shotClock.timeUp ? resetButton.label : stopButton.label; |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The label is redundantly extracted from the same button that was just assigned to activeButton. Since activeButton.label already contains the correct label, this line can be simplified to const char* label = activeButton.label;
| const char* label = shotClock.timeUp ? resetButton.label : stopButton.label; | |
| const char* label = activeButton.label; |
Summary
Testing