Predict a decline before offering firmware a node already runs #81
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/manifest-version-skip"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
CCS-UHA-23. Firmware (spec section 11.1) gained a
versionmember on amanifest entry and declines an image it is already running. We were not
sending it, so every install rewrote the same image: minutes of the van's
WiFi down and a needless flash cycle for nothing.
Two halves:
updater/file_server.pynow sendsversionon each manifest entry —CachedImage.versionalready carried it.updater/session.pypredicts the decline itself, using the node's lastSYS_ANNOUNCEand the release being offered, before the hotspot ever goesup. A development build (
0x0000) and a node that has never announced bothstill get sent, since neither can be predicted safely. The manifest field is
still sent regardless — the node is the authority on its own state.
For the residual case (an unknown running version, offered anyway, and the
node declines silently) the session still gives up on the usual timeout, but
the detail text tells a node that reached state 2 and never state 4 apart
from one that genuinely vanished, since that pattern is what a decline looks
like on the bus. It's a guess, not a certainty, so the outcome stays
lost.On the Home Assistant side,
version_is_newer(rewritten in CCS-UHA-22)already declines to offer an install once a release is not newer, so nothing
further was needed there.
Also corrects the README, which still described a three-field manifest from
before CCS-UHA-20 grew a fourth.
Tested: new unit tests in
tests/test_updater_session.pycover the predicteddecline, the development-build and unknown-version exceptions, and the
residual-case detail text;
tests/test_node_file_server.pycovers the newmanifest field. Full suite, ruff and mypy pass.
Logic in
declined_in_advancechecks out against the spec rule as described (dev build and unknown-running both proceed, unparseable offered proceeds, equal/older offered declines).ruff,mypyand the full test suite are clean, and the manifest/link plumbing looks like the minimal way to getofferedto the session.One real gap: nothing exercises the case that matters most in production — an offered release that genuinely is newer than what's running. Every new test that expects the session to proceed (dev build, never-announced) short-circuits on an earlier guard in
declined_in_advance(running is None or running == DEVELOPMENT) before ever reaching therelease > runningcomparison. Coverage confirms it:updater/session.pyline 196 — thereturn Nonefor a genuinely-newer release — is never hit by the suite. A flipped or mistyped comparison there (e.g.<for>) would make every real update look like a decline, and nothing would fail. Worth a one-line addition:Minor, not blocking: the
not orders(running)defensive branch (a garbage, non-development running value) is also untested, but it's a much less likely path._watch()'s residual-case wording is honest — "consistent with" rather than asserting it, and keepingOutcome.LOSTrather than inventing a new outcome matches the ticket. README and manifest/file_server changes look accurate and proportionate.Pushed a fix for the real defect flagged in review: a predicted decline was returning
Outcome.UNCHANGED, whichupdate.py'sTOOKset doesn't recognise, so pressing Install on a node already on the offered release raisedHomeAssistantError.version_is_neweronly stops the entity offering the install button — it doesn't coverupdate.installbeing called directly.Fixed by giving the predicted decline its own outcome,
Outcome.UP_TO_DATE, kept separate fromUNCHANGED(which still means "a session ran and the node may have rolled back" — warning level, ambiguous, unlike this).update.pynow treatsup_to_dateas a non-event: logged, not raised.Also added the test coverage the automated review flagged as missing (a release genuinely newer than the running one is not declined) plus a running version this build can't order, and a test that a predicted decline never leaves the session marked busy for the next node. New integration test in
tests/test_updater_link.pycovers the full path end to end — control API in, session predicts, node never sent anything, hotspot never raised.Full suite (1171 tests), ruff and mypy all pass;
session.pyandupdate.pyare both at 100% coverage.Round 2 re-review. The fix is correct and I traced it end to end rather than trusting the description. (Posting as COMMENT — Forgejo won't let this account APPROVE its own PR.)
Outcome.UP_TO_DATE = "up_to_date"inupdater/session.py→_finished()inupdater/api.pyputs the enum straight intoUpdateStatus.reason/the JSON"reason"field →custom_components/campervan/firmware.pyreads it back as a plain string →update.py'sUP_TO_DATE = "up_to_date"literal matches. No typo, no drift.UP_TO_DATEseparate fromUNCHANGEDis the right amount of change: they genuinely differ in behaviour (one logs at warning as a possible silent rollback, the other at info as a certainty), and each drives exactly one branch inupdate.py. One enum member, onerun()branch, oneupdate.pybranch, as scoped.radio.raisedstays0through the declined first call and only becomes1on the second, legitimate call. If the decline check inrun()were ever moved afterself._running = True, the second call would short-circuit toOutcome.BUSYbeforeraise_hotspotis ever called, and this test would fail.declined_in_advance'srelease > runningbranch is now covered (test_a_release_newer_than_what_a_node_runs_is_sent), closing the round-1 gap.update.install) with no redundant overlap."unchanged"/"up_to_date"/TOOK/Outcome.— nothing else needs updating;api.py/link.pydo pass outcome/reason through opaquely as claimed.tests/test_updater_session.py,tests/test_updater_link.py,tests/test_firmware_updates.py: all pass.ruff check/ruff format --checkclean on the changed files. Tabs, British English, comment discipline all fine, no scope creep.No changes requested.