-
Notifications
You must be signed in to change notification settings - Fork 293
Removed halt and replaced it with a delay and reboot #1132
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: dev
Are you sure you want to change the base?
Conversation
and instead delays 10sec and reboots.
|
halt means stop. if you delay 10 seconds and then reboot, then it is a reboot and not a halt any more, is it? |
And I guess that's exactly what is intended. If there is a critical failure, there is no reason to continue. Rebooting could cause nasty boot loops that won't help either. |
|
For radio init failure, if the device has a screen, I'd probably prefer to show a message that says radio init failed. |
|
The halt function is only called in one place in the whole code base, just for that radio init in the being of boot. It's definitely a contributing factor for solar nodes not booting up properly. And I have already seen improvements in getting my repeaters to start from a BMS cut off. I would also hazard to say that a rover on Mars would not have a function like halt or a call to a function that doesn't try to get the rover back to an operational status. Most repeaters are in places where reset buttons are a pain to get too. And I thinking adding a message for nodes with a screens is a good idea with a count down to reset or something. |
|
I'll change the function name. A halt function like this one is bad way to handle a radio init problem.
|
|
I personally would rather have a boot loop then a non-responsive repeater, with a chance of a fully functional boot.
|
Displays a error message before rebooting.

Replaced the halt function with a delay and reboot, instead of an infinite loop. This removes an edge case were there's a bad brown out and the radio fails to initialize properly.
As @liamcottle suggested I have added an error message that is customizable to other failure cases. Available on node with a display enabled.