Tell a finished update by its build identifier #66

Merged
Claude merged 3 commits from feat/build-id-confirms-update into feat/firmware-updates 2026-09-19 21:25:06 +00:00
Collaborator

Section 11: "a new build identifier in SYS_ANNOUNCE after it is what says an update took". The session watched the firmware version instead, which is a hand-written 0x0001 on every node and does not move when a node is reflashed, so every successful update looked like nothing had happened.

A session now takes the build the node was last heard announcing, and judges the one it comes back on against it. An announce heard during the session — the node answering a descriptor query on the image it still runs — replaces the caller's word, since it is the node's own.

Three outcomes where there was one:

  • UPDATED — back on a build it did not go in on.
  • UNCHANGED — back on the one it went in on. No node reports a rollback, and this is what one looks like; so does being sent the image it was already running. It is not a failed fetch, so it never contradicts the fetch_failed attribute, which covers the failures a node is left running to report.
  • RETURNED — back, with nothing to judge it against.

Versions are still not comparable, and this does not try: version_is_newer stays as it is. This makes "did it take" answerable; "is it behind" still is not.

Nothing needed adding on the entity side. firmware_label already puts both the version and the build on the device and in installed_version, and OtaStore already ends a transfer on the pair changing.

Tested: a node back on a new build, back on the same one, and back with an unchanged version across a real update — the case that was broken. CCS-UHA-16.

Section 11: "a new build identifier in `SYS_ANNOUNCE` after it is what says an update took". The session watched the firmware version instead, which is a hand-written `0x0001` on every node and does not move when a node is reflashed, so every successful update looked like nothing had happened. A session now takes the build the node was last heard announcing, and judges the one it comes back on against it. An announce heard during the session — the node answering a descriptor query on the image it still runs — replaces the caller's word, since it is the node's own. Three outcomes where there was one: - `UPDATED` — back on a build it did not go in on. - `UNCHANGED` — back on the one it went in on. No node reports a rollback, and this is what one looks like; so does being sent the image it was already running. It is not a failed fetch, so it never contradicts the `fetch_failed` attribute, which covers the failures a node is left running to report. - `RETURNED` — back, with nothing to judge it against. Versions are still not comparable, and this does not try: `version_is_newer` stays as it is. This makes "did it take" answerable; "is it behind" still is not. Nothing needed adding on the entity side. `firmware_label` already puts both the version and the build on the device and in `installed_version`, and `OtaStore` already ends a transfer on the pair changing. Tested: a node back on a new build, back on the same one, and back with an unchanged version across a real update — the case that was broken. CCS-UHA-16.
Judge a node's return by its build identifier
Some checks failed
Lint, type check and test / hassfest (pull_request) Successful in 8s
Lint, type check and test / release (pull_request) Has been cancelled
Lint, type check and test / quality (pull_request) Has been cancelled
65dd9e8024
Pin the build as what a reflash shows on the entity
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 10s
Lint, type check and test / quality (pull_request) Successful in 4m46s
Lint, type check and test / release (pull_request) Has been skipped
e9ed2c929e
Claude left a comment

Judgement matches spec section 11 and the codec. Three things.

tests/test_updater_session.py:464 — test_a_version_that_never_moved_is_still_an_update_that_took is the same FakeNode and the same run call as the test directly above it; only the assertion differs. Nothing in it varies the firmware version, so it cannot fail for the reason its docstring gives. Mutating _judged to always return UPDATED leaves both passing. Move assert result.firmware == 1 into the test above and drop this one.

updater/session.py:554 — the new module-level _judged shares its name with UpdateSession._judged at line 416, which judges a BULK_END result. Two unrelated things under one name in one file. Rename the new one, e.g. _what_the_build_says.

updater/session.py:575 — the UNCHANGED warning and the _LOGGER.info at line 583 both fire on the same return, and the info line already carries the outcome. Drop one.

Not a defect, but worth knowing: tests/test_firmware_updates.py:466 passes unchanged against feat/firmware-updates. It pins firmware_label, not anything in this diff.

Judgement matches spec section 11 and the codec. Three things. `tests/test_updater_session.py:464` — `test_a_version_that_never_moved_is_still_an_update_that_took` is the same `FakeNode` and the same `run` call as the test directly above it; only the assertion differs. Nothing in it varies the firmware version, so it cannot fail for the reason its docstring gives. Mutating `_judged` to always return `UPDATED` leaves both passing. Move `assert result.firmware == 1` into the test above and drop this one. `updater/session.py:554` — the new module-level `_judged` shares its name with `UpdateSession._judged` at line 416, which judges a `BULK_END` result. Two unrelated things under one name in one file. Rename the new one, e.g. `_what_the_build_says`. `updater/session.py:575` — the `UNCHANGED` warning and the `_LOGGER.info` at line 583 both fire on the same return, and the info line already carries the outcome. Drop one. Not a defect, but worth knowing: `tests/test_firmware_updates.py:466` passes unchanged against `feat/firmware-updates`. It pins `firmware_label`, not anything in this diff.
@ -430,0 +461,4 @@
assert result.build == NEW_BUILD
async def test_a_version_that_never_moved_is_still_an_update_that_took() -> None:
Author
Collaborator

Same FakeNode and same run call as test_a_node_back_on_a_new_build_is_a_node_that_updated above; only the assertion differs. Nothing here varies the firmware version, so it cannot fail for the reason the docstring gives — mutating _judged to always return UPDATED leaves both passing. Fold assert result.firmware == 1 into the test above and delete this one.

Same `FakeNode` and same `run` call as `test_a_node_back_on_a_new_build_is_a_node_that_updated` above; only the assertion differs. Nothing here varies the firmware version, so it cannot fail for the reason the docstring gives — mutating `_judged` to always return `UPDATED` leaves both passing. Fold `assert result.firmware == 1` into the test above and delete this one.
@ -527,6 +551,52 @@ class UpdateSession:
return self._transfer
def _judged(
Author
Collaborator

Name collides with UpdateSession._judged at line 416, which judges a BULK_END transfer result. Rename, e.g. _what_the_build_says.

Name collides with `UpdateSession._judged` at line 416, which judges a `BULK_END` transfer result. Rename, e.g. `_what_the_build_says`.
@ -530,0 +572,4 @@
elif build == before:
outcome = Outcome.UNCHANGED
detail = f"the node came back on build {shown}, the one it went in on"
_LOGGER.warning(
Author
Collaborator

This warning and the _LOGGER.info below fire together on every UNCHANGED return, and the info line already names the outcome. Drop one.

This warning and the `_LOGGER.info` below fire together on every `UNCHANGED` return, and the info line already names the outcome. Drop one.
Take the review: one log line, one name, one version test
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 21s
Lint, type check and test / quality (pull_request) Successful in 1m58s
Lint, type check and test / release (pull_request) Has been skipped
a34a53142b
Claude left a comment

All three taken in a34a531. Nothing further.

test_a_node_announcing_before_it_writes_says_what_it_went_in_on now bites: with both _watch and _returned mutated to read firmware instead of build, it fails along with the other three build tests.

All three taken in a34a531. Nothing further. `test_a_node_announcing_before_it_writes_says_what_it_went_in_on` now bites: with both `_watch` and `_returned` mutated to read `firmware` instead of `build`, it fails along with the other three build tests.
Claude merged commit 09942df285 into feat/firmware-updates 2026-09-19 21:25:06 +00:00
Claude deleted branch feat/build-id-confirms-update 2026-09-19 21:25:06 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
rob/CampervanHomeAssistant!66
No description provided.