Follow the session lifecycle section 11 now fixes #65

Merged
Claude merged 7 commits from feat/session-lifecycle into feat/firmware-updates 2026-09-19 21:05:11 +00:00
Collaborator

Section 11 gained rules about how an update session ends, and UpdateSession sat through its own timeouts in cases where the node had already given it the answer.

State 6, a fetch that failed, now ends a session where it was watched to the ten minute deadline and then reported as LOST. It joins states 3 and 5 in one table of the states a node has nothing more to say after, each with the reason it stands for, so a failure the node explained is a FAILED with a cause rather than a timeout. LOST is left meaning the node stopped talking. State 0 is deliberately not among them: the radio goes off on the way to the reboot a finished update ends in.

The deadline is now the node's own five minutes for a stalled transfer plus a minute for the reboot after it, rather than ten minutes picked here. A node that says nothing at all about its radio is reported lost after thirty-five seconds — its own join deadline plus the five second status cadence — instead of holding the hotspot for the full deadline. radio_minutes keeps byte 1 of SYS_WIFI_CONTROL inside 1..255, since 0 means the idle timeout never fires and the codec keeps only the low byte of anything larger.

No behavioural change to the happy path, and nothing new goes on the wire: a node past the boot slot switch is still sent nothing, which is what the specification says is ignored.

Tested with the existing fake bus at the send and receive seam. The terminal-state tests run against a deadline nothing reaching them could ever hit, so ending at the timeout instead fails them.

CCS-UHA-17.

Section 11 gained rules about how an update session ends, and `UpdateSession` sat through its own timeouts in cases where the node had already given it the answer. State 6, a fetch that failed, now ends a session where it was watched to the ten minute deadline and then reported as `LOST`. It joins states 3 and 5 in one table of the states a node has nothing more to say after, each with the reason it stands for, so a failure the node explained is a `FAILED` with a cause rather than a timeout. `LOST` is left meaning the node stopped talking. State 0 is deliberately not among them: the radio goes off on the way to the reboot a finished update ends in. The deadline is now the node's own five minutes for a stalled transfer plus a minute for the reboot after it, rather than ten minutes picked here. A node that says nothing at all about its radio is reported lost after thirty-five seconds — its own join deadline plus the five second status cadence — instead of holding the hotspot for the full deadline. `radio_minutes` keeps byte 1 of `SYS_WIFI_CONTROL` inside 1..255, since 0 means the idle timeout never fires and the codec keeps only the low byte of anything larger. No behavioural change to the happy path, and nothing new goes on the wire: a node past the boot slot switch is still sent nothing, which is what the specification says is ignored. Tested with the existing fake bus at the send and receive seam. The terminal-state tests run against a deadline nothing reaching them could ever hit, so ending at the timeout instead fails them. CCS-UHA-17.
Report a node that never brings its radio up as lost
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 27s
Lint, type check and test / quality (pull_request) Successful in 4m53s
Lint, type check and test / release (pull_request) Has been skipped
a0098e3766
Claude left a comment

Five points, the first two worth acting on before merge.

  1. updater/session.py:116 — the default RADIO_SILENCE is never exercised. Every test injects its own through the session() helper, so setting RADIO_SILENCE = UPDATE_TIMEOUT leaves the whole suite green — the regression this branch exists to prevent. Pin it the way UPDATE_TIMEOUT is pinned, e.g. JOIN_DEADLINE < RADIO_SILENCE < UPDATE_TIMEOUT in test_the_deadline_sits_past_the_node_giving_up_and_no_further.

  2. updater/session.py:101 — UPDATE_TIMEOUT is measured from the control frame (started in _watch), but it is composed as if the clock starts at the fetch. The join takes up to 30 s before the node's own five minutes begin (spec section 11), so that 30 s comes out of REBOOT_ALLOWANCE. A node that joins at 29 s and uses the full five minutes has its last byte at 5:29, leaving under a minute for the 30 s provisional window plus a bootloader revert and boot — the docstring's stated budget. Narrow, but the fix is cheap: JOIN_DEADLINE + NODE_PATIENCE + REBOOT_ALLOWANCE is 6:30 and still well inside RESTORE_AFTER.

  3. updater/session.py:182 — "The detail is what the control API and the update entity show" is not true yet. Nothing consumes SessionResult; updater/api.py:_outcome builds its own reason and detail from started and connected. Drop the sentence or say it is where the detail is headed.

  4. tests/test_updater_session.py:847 — this still asserts the unclamped RESTORE_AFTER.total_seconds() // 60. If RESTORE_AFTER ever drops under a minute it fails against the clamp instead of confirming it. Assert radio_minutes(RESTORE_AFTER).

  5. tests/test_updater_session.py:613 — count("SYS_WIFI_CONTROL") == 1 cannot fail: the session sends exactly one on every path through _converse. The UPDATED assertion above it is what carries the test.

The rest verifies: state 3, 5 and 6 are terminal and each is tied to a distinct detail; the timeout reschedule is gated on a real SYS_WIFI_STATUS and dropping it fails the suite; state 0 is correctly left out; and radio_minutes matches the codec's masking behaviour, so the "low byte" reasoning in its test holds.

Five points, the first two worth acting on before merge. 1. `updater/session.py:116` — the default `RADIO_SILENCE` is never exercised. Every test injects its own through the `session()` helper, so setting `RADIO_SILENCE = UPDATE_TIMEOUT` leaves the whole suite green — the regression this branch exists to prevent. Pin it the way `UPDATE_TIMEOUT` is pinned, e.g. `JOIN_DEADLINE < RADIO_SILENCE < UPDATE_TIMEOUT` in `test_the_deadline_sits_past_the_node_giving_up_and_no_further`. 2. `updater/session.py:101` — `UPDATE_TIMEOUT` is measured from the control frame (`started` in `_watch`), but it is composed as if the clock starts at the fetch. The join takes up to 30 s before the node's own five minutes begin (spec section 11), so that 30 s comes out of `REBOOT_ALLOWANCE`. A node that joins at 29 s and uses the full five minutes has its last byte at 5:29, leaving under a minute for the 30 s provisional window plus a bootloader revert and boot — the docstring's stated budget. Narrow, but the fix is cheap: `JOIN_DEADLINE + NODE_PATIENCE + REBOOT_ALLOWANCE` is 6:30 and still well inside `RESTORE_AFTER`. 3. `updater/session.py:182` — "The detail is what the control API and the update entity show" is not true yet. Nothing consumes `SessionResult`; `updater/api.py:_outcome` builds its own reason and detail from `started` and `connected`. Drop the sentence or say it is where the detail is headed. 4. `tests/test_updater_session.py:847` — this still asserts the unclamped `RESTORE_AFTER.total_seconds() // 60`. If `RESTORE_AFTER` ever drops under a minute it fails against the clamp instead of confirming it. Assert `radio_minutes(RESTORE_AFTER)`. 5. `tests/test_updater_session.py:613` — `count("SYS_WIFI_CONTROL") == 1` cannot fail: the session sends exactly one on every path through `_converse`. The `UPDATED` assertion above it is what carries the test. The rest verifies: state 3, 5 and 6 are terminal and each is tied to a distinct detail; the timeout `reschedule` is gated on a real `SYS_WIFI_STATUS` and dropping it fails the suite; state 0 is correctly left out; and `radio_minutes` matches the codec's masking behaviour, so the "low byte" reasoning in its test holds.
@ -513,0 +610,4 @@
result = await ended(bus_, FakeRadio())
assert result.outcome is Outcome.UPDATED
assert sent_names(bus_.sent).count("SYS_WIFI_CONTROL") == 1
Author
Collaborator

This cannot fail: the session sends exactly one SYS_WIFI_CONTROL on every path through _converse. The UPDATED assertion is the one doing the work.

This cannot fail: the session sends exactly one `SYS_WIFI_CONTROL` on every path through `_converse`. The `UPDATED` assertion is the one doing the work.
@ -734,6 +847,27 @@ async def test_the_node_turns_its_radio_off_when_the_hotspot_goes() -> None:
assert fields_of(bus_.sent[-1])["timeout"] == RESTORE_AFTER.total_seconds() // 60
Author
Collaborator

Still the unclamped formula. If RESTORE_AFTER ever drops under a minute this fails against the clamp rather than confirming it — assert radio_minutes(RESTORE_AFTER).

Still the unclamped formula. If `RESTORE_AFTER` ever drops under a minute this fails against the clamp rather than confirming it — assert `radio_minutes(RESTORE_AFTER)`.
@ -76,0 +98,4 @@
whatever the bootloader reverts to if it does not.
"""
UPDATE_TIMEOUT = NODE_PATIENCE + REBOOT_ALLOWANCE
Author
Collaborator

Composed as if the clock starts at the fetch, but _watch starts it at the control frame. The join can take 30 s first (spec section 11), so it eats REBOOT_ALLOWANCE: last byte at 5:29 leaves under a minute for the 30 s provisional window plus a revert and boot. JOIN_DEADLINE + NODE_PATIENCE + REBOOT_ALLOWANCE is 6:30 and still inside RESTORE_AFTER.

Composed as if the clock starts at the fetch, but `_watch` starts it at the control frame. The join can take 30 s first (spec section 11), so it eats `REBOOT_ALLOWANCE`: last byte at 5:29 leaves under a minute for the 30 s provisional window plus a revert and boot. `JOIN_DEADLINE + NODE_PATIENCE + REBOOT_ALLOWANCE` is 6:30 and still inside `RESTORE_AFTER`.
@ -80,0 +113,4 @@
JOIN_DEADLINE = timedelta(seconds=30)
"""How long a node tries to join before raising its own access point. Spec 11."""
RADIO_SILENCE = JOIN_DEADLINE + STATUS_INTERVAL
Author
Collaborator

The default value here is never exercised — every test passes its own radio_silence. Setting this to UPDATE_TIMEOUT keeps the whole suite green, which is exactly the regression the branch is guarding against. Pin it alongside UPDATE_TIMEOUT, e.g. JOIN_DEADLINE < RADIO_SILENCE < UPDATE_TIMEOUT.

The default value here is never exercised — every test passes its own `radio_silence`. Setting this to `UPDATE_TIMEOUT` keeps the whole suite green, which is exactly the regression the branch is guarding against. Pin it alongside `UPDATE_TIMEOUT`, e.g. `JOIN_DEADLINE < RADIO_SILENCE < UPDATE_TIMEOUT`.
@ -119,1 +180,4 @@
`BUSY` never reached the node at all.
A node that said why it failed is never `LOST`. The detail is what the
control API and the update entity show, and a reason is worth more to
Author
Collaborator

Not true yet — nothing consumes SessionResult, and updater/api.py:_outcome builds its own reason and detail from started and connected. Drop it or mark it as where the detail is headed.

Not true yet — nothing consumes `SessionResult`, and `updater/api.py:_outcome` builds its own reason and detail from `started` and `connected`. Drop it or mark it as where the detail is headed.
Take the review: tighten the tests the clamp made loose
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 2m23s
Lint, type check and test / release (pull_request) Has been skipped
bf9d57068c
Claude left a comment

All five taken, and the two mutations that slipped through last time — RADIO_SILENCE = UPDATE_TIMEOUT and a deadline short of the join — now fail test_every_window_a_session_waits_out_is_one_the_node_named. 6:30 against a 15 minute RESTORE_AFTER leaves the node's idle timeout as the backstop it was meant to be. Nothing further from me.

All five taken, and the two mutations that slipped through last time — `RADIO_SILENCE = UPDATE_TIMEOUT` and a deadline short of the join — now fail `test_every_window_a_session_waits_out_is_one_the_node_named`. 6:30 against a 15 minute `RESTORE_AFTER` leaves the node's idle timeout as the backstop it was meant to be. Nothing further from me.
Merge feat/firmware-updates into feat/session-lifecycle
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 1m46s
Lint, type check and test / release (pull_request) Has been skipped
f7fb0e9f8c
Claude merged commit c266dc34f5 into feat/firmware-updates 2026-09-19 21:05:11 +00:00
Claude deleted branch feat/session-lifecycle 2026-09-19 21:05:11 +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!65
No description provided.