Drop the MarigoldOS lineage from the firmware identity - #78
Merged
Conversation
📝 WalkthroughWalkthroughThe PR standardizes X3/X4 firmware identities to canonical ChangesFirmware identity migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant ComputerOrOEMFlasher
participant Slot0
participant InAppUpdater
participant Slot1
Operator->>ComputerOrOEMFlasher: flash migration image to slot 0
ComputerOrOEMFlasher->>Slot0: write and boot slot 0
Operator->>InAppUpdater: verify slot 0 and start normal update
InAppUpdater->>Slot1: populate slot 1
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/agents/release.md`:
- Around line 46-58: Update the published-artifact validation procedure to use
the requested v<version> release URL with curl --fail, and validate both
firmware-x4.bin and firmware-x3.bin using their corresponding IDENTITY_X4 and
IDENTITY_X3 values. Ensure each version or identity mismatch exits non-zero
rather than only printing an error, while retaining exact descriptor-field
comparisons.
In `@docs/FLASHING.md`:
- Around line 268-271: Update the anchor identity sentence near the CalendulaOS
command to say “the anchor's identity must match exactly,” completing the
sentence without changing the surrounding explanation.
In `@web/index.html`:
- Around line 611-619: Update the migration instructions in the paragraph near
the v0.5.0-or-older warning to explicitly state that the computer or SD flash
writes only slot 0; the user must boot slot 0 and complete one ordinary in-app
update to rewrite slot 1 before card updates work normally. Keep the existing
link and explanation while correcting the claim that a single flash completes
the migration.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7442424a-50bd-4d5a-90f2-b7b1dc70edc3
📒 Files selected for processing (5)
docs/FLASHING.mddocs/agents/release.mdfw/src/main.rsproto/src/ota.rsweb/index.html
Every build stamped `CalendulaOS <board> u<gen> (MarigoldOS)`, recording a fork lineage this project has diverged from. Drop the suffix and settle the format at `CalendulaOS <board> u<generation>` for both boards at once, so the board added next inherits a settled format rather than choosing one. The generation digit moves 1 -> 2 on both boards. Its documented rule is that it bumps whenever the update hand-off changes, and the rename is exactly that discontinuity: staged_image_is_installable is exact equality, so firmware carrying the old name refuses a renamed image outright. Leaving the digit at u1 would claim a continuity that does not exist. IDENTITY_SUFFIX and its strip_suffix step go with it; the parser handles the one current format. That costs nothing in refusal correctness, because legacy handling has never gone through the parser -- anchor_can_apply_update and staged_image_is_installable compare the raw descriptor field, and that is the path the new refusal tests for the superseded `u1 (MarigoldOS)` forms take. MarigoldOS card-format compatibility (durable, nvm, and the position mirrors) and the fork attribution in README/web are untouched: separate, live contracts. ## Crossing the break takes two flashes, and the docs now say so A u1 device refuses a u2 image by design, and refuses it *silently* -- the trigger is simply not the filename it looks for. The people most likely to read the site's "already running Calendula? use the in-app updater" line are exactly the ones it strands, and the symptom is "nothing happened", which reads as a broken download. Every USB/OEM route writes slot 0 only, so slot 1 keeps its u1 image and the reader may still boot it. Both FLASHING.md and the site now spell out the full crossing: flash the anchor, force the anchor boot with Back + Up, then run one ordinary in-app update to replace slot 1. Someone who stopped after the first step would reasonably conclude the fix had failed. The migration section also offered a full-chip erase as a one-step shortcut, sitting directly after three routes that write only the app slot at 0x10000. Erasing first takes the bootloader and partition table with the stale image and leaves an app-only flash unable to restore either -- a brick, in the one section a stranded user is most likely to follow. It is now a warning that names those routes, with the erase kept where it is genuinely useful: an unlocked bench unit paired with `full-flash*.bin` at 0x0. ## The post-release check accepted the artifact it exists to catch `strings | grep -E "CalendulaOS X4 u[0-9]+|<version>"` failed twice over. The alternation means either half satisfies it, and the identity half is a substring match -- so `CalendulaOS X4 u1 (MarigoldOS)` matches `CalendulaOS X4 u[0-9]+`, and a stale v0.5.0 artifact passed the very check this branch makes it fail. It now reads the descriptor fields at 48 and 80 and compares each exactly, the way prepare-release.sh already checks the image it builds: both boards against their own constant, fetched from the explicit `v<version>` tag rather than `latest`, with `curl -f` so a missing asset is an error rather than an HTML page silently failing the comparison, and `return 1` on every mismatch, since a check that only prints cannot be chained. The expected identity is read from `v<version>:proto/src/ota.rs` rather than the working tree, so both sides of the comparison come from the release being checked. Reading the tree instead fails a valid older release once main moves the identity on -- re-checking the published v0.5.0, which shipped `u1 (MarigoldOS)`, from a u2 tree -- and compares against the wrong expected value from a stale checkout. Exercised against real `save-image` builds of both boards, against the published v0.5.0 assets, against a forged `u1 (MarigoldOS)` artifact, and against an X3 image checked with the X4 constant. Every negative exits 1. `tools/check.sh all` passes.
chongfun
force-pushed
the
feature/ota-identity-rename
branch
from
August 8, 2026 04:39
7fcbfcf to
4e3124c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/agents/release.md`:
- Around line 66-67: Replace the head -1 pipelines in the ver and id assignments
with the pipefail-safe capture-and-truncate pattern already used by
tools/prepare-release.sh, preserving the existing descriptor offsets and
extracted values before the version and identity checks.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 31672b64-c38b-4e03-8392-95a4a45d297e
📒 Files selected for processing (3)
docs/FLASHING.mddocs/agents/release.mdweb/index.html
The verify snippet cut each field with `| head -1`, the one shape
prepare-release.sh deliberately avoids: it captures the NUL-to-newline
conversion and truncates with `${raw%%$'\n'*}` instead, because `head` exiting
early can SIGPIPE `tr` and trip pipefail. This block is documented as chainable,
so it should hold up under `set -euo pipefail` like the script it cites.
Latent rather than live: at a 32-byte field the data fits the pipe buffer, so
`tr` finishes writing before `head` exits and no SIGPIPE occurs. Verified that
`head -1` does not currently fail here. The value is consistency with the
sibling helper -- one extraction rule, one rationale, one place to fix.
Offsets 48 and 80 and both extracted values are unchanged; old and new forms
produce identical `ver` and `id` against the published v0.5.0 assets. Exercised
under `set -euo pipefail` (both boards pass, exit 0) and against an X3 image
checked with the X4 constant, which still fails.
`tools/check.sh all` passes.
chongfun
added a commit
that referenced
this pull request
Aug 14, 2026
Each of these either shipped or was absorbed, and in every case the reasoning worth keeping already has a durable home in code or docs. Recovery SHAs, since the point of writing them down is that nobody has to dig: board-identity-guard 2c200c1 shipped as #77 ota-identity-rename 160f76a shipped as #78 panel-controller-detection 733d27c shipped as #76 cache3-architecture 833221b absorbed by user-managed-library board-identity-guard's module header (fw/src/board_guard.rs) carries every non-obvious decision the PRD argued: only a confirmed mismatch refuses, it never touches the screen and why the earlier version that did was wrong, and it halts rather than reboot-loops. ota-identity-rename's one durable idea was why a board added later starts at u1 rather than inheriting u2, and that is already a doc comment on IDENTITY_X4 in proto/src/ota.rs -- next to the constant someone adding IDENTITY_STICKY will be editing, which is the right place for it. panel-controller-detection is deleted on stronger grounds than "it shipped". All six of its load-bearing probe details survive in display/src/epd/probe.rs, hal-ext/src/epd_probe.rs and docs/ARCHITECTURE.md:383-446, several of them more precisely -- the shipped rule 5 carries the actual LUT_VER values the PRD never had. Meanwhile five of its claims now contradict the code, and one is actively dangerous: its pass-2 reset table keys the confirming pass off a bare match, which is the exact mis-derivation display/src/epd/probe.rs:206-215 exists to warn against, and which resolves a real UC8279d as DefaultAssumed. Its Context paragraph also still teaches a timeout-based fallback that was never built. A document that would reintroduce a fixed bug is worse than no document. cache3-architecture's citation needed fixing before it could be deleted: the absorbing PRD pointed at 49b2c0a, a pre-rebase copy unreachable from any branch and eventually collectable. Repointed at 833221b, which is reachable, with the reason recorded so the same mistake is not made from the reflog next time. Left behind deliberately: uc8279-x3-driver and uc8179-x4-driver still reference the detection PRD, including one pointer to "rule 4" that was already wrong before this commit (the rules renumbered on the way into the code). Those two files are mid-audit; their references get fixed in that pass rather than being touched twice.
chongfun
added a commit
that referenced
this pull request
Aug 22, 2026
Each of these either shipped or was absorbed, and in every case the reasoning worth keeping already has a durable home in code or docs. Recovery SHAs, since the point of writing them down is that nobody has to dig: board-identity-guard 2c200c1 shipped as #77 ota-identity-rename 160f76a shipped as #78 panel-controller-detection 733d27c shipped as #76 cache3-architecture 833221b absorbed by user-managed-library board-identity-guard's module header (fw/src/board_guard.rs) carries every non-obvious decision the PRD argued: only a confirmed mismatch refuses, it never touches the screen and why the earlier version that did was wrong, and it halts rather than reboot-loops. ota-identity-rename's one durable idea was why a board added later starts at u1 rather than inheriting u2, and that is already a doc comment on IDENTITY_X4 in proto/src/ota.rs -- next to the constant someone adding IDENTITY_STICKY will be editing, which is the right place for it. panel-controller-detection is deleted on stronger grounds than "it shipped". All six of its load-bearing probe details survive in display/src/epd/probe.rs, hal-ext/src/epd_probe.rs and docs/ARCHITECTURE.md:383-446, several of them more precisely -- the shipped rule 5 carries the actual LUT_VER values the PRD never had. Meanwhile five of its claims now contradict the code, and one is actively dangerous: its pass-2 reset table keys the confirming pass off a bare match, which is the exact mis-derivation display/src/epd/probe.rs:206-215 exists to warn against, and which resolves a real UC8279d as DefaultAssumed. Its Context paragraph also still teaches a timeout-based fallback that was never built. A document that would reintroduce a fixed bug is worse than no document. cache3-architecture's citation needed fixing before it could be deleted: the absorbing PRD pointed at 49b2c0a, a pre-rebase copy unreachable from any branch and eventually collectable. Repointed at 833221b, which is reachable, with the reason recorded so the same mistake is not made from the reflog next time. Left behind deliberately: uc8279-x3-driver and uc8179-x4-driver still reference the detection PRD, including one pointer to "rule 4" that was already wrong before this commit (the rules renumbered on the way into the code). Those two files are mid-audit; their references get fixed in that pass rather than being touched twice.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Verification
Before requesting review, please confirm you have run the appropriate checks:
tools/check.sh fmtpassedtools/check.sh fastpassed (host Clippy and tests)tools/check.sh emulatorpassed (emulator tests and goldens)tools/check.sh firmwarepassed (firmware Clippy and release builds)tools/check.sh allpassed (required before the pull request is considered ready)Skipped checks
If you skipped any checks, please list them below and explain why:
Description
Every build stamped 'CalendulaOS u (MarigoldOS)', recording a fork lineage this project has diverged from. Drop the suffix and settle the format at 'CalendulaOS u' for all boards at once, so the board added next inherits a settled format rather than choosing one.
The generation digit moves 1 -> 2 on both boards. Its documented rule is that it bumps whenever the update hand-off changes, and the rename is exactly that discontinuity: staged_image_is_installable is exact equality, so firmware carrying the old name refuses a renamed image outright. Leaving the digit at u1 would claim a continuity that does not exist. Crossing the break needs a one-time USB or OEM reflash of the slot-0 anchor, now documented in docs/FLASHING.md.
IDENTITY_SUFFIX and its strip_suffix step go with it; the parser handles the one current format. That costs nothing in refusal correctness, because legacy handling has never gone through the parser -- anchor_can_apply_update and staged_image_is_installable compare the raw descriptor field, and that is the path the new refusal tests for the superseded 'u1 (MarigoldOS)' forms take.
MarigoldOS card-format compatibility (durable, nvm, and the position mirrors) and the fork attribution in README/web are untouched: separate, live contracts.
The post-release grep looked for 'CalendulaOS (MarigoldOS)', the pre-board form no build has stamped since identities gained a board and generation — so it silently matched nothing already. The rename makes that worse than useless: someone running it now gets no output and could reasonably read that as the stamp having gone missing.
Matches the board-and-generation form instead, with the generation left open so the next bump does not stale it again.
Summary by CodeRabbit
New Features
CalendulaOS <board> u2format.Documentation