-
Notifications
You must be signed in to change notification settings - Fork 0
added low battery alert #40
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
Conversation
matthewcwheeler
left a comment
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.
lgtm, just wondering if the alert logic can be cleaned up a bit.
src/main/java/frc/robot/Robot.java
Outdated
| } | ||
| double batteryVoltage = RobotController.getBatteryVoltage(); | ||
|
|
||
| if(batteryVoltage<= kLowBatteryVoltage && disabledTimer.hasElapsed(kLowBatteryDisabledTime) && lowBatteryCycleCount >= kLowBatteryMinCycles){ |
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.
How often is it the case that cycle count is less than 10, but that the elapsed time is greater than 1.5 (seconds, minutes?)? That is, do you need both checks?
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.
i dont think we need both checks - updated in the newest commit
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.
@matthewcwheeler can you do one more check before i merge?
Signed-off-by: Bang Xiao <bangxiao0927@gmail.com>
bangxiao0927
left a comment
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.
lgtm
Signed-off-by: Bang Xiao <bangxiao0927@gmail.com>
6328 and a few other teams have these low battery alerts so when they are testing or during drive practice they know when to change batteries to maximize the lifetime, I thought we could try it next season, and it will also help during drive practice