OTA: fix the flash-area ID that bricks the board, add an integration test and a CI job by nateinaction · Pull Request #492 · Open-Source-Space-Foundation/proves-core-reference

nateinaction · 2026-08-03T20:54:09Z

Why

Reported from the bench: PREPARE_UPDATE bricks the cube on hmac-to-storage /
feature/expose-active-key-spis.

Reproduced by reading the generated devicetree headers. FlashWorker::REGION_NUMBER
was hardcoded to 2, with a comment asserting 0: bootloader, 1: slot0, 2: slot1.
That mapping is not a Zephyr guarantee — flash-area IDs are assigned by devicetree
dependency ordinal (a global counter in gen_defines.py over nodes sorted by
dep_ordinal), so adding a partition anywhere in the DT renumbers every area:

layout	ID 0	1	2	3
`main` (5 partitions)	mcuboot	slot0	slot1 ✅	slot2
branches that add `keystore_partition`	keystore	mcuboot	slot0 💀	slot1

On those branches boot_erase_img_bank(2) erases the running firmware. That is
the brick, and it happens the instant the command lands — before any image is
uplinked. Note this means OTA on main is not broken by this; if main also
bricked on that board, the board was already dead from the branch run.

What

Fix. Resolve the region from the devicetree label and make the failure mode
impossible to reintroduce silently:

constexpr static U8 REGION_NUMBER = PARTITION_ID(slot1_partition);
static_assert(PARTITION_OFFSET(slot1_partition) != DT_REG_ADDR(DT_CHOSEN(zephyr_code_partition)),
              "FlashWorker update region overlaps the running code partition");

Test. PROVESFlightControllerReference/test/int/ota_test.py, marked ota and
excluded from the default run:

PREPARE_UPDATE, then assert the board still answers commands — the direct
regression guard for this brick.
UPDATE_IMAGE_FROM without a preceding prepare is rejected.
Prepare → uplink → write → CONFIGURE_NEXT_BOOT TEST → reboot → assert the
reported ProjectVersion matches the version it verified is present in the
uplinked image bytes. The trial image is deliberately left unconfirmed.
The unconfirmed image reverts on the next reboot. This is the property
that makes a bad update survivable without a ground pass: an image that comes
up too broken to send CONFIRM_UPDATE undoes itself on the next reset.
Re-stage, CONFIRM_UPDATE, reboot again, and assert it stuck.

Both endings of step 6 of the update cycle are flight-critical — a good image
must stick and a bad one must roll itself back — so both are covered. Steps 4
and 5 stage the trial image twice from a single uplink, because re-flashing
the staging slot costs seconds while re-uplinking costs the better part of an
hour.

Build. make ota-test-image builds the distinctly-versioned staging image,
so the bench and CI produce it the same way. The OTA test only proves a swap
happened if the uplinked image reports a different project_version than the
running one — uplinking the running build makes the assertion vacuously true.
Project version comes from git describe, so a throwaway tag renames the build
and changes nothing else: same sources, same config, same signing key. The tag
is deleted again on the way out, including on failure, and bootable.uf2 /
bootable.signed.hex keep pointing at the build you flash.

The target exists because doing this by hand fails silently two ways, and both
cost a bench session to find:

fprime generates versions/version.{hpp,cpp,json} from a cmake custom command
with no declared inputs, so ninja considers them up to date as soon as they
exist — a rebuild after a new tag re-links a binary carrying the old version
string. They have to be deleted first.
git describe --tags does not prefer the newest tag when several point at
HEAD. It picked an alphabetically-lower stale tag over a just-created one.

CI. The OTA test lives in its own ota workflow, on a 10:00 UTC daily
schedule plus manual dispatch — it ties up the single integration cube for the
better part of an hour, so it does not belong in the PR run, and the previous
opt-in triggers (a workflow_dispatch input and a test-ota PR label) meant it
never ran on its own. The build job is now a reusable build workflow shared by
ci and ota, and it gets the staging image from the same make ota-test-image
target the bench uses. Both flash slots are erased/reflashed on the way in and
out so a leftover staged image cannot swap itself in mid-job or leak into the
next job on the runner.

The job timeout is 180 minutes. The suite now writes the staging slot twice and
reboots the board four times, and uplink runtime is not stable — see below.

Docs. The README OTA section was working notes ending in the advice to try
flipping the region number by hand; replaced with the procedure and a
failure-mode guide.

Bench verification

5 passed in 27:04 on hardware. Bench is a V5e flight controller board plus
face, no battery board, on /dev/cu.usbmodem1101 with the debug probe on
/dev/cu.usbmodem102. The run mirrored the CI job step for step: make ota-test-image -> GDS -> sync sequence number -> format filesystem -> reset ->
sync -> make test-integration TEST=ota_test.py FILTER=ota PYTEST_ARGS="--ota-image=ota-image/zephyr.signed.bin".

The swap is proven, not vacuous: the board was flashed with one build and
uplinked a differently tagged one. It reported the uplinked project_version
after the trial boot, went back to the pre-update version on the
unconfirmed reboot, and held the uplinked version after the confirmation
reboot. Flight events: PrepareUpdateSucceeded -> FileReceived /update/zephyr.signed.bin -> UpdateSucceeded (the CRC is checked flight-side
before a byte is written, so the uplink was lossless) -> SetNextBoot TEST ->
ConfirmBoot.

make ota-test-image was exercised twice and cleaned up its throwaway tag both
times.

Answers to the three open questions above:

Uplink timing is not stable. 726 KB at the fprime-gds.yml defaults is
~3560 chunks and ~25 minutes on the paper rate, which is what the green run
did. An earlier attempt on the same bench and image spent ~70 minutes in
uplink alone, apparently in the stall-and-restart path. The 90 minute per-file
ceiling absorbed it, but the 150 minute job timeout would not have, hence 180.
PREPARE_UPDATE fits easily. The 1 MB erase completes in a few seconds,
well inside the 120 s allowed.
The OpenOCD erase addresses are correct in situ. flash erase_address 0x10200000 0x100000 erases exactly slot1 and leaves the running image alive.

Three flight/GDS fixes were needed to get there, one per commit:

--file-uplink-cooldown below 0.400 is not safe on this link, and fails
silently. There is no flight-side back-pressure: ProvesRouter takes each
file packet from a pool of commsFileBuffCount (5) buffers and drops the
packet when the pool is empty, while the ground keeps streaming. The transfer
still emits FileReceived at full length and only the CRC reveals that
everything past the first few packets was discarded (PacketOutOfOrder
reporting "packet N after packet 5" is the tell). Measured against a 32 KB
file with fileManager.CalculateCrc: 0.100 corrupts, 0.400 and 1.000 are
clean. The ota_uplink_cooldown workflow input should not be lowered until
that pool grows. The test now pins the chunk size and cooldown itself,
because IntegrationTestAPI builds its own StandardPipeline and never reads
fprime-gds.yml -- its 256 B chunk default also overflows the 248 B frame.
The GDS could emit a duplicate TC sequence number, which the flight side
correctly rejected as a replay. The file-uplink encoding chain runs in a
different process from the one framing commands, and each AuthenticateFramer
cached the number rather than reading it per frame under a lock.
A dropped uplink Start or handshake stranded the transfer. A Start packet
carries no opcode so it can never match PacketBypasser's allowlist; if it is
dropped the uplinker sits until its 20 s timeout and gives up. The test now
watches the uplinker's seek for progress and restarts a stalled transfer.

Bench hazard worth knowing before you run this

Two host-side failures during bring-up looked exactly like OTA product defects
and neither is one:

A stale fprime_gds.executables.comm from an earlier session fighting for
the serial port. It presents as Checksum validation failed with empty
event.log / channel.log and every test erroring in setup, while a raw
pyserial read of the same port shows the board streaming happily. Check
pgrep -f fprime_gds.executables.comm returns exactly one PID.
The swap reboot can wedge the macOS CDC driver instance. The board still
enumerates (ioreg shows PROVES Flight Control Board v5e, so the app booted
past USB init) but a fresh open() on the port returns zero bytes, and
OpenOCD simultaneously fails to read USB string descriptors for every device.
It surfaces as board did not come back after the swap reboot. Only a
physical replug clears it; the identical swap then succeeded in 46 s and four
more times in the green run. Note the replug power-cycles the board, so an
unconfirmed trial image reverts — leaving the board on the pre-update version,
which reads as "the swap never happened".

To iterate on the back half without re-paying the uplink: the image stays at
/update/zephyr.signed.bin across reboots, so stage_image + arm_trial_boot

reboot re-runs the swap in ~46 s.

Also verified locally: make build is clean with the static_assert passing;
slot1_partition resolves to partition@200000; the test's CRC is
byte-identical to tools/bin/calculate-crc.py.

Still unexercised by this run: the ota workflow itself, which has not yet
executed end to end on the runner.

Worth landing #495 alongside this. That fixes a latent ComQueue arming race that
makes a build boot permanently silent, and the outcome is deterministic per
binary -- this test is unusually exposed to it because it runs two different
binaries (the flashed one and the uplinked one), so both have to win the race
independently. A build that lost would surface here as "board did not come back
after the swap reboot", i.e. as an OTA failure rather than as the comms bug it
is. The bench run above did not hit it.

Out of scope, noted

Updater.cpp (in the fprime-extras submodule) guards its busy flag with
compare_exchange_weak, which is allowed to fail spuriously. Today that only
means an occasional bogus BUSY rejection, but it should be _strong.

… literal PREPARE_UPDATE erases FlashWorker::REGION_NUMBER, which was hardcoded to 2 with a comment claiming that meant slot1. Zephyr assigns flash-area IDs by devicetree dependency ordinal, so adding any partition anywhere in the devicetree renumbers every area. On the current main layout 2 is slot1 and OTA works; on a tree that adds one more partition (a keystore partition, for instance) 2 becomes slot0_partition, and PREPARE_UPDATE erases the firmware that is executing -- bricking the board until it is reflashed over UF2 or SWD. Resolve the region from the slot1_partition label instead, and static_assert that it never coincides with the running code partition. Adds an OTA integration test covering the full cycle -- prepare, uplink, UPDATE_IMAGE_FROM, CONFIGURE_NEXT_BOOT TEST, swap reboot, CONFIRM_UPDATE -- with a first case that asserts the board still answers commands after PREPARE_UPDATE, which is the direct regression guard for the brick. The suite is marked `ota` and excluded from the default integration run since it erases a flash slot, uplinks ~1.4 MB and reboots the board. Rewrites the README OTA section, which was working notes ending in the advice to try flipping the region number by hand.

Runs the full over-the-air cycle on the integration cube: erase the staging slot, uplink a signed image, write it to slot1, arm a TEST boot, reboot through the MCUboot swap, and confirm. To prove a swap actually happened rather than that the commands merely returned OK, the build job produces a second image under a throwaway `ota-ci-<run_id>` git tag. Project version comes from `git describe`, so that image reports a distinct version and nothing else about it changes; the test asserts the board reports that version after the reboot. The version files have to be deleted before the second build -- fprime generates them from a cmake custom command with no declared inputs, so ninja otherwise keeps the stale string. The job is opt-in (workflow_dispatch input or the `test-ota` PR label): it holds the single integration cube for most of an hour, since a 1.4 MB uplink at the default file-uplink cooldown takes ~48 minutes on its own. `GDS_EXTRA_ARGS` on `make gds-integration` is the lever to speed that up, exposed as a workflow input. Both slots are erased/reflashed on the way in and on the way out, so a leftover staged image from a previous run cannot swap itself in mid-job and the next job on the runner starts from the artifact build.

coderabbitai · 2026-08-03T20:54:17Z

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 21714ccc-eb43-43f1-9c9e-9f7596f25003

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

The OTA job is gated on the `test-ota` label, but `on: pull_request` without an explicit `types:` list does not include `labeled`, so adding the label to an open PR triggered nothing. Spell out the default types plus `labeled`.

Every integration job ends with "Power Off Satellite", and the OTA job's first hardware action is an OpenOCD erase rather than the flash-firmware composite that powers the board up elsewhere. The erase therefore ran against a dark board.

There is one integration cube. While iterating on the OTA job, leaving these enabled means every attempt queues behind a full uart+radio suite. Restore before merging.

The CI run showed PREPARE_UPDATE dropped by the TC replay window (SequenceNumberInvalid: Received=37, LastAccepted=37) because the GDS emitted the same sequence number for the session's first two commands. prepare_update used a raw send_command with no retry, so the drop burned the full 120 s event timeout and failed test_01 outright. Retry on the component's "started" event rather than its completion, so a retry cannot re-issue work already underway, and match the outcome against both the success and failure events so a failed write reports immediately instead of waiting out the 15-minute ceiling. Also drops the claim that the erase outruns the ack window: it measured 2.8 s on the CI cube.

The GDS runs the file-uplink encoding chain in a separate process from the one framing commands, and each process builds its own AuthenticateFramer. The instances have no way to learn what sequence number the other has used beyond the shared file, and the old read-modify-write was neither atomic nor consulted per frame: the constructor cached the file's value and each frame emitted the cached number before incrementing. Two frames built at nearly the same time in different processes could therefore emit the same number, which the flight side correctly rejects as a replay (SequenceNumberInvalid). Derive every frame's number from the file under an flock, guarded within the process by a mutex. The file is opened "a+" so the create-if-missing case falls out of the same path instead of needing its own FileNotFoundError arm.

writeSequenceNumber persists on every accepted TC frame, and file-uplink Data packets are TC frames too, so a 726 KB image means ~3600 of these writes interleaved with FileUplink's own writes to a different file on the same disk. Opening and closing the file each time forces the flash-disk driver's single-page write-back cache to commit and reload per frame, thrashing against those uplink writes. Hold the handle for the component's lifetime and seek to 0 before each write, turning each persist into a seek plus write on an already-open file. The seek is required because writeToFile writes at the current position, which for a handle that stays open is just past the previous write.

fprime_gds's FileUplinker has no retry of its own. A Start packet is a bare FW_PACKET_FILE frame with no opcode, so it can never match PacketBypasser's allowlist; if it lands on the TC replay window it comes back unauthenticated and ProvesRouter drops it silently, after which the uplinker sits until its 20 s handshake timeout expires and gives up. Poll the active TransmitFile's seek for progress and re-enqueue when it stalls, cancelling both ends first so the flight-side receiver abandons its partial file. The uplinker's own timeout may already have called finish(), so retry only when it is not IDLE -- a second finish() raises FileNotFoundError on the unlinked up_store copy. Also reapply the file-uplink chunk size and cooldown here. IntegrationTestAPI builds its own StandardPipeline and never reads fprime-gds.yml, so it defaults to a 256 B chunk that overflows the 248 B frame, and to a cooldown fast enough to overrun the flight side. There is no back-pressure on file uplink: packets that arrive with the ProvesRouter buffer pool empty are dropped while the ground keeps streaming, and the transfer still reports FileReceived at full length -- only the CRC reveals the loss. 0.400 matches fprime-gds.yml and is the lowest value measured clean on the bench. Ignore ota-image/, where the locally built staging image is kept.

The OTA job ties up the single integration cube for the better part of an hour, so it was opt-in via a workflow_dispatch input or the `test-ota` PR label. Neither gets it run regularly, and the label trigger forced `ci` to carry OTA-specific inputs and a non-default `labeled` PR trigger. Give it its own `ota` workflow on a 10:00 UTC daily schedule plus manual dispatch, and extract the build job into a reusable `build` workflow so the two callers share it rather than duplicating the Zephyr setup. The OTA staging image is now gated on a `build_ota_image` input.

…-core-reference into ota-integration-test

…arget The test proved a confirmed update sticks, but not the property that makes a bad update survivable without a ground pass: an image that never sends CONFIRM_UPDATE must be rolled back by MCUboot on its own. Both endings are flight-critical, so cover both. test_03 now leaves the trial image unconfirmed, test_04 reboots out of it and asserts the board is back on the pre-update version, and test_05 re-stages, confirms, and reboots again. The trial image is staged twice from a single uplink because re-flashing the staging slot costs seconds while re-uplinking costs the better part of an hour. The image build moves out of build.yaml into an `ota-test-image` target so the bench and CI produce it the same way. Doing it by hand fails silently two ways: fprime only writes versions/version.json when it is missing, so a plain rebuild after a new tag re-links the old version string, and `git describe --tags` does not prefer the newest tag when several point at HEAD. The target also deletes its throwaway tag on the way out, including on failure, and leaves bootable.* pointing at the build you flash. Bench: 5 passed in 27:04 on a V5e flight controller board plus face, no battery board. The swap is proven rather than vacuous -- the board went from ota-0804181900-dirty to the freshly built ota-0804201828-dirty and back again on the revert. Raise the ota job timeout to 180 minutes: the suite now writes the staging slot twice and reboots four times, and uplink runtime is not stable -- one bench attempt spent ~70 minutes in uplink against ~25 in the green run.

nateinaction added 2 commits August 3, 2026 22:43

github-project-automation Bot added this to V1.X.X Aug 3, 2026

nateinaction added the test-ota Run the hardware over-the-air update integration job on this PR label Aug 3, 2026

nateinaction and others added 5 commits August 3, 2026 22:59

ci: fire pull_request runs on label changes

95f45cb

The OTA job is gated on the `test-ota` label, but `on: pull_request` without an explicit `types:` list does not include `labeled`, so adding the label to an open PR triggered nothing. Spell out the default types plus `labeled`.

ci: temporarily disable the uart and radio integration jobs

6048c89

There is one integration cube. While iterating on the OTA job, leaving these enabled means every attempt queues behind a full uart+radio suite. Restore before merging.

set downlink repeater channels

39f4115

nateinaction added 7 commits August 4, 2026 18:33

Revert changes to ci.yaml

23c6de0

Merge branch 'main' of github.com:open-source-space-foundation/proves…

acd6f86

…-core-reference into ota-integration-test

Navigation Menu

Uh oh!

OTA: fix the flash-area ID that bricks the board, add an integration test and a CI job - #492

OTA: fix the flash-area ID that bricks the board, add an integration test and a CI job#492
nateinaction wants to merge 14 commits into
mainfrom
ota-integration-test

nateinaction commented Aug 3, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Aug 3, 2026 •

edited

Loading

Review skipped

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

nateinaction commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What

Bench verification

Bench hazard worth knowing before you run this

Related

Out of scope, noted

Uh oh!

coderabbitai Bot commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nateinaction commented Aug 3, 2026 •

edited

Loading

coderabbitai Bot commented Aug 3, 2026 •

edited

Loading