Skip a firmware fetch the manifest says is not newer #62
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/uf-19-manifest-version"
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?
Each manifest entry now carries
version, the image's CalVer release as a string. The parser packs it withcan::version::pack, and the node skips the image fetch when it is not greater than its own — no slot erase, no 880 kB download, no reboot to arrive where it started.A skipped fetch is a success: the session ends in a new
Stage::UpToDate, which is neither in progress, installed nor failed, soSYS_WIFI_STATUSreports state 2 and the radio idles out its timeout. It never reports state 6.An entry with no
version, or one that is not three decimal fields in range, packs todevelopmentand is fetched unconditionally, as today. That keeps a bridge serving an older manifest working, and it is why a desk build — which reports0x0000and so is never newer than a release — always accepts an update.Spec section 11.1 carries the rule and the example. No frame's bytes change, so the vectors are untouched; the manifest format is a contract with the Home Assistant updater, tracked as CCS-UHA-22 and CCS-UHA-13.
Tested with
pio test -e native(436 cases), both node builds andhost_sim. CCS-UF-19.The skip logic itself holds up:
UpToDateis outsideinProgress(), soloop()returns before the five minute check and a skip can never report state 6; the packeduint16sorts in release order, so>is the right comparison; andreadVersion's string span is exact (mutatingat_ - text - 1either way fails the suite). Seven things to fix.The five-digit field cap is load-bearing and untested.
"4294969322.09.47"(2^32 + 2026) wrapsfields[0]to 2026 without the cap and packs to0x04AF— a forged release that would then be compared as real. Removing&& digits < versionFieldDigitsleaves the whole suite green. Add that string to the will-not-parse list.Log the version on the success path.
firmware_updater.cpp:174prints path, size and digest but notimage_.version. A bridge that emits a typo'd version silently reverts to the old unconditional-reinstall behaviour with nothing on serial to say so — which is the one failure mode this design has.Remaining points inline.
@ -1062,1 +1065,4 @@`version` is the image's CalVer release as `YYYY.MM.<counter>`, the same releasethe firmware reports in section 8.1. **The image is fetched only when thatversion is greater than the one the node is running**, comparing the packedMid-paragraph bold of a whole clause isn't this document's style; bold is used for message headings and for statements that open a paragraph. Make it a plain sentence.
@ -1063,0 +1070,4 @@not fetched: nothing is erased or written, the session reports no failure, andthe node stays in state 2 until the radio idles out its timeout.An entry whose `version` is absent, is not a string, or is not three decimalOut of step with the code.
2026.09.000047is three decimal fields within the ranges of 8.1, so this sentence says it gets compared;packVersioncaps each field at five digits and reads it as a development build, so it gets fetched unconditionally. Either say "at most five digits each" here, or drop the cap — but the cap is what stops a long field wrappinguint32into a valid-looking release, so the spec is the side to change.@ -1063,0 +1072,4 @@An entry whose `version` is absent, is not a string, or is not three decimalfields within the ranges of section 8.1 is fetched unconditionally, whatever thenode is running. A manifest written before this member existed therefore stillBoth of these are derivations rather than rules and should go. Worse, "A development build reports
0x0000..." is about the version the node is running, sitting in the paragraph about the manifest'sversion— the one thing an implementer of CCS-UHA-22 could misread. The rule plus 8.1 already gives it.@ -25,6 +28,28 @@ bool hexValue(char c, uint8_t& out) {return false;}// Anything that is not three decimal fields in range reads as a development"which is never newer than a release" points the reader the wrong way: development in the manifest means fetched unconditionally, not never fetched. The first sentence restates the guards below it.
newerThanRunningalready carries the real why — cut this to the section reference, or drop it.@ -28,0 +39,4 @@if (i > 0 && (at >= end || *at++ != '.')) return can::version::development;uint8_t digits = 0;while (at < end && *at >= '0' && *at <= '9' && digits < versionFieldDigits) {Nothing pins this cap. Removing
&& digits < versionFieldDigitskeeps all 436 cases green, yet"4294969322.09.47"then wrapsfields[0]to 2026 and packs to0x04AF. Add it as a will-not-parse case.@ -28,0 +43,4 @@fields[i] = fields[i] * 10 + static_cast<uint32_t>(*at++ - '0');++digits;}if (digits == 0) return can::version::development;Redundant. Every in-range field needs at least one digit, so
can::version::packalready rejects anything that reaches here withdigits == 0; brute-forcing over0,.,2,9up to length 7 finds no input whose result changes when this line is removed, and the mutation survives the suite. Drop it.@ -78,1 +79,4 @@constexpr uint16_t runningRelease = can::version::pack(2026, 9, 7);// The same manifest naming the release the node is already running.Restates
sameVersionManifest. Delete.All seven acted on in
ce436e1.4294969322.09.47,2026.4294967305.47and2026.09.4294967343— one per field — to the will-not-parse list. Removing the cap now fails the suite.2026.09.000047is pinned as not parsing.version 0x%04X.digits == 0removed. Every missing field leaves zero, whichcan::version::packrejects on all three ranges, so the ranges stay in one place.Round two. All seven are genuinely addressed — the parser now matches the reworded 11.1 exactly (brute-forced against a reference built from the sentence's literal words over
0 . 1 2 9 x, lengths 0-8, plus the padding and wrap cases: zero divergence), and droppingdigits == 0changes no result on any of those inputs, so the reasoning in point 5 holds. Two small things left.versionFieldDigitsis still only pinned from above. Dropping it to 4 leaves the suite green, so nothing says five digits must be accepted —2026.09.00047is a legal five-digit counter and there is no valid-version case with a five-digit field.The replacement comment on
packVersionmisattributes the mechanism. Details inline.@ -25,6 +28,28 @@ bool hexValue(char c, uint8_t& out) {return false;}// The CalVer release string of spec section 11.1. A field longer than its digit"for the caller to trip over" is the wrong actor — the leftover digit is rejected inside this function, by the
*at++ != '.'separator check on the next field or byat != endat the bottom.packVersion's only caller,readVersion, does nothing but take the returneduint16. As written, a future editor could read this as licence to drop theat != endguard. Say which check does the rejecting.@ -207,0 +231,4 @@TEST_ASSERT_TRUE(parse(versioned("2026.01.1"), bathroom, image));TEST_ASSERT_EQUAL_HEX16(can::version::pack(2026, 1, 1), image.version);TEST_ASSERT_TRUE(parse(versioned("2057.12.127"), bathroom, image));Setting
versionFieldDigitsto 4 leaves all 436 cases green: the wrap cases pin the cap from above, nothing pins it from below. Addversioned("2026.09.00047")expectingpack(2026, 9, 47)here — a legal five-digit field that a tighter cap would read as a development build.Both round-two findings acted on in
034ce49.versioned("2026.09.00047")expectingpack(2026, 9, 47)added to the boundary test. Setting the cap to 4 now fails it, so it is pinned from both sides.packVersion's own separator and end checks rather than the caller.Clean, nothing left.
versionFieldDigitsnow fails the suite at 4, at 6 and with the cap removed, and the comment names the right checks.On comparing the running partition's digest with
esp_partition_get_sha256()instead: not done. It hashes the whole running slot on every session — roughly 880 kB of SHA-256 before anything is fetched — and it can only ever say "not the same bytes", not "older", so on its own it would reinstall a rebuild of the same release just as readily. The version claim is what the ticket is about, and the release counter comes from the package registry, so the job cannot publish two different images under one version. Happy to add it as a belt-and-braces second check if you want it, but it does not replace the comparison.Review came back clean after two rounds. Not merged — that is your call.