Conversation
|
There was a problem hiding this comment.
Pull request overview
This PR enhances the authenticated Hub OTA update experience by adding richer UI/state for firmware updates, progress tracking, and surfaced results coming back from SignalR.
Changes:
- Revamps the hub update page UI with confirmation dialog, weighted overall progress + task progress, and an OTA update history table.
- Introduces
otaResultintoHubOnlineStateand updates SignalR handlers to populate it on success/failure/rollback (plus toast notifications). - Adjusts device status SignalR handling to preserve ongoing OTA install state/result across status updates.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/routes/(authenticated)/hubs/[hubId=guid]/update/+page.svelte |
New OTA update UX: confirm dialog, progress model, status/result rendering, update history table. |
src/lib/stores/HubsStore.ts |
Adds otaResult to the online hub state type. |
src/lib/signalr/handlers/OtaRollback.ts |
Sets otaResult for rollback and emits warning toast. |
src/lib/signalr/handlers/OtaInstallSucceeded.ts |
Sets otaResult for success and emits success toast. |
src/lib/signalr/handlers/OtaInstallFailed.ts |
Sets otaResult for failure and emits error toast. |
src/lib/signalr/handlers/DeviceStatus.ts |
Preserves existing otaInstall/otaResult when processing device status updates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -37,100 +193,209 @@ | |||
| .devicesOtaGetOtaUpdateHistory(hubId) | |||
| .then((resp) => { | |||
| if (resp.data === null) { | |||
| hub = null; | |||
| hubLoaded = false; | |||
| return; | |||
| } | |||
|
|
|||
| hub = $OnlineHubsStore.get(hubId) ?? { | |||
| hubId, | |||
| isOnline: false, | |||
| firmwareVersion: null, | |||
| otaInstall: null, | |||
| }; | |||
| hubLoaded = true; | |||
| otaLogs = resp.data; | |||
| }) | |||
| .catch(handleApiError) | |||
| .finally(() => (isLoading = false)); | |||
| } | |||
There was a problem hiding this comment.
hubLoaded is used as a gate for rendering hub details, but it’s only set to true when the OTA history request succeeds (resp.data !== null). If the request fails (network/API error), hubLoaded stays false, which causes the UI to fall through to the "Hub not found" state even though the hub may exist (and the error was already handled via handleApiError). Consider separating "hub exists" from "history loaded" (e.g., keep rendering hub info from OnlineHubsStore and add a dedicated historyLoadError/historyLoaded state for the table).
| function fetchOtaLogs(hubId: string | undefined) { | ||
| if (hubId === undefined) { | ||
| hub = null; | ||
| hubLoaded = false; |
There was a problem hiding this comment.
When fetchOtaLogs is called with hubId === undefined, it returns early without resetting isLoading. If a navigation/param change happens mid-request, this can leave the page stuck in a loading-disabled state (spinner / disabled refresh). Set isLoading = false (and likely clear otaLogs) in the early-return path as well.
| hubLoaded = false; | |
| hubLoaded = false; | |
| isLoading = false; | |
| otaLogs = []; |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7985abaf75
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| stopRebootTimer(); | ||
| } | ||
|
|
||
| return stopRebootTimer; |
There was a problem hiding this comment.
Returning stopRebootTimer as the effect cleanup causes the reboot simulation to reset on every reactive re-run, not just on unmount. Because hub is rebuilt from store updates, any OnlineHubsStore change during the reboot phase (for example DeviceStatus online/offline transitions) will clear the interval and set rebootProgress back to 0, so the displayed reboot progress can jump backward or appear stuck until another terminal OTA event arrives.
Useful? React with 👍 / 👎.
No description provided.