From 1d5427f9127a72608bbb8fe6346d0a10f2625e10 Mon Sep 17 00:00:00 2001 From: Nate Gay Date: Mon, 3 Aug 2026 22:43:28 +0200 Subject: [PATCH 01/11] fix(FlashWorker): resolve the OTA staging slot from devicetree, not a 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. --- Makefile | 2 +- .../Components/FlashWorker/FlashWorker.hpp | 14 +- .../Components/FlashWorker/docs/sdd.md | 22 +- .../test/int/conftest.py | 13 + .../test/int/ota_test.py | 290 ++++++++++++++++++ README.md | 89 +++++- docs-site/components/FlashWorker.md | 22 +- pytest.ini | 1 + 8 files changed, 436 insertions(+), 17 deletions(-) create mode 100644 PROVESFlightControllerReference/test/int/ota_test.py diff --git a/Makefile b/Makefile index d78000ab..8a451fee 100644 --- a/Makefile +++ b/Makefile @@ -209,7 +209,7 @@ test-unit: ## Run unit tests cmake --build build-gtest ctest --test-dir build-gtest -FILTER ?= not sync_sequence_number and not format_filesystem and not provision_key +FILTER ?= not sync_sequence_number and not format_filesystem and not provision_key and not ota .PHONY: test-integration test-integration: uv ## Run integration tests (set TEST= or pass test targets) diff --git a/PROVESFlightControllerReference/Components/FlashWorker/FlashWorker.hpp b/PROVESFlightControllerReference/Components/FlashWorker/FlashWorker.hpp index 657fdca2..58739a8d 100644 --- a/PROVESFlightControllerReference/Components/FlashWorker/FlashWorker.hpp +++ b/PROVESFlightControllerReference/Components/FlashWorker/FlashWorker.hpp @@ -8,12 +8,24 @@ #define Update_FlashWorker_HPP #include "Os/File.hpp" #include "PROVESFlightControllerReference/Components/FlashWorker/FlashWorkerComponentAc.hpp" +#include #include +#include namespace Components { class FlashWorker final : public FlashWorkerComponentBase { public: - constexpr static U8 REGION_NUMBER = 2; // 0: bootloader, 1: slot0, **2: slot1** + //! MCUboot secondary slot: where an uploaded image is staged before the bootloader swaps it in. + //! + //! Resolved from the devicetree label, never hardcoded. Zephyr hands out flash-area IDs in + //! devicetree dependency-ordinal order, so adding a partition anywhere in the DT renumbers every + //! area. A hardcoded ID silently starts pointing at a different partition -- and erasing the + //! wrong one here wipes the running firmware. + constexpr static U8 REGION_NUMBER = PARTITION_ID(slot1_partition); + + //! Guard the failure above: the staging region must never be the slot we are executing from. + static_assert(PARTITION_OFFSET(slot1_partition) != DT_REG_ADDR(DT_CHOSEN(zephyr_code_partition)), + "FlashWorker update region overlaps the running code partition"); enum Step { IDLE, PREPARE, UPDATE }; // ---------------------------------------------------------------------- // Component construction and destruction diff --git a/PROVESFlightControllerReference/Components/FlashWorker/docs/sdd.md b/PROVESFlightControllerReference/Components/FlashWorker/docs/sdd.md index cbbee1ae..324cafe3 100644 --- a/PROVESFlightControllerReference/Components/FlashWorker/docs/sdd.md +++ b/PROVESFlightControllerReference/Components/FlashWorker/docs/sdd.md @@ -2,8 +2,26 @@ Performs long-running operations for the flash subsystem. The flash worker is responsible for handling the actual operations needed for flight-software update specific to the Zephyr flash API. - - +## Update region + +Every flash operation here targets `REGION_NUMBER`, the MCUboot **secondary +slot** — the staging area an uploaded image is written to before the bootloader +swaps it into the primary slot. It is resolved at compile time from the +devicetree label: + +```cpp +constexpr static U8 REGION_NUMBER = PARTITION_ID(slot1_partition); +``` + +**Never replace this with a literal number.** Zephyr assigns flash-area IDs by +devicetree *dependency ordinal*, not by address or declaration order, so adding +a partition anywhere in the devicetree renumbers every area. A hardcoded ID +therefore starts silently pointing at a different partition — and when that +partition is `slot0_partition`, `prepareImage` erases the firmware that is +currently executing and the board is bricked until it is reflashed over UF2 or +SWD. A `static_assert` in `FlashWorker.hpp` fails the build if the update region +ever resolves to the running code partition, and +`PROVESFlightControllerReference/test/int/ota_test.py` covers it on hardware. ## Usage Examples diff --git a/PROVESFlightControllerReference/test/int/conftest.py b/PROVESFlightControllerReference/test/int/conftest.py index 9dec6bac..cff8b452 100644 --- a/PROVESFlightControllerReference/test/int/conftest.py +++ b/PROVESFlightControllerReference/test/int/conftest.py @@ -100,6 +100,19 @@ def pytest_addoption(parser: pytest.Parser) -> None: "wedges the face I2C bus for the rest of the session. This option drops " "SafeModeEntryVoltage to 0 before each test so the auto-entry never fires.", ) + parser.addoption( + "--ota-image", + default="build-artifacts/zephyr.signed.bin", + help="Signed MCUboot image the OTA test uplinks and swaps to. Defaults to " + "the artifact produced by 'make build'.", + ) + parser.addoption( + "--ota-expect-version", + default=None, + help="Project version string the board must report after the OTA swap. " + "Defaults to the project_version in build-fprime-automatic-zephyr/versions/" + "version.json, which is only right when --ota-image came from that build.", + ) parser.addoption( "--bare-flight-controler-board", action="store_true", diff --git a/PROVESFlightControllerReference/test/int/ota_test.py b/PROVESFlightControllerReference/test/int/ota_test.py new file mode 100644 index 00000000..7afe3209 --- /dev/null +++ b/PROVESFlightControllerReference/test/int/ota_test.py @@ -0,0 +1,290 @@ +""" +ota_test.py: + +End-to-end integration test for over-the-air (OTA) firmware updates. + +The satellite boots through MCUboot in swap mode. Flash is split into a +bootloader partition, a primary slot (``slot0_partition``, the running image) +and a secondary slot (``slot1_partition``, the staging area). An update is: + +1. ``PREPARE_UPDATE`` -- erase the secondary slot. +2. file uplink -- put the new signed image on the on-board filesystem. +3. ``UPDATE_IMAGE_FROM``-- copy that file into the secondary slot, CRC-checked. +4. ``CONFIGURE_NEXT_BOOT TEST`` -- mark the staged image for a one-shot trial. +5. reboot -- MCUboot swaps the slots and runs the new image. +6. ``CONFIRM_UPDATE`` -- make the swap permanent. Without this, the *next* + reboot reverts to the previous image. + +The whole module is marked ``ota`` and is excluded from the default integration +run: it erases a flash slot, uplinks a ~1.4 MB file, and reboots the board. +Run it deliberately: + + make test-integration TEST=ota FILTER=ota + +By default it re-flashes the image in ``build-artifacts/zephyr.signed.bin``, +i.e. the build sitting in the working tree. Point somewhere else with +``--ota-image``. The test proves a real swap happened by reading the project +version out of the image it uplinks and asserting the board reports that same +version after the reboot -- so uplinking a *different* build than the running +one makes the test strictly stronger. +""" + +import json +import time +import zlib +from datetime import datetime +from pathlib import Path + +import pytest +from common import cmdDispatch, proves_send_and_assert_command +from fprime_gds.common.data_types.event_data import EventData +from fprime_gds.common.models.serialize.time_type import TimeType +from fprime_gds.common.testing_fw.api import IntegrationTestAPI + +# OTA severs the RF link (it reboots the board), so it only makes sense on UART. +pytestmark = [pytest.mark.uart_only, pytest.mark.ota] + +updater = "Update.updater" +worker = "Update.worker" +version = "CdhCore.version" +fileManager = "FileHandling.fileManager" + +# Directory on the satellite filesystem that holds the staged image. +UPDATE_DIR = "/update" + +# Uplinking ~1.4 MB in file-uplink packets takes minutes even over UART, and the +# flash write that follows walks the image in CONFIG_IMG_BLOCK_BUF_SIZE (512 B) +# chunks with a 5 ms settle delay per chunk. +UPLINK_TIMEOUT_S = 45 * 60 +IMAGE_WRITE_TIMEOUT_S = 15 * 60 +# Cold reset plus an MCUboot swap of two 1 MB slots, then a full FSW boot. +SWAP_REBOOT_TIMEOUT_S = 180 +# Erasing the whole 1 MB staging slot outruns the command-ack window that +# proves_send_and_assert_command allows, so PREPARE_UPDATE is awaited by event. +PREPARE_TIMEOUT_S = 120 + +# Svc.Version declares its version strings as `string size 40`, so anything +# longer is truncated in flight before it reaches telemetry. +VERSION_STRING_SIZE = 40 + + +def prepare_update(fprime_test_api: IntegrationTestAPI) -> None: + """Erase the staging slot and wait for the erase to report success.""" + fprime_test_api.clear_histories() + fprime_test_api.send_command(f"{updater}.PREPARE_UPDATE") + assert ( + fprime_test_api.await_event( + f"{updater}.PrepareUpdateSucceeded", timeout=PREPARE_TIMEOUT_S + ) + is not None + ), "PREPARE_UPDATE did not report success" + + +def fprime_crc32(path: Path) -> int: + """CRC32 of a file in the convention ``Os::File::calculateCrc`` uses. + + F Prime seeds with 0xFFFFFFFF and does *not* apply the trailing XOR that + zlib does, so the flight-side value is the complement of ``zlib.crc32``. + This matches ``tools/bin/calculate-crc.py``; keeping the two in agreement is + what stops ``UPDATE_IMAGE_FROM`` from failing with IMAGE_CRC_MISMATCH. + """ + crc = 0 + with open(path, "rb") as handle: + while chunk := handle.read(8192): + crc = zlib.crc32(chunk, crc) + return ~crc & 0xFFFFFFFF + + +def read_project_version(fprime_test_api: IntegrationTestAPI) -> str: + """Ask the running image which project version it is.""" + proves_send_and_assert_command(fprime_test_api, f"{version}.VERSION", ["PROJECT"]) + event: EventData = fprime_test_api.assert_event( + f"{version}.ProjectVersion", timeout=10 + ) + return str(event.args[0].val) + + +def assert_board_responsive(fprime_test_api: IntegrationTestAPI) -> None: + """Fail loudly if the satellite has stopped answering commands.""" + proves_send_and_assert_command(fprime_test_api, f"{cmdDispatch}.CMD_NO_OP") + + +@pytest.fixture(scope="module") +def ota_image(request: pytest.FixtureRequest) -> Path: + image = Path(request.config.getoption("--ota-image")) + if not image.is_file(): + pytest.skip( + f"OTA image {image} not found -- run 'make build' or pass --ota-image" + ) + return image + + +@pytest.fixture(scope="module") +def expected_version(request: pytest.FixtureRequest, ota_image: Path) -> str: + """The project version the board must report once ``ota_image`` is running. + + Taken from ``--ota-expect-version`` when given, otherwise from the + ``version.json`` the F Prime build writes next to the image. Either way it + is checked against the image bytes, so a stale version.json cannot quietly + turn the post-reboot assertion into a no-op. + """ + override = request.config.getoption("--ota-expect-version") + if override: + candidate = override + else: + version_json = ( + Path("build-fprime-automatic-zephyr") / "versions" / "version.json" + ) + if not version_json.is_file(): + pytest.skip( + f"{version_json} not found and --ota-expect-version not given; " + "cannot tell which version the uplinked image should report" + ) + candidate = json.loads(version_json.read_text())["project_version"] + + if candidate.encode() not in ota_image.read_bytes(): + pytest.skip( + f"project version {candidate!r} does not appear in {ota_image}; the " + "version metadata and the image are from different builds" + ) + return candidate[: VERSION_STRING_SIZE - 1] + + +def test_01_prepare_update_keeps_the_board_alive( + fprime_test_api: IntegrationTestAPI, start_gds +): + """PREPARE_UPDATE must erase the staging slot, not the running one. + + The flash area erased here is resolved from the ``slot1_partition`` + devicetree label. Zephyr numbers flash areas by devicetree dependency + ordinal, so a hardcoded ID silently retargets whenever a partition is added + anywhere in the DT -- which is how PREPARE_UPDATE once erased the live + firmware and bricked the board. Asserting the board still answers commands + afterwards is the regression guard for exactly that. + """ + prepare_update(fprime_test_api) + + assert_board_responsive(fprime_test_api) + assert read_project_version(fprime_test_api), "board lost its version telemetry" + + +def test_02_update_image_without_prepare_is_rejected( + fprime_test_api: IntegrationTestAPI, start_gds +): + """A write that was not preceded by a successful PREPARE_UPDATE must fail. + + FlashWorker tracks the last successful step; writing into a slot that was + never erased would produce a corrupt image that MCUboot may still try to + swap in. + """ + # test_01 left the worker in the PREPARE state, so consume it with a write + # that cannot succeed (the file does not exist), then retry from IDLE. + fprime_test_api.send_command( + f"{updater}.UPDATE_IMAGE_FROM", [f"{UPDATE_DIR}/does-not-exist.bin", "0"] + ) + fprime_test_api.await_event(f"{updater}.UpdateFailed", timeout=30) + + fprime_test_api.clear_histories() + fprime_test_api.send_command( + f"{updater}.UPDATE_IMAGE_FROM", [f"{UPDATE_DIR}/does-not-exist.bin", "0"] + ) + assert ( + fprime_test_api.await_event(f"{worker}.NoImagePrepared", timeout=30) is not None + ), "unprepared write was not rejected" + + assert_board_responsive(fprime_test_api) + + +def test_03_full_ota_cycle( + fprime_test_api: IntegrationTestAPI, + start_gds, + ota_image: Path, + expected_version: str, +): + """Stage an image, swap to it across a reboot, and confirm it.""" + before = read_project_version(fprime_test_api) + crc32 = fprime_crc32(ota_image) + destination = f"{UPDATE_DIR}/{ota_image.name}" + + # 1. Erase the staging slot. + prepare_update(fprime_test_api) + + # 2. Uplink the signed image. CreateDirectory is best-effort: /update may + # already exist from an earlier run, and fileManager errors on that. + fprime_test_api.send_command(f"{fileManager}.CreateDirectory", [UPDATE_DIR]) + time.sleep(1) + fprime_test_api.clear_histories() + fprime_test_api.uplink_file(str(ota_image), destination) + assert ( + fprime_test_api.await_event("FileReceived", timeout=UPLINK_TIMEOUT_S) + is not None + ), f"uplink of {ota_image} to {destination} never completed" + + # 3. Copy it into the staging slot. The CRC is checked flight-side before a + # single byte is written, so a mismatch here means the uplink was lossy. + fprime_test_api.clear_histories() + fprime_test_api.send_command( + f"{updater}.UPDATE_IMAGE_FROM", [destination, str(crc32)] + ) + assert ( + fprime_test_api.await_event( + f"{updater}.UpdateSucceeded", timeout=IMAGE_WRITE_TIMEOUT_S + ) + is not None + ), "image write to the staging slot did not succeed" + + # 4. Arm the one-shot trial boot. TEST rather than PERMANENT so that a bad + # image reverts on the following reboot instead of stranding the board. + fprime_test_api.clear_histories() + proves_send_and_assert_command( + fprime_test_api, f"{updater}.CONFIGURE_NEXT_BOOT", ["TEST"] + ) + fprime_test_api.assert_event(f"{updater}.SetNextBoot", timeout=10) + + # 5. Reboot and let MCUboot perform the swap. + start: TimeType = TimeType().set_datetime( + datetime.now(), time_base=TimeType.TimeBase("TB_DONT_CARE") + ) + fprime_test_api.send_command("ReferenceDeployment.resetManager.COLD_RESET") + assert ( + fprime_test_api.await_event( + f"{version}.FrameworkVersion", start=start, timeout=SWAP_REBOOT_TIMEOUT_S + ) + is not None + ), "board did not come back after the swap reboot" + + # 6. The swapped-in image must be the one we uplinked. + after = read_project_version(fprime_test_api) + assert after == expected_version, ( + f"running version {after!r} after the swap, expected {expected_version!r} " + f"(was {before!r} before the update)" + ) + + # 7. Make it permanent. Skipping this would revert on the next reboot. + fprime_test_api.clear_histories() + proves_send_and_assert_command(fprime_test_api, f"{updater}.CONFIRM_UPDATE") + fprime_test_api.assert_event(f"{updater}.ConfirmBoot", timeout=10) + + +def test_04_survives_a_second_reboot(fprime_test_api: IntegrationTestAPI, start_gds): + """After CONFIRM_UPDATE the image sticks -- no revert on the next boot. + + This is the assertion that separates a confirmed update from a trial one: an + unconfirmed TEST image is rolled back by MCUboot here. + """ + expected = read_project_version(fprime_test_api) + + start: TimeType = TimeType().set_datetime( + datetime.now(), time_base=TimeType.TimeBase("TB_DONT_CARE") + ) + fprime_test_api.send_command("ReferenceDeployment.resetManager.COLD_RESET") + assert ( + fprime_test_api.await_event( + f"{version}.FrameworkVersion", start=start, timeout=SWAP_REBOOT_TIMEOUT_S + ) + is not None + ), "board did not come back after the confirmation reboot" + + assert read_project_version(fprime_test_api) == expected, ( + "image reverted after reboot -- CONFIRM_UPDATE did not take" + ) diff --git a/README.md b/README.md index d3338976..a2806a94 100644 --- a/README.md +++ b/README.md @@ -242,23 +242,90 @@ You can control the specific command lists of the satellite by writing a sequenc ## Conducting Over the Air Updates -When you run the gds, +Updates are performed by MCUboot in swap mode. Flash is divided into a bootloader +partition, a **primary slot** (`slot0_partition`, where the running firmware +lives) and a **secondary slot** (`slot1_partition`, the staging area). You uplink +a new signed image to the on-board filesystem, copy it into the secondary slot, +mark it for boot, and reboot — MCUboot swaps the two slots and runs the new +image. If the new image is booted in `TEST` mode and never confirmed, the next +reboot swaps back. -``` fprime-gds --file-uplink-cooldown 0.8``` +Both slots are 1 MB (see +`boards/bronco_space/proves_flight_control_board_v5/proves_flight_control_board_v5.dtsi`), +so an image must fit in 1 MB minus the MCUboot trailer. -Now to fileuplink and update other parts. Upload Zephyr.signed.bin using the file uplink file +### Prerequisites +- The board is running the MCUboot bootloader (see [Bootloader (MCUBoot)](#bootloader-mcuboot)). +- `keys/proves.pem` is the key the installed bootloader was built with. An image + signed with a different key will be rejected by MCUboot and the board will + fall back to the old image. +- A signed image to install: `make build` writes `build-artifacts/zephyr.signed.bin`. -1. prepare image -2. update from (pass in the path) -3. configure_next_boot = test +### Procedure -to find the crc ./tools/bin/calculate-crc.py build-artifacts/zephyr.signed.bin +Start the GDS with a file-uplink cooldown, since the image is large: -(either power cycle or run the reboot command, should reboot and come into that old version of software, check the version telemetry) +```shell +fprime-gds --file-uplink-cooldown 0.8 +``` + +1. **Compute the image CRC.** Flight software verifies it before writing a + single byte, so this must be the value the flight-side CRC produces: + + ```shell + ./tools/bin/calculate-crc.py build-artifacts/zephyr.signed.bin + ``` + +2. **Uplink the image** to the satellite filesystem with the GDS file uplink + panel, e.g. to `/update/zephyr.signed.bin`. + +3. **`Update.updater.PREPARE_UPDATE`** — erases the secondary slot. Wait for + `PrepareUpdateSucceeded`. + +4. **`Update.updater.UPDATE_IMAGE_FROM`** with the uplinked path and the CRC + from step 1. Wait for `UpdateSucceeded`. A CRC mismatch surfaces as + `Update.worker.ImageFileCrcMismatch`. -Go to components/flashworker +5. **`Update.updater.CONFIGURE_NEXT_BOOT`** with `TEST`. Use `TEST`, not + `PERMANENT`: a `TEST` image that fails to boot is automatically reverted, + a `PERMANENT` one is not. -regionnumber = 1 try instead region number=2 +6. **Reboot** — `ReferenceDeployment.resetManager.COLD_RESET`, or power cycle. + MCUboot performs the swap during boot, which takes noticeably longer than a + normal reset. + +7. **Verify** the new image is running: send `CdhCore.version.VERSION` with + `PROJECT` and check the `ProjectVersion` event against the version of the + build you uplinked. + +8. **`Update.updater.CONFIRM_UPDATE`** — makes the swap permanent. Until you do + this, the *next* reboot reverts to the previous image. + +### Testing it + +`PROVESFlightControllerReference/test/int/ota_test.py` runs this whole cycle +against real hardware and checks the reported project version after the swap. +It is excluded from the default integration run because it erases a flash slot, +uplinks a large file and reboots the board: + +```shell +make test-integration TEST=ota FILTER=ota +``` -(redo all the stuff) +### If an update goes wrong + +- **The board stops responding right after `PREPARE_UPDATE`.** That means the + erase hit the running slot instead of the staging slot. `FlashWorker` resolves + the target from the `slot1_partition` devicetree label + (`PARTITION_ID(slot1_partition)`); do **not** replace this with a literal + number. Zephyr assigns flash-area IDs in devicetree dependency-ordinal order, + so adding any partition anywhere in the devicetree renumbers every area, and a + hardcoded ID starts pointing at a different partition. Recover by copying + `bootable.uf2` onto the board in UF2 bootloader mode, or over SWD with + `make debug-install bootable.signed.hex`. +- **The board boots the old image after the swap reboot.** MCUboot rejected the + staged image — usually a signature mismatch (`keys/proves.pem` does not match + the installed bootloader) or an image that overflows the slot. +- **The new image works, then disappears after a later reboot.** `CONFIRM_UPDATE` + was never sent, so MCUboot reverted the trial boot. diff --git a/docs-site/components/FlashWorker.md b/docs-site/components/FlashWorker.md index cbbee1ae..324cafe3 100644 --- a/docs-site/components/FlashWorker.md +++ b/docs-site/components/FlashWorker.md @@ -2,8 +2,26 @@ Performs long-running operations for the flash subsystem. The flash worker is responsible for handling the actual operations needed for flight-software update specific to the Zephyr flash API. - - +## Update region + +Every flash operation here targets `REGION_NUMBER`, the MCUboot **secondary +slot** — the staging area an uploaded image is written to before the bootloader +swaps it into the primary slot. It is resolved at compile time from the +devicetree label: + +```cpp +constexpr static U8 REGION_NUMBER = PARTITION_ID(slot1_partition); +``` + +**Never replace this with a literal number.** Zephyr assigns flash-area IDs by +devicetree *dependency ordinal*, not by address or declaration order, so adding +a partition anywhere in the devicetree renumbers every area. A hardcoded ID +therefore starts silently pointing at a different partition — and when that +partition is `slot0_partition`, `prepareImage` erases the firmware that is +currently executing and the board is bricked until it is reflashed over UF2 or +SWD. A `static_assert` in `FlashWorker.hpp` fails the build if the update region +ever resolves to the running code partition, and +`PROVESFlightControllerReference/test/int/ota_test.py` covers it on hardware. ## Usage Examples diff --git a/pytest.ini b/pytest.ini index 97b1fc54..97916796 100644 --- a/pytest.ini +++ b/pytest.ini @@ -9,6 +9,7 @@ markers = requires_battery: marks tests that require the battery board connected with power flowing from the battery terminals; skip on a bare flight controller requires_watchdog_jumper: marks tests that require the JP6 watchdog jumper to be bridged so the watchdog can reset the MCU; skip when JP6 is open slow: marks tests that take tens of seconds because they wait on a reboot or a timeout expiring + ota: marks the over-the-air update tests; they erase the MCUboot staging slot, uplink a ~1.4 MB image and reboot the board, so they are excluded from the default run filterwarnings = ignore::DeprecationWarning:yamcs\..* ignore::DeprecationWarning:google\.protobuf\..* From 52e24e91dd5bcfaa24d635722f725573bcc6a7e3 Mon Sep 17 00:00:00 2001 From: Nate Gay Date: Mon, 3 Aug 2026 22:53:33 +0200 Subject: [PATCH 02/11] ci: add a manually-triggered hardware OTA update job 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-` 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. --- .github/workflows/ci.yaml | 225 ++++++++++++++++++ Makefile | 6 +- .../test/int/ota_test.py | 13 +- README.md | 24 +- 4 files changed, 261 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index ae474ca0..242e8d2a 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -5,6 +5,16 @@ on: push: branches: - main + workflow_dispatch: + inputs: + run_ota: + description: "Run the hardware over-the-air update test" + type: boolean + default: false + ota_uplink_cooldown: + description: "Seconds between file-uplink chunks during the OTA test (repo default is 0.400)" + type: string + default: "0.400" jobs: lint: @@ -112,6 +122,40 @@ jobs: yamcs/yamcs-data/mdb/fprime.xtce.xml retention-days: 30 + # --- OTA staging image ------------------------------------------------- + # The OTA test has to prove a *swap* happened, not merely that the update + # commands returned OK, so the image it uplinks must be distinguishable + # from the one flashed on the board. Project version comes from + # `git describe --tags --always --dirty`, so a throwaway tag renames this + # build and changes nothing else about it. + # + # The rm is required: fprime generates version.{hpp,cpp,json} from a cmake + # custom command with no declared inputs, so ninja considers it up to date + # as soon as the outputs exist and a rebuild would keep the old string. + # + # This runs after the artifact upload above, so overwriting bootable.* here + # cannot affect the image the other integration jobs flash. + - name: Build OTA staging image + if: (github.event_name == 'workflow_dispatch' && inputs.run_ota) || contains(github.event.pull_request.labels.*.name, 'test-ota') + run: | + git tag "ota-ci-${GITHUB_RUN_ID}" + rm -f build-fprime-automatic-zephyr/versions/version.hpp \ + build-fprime-automatic-zephyr/versions/version.cpp \ + build-fprime-automatic-zephyr/versions/version.json + BUILD_YAMCS_MDB=0 make build + mkdir -p ota-image + cp build-artifacts/zephyr.signed.bin ota-image/ + cp build-fprime-automatic-zephyr/versions/version.json ota-image/ + echo "OTA staging image version: $(cat ota-image/version.json)" + + - name: Upload OTA staging image + if: (github.event_name == 'workflow_dispatch' && inputs.run_ota) || contains(github.event.pull_request.labels.*.name, 'test-ota') + uses: actions/upload-artifact@v4 + with: + name: ota-image + path: ota-image + retention-days: 7 + integration-uart: runs-on: - integration @@ -592,6 +636,187 @@ jobs: [ -f server-rf.pid ] && kill $(cat server-rf.pid) || true ~/korad_control/.venv/bin/python ~/korad_control/korad_control.py -d /dev/ttyPWR --output 0 + integration-ota: + # Not part of the normal PR run. It monopolises the single integration cube + # for the better part of an hour (a 1.4 MB uplink at the default cooldown is + # ~48 min on its own), erases a flash slot, and reboots the board twice. + # Trigger it deliberately: run the workflow manually with "Run the hardware + # over-the-air update test" checked, or put the `test-ota` label on a PR. + if: (github.event_name == 'workflow_dispatch' && inputs.run_ota) || contains(github.event.pull_request.labels.*.name, 'test-ota') + runs-on: + - integration + needs: build + timeout-minutes: 150 + steps: + - uses: actions/checkout@v6 + + - uses: actions/download-artifact@v6 + with: + name: artifacts + path: . + + - uses: actions/download-artifact@v6 + with: + name: ota-image + path: ota-image + + - name: Erase the MCUboot staging slot + run: | + # slot1_partition is 0x200000..0x300000 within the 4 MB QSPI flash + # mapped at 0x10000000 (see + # boards/bronco_space/proves_flight_control_board_v5/proves_flight_control_board_v5.dtsi). + # + # A previous run can leave a staged image plus swap flags in slot1. If + # those survive, MCUboot may swap it in on the first boot after we + # flash the primary slot below, and the job would silently test the + # wrong firmware. Erasing first makes the starting state unambiguous. + ~/openocd/src/openocd -s ~/openocd/tcl \ + -f ~/openocd/tcl/interface/cmsis-dap.cfg \ + -f ~/openocd/tcl/target/rp2350.cfg \ + -c "adapter speed 5000" \ + -c "init; halt; flash erase_address 0x10200000 0x100000; reset run; exit" + + - name: Flash Firmware + uses: ./.github/actions/flash-firmware + + - name: Load .env file + run: | + while IFS= read -r line || [ -n "$line" ]; do + # Skip comments and empty lines + [[ "$line" =~ ^#.*$ || -z "$line" ]] && continue + echo "$line" >> $GITHUB_ENV + done < ~/actions-runner/.env + + - name: Set up dependencies + run: | + make submodules fprime-venv + + - name: Apply CI spacecraft ID overrides + run: | + make make-ci-spacecraft-id + + - name: Set Authentication Key + env: + AUTH_KEY: ${{ secrets.AUTH_KEY }} + run: | + echo "#define AUTH_DEFAULT_KEY \"$AUTH_KEY\"" > PROVESFlightControllerReference/Components/TcSecurityDeframer/AuthDefaultKey.h + + - name: Install Framer Plugin + run: | + make framer-plugin + + - name: Record the staged image version + env: + # Quoted through the environment rather than interpolated into the + # shell, so a workflow input cannot inject a command. + UPLINK_COOLDOWN: ${{ inputs.ota_uplink_cooldown || '0.400' }} + run: | + OTA_VERSION=$(./fprime-venv/bin/python -c \ + 'import json; print(json.load(open("ota-image/version.json"))["project_version"])') + echo "Board will be flashed with the default build; OTA will uplink: $OTA_VERSION" + echo "OTA_VERSION=$OTA_VERSION" >> $GITHUB_ENV + echo "GDS_EXTRA_ARGS=--file-uplink-cooldown $UPLINK_COOLDOWN" >> $GITHUB_ENV + + - name: Detect Board TTY + run: | + ZEPHYR_TTY=$(./tools/ci/detect-board-tty.sh) + echo "Selected Zephyr board tty: $ZEPHYR_TTY" + echo "UART_DEVICE=$ZEPHYR_TTY" >> $GITHUB_ENV + + - name: Kill all python processes + run: | + pkill -9 python || true + + - name: Start GDS + run: | + nohup make gds-integration UART_DEVICE="$UART_DEVICE" GDS_EXTRA_ARGS="$GDS_EXTRA_ARGS" > gds-bootstrap.log 2>&1 & + echo $! > server.pid + + - name: Sync Sequence Number + run: | + make test-integration FILTER=sync_sequence_number + + - name: Format Filesystem + run: | + # The uplinked image is ~1.4 MB and lands in /update. Starting from a + # formatted filesystem keeps a previous run's copy from filling it. + make test-integration FILTER=format_filesystem + + - name: Power-Cycle Satellite + run: | + # /seq, /antenna and friends are only recreated at boot, so the format + # above has to be followed by a reboot before anything else runs. + [ -f server.pid ] && kill $(cat server.pid) || true + pkill -9 python || true + ~/korad_control/.venv/bin/python ~/korad_control/korad_control.py -d /dev/ttyPWR --output 0 + sleep 3 + ~/korad_control/.venv/bin/python ~/korad_control/korad_control.py -d /dev/ttyPWR --output 1 + sleep 3 + + - name: Detect Board TTY + run: | + ZEPHYR_TTY=$(./tools/ci/detect-board-tty.sh) + echo "Selected Zephyr board tty: $ZEPHYR_TTY" + echo "UART_DEVICE=$ZEPHYR_TTY" >> $GITHUB_ENV + + - name: Start GDS + run: | + nohup make gds-integration UART_DEVICE="$UART_DEVICE" GDS_EXTRA_ARGS="$GDS_EXTRA_ARGS" > gds.log 2>&1 & + echo $! > server.pid + + - name: Sync Sequence Number + run: | + make test-integration FILTER=sync_sequence_number + + - name: Run OTA Integration Test + run: | + make test-integration TEST=ota_test.py FILTER=ota \ + PYTEST_ARGS="--ota-image=ota-image/zephyr.signed.bin --ota-expect-version=$OTA_VERSION" + + - name: Upload GDS logs + if: always() + uses: actions/upload-artifact@v4 + with: + name: gds-log-ota + path: | + gds-bootstrap.log + gds.log + logs + !logs/**/*.xlsx + if-no-files-found: ignore + retention-days: 7 + + - name: Kill GDS + if: always() + run: | + [ -f server.pid ] && kill $(cat server.pid) || true + pkill -9 python || true + fuser -k /dev/ttyBOARD 2>/dev/null || true + sleep 2 + + - name: Restore the board + if: always() + run: | + # The board is left running the OTA staging image, and slot1 holds + # whatever the swap displaced. Put both slots back to a known state so + # the next job on this runner starts from the artifact build, not from + # a leftover OTA image. + ~/openocd/src/openocd -s ~/openocd/tcl \ + -f ~/openocd/tcl/interface/cmsis-dap.cfg \ + -f ~/openocd/tcl/target/rp2350.cfg \ + -c "adapter speed 5000" \ + -c "init; halt; flash erase_address 0x10200000 0x100000; reset run; exit" || true + ~/openocd/src/openocd -s ~/openocd/tcl \ + -f ~/openocd/tcl/interface/cmsis-dap.cfg \ + -f ~/openocd/tcl/target/rp2350.cfg \ + -c "adapter speed 5000" \ + -c "program bootable.signed.hex verify reset exit" || true + + - name: Power Off Satellite + if: always() + run: | + ~/korad_control/.venv/bin/python ~/korad_control/korad_control.py -d /dev/ttyPWR --output 0 + yamcs-build: runs-on: ubuntu-latest needs: build diff --git a/Makefile b/Makefile index 8a451fee..d0a07314 100644 --- a/Makefile +++ b/Makefile @@ -423,8 +423,12 @@ delete-shadow-gds: @$(UV_RUN) pkill -9 -f fprime-gds .PHONY: gds-integration +# GDS_EXTRA_ARGS appends to the options in fprime-gds.yml. Mainly a tuning lever +# for file uplink: at the repo default (--file-uplink-cooldown 0.400, +# --file-uplink-chunk-size 204) a 1.4 MB firmware image takes ~48 minutes to +# uplink, so the OTA job lowers the cooldown. gds-integration: framer-plugin - @$(GDS_COMMAND) --gui=none --output-unframed-data --uart-device=$(if $(UART_DEVICE),$(UART_DEVICE),/dev/ttyBOARD) + @$(GDS_COMMAND) --gui=none --output-unframed-data --uart-device=$(if $(UART_DEVICE),$(UART_DEVICE),/dev/ttyBOARD) $(GDS_EXTRA_ARGS) .PHONY: DoL_test DoL_test: diff --git a/PROVESFlightControllerReference/test/int/ota_test.py b/PROVESFlightControllerReference/test/int/ota_test.py index 7afe3209..29934a14 100644 --- a/PROVESFlightControllerReference/test/int/ota_test.py +++ b/PROVESFlightControllerReference/test/int/ota_test.py @@ -19,7 +19,7 @@ run: it erases a flash slot, uplinks a ~1.4 MB file, and reboots the board. Run it deliberately: - make test-integration TEST=ota FILTER=ota + make test-integration TEST=ota_test.py FILTER=ota By default it re-flashes the image in ``build-artifacts/zephyr.signed.bin``, i.e. the build sitting in the working tree. Point somewhere else with @@ -52,10 +52,13 @@ # Directory on the satellite filesystem that holds the staged image. UPDATE_DIR = "/update" -# Uplinking ~1.4 MB in file-uplink packets takes minutes even over UART, and the -# flash write that follows walks the image in CONFIG_IMG_BLOCK_BUF_SIZE (512 B) -# chunks with a 5 ms settle delay per chunk. -UPLINK_TIMEOUT_S = 45 * 60 +# Uplink is the long pole. fprime-gds.yml sets file-uplink-chunk-size 204 and +# file-uplink-cooldown 0.400, so a 1.4 MB image is ~7100 chunks -> ~48 min at the +# repo default. Lower the cooldown (GDS_EXTRA_ARGS on `make gds-integration`) to +# go faster; this ceiling is sized so the default still fits. +UPLINK_TIMEOUT_S = 90 * 60 +# The flash write walks the image in CONFIG_IMG_BLOCK_BUF_SIZE (512 B) chunks +# with a 5 ms settle delay per chunk, reading each chunk back off littlefs. IMAGE_WRITE_TIMEOUT_S = 15 * 60 # Cold reset plus an MCUboot swap of two 1 MB slots, then a full FSW boot. SWAP_REBOOT_TIMEOUT_S = 180 diff --git a/README.md b/README.md index a2806a94..22d654dd 100644 --- a/README.md +++ b/README.md @@ -310,9 +310,31 @@ It is excluded from the default integration run because it erases a flash slot, uplinks a large file and reboots the board: ```shell -make test-integration TEST=ota FILTER=ota +make test-integration TEST=ota_test.py FILTER=ota ``` +Run against the build in your working tree, it uplinks the image the board is +already running, so the post-swap version check passes trivially. To make the +check meaningful, point `--ota-image` at a *different* build: + +```shell +make test-integration TEST=ota_test.py FILTER=ota \ + PYTEST_ARGS="--ota-image=/path/to/other/zephyr.signed.bin --ota-expect-version=v1.2.3" +``` + +In CI this is the `integration-ota` job. It is not part of the normal PR run — +it ties up the integration cube for the better part of an hour. Trigger it by +running the `ci` workflow manually with **Run the hardware over-the-air update +test** checked, or by adding the `test-ota` label to a PR. The job builds a +second image under a throwaway git tag so the project version differs from the +one flashed on the board, which is what lets it prove a swap actually happened. + +Uplink dominates the runtime: at the `fprime-gds.yml` defaults +(`file-uplink-chunk-size: 204`, `file-uplink-cooldown: 0.400`) a 1.4 MB image is +about 7100 chunks, ~48 minutes. Lower the cooldown to go faster — locally via +`make gds-integration GDS_EXTRA_ARGS="--file-uplink-cooldown 0.05"`, or in CI via +the workflow's uplink-cooldown input. + ### If an update goes wrong - **The board stops responding right after `PREPARE_UPDATE`.** That means the From 2f464ef39e9fd0c9fb9e5414c8242aad02d73c58 Mon Sep 17 00:00:00 2001 From: Nate Gay Date: Mon, 3 Aug 2026 22:59:42 +0200 Subject: [PATCH 03/11] ci: fire pull_request runs on label changes 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`. --- .github/workflows/ci.yaml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 242e8d2a..8ec9dc7c 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -2,6 +2,15 @@ name: ci on: pull_request: + # `labeled` is not in the default set. Without it, adding `test-ota` to an + # open PR does nothing and the OTA job can only be reached by pushing a + # commit after the label is already on. Listing the types restores the + # defaults and makes labelling an actual trigger. + types: + - opened + - synchronize + - reopened + - labeled push: branches: - main From 7a1c92218827d77c730180aa2a11f3e923819030 Mon Sep 17 00:00:00 2001 From: Nate Gay Date: Mon, 3 Aug 2026 23:04:10 +0200 Subject: [PATCH 04/11] ci: power the cube on before the OTA job's staging-slot erase 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. --- .github/workflows/ci.yaml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 8ec9dc7c..8e0ac02c 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -669,6 +669,17 @@ jobs: name: ota-image path: ota-image + - name: Power On Satellite + run: | + # Every other integration job's first hardware action is the + # flash-firmware composite, which powers the cube up before it talks + # to OpenOCD. This job reaches for OpenOCD first, and every job on + # this runner ends with "Power Off Satellite", so without this the + # erase below would run against an unpowered board and fail to + # attach. + ~/korad_control/.venv/bin/python ~/korad_control/korad_control.py -d /dev/ttyPWR --output 1 + sleep 3 + - name: Erase the MCUboot staging slot run: | # slot1_partition is 0x200000..0x300000 within the 4 MB QSPI flash From 6caf4bc31ae891c0834e18ef00f5a8d91798cc7f Mon Sep 17 00:00:00 2001 From: Nate Gay Date: Mon, 3 Aug 2026 23:59:08 +0200 Subject: [PATCH 05/11] test(ota): retry commands that are awaited by event, not by ack 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. --- .../test/int/ota_test.py | 99 +++++++++++++++---- 1 file changed, 80 insertions(+), 19 deletions(-) diff --git a/PROVESFlightControllerReference/test/int/ota_test.py b/PROVESFlightControllerReference/test/int/ota_test.py index 29934a14..debb9bc5 100644 --- a/PROVESFlightControllerReference/test/int/ota_test.py +++ b/PROVESFlightControllerReference/test/int/ota_test.py @@ -39,6 +39,7 @@ from common import cmdDispatch, proves_send_and_assert_command from fprime_gds.common.data_types.event_data import EventData from fprime_gds.common.models.serialize.time_type import TimeType +from fprime_gds.common.testing_fw import predicates from fprime_gds.common.testing_fw.api import IntegrationTestAPI # OTA severs the RF link (it reboots the board), so it only makes sense on UART. @@ -62,25 +63,78 @@ IMAGE_WRITE_TIMEOUT_S = 15 * 60 # Cold reset plus an MCUboot swap of two 1 MB slots, then a full FSW boot. SWAP_REBOOT_TIMEOUT_S = 180 -# Erasing the whole 1 MB staging slot outruns the command-ack window that -# proves_send_and_assert_command allows, so PREPARE_UPDATE is awaited by event. +# Erasing the whole 1 MB staging slot is awaited by event rather than through +# proves_send_and_assert_command's ack window. Measured at 2.8 s on the CI cube. PREPARE_TIMEOUT_S = 120 +# The TC replay window silently drops any frame whose sequence number the board +# has already accepted, and the GDS has been observed emitting the same sequence +# number twice for the first two commands of a session. Every other command in +# the suite absorbs that through proves_send_and_assert_command's retries; the +# commands here are awaited by event instead, so they need their own. Retrying +# is keyed on the component's "started" event rather than its completion event, +# so a retry can never re-issue work that is already underway. +COMMAND_ATTEMPTS = 3 +DISPATCH_TIMEOUT_S = 20 + # Svc.Version declares its version strings as `string size 40`, so anything # longer is truncated in flight before it reaches telemetry. VERSION_STRING_SIZE = 40 +def send_and_confirm_dispatch( + fprime_test_api: IntegrationTestAPI, + command: str, + started_event: str, + args: list[str] | None = None, +) -> None: + """Send a command, retrying until the flight side reports it started. + + Clears histories before each attempt, so the caller can search from index 0 + for whatever the command goes on to emit. + """ + for _ in range(COMMAND_ATTEMPTS): + fprime_test_api.clear_histories() + fprime_test_api.send_command(command, args or []) + if ( + fprime_test_api.await_event(started_event, timeout=DISPATCH_TIMEOUT_S) + is not None + ): + return + raise AssertionError( + f"{command} never reached the flight software in {COMMAND_ATTEMPTS} attempts" + ) + + +def await_outcome( + fprime_test_api: IntegrationTestAPI, names: list[str], timeout: int +) -> EventData: + """Await whichever of ``names`` lands first, searching the whole history. + + ``await_event`` coerces a non-predicate argument into an event-*ID* + predicate, which is why ``satisfies_any`` of event_predicates silently never + matches here; a member-of over translated IDs is the form that works. + """ + ids = [fprime_test_api.translate_event_name(name) for name in names] + return fprime_test_api.await_event( + predicates.is_a_member_of(ids), timeout=timeout, start=0 + ) + + def prepare_update(fprime_test_api: IntegrationTestAPI) -> None: """Erase the staging slot and wait for the erase to report success.""" - fprime_test_api.clear_histories() - fprime_test_api.send_command(f"{updater}.PREPARE_UPDATE") - assert ( - fprime_test_api.await_event( - f"{updater}.PrepareUpdateSucceeded", timeout=PREPARE_TIMEOUT_S - ) - is not None - ), "PREPARE_UPDATE did not report success" + send_and_confirm_dispatch( + fprime_test_api, f"{updater}.PREPARE_UPDATE", f"{updater}.PrepareUpdate" + ) + outcome = await_outcome( + fprime_test_api, + [f"{updater}.PrepareUpdateSucceeded", f"{updater}.PrepareUpdateFailed"], + timeout=PREPARE_TIMEOUT_S, + ) + assert outcome is not None, "PREPARE_UPDATE reported neither success nor failure" + assert outcome.template.get_name() == "PrepareUpdateSucceeded", ( + f"PREPARE_UPDATE failed: {outcome.get_str()}" + ) def fprime_crc32(path: Path) -> int: @@ -225,16 +279,23 @@ def test_03_full_ota_cycle( # 3. Copy it into the staging slot. The CRC is checked flight-side before a # single byte is written, so a mismatch here means the uplink was lossy. - fprime_test_api.clear_histories() - fprime_test_api.send_command( - f"{updater}.UPDATE_IMAGE_FROM", [destination, str(crc32)] + send_and_confirm_dispatch( + fprime_test_api, + f"{updater}.UPDATE_IMAGE_FROM", + f"{updater}.Update", + [destination, str(crc32)], + ) + outcome = await_outcome( + fprime_test_api, + [f"{updater}.UpdateSucceeded", f"{updater}.UpdateFailed"], + timeout=IMAGE_WRITE_TIMEOUT_S, + ) + assert outcome is not None, ( + "image write to the staging slot reported neither success nor failure" + ) + assert outcome.template.get_name() == "UpdateSucceeded", ( + f"image write to the staging slot failed: {outcome.get_str()}" ) - assert ( - fprime_test_api.await_event( - f"{updater}.UpdateSucceeded", timeout=IMAGE_WRITE_TIMEOUT_S - ) - is not None - ), "image write to the staging slot did not succeed" # 4. Arm the one-shot trial boot. TEST rather than PERMANENT so that a bad # image reverts on the following reboot instead of stranding the board. From 6968ef7389d18b41eeb5715bf654f6ee9db70d03 Mon Sep 17 00:00:00 2001 From: hrfarmer Date: Mon, 3 Aug 2026 17:12:29 -0500 Subject: [PATCH 06/11] set downlink repeater channels --- PROVESFlightControllerReference/test/int/ota_test.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/PROVESFlightControllerReference/test/int/ota_test.py b/PROVESFlightControllerReference/test/int/ota_test.py index debb9bc5..12bb5595 100644 --- a/PROVESFlightControllerReference/test/int/ota_test.py +++ b/PROVESFlightControllerReference/test/int/ota_test.py @@ -49,6 +49,7 @@ worker = "Update.worker" version = "CdhCore.version" fileManager = "FileHandling.fileManager" +downlinkRepeater = "ReferenceDeployment.downlinkRepeater" # Directory on the satellite filesystem that holds the staged image. UPDATE_DIR = "/update" @@ -270,6 +271,15 @@ def test_03_full_ota_cycle( # already exist from an earlier run, and fileManager errors on that. fprime_test_api.send_command(f"{fileManager}.CreateDirectory", [UPDATE_DIR]) time.sleep(1) + + # Route file traffic over UART only. Confirm the parameter command before + # starting the long uplink so another output cannot consume its buffers. + proves_send_and_assert_command( + fprime_test_api, + f"{downlinkRepeater}.CHANNEL_ENABLED_PRM_SET", + [json.dumps(["ENABLED", "DISABLED", "DISABLED"])], + ) + fprime_test_api.clear_histories() fprime_test_api.uplink_file(str(ota_image), destination) assert ( From 9be13dfd73463fc818cf285bde45807fd2a0b408 Mon Sep 17 00:00:00 2001 From: Nate Gay Date: Tue, 4 Aug 2026 18:33:37 +0200 Subject: [PATCH 07/11] fix(gds): lock the uplink sequence number across processes 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. --- Framing/src/authenticate_plugin.py | 59 ++++++++++++++++++------------ 1 file changed, 36 insertions(+), 23 deletions(-) diff --git a/Framing/src/authenticate_plugin.py b/Framing/src/authenticate_plugin.py index a09c2cbb..af0b21f7 100644 --- a/Framing/src/authenticate_plugin.py +++ b/Framing/src/authenticate_plugin.py @@ -1,8 +1,10 @@ """Authenticate Plugin for Framing Package.""" +import fcntl import hashlib import hmac import os +import threading from typing import List, Type from fprime_gds.common.communication.ccsds.chain import ChainedFramerDeframer @@ -105,11 +107,10 @@ def __init__( """ super().__init__() - # Initialize sequence number from CLI argument or default to 0 - seq_num = self.get_sequence_number_from_file( - SEQUENCE_NUMBER_FILE, addition=False - ) - self.bytes_seq_num = seq_num.to_bytes(4, byteorder="big", signed=False) + # Guards against races within this process; see get_sequence_number_from_file + # for why cross-process races also need an OS-level file lock. + self._frame_lock = threading.Lock() + # Store values from CLI arguments or defaults self.spi = spi self.window_size = window_size @@ -131,19 +132,29 @@ def get_sequence_number_from_file(self, filename: str, addition: bool) -> int: If addition is True, increment the sequence number and write back to file Otherwise just return what is on there + The GDS runs the file-uplink encoding chain in a separate process + (CustomDataHandlers) from the one framing commands (comm), so this + file is the only thing the two agree on -- each process's own + AuthenticateFramer instance has no other way to learn what sequence + number the other has already used. flock makes the read-modify-write + atomic across processes; without it, two frames built by different + processes at nearly the same time can both read the same pending + value and emit it as an on-wire duplicate, which the flight side then + correctly rejects as a replay (TcSecurityDeframer SequenceNumberInvalid). """ - file_number = 0 - try: - with open(filename, "r") as f: - file_number = int(f.read()) - if addition: - file_number += 1 - # Write the incremented value back to file - with open(filename, "w") as f: + with open(filename, "a+") as f: + fcntl.flock(f, fcntl.LOCK_EX) + try: + f.seek(0) + content = f.read() + file_number = int(content) if content else 0 + if addition: + file_number += 1 + f.seek(0) + f.truncate() f.write(str(file_number)) - except FileNotFoundError: - with open(filename, "w") as f: - f.write(str(file_number)) + finally: + fcntl.flock(f, fcntl.LOCK_UN) return file_number @@ -156,14 +167,16 @@ def frame(self, data: bytes) -> bytes: header = b"" bytes_spi = self.spi.to_bytes(2, byteorder="big", signed=False) header += bytes_spi - # Sequence Number (32 bits/4 bytes, starts at 0x00000000): - header += self.bytes_seq_num - sequence_number = self.get_sequence_number_from_file( - SEQUENCE_NUMBER_FILE, addition=True - ) - - self.bytes_seq_num = sequence_number.to_bytes(4, byteorder="big", signed=False) + # Sequence Number (32 bits/4 bytes, starts at 0x00000000): always + # derive this frame's number directly from the shared file rather + # than an in-memory cache, so this instance can never emit a number + # another instance (in this process or another) has already used. + with self._frame_lock: + sequence_number = self.get_sequence_number_from_file( + SEQUENCE_NUMBER_FILE, addition=True + ) + header += sequence_number.to_bytes(4, byteorder="big", signed=False) data = header + data From d636cca087849a3025ec4ce60b1db6afdd13d53d Mon Sep 17 00:00:00 2001 From: Nate Gay Date: Tue, 4 Aug 2026 18:35:53 +0200 Subject: [PATCH 08/11] perf(tc-security): keep the sequence-number file open across frames 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. --- .../TcSecurityDeframer/TcSecurityDeframer.cpp | 18 +++++++++++++++++- .../TcSecurityDeframer/TcSecurityDeframer.hpp | 11 +++++++++-- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/PROVESFlightControllerReference/Components/TcSecurityDeframer/TcSecurityDeframer.cpp b/PROVESFlightControllerReference/Components/TcSecurityDeframer/TcSecurityDeframer.cpp index a75f6d64..ff270cc6 100644 --- a/PROVESFlightControllerReference/Components/TcSecurityDeframer/TcSecurityDeframer.cpp +++ b/PROVESFlightControllerReference/Components/TcSecurityDeframer/TcSecurityDeframer.cpp @@ -551,7 +551,23 @@ Os::File::Status TcSecurityDeframer ::readSequenceNumber(U32& value) { } Os::File::Status TcSecurityDeframer ::writeSequenceNumber(const U32 value) { - Os::File::Status status = Utilities::FileHelper::writeToFile(this->m_sequenceNumberFilePath.toChar(), value); + Os::File::Status status; + + if (!this->m_sequenceNumberFile.isOpen()) { + status = this->m_sequenceNumberFile.open(this->m_sequenceNumberFilePath.toChar(), Os::File::Mode::OPEN_CREATE, + Os::File::OverwriteType::OVERWRITE); + if (status != Os::File::OP_OK) { + this->log_WARNING_HI_SequenceNumberWriteFailed(static_cast(status)); + return status; + } + } + + // writeToFile(file, primitive) writes at the file's current position; rewind first since this + // handle stays open (and thus positioned after the previous write) across calls. + status = this->m_sequenceNumberFile.seek(0, Os::File::SeekType::ABSOLUTE); + if (status == Os::File::OP_OK) { + status = Utilities::FileHelper::writeToFile(this->m_sequenceNumberFile, value); + } if (status != Os::File::OP_OK) { // Log the failure to write the default sequence number this->log_WARNING_HI_SequenceNumberWriteFailed(static_cast(status)); diff --git a/PROVESFlightControllerReference/Components/TcSecurityDeframer/TcSecurityDeframer.hpp b/PROVESFlightControllerReference/Components/TcSecurityDeframer/TcSecurityDeframer.hpp index 0c09263d..e3263533 100644 --- a/PROVESFlightControllerReference/Components/TcSecurityDeframer/TcSecurityDeframer.hpp +++ b/PROVESFlightControllerReference/Components/TcSecurityDeframer/TcSecurityDeframer.hpp @@ -164,8 +164,15 @@ class TcSecurityDeframer final : public TcSecurityDeframerComponentBase { // they are protected by the same mutex to ensure atomicity of updates across both mediums Os::Mutex m_sequenceNumberLock; //!< Mutex protecting sequence number state atomicity Fw::String m_sequenceNumberFilePath; //!< File path where sequence number is stored - U32 m_sequenceNumber; //!< The current sequence number - U32 m_sequenceNumberWindow; //!< The allowed window for sequence number validation + // writeSequenceNumber persists on every accepted TC frame -- including every file-uplink + // Data packet, since those are TC frames too. Reopening+closing the file each call forces + // the flash-disk driver's single-page write-back cache to commit and reload on every frame, + // thrashing against FileUplink's own writes to a different file on the same disk. Keeping + // this handle open for the component's lifetime turns each persist into a seek+write against + // an already-open file, avoiding that churn. + Os::File m_sequenceNumberFile; //!< Persistent handle for m_sequenceNumberFilePath + U32 m_sequenceNumber; //!< The current sequence number + U32 m_sequenceNumberWindow; //!< The allowed window for sequence number validation // Key store state is coupled between in-memory runtime state (m_keyStore/m_keyIds) and on-disk // persistent storage; both are protected by keyStoreLock() in the .cpp. That mutex is From 0bf967dbf480056f083d6001194249c5f3286729 Mon Sep 17 00:00:00 2001 From: Nate Gay Date: Tue, 4 Aug 2026 18:36:10 +0200 Subject: [PATCH 09/11] test(ota): make the image uplink resilient to a lost start or handshake 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. --- .gitignore | 3 + .../test/int/ota_test.py | 102 ++++++++++++++++-- 2 files changed, 99 insertions(+), 6 deletions(-) diff --git a/.gitignore b/.gitignore index dba5eec9..ae73a328 100644 --- a/.gitignore +++ b/.gitignore @@ -58,3 +58,6 @@ yamcs/yamcs-runtime/ /circuit-python-passthrough/firmware.uf2 /circuit-python-passthrough/lib/ /circuit-python-passthrough/tools/ + +# Locally staged OTA uplink image (see ota_test.py --ota-image) +ota-image/ diff --git a/PROVESFlightControllerReference/test/int/ota_test.py b/PROVESFlightControllerReference/test/int/ota_test.py index 12bb5595..f82a0781 100644 --- a/PROVESFlightControllerReference/test/int/ota_test.py +++ b/PROVESFlightControllerReference/test/int/ota_test.py @@ -38,6 +38,7 @@ import pytest from common import cmdDispatch, proves_send_and_assert_command from fprime_gds.common.data_types.event_data import EventData +from fprime_gds.common.files.helpers import FileStates from fprime_gds.common.models.serialize.time_type import TimeType from fprime_gds.common.testing_fw import predicates from fprime_gds.common.testing_fw.api import IntegrationTestAPI @@ -68,6 +69,42 @@ # proves_send_and_assert_command's ack window. Measured at 2.8 s on the CI cube. PREPARE_TIMEOUT_S = 120 +# fprime_gds's FileUplinker has no retry of its own: a Start packet is a bare +# Fw::ComPacketType::FW_PACKET_FILE frame with no opcode, so it can never match +# PacketBypasser's opcode allowlist. If it lands on the TC replay window (the +# same GDS duplicate-sequence-number quirk every other command here works +# around) it comes back unauthenticated and ProvesRouter silently drops it +# (RejectedPackets telemetry, no event) -- the FileUplinker then just sits +# until its own internal 20 s handshake timeout expires and quietly gives up. +# Poll for read progress within that window and re-enqueue if none was made. +UPLINK_START_TIMEOUT_S = 30 +UPLINK_START_ATTEMPTS = 5 +# Each accepted data packet produces a handshake and advances TransmitFile.seek. +# If that value stops changing, a data packet or its reply was lost. +UPLINK_PROGRESS_TIMEOUT_S = 45 +# IntegrationTestAPI owns a separate FileUplinker from the headless GDS process: +# the pytest plugin builds its own StandardPipeline and never reads +# fprime-gds.yml, so neither file-uplink-cooldown nor file-uplink-chunk-size +# reaches it. Both have to be reapplied here. +# +# fprime_gds defaults the chunk to 256; fprime-gds.yml configures 204 to fit the +# 248 B frame (frame-size) once the space packet header, the file packet header +# and the TC security header with its 16 B MAC are accounted for. Match it. +UPLINK_CHUNK_SIZE = 204 +# The cooldown is the one that bites, and for a reason that is easy to miss: the +# flight side applies no back-pressure to file uplink. ProvesRouter copies each +# file packet into a buffer from a pool of ComCcsdsConfig.BuffMgr +# .commsFileBuffCount (5) before handing it to fileUplink, and when that pool is +# empty it drops the packet -- while the ground goes on streaming at its full +# configured rate. Uplink faster than the flight side can drain that pool and +# the transfer *appears* to succeed: FileReceived is still emitted, the file is +# still full length, and only the checksum reveals that everything after the +# first ~5 packets was thrown away (PacketOutOfOrder reporting "packet N after +# packet 5" is the tell). Measured on the bench against a 32 KB file checked +# with fileManager.CalculateCrc: 0.100 corrupts, 0.400 and 1.000 are clean. +# 0.400 is also what fprime-gds.yml configures, so the two agree. +UPLINK_COOLDOWN_S = 0.4 + # The TC replay window silently drops any frame whose sequence number the board # has already accepted, and the GDS has been observed emitting the same sequence # number twice for the first two commands of a session. Every other command in @@ -138,6 +175,64 @@ def prepare_update(fprime_test_api: IntegrationTestAPI) -> None: ) +def uplink_with_retry( + fprime_test_api: IntegrationTestAPI, + image_path: Path, + destination: str, + timeout: int, +) -> None: + """Uplink a file, restarting it if its start or any later handshake stalls. + + Progress is detected via the uplinker's active TransmitFile.seek, which + only advances once a chunk is read in response to a received handshake. + A retry resets both ends: send_cancel_packet only emits the wire packet; + it does not release FileUplink's local queue after a lost data handshake. + """ + uplinker = fprime_test_api.pipeline.files.uplinker + uplinker.chunk = UPLINK_CHUNK_SIZE + uplinker.cooldown = UPLINK_COOLDOWN_S + deadline = time.monotonic() + timeout + + for attempt in range(UPLINK_START_ATTEMPTS): + fprime_test_api.clear_histories() + fprime_test_api.uplink_file(str(image_path), destination) + + last_seek = 0 + last_progress = time.monotonic() + while time.monotonic() < deadline: + active = uplinker.active + if active is not None and active.seek > last_seek: + last_seek = active.seek + last_progress = time.monotonic() + if fprime_test_api.await_event("FileReceived", timeout=1) is not None: + return + stall_timeout = ( + UPLINK_START_TIMEOUT_S if last_seek == 0 else UPLINK_PROGRESS_TIMEOUT_S + ) + if time.monotonic() - last_progress >= stall_timeout: + break + else: + break + + # The flight-side receiver has no timeout. Tell it to abandon the + # partial file and release the GDS queue before enqueuing the retry. + # + # Only when the uplinker has not already torn itself down: its own 20 s + # handshake timeout calls finish(), which closes the handle and unlinks + # the up_store copy, so a second finish() here would raise + # FileNotFoundError. State, not `active`, is the thing to check -- + # `active` stays set after a finish. + if uplinker.state != FileStates.IDLE: + uplinker.send_cancel_packet() + uplinker.finish(wait_for_handshake=False) + time.sleep(1) + else: + raise AssertionError( + f"uplink of {image_path} stalled in {UPLINK_START_ATTEMPTS} attempts" + ) + raise AssertionError(f"uplink of {image_path} to {destination} timed out") + + def fprime_crc32(path: Path) -> int: """CRC32 of a file in the convention ``Os::File::calculateCrc`` uses. @@ -280,12 +375,7 @@ def test_03_full_ota_cycle( [json.dumps(["ENABLED", "DISABLED", "DISABLED"])], ) - fprime_test_api.clear_histories() - fprime_test_api.uplink_file(str(ota_image), destination) - assert ( - fprime_test_api.await_event("FileReceived", timeout=UPLINK_TIMEOUT_S) - is not None - ), f"uplink of {ota_image} to {destination} never completed" + uplink_with_retry(fprime_test_api, ota_image, destination, UPLINK_TIMEOUT_S) # 3. Copy it into the staging slot. The CRC is checked flight-side before a # single byte is written, so a mismatch here means the uplink was lossy. From 5104b1c2d857a1b9ec5049f5e1011eb68050f3b9 Mon Sep 17 00:00:00 2001 From: Nate Gay Date: Tue, 4 Aug 2026 19:04:37 +0200 Subject: [PATCH 10/11] ci(ota): move the OTA test to its own nightly workflow 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. --- .github/workflows/build.yaml | 125 ++++++++++++++++++ .github/workflows/ci.yaml | 245 ----------------------------------- .github/workflows/ota.yaml | 212 ++++++++++++++++++++++++++++++ README.md | 12 +- 4 files changed, 343 insertions(+), 251 deletions(-) create mode 100644 .github/workflows/build.yaml create mode 100644 .github/workflows/ota.yaml diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml new file mode 100644 index 00000000..b3db011e --- /dev/null +++ b/.github/workflows/build.yaml @@ -0,0 +1,125 @@ +name: build + +# Reusable build job, called by `ci` and by `ota`. Both need the same firmware +# artifacts; only the OTA workflow needs the extra staging image, which is +# gated behind `build_ota_image`. +on: + workflow_call: + inputs: + build_ota_image: + description: "Also build and upload a distinctly-versioned OTA staging image" + type: boolean + default: false + +jobs: + build: + runs-on: deathstar + steps: + - uses: actions/checkout@v6 + + - name: Download bin tools + if: steps.cache-bin.outputs.cache-hit != 'true' + run: | + make download-bin + + - name: Setup submodules + if: steps.cache-submodules.outputs.cache-hit != 'true' + run: | + make submodules + + - name: Create python venv + run: | + make fprime-venv + + - name: Setup Zephyr + if: steps.cache-zephyr-workspace.outputs.cache-hit != 'true' + run: | + make zephyr-workspace + + - name: Setup Zephyr SDK + if: steps.cache-zephyr-sdk.outputs.cache-hit != 'true' + run: | + make zephyr-sdk + + - name: Setup Zephyr Export + run: | + make zephyr-export + + - name: Install Zephyr Python Dependencies + run: | + make zephyr-python-deps + + - name: Change to CI Spacecraft ID + run: | + make make-ci-spacecraft-id + + - name: Generate + run: | + make generate + + - name: Set Authentication Key + env: + AUTH_KEY: ${{ secrets.AUTH_KEY }} + run: | + echo "#define AUTH_DEFAULT_KEY \"$AUTH_KEY\"" > PROVESFlightControllerReference/Components/TcSecurityDeframer/AuthDefaultKey.h + + - name: Build MCUBoot + run: | + make build-mcuboot + + - name: Build Flight Software + run: | + make build + + - name: Ensure console disabled + # The Zephyr console shares cdc_acm_uart0 with the F' downlink; if it is + # re-enabled, console text interleaves with the CCSDS TM frames and + # desyncs the GDS deframer. Fail the build so this cannot reach main. + run: | + make check-console-disabled + + - name: Upload build artifacts + uses: actions/upload-artifact@v4 + with: + name: artifacts + path: | + mcuboot.elf + bootable.uf2 + bootable.signed.hex + build-artifacts/zephyr/fprime-zephyr-deployment + yamcs/yamcs-data/mdb/fprime.xtce.xml + retention-days: 30 + + # --- OTA staging image ------------------------------------------------- + # The OTA test has to prove a *swap* happened, not merely that the update + # commands returned OK, so the image it uplinks must be distinguishable + # from the one flashed on the board. Project version comes from + # `git describe --tags --always --dirty`, so a throwaway tag renames this + # build and changes nothing else about it. + # + # The rm is required: fprime generates version.{hpp,cpp,json} from a cmake + # custom command with no declared inputs, so ninja considers it up to date + # as soon as the outputs exist and a rebuild would keep the old string. + # + # This runs after the artifact upload above, so overwriting bootable.* here + # cannot affect the image the other integration jobs flash. + - name: Build OTA staging image + if: inputs.build_ota_image + run: | + git tag "ota-ci-${GITHUB_RUN_ID}" + rm -f build-fprime-automatic-zephyr/versions/version.hpp \ + build-fprime-automatic-zephyr/versions/version.cpp \ + build-fprime-automatic-zephyr/versions/version.json + BUILD_YAMCS_MDB=0 make build + mkdir -p ota-image + cp build-artifacts/zephyr.signed.bin ota-image/ + cp build-fprime-automatic-zephyr/versions/version.json ota-image/ + echo "OTA staging image version: $(cat ota-image/version.json)" + + - name: Upload OTA staging image + if: inputs.build_ota_image + uses: actions/upload-artifact@v4 + with: + name: ota-image + path: ota-image + retention-days: 7 diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 8e0ac02c..ae474ca0 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -2,28 +2,9 @@ name: ci on: pull_request: - # `labeled` is not in the default set. Without it, adding `test-ota` to an - # open PR does nothing and the OTA job can only be reached by pushing a - # commit after the label is already on. Listing the types restores the - # defaults and makes labelling an actual trigger. - types: - - opened - - synchronize - - reopened - - labeled push: branches: - main - workflow_dispatch: - inputs: - run_ota: - description: "Run the hardware over-the-air update test" - type: boolean - default: false - ota_uplink_cooldown: - description: "Seconds between file-uplink chunks during the OTA test (repo default is 0.400)" - type: string - default: "0.400" jobs: lint: @@ -131,40 +112,6 @@ jobs: yamcs/yamcs-data/mdb/fprime.xtce.xml retention-days: 30 - # --- OTA staging image ------------------------------------------------- - # The OTA test has to prove a *swap* happened, not merely that the update - # commands returned OK, so the image it uplinks must be distinguishable - # from the one flashed on the board. Project version comes from - # `git describe --tags --always --dirty`, so a throwaway tag renames this - # build and changes nothing else about it. - # - # The rm is required: fprime generates version.{hpp,cpp,json} from a cmake - # custom command with no declared inputs, so ninja considers it up to date - # as soon as the outputs exist and a rebuild would keep the old string. - # - # This runs after the artifact upload above, so overwriting bootable.* here - # cannot affect the image the other integration jobs flash. - - name: Build OTA staging image - if: (github.event_name == 'workflow_dispatch' && inputs.run_ota) || contains(github.event.pull_request.labels.*.name, 'test-ota') - run: | - git tag "ota-ci-${GITHUB_RUN_ID}" - rm -f build-fprime-automatic-zephyr/versions/version.hpp \ - build-fprime-automatic-zephyr/versions/version.cpp \ - build-fprime-automatic-zephyr/versions/version.json - BUILD_YAMCS_MDB=0 make build - mkdir -p ota-image - cp build-artifacts/zephyr.signed.bin ota-image/ - cp build-fprime-automatic-zephyr/versions/version.json ota-image/ - echo "OTA staging image version: $(cat ota-image/version.json)" - - - name: Upload OTA staging image - if: (github.event_name == 'workflow_dispatch' && inputs.run_ota) || contains(github.event.pull_request.labels.*.name, 'test-ota') - uses: actions/upload-artifact@v4 - with: - name: ota-image - path: ota-image - retention-days: 7 - integration-uart: runs-on: - integration @@ -645,198 +592,6 @@ jobs: [ -f server-rf.pid ] && kill $(cat server-rf.pid) || true ~/korad_control/.venv/bin/python ~/korad_control/korad_control.py -d /dev/ttyPWR --output 0 - integration-ota: - # Not part of the normal PR run. It monopolises the single integration cube - # for the better part of an hour (a 1.4 MB uplink at the default cooldown is - # ~48 min on its own), erases a flash slot, and reboots the board twice. - # Trigger it deliberately: run the workflow manually with "Run the hardware - # over-the-air update test" checked, or put the `test-ota` label on a PR. - if: (github.event_name == 'workflow_dispatch' && inputs.run_ota) || contains(github.event.pull_request.labels.*.name, 'test-ota') - runs-on: - - integration - needs: build - timeout-minutes: 150 - steps: - - uses: actions/checkout@v6 - - - uses: actions/download-artifact@v6 - with: - name: artifacts - path: . - - - uses: actions/download-artifact@v6 - with: - name: ota-image - path: ota-image - - - name: Power On Satellite - run: | - # Every other integration job's first hardware action is the - # flash-firmware composite, which powers the cube up before it talks - # to OpenOCD. This job reaches for OpenOCD first, and every job on - # this runner ends with "Power Off Satellite", so without this the - # erase below would run against an unpowered board and fail to - # attach. - ~/korad_control/.venv/bin/python ~/korad_control/korad_control.py -d /dev/ttyPWR --output 1 - sleep 3 - - - name: Erase the MCUboot staging slot - run: | - # slot1_partition is 0x200000..0x300000 within the 4 MB QSPI flash - # mapped at 0x10000000 (see - # boards/bronco_space/proves_flight_control_board_v5/proves_flight_control_board_v5.dtsi). - # - # A previous run can leave a staged image plus swap flags in slot1. If - # those survive, MCUboot may swap it in on the first boot after we - # flash the primary slot below, and the job would silently test the - # wrong firmware. Erasing first makes the starting state unambiguous. - ~/openocd/src/openocd -s ~/openocd/tcl \ - -f ~/openocd/tcl/interface/cmsis-dap.cfg \ - -f ~/openocd/tcl/target/rp2350.cfg \ - -c "adapter speed 5000" \ - -c "init; halt; flash erase_address 0x10200000 0x100000; reset run; exit" - - - name: Flash Firmware - uses: ./.github/actions/flash-firmware - - - name: Load .env file - run: | - while IFS= read -r line || [ -n "$line" ]; do - # Skip comments and empty lines - [[ "$line" =~ ^#.*$ || -z "$line" ]] && continue - echo "$line" >> $GITHUB_ENV - done < ~/actions-runner/.env - - - name: Set up dependencies - run: | - make submodules fprime-venv - - - name: Apply CI spacecraft ID overrides - run: | - make make-ci-spacecraft-id - - - name: Set Authentication Key - env: - AUTH_KEY: ${{ secrets.AUTH_KEY }} - run: | - echo "#define AUTH_DEFAULT_KEY \"$AUTH_KEY\"" > PROVESFlightControllerReference/Components/TcSecurityDeframer/AuthDefaultKey.h - - - name: Install Framer Plugin - run: | - make framer-plugin - - - name: Record the staged image version - env: - # Quoted through the environment rather than interpolated into the - # shell, so a workflow input cannot inject a command. - UPLINK_COOLDOWN: ${{ inputs.ota_uplink_cooldown || '0.400' }} - run: | - OTA_VERSION=$(./fprime-venv/bin/python -c \ - 'import json; print(json.load(open("ota-image/version.json"))["project_version"])') - echo "Board will be flashed with the default build; OTA will uplink: $OTA_VERSION" - echo "OTA_VERSION=$OTA_VERSION" >> $GITHUB_ENV - echo "GDS_EXTRA_ARGS=--file-uplink-cooldown $UPLINK_COOLDOWN" >> $GITHUB_ENV - - - name: Detect Board TTY - run: | - ZEPHYR_TTY=$(./tools/ci/detect-board-tty.sh) - echo "Selected Zephyr board tty: $ZEPHYR_TTY" - echo "UART_DEVICE=$ZEPHYR_TTY" >> $GITHUB_ENV - - - name: Kill all python processes - run: | - pkill -9 python || true - - - name: Start GDS - run: | - nohup make gds-integration UART_DEVICE="$UART_DEVICE" GDS_EXTRA_ARGS="$GDS_EXTRA_ARGS" > gds-bootstrap.log 2>&1 & - echo $! > server.pid - - - name: Sync Sequence Number - run: | - make test-integration FILTER=sync_sequence_number - - - name: Format Filesystem - run: | - # The uplinked image is ~1.4 MB and lands in /update. Starting from a - # formatted filesystem keeps a previous run's copy from filling it. - make test-integration FILTER=format_filesystem - - - name: Power-Cycle Satellite - run: | - # /seq, /antenna and friends are only recreated at boot, so the format - # above has to be followed by a reboot before anything else runs. - [ -f server.pid ] && kill $(cat server.pid) || true - pkill -9 python || true - ~/korad_control/.venv/bin/python ~/korad_control/korad_control.py -d /dev/ttyPWR --output 0 - sleep 3 - ~/korad_control/.venv/bin/python ~/korad_control/korad_control.py -d /dev/ttyPWR --output 1 - sleep 3 - - - name: Detect Board TTY - run: | - ZEPHYR_TTY=$(./tools/ci/detect-board-tty.sh) - echo "Selected Zephyr board tty: $ZEPHYR_TTY" - echo "UART_DEVICE=$ZEPHYR_TTY" >> $GITHUB_ENV - - - name: Start GDS - run: | - nohup make gds-integration UART_DEVICE="$UART_DEVICE" GDS_EXTRA_ARGS="$GDS_EXTRA_ARGS" > gds.log 2>&1 & - echo $! > server.pid - - - name: Sync Sequence Number - run: | - make test-integration FILTER=sync_sequence_number - - - name: Run OTA Integration Test - run: | - make test-integration TEST=ota_test.py FILTER=ota \ - PYTEST_ARGS="--ota-image=ota-image/zephyr.signed.bin --ota-expect-version=$OTA_VERSION" - - - name: Upload GDS logs - if: always() - uses: actions/upload-artifact@v4 - with: - name: gds-log-ota - path: | - gds-bootstrap.log - gds.log - logs - !logs/**/*.xlsx - if-no-files-found: ignore - retention-days: 7 - - - name: Kill GDS - if: always() - run: | - [ -f server.pid ] && kill $(cat server.pid) || true - pkill -9 python || true - fuser -k /dev/ttyBOARD 2>/dev/null || true - sleep 2 - - - name: Restore the board - if: always() - run: | - # The board is left running the OTA staging image, and slot1 holds - # whatever the swap displaced. Put both slots back to a known state so - # the next job on this runner starts from the artifact build, not from - # a leftover OTA image. - ~/openocd/src/openocd -s ~/openocd/tcl \ - -f ~/openocd/tcl/interface/cmsis-dap.cfg \ - -f ~/openocd/tcl/target/rp2350.cfg \ - -c "adapter speed 5000" \ - -c "init; halt; flash erase_address 0x10200000 0x100000; reset run; exit" || true - ~/openocd/src/openocd -s ~/openocd/tcl \ - -f ~/openocd/tcl/interface/cmsis-dap.cfg \ - -f ~/openocd/tcl/target/rp2350.cfg \ - -c "adapter speed 5000" \ - -c "program bootable.signed.hex verify reset exit" || true - - - name: Power Off Satellite - if: always() - run: | - ~/korad_control/.venv/bin/python ~/korad_control/korad_control.py -d /dev/ttyPWR --output 0 - yamcs-build: runs-on: ubuntu-latest needs: build diff --git a/.github/workflows/ota.yaml b/.github/workflows/ota.yaml new file mode 100644 index 00000000..8460c1e6 --- /dev/null +++ b/.github/workflows/ota.yaml @@ -0,0 +1,212 @@ +name: ota + +# The hardware over-the-air update test lives in its own workflow because it +# monopolises the single integration cube for the better part of an hour (a +# 1.4 MB uplink at the default cooldown is ~48 min on its own), erases a flash +# slot, and reboots the board twice. Running it alongside every PR would starve +# the normal integration jobs, so it runs on a nightly schedule and on demand. +on: + schedule: + # 10:00 UTC daily. GitHub only runs schedules from the default branch, so + # edits here take effect once they land on main. + - cron: "0 10 * * *" + workflow_dispatch: + inputs: + ota_uplink_cooldown: + description: "Seconds between file-uplink chunks during the OTA test (repo default is 0.400)" + type: string + default: "0.400" + +jobs: + build: + uses: ./.github/workflows/build.yaml + with: + build_ota_image: true + secrets: inherit + + integration-ota: + runs-on: + - integration + needs: build + timeout-minutes: 150 + steps: + - uses: actions/checkout@v6 + + - uses: actions/download-artifact@v6 + with: + name: artifacts + path: . + + - uses: actions/download-artifact@v6 + with: + name: ota-image + path: ota-image + + - name: Power On Satellite + run: | + # Every other integration job's first hardware action is the + # flash-firmware composite, which powers the cube up before it talks + # to OpenOCD. This job reaches for OpenOCD first, and every job on + # this runner ends with "Power Off Satellite", so without this the + # erase below would run against an unpowered board and fail to + # attach. + ~/korad_control/.venv/bin/python ~/korad_control/korad_control.py -d /dev/ttyPWR --output 1 + sleep 3 + + - name: Erase the MCUboot staging slot + run: | + # slot1_partition is 0x200000..0x300000 within the 4 MB QSPI flash + # mapped at 0x10000000 (see + # boards/bronco_space/proves_flight_control_board_v5/proves_flight_control_board_v5.dtsi). + # + # A previous run can leave a staged image plus swap flags in slot1. If + # those survive, MCUboot may swap it in on the first boot after we + # flash the primary slot below, and the job would silently test the + # wrong firmware. Erasing first makes the starting state unambiguous. + ~/openocd/src/openocd -s ~/openocd/tcl \ + -f ~/openocd/tcl/interface/cmsis-dap.cfg \ + -f ~/openocd/tcl/target/rp2350.cfg \ + -c "adapter speed 5000" \ + -c "init; halt; flash erase_address 0x10200000 0x100000; reset run; exit" + + - name: Flash Firmware + uses: ./.github/actions/flash-firmware + + - name: Load .env file + run: | + while IFS= read -r line || [ -n "$line" ]; do + # Skip comments and empty lines + [[ "$line" =~ ^#.*$ || -z "$line" ]] && continue + echo "$line" >> $GITHUB_ENV + done < ~/actions-runner/.env + + - name: Set up dependencies + run: | + make submodules fprime-venv + + - name: Apply CI spacecraft ID overrides + run: | + make make-ci-spacecraft-id + + - name: Set Authentication Key + env: + AUTH_KEY: ${{ secrets.AUTH_KEY }} + run: | + echo "#define AUTH_DEFAULT_KEY \"$AUTH_KEY\"" > PROVESFlightControllerReference/Components/TcSecurityDeframer/AuthDefaultKey.h + + - name: Install Framer Plugin + run: | + make framer-plugin + + - name: Record the staged image version + env: + # Quoted through the environment rather than interpolated into the + # shell, so a workflow input cannot inject a command. The scheduled + # run has no inputs, so it falls through to the repo default. + UPLINK_COOLDOWN: ${{ inputs.ota_uplink_cooldown || '0.400' }} + run: | + OTA_VERSION=$(./fprime-venv/bin/python -c \ + 'import json; print(json.load(open("ota-image/version.json"))["project_version"])') + echo "Board will be flashed with the default build; OTA will uplink: $OTA_VERSION" + echo "OTA_VERSION=$OTA_VERSION" >> $GITHUB_ENV + echo "GDS_EXTRA_ARGS=--file-uplink-cooldown $UPLINK_COOLDOWN" >> $GITHUB_ENV + + - name: Detect Board TTY + run: | + ZEPHYR_TTY=$(./tools/ci/detect-board-tty.sh) + echo "Selected Zephyr board tty: $ZEPHYR_TTY" + echo "UART_DEVICE=$ZEPHYR_TTY" >> $GITHUB_ENV + + - name: Kill all python processes + run: | + pkill -9 python || true + + - name: Start GDS + run: | + nohup make gds-integration UART_DEVICE="$UART_DEVICE" GDS_EXTRA_ARGS="$GDS_EXTRA_ARGS" > gds-bootstrap.log 2>&1 & + echo $! > server.pid + + - name: Sync Sequence Number + run: | + make test-integration FILTER=sync_sequence_number + + - name: Format Filesystem + run: | + # The uplinked image is ~1.4 MB and lands in /update. Starting from a + # formatted filesystem keeps a previous run's copy from filling it. + make test-integration FILTER=format_filesystem + + - name: Power-Cycle Satellite + run: | + # /seq, /antenna and friends are only recreated at boot, so the format + # above has to be followed by a reboot before anything else runs. + [ -f server.pid ] && kill $(cat server.pid) || true + pkill -9 python || true + ~/korad_control/.venv/bin/python ~/korad_control/korad_control.py -d /dev/ttyPWR --output 0 + sleep 3 + ~/korad_control/.venv/bin/python ~/korad_control/korad_control.py -d /dev/ttyPWR --output 1 + sleep 3 + + - name: Detect Board TTY + run: | + ZEPHYR_TTY=$(./tools/ci/detect-board-tty.sh) + echo "Selected Zephyr board tty: $ZEPHYR_TTY" + echo "UART_DEVICE=$ZEPHYR_TTY" >> $GITHUB_ENV + + - name: Start GDS + run: | + nohup make gds-integration UART_DEVICE="$UART_DEVICE" GDS_EXTRA_ARGS="$GDS_EXTRA_ARGS" > gds.log 2>&1 & + echo $! > server.pid + + - name: Sync Sequence Number + run: | + make test-integration FILTER=sync_sequence_number + + - name: Run OTA Integration Test + run: | + make test-integration TEST=ota_test.py FILTER=ota \ + PYTEST_ARGS="--ota-image=ota-image/zephyr.signed.bin --ota-expect-version=$OTA_VERSION" + + - name: Upload GDS logs + if: always() + uses: actions/upload-artifact@v4 + with: + name: gds-log-ota + path: | + gds-bootstrap.log + gds.log + logs + !logs/**/*.xlsx + if-no-files-found: ignore + retention-days: 7 + + - name: Kill GDS + if: always() + run: | + [ -f server.pid ] && kill $(cat server.pid) || true + pkill -9 python || true + fuser -k /dev/ttyBOARD 2>/dev/null || true + sleep 2 + + - name: Restore the board + if: always() + run: | + # The board is left running the OTA staging image, and slot1 holds + # whatever the swap displaced. Put both slots back to a known state so + # the next job on this runner starts from the artifact build, not from + # a leftover OTA image. + ~/openocd/src/openocd -s ~/openocd/tcl \ + -f ~/openocd/tcl/interface/cmsis-dap.cfg \ + -f ~/openocd/tcl/target/rp2350.cfg \ + -c "adapter speed 5000" \ + -c "init; halt; flash erase_address 0x10200000 0x100000; reset run; exit" || true + ~/openocd/src/openocd -s ~/openocd/tcl \ + -f ~/openocd/tcl/interface/cmsis-dap.cfg \ + -f ~/openocd/tcl/target/rp2350.cfg \ + -c "adapter speed 5000" \ + -c "program bootable.signed.hex verify reset exit" || true + + - name: Power Off Satellite + if: always() + run: | + ~/korad_control/.venv/bin/python ~/korad_control/korad_control.py -d /dev/ttyPWR --output 0 diff --git a/README.md b/README.md index 22d654dd..b9e29801 100644 --- a/README.md +++ b/README.md @@ -322,12 +322,12 @@ make test-integration TEST=ota_test.py FILTER=ota \ PYTEST_ARGS="--ota-image=/path/to/other/zephyr.signed.bin --ota-expect-version=v1.2.3" ``` -In CI this is the `integration-ota` job. It is not part of the normal PR run — -it ties up the integration cube for the better part of an hour. Trigger it by -running the `ci` workflow manually with **Run the hardware over-the-air update -test** checked, or by adding the `test-ota` label to a PR. The job builds a -second image under a throwaway git tag so the project version differs from the -one flashed on the board, which is what lets it prove a swap actually happened. +In CI this lives in its own `ota` workflow, separate from `ci`, because it ties +up the integration cube for the better part of an hour. It runs on a schedule at +10:00 UTC daily, and can be triggered on demand by running the `ota` workflow +manually. The workflow builds a second image under a throwaway git tag so the +project version differs from the one flashed on the board, which is what lets it +prove a swap actually happened. Uplink dominates the runtime: at the `fprime-gds.yml` defaults (`file-uplink-chunk-size: 204`, `file-uplink-cooldown: 0.400`) a 1.4 MB image is From d7f503fbbcd23c3bc6ff741497e392481b2e2012 Mon Sep 17 00:00:00 2001 From: Nate Gay Date: Tue, 4 Aug 2026 22:54:01 +0200 Subject: [PATCH 11/11] test(ota): cover the revert ending, and make the trial image a make target 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. --- .github/workflows/build.yaml | 27 +-- .github/workflows/ota.yaml | 5 +- Makefile | 37 +++ .../test/int/ota_test.py | 228 ++++++++++++------ README.md | 29 ++- 5 files changed, 219 insertions(+), 107 deletions(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index b3db011e..6cd30a11 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -91,30 +91,17 @@ jobs: retention-days: 30 # --- OTA staging image ------------------------------------------------- - # The OTA test has to prove a *swap* happened, not merely that the update - # commands returned OK, so the image it uplinks must be distinguishable - # from the one flashed on the board. Project version comes from - # `git describe --tags --always --dirty`, so a throwaway tag renames this - # build and changes nothing else about it. + # A second build of the same sources under a throwaway tag, so the image + # the OTA test uplinks reports a different project version than the one + # flashed on the board. See the `ota-test-image` target for why that + # matters and why the version files have to be deleted first. # - # The rm is required: fprime generates version.{hpp,cpp,json} from a cmake - # custom command with no declared inputs, so ninja considers it up to date - # as soon as the outputs exist and a rebuild would keep the old string. - # - # This runs after the artifact upload above, so overwriting bootable.* here - # cannot affect the image the other integration jobs flash. + # This runs after the artifact upload above, so the rebuilt + # build-artifacts/ cannot affect what the other integration jobs flash. - name: Build OTA staging image if: inputs.build_ota_image run: | - git tag "ota-ci-${GITHUB_RUN_ID}" - rm -f build-fprime-automatic-zephyr/versions/version.hpp \ - build-fprime-automatic-zephyr/versions/version.cpp \ - build-fprime-automatic-zephyr/versions/version.json - BUILD_YAMCS_MDB=0 make build - mkdir -p ota-image - cp build-artifacts/zephyr.signed.bin ota-image/ - cp build-fprime-automatic-zephyr/versions/version.json ota-image/ - echo "OTA staging image version: $(cat ota-image/version.json)" + make ota-test-image OTA_TAG="ota-ci-${GITHUB_RUN_ID}" - name: Upload OTA staging image if: inputs.build_ota_image diff --git a/.github/workflows/ota.yaml b/.github/workflows/ota.yaml index 8460c1e6..a3fedb50 100644 --- a/.github/workflows/ota.yaml +++ b/.github/workflows/ota.yaml @@ -28,7 +28,10 @@ jobs: runs-on: - integration needs: build - timeout-minutes: 150 + # Uplink alone is ~48 min at the default cooldown; on top of that the suite + # writes the staging slot twice (once for the revert path, once for the + # confirm path) and reboots the board four times. + timeout-minutes: 180 steps: - uses: actions/checkout@v6 diff --git a/Makefile b/Makefile index d0a07314..48ec5ead 100644 --- a/Makefile +++ b/Makefile @@ -148,6 +148,43 @@ build: submodules zephyr fprime-venv generate-if-needed ## Build FPrime-Zephyr P mv ./build-artifacts/zephyr.signed.hex bootable.signed.hex @if [ "$(BUILD_YAMCS_MDB)" = "1" ]; then $(MAKE) yamcs-mdb; else echo "Skipping yamcs-mdb (BUILD_YAMCS_MDB=$(BUILD_YAMCS_MDB))"; fi +# --- OTA staging image ------------------------------------------------------ +# The OTA test proves a *swap* really happened by asserting the board reports +# the uplinked image's project version after the reboot, so that image has to +# be distinguishable from the one already flashed -- uplinking the running +# build makes the assertion vacuously true. +# +# Project version comes from `git describe --tags --always --dirty`, so a +# throwaway tag renames the build and changes nothing else about it: same +# sources, same config, same signing key. The tag is deleted again on the way +# out, including on failure. +# +# `fprime-util build` is invoked directly rather than through `make build` so +# bootable.uf2 / bootable.signed.hex keep pointing at the build you flash; +# only build-artifacts/zephyr.signed.bin and the versions/ metadata move. +OTA_IMAGE_DIR ?= $(shell pwd)/ota-image +OTA_TAG ?= ota-$(shell date -u +%m%d%H%M%S) + +.PHONY: ota-test-image +ota-test-image: submodules zephyr fprime-venv generate-if-needed ## Build a distinctly-versioned signed image for the OTA test (OTA_TAG=) + @if git rev-parse -q --verify "refs/tags/$(OTA_TAG)" >/dev/null; then \ + echo "Error: tag $(OTA_TAG) already exists; pass OTA_TAG="; \ + exit 1; \ + fi + @git tag "$(OTA_TAG)" + @trap 'git tag -d "$(OTA_TAG)" >/dev/null 2>&1 || true' EXIT INT TERM; \ + echo "Building OTA image as version $$(git describe --tags --always --dirty --broken)"; \ + rm -f $(BUILD_DIR)/versions/version.hpp \ + $(BUILD_DIR)/versions/version.cpp \ + $(BUILD_DIR)/versions/version.json; \ + $(UV_RUN) fprime-util build || exit 1; \ + mkdir -p $(OTA_IMAGE_DIR); \ + cp build-artifacts/zephyr.signed.bin $(OTA_IMAGE_DIR)/; \ + cp $(BUILD_DIR)/versions/version.json $(OTA_IMAGE_DIR)/; \ + echo "OTA image: $(OTA_IMAGE_DIR)/zephyr.signed.bin"; \ + echo "Its version: $$($(UV_RUN) python3 -c \ + 'import json; print(json.load(open("$(OTA_IMAGE_DIR)/version.json"))["project_version"])')" + .PHONY: check-console-disabled ZEPHYR_CONFIG ?= $(BUILD_DIR)/zephyr/.config check-console-disabled: uv ## Fail if the Zephyr UART console is enabled (it corrupts the F' downlink); run after 'make build' diff --git a/PROVESFlightControllerReference/test/int/ota_test.py b/PROVESFlightControllerReference/test/int/ota_test.py index f82a0781..68814a85 100644 --- a/PROVESFlightControllerReference/test/int/ota_test.py +++ b/PROVESFlightControllerReference/test/int/ota_test.py @@ -15,6 +15,13 @@ 6. ``CONFIRM_UPDATE`` -- make the swap permanent. Without this, the *next* reboot reverts to the previous image. +Both outcomes of step 6 are covered, because both are flight-critical: a good +image must stick, and a bad one must roll itself back without a ground pass. +The trial image is staged twice from a single uplink -- once left unconfirmed +to observe the revert, once confirmed to observe it stick -- since re-flashing +the staging slot costs minutes while re-uplinking costs the better part of an +hour. + The whole module is marked ``ota`` and is excluded from the default integration run: it erases a flash slot, uplinks a ~1.4 MB file, and reboots the board. Run it deliberately: @@ -119,6 +126,12 @@ # longer is truncated in flight before it reaches telemetry. VERSION_STRING_SIZE = 40 +# The version the board was running before the update, captured by test_03 and +# asserted by test_04 after the revert. Module state rather than a fixture +# because it has to outlive a test, and the value can only be read from the +# board while the pre-update image is still the one running. +_pre_update_version: str | None = None + def send_and_confirm_dispatch( fprime_test_api: IntegrationTestAPI, @@ -248,6 +261,67 @@ def fprime_crc32(path: Path) -> int: return ~crc & 0xFFFFFFFF +def stage_image( + fprime_test_api: IntegrationTestAPI, destination: str, crc32: int +) -> None: + """Erase the staging slot and copy an already-uplinked image into it. + + The CRC is checked flight-side before a single byte is written, so a + mismatch here means the file on the filesystem is not what ground sent. + """ + prepare_update(fprime_test_api) + + send_and_confirm_dispatch( + fprime_test_api, + f"{updater}.UPDATE_IMAGE_FROM", + f"{updater}.Update", + [destination, str(crc32)], + ) + outcome = await_outcome( + fprime_test_api, + [f"{updater}.UpdateSucceeded", f"{updater}.UpdateFailed"], + timeout=IMAGE_WRITE_TIMEOUT_S, + ) + assert outcome is not None, ( + "image write to the staging slot reported neither success nor failure" + ) + assert outcome.template.get_name() == "UpdateSucceeded", ( + f"image write to the staging slot failed: {outcome.get_str()}" + ) + + +def arm_trial_boot(fprime_test_api: IntegrationTestAPI) -> None: + """Mark the staged image for a one-shot trial boot. + + TEST rather than PERMANENT so that an image that fails to come up reverts + on the following reboot instead of stranding the board. + """ + fprime_test_api.clear_histories() + proves_send_and_assert_command( + fprime_test_api, f"{updater}.CONFIGURE_NEXT_BOOT", ["TEST"] + ) + fprime_test_api.assert_event(f"{updater}.SetNextBoot", timeout=10) + + +def reboot(fprime_test_api: IntegrationTestAPI, why: str) -> None: + """Cold-reset the board and wait for the boot-time version events. + + COLD_RESET is sent without expecting a completion: the board reboots before + it can be acknowledged. FrameworkVersion is emitted at startup, so a fresh + one after the send time is what proves the restart happened. + """ + start: TimeType = TimeType().set_datetime( + datetime.now(), time_base=TimeType.TimeBase("TB_DONT_CARE") + ) + fprime_test_api.send_command("ReferenceDeployment.resetManager.COLD_RESET") + assert ( + fprime_test_api.await_event( + f"{version}.FrameworkVersion", start=start, timeout=SWAP_REBOOT_TIMEOUT_S + ) + is not None + ), f"board did not come back after the {why} reboot" + + def read_project_version(fprime_test_api: IntegrationTestAPI) -> str: """Ask the running image which project version it is.""" proves_send_and_assert_command(fprime_test_api, f"{version}.VERSION", ["PROJECT"]) @@ -276,18 +350,21 @@ def ota_image(request: pytest.FixtureRequest) -> Path: def expected_version(request: pytest.FixtureRequest, ota_image: Path) -> str: """The project version the board must report once ``ota_image`` is running. - Taken from ``--ota-expect-version`` when given, otherwise from the - ``version.json`` the F Prime build writes next to the image. Either way it - is checked against the image bytes, so a stale version.json cannot quietly - turn the post-reboot assertion into a no-op. + Taken from ``--ota-expect-version`` when given, otherwise from a + ``version.json`` beside the image (``make ota-test-image`` copies one there) + and failing that from the build tree. Either way it is checked against the + image bytes, so a stale version.json cannot quietly turn the post-reboot + assertion into a no-op. """ override = request.config.getoption("--ota-expect-version") if override: candidate = override else: - version_json = ( - Path("build-fprime-automatic-zephyr") / "versions" / "version.json" - ) + version_json = ota_image.parent / "version.json" + if not version_json.is_file(): + version_json = ( + Path("build-fprime-automatic-zephyr") / "versions" / "version.json" + ) if not version_json.is_file(): pytest.skip( f"{version_json} not found and --ota-expect-version not given; " @@ -348,22 +425,24 @@ def test_02_update_image_without_prepare_is_rejected( assert_board_responsive(fprime_test_api) -def test_03_full_ota_cycle( +def test_03_trial_image_boots_after_the_swap( fprime_test_api: IntegrationTestAPI, start_gds, ota_image: Path, expected_version: str, ): - """Stage an image, swap to it across a reboot, and confirm it.""" - before = read_project_version(fprime_test_api) - crc32 = fprime_crc32(ota_image) - destination = f"{UPDATE_DIR}/{ota_image.name}" + """Uplink an image, stage it as a trial boot, and swap to it. - # 1. Erase the staging slot. - prepare_update(fprime_test_api) + Leaves the board running the *unconfirmed* trial image, which is what + test_04 goes on to reboot out of, and leaves the uplinked file on the + filesystem for test_05 to re-stage without a second uplink. + """ + global _pre_update_version + _pre_update_version = read_project_version(fprime_test_api) + destination = f"{UPDATE_DIR}/{ota_image.name}" - # 2. Uplink the signed image. CreateDirectory is best-effort: /update may - # already exist from an earlier run, and fileManager errors on that. + # Uplink the signed image. CreateDirectory is best-effort: /update may + # already exist from an earlier run, and fileManager errors on that. fprime_test_api.send_command(f"{fileManager}.CreateDirectory", [UPDATE_DIR]) time.sleep(1) @@ -377,78 +456,75 @@ def test_03_full_ota_cycle( uplink_with_retry(fprime_test_api, ota_image, destination, UPLINK_TIMEOUT_S) - # 3. Copy it into the staging slot. The CRC is checked flight-side before a - # single byte is written, so a mismatch here means the uplink was lossy. - send_and_confirm_dispatch( - fprime_test_api, - f"{updater}.UPDATE_IMAGE_FROM", - f"{updater}.Update", - [destination, str(crc32)], - ) - outcome = await_outcome( - fprime_test_api, - [f"{updater}.UpdateSucceeded", f"{updater}.UpdateFailed"], - timeout=IMAGE_WRITE_TIMEOUT_S, + stage_image(fprime_test_api, destination, fprime_crc32(ota_image)) + arm_trial_boot(fprime_test_api) + reboot(fprime_test_api, "swap") + + after = read_project_version(fprime_test_api) + assert after == expected_version, ( + f"running version {after!r} after the swap, expected {expected_version!r} " + f"(was {_pre_update_version!r} before the update)" ) - assert outcome is not None, ( - "image write to the staging slot reported neither success nor failure" + + +def test_04_unconfirmed_image_reverts_on_the_next_reboot( + fprime_test_api: IntegrationTestAPI, + start_gds, + expected_version: str, +): + """An image that is never confirmed is rolled back by MCUboot. + + This is the property that makes a bad update survivable without a ground + pass: an image that comes up broken enough never to send CONFIRM_UPDATE + gets undone by the next watchdog reset on its own. test_03 deliberately + left the trial image unconfirmed so this reboot exercises that path. + """ + assert _pre_update_version is not None, ( + "test_03 did not run, so the version to revert to is unknown" ) - assert outcome.template.get_name() == "UpdateSucceeded", ( - f"image write to the staging slot failed: {outcome.get_str()}" + assert read_project_version(fprime_test_api) == expected_version, ( + "board is not running the trial image; test_03 must run first" ) - # 4. Arm the one-shot trial boot. TEST rather than PERMANENT so that a bad - # image reverts on the following reboot instead of stranding the board. - fprime_test_api.clear_histories() - proves_send_and_assert_command( - fprime_test_api, f"{updater}.CONFIGURE_NEXT_BOOT", ["TEST"] - ) - fprime_test_api.assert_event(f"{updater}.SetNextBoot", timeout=10) + reboot(fprime_test_api, "revert") - # 5. Reboot and let MCUboot perform the swap. - start: TimeType = TimeType().set_datetime( - datetime.now(), time_base=TimeType.TimeBase("TB_DONT_CARE") + reverted = read_project_version(fprime_test_api) + assert reverted == _pre_update_version, ( + f"running version {reverted!r} after the second reboot, expected the " + f"pre-update {_pre_update_version!r} -- an unconfirmed image did not revert" ) - fprime_test_api.send_command("ReferenceDeployment.resetManager.COLD_RESET") - assert ( - fprime_test_api.await_event( - f"{version}.FrameworkVersion", start=start, timeout=SWAP_REBOOT_TIMEOUT_S - ) - is not None - ), "board did not come back after the swap reboot" - # 6. The swapped-in image must be the one we uplinked. - after = read_project_version(fprime_test_api) - assert after == expected_version, ( - f"running version {after!r} after the swap, expected {expected_version!r} " - f"(was {before!r} before the update)" + +def test_05_confirmed_image_survives_the_next_reboot( + fprime_test_api: IntegrationTestAPI, + start_gds, + ota_image: Path, + expected_version: str, +): + """After CONFIRM_UPDATE the image sticks across the following reboot. + + The same image test_03 uplinked is still on the filesystem -- test_04 + reverted the flash slots, not the file -- so this re-stages from it rather + than paying for another uplink. + """ + destination = f"{UPDATE_DIR}/{ota_image.name}" + + stage_image(fprime_test_api, destination, fprime_crc32(ota_image)) + arm_trial_boot(fprime_test_api) + reboot(fprime_test_api, "second swap") + + assert read_project_version(fprime_test_api) == expected_version, ( + "trial image did not boot on the second swap" ) - # 7. Make it permanent. Skipping this would revert on the next reboot. fprime_test_api.clear_histories() proves_send_and_assert_command(fprime_test_api, f"{updater}.CONFIRM_UPDATE") fprime_test_api.assert_event(f"{updater}.ConfirmBoot", timeout=10) + reboot(fprime_test_api, "confirmation") -def test_04_survives_a_second_reboot(fprime_test_api: IntegrationTestAPI, start_gds): - """After CONFIRM_UPDATE the image sticks -- no revert on the next boot. - - This is the assertion that separates a confirmed update from a trial one: an - unconfirmed TEST image is rolled back by MCUboot here. - """ - expected = read_project_version(fprime_test_api) - - start: TimeType = TimeType().set_datetime( - datetime.now(), time_base=TimeType.TimeBase("TB_DONT_CARE") - ) - fprime_test_api.send_command("ReferenceDeployment.resetManager.COLD_RESET") - assert ( - fprime_test_api.await_event( - f"{version}.FrameworkVersion", start=start, timeout=SWAP_REBOOT_TIMEOUT_S - ) - is not None - ), "board did not come back after the confirmation reboot" - - assert read_project_version(fprime_test_api) == expected, ( - "image reverted after reboot -- CONFIRM_UPDATE did not take" + after = read_project_version(fprime_test_api) + assert after == expected_version, ( + f"running version {after!r} after the confirmation reboot, expected " + f"{expected_version!r} -- CONFIRM_UPDATE did not take" ) diff --git a/README.md b/README.md index b9e29801..1a373fe2 100644 --- a/README.md +++ b/README.md @@ -305,29 +305,38 @@ fprime-gds --file-uplink-cooldown 0.8 ### Testing it `PROVESFlightControllerReference/test/int/ota_test.py` runs this whole cycle -against real hardware and checks the reported project version after the swap. -It is excluded from the default integration run because it erases a flash slot, -uplinks a large file and reboots the board: +against real hardware, checks the reported project version after the swap, and +covers both endings: an unconfirmed image must revert on the next reboot, and a +confirmed one must stick. It is excluded from the default integration run +because it erases a flash slot, uplinks a large file and reboots the board: ```shell make test-integration TEST=ota_test.py FILTER=ota ``` -Run against the build in your working tree, it uplinks the image the board is -already running, so the post-swap version check passes trivially. To make the -check meaningful, point `--ota-image` at a *different* build: +Run against the build in your working tree, it would uplink the image the board +is already running, so the post-swap version check would pass trivially. Build a +distinctly-versioned image first — `make ota-test-image` rebuilds the same +sources under a throwaway git tag, so only the project version changes: ```shell +make build # flash bootable.uf2 / bootable.signed.hex +make ota-test-image # -> ota-image/{zephyr.signed.bin,version.json} make test-integration TEST=ota_test.py FILTER=ota \ - PYTEST_ARGS="--ota-image=/path/to/other/zephyr.signed.bin --ota-expect-version=v1.2.3" + PYTEST_ARGS="--ota-image=ota-image/zephyr.signed.bin" ``` +The expected version is read from the `version.json` written beside the image, +and is checked against the image bytes before the test runs, so a stale +version.json skips the test rather than turning the post-swap assertion into a +no-op. `--ota-expect-version` overrides it for an image built elsewhere. + In CI this lives in its own `ota` workflow, separate from `ci`, because it ties up the integration cube for the better part of an hour. It runs on a schedule at 10:00 UTC daily, and can be triggered on demand by running the `ota` workflow -manually. The workflow builds a second image under a throwaway git tag so the -project version differs from the one flashed on the board, which is what lets it -prove a swap actually happened. +manually. It uses the same `make ota-test-image` target to get an image whose +version differs from the one flashed on the board, which is what lets it prove a +swap actually happened. Uplink dominates the runtime: at the `fprime-gds.yml` defaults (`file-uplink-chunk-size: 204`, `file-uplink-cooldown: 0.400`) a 1.4 MB image is