Skip to content

Hotfix 4.0.1: suppress snapshot requests during OTA update - #109

Merged
zakery292 merged 1 commit into
mainfrom
development
Jul 12, 2026
Merged

Hotfix 4.0.1: suppress snapshot requests during OTA update#109
zakery292 merged 1 commit into
mainfrom
development

Conversation

@zakery292

Copy link
Copy Markdown
Collaborator

While a dongle is in OTA mode it has no snapshot queue allocated, so the {"what":"all"} request triggered by its mid-update MQTT reconnect crashed it into a reboot loop. The update entity now marks the dongle as in-OTA around the whole install flow; the coordinator suppresses all snapshot paths (bootstrap, forced, recovery) for a marked dongle without burning the recovery debounce window, and one forced snapshot is sent after the OTA finishes so entities repopulate on the new firmware.

While a dongle is in OTA mode it has no snapshot queue allocated, so the
{"what":"all"} request triggered by its mid-update MQTT reconnect crashed
it into a reboot loop. The update entity now marks the dongle as in-OTA
around the whole install flow; the coordinator suppresses all snapshot
paths (bootstrap, forced, recovery) for a marked dongle without burning
the recovery debounce window, and one forced snapshot is sent after the
OTA finishes so entities repopulate on the new firmware.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown

Claude finished @zakery292's task in 2m 20s —— View job


Code Review Status

Tasks:

  • Gather context (PR description, changed files)
  • Review coordinator.py changes
  • Review update.py changes
  • Review manifest.json version bump
  • Review new test file
  • Post final review

Summary

This is a well-scoped hotfix. The root cause (a {"what":"all"} snapshot request landing on a dongle with no OTA-mode snapshot queue, crashing it into a reboot loop) is fixed cleanly by funneling every snapshot path — bootstrap (coordinator.py:1707, 1808), reboot-detected recovery (1805), and reconnect/availability recovery (mark_dongle_seenrequest_recovery_snapshot) — through the two chokepoints request_snapshot() and request_recovery_snapshot(), both of which now check is_ota_in_progress() first. I traced all call sites and confirmed there's no snapshot path that bypasses these two methods.

Verified logic

  • Debounce not burned during suppression (coordinator.py:237-244): request_recovery_snapshot checks OTA state before touching _last_recovery_snapshot, so a suppressed call during OTA doesn't consume the 30s debounce window — the post-OTA forced snapshot in update.py:372-376 won't be swallowed. Confirmed by test_recovery_snapshot_suppressed_and_debounce_not_burned.
  • Suppressed snapshot doesn't mark as sent: request_snapshot returns before touching _snapshot_requested when OTA is in progress, so the dedup gate doesn't fire a false-positive "already requested" once OTA ends.
  • finally block ordering in update.py:362-379: OTA flag is cleared before the post-OTA snapshot request, and the snapshot is only sent when ack["ok"] — i.e. only when the dongle actually accepted the OTA command, not on a rejected command or a lost ack. Reasonable: if OTA never really started, no need to re-snapshot; if the ack was lost but OTA proceeded anyway, the existing boot-count-change recovery path (coordinator.py:1787-1805) will catch it once the dongle reconnects post-reboot.
  • getattr defensive read/write in set_ota_in_progress/is_ota_in_progress (coordinator.py:186-197) correctly handles the test fixture pattern where coordinator.__new__ bypasses __init__ (matches the existing conftest.py fixture style).
  • Manually walked through all three new tests in test_ota_snapshot_suppression.py against the implementation — assertions hold.

Note: I wasn't able to execute pytest directly in this sandbox (tool permissions blocked it), so this is a manual trace rather than an actual run. Worth confirming green in CI once it runs.

Minor, non-blocking observations

  • custom_components/monitormysolar/CHANGELOG.md still tops out at "Version 4.0.0" — might be worth a short 4.0.1 entry mentioning the OTA reboot-loop fix, consistent with how prior version bumps documented themselves there.
  • No test exercises the update.py async_install flow itself (the set_ota_in_progress(True)/(False) calls and the post-OTA forced snapshot) — the new test file only covers the coordinator-side suppression logic. That said, this matches the existing repo convention (no other test file touches update.py, which needs heavier HA mocking), so I wouldn't block on it.

No correctness bugs, security issues, or regressions found. The fix is targeted, the suppression logic is centralized rather than duplicated per-path, and test coverage for the new coordinator behavior is solid.

@zakery292
zakery292 merged commit c34a617 into main Jul 12, 2026
1 check passed
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.

1 participant