Decode the packed CalVer firmware version, and order on it #78
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/calver-firmware-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?
The firmware's CalVer packing has landed on
main, soSYS_ANNOUNCEbytes 2-3and
SYS_HEARTBEATbytes 6-7 carry a real release instead of a hardcoded0x0001. This decodes it and puts the ordering back.CCS-UHA-22.
The vector refresh
tests/vectors/SOURCEis back tomain. The firmware'sfeat/firmware-updatesbranch is gone, so the old value would 404 — which is why this branch and the
base are red, and why merging this also unblocks #53.
The refresh brought in exactly the three expected cases —
announce_release_2026_09_47,announce_development_build,heartbeat_release_2026_09_127— and changed nothing else. The full suitepassed on the refreshed vectors with no codec change, so there is no drift to
report.
The decode
protocol/versions.pyis the one transcription of the packing, inprotocol/because the updater's container mounts that and nothing else of the integration.
That is what lets
link.pystop writing the version label out a second time.release_ofgives a version's CalVer, and nothing for a development build ora value the packing cannot explain. The month check is what rejects zero, so
no path reads the year of a build that has none.
packedgoes the other way, so an offered release can be put beside a node'sraw
uint16. Every comparison is a greater-than on the raw value, never onthe text — "2026.09.10" sorts before "2026.09.9".
orderssays whether a value can be trusted to sort. A development build can,because it is below every release by construction. Month 15 cannot.
version_is_newerdoes a real comparison now, and its docstring no longerclaims the two sides do not order. The
-1 in version_order(installed)guardin
updater/api.pyis gone the same way.Anything unorderable is ignored, not rejected, and the two sides answer
differently on purpose: the entity offers it (an update nobody can see reads as
a fault in the van) and
/api/nodesreportsupdate_available: null(a routestating facts should not guess).
The judgement call, and what I chose
I kept the build identifier and added the release beside it: a session counts as
updated if either moved.
Dropping the build check would have lost two things. A bootloader rollback to
another build of the same release is invisible to the release alone, and that is
the one failure nothing else on this bus reports. Two builds of one release are
also a real thing during development.
What the release adds is the other gap: a node flashed from a desk announces
build
0x00000000, which identifies nothing, so until now every such return wasRETURNED— "nothing to judge it against". Its release still moves. That is thebehaviour change in
updater/session.py, and two existing tests moved fromRETURNEDtoUNCHANGEDbecause of it, which is the stronger and still honestanswer: neither the build nor the release moved.
Outcome.UPDATEDnow says whatit actually claims — running something it was not, never that it is running the
image that was served.
Two things the review pulled in
The bridge's own announce.
encode_versionpacked the Pi'smajor.minor.patchwith a nibble scheme of its own, which the spec nowcontradicts:
1.2.3would have gone out as0x0123and decoded to"2026.02.35", a release nobody published. It is
packed(version) or DEVELOPMENTnow. This integration is released under the same CalVer the firmware is, so a
published copy packs its real release there and a checkout — whose committed
version stays below every release by design — announces the development build it
is.
Absent is not zero.
Node.firmwareisint | None, matching the updater'sAnnounced. ASYS_ANNOUNCEtoo short to carry a version now reads asunknownon the device page rather than claiming a desk build, since zero is areal answer with a real meaning.
Not done here
updater/file_server.pyis untouched: the manifestversionmember thefirmware added is its own ticket, and the two branches would collide.
Testing
New
tests/test_firmware_versions.py, driven off the refreshed vectors ratherthan a second copy of the packing — including the development-build zero, the
0x04FFcounter ceiling and0xFFFFas a version that cannot be explained. Theentity's ordering is covered end to end in
tests/test_firmware_updates.py,whose fixtures now announce real releases. 1164 tests, ruff and mypy clean.
versions.pymatcheslib/can-protocol/src/version.hbit for bit, andtests/vectors/frames.jsonis byte-identical to the firmware'smain. Agreed on "either moved": build alone is blind to a desk-flashed node, release alone is blind to a rebuild, and the only way to get a falseUPDATEDis a reboot onto a different image, which is the thing being asked about. Worth saying in the docstring that it means "running something different", not "running the release that was offered" —_returnednever sees the image version it served.README.mdlines 83-89 still say the two versions are not comparable and that "nothing here claims a node is up to date". That is exactly what this branch changed, and it is the paragraph a reader reaches first.discovery.py:588—fields.get("firmware", 0)collapses an absent field into the development sentinel, so a truncatedSYS_ANNOUNCE(bytes 2-3 missing decodes cleanly, I checked) now shows "development" on the device page: a positive claim where "0x0000" was merely useless.updater/link.pygets this right withfirmware: int | Noneand "unknown";Node.firmware: int = 0cannot.This is the one real defect and I would not leave it to a later ticket.
1.2.3goes out as0x0123, which decodes to "2026.02.35" — a release that never existed — and anyx.y.zwith a non-zero minor and patch does the same.0.1.0only escapes by accident.The firmware's own
can::version::packreturnsdevelopmentfor anything it cannot carry. Doing the same here (return DEVELOPMENTunlesspacked(version)succeeds) is a line, and it makes the bridge announce something the spec permits. The docstring's "goes out as zero rather than aliasing onto another number" also needs a look now that zero has a defined meaning: zero is no longer neutral, it says "desk build", which for the Pi is the honest answer anyway.@ -0,0 +33,4 @@can mean, which is ignored rather than rejected as everywhere on this bus."""if version == DEVELOPMENT:return NoneDead: deleting these two lines passes the whole suite, because month 0 already rejects zero two lines below. Requirement "check for zero before unpacking" is actually carried by the month check, not by this. Fine to keep as documentation mirroring the firmware header's warning, but no test can fail on it, so do not count it as the defence.
@ -100,0 +108,4 @@version = "unknown" if self.firmware is None else version_label(self.firmware)label = version if self.build is None else f"{version} ({self.build:08x})"if self.firmware is None or not orders(self.firmware):return Running(label)Untested. This is the updater half of "ignored, not rejected", and no test reaches it —
test_control_api'sFakeLinkbuildsRunningby re-packing the label, so it never exercisesAnnounced.running(), andtest_firmware_versions.running()is another hand-rolled stand-in. A link-level test with a node announcing0xFFFFassertingupdate_availableis null would close it.Took all five, in
627a5e0.1. The bridge's own announce. You were right that this could not wait — and
going through it, the fix is smaller than the guard I was picturing. This
integration is released under the same CalVer the firmware is (
2026.09.3), soencode_versionis now justpacked(version) or DEVELOPMENT: a published copypacks its own release into that field, and a checkout, whose committed version
stays below every release by design, goes out as the development build it
actually is. The nibble scheme is gone rather than guarded, so there is one
packing on this bus and no version of it that can produce a date nobody
published. One extra case pinned while I was there:
tools/releasewill write2026.1.1as readily as2026.09.1, and both pack.2. README. Rewritten. It now says both sides are releases and that an
unorderable one is offered rather than hidden.
3.
link.py:111. The stand-in intest_firmware_versions.pyis gone;running()builds a realAnnouncedand asserts on both the label and theversionthat sorts, including the0xFFFFcase that reaches line 111.4. Absent is not development.
Node.firmwareisint | Nonenow, matchingAnnounced, anddiscovery.pypassesfields.get("firmware")straightthrough. A truncated announce reads as
unknown (…)on the device page ratherthan claiming a desk build, and there is a test for it in
test_discovery.py.5. The dead zero guard. Deleted. The month check is what rejects zero, and
the docstring now says so and says why reading the year first would report
2026.
On the wording. Fixed —
Outcome.UPDATEDnow says "running something it wasnot … what it came back on is never checked against the image served".
1164 tests, ruff and mypy clean.
All five taken, and the
encode_versionrewrite is the better answer than the guard I suggested.link.py:111is covered now, andNode.firmwarebeingint | Nonehas no holes: the three readers all guard it,discovery.py:251compares it inside a tuple whereNoneis fine, and everyNode(...)in the tree is keyword-constructed so moving the field pastbuildcannot swap anything. The release path is intact —tools/release/package.stamped_manifestwritesYYYY.MM.N, which packs, and the committed0.1.0does not. No doc anywhere still describes the nibble scheme.One line to fix, below. Nothing else from me.
@ -314,2 +314,2 @@assert encode_version("1.0.16") == 0assert encode_version("256.0.0") == 0def test_a_month_without_its_leading_zero_still_packs() -> None:"""`tools/release` writes 2026.1.1 as readily as it writes 2026.09.1."""This is not true, and it is the kind of claim the last round was about.
.forgejo/workflows/ci.yml:130builds the prefix withdate -u +%Y.%m, which always pads, andnext_versiononly appends the counter to it — so the release path cannot emit2026.1.1. Keep the test (leniency is right, since the version could be hand-set andpackedshould not care), but say what it is actually guarding rather than naming a tool that never produces the input.Reads right, and it is the one-line change it says it is. Nothing further from me.