-
Notifications
You must be signed in to change notification settings - Fork 246
Improves OTA update reporting and process #838
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
Improves OTA update reporting and process #838
Conversation
|
@adamshiervani Not actually sure how to go about testing this... |
2389544 to
d71c887
Compare
|
In the Makefile, you just set a low number and run I'll try that out now. |
adamshiervani
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.
The OTA mechanism technically works. I was able to upgrade a device by lowering the version number in the Makefile. The code cleanup and the more granular state pushes to the client are great, but I have a few usability and robustness concerns.
The updated UI is confusing (I suspect flex-col is missing 😄). Despite the copy, it’s not immediately clear whether the reboot is automatic or requires the user to click “Reboot,” and it’s not obvious what happens if a reboot is already pending. All valid UX questions from the user’s perspective.
With the current implementation, if the reboot + IP assignment takes longer than the WebSocket reconnect policy allows, the “Reconnect” and “Reboot” buttons can lead the client to stale, lazy-loaded settings routes that break the app.
This PR aside, the only actual problem I noticed with OTA is the client-side state becomes desynchronized after a reboot: the client doesn’t perform a full refresh when the update completes. This is likely not caused by insufficient OTA-status events, but rather, the WebSocket reconnect threshold is being exceeded during the reboot/IP-assignment cycle (for example, DHCP can take a while). You can actually reproduce this by pulling the Ethernet cable during reboot -- the client hits the reconnect timeout and stops trying.
I would simply suggest extending the WebSocket reconnect threshold during updates to infinity. This way, the only thing that I can think of that could break the client-side part of the OTA mechanism is if DHCP assigned a different IP to the JetKVM, but that's fine for now.
b97c52c to
70cd19d
Compare
|
@adamshiervani clean-up complete and added exponential backoff and 2000 retries (so effectively 5.5 hours) |
| text="Reboot the KVM" | ||
| LeadingIcon={MdRestartAlt} | ||
| textAlign="center" | ||
| reloadDocument={true} |
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.
This forces a page reload as soon as the navigation is registered
| text="Reconnect to KVM" | ||
| LeadingIcon={MdConnectWithoutContact} | ||
| textAlign="center" | ||
| reloadDocument={true} |
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.
This forces a page reload as soon as the navigation is registered
In that case, we wouldn't be able to reconnect if they were using an IP, but if they were using a hostname or accessing through app.jetkvm.com it would eventually reconnect ;) |
|
@adamshiervani This should be ready for retest/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
This PR improves the OTA (Over-The-Air) update process by adding better reporting and handling of reboot requirements, including enhanced logging, state management for reboots, and UI changes to indicate reboot status with reconnect functionality.
- Adds
rebootNeededandrebootingstate management to track update progress through completion - Implements UI changes to show reboot status and provide manual reconnect/reboot buttons
- Improves error handling and logging throughout the update process
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/routes/devices.$id.tsx | Refactors authentication logic and improves WebSocket reconnection handling |
| ui/src/routes/devices.$id.settings.general.update.tsx | Adds reboot state UI with reconnect and manual reboot buttons |
| ui/src/routes/devices.$id.settings.general.reboot.tsx | Adds window reload on dialog close for proper state refresh |
| ui/src/routes/devices.$id.settings.access._index.tsx | Fixes grammar in comment |
| ui/src/main.tsx | Simplifies authentication check logic |
| ui/src/hooks/stores.ts | Adds rebootNeeded and rebooting state fields to OTA store |
| ui/src/components/Button.tsx | Adds reloadDocument prop support to LinkButton component |
| ota.go | Improves error messages, logging, and adds reboot state management |
| main.go | Enhances auto-update logic with better logging and timing |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
cfd9d3f to
fe91742
Compare
b3385ee to
48f5cb3
Compare
|
Rebased and ready for review. |
Add ability to request a reload to LinkButton and Link Added exponential backoff to reconnection Made the number of reconnect attempts settable Doesn't attempt a reconnection if we intentionally disconnect Make sure the fire-and-forget for TURN activity doesn't result in unhandled promise rejection. Fix comment Added force page reload to the onClose events of update/reboot Ensure we get the correct UI version. Fixed comment about the system update progress Removed duplicate code between main and devices.$id The isOnDevice and checkAuth can be leveraged from devices.$id.tsx Extend retries to 2000 (which is 20000 seconds after backing off)
Also cleaned up the loader function error-state returns
ffb5bb5 to
98f7233
Compare
|
@IDisposable I'm not familiar with react/tsx that much but your changes introduced compilation errors: I would say this is a regression and not an issue with the version of the npm or other build tool used to compile this code. |
|
Yikes, sorry about that, the rebase broke that code (it was an old PR). Thanks for the reminder to recompile after doing rebases 😄 |


Improves the OTA (Over-The-Air) update process by adding better reporting and handling of reboot requirements.