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

Add option to reboot device #121

Merged
merged 9 commits into from
Aug 5, 2024
Merged

Add option to reboot device #121

merged 9 commits into from
Aug 5, 2024

Conversation

Teko012
Copy link
Collaborator

@Teko012 Teko012 commented Jul 10, 2024

From time to time, I experience an issue, where the device looses connection to the hardware, and can't find it anymore. This adds an option to be able to automatically reboot the device if it can't find any hardware.

@Teko012
Copy link
Collaborator Author

Teko012 commented Jul 10, 2024

@ketilmo If you have some time to check this as well if I haven't broken anything, that would be great :)

@ketilmo
Copy link
Owner

ketilmo commented Aug 4, 2024

Hi @Teko012,

Thanks for another excellent suggestion. I've experienced the same problem a few times, with the USB dongle needing a complete reboot after a crash/exit to be operational again. Implementing a way to handle this automatically when it happens is a good idea. Your code looks good, but I have a few comments/points for discussion:

  • Making the variable naming generic: For instance, a common variable across the services with the name REBOOT_DEVICE_ON_SERVICE_EXIT or similar would be passed to the respective service as a service variable. This would make it easier to scale in the future and maintain, with less hardcoding per service.
  • Rebooting the device using the balena API: Inspired by your PR Stop disabled services #123, using the API would ensure a graceful shutdown across the full range of balena's supported hardware devices. See the balena API docs here: https://docs.balena.io/reference/supervisor/supervisor-api/#post-v1reboot (using the --force option might be necessary?)
  • Writing a line to the console when functionality is active: To remind the user that the option is turned on, write something like "Device reboot on service exit is enabled" as part of the variable validation at the start.sh.
  • Add a short description to the docs: Under Advanced Configuration, with the heading "Automatic Device Reboot" or similar: https://github.com/ketilmo/balena-ads-b?tab=readme-ov-file#part-13--advanced-configuration

What do you think?

@Teko012
Copy link
Collaborator Author

Teko012 commented Aug 5, 2024

Hi @Teko012,

Thanks for another excellent suggestion. I've experienced the same problem a few times, with the USB dongle needing a complete reboot after a crash/exit to be operational again. Implementing a way to handle this automatically when it happens is a good idea. Your code looks good, but I have a few comments/points for discussion:

  • Making the variable naming generic: For instance, a common variable across the services with the name REBOOT_DEVICE_ON_SERVICE_EXIT or similar would be passed to the respective service as a service variable. This would make it easier to scale in the future and maintain, with less hardcoding per service.
  • Rebooting the device using the balena API: Inspired by your PR Stop disabled services #123, using the API would ensure a graceful shutdown across the full range of balena's supported hardware devices. See the balena API docs here: https://docs.balena.io/reference/supervisor/supervisor-api/#post-v1reboot (using the --force option might be necessary?)
  • Writing a line to the console when functionality is active: To remind the user that the option is turned on, write something like "Device reboot on service exit is enabled" as part of the variable validation at the start.sh.
  • Add a short description to the docs: Under Advanced Configuration, with the heading "Automatic Device Reboot" or similar: https://github.com/ketilmo/balena-ads-b?tab=readme-ov-file#part-13--advanced-configuration

What do you think?

Hey @ketilmo good feedback, I have updated the PR :)

@ketilmo
Copy link
Owner

ketilmo commented Aug 5, 2024

@Teko012 I just tested this functionality, and it works like a charm. One again: Great work, and thanks! I'm merging the PR now.

@ketilmo ketilmo merged commit 5b6163b into ketilmo:master Aug 5, 2024
@Teko012 Teko012 deleted the add/restart branch August 5, 2024 20:55
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