Show a fetch the node says failed on its update entity #64
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/failed-fetch-on-update-entity"
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?
SYS_WIFI_STATUSstate 6 was read as any other radio state, so a failed fetchlooked like an update that quietly stopped part way. It now reads as
fetch_failedon the node's update entity, beside the progress that alreadycomes off byte 2 of the same frame. Spec section 11.
The failure outlives the session on purpose. The bridge ends a session within
seconds of state 6, so the radio going off is what a failed node says next;
clearing on it would take the failure away before anyone saw it. What does clear
it is the node writing an image again, announcing a different build, going
quiet, or the bus going away.
State 6 is only the failures a node is left running to report, so
fetch_failedgoing untrue is not an update having taken — an image that boots and never
commits is reverted by the bootloader and nothing on the bus reports it. What
says an update took is a new build identifier in
SYS_ANNOUNCE, which isCCS-UHA-16 and not here.
Tests cover state 6 during a transfer, state 6 with no transfer showing, the
radio idling out afterwards, another transfer starting, and a normal install
still completing. The store's own clearing paths are tested directly.
Touches
firmware.pyas well asupdate.py, because that is whereSYS_WIFI_STATUSis decoded. Nothing inupdater/.Ref: CCS-UHA-15.
Three things.
Asking for another update doesn't clear the failure. Spec 11: state 6 stands "until the session ends or another update is asked for". Only state 4 clears it here, so from pressing Install until the image starts writing — credentials, join, manifest fetch, all of which take the
elsebranch inasync_handle_frame— the entity still readsfetch_failed: truefrom the previous attempt. Worse, if the retry fails the same way (server still unreachable), state 6 arrives,_movedsees no change and nothing redraws, so a fresh failure is indistinguishable from the stale one. Clear it inasync_installwhen the updater says it started._announced'sfailed=Falsehas no test. Change it tofailed=Noneand all 47 still pass.test_an_install_that_ran_to_the_end_leaves_no_failure_behindis the test that looks like it covers it, but it never broadcasts state 6, soFETCH_FAILED is Falseis true before the announce and cannot fail; its other two assertions duplicatetest_a_node_that_comes_back_from_a_transfer_stops_showing_progress. Send state 6 before the new-build announce and it becomes a real test. The PR body's "the store's own clearing paths are tested directly" isn't true of that one.The bus going away wipes every failure (
firmware.py:330), and the docstring's reason — "none can be followed" — is about progress, which is a thing being followed. A failure is a fact already learned, and nothing ever re-reports it: the session is over, so after a CAN blip the flag is gone for good. That's the same silent disappearance the state-0 retention exists to prevent. Same for a Home Assistant restart, since the set is in memory only. I'm not certain this is wrong, but it looks unintended given the rest of the branch; if it is deliberate, the docstring should carry that reason rather than the progress one.Took 1 and 2 in
a104c4b.A started install now calls
OtaStore.async_update_asked_for, which clears the node's failure at the press rather than a join later. The point about a retry failing identically was the one that decided it:_movedwould have seen no change and redrawn nothing. Two tests: a started install clears it, an install the updater sent nothing for leaves it showing, because nothing was asked of the node.test_a_node_that_comes_back_on_a_new_build_clears_its_last_failurebroadcasts state 6 before the new-build announce, so the announce path bites now. Kepttest_an_install_that_ran_to_the_end_leaves_no_failure_behindas the plain "a normal install still completes" case the ticket asked for.Leaving the bus-down clearing as it is, and the docstring now says why. Nothing re-reports a failure, so holding one across a gap means claiming a node has just failed a fetch on the strength of something it said before the interface went down — and it may have rebooted since, which does not change the build and so would never clear it. A lost record costs a press of install; a false one is a fault indicator that is wrong, which this integration would rather not have. Unavailable is the answer while the bus is down. In-memory only is the whole store, progress included, and no more true of failures than of anything else here.
Round two. The install-clears path and the new announce test both bite under mutation, and bus-down is your call to make — the docstring now carries the reason, which is what I was after.
async_update_asked_fordrops progress as well as the failure, and nothing tests that either way (firmware.py:316). Change it toself._moved(node, self._progress.get(node), failed=False)and all 50 still pass. The one case where it shows is install pressed while the node is writing: the entity reads "not writing" until the nextSYS_WIFI_STATUSup to 5 s later, which is less true than the 42% it replaced — the node is the authority on its own transfer and it has not said the transfer ended. I'd keep the progress and clear only the failure; the docstring only argues for the failure anyway. If you keep the drop, it needs a test and a line saying why.The entity calling into the store fits —
switch.py:92andselect.py:55do the same thing, andasync_installclearing only afterstartedis the right side of the raise.Two small ones.
README.md:104was edited without re-wrapping, leaving "a different build, or going quiet. It covers" short against the 80-column paragraph around it. Andtest_an_install_that_ran_to_the_end_leaves_no_failure_behindis now fully redundant: its progress assertions duplicatetest_a_node_that_comes_back_from_a_transfer_stops_showing_progressand itsFETCH_FAILED is Falsestill cannot fail, since the newtest_a_node_that_comes_back_on_a_new_build_clears_its_last_failureis the one that sets the failure first. Drop it.Clean. Nothing further from me.