From bb93c69828bb8fc85065a07f02ec63da6555c5b5 Mon Sep 17 00:00:00 2001 From: mikecarper Date: Fri, 16 Oct 2026 00:08:47 -0700 Subject: [PATCH 01/14] Preserve seeder progress counters when logs are unavailable --- tools/lora_ota/lora_ota.py | 2 +- tools/lora_ota/test_lora_ota.py | 50 +++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/tools/lora_ota/lora_ota.py b/tools/lora_ota/lora_ota.py index 6da1acc75..673cbc4f9 100755 --- a/tools/lora_ota/lora_ota.py +++ b/tools/lora_ota/lora_ota.py @@ -3956,7 +3956,7 @@ class SeederProcess: try: detail = self.log_path.read_text(encoding="utf-8", errors="replace") except OSError: - return 0, math.ceil(package.payload_size / package.block_size) + return 0, math.ceil(package.payload_size / package.block_size), 0 offsets = [ int(match) for match in re.findall(r"\bREAD\s+\d+\s+@(\d+)\s+OK\b", detail) diff --git a/tools/lora_ota/test_lora_ota.py b/tools/lora_ota/test_lora_ota.py index 49698e15a..845eed931 100644 --- a/tools/lora_ota/test_lora_ota.py +++ b/tools/lora_ota/test_lora_ota.py @@ -2543,6 +2543,40 @@ class DownloadSessionTests(unittest.TestCase): self.assertEqual(controller.commands, ["ota status"]) self.assertEqual(seeder.calls, total) + def test_monitor_uses_quiet_window_when_source_log_is_unavailable(self) -> None: + args = argparse.Namespace( + target="remote", transfer_timeout_minutes=1, poll_seconds=1, + ) + with tempfile.TemporaryDirectory() as directory: + seeder = object.__new__(ota.SeederProcess) + seeder.log_path = Path(directory) / "missing-serve.log" + seeder.log_file = None + seeder.process = mock.Mock() + seeder.process.poll.return_value = None + for unreadable in (False, True): + with self.subTest(unreadable=unreadable), contextlib.ExitStack() as stack: + if unreadable: + stack.enter_context(mock.patch.object( + Path, "read_text", side_effect=PermissionError("unreadable log"))) + elapsed = [0.0] + + def advance(seconds: float) -> None: + elapsed[0] += seconds + + stack.enter_context(mock.patch.object( + ota.time, "monotonic", side_effect=lambda: elapsed[0])) + stack.enter_context(mock.patch.object( + ota.time, "sleep", side_effect=advance)) + stack.enter_context(contextlib.redirect_stdout(io.StringIO())) + controller = self.Controller([ + f"OTA | download: ready to install 9/9 id={self.package.manifest_id} 2s" + ]) + status = ota.monitor_download(controller, args, self.package, seeder) + self.assertIn(self.package.manifest_id, status) + self.assertEqual(controller.commands, ["ota status"]) + self.assertEqual( + elapsed[0], ota.passive_progress_stall_seconds(args, self.package)) + def test_lost_pull_reply_is_resolved_without_blind_replay(self) -> None: controller = self.Controller([ "OTA | no download", @@ -5315,6 +5349,22 @@ class ReliabilityTests(unittest.TestCase): seeder.log_file = None self.assertEqual(seeder.payload_read_progress(package), (2, 3, 2)) + def test_seeder_progress_missing_log_keeps_all_three_counters(self) -> None: + package = ota.parse_mota(mota_blob(firmware(b"B" * 2500, VERSION_NEW))) + with tempfile.TemporaryDirectory() as directory: + seeder = object.__new__(ota.SeederProcess) + seeder.log_path = Path(directory) / "missing-serve.log" + seeder.log_file = None + self.assertEqual(seeder.payload_read_progress(package), (0, 3, 0)) + + def test_seeder_progress_unreadable_log_keeps_all_three_counters(self) -> None: + package = ota.parse_mota(mota_blob(firmware(b"B" * 2500, VERSION_NEW))) + seeder = object.__new__(ota.SeederProcess) + seeder.log_path = Path("unreadable-serve.log") + seeder.log_file = None + with mock.patch.object(Path, "read_text", side_effect=PermissionError("unreadable log")): + self.assertEqual(seeder.payload_read_progress(package), (0, 3, 0)) + def test_marked_output_excludes_queued_messages(self) -> None: controller = object.__new__(ota.Controller) controller._execute = lambda _commands, _label: subprocess.CompletedProcess( From 4846191fff10f12f79676c4217618220254dc8f0 Mon Sep 17 00:00:00 2001 From: mikecarper Date: Fri, 16 Oct 2026 00:10:57 -0700 Subject: [PATCH 02/14] Never roll beacon IDs back to an unrelated stale backup --- src/helpers/BeaconManager.cpp | 7 +++- test/fixtures/beacons/manager_test.cpp | 52 ++++++++++++++++++++++++++ test/test_beacon_manager.py | 11 ++++++ 3 files changed, 69 insertions(+), 1 deletion(-) diff --git a/src/helpers/BeaconManager.cpp b/src/helpers/BeaconManager.cpp index 484d4d327..3a58e76b0 100644 --- a/src/helpers/BeaconManager.cpp +++ b/src/helpers/BeaconManager.cpp @@ -254,14 +254,19 @@ BeaconManager::ReadResult BeaconManager::readIds(const char* path, uint32_t& las bool BeaconManager::commit(const char* path, const char* backup, const char* temporary, bool& hold) { bool active_present = false, backup_present = false; + bool active_backed_up = false; if (!filePresence(fs_, path, active_present) || !filePresence(fs_, backup, backup_present)) return false; if (active_present) { if (backup_present && !fs_->remove(backup)) return false; if (!fs_->rename(path, backup)) return false; + active_backed_up = true; backup_present = true; } if (!fs_->rename(temporary, path)) { - if (backup_present && !fs_->rename(backup, path)) hold = true; + // A pre-existing backup can be older than an already transmitted ID. + // Roll back only the active image moved by this transaction; promoting + // an unrelated backup could make that ID reusable after the next boot. + if (active_backed_up && !fs_->rename(backup, path)) hold = true; return false; } // A leftover backup after a successful commit is harmless. Never choose it diff --git a/test/fixtures/beacons/manager_test.cpp b/test/fixtures/beacons/manager_test.cpp index 3a264d872..ab8472504 100644 --- a/test/fixtures/beacons/manager_test.cpp +++ b/test/fixtures/beacons/manager_test.cpp @@ -350,6 +350,57 @@ void staleBackup() { && uninitialized.fs.files.at("/beacon_ids.bak") == stale); } +void liveBackupRollback() { + { + Fixture f; + f.mt(); + const auto stale = f.fs.files.at("/beacon_ids"); + f.tick(2 * H); + assert(f.mesh.sends.size() == 1 && f.mesh.sends[0].id == 0x1234567a); + // A failed backup cleanup can leave an older image. If the active entry + // disappears during this boot, a later failed promotion must not turn + // that unrelated image into authoritative metadata. + f.fs.files["/beacon_ids.bak"] = stale; + f.fs.files.erase("/beacon_ids"); + f.fs.fail_rename_from = {"/beacon_ids.tmp"}; + f.tick(24 * H); + assert(f.mesh.sends.size() == 1); + assert(!f.fs.files.count("/beacon_ids")); + assert(f.fs.files.at("/beacon_ids.bak") == stale); + f.fs.fail_rename_from.clear(); + f.tick(24 * H); // The failed counter write holds this boot too. + assert(f.mesh.sends.size() == 1); + f.manager.begin(&f.fs, &f.mesh, &f.clock, f.name); + f.tick(2 * H); + assert(f.mesh.sends.size() == 1); // Reboot must not reuse 0x1234567a. + f.cmd("set beacon 1 meshtext US LongFast 14 Still held", false); + f.mc(2); + f.tick(2 * H); + assert(f.mesh.sends.size() == 2 + && f.mesh.sends.back().slot.radio.protocol == mesh::RadioProfileProtocol::MeshCore); + } + { + Fixture f; + f.mc(1, "Old"); + const auto stale = f.fs.files.at("/beacons"); + f.mc(1, "Current"); + f.fs.files["/beacons.bak"] = stale; + f.fs.files.erase("/beacons"); + f.fs.fail_rename_from = {"/beacons.tmp"}; + f.cmd("set beacon 1 meshcore 907.5,125,8,6,10 New", false); + assert(!f.fs.files.count("/beacons")); + assert(f.fs.files.at("/beacons.bak") == stale); + assert(std::strstr(f.cmd("get beacon 1"), "Current")); + // Configuration can still commit a complete replacement on a later + // successful command, removing the stale backup after that commit. + f.fs.fail_rename_from.clear(); + f.cmd("set beacon 1 meshcore 907.5,125,8,6,10 New"); + assert(!f.fs.files.count("/beacons.bak")); + f.manager.begin(&f.fs, &f.mesh, &f.clock, f.name); + assert(std::strstr(f.cmd("get beacon 1"), "New")); + } +} + int main(int argc, char** argv) { assert(argc == 2); if (!std::strcmp(argv[1], "commands")) commands(); @@ -359,6 +410,7 @@ int main(int argc, char** argv) { else if (!std::strcmp(argv[1], "ids")) ids(); else if (!std::strcmp(argv[1], "mutations")) mutations(); else if (!std::strcmp(argv[1], "stale_backup")) staleBackup(); + else if (!std::strcmp(argv[1], "live_backup")) liveBackupRollback(); else assert(false); std::printf("beacon manager %s passed\n", argv[1]); } diff --git a/test/test_beacon_manager.py b/test/test_beacon_manager.py index b7b4ca4c5..f06b8d7b9 100644 --- a/test/test_beacon_manager.py +++ b/test/test_beacon_manager.py @@ -66,6 +66,9 @@ class BeaconManagerTest(unittest.TestCase): def test_missing_active_counter_never_rewinds_to_backup(self): self.run_scenario("stale_backup") + def test_failed_live_promotion_never_restores_an_unrelated_backup(self): + self.run_scenario("live_backup") + def negative_control(self, name, source, scenario): generated = Path(self.work.name) / (name + ".cpp") generated.write_text(source, encoding="ascii") @@ -97,6 +100,14 @@ class BeaconManagerTest(unittest.TestCase): self.assertEqual(source.count(held), 1) self.negative_control("rewound", source.replace(held, restored), "stale_backup") + def test_negative_control_live_counter_backup_rewind_is_detected(self): + source = (ROOT / "src/helpers/BeaconManager.cpp").read_text(encoding="ascii") + guard = "if (active_backed_up && !fs_->rename(backup, path))" + self.assertEqual(source.count(guard), 1) + self.negative_control("live_rewound", source.replace( + guard, "if ((active_backed_up || backup_present) && !fs_->rename(backup, path))"), + "live_backup") + if __name__ == "__main__": unittest.main() From 55557722f9b516e9f788feae5c1aa9af2ca9f236 Mon Sep 17 00:00:00 2001 From: mikecarper Date: Fri, 16 Oct 2026 00:11:35 -0700 Subject: [PATCH 03/14] Include MQTT consent in independent beacon wire vectors --- test/test_beacon_frames.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/test_beacon_frames.py b/test/test_beacon_frames.py index 0e4221bab..4599ed1cb 100644 --- a/test/test_beacon_frames.py +++ b/test/test_beacon_frames.py @@ -340,10 +340,13 @@ class BeaconFramesTest(unittest.TestCase): expected = b"\x15\0" + hashlib.sha256(PUBLIC_KEY).digest()[:1] + mac + encrypted self.run_case("public", expected.hex()) - def test_longfast_and_mediumfast_match_independent_aes_ctr_wire_oracle(self): + def test_longfast_and_mediumfast_match_independent_wire_and_mqtt_consent_oracle(self): for preset, hops in (("LongFast", 3), ("MediumFast", 7)): with self.subTest(preset=preset): - frame = reference_frame(data_text("Node: caf\u00e9".encode()), preset, DEFAULT_KEY, + # The text encoder also grants MQTT upload consent in Data + # field 9, bit 0. This is not the radio via_mqtt origin flag. + data = data_text("Node: caf\u00e9".encode()) + b"\x48\x01" + frame = reference_frame(data, preset, DEFAULT_KEY, flags=hops | (hops << 5)) self.run_case(preset.lower(), frame.hex()) From 6322b0765ff1ae736397ce0c9e7e2d316596f264 Mon Sep 17 00:00:00 2001 From: mikecarper Date: Fri, 16 Oct 2026 00:15:47 -0700 Subject: [PATCH 04/14] Classify OTA source errors without rejecting welcome names --- tools/lora_ota/lora_ota.py | 34 +++++++++-- .../lora_ota/test_lora_ota_source_identity.py | 60 +++++++++++++++++++ 2 files changed, 88 insertions(+), 6 deletions(-) diff --git a/tools/lora_ota/lora_ota.py b/tools/lora_ota/lora_ota.py index 673cbc4f9..f42ca0d12 100755 --- a/tools/lora_ota/lora_ota.py +++ b/tools/lora_ota/lora_ota.py @@ -3146,12 +3146,19 @@ def prompt_for_next_125khz_sf(args: argparse.Namespace) -> bool: return True +_SOURCE_TERMINAL_HEADER = re.compile( + r"(?m)^===== MeshCore (?:Full )?Companion Terminal =====[ \t]*$" +) +_SOURCE_CLI_ERROR_LINE = re.compile( + r"^[ \t]*(?:->[ \t]*)?(?:error|err|fail|unknown[ \t]+command)\b", + re.IGNORECASE | re.MULTILINE, +) + + def parse_source_terminal_banner(value: str) -> tuple[str, str]: """Read one Full/Companion welcome identity, not an advert or cached name.""" text = value.replace("\r", "") - headers = list(re.finditer( - r"(?m)^===== MeshCore (?:Full )?Companion Terminal =====[ \t]*$", text, - )) + headers = list(_SOURCE_TERMINAL_HEADER.finditer(text)) if len(headers) != 1: raise OtaError("OTA source did not expose one fresh Companion terminal banner") tail = text[headers[0].end():] @@ -3169,6 +3176,21 @@ def parse_source_terminal_banner(value: str) -> tuple[str, str]: return keys[0].lower(), tail[prompt.end():] +def _source_cli_response_text(value: str) -> str: + """Exclude a validated welcome banner from command reply classification.""" + text = value.replace("\r", "") + header = _SOURCE_TERMINAL_HEADER.search(text) + if header is None: + return text + _, response = parse_source_terminal_banner(text) + banner_end = len(text) - len(response) + # Preserve errors outside the welcome record and unexpected rejection + # lines mixed into it. A valid welcome key cannot hide a real failure. + banner_errors = [line for line in text[header.start():banner_end].splitlines() + if _SOURCE_CLI_ERROR_LINE.search(line)] + return "\n".join((text[:header.start()], *banner_errors, response)) + + def source_cli_command( args: argparse.Namespace, command_text: str, @@ -3319,9 +3341,9 @@ def source_cli_command( except OtaError as exc: raise TransmissionError(f"source CLI link failed: {exc}") from exc output = f"{result.stdout}\n{result.stderr}".strip() - lowered = output.lower() - if ("error" in lowered or "unknown command" in lowered or "err " in lowered - or re.search(r"(?:^|\n)\s*(?:->\s*)?(?:err|fail)(?:\b|:)", lowered)): + response_output = _source_cli_response_text(output) + lowered = response_output.lower() + if _SOURCE_CLI_ERROR_LINE.search(response_output): raise OtaError(f"source rejected {command_text!r}: {output}") if full_companion_identity: version_output = output.replace("\r", "") diff --git a/tools/lora_ota/test_lora_ota_source_identity.py b/tools/lora_ota/test_lora_ota_source_identity.py index f7ac64586..33fc56483 100644 --- a/tools/lora_ota/test_lora_ota_source_identity.py +++ b/tools/lora_ota/test_lora_ota_source_identity.py @@ -77,6 +77,66 @@ class SourceIdentityTests(unittest.TestCase): self.assertNotIn("get public.key", command[-1]) self.assertFalse(self.args.source_companion_terminal) # No persistent mode flag change. + def test_usb_identity_accepts_error_words_in_valid_welcome_names(self): + self.args.source_cli_tcp = None + self.args.source_serial = "COM42" + for name in ("Error Ridge", "ERR Station", "Unknown command Ridge"): + result = subprocess.CompletedProcess([], 0, banner(name=name) + VERSION + "\r\n", "") + with self.subTest(name=name), mock.patch.object(ota, "run_checked", return_value=result): + self.assertEqual(ota.read_source_public_key_bounded(self.args), KEY) + + def test_usb_ordinary_commands_accept_error_words_in_welcome_and_get_values(self): + self.args.source_cli_tcp = None + self.args.source_serial = "COM42" + for command, reply in (("ota status", "OTA seeder | install:disabled | serving:0"), + ("get name", "> Error Ridge")): + output = banner(name="Error Ridge") + "\r\n -> " + reply + "\r\n> " + result = subprocess.CompletedProcess([], 0, output, "") + with self.subTest(command=command), mock.patch.object(ota, "run_checked", return_value=result): + self.assertIn(reply, ota.source_cli_command(self.args, command)) + + def test_tcp_identity_and_ordinary_commands_accept_error_words_in_names(self): + for command, reply in (("ver", VERSION), ("get name", "> Error Ridge"), + ("ota status", "OTA seeder | install:disabled | serving:0")): + conn = connection(banner(name="Error Ridge"), reply) + with self.subTest(command=command), mock.patch.object( + ota.socket, "create_connection", return_value=conn): + if command == "ver": + self.assertEqual(ota.read_source_public_key_bounded(self.args), KEY) + else: + self.assertEqual(ota.source_cli_command(self.args, command), reply) + + def test_usb_real_rejections_survive_valid_welcome_and_identity_modes(self): + self.args.source_cli_tcp = None + self.args.source_serial = "COM42" + for reply in ("ERR permission denied", "ERROR: unavailable", "FAIL", "Unknown command"): + for identity in (False, True): + output = banner(name="Error Ridge") + "\r\n -> " + reply + "\r\n> " + result = subprocess.CompletedProcess([], 0, output, "") + with self.subTest(reply=reply, identity=identity), mock.patch.object( + ota, "run_checked", return_value=result): + with self.assertRaisesRegex(ota.OtaError, "source rejected"): + ota.source_cli_command(self.args, "ver" if identity else "ota status", + full_companion_identity=identity) + + def test_valid_usb_banner_cannot_hide_errors_before_or_inside_it(self): + self.args.source_cli_tcp = None + self.args.source_serial = "COM42" + welcome = banner(name="Error Ridge") + for output in ("ERROR: old mode failed\r\n" + welcome + VERSION, + welcome.replace("\r\n> ", "\r\nERR terminal unavailable\r\n> ") + VERSION): + result = subprocess.CompletedProcess([], 0, output, "") + with self.subTest(output=output), mock.patch.object(ota, "run_checked", return_value=result): + with self.assertRaisesRegex(ota.OtaError, "source rejected"): + ota.read_source_public_key_bounded(self.args) + + def test_tcp_real_rejections_remain_errors_after_valid_welcome(self): + for reply in ("ERR permission denied", "ERROR: unavailable", "FAIL", "Unknown command"): + conn = connection(banner(name="Error Ridge"), reply) + with self.subTest(reply=reply), mock.patch.object(ota.socket, "create_connection", return_value=conn): + with self.assertRaisesRegex(ota.OtaError, "source rejected"): + ota.source_cli_command(self.args, "ota status") + def test_usb_can_use_explicit_cli_port(self): self.args.source_cli_tcp = None self.args.source_cli_serial = "COM43" From edf3f85199d495f57d9704c723402f21adea20b4 Mon Sep 17 00:00:00 2001 From: mikecarper Date: Fri, 16 Oct 2026 00:15:55 -0700 Subject: [PATCH 05/14] Reject ambiguous management firmware versions without scanf --- src/helpers/ManagementReporter.cpp | 37 ++++++++++++--- test/fixtures/management/CommonCLI.h | 3 +- test/fixtures/management/runtime_test.cpp | 55 ++++++++++++++++++++++- test/test_management_report.py | 37 +++++++++++++++ 4 files changed, 125 insertions(+), 7 deletions(-) diff --git a/src/helpers/ManagementReporter.cpp b/src/helpers/ManagementReporter.cpp index 5ff817362..c84d7fe5e 100644 --- a/src/helpers/ManagementReporter.cpp +++ b/src/helpers/ManagementReporter.cpp @@ -34,6 +34,35 @@ static unsigned parseDays(const char* text) { } while (*text); return days; } + +// MGR2 has three or four decimal version bytes, followed only by an optional +// release/build label. Keep that label opaque for existing tagged builds. +static bool parseManagementFirmwareVersion(const char* text, uint32_t& version) { + if (!text) return false; + if (*text == 'v') ++text; + uint32_t packed = 0; + unsigned components = 0; + while (true) { + if (*text < '0' || *text > '9') return false; + unsigned component = 0; + do { + const unsigned digit = *text++ - '0'; + if (component > (255U - digit) / 10U) return false; + component = component * 10U + digit; + } while (*text >= '0' && *text <= '9'); + packed = (packed << 8) | component; + ++components; + if (*text != '.') break; + if (components == 4) return false; + ++text; + } + if (components < 3) return false; + if (*text && ((*text != '-' && *text != '+') || !text[1])) return false; + if (components == 3) packed <<= 8; + version = packed; + return true; +} + struct ManagementReporter::Working { History history; Extrema during_report; @@ -201,11 +230,9 @@ void ManagementReporter::snapshot() { #endif // If EndF is absent, only publish a version we can parse unambiguously. if (!(valid & FIRMWARE)) { - unsigned major = 0, minor = 0, patch = 0, pre = 0; - const char* version = callbacks.getFirmwareVer(); if (*version == 'v') ++version; - const int n = sscanf(version, "%u.%u.%u.%u", &major, &minor, &patch, &pre); - if (n >= 3 && major <= 255 && minor <= 255 && patch <= 255 && pre <= 255) { - write32(p + 28, major << 24 | minor << 16 | patch << 8 | pre); valid |= FIRMWARE; + uint32_t version; + if (parseManagementFirmwareVersion(callbacks.getFirmwareVer(), version)) { + write32(p + 28, version); valid |= FIRMWARE; } } write16(p + 76, valid); diff --git a/test/fixtures/management/CommonCLI.h b/test/fixtures/management/CommonCLI.h index 86517e792..b003154ef 100644 --- a/test/fixtures/management/CommonCLI.h +++ b/test/fixtures/management/CommonCLI.h @@ -29,7 +29,8 @@ public: }; class CommonCLICallbacks { public: - const char* getFirmwareVer() { return "v1.17.1.5"; } + const char* firmware_version = "v1.17.1.5"; + const char* getFirmwareVer() { return firmware_version; } const char* getRole() { return "repeater"; } }; struct LocationProvider { bool isEnabled() { return false; } }; diff --git a/test/fixtures/management/runtime_test.cpp b/test/fixtures/management/runtime_test.cpp index d379c70e4..8b4acbf11 100644 --- a/test/fixtures/management/runtime_test.cpp +++ b/test/fixtures/management/runtime_test.cpp @@ -58,7 +58,60 @@ struct Fixture { return legacy; } }; -int main() { +static void versionSnapshot(const char* version, bool valid_version, uint32_t packed = 0) { + Fixture f; + f.callbacks.firmware_version = version; + f.configure(); f.advance(5 * DAY + 60); + assert(f.mesh.packets.size() == 1); + const auto* header = f.mesh.packets[0].payload; + assert(!memcmp(header, "MGR2", 4)); + const bool reported_valid = (read16(header + 76) & FIRMWARE) != 0; + const uint32_t reported_version = read32(header + 28); + if (reported_valid != valid_version || reported_version != packed) + fprintf(stderr, "version %s: expected valid=%u packed=%08x; got valid=%u packed=%08x\n", + version ? version : "(null)", unsigned(valid_version), unsigned(packed), + unsigned(reported_valid), unsigned(reported_version)); + assert(reported_valid == valid_version && reported_version == packed); +} + +static void versionSnapshots() { + for (const char* version : {"1.2.3", "v1.2.3", "v001.002.003", + "v1.2.3-release", "v1.2.3+build_42", "v1.2.3-rc.1+build_42"}) + versionSnapshot(version, true, 0x01020300); + for (const char* version : {"1.2.3.4", "v1.2.3.004", "v1.2.3.4-preview1", + "v1.2.3.4+build_42", "v1.2.3.4-label_with_underscores"}) + versionSnapshot(version, true, 0x01020304); + versionSnapshot("v1.18.0", true, 0x01120000); // Direct-build example default. + versionSnapshot("v1.17.1.9-halo-keymind-cascade-dev-5f4d242f", true, 0x01110109); + versionSnapshot("v1.17.1.9-halo-keymind-cascade-dev.42-observer-dev-5f4d242f", true, 0x01110109); + versionSnapshot("v1.18.0.42-5f4d242f", true, 0x0112002a); // Three-part base + build number. + versionSnapshot("255.255.255.255", true, UINT32_MAX); + versionSnapshot("0.0.0", true); + versionSnapshot("v0.0.0.0+build_0", true); + for (const char* version : {"", "v", "1", "1.2", "1..3", "1.2.", "1.2.3.", + "1.2.3.4.5", "1.2.3.xyz", "1.2.3garbage", "1.2.3.4garbage", "1.2.3 arbitrary text", + "1.2.3-", "1.2.3+", "1.2.3.4-", "1.2.3.4+", "release-v1.2.3", + " 1.2.3", "1. 2.3", "1.2.3 ", "1.2.3. 4", "+1.2.3", "1.+2.3", "1.2.-3", + "1.2.3.+4", "1.2.3.-4", "256.0.0", "0.256.0", "0.0.256", "0.0.0.256", + "v4294967297.2.3", "v1.2.3.4294967297", "v-4294967295.2.3", "dev-5f4d242f"}) + versionSnapshot(version, false); + const std::string huge_component(4096, '9'); + versionSnapshot(("v" + huge_component + ".2.3").c_str(), false); + versionSnapshot(("v1.2.3." + huge_component).c_str(), false); + versionSnapshot(nullptr, false); +} + +int main(int argc, char** argv) { + if (argc == 2 && !strcmp(argv[1], "versions")) { + versionSnapshots(); + puts("PASS production MGR2 firmware version snapshots"); + return 0; + } + if (argc == 2 && !strcmp(argv[1], "version-malformed")) { + versionSnapshot("v1.2.3garbage", false); + return 0; + } + assert(argc == 1); { Fixture f; assert(f.cmd("get mgmt").find("direct=5d") != std::string::npos); diff --git a/test/test_management_report.py b/test/test_management_report.py index e50850703..7a2bb24df 100644 --- a/test/test_management_report.py +++ b/test/test_management_report.py @@ -18,6 +18,18 @@ spec = importlib.util.spec_from_file_location("management", ROOT / "tools/manage report = importlib.util.module_from_spec(spec) spec.loader.exec_module(report) +# Audited old snapshot fallback: the real libc conversion accepts a malformed +# numeric prefix as a valid-looking MGR2 version. Keep a negative runtime +# control so the snapshot regression proves it detects that bug. +LEGACY_VERSION_FALLBACK = r'''if (!(valid & FIRMWARE)) { + unsigned major = 0, minor = 0, patch = 0, pre = 0; + const char* version = callbacks.getFirmwareVer(); if (*version == 'v') ++version; + const int n = sscanf(version, "%u.%u.%u.%u", &major, &minor, &patch, &pre); + if (n >= 3 && major <= 255 && minor <= 255 && patch <= 255 && pre <= 255) { + write32(p + 28, major << 24 | minor << 16 | patch << 8 | pre); valid |= FIRMWARE; + } + }''' + class ManagementTests(unittest.TestCase): @classmethod @@ -76,6 +88,17 @@ class ManagementTests(unittest.TestCase): runtime_cmd += [str(cls.crypto / p) for p in sources] + ["-o", str(runtime)] subprocess.run(runtime_cmd, check=True, capture_output=True, text=True) cls.runtime = runtime + runtime_source = Path(cls.work.name) / "ManagementReporter.cpp" + runtime_text = runtime_source.read_text(encoding="ascii") + fallback = extract_braced(runtime_text, "if (!(valid & FIRMWARE))") + legacy_source = Path(cls.work.name) / "ManagementReporter-legacy.cpp" + legacy_source.write_text(runtime_text.replace(fallback, LEGACY_VERSION_FALLBACK, 1), + encoding="ascii") + cls.legacy_runtime = Path(cls.work.name) / "runtime-legacy-version" + legacy_cmd = list(runtime_cmd) + legacy_cmd[legacy_cmd.index(str(runtime_source))] = str(legacy_source) + legacy_cmd[-1] = str(cls.legacy_runtime) + subprocess.run(legacy_cmd, check=True, capture_output=True, text=True) @classmethod def tearDownClass(cls): @@ -437,6 +460,20 @@ process.stdin.on('end', async () => { def test_runtime_storage_scheduling_rollover_and_failures(self): subprocess.run([str(self.runtime)], check=True) + def test_production_snapshot_versions_are_bounded_and_release_compatible(self): + result = subprocess.run([str(self.runtime), "versions"], capture_output=True, + text=True, timeout=30) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("PASS production MGR2 firmware version snapshots", result.stdout) + + def test_previous_snapshot_parser_reports_malformed_input_as_a_valid_version(self): + result = subprocess.run([str(self.legacy_runtime), "version-malformed"], capture_output=True, + text=True, timeout=30) + self.assertNotEqual(result.returncode, 0) + self.assertIn("version v1.2.3garbage: expected valid=0", result.stderr) + self.assertIn("got valid=1", result.stderr) + self.assertIn("Assertion", result.stderr) + def test_actual_legacy_route_adoption_persistence_and_precedence(self): source = (ROOT / "src/helpers/CommonCLI_Management.cpp").read_text(encoding="utf-8") state = source[source.index("static constexpr char DATA_ROUTE_FILE[]"): From afbf8f9b9a04068e5f872423c34df47d8680f54b Mon Sep 17 00:00:00 2001 From: mikecarper Date: Fri, 16 Oct 2026 00:20:23 -0700 Subject: [PATCH 06/14] Keep seeder log inspection working after flush errors --- tools/lora_ota/lora_ota.py | 18 ++++++---- tools/lora_ota/test_lora_ota.py | 33 +++++++++++++++++-- .../lora_ota/test_lora_ota_seeder_startup.py | 33 +++++++++++++++++++ 3 files changed, 76 insertions(+), 8 deletions(-) diff --git a/tools/lora_ota/lora_ota.py b/tools/lora_ota/lora_ota.py index f42ca0d12..83b42edbe 100755 --- a/tools/lora_ota/lora_ota.py +++ b/tools/lora_ota/lora_ota.py @@ -3868,11 +3868,19 @@ class SeederProcess: self._wait_until_attached() print("[seeder] running (device COUNT confirmed)") - def _log_tail(self) -> str: + def _read_log(self) -> str: if self.log_file is not None and not self.log_file.closed: - self.log_file.flush() + try: + self.log_file.flush() + except OSError: + # The subprocess writes directly to this descriptor. A parent + # flush failure must not hide bytes already readable on disk. + pass + return self.log_path.read_text(encoding="utf-8", errors="replace") + + def _log_tail(self) -> str: try: - detail = self.log_path.read_text(encoding="utf-8", errors="replace") + detail = self._read_log() except OSError as exc: return f"could not read seeder log: {exc}" return detail[-4096:].strip() or "no seeder log output" @@ -3973,10 +3981,8 @@ class SeederProcess: ``ota status``, reading it consumes no LoRa airtime and cannot collide with the transfer it is observing. """ - if self.log_file is not None and not self.log_file.closed: - self.log_file.flush() try: - detail = self.log_path.read_text(encoding="utf-8", errors="replace") + detail = self._read_log() except OSError: return 0, math.ceil(package.payload_size / package.block_size), 0 offsets = [ diff --git a/tools/lora_ota/test_lora_ota.py b/tools/lora_ota/test_lora_ota.py index 845eed931..1f4efb467 100644 --- a/tools/lora_ota/test_lora_ota.py +++ b/tools/lora_ota/test_lora_ota.py @@ -2553,8 +2553,13 @@ class DownloadSessionTests(unittest.TestCase): seeder.log_file = None seeder.process = mock.Mock() seeder.process.poll.return_value = None - for unreadable in (False, True): - with self.subTest(unreadable=unreadable), contextlib.ExitStack() as stack: + for unreadable, flush_failed in ((False, False), (True, False), + (False, True), (True, True)): + with self.subTest(unreadable=unreadable, flush_failed=flush_failed), \ + contextlib.ExitStack() as stack: + seeder.log_file = mock.Mock(closed=False) if flush_failed else None + if flush_failed: + seeder.log_file.flush.side_effect = OSError("log flush failed") if unreadable: stack.enter_context(mock.patch.object( Path, "read_text", side_effect=PermissionError("unreadable log"))) @@ -5365,6 +5370,30 @@ class ReliabilityTests(unittest.TestCase): with mock.patch.object(Path, "read_text", side_effect=PermissionError("unreadable log")): self.assertEqual(seeder.payload_read_progress(package), (0, 3, 0)) + def test_seeder_progress_flush_failure_preserves_readable_progress(self) -> None: + package = ota.parse_mota(mota_blob(firmware(b"B" * 2500, VERSION_NEW))) + seeder = object.__new__(ota.SeederProcess) + seeder.log_path = Path("serve.log") + seeder.log_file = mock.Mock(closed=False) + seeder.log_file.flush.side_effect = OSError("log flush failed") + detail = (f"[dev] READ 0 @{package.payload_offset} OK\n" + f"[dev] READ 0 @{package.payload_offset + 2 * package.block_size} OK\n" + f"[dev] READ 0 @{package.payload_offset} OK\n") + with mock.patch.object(Path, "read_text", return_value=detail): + self.assertEqual(seeder.payload_read_progress(package), (2, 3, 3)) + + def test_seeder_progress_flush_and_read_failures_keep_zero_activity(self) -> None: + package = ota.parse_mota(mota_blob(firmware(b"B" * 2500, VERSION_NEW))) + seeder = object.__new__(ota.SeederProcess) + seeder.log_path = Path("serve.log") + seeder.log_file = mock.Mock(closed=False) + seeder.log_file.flush.side_effect = OSError("log flush failed") + for error in (FileNotFoundError("missing log"), PermissionError("unreadable log")): + with self.subTest(error=error), mock.patch.object( + Path, "read_text", side_effect=error) as read: + self.assertEqual(seeder.payload_read_progress(package), (0, 3, 0)) + read.assert_called_once() + def test_marked_output_excludes_queued_messages(self) -> None: controller = object.__new__(ota.Controller) controller._execute = lambda _commands, _label: subprocess.CompletedProcess( diff --git a/tools/lora_ota/test_lora_ota_seeder_startup.py b/tools/lora_ota/test_lora_ota_seeder_startup.py index 4033b5043..1fce6b1a5 100644 --- a/tools/lora_ota/test_lora_ota_seeder_startup.py +++ b/tools/lora_ota/test_lora_ota_seeder_startup.py @@ -209,6 +209,39 @@ class SeederStartupTests(unittest.TestCase): seeder._wait_until_attached() self.assertEqual(self.clock.now, 10) + def test_flush_failure_does_not_hide_a_real_count(self): + seeder = ota.SeederProcess(self.args, Path("/served"), Path("/work")) + seeder.process = mock.Mock() + seeder.process.poll.return_value = None + seeder.log_file = mock.Mock(closed=False) + seeder.log_file.flush.side_effect = OSError("log flush failed") + with mock.patch.object(Path, "read_text", return_value="[dev] COUNT -> 1\n"): + seeder._wait_until_attached() + self.assertEqual(self.clock.now, 0) + + def test_flush_and_read_failures_cannot_prove_readiness_or_skip_deadline(self): + seeder = ota.SeederProcess(self.args, Path("/served"), Path("/work")) + seeder.process = mock.Mock() + seeder.process.poll.return_value = None + seeder.log_file = mock.Mock(closed=False) + seeder.log_file.flush.side_effect = OSError("log flush failed") + with mock.patch.object(Path, "read_text", side_effect=PermissionError("unreadable log")): + with self.assertRaisesRegex(ota.OtaError, "COUNT acknowledgement within 5s") as failure: + seeder._wait_until_attached() + self.assertIn("could not read seeder log", str(failure.exception)) + self.assertEqual(self.clock.now, 5) + + def test_flush_failure_preserves_process_exit_diagnostics(self): + seeder = ota.SeederProcess(self.args, Path("/served"), Path("/work")) + seeder.process = mock.Mock() + seeder.process.poll.return_value = 17 + seeder.log_file = mock.Mock(closed=False) + seeder.log_file.flush.side_effect = OSError("log flush failed") + with mock.patch.object(Path, "read_text", return_value="child diagnostic\n"): + with self.assertRaisesRegex(ota.OtaError, "exited with status 17") as failure: + seeder.ensure_running("during startup") + self.assertIn("child diagnostic", str(failure.exception)) + class SeederOptionTests(unittest.TestCase): def arguments(self, chain=False, *options): From adbeb2d007975502a975f4110079a784927516e4 Mon Sep 17 00:00:00 2001 From: mikecarper Date: Fri, 16 Oct 2026 00:28:56 -0700 Subject: [PATCH 07/14] Finish OTA policy cleanup when seeder log close fails --- tools/lora_ota/lora_ota.py | 38 ++++-- tools/lora_ota/test_lora_ota.py | 94 +++++++++++++ .../lora_ota/test_lora_ota_seeder_startup.py | 126 ++++++++++++++++++ 3 files changed, 246 insertions(+), 12 deletions(-) diff --git a/tools/lora_ota/lora_ota.py b/tools/lora_ota/lora_ota.py index 83b42edbe..f1685c378 100755 --- a/tools/lora_ota/lora_ota.py +++ b/tools/lora_ota/lora_ota.py @@ -3863,11 +3863,23 @@ class SeederProcess: creationflags=creationflags, ) except FileNotFoundError as exc: - self.log_file.close() + self._close_log() raise OtaError(f"required command was not found: {self.args.motatool}") from exc self._wait_until_attached() print("[seeder] running (device COUNT confirmed)") + def _close_log(self) -> None: + log_file = self.log_file + self.log_file = None + if log_file is not None and not log_file.closed: + try: + log_file.close() + except OSError as exc: + print( + f"WARNING: could not close seeder log {self.log_path}: {exc}", + file=sys.stderr, + ) + def _read_log(self) -> str: if self.log_file is not None and not self.log_file.closed: try: @@ -3998,22 +4010,24 @@ class SeederProcess: return len(set(payload_reads)), total, len(payload_reads) def stop(self) -> None: - if self.process is None: - return - if self.process.poll() is None: + process = self.process + if process is not None and process.poll() is None: try: if os.name == "nt": - self.process.terminate() + process.terminate() else: - self.process.send_signal(signal.SIGINT) - self.process.wait(timeout=10) + process.send_signal(signal.SIGINT) + process.wait(timeout=10) except (subprocess.TimeoutExpired, ProcessLookupError): - self.process.kill() - self.process.wait(timeout=5) - if self.log_file is not None and not self.log_file.closed: - self.log_file.close() + process.kill() + process.wait(timeout=5) + # Keep ownership if shutdown fails: the child may still own the source + # serial port. Once exit is confirmed, a log-close failure must not + # prevent the caller from restoring radio and RXPS policies. self.process = None - print("[seeder] stopped") + self._close_log() + if process is not None: + print("[seeder] stopped") def parse_relay(value: str, default_password: str) -> tuple[str, str]: diff --git a/tools/lora_ota/test_lora_ota.py b/tools/lora_ota/test_lora_ota.py index 1f4efb467..6c72ceb9a 100644 --- a/tools/lora_ota/test_lora_ota.py +++ b/tools/lora_ota/test_lora_ota.py @@ -1670,6 +1670,100 @@ class SourceCliTests(unittest.TestCase): self.assertEqual(result, 2) self.assertEqual(restore_source.call_count, 2) + def test_seeder_log_close_failure_does_not_skip_failure_policy_restore(self) -> None: + saved = ota.RxpsSettings(True, 18205, 20423, 8, 16) + image = firmware(b"seeder-close-failure" * 500, VERSION_NEW) + package = ota.parse_mota(mota_blob(image)) + expected_body_hash = ota.parse_endf(image).body_hash + normal = ota.RadioSettings(910.525, 62.5, 7, 5, False) + controller = mock.Mock() + controller.get_radio.return_value = normal + controller.remote_command.return_value = "OK" + cleanup = [] + + def fail_close(): + cleanup.append("close log") + raise OSError("log close failed") + + with tempfile.TemporaryDirectory() as directory: + work_dir = Path(directory) / "work" + argv = [ + "release.mota", "remote", + "--controller-serial", "/dev/controller", + "--source-serial", "/dev/source", + "--password", "secret", "--work-dir", str(work_dir), "--yes", + ] + seeder = ota.SeederProcess( + argparse.Namespace( + motatool="motatool", source_serial="/dev/source", + source_tcp=None, source_baud=115200, + ), + Path(directory), work_dir, + ) + seeder.start = mock.Mock() + seeder.process = mock.Mock() + seeder.process.poll.return_value = 0 + log_file = seeder.log_file = mock.Mock(closed=False) + log_file.close.side_effect = fail_close + error = io.StringIO() + with contextlib.ExitStack() as stack: + for name in ( + "preflight_inputs", "preflight_source_cli", + "ensure_source_clock_gate_safe", "ensure_controller_clock_safe", + "verify_shared_source_identity", "confirm_update", + "run_temp_radio_preflight", "arm_target_temp_radio", + "arm_source_temp_radio_once", "switch_controller_to_temp_radio", + "find_and_start_pull", + ): + stack.enter_context(mock.patch.object(ota, name)) + stack.enter_context(mock.patch.object(ota, "read_source_rxps", return_value=saved)) + stack.enter_context(mock.patch.object(ota, "disable_source_rxps", return_value=True)) + restore_source = stack.enter_context(mock.patch.object( + ota, "restore_source_rxps", + side_effect=lambda *args: cleanup.append("restore source RXPS"), + )) + stack.enter_context(mock.patch.object(ota, "query_target", return_value=target())) + stack.enter_context(mock.patch.object( + ota, "prepare_package", + return_value=(Path("release.mota"), package, expected_body_hash), + )) + stack.enter_context(mock.patch.object( + ota, "read_lora_ota_participant_versions", + return_value={"destination": VERSION_NEW}, + )) + stack.enter_context(mock.patch.object( + ota, "read_remote_rxps", return_value=ota.RxpsSettings(False, 0, 0), + )) + monitor = stack.enter_context(mock.patch.object( + ota, "monitor_download", side_effect=ota.OtaError("synthetic transfer failure"), + )) + shorten_source = stack.enter_context(mock.patch.object( + ota, "shorten_source_temp_window", return_value=True, + )) + stack.enter_context(mock.patch.object(ota, "SeederProcess", return_value=seeder)) + stack.enter_context(mock.patch.object(ota.time, "sleep")) + source_command = stack.enter_context(mock.patch.object( + ota, "source_cli_command", + side_effect=lambda args, command, **kwargs: cleanup.append(command) or "OK", + )) + stack.enter_context(contextlib.redirect_stdout(io.StringIO())) + stack.enter_context(contextlib.redirect_stderr(error)) + result = ota.main(argv, controller_override=controller) + + self.assertEqual(result, 2) + monitor.assert_called_once() + log_file.close.assert_called_once_with() + self.assertIsNone(seeder.process) + self.assertIsNone(seeder.log_file) + source_command.assert_called_once_with(mock.ANY, "ota folder off", check=False) + shorten_source.assert_called_once_with(mock.ANY, check=False) + controller.set_radio.assert_called_once_with(normal, "restore controller radio after failure") + restore_source.assert_called_once_with(mock.ANY, saved) + self.assertEqual(cleanup, ["close log", "ota folder off", "restore source RXPS"]) + self.assertIn("ERROR: synthetic transfer failure", error.getvalue()) + self.assertIn("WARNING: could not close seeder log", error.getvalue()) + self.assertIn("log close failed", error.getvalue()) + def test_shared_failure_cleanup_never_persists_temp_radio_tuple(self) -> None: normal = ota.RadioSettings(910.525, 62.5, 7, 5, False) controller = mock.Mock() diff --git a/tools/lora_ota/test_lora_ota_seeder_startup.py b/tools/lora_ota/test_lora_ota_seeder_startup.py index 1fce6b1a5..ad44ca739 100644 --- a/tools/lora_ota/test_lora_ota_seeder_startup.py +++ b/tools/lora_ota/test_lora_ota_seeder_startup.py @@ -4,6 +4,7 @@ import argparse import contextlib import io from pathlib import Path +import subprocess import sys import tempfile import unittest @@ -243,6 +244,130 @@ class SeederStartupTests(unittest.TestCase): self.assertIn("child diagnostic", str(failure.exception)) +class SeederStopTests(unittest.TestCase): + def seeder(self, return_code=0): + args = argparse.Namespace( + motatool="motatool", source_serial="/dev/source", + source_tcp=None, source_baud=115200, + ) + seeder = ota.SeederProcess(args, Path("/served"), Path("/work")) + seeder.process = mock.Mock() + seeder.process.poll.return_value = return_code + seeder.log_file = mock.Mock(closed=False) + return seeder + + def test_log_close_failure_warns_once_and_allows_later_cleanup(self): + seeder = self.seeder() + process, log_file = seeder.process, seeder.log_file + log_file.close.side_effect = OSError("log close failed") + output, error = io.StringIO(), io.StringIO() + restored = [] + with contextlib.redirect_stdout(output), contextlib.redirect_stderr(error): + seeder.stop() + restored.append("policy restored") + seeder.stop() + self.assertEqual(restored, ["policy restored"]) + self.assertIsNone(seeder.process) + self.assertIsNone(seeder.log_file) + process.send_signal.assert_not_called() + process.kill.assert_not_called() + log_file.close.assert_called_once_with() + self.assertEqual(output.getvalue().count("[seeder] stopped"), 1) + self.assertEqual(error.getvalue().count("WARNING:"), 1) + self.assertIn("/work/motatool-serve.log", error.getvalue()) + self.assertIn("log close failed", error.getvalue()) + + def test_running_child_is_waited_for_before_releasing_ownership(self): + seeder = self.seeder(return_code=None) + process, log_file = seeder.process, seeder.log_file + with mock.patch.object(ota.os, "name", "posix"), contextlib.redirect_stdout(io.StringIO()): + seeder.stop() + process.send_signal.assert_called_once_with(ota.signal.SIGINT) + process.wait.assert_called_once_with(timeout=10) + process.kill.assert_not_called() + log_file.close.assert_called_once_with() + self.assertIsNone(seeder.process) + self.assertIsNone(seeder.log_file) + + def test_graceful_timeout_is_followed_by_confirmed_kill(self): + seeder = self.seeder(return_code=None) + process, log_file = seeder.process, seeder.log_file + process.wait.side_effect = [subprocess.TimeoutExpired("motatool", 10), 0] + with contextlib.redirect_stdout(io.StringIO()): + seeder.stop() + process.kill.assert_called_once_with() + self.assertEqual(process.wait.call_args_list, [ + mock.call(timeout=10), mock.call(timeout=5), + ]) + log_file.close.assert_called_once_with() + self.assertIsNone(seeder.process) + self.assertIsNone(seeder.log_file) + + def test_failed_shutdown_retains_ownership_and_does_not_claim_stopped(self): + for failure in ("kill", "final wait"): + with self.subTest(failure=failure): + seeder = self.seeder(return_code=None) + process, log_file = seeder.process, seeder.log_file + process.wait.side_effect = [ + subprocess.TimeoutExpired("motatool", 10), + subprocess.TimeoutExpired("motatool", 5), + ] + expected = subprocess.TimeoutExpired + if failure == "kill": + process.kill.side_effect = OSError("kill failed") + expected = OSError + output = io.StringIO() + with contextlib.redirect_stdout(output), self.assertRaises(expected): + seeder.stop() + self.assertIs(seeder.process, process) + self.assertIs(seeder.log_file, log_file) + log_file.close.assert_not_called() + self.assertNotIn("stopped", output.getvalue()) + + def test_log_without_a_child_is_closed_and_detached(self): + for close_error in (None, OSError("log close failed")): + with self.subTest(close_error=close_error): + seeder = self.seeder() + seeder.process = None + log_file = seeder.log_file + log_file.close.side_effect = close_error + output, error = io.StringIO(), io.StringIO() + with contextlib.redirect_stdout(output), contextlib.redirect_stderr(error): + seeder.stop() + log_file.close.assert_called_once_with() + self.assertIsNone(seeder.log_file) + self.assertNotIn("stopped", output.getvalue()) + self.assertEqual(bool(error.getvalue()), close_error is not None) + + def test_already_closed_log_is_detached_without_closing_again(self): + seeder = self.seeder() + log_file = seeder.log_file + log_file.closed = True + with contextlib.redirect_stdout(io.StringIO()): + seeder.stop() + log_file.close.assert_not_called() + self.assertIsNone(seeder.process) + self.assertIsNone(seeder.log_file) + + def test_spawn_failure_retains_primary_diagnostic_if_log_close_fails(self): + seeder = self.seeder() + seeder.process = None + log_file = seeder.log_file + log_file.close.side_effect = OSError("log close failed") + error = io.StringIO() + with ( + mock.patch.object(Path, "open", return_value=log_file), + mock.patch.object(ota.subprocess, "Popen", side_effect=FileNotFoundError("missing")), + contextlib.redirect_stdout(io.StringIO()), + contextlib.redirect_stderr(error), + self.assertRaisesRegex(ota.OtaError, "required command was not found: motatool"), + ): + seeder.start() + self.assertIn("log close failed", error.getvalue()) + self.assertIsNone(seeder.process) + self.assertIsNone(seeder.log_file) + + class SeederOptionTests(unittest.TestCase): def arguments(self, chain=False, *options): parser = rak_chain.build_parser() if chain else ota.build_parser() @@ -317,6 +442,7 @@ class SeederSubprocessTests(unittest.TestCase): self.assertIn("running (device COUNT confirmed)", output.getvalue()) self.assertIn("host compression complete", output.getvalue()) self.assertIsNone(seeder.process) + self.assertIsNone(seeder.log_file) if __name__ == "__main__": From d5cc73c43d733501855fdfa246e5fda650e7293a Mon Sep 17 00:00:00 2001 From: mikecarper Date: Fri, 16 Oct 2026 00:39:07 -0700 Subject: [PATCH 08/14] Validate beacon power without stranding saved settings --- .github/workflows/run-unit-tests.yml | 1 + docs/tx_beacons.md | 12 ++ src/Dispatcher.h | 3 + src/MeshBeacon.cpp | 3 +- src/helpers/BeaconManager.cpp | 16 +- src/helpers/radiolib/DacPaSX1276Wrapper.h | 6 + src/helpers/radiolib/RadioLibWrappers.cpp | 39 +++- src/helpers/radiolib/RadioLibWrappers.h | 6 + test/fixtures/beacons/manager_test.cpp | 57 +++++ test/fixtures/beacons/power_radio.h | 90 ++++++++ test/fixtures/beacons/power_test.cpp | 251 ++++++++++++++++++++++ test/test_beacon_frames.py | 1 + test/test_beacon_manager.py | 3 + test/test_beacon_power.py | 167 ++++++++++++++ test/test_beacon_radio.py | 14 ++ 15 files changed, 662 insertions(+), 7 deletions(-) create mode 100644 test/fixtures/beacons/power_radio.h create mode 100644 test/fixtures/beacons/power_test.cpp create mode 100644 test/test_beacon_power.py diff --git a/.github/workflows/run-unit-tests.yml b/.github/workflows/run-unit-tests.yml index f343abed2..7ef970b9c 100644 --- a/.github/workflows/run-unit-tests.yml +++ b/.github/workflows/run-unit-tests.yml @@ -422,6 +422,7 @@ jobs: '--storage-dir', '.pio/libdeps/radio-status-test'], check=True) PY python3 -B test/test_sx126x_receive_mode.py + python3 -B test/test_beacon_power.py -v - name: Verify temporary-radio timing and exact reply delivery run: | diff --git a/docs/tx_beacons.md b/docs/tx_beacons.md index 8c66e1193..b9a8359db 100644 --- a/docs/tx_beacons.md +++ b/docs/tx_beacons.md @@ -40,11 +40,23 @@ Meshtext takes an explicit `us`, `eu868`, `eu433`, or `anz` region, a recognized preset, and dBm. Its frequency is calculated using the stock preset channel; its preamble is fixed at 16 symbols. Invalid modem, region, power, or text settings are rejected. Use settings permitted for your hardware and location. +Power is checked against the driver's range at the beacon frequency and any +explicit board limits. External-PA drivers interpret it in antenna dBm using +their calibration range; the normal receive profile's power is unchanged. Each message body is limited to 80 bytes and is prefixed with the node name. Settings are saved separately from ordinary preferences and channels. Configuration changes do not immediately transmit a beacon. +A structurally valid saved slot that the current board or firmware cannot +transmit remains available through `get beacon`. Its scheduled opportunity is +skipped, while supported slots continue normally. Use `set beacon` to repair +that slot or `delete beacon` to remove it; unsupported Meshtext slots do not +consume packet IDs. This also permits repairing an out-of-range power saved +by older firmware. Corrupt images retain the existing safe backup recovery; +without a valid backup they remain held. Unreadable or unknown-version files +remain held without transmitting or overwriting them. + ## Radio ownership and limits These are ordinary channel text messages, not MeshCore identity advertisements diff --git a/src/Dispatcher.h b/src/Dispatcher.h index 0a0ce667c..09b67bccf 100644 --- a/src/Dispatcher.h +++ b/src/Dispatcher.h @@ -94,6 +94,9 @@ public: virtual bool supportsMeshtext() const { return false; } virtual bool supportsEthernetTunnel() const { return false; } #if MESH_ENABLE_TX_BEACONS + // Check the requested antenna power at the prospective beacon frequency. + // Validation must not retune the radio or alter its current power setting. + virtual bool validateBeaconPower(float, int8_t) const { return false; } virtual RadioParamApplyResult prepareBeaconTransmit(const RadioProfileParams&, int8_t) { return RadioParamApplyResult::FAILED; } diff --git a/src/MeshBeacon.cpp b/src/MeshBeacon.cpp index 0df2b58a2..297ab2a14 100644 --- a/src/MeshBeacon.cpp +++ b/src/MeshBeacon.cpp @@ -3,7 +3,6 @@ #if MESH_ENABLE_TX_BEACONS #include -#include #include namespace mesh { @@ -12,7 +11,7 @@ bool Mesh::validateTextBeacon(const BeaconSlot& slot) const { if (!beacon::validText(slot.text) || slot.hops > 7 || !RadioProfiles::valid(slot.radio) || !RadioProfiles::safePreamble(slot.radio, slot.radio.preamble ? slot.radio.preamble : 32) - || slot.power < -9 || !isLoRaTxPowerValid(slot.power, slot.radio.freq) + || !_radio->validateBeaconPower(slot.radio.freq, slot.power) || !_radio->validateProfile(slot.radio)) return false; if (slot.radio.protocol == RadioProfileProtocol::MeshCore) return true; #if MESH_ENABLE_MESHTEXT_WIRE diff --git a/src/helpers/BeaconManager.cpp b/src/helpers/BeaconManager.cpp index 3a58e76b0..9309c1bec 100644 --- a/src/helpers/BeaconManager.cpp +++ b/src/helpers/BeaconManager.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -75,12 +76,15 @@ bool decodeSlot(const uint8_t bytes[RecordSize], BeaconSlot& slot) { slot.power = int8_t(bytes[13]); slot.preset = bytes[14]; slot.hops = bytes[15]; slot.enabled = true; memcpy(slot.text, bytes + 17, BeaconSlot::TextCapacity); + const size_t text_length = strlen(slot.text); // Reject noncanonical trailing data instead of carrying hidden text bytes. - for (size_t i = strlen(slot.text) + 1; i < BeaconSlot::TextCapacity; ++i) + for (size_t i = text_length + 1; i < BeaconSlot::TextCapacity; ++i) if (slot.text[i]) return false; for (const unsigned char* c = reinterpret_cast(slot.text); *c; ++c) if (*c < 32 || *c == 127) return false; - return isfinite(slot.radio.freq) && isfinite(slot.radio.bw) + return validUtf8PrefixLength(slot.text, text_length) == text_length + && RadioProfiles::valid(slot.radio) + && RadioProfiles::safePreamble(slot.radio, slot.radio.preamble ? slot.radio.preamble : 32) && (slot.radio.protocol != RadioProfileProtocol::Meshtext || (slot.preset <= uint8_t(mt::Preset::LongSlow) && slot.radio.preamble == 16)); } @@ -218,7 +222,9 @@ BeaconManager::ReadResult BeaconManager::readConfig( } crc = crcUpdate(crc, bytes, sizeof(bytes)); BeaconSlot slot; - if (!decodeSlot(bytes, slot) || (slot.enabled && (!mesh_ || !mesh_->validateTextBeacon(slot))) + // Disk validity is independent of the current board's radio capabilities. + // Preserve unsupported slots so an upgrade can still query/repair them. + if (!decodeSlot(bytes, slot) || (slot.enabled && slot.radio.protocol == RadioProfileProtocol::Meshtext && !header[6])) valid = false; if (output) output[i] = slot; @@ -403,7 +409,9 @@ void BeaconManager::loop(uint32_t now) { if (hour > 0 && handled_hour_ != hour) { const unsigned index = unsigned((hour - 1) % SlotCount); BeaconSlot& slot = slots_[index]; - if (!slot.enabled) handled_hour_ = hour; + // A retained slot may be unsupported by this board or firmware. Skip it + // before allocating a durable Meshtext packet ID or entering the TX queue. + if (!slot.enabled || !mesh_->validateTextBeacon(slot)) handled_hour_ = hour; else { uint64_t guard = last_send_ && elapsed_ - last_send_ < HourMillis ? HourMillis - (elapsed_ - last_send_) : 0; diff --git a/src/helpers/radiolib/DacPaSX1276Wrapper.h b/src/helpers/radiolib/DacPaSX1276Wrapper.h index a773ba053..73779cc57 100644 --- a/src/helpers/radiolib/DacPaSX1276Wrapper.h +++ b/src/helpers/radiolib/DacPaSX1276Wrapper.h @@ -139,6 +139,12 @@ public: } protected: +#if MESH_ENABLE_TX_BEACONS + bool checkBeaconOutputPower(float frequency_mhz, int8_t dbm) const override { + (void)frequency_mhz; + return dbm >= _min_dbm && dbm <= _max_dbm; + } +#endif // Emit a gain-control code. Override for a PA driven by PWM or an external // DAC instead of the MCU's own. virtual void writeGainControl(uint8_t code) { diff --git a/src/helpers/radiolib/RadioLibWrappers.cpp b/src/helpers/radiolib/RadioLibWrappers.cpp index 68e12c3c8..bcd4d16fa 100644 --- a/src/helpers/radiolib/RadioLibWrappers.cpp +++ b/src/helpers/radiolib/RadioLibWrappers.cpp @@ -3,6 +3,9 @@ #include "RadioLibWrappers.h" #include "RadioInterruptEvents.h" #include "RadioProfileChip.h" +#if MESH_ENABLE_TX_BEACONS +#include "RadioPowerLimits.h" +#endif #include #if defined(ESP32_PLATFORM) #include @@ -559,6 +562,39 @@ mesh::RadioParamApplyResult RadioLibWrapper::tuneProfile(uint8_t profile) { } #if MESH_ENABLE_TX_BEACONS +bool RadioLibWrapper::validateBeaconPower(float frequency_mhz, int8_t power) const { + return isfinite(frequency_mhz) && mesh::isLoRaTxPowerValid(power, frequency_mhz) + && checkBeaconOutputPower(frequency_mhz, power); +} + +bool RadioLibWrapper::checkBeaconOutputPower(float frequency_mhz, int8_t power) const { + (void)frequency_mhz; +#if MESH_RADIO_PROFILE_CHIP == 1276 + // SX127x's setter automatically selects RFO below 2 dBm. Its two-argument + // checker assumes PA_BOOST, so use the same selection as setOutputPower(). + return static_cast(_radio)->checkOutputPower(power, nullptr, power < 2) + == RADIOLIB_ERR_NONE; +#elif MESH_RADIO_PROFILE_CHIP == 1272 + return static_cast(_radio)->checkOutputPower(power, nullptr, power < 2) + == RADIOLIB_ERR_NONE; +#elif MESH_RADIO_PROFILE_CHIP == 1121 + // RadioLib's checker uses its current band. Validate the prospective band + // without changing the cached frequency or triggering a hardware retune. + if (frequency_mhz > 1000.0f) return power >= -18 && power <= 13; +#ifdef LR1121_FORCE_HP_PA + return power >= -9 && power <= 22; +#else + return power >= -17 && power <= 22; +#endif +#elif MESH_RADIO_PROFILE_CHIP == 2021 + return mesh::lr2021::isHighBand(frequency_mhz) + ? power >= -19 && power <= 12 : power >= -9 && power <= 22; +#else + // These RadioLib checkers inspect configuration only; no SPI is performed. + return _radio->checkOutputPower(power, nullptr) == RADIOLIB_ERR_NONE; +#endif +} + uint32_t RadioLibWrapper::getBeaconAirtime(const mesh::RadioProfileParams& p, int length) { if (length <= 0 || length > MAX_TRANS_UNIT || !validateProfile(p)) return 0; const uint16_t preamble = p.protocol == mesh::RadioProfileProtocol::Meshtext @@ -596,7 +632,8 @@ mesh::RadioParamApplyResult RadioLibWrapper::prepareBeaconTransmit( _hil_beacon_tx_frequency_mhz = 0; if (!mesh::hilTransmitFrequencyAllowed(p.freq)) return Result::FAILED; #endif - if (!_params_valid || !_dbm_valid || !getBeaconAirtime(p, MAX_TRANS_UNIT)) return Result::FAILED; + if (!_params_valid || !_dbm_valid || !getBeaconAirtime(p, MAX_TRANS_UNIT) + || !validateBeaconPower(p.freq, power)) return Result::FAILED; if (isCalibratingNoiseFloor()) return Result::BUSY; beginProfileRetune(isInRecvMode() && !_rx_ps_armed && !_rx_ps_enabled); diff --git a/src/helpers/radiolib/RadioLibWrappers.h b/src/helpers/radiolib/RadioLibWrappers.h index e3dd8e919..51f178640 100644 --- a/src/helpers/radiolib/RadioLibWrappers.h +++ b/src/helpers/radiolib/RadioLibWrappers.h @@ -272,6 +272,11 @@ protected: virtual int16_t applyCachedTxPower(int8_t dbm) { return _radio->setOutputPower(dbm); } +#if MESH_ENABLE_TX_BEACONS + // External amplifiers override this in antenna dBm. The public capability + // still checks the board's explicit limits before consulting this hook. + virtual bool checkBeaconOutputPower(float frequency_mhz, int8_t dbm) const; +#endif virtual bool applyRxBoostedGainMode(bool) { return false; } virtual int16_t enterCarrierWave() { return _radio->transmitDirect(); } virtual bool validateMorseFrequency(float frequency_mhz) const { @@ -409,6 +414,7 @@ public: mesh::RadioParamApplyResult prepareTransmitProfile(uint8_t profile) override; mesh::RadioParamApplyResult prepareTransmitProfile(uint8_t profile, bool reply_rx_override) override; #if MESH_ENABLE_TX_BEACONS + bool validateBeaconPower(float frequency_mhz, int8_t power) const override; mesh::RadioParamApplyResult prepareBeaconTransmit(const mesh::RadioProfileParams& params, int8_t power) override; mesh::RadioParamApplyResult restoreBeaconReceive() override; diff --git a/test/fixtures/beacons/manager_test.cpp b/test/fixtures/beacons/manager_test.cpp index ab8472504..5346af1f9 100644 --- a/test/fixtures/beacons/manager_test.cpp +++ b/test/fixtures/beacons/manager_test.cpp @@ -57,6 +57,62 @@ void put32(uint8_t* bytes, uint32_t value) { } void fixCrc(std::vector& bytes) { put32(bytes.data() + bytes.size() - 4, crc(bytes.data(), bytes.size() - 4)); } +void structuralStorage() { + Fixture mc; mc.mc(1, "Old"); + Fixture mt; mt.mt(); + for (unsigned fault = 0; fault < 15; ++fault) { + Fixture f; + f.fs.files = fault >= 12 ? mt.fs.files : mc.fs.files; + auto& bytes = f.fs.files.at("/beacons"); + uint8_t* record = bytes.data() + 8; + switch (fault) { + case 0: record[10] = 255; break; // Invalid SF. + case 1: record[11] = 4; break; // Invalid coding rate. + case 2: { + const float bw = 123; + std::memcpy(record + 4, &bw, 4); break; + } + case 3: record[8] = 7; record[9] = 0; break; + case 4: { // Valid tuple, but its long preamble overflows safe airtime. + const float bw = 7.8f; + std::memcpy(record + 4, &bw, 4); + record[10] = 12; record[8] = 248; record[9] = 255; break; + } + case 5: put32(record, 0x7fc00000U); break; // NaN frequency. + case 6: record[17] = 255; break; // Malformed UTF-8. + case 7: record[17] = '\n'; break; + case 8: record[21] = 'X'; break; // Hidden bytes after the terminator. + case 9: record[16] = 0; break; // Disabled records must be all zero. + case 10: record[12] = uint8_t(mesh::RadioProfileProtocol::Ethernet); break; + case 11: record[15] = 8; break; + case 12: record[14] = 255; break; // Invalid MT preset. + case 13: record[8] = 32; record[9] = 0; break; // MT preamble is fixed. + case 14: bytes[6] = 0; break; // MT requires durable ID initialization. + } + fixCrc(bytes); + const auto original = f.fs.files; + f.manager.begin(&f.fs, &f.mesh, &f.clock, f.name); + f.cmd("get beacon", false); + f.cmd("set beacon.interval 12", false); + f.cmd("delete beacon all", false); + f.tick(48 * H); + assert(f.mesh.attempts == 0 && f.fs.files == original); + } + // Neither a future schema nor a read error may fall back to an older image. + for (bool unreadable : {false, true}) { + Fixture f; + f.fs.files = mc.fs.files; + f.fs.files["/beacons.bak"] = f.fs.files.at("/beacons"); + if (unreadable) f.fs.fail_read_open = true; + else f.fs.files.at("/beacons")[4] = 2; + const auto original = f.fs.files; + f.manager.begin(&f.fs, &f.mesh, &f.clock, f.name); + f.cmd("get beacon", false); + f.tick(48 * H); + assert(f.mesh.attempts == 0 && f.fs.files == original); + } +} + void commands() { Fixture f; assert(!f.mesh.wake_enabled && f.fs.files.empty()); @@ -407,6 +463,7 @@ int main(int argc, char** argv) { else if (!std::strcmp(argv[1], "schedule")) schedule(); else if (!std::strcmp(argv[1], "rolling")) rolling(); else if (!std::strcmp(argv[1], "storage")) storage(); + else if (!std::strcmp(argv[1], "structural_storage")) structuralStorage(); else if (!std::strcmp(argv[1], "ids")) ids(); else if (!std::strcmp(argv[1], "mutations")) mutations(); else if (!std::strcmp(argv[1], "stale_backup")) staleBackup(); diff --git a/test/fixtures/beacons/power_radio.h b/test/fixtures/beacons/power_radio.h new file mode 100644 index 000000000..f9a6c0c89 --- /dev/null +++ b/test/fixtures/beacons/power_radio.h @@ -0,0 +1,90 @@ +#pragma once +#include +#include +#include +#include +#include +#include + +#define RADIOLIB_ERR_NONE 0 +#define RADIOLIB_ERR_INVALID_OUTPUT_POWER -1 +#define RADIOLIB_MIN(a, b) std::min(int(a), int(b)) +#define RADIOLIB_MAX(a, b) std::max(int(a), int(b)) +#define RADIOLIB_CHECK_RANGE(value, low, high, error) \ + if ((value) < (low) || (value) > (high)) return (error) + +extern unsigned power_hardware_calls; +inline void dacWrite(uint8_t, uint8_t) { ++power_hardware_calls; } +namespace mesh { +struct MainBoard {}; +struct Radio { + @RADIO_CAPABILITY@ +}; +} +struct PhysicalLayer { + bool highFreq = false; + float frequency = 909.5f; + virtual int16_t checkOutputPower(int8_t, int8_t*) = 0; + int16_t setOutputPower(int8_t, bool) { ++power_hardware_calls; return 0; } +}; +struct SX1262 : PhysicalLayer { + int16_t checkOutputPower(int8_t, int8_t*) override; +}; +struct SX1276 : PhysicalLayer { + int16_t checkOutputPower(int8_t, int8_t*) override; + int16_t checkOutputPower(int8_t, int8_t*, bool); +}; +struct SX1272 : PhysicalLayer { + int16_t checkOutputPower(int8_t, int8_t*) override; + int16_t checkOutputPower(int8_t, int8_t*, bool); +}; +struct LR1110 : PhysicalLayer { + int16_t checkOutputPower(int8_t, int8_t*) override; + int16_t checkOutputPower(int8_t, int8_t*, bool); +}; +struct LR1121 : PhysicalLayer { + int16_t checkOutputPower(int8_t, int8_t*) override; + int16_t checkOutputPower(int8_t, int8_t*, bool); +}; +struct LR2021 : PhysicalLayer { + int16_t checkOutputPower(int8_t, int8_t*) override; +}; +struct Module { + bool hp = true, lp = true; + bool findRfSwitchMode(int mode) const { return mode == 1 ? hp : lp; } +}; +struct STM32WLx : PhysicalLayer { + static constexpr int MODE_TX_HP = 1, MODE_TX_LP = 2; + Module module; + const Module* getMod() const { return &module; } + int16_t checkOutputPower(int8_t, int8_t*) override; +}; +@DRIVER_CHECKS@ + +class RadioLibWrapper : public mesh::Radio { +protected: + PhysicalLayer* _radio; + virtual bool checkBeaconOutputPower(float, int8_t) const; +public: + RadioLibWrapper(PhysicalLayer& radio, mesh::MainBoard&) : _radio(&radio) {} + virtual void loop() { ++power_hardware_calls; } + virtual int16_t applyCachedTxPower(int8_t) { ++power_hardware_calls; return 0; } + bool setTxPower(int8_t power) { return applyCachedTxPower(power) == 0; } + bool validateBeaconPower(float, int8_t) const override; + bool validateProfile(const mesh::RadioProfileParams&) const; + bool supportsMeshtext() const { return true; } + bool supportsEthernetTunnel() const { return true; } +}; +using CustomSX1276 = SX1276; +using CustomSX1262 = SX1262; +struct CustomSX1276Wrapper : RadioLibWrapper { using RadioLibWrapper::RadioLibWrapper; }; +struct CustomSX1262Wrapper : RadioLibWrapper { using RadioLibWrapper::RadioLibWrapper; }; +struct IndicatorRadioHal { + unsigned services = 0; + void serviceInterrupt() { ++services; ++power_hardware_calls; } +}; +@INDICATOR_WRAPPER@ +@DAC_LEVEL@ +#define DAC_PA_DEFAULT_DRIVE_DBM 2 +#define DAC_PA_TABLE_MAX INT8_MAX +@DAC_WRAPPER@ diff --git a/test/fixtures/beacons/power_test.cpp b/test/fixtures/beacons/power_test.cpp new file mode 100644 index 000000000..a2b16d87a --- /dev/null +++ b/test/fixtures/beacons/power_test.cpp @@ -0,0 +1,251 @@ +#include +#include +#include +#include +#include +#include + +unsigned power_hardware_calls = 0; +uint64_t beacon_test_time = 0; +@PRODUCTION_METHODS@ + +static bool boardAllows(int power, float frequency) { + // Explicit board caps are independent of the chip and calibrated PA range. + (void)power; + (void)frequency; +#ifdef MIN_LORA_TX_POWER + if (power < MIN_LORA_TX_POWER) return false; +#endif +#ifdef MAX_LORA_TX_POWER + if (power > MAX_LORA_TX_POWER) return false; +#endif + return true; +} +static bool driverAllows(int power, float frequency) { + (void)power; +#if defined(TEST_STM32) + (void)frequency; +#ifdef TEST_NO_PA + return false; +#elif defined(TEST_LP_ONLY) + return power >= -17 && power <= 14; +#elif defined(TEST_HP_ONLY) + return power >= -9 && power <= 22; +#else + return power >= -17 && power <= 22; +#endif +#elif MESH_RADIO_PROFILE_CHIP == 1276 + (void)frequency; + return (power >= -4 && power <= 17) || power == 20; +#elif MESH_RADIO_PROFILE_CHIP == 1272 + (void)frequency; + return power >= -1 && power <= 20; +#elif MESH_RADIO_PROFILE_CHIP == 1110 + (void)frequency; + return power >= -17 && power <= 22; +#elif MESH_RADIO_PROFILE_CHIP == 1121 + if (frequency > 1000) return power >= -18 && power <= 13; +#ifdef LR1121_FORCE_HP_PA + return power >= -9 && power <= 22; +#else + return power >= -17 && power <= 22; +#endif +#elif MESH_RADIO_PROFILE_CHIP == 2021 + return frequency >= 1900 ? power >= -19 && power <= 12 : power >= -9 && power <= 22; +#else + (void)frequency; + return power >= -9 && power <= 22; +#endif +} + +static void capability(RadioLibWrapper& wrapper, PhysicalLayer& chip) { + mesh::Radio unsupported; + assert(!unsupported.validateBeaconPower(909.5f, 17)); + assert(!wrapper.validateBeaconPower(std::numeric_limits::quiet_NaN(), 10)); + for (bool normal_high_band : {false, true}) { + chip.highFreq = normal_high_band; + chip.frequency = normal_high_band ? 2440.0f : 909.5f; + for (float requested : {433.875f, 909.5f, 2440.0f}) { + for (int power = -128; power <= 127; ++power) { + const bool expected = boardAllows(power, requested) && driverAllows(power, requested); + assert(wrapper.validateBeaconPower(requested, int8_t(power)) == expected); + } + assert(chip.frequency == (normal_high_band ? 2440.0f : 909.5f)); + assert(chip.highFreq == normal_high_band && power_hardware_calls == 0); + } + } +} + +#if !defined(TEST_NO_PA) +static void image(const char* path, std::vector& bytes, bool write) { + FILE* file = std::fopen(path, write ? "wb" : "rb"); + assert(file); + if (write) assert(std::fwrite(bytes.data(), 1, bytes.size(), file) == bytes.size()); + else { + bytes.clear(); + for (int value; (value = std::fgetc(file)) != EOF;) bytes.push_back(uint8_t(value)); + assert(!std::ferror(file)); + } + assert(std::fclose(file) == 0); +} + +static void upgrade(RadioLibWrapper& wrapper, bool legacy) { + MemoryFS fs; + if (!legacy) { + image("upgrade.beacons", fs.files["/beacons"], false); + image("upgrade.ids", fs.files["/beacon_ids"], false); + } + mesh::Mesh mesh; + mesh._radio = &wrapper; + mesh::RTCClock rtc; + mesh::BeaconManager manager; + manager.begin(&fs, &mesh, &rtc, "Upgrade test"); + char reply[256]; + const auto command = [&](const char* text) { + assert(manager.handle(text, reply, sizeof(reply))); + assert(std::strncmp(reply, "Err", 3)); + }; + const auto tick = [&](unsigned hours) { + g_mock_millis += hours * mesh::BeaconManager::HourMillis; + manager.loop(g_mock_millis); + }; + if (legacy) { + // These images are produced by the real manager and old Mesh validator, + // not hand-crafted bytes that the old firmware might never have saved. + command("set beacon 1 meshcore 909.5,125,7,5,100 Old MC"); + command("set beacon 2 meshcore 909.5,125,7,5,22 Sibling"); + command("set beacon 3 meshtext US LongFast 100 Old MT"); + tick(6); + assert(mesh.sends.size() == 1 && mesh.sends[0].id != 0); + image("upgrade.beacons", fs.files.at("/beacons"), true); + image("upgrade.ids", fs.files.at("/beacon_ids"), true); + return; + } + const auto original = fs.files; + assert(original.at("/beacons")[6] == 1); // Durable IDs were initialized. + command("get beacon"); + command("get beacon 1"); assert(std::strstr(reply, "100") && std::strstr(reply, "Old MC")); + command("get beacon 3"); assert(std::strstr(reply, "100") && std::strstr(reply, "Old MT")); + tick(2); // Unsupported MC consumes no queue attempt or storage write. + assert(mesh.attempts == 0 && mesh.sends.empty() && fs.files == original); + tick(2); // A supported sibling still transmits. + assert(mesh.attempts == 1 && mesh.sends.size() == 1 && mesh.sends[0].slot.power == 22); + assert(fs.files == original); + tick(2); // Unsupported MT must be rejected before durable nextId(). + assert(mesh.attempts == 1 && mesh.sends.size() == 1 && fs.files == original); + command("set beacon.interval 12"); + command("set beacon 2 meshcore 909.5,125,7,5,20 Updated sibling"); + const auto& before = original.at("/beacons"); + const auto& after = fs.files.at("/beacons"); + for (unsigned index : {0U, 2U}) + assert(std::equal(before.begin() + 8 + index * 98, before.begin() + 8 + (index + 1) * 98, + after.begin() + 8 + index * 98)); + assert(fs.files.at("/beacon_ids") == original.at("/beacon_ids")); + command("get beacon 3"); assert(std::strstr(reply, "100")); + command("set beacon 1 meshcore 909.5,125,7,5,20 Repaired"); + command("delete beacon 3"); + assert(fs.files.at("/beacon_ids") == original.at("/beacon_ids")); + manager.begin(&fs, &mesh, &rtc, "Upgrade test"); + command("get beacon 1"); assert(std::strstr(reply, "Repaired")); + command("get beacon.interval"); assert(std::strstr(reply, "12")); + tick(1); + assert(mesh.sends.size() == 2 && mesh.sends.back().slot.power == 20); + tick(1); + assert(mesh.sends.size() == 3 && std::strstr(mesh.sends.back().slot.text, "Updated sibling")); + assert(fs.files.at("/beacon_ids") == original.at("/beacon_ids")); + assert(power_hardware_calls == 0); +} + +static void manager(RadioLibWrapper& wrapper) { + MemoryFS fs; + mesh::Mesh mesh; + mesh._radio = &wrapper; + mesh::RTCClock rtc; + mesh::BeaconManager manager; + manager.begin(&fs, &mesh, &rtc, "Power test"); + char reply[256], command[128]; + const float frequency = MESH_RADIO_PROFILE_CHIP == 1268 ? 433.875f : 909.5f; + for (int bad : {-128, 100, 127}) { + std::snprintf(command, sizeof(command), "set beacon 1 meshcore %g,125,7,5,%d Hello", double(frequency), bad); + assert(manager.handle(command, reply, sizeof(reply))); + assert(!std::strncmp(reply, "Err", 3)); + assert(fs.files.empty()); // Rejection happens before persistence. + } + int accepted = 22; + while (!driverAllows(accepted, frequency) || !boardAllows(accepted, frequency)) --accepted; + std::snprintf(command, sizeof(command), "set beacon 1 meshcore %g,125,7,5,%d Hello", double(frequency), accepted); + assert(manager.handle(command, reply, sizeof(reply)) && !std::strncmp(reply, "OK", 2)); + assert(fs.files.count("/beacons") && power_hardware_calls == 0); + manager.begin(&fs, &mesh, &rtc, "Power test"); + g_mock_millis += 2 * mesh::BeaconManager::HourMillis; + manager.loop(g_mock_millis); + assert(mesh.sends.size() == 1 && mesh.sends[0].slot.power == accepted); + assert(power_hardware_calls == 0); +} +#endif + +#if MESH_RADIO_PROFILE_CHIP == 1276 +static void externalAmplifier() { + mesh::MainBoard board; + CustomSX1276 chip; + const DacPaLevel levels[] = {{10, 30}, {17, 50}, {30, 130}, {33, 225}}; + DacPaSX1276Wrapper amplifier(chip, board, 4, levels, 4, 30); + for (int power = -128; power <= 127; ++power) + assert(amplifier.validateBeaconPower(909.5f, int8_t(power)) + == (boardAllows(power, 909.5f) && power >= 10 && power <= 30)); + DacPaSX1276Wrapper empty(chip, board, 4, nullptr, 0); + assert(!empty.validateBeaconPower(909.5f, 17)); + DacPaSX1276Wrapper impossible(chip, board, 4, levels, 4, 9); + assert(!impossible.validateBeaconPower(909.5f, 17)); + assert(power_hardware_calls == 0); +} +#endif + +int main(int argc, char** argv) { + mesh::MainBoard board; +#if defined(TEST_STM32) + STM32WLx chip; +#ifdef TEST_LP_ONLY + chip.module.hp = false; +#elif defined(TEST_HP_ONLY) + chip.module.lp = false; +#elif defined(TEST_NO_PA) + chip.module.hp = chip.module.lp = false; +#endif +#elif MESH_RADIO_PROFILE_CHIP == 1276 + SX1276 chip; +#elif MESH_RADIO_PROFILE_CHIP == 1272 + SX1272 chip; +#elif MESH_RADIO_PROFILE_CHIP == 1110 + LR1110 chip; +#elif MESH_RADIO_PROFILE_CHIP == 1121 + LR1121 chip; +#elif MESH_RADIO_PROFILE_CHIP == 2021 + LR2021 chip; +#else + SX1262 chip; +#endif + RadioLibWrapper wrapper(chip, board); +#if !defined(TEST_NO_PA) + if (argc == 2 && !std::strcmp(argv[1], "manager")) { manager(wrapper); return 0; } + if (argc == 2 && !std::strcmp(argv[1], "legacy_write")) { upgrade(wrapper, true); return 0; } + if (argc == 2 && !std::strcmp(argv[1], "upgrade")) { upgrade(wrapper, false); return 0; } +#else + (void)argc; (void)argv; +#endif + capability(wrapper, chip); +#if !defined(TEST_NO_PA) + manager(wrapper); +#endif +#if MESH_RADIO_PROFILE_CHIP == 1276 + externalAmplifier(); +#endif +#if MESH_RADIO_PROFILE_CHIP == 1262 && !defined(TEST_STM32) + IndicatorRadioHal hal; + IndicatorSX1262Wrapper indicator(chip, board, hal); + assert(indicator.validateBeaconPower(909.5f, 10) == boardAllows(10, 909.5f)); + assert(!indicator.validateBeaconPower(909.5f, 100)); + assert(hal.services == 0 && power_hardware_calls == 0); +#endif + puts("PASS beacon power capability and persisted CLI validation"); +} diff --git a/test/test_beacon_frames.py b/test/test_beacon_frames.py index 4599ed1cb..c73da23b5 100644 --- a/test/test_beacon_frames.py +++ b/test/test_beacon_frames.py @@ -42,6 +42,7 @@ public: bool valid = true, meshtext = true; RadioProfiles profiles; bool validateProfile(const RadioProfileParams& p) const { return valid && RadioProfiles::valid(p); } + bool validateBeaconPower(float, int8_t power) const { return power >= -9 && power <= 22; } bool supportsMeshtext() const { return meshtext; } }; class TestClock { diff --git a/test/test_beacon_manager.py b/test/test_beacon_manager.py index f06b8d7b9..3c0418410 100644 --- a/test/test_beacon_manager.py +++ b/test/test_beacon_manager.py @@ -57,6 +57,9 @@ class BeaconManagerTest(unittest.TestCase): def test_every_config_short_write_and_recovery(self): self.run_scenario("storage") + def test_structural_config_corruption_remains_held(self): + self.run_scenario("structural_storage") + def test_durable_meshtext_ids_fail_closed(self): self.run_scenario("ids") diff --git a/test/test_beacon_power.py b/test/test_beacon_power.py new file mode 100644 index 000000000..76c43e62f --- /dev/null +++ b/test/test_beacon_power.py @@ -0,0 +1,167 @@ +"""Run production beacon validation with real read-only RadioLib checkers. + +Only the driver hardware boundaries and Mesh queue are replaced. No retune, +power write, DAC write, or Indicator HAL service is allowed during validation. +""" +from pathlib import Path +import os +import shutil +import subprocess +import sys +import tempfile +import unittest + +from cpp_source import body + +ROOT = Path(__file__).resolve().parents[1] +RADIO = ROOT / "src/helpers/radiolib" + + +class BeaconPowerTests(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.compiler = os.environ.get("CXX") or shutil.which("g++") or shutil.which("clang++") + if not cls.compiler: + raise RuntimeError("beacon power regression requires a C++17 compiler") + relative = "src/modules/SX126x/SX1262.cpp" + required = (relative, "src/modules/SX127x/SX1278.cpp", "src/modules/SX127x/SX1272.cpp", + "src/modules/LR11x0/LR1110.cpp", "src/modules/LR11x0/LR1120.cpp", + "src/modules/LR2021/LR2021_config.cpp", "src/modules/SX126x/STM32WLx.cpp") + override = os.environ.get("MESHCORE_RADIOLIB_DIR") + candidates = [Path(override)] if override else sorted((ROOT / ".pio/libdeps").glob("*/RadioLib")) + sdk = next((folder for folder in candidates if all((folder / name).is_file() for name in required)), None) + if sdk is None: + raise RuntimeError("an existing pinned RadioLib checkout is required (no download)") + methods = [] + for filename, original, replacement, signatures in ( + (relative, "SX1262", "SX1262", ("int16_t SX1262::checkOutputPower(",)), + ("src/modules/SX127x/SX1278.cpp", "SX1278", "SX1276", + ("int16_t SX1278::checkOutputPower(int8_t power, int8_t* clipped)", + "int16_t SX1278::checkOutputPower(int8_t power, int8_t* clipped, bool")), + ("src/modules/SX127x/SX1272.cpp", "SX1272", "SX1272", + ("int16_t SX1272::checkOutputPower(int8_t power, int8_t* clipped)", + "int16_t SX1272::checkOutputPower(int8_t power, int8_t* clipped, bool")), + ("src/modules/LR11x0/LR1110.cpp", "LR1110", "LR1110", + ("int16_t LR1110::checkOutputPower(int8_t power, int8_t* clipped)", + "int16_t LR1110::checkOutputPower(int8_t power, int8_t* clipped, bool")), + ("src/modules/LR11x0/LR1120.cpp", "LR1120", "LR1121", + ("int16_t LR1120::checkOutputPower(int8_t power, int8_t* clipped)", + "int16_t LR1120::checkOutputPower(int8_t power, int8_t* clipped, bool")), + ("src/modules/LR2021/LR2021_config.cpp", "LR2021", "LR2021", + ("int16_t LR2021::checkOutputPower(",)), + ("src/modules/SX126x/STM32WLx.cpp", "STM32WLx", "STM32WLx", + ("int16_t STM32WLx::checkOutputPower(",))): + text = (sdk / filename).read_text(encoding="utf-8") + methods += [body(text, signature).replace(original + "::", replacement + "::") + for signature in signatures] + header = (ROOT / "test/fixtures/beacons/power_radio.h").read_text(encoding="ascii") + header = header.replace("@DRIVER_CHECKS@", "") + header = header.replace("@RADIO_CAPABILITY@", body( + (ROOT / "src/Dispatcher.h").read_text(encoding="ascii"), "virtual bool validateBeaconPower(")) + header = header.replace("@INDICATOR_WRAPPER@", body( + (ROOT / "variants/sensecap_indicator-espnow/IndicatorSX1262Wrapper.h").read_text(encoding="ascii"), + "class IndicatorSX1262Wrapper") + ";") + dac = (RADIO / "DacPaSX1276Wrapper.h").read_text(encoding="ascii") + header = header.replace("@DAC_LEVEL@", body(dac, "struct DacPaLevel") + ";") + header = header.replace("@DAC_WRAPPER@", body(dac, "class DacPaSX1276Wrapper") + ";") + mesh = (ROOT / "test/fixtures/beacons/mocks/Mesh.h").read_text(encoding="ascii") + validator = body(mesh, "bool validateTextBeacon(") + mesh = mesh.replace(validator, "bool validateTextBeacon(const BeaconSlot&) const;") + mesh = mesh.replace("#pragma once", '#pragma once\n#include "PowerRadio.h"') + mesh = mesh.replace("class Mesh {\n public:", "class Mesh {\n public:\n ::RadioLibWrapper* _radio = nullptr;") + cls.shared = (RADIO / "RadioLibWrappers.cpp").read_text(encoding="utf-8") + cls.mesh_source = (ROOT / "src/MeshBeacon.cpp").read_text(encoding="ascii") + production = "\n".join(methods) + "\n" + "\n".join(body(cls.shared, signature) for signature in ( + "bool RadioLibWrapper::validateBeaconPower(", "bool RadioLibWrapper::checkBeaconOutputPower(", + "bool RadioLibWrapper::validateProfile(")) + production += "\nnamespace mesh {\n" + body(cls.mesh_source, "bool Mesh::validateTextBeacon(") + "\n}" + production += "\nnamespace mesh { namespace beacon {\n" + body( + (ROOT / "src/helpers/BeaconFrames.cpp").read_text(encoding="ascii"), "bool validText(") + "\n}}" + cls.fixture = (ROOT / "test/fixtures/beacons/power_test.cpp").read_text(encoding="ascii") + cls.production = production + cls.work = tempfile.TemporaryDirectory(prefix="meshcore-beacon-power-") + cls.addClassCleanup(cls.work.cleanup) + cls.directory = Path(cls.work.name) + (cls.directory / "PowerRadio.h").write_text(header, encoding="ascii") + (cls.directory / "Mesh.h").write_text(mesh, encoding="ascii") + + def compile_run(self, name, flags, production=None, scenario=None, negative=False, manager_source=None): + cpp, binary = self.directory / (name + ".cpp"), self.directory / name + cpp.write_text(self.fixture.replace("@PRODUCTION_METHODS@", production or self.production), encoding="ascii") + sanitizers = (["-fsanitize=address,undefined", "-fno-sanitize-recover=all", "-fno-pie", "-no-pie"] + if sys.platform.startswith("linux") else []) + built = subprocess.run([ + self.compiler, "-std=c++17", "-O1", "-g", "-Wall", "-Wextra", "-Werror", + "-DMESH_ENABLE_TX_BEACONS=1", "-DMESH_ENABLE_MESHTEXT_CHAT=0", + "-DMESH_ENABLE_MESHTEXT_WIRE=1", "-DLORA_TX_POWER=2", *flags, *sanitizers, + "-I" + str(self.directory), "-I" + str(ROOT / "test/fixtures/beacons/mocks"), + "-I" + str(ROOT / "test/fixtures/radio_profiles/mocks"), "-isystem", str(ROOT / "test/mocks"), + "-I" + str(ROOT / "src"), "-I" + str(ROOT / "src/helpers"), + str(manager_source or ROOT / "src/helpers/BeaconManager.cpp"), + str(ROOT / "src/helpers/MeshtextChat.cpp"), str(cpp), "-o", str(binary)], + capture_output=True, text=True, timeout=60) + self.assertEqual(built.returncode, 0, built.stdout + built.stderr) + result = subprocess.run([str(binary), *([scenario] if scenario else [])], + capture_output=True, text=True, cwd=self.directory, timeout=15) + if negative: + self.assertNotEqual(result.returncode, 0, "old power validation unexpectedly passed") + self.assertIn("Assertion", result.stderr) + else: + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + def test_chip_ranges_prospective_bands_board_caps_and_external_pa(self): + for name, flags in ( + ("sx1262", ["-DRADIO_CLASS=CustomSX1262"]), + ("sx1268", ["-DRADIO_CLASS=CustomSX1268"]), + ("llcc68", ["-DRADIO_CLASS=CustomLLCC68"]), + ("sx1276", ["-DRADIO_CLASS=CustomSX1276"]), + ("sx1272", ["-DUSE_SX1272=1"]), + ("lr1110", ["-DRADIO_CLASS=CustomLR1110"]), + ("lr1121", ["-DRADIO_CLASS=CustomLR1121"]), + ("lr1121_hp", ["-DRADIO_CLASS=CustomLR1121", "-DLR1121_FORCE_HP_PA=1"]), + ("lr2021", ["-DRADIO_CLASS=CustomLR2021", "-DUSE_LR2021=1"]), + ("stm32_dual", ["-DRADIO_CLASS=CustomSTM32WLx", "-DTEST_STM32=1"]), + ("stm32_lp", ["-DRADIO_CLASS=CustomSTM32WLx", "-DTEST_STM32=1", "-DTEST_LP_ONLY=1"]), + ("stm32_hp", ["-DRADIO_CLASS=CustomSTM32WLx", "-DTEST_STM32=1", "-DTEST_HP_ONLY=1"]), + ("stm32_no_pa", ["-DRADIO_CLASS=CustomSTM32WLx", "-DTEST_STM32=1", "-DTEST_NO_PA=1"]), + ("board_cap", ["-DRADIO_CLASS=CustomSX1262", "-DMIN_LORA_TX_POWER=0", "-DMAX_LORA_TX_POWER=13"]), + ("dac_cap", ["-DRADIO_CLASS=CustomSX1276", "-DMIN_LORA_TX_POWER=17", "-DMAX_LORA_TX_POWER=30"])): + with self.subTest(chip=name): + self.compile_run(name, flags) + + def test_previous_mesh_validator_accepts_impossible_power_before_persistence(self): + guard = "!_radio->validateBeaconPower(slot.radio.freq, slot.power)" + self.assertEqual(self.production.count(guard), 1) + old = self.production.replace(guard, + "(slot.power < -9 || !isLoRaTxPowerValid(slot.power, slot.radio.freq))") + self.compile_run("old_mesh", ["-DRADIO_CLASS=CustomSX1262"], old, "manager", negative=True) + + def test_legacy_power_image_remains_queryable_and_repairable(self): + guard = "!_radio->validateBeaconPower(slot.radio.freq, slot.power)" + self.assertEqual(self.production.count(guard), 1) + legacy = self.production.replace(guard, + "(slot.power < -9 || !isLoRaTxPowerValid(slot.power, slot.radio.freq))") + flags = ["-DRADIO_CLASS=CustomSX1262"] + self.compile_run("legacy_image", flags, legacy, "legacy_write") + self.compile_run("upgrade", flags, scenario="upgrade") + # Restoring the former disk-time capability check bricks every beacon + # command on precisely the same actual legacy image. + source = (ROOT / "src/helpers/BeaconManager.cpp").read_text(encoding="ascii") + marker = "if (!decodeSlot(bytes, slot)" + self.assertEqual(source.count(marker), 1) + held = self.directory / "held_manager.cpp" + held.write_text(source.replace(marker, + marker + " || (slot.enabled && !mesh_->validateTextBeacon(slot))"), encoding="ascii") + self.compile_run("held_upgrade", flags, scenario="upgrade", negative=True, manager_source=held) + + def test_current_band_driver_check_is_not_a_prospective_band_check(self): + checker = body(self.shared, "bool RadioLibWrapper::checkBeaconOutputPower(") + old = self.production.replace(checker, """bool RadioLibWrapper::checkBeaconOutputPower( + float, int8_t power) const { + return _radio->checkOutputPower(power, nullptr) == RADIOLIB_ERR_NONE; + }""") + self.compile_run("old_band", ["-DRADIO_CLASS=CustomLR1121"], old, negative=True) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/test_beacon_radio.py b/test/test_beacon_radio.py index 5ee14fcd0..0472583e8 100644 --- a/test/test_beacon_radio.py +++ b/test/test_beacon_radio.py @@ -25,6 +25,8 @@ HARNESS = r''' #include #include #include +#include +#include #define MESH_ENABLE_TX_BEACONS 1 #define MESH_ENABLE_MESHTEXT_WIRE 1 #define STATE_IDLE 0 @@ -60,6 +62,7 @@ struct Modem { int finishTransmit() { ++finishes; return 0; } int startTransmit(uint8_t*, int) { return tx_result; } int setPreambleLength(uint16_t value) { preamble = value; return 0; } + int checkOutputPower(int8_t value, int8_t*) { return value >= -9 && value <= 22 ? 0 : -1; } }; struct RadioLibWrapper { Modem modem; @@ -162,6 +165,8 @@ struct RadioLibWrapper { bool startSendRaw(const uint8_t*, int); void onSendFinished(); uint32_t getBeaconAirtime(const mesh::RadioProfileParams&, int); + bool validateBeaconPower(float, int8_t) const; + virtual bool checkBeaconOutputPower(float, int8_t) const; void cacheBeaconRestoreParams(); Result prepareBeaconTransmit(const mesh::RadioProfileParams&, int8_t); Result restoreBeaconReceive(); @@ -311,6 +316,13 @@ int main() { assert(w.prepareBeaconTransmit(p, 10) == Result::FAILED && !w._beacon_owned); ++cases; } + for (int power : {-10, 23, 100, 127}) { + RadioLibWrapper w; + assert(w.prepareBeaconTransmit(beacon(), int8_t(power)) == Result::FAILED); + assert(!w._beacon_owned && !w.applications && !w.power_sets); + assert(!w.modem.standbys && !w.retune_begins && !w.stops); + ++cases; + } std::printf("PASS: %u actual-method beacon radio cases\n", cases); } ''' @@ -327,6 +339,8 @@ class BeaconRadioTest(unittest.TestCase): "bool RadioLibWrapper::startSendRaw(const uint8_t* bytes, int len)", "void RadioLibWrapper::onSendFinished()", "uint32_t RadioLibWrapper::getBeaconAirtime(", + "bool RadioLibWrapper::validateBeaconPower(", + "bool RadioLibWrapper::checkBeaconOutputPower(", "void RadioLibWrapper::cacheBeaconRestoreParams()", "mesh::RadioParamApplyResult RadioLibWrapper::prepareBeaconTransmit(", "bool RadioLibWrapper::isBeaconChannelBusy()", From 41564bf6b7939d8407eba9b2bf38220e93378fe5 Mon Sep 17 00:00:00 2001 From: mikecarper Date: Fri, 16 Oct 2026 00:47:45 -0700 Subject: [PATCH 09/14] Verify SX126x status reads against the current RadioLib pin --- src/helpers/radiolib/SX126xReceiveMode.h | 3 +- test/test_sx126x_receive_mode.py | 109 ++++++++++++++++++----- 2 files changed, 91 insertions(+), 21 deletions(-) diff --git a/src/helpers/radiolib/SX126xReceiveMode.h b/src/helpers/radiolib/SX126xReceiveMode.h index 706ff773a..98ed60de5 100644 --- a/src/helpers/radiolib/SX126xReceiveMode.h +++ b/src/helpers/radiolib/SX126xReceiveMode.h @@ -3,7 +3,8 @@ // -1: no reliable observation, 0: confirmed non-RX mode, 1: RX. // GetStatus is C0 followed by one response byte (Semtech sx126x driver). -// RadioLib's pinned getStatus() requests zero payload bytes and returns zero. +// RadioLib 7.8.1 also reads that response directly. This probe additionally +// avoids waiting on BUSY, preserves the SPI layout and checks transport errors. template inline int8_t sx126xReceiveMode(Radio* radio) { Module* module = radio->getMod(); diff --git a/test/test_sx126x_receive_mode.py b/test/test_sx126x_receive_mode.py index 32d98bb3e..d06d177b3 100644 --- a/test/test_sx126x_receive_mode.py +++ b/test/test_sx126x_receive_mode.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -"""Exercise the pinned RadioLib SPI transport with a fake Semtech status response.""" +"""Exercise legacy/current status reads and the mode probe on pinned SPI transport.""" from pathlib import Path import os import re @@ -19,22 +19,27 @@ struct Hal : RadioLibHal { Hal() : RadioLibHal(0,1,0,1,1,0) {} bool busy = false; uint8_t status = 0x54; - unsigned transfers = 0; + unsigned transfers = 0, gpio_reads = 0; + Module* module = nullptr; + Module::BitWidth_t transferred_status_width = Module::BITS_8; uint32_t time = 0; void pinMode(uint32_t,uint32_t) override {} void digitalWrite(uint32_t,uint32_t) override {} - uint32_t digitalRead(uint32_t) override { return busy; } + uint32_t digitalRead(uint32_t) override { ++gpio_reads; return busy; } void attachInterrupt(uint32_t,void(*)(),uint32_t) override {} void detachInterrupt(uint32_t) override {} void delay(RadioLibTime_t ms) override { time += ms; } void delayMicroseconds(RadioLibTime_t) override {} RadioLibTime_t millis() override { return time; } RadioLibTime_t micros() override { return time * 1000; } + void yield() override { ++time; } long pulseIn(uint32_t,uint32_t,RadioLibTime_t) override { return 0; } void spiBegin() override {} void spiBeginTransaction() override {} void spiTransfer(uint8_t* out, size_t size, uint8_t* in) override { assert(size == 2 && out[0] == 0xC0); + assert(out[1] == RADIOLIB_SX126X_CMD_NOP); + transferred_status_width = module->spiConfig.widths[RADIOLIB_MODULE_SPI_WIDTH_STATUS]; memset(in, 0, size); in[1] = status; // GetStatus has one response byte, no payload after it. ++transfers; @@ -45,39 +50,98 @@ struct Hal : RadioLibHal { struct Radio { Module* mod; Module* getMod() { return mod; } - @LEGACY@ + // Explicit historical bug: zero payload leaves data untouched even though + // the real Module transport clocks the status-prefix byte from the chip. + uint8_t getLegacyStatus() { + uint8_t data = 0; + this->mod->SPIreadStream(RADIOLIB_SX126X_CMD_GET_STATUS, &data, 0); + return data; + } + @CURRENT_GET_STATUS@ + static @STATUS_PARSER@ }; + +static void assertLayout(const Module& module, const Module::SPIConfig_t& before) { + for (unsigned i = 0; i < 3; ++i) assert(module.spiConfig.widths[i] == before.widths[i]); + for (unsigned i = 0; i < 4; ++i) assert(module.spiConfig.cmds[i] == before.cmds[i]); + assert(module.spiConfig.statusPos == before.statusPos); + assert(module.spiConfig.parseStatusCb == before.parseStatusCb); + assert(module.spiConfig.checkStatusCb == before.checkStatusCb); + assert(module.spiConfig.timeout == before.timeout); + assert(module.spiConfig.stream == before.stream); +} + int main() { Hal hal; Module module(&hal,1,2,3,4); + hal.module = &module; + module.spiConfig.widths[RADIOLIB_MODULE_SPI_WIDTH_ADDR] = Module::BITS_16; module.spiConfig.widths[RADIOLIB_MODULE_SPI_WIDTH_CMD] = Module::BITS_8; module.spiConfig.widths[RADIOLIB_MODULE_SPI_WIDTH_STATUS] = Module::BITS_8; + module.spiConfig.cmds[RADIOLIB_MODULE_SPI_COMMAND_NOP] = RADIOLIB_SX126X_CMD_NOP; module.spiConfig.statusPos = 1; - module.spiConfig.parseStatusCb = nullptr; + module.spiConfig.parseStatusCb = Radio::SPIparseStatus; module.spiConfig.checkStatusCb = nullptr; Radio radio{&module}; - assert(radio.getStatus() == 0); // reproduces the PR's false non-RX observation + assert(radio.getLegacyStatus() == 0); // reproduce the old false non-RX observation + assert(hal.transfers == 1 && hal.transferred_status_width == Module::BITS_8); + assert(radio.getStatus() == 0x54); // the pinned implementation already fixes that bug + assert(hal.transfers == 2 && hal.transferred_status_width == Module::BITS_0); + assert(module.spiConfig.widths[RADIOLIB_MODULE_SPI_WIDTH_STATUS] == Module::BITS_8); assert(sx126xReceiveMode(&radio) == 1); - assert(module.spiConfig.widths[RADIOLIB_MODULE_SPI_WIDTH_STATUS] == 8); - for (uint8_t mode : {0x24, 0x34, 0x44, 0x64}) { - hal.status = mode; - assert(sx126xReceiveMode(&radio) == 0); - } - for (uint8_t invalid : {0x00, 0xFF, 0x14, 0x74}) { - hal.status = invalid; - assert(sx126xReceiveMode(&radio) == -1); - assert(module.spiConfig.widths[RADIOLIB_MODULE_SPI_WIDTH_STATUS] == 8); + assert(hal.transfers == 3 && hal.transferred_status_width == Module::BITS_0); + struct Observation { uint8_t status; int8_t mode; }; + for (Observation observation : { + Observation{0x24, 0}, {0x34, 0}, {0x44, 0}, {0x54, 1}, {0x64, 0}, + {0x00, -1}, {0xFF, -1}, {0x04, -1}, {0x14, -1}, {0x74, -1}, + {0x56, -1}, {0x58, -1}, {0x5A, -1}}) { + hal.status = observation.status; + // The current upstream method returns raw status, including command-error + // responses. The helper must reject those through the real status parser. + assert(radio.getStatus() == observation.status); + assert(hal.transferred_status_width == Module::BITS_0); + assert(module.spiConfig.widths[RADIOLIB_MODULE_SPI_WIDTH_STATUS] == Module::BITS_8); + for (Module::BitWidth_t width : { + Module::BITS_0, Module::BITS_8, Module::BITS_16, Module::BITS_24, Module::BITS_32}) { + module.spiConfig.widths[RADIOLIB_MODULE_SPI_WIDTH_STATUS] = width; + const auto layout = module.spiConfig; + const unsigned before = hal.transfers; + assert(sx126xReceiveMode(&radio) == observation.mode); + assert(hal.transfers == before + 1 && hal.transferred_status_width == Module::BITS_0); + assertLayout(module, layout); + } + module.spiConfig.widths[RADIOLIB_MODULE_SPI_WIDTH_STATUS] = Module::BITS_8; } const unsigned before = hal.transfers; + const auto layout = module.spiConfig; + const uint32_t time = hal.time; hal.busy = true; assert(sx126xReceiveMode(&radio) == -1); assert(hal.transfers == before); // no SPI while the chip is asleep/busy + assert(hal.time == time); // do not wait for the transport's BUSY timeout + assertLayout(module, layout); + + // Unlike the helper, the pinned getStatus() waits for BUSY to time out. + assert(radio.getStatus() == 0); + assert(hal.transfers == before && hal.time >= time + module.spiConfig.timeout); + assertLayout(module, layout); + + // A board with no BUSY pin must still be able to observe the response. + Module no_busy(&hal,1,2,3,RADIOLIB_NC); + no_busy.spiConfig = module.spiConfig; + hal.module = &no_busy; + hal.status = 0x54; + const unsigned reads = hal.gpio_reads; + Radio no_busy_radio{&no_busy}; + assert(sx126xReceiveMode(&no_busy_radio) == 1); + assert(hal.transfers == before + 1 && hal.gpio_reads == reads); + assertLayout(no_busy, layout); } ''' class ReceiveModeTest(unittest.TestCase): - def test_real_spi_transport_and_legacy_status(self): + def test_real_spi_transport_legacy_current_and_mode_probe(self): candidates = sorted((ROOT / '.pio/libdeps').glob('*/RadioLib/src/Module.cpp')) pin = re.search(r'jgromes/RadioLib.git#([0-9a-f]{40})', (ROOT / 'platformio.ini').read_text()).group(1) @@ -94,19 +158,24 @@ class ReceiveModeTest(unittest.TestCase): commands = (lib / 'modules/SX126x/SX126x_commands.cpp').read_text() start = commands.index('uint8_t SX126x::getStatus()') end = commands.index('\n}', start) + 2 - legacy = commands[start:end].replace('SX126x::', '') + current = commands[start:end].replace('SX126x::', '') + radio_source = (lib / 'modules/SX126x/SX126x.cpp').read_text() + start = radio_source.index('int16_t SX126x::SPIparseStatus(') + end = radio_source.index('\n}', start) + 2 + parser = radio_source[start:end].replace('SX126x::', '') with tempfile.TemporaryDirectory(prefix='meshcore-status-') as tmp: source = Path(tmp) / 'test.cpp' binary = Path(tmp) / 'test' - source.write_text(HARNESS.replace('@LEGACY@', legacy)) + source.write_text(HARNESS.replace('@CURRENT_GET_STATUS@', current) + .replace('@STATUS_PARSER@', parser), encoding='ascii') command = [os.environ.get('CXX', 'c++'), '-std=c++17', '-O1', '-DRADIOLIB_GODMODE=1', '-DRADIOLIB_STATIC_ONLY=1', '-ffunction-sections', '-fdata-sections', '-Wl,--gc-sections', '-I', str(lib), '-I', str(ROOT / 'src'), str(source), str(lib / 'Module.cpp'), str(lib / 'Hal.cpp'), '-o', str(binary)] - result = subprocess.run(command, capture_output=True, text=True) + result = subprocess.run(command, capture_output=True, text=True, timeout=60) self.assertEqual(result.returncode, 0, result.stderr) - result = subprocess.run([str(binary)], capture_output=True, text=True) + result = subprocess.run([str(binary)], capture_output=True, text=True, timeout=10) self.assertEqual(result.returncode, 0, result.stderr) From e629f1c44f3c46183e32a11c93cd545ba98273da Mon Sep 17 00:00:00 2001 From: mikecarper Date: Fri, 16 Oct 2026 00:50:55 -0700 Subject: [PATCH 10/14] Fit headless RAK Companion without shrinking storage --- .github/workflows/run-unit-tests.yml | 1 + test/test_stm32_companion_profile.py | 56 ++++++++++++++++++++++++++++ variants/rak3x72/platformio.ini | 4 ++ 3 files changed, 61 insertions(+) create mode 100644 test/test_stm32_companion_profile.py diff --git a/.github/workflows/run-unit-tests.yml b/.github/workflows/run-unit-tests.yml index 7ef970b9c..79aed4f28 100644 --- a/.github/workflows/run-unit-tests.yml +++ b/.github/workflows/run-unit-tests.yml @@ -96,6 +96,7 @@ jobs: - name: Verify compact STM32 signature arithmetic run: | + python3 -B test/test_stm32_companion_profile.py -v python3 -B test/test_ed25519_compact_scalar.py -v python3 -B test/test_ed25519_compact.py -v diff --git a/test/test_stm32_companion_profile.py b/test/test_stm32_companion_profile.py new file mode 100644 index 000000000..aa82cd8fb --- /dev/null +++ b/test/test_stm32_companion_profile.py @@ -0,0 +1,56 @@ +#!/usr/bin/env python3 +"""Guard the headless RAK Companion's size recipe and deployed storage contract. + +The stm32-companion-smoke job also links the actual image. These checks prevent +fixing its size limit by silently shrinking storage or Companion capacities. +""" + +import configparser +from pathlib import Path +import re +import unittest + + +ROOT = Path(__file__).resolve().parents[1] + + +class STM32CompanionProfileTest(unittest.TestCase): + def setUp(self): + self.config = configparser.ConfigParser( + interpolation=None, inline_comment_prefixes=(";",)) + self.config.read(ROOT / "variants/rak3x72/platformio.ini") + self.companion = self.config["env:RAK_3x72_companion_radio_usb"] + + def test_unused_timer_wrappers_are_removed_only_from_headless_companion(self): + for flag in ("RADIOLIB_TONE_UNSUPPORTED", "HAL_TIM_MODULE_ONLY"): + self.assertRegex(self.companion["build_flags"], r"-D\s+" + flag + r"\b") + for section in self.config.sections(): + if section != self.companion.name: + self.assertNotIn(flag, self.config[section].get("build_flags", "")) + + def test_flash_and_filesystem_boundaries_do_not_move(self): + self.assertNotIn("board_upload.maximum_size", self.companion) + self.assertEqual(self.config["rak3x72"].getint("board_upload.maximum_size"), + 229376) + self.assertNotIn("LFS_FLASH_TOTAL_SIZE", self.companion["build_flags"]) + self.assertNotIn("LFS_FLASH_TOTAL_SIZE", self.config["rak3x72"]["build_flags"]) + header = (ROOT / "src/helpers/stm32/InternalFileSystem.h").read_text() + self.assertRegex(header, r"#define\s+LFS_FLASH_TOTAL_SIZE\s+\(16\s*\*\s*2048\)") + self.assertEqual(229376 + 16 * 2048, 256 * 1024) + + def test_companion_capacities_and_interfaces_are_retained(self): + flags = self.companion["build_flags"] + for definition in ("MAX_CONTACTS=100", "MAX_GROUP_CHANNELS=8", + "ENABLE_USB_INTERFACE"): + self.assertRegex(flags, r"-D\s+" + re.escape(definition) + r"\b") + for flags in (flags, self.config["rak3x72"]["build_flags"]): + self.assertNotRegex(flags, r"COMPANION_FEATURE_\w+=0") + self.assertNotRegex(flags, r"MESH_ENABLE_\w+=0") + self.assertIn("+<../examples/companion_radio/*.cpp>", + self.companion["build_src_filter"]) + self.assertIn("-Oz", self.companion["build_flags"]) + self.assertEqual(self.companion["build_unflags"], "-Os") + + +if __name__ == "__main__": + unittest.main() diff --git a/variants/rak3x72/platformio.ini b/variants/rak3x72/platformio.ini index ae47ba55a..ddd87e295 100644 --- a/variants/rak3x72/platformio.ini +++ b/variants/rak3x72/platformio.ini @@ -66,6 +66,10 @@ extends = rak3x72 platform_packages = platformio/toolchain-gccarmnoneeabi@^1.140201.0 build_unflags = -Os build_flags = ${rak3x72.build_flags} + ; This headless UART Companion uses GPIO, ADC and SysTick, not PWM/tone. + ; Drop unused Arduino timer wrappers, retaining the low-level STM32 TIM HAL. + -D RADIOLIB_TONE_UNSUPPORTED + -D HAL_TIM_MODULE_ONLY -D ED25519_COMPACT_SHA512=1 ; same hash rounds, compact loop preserves the filesystem boundary -D ED25519_COMPACT_SCALAR=1 ; same ref10 arithmetic with fixed loops instead of unrolled code -Oz From 6614be22f24996472daac8c2602b4527b966906f Mon Sep 17 00:00:00 2001 From: mikecarper Date: Fri, 16 Oct 2026 00:53:06 -0700 Subject: [PATCH 11/14] Retain USB commands until required replies can be queued --- .github/workflows/run-unit-tests.yml | 1 + src/helpers/ArduinoSerialInterface.cpp | 46 +++ src/helpers/ArduinoSerialInterface.h | 7 + .../arduino_serial_backpressure/test.cpp | 364 ++++++++++++++++++ test/test_arduino_serial_backpressure.py | 91 +++++ test/test_companion_usb_logging_reply.py | 14 +- 6 files changed, 521 insertions(+), 2 deletions(-) create mode 100644 test/fixtures/arduino_serial_backpressure/test.cpp create mode 100644 test/test_arduino_serial_backpressure.py diff --git a/.github/workflows/run-unit-tests.yml b/.github/workflows/run-unit-tests.yml index 79aed4f28..14ab4ddda 100644 --- a/.github/workflows/run-unit-tests.yml +++ b/.github/workflows/run-unit-tests.yml @@ -565,6 +565,7 @@ jobs: run: | python3 -B test/test_companion_usb_default.py python3 -B test/test_companion_usb_input.py + python3 -B test/test_arduino_serial_backpressure.py -v python3 -B test/test_companion_ota_includes.py python3 -B test/test_companion_usb_logging_handoff.py python3 -B test/test_companion_usb_mota_exit.py diff --git a/src/helpers/ArduinoSerialInterface.cpp b/src/helpers/ArduinoSerialInterface.cpp index 92d3d17f9..4e54a42a6 100644 --- a/src/helpers/ArduinoSerialInterface.cpp +++ b/src/helpers/ArduinoSerialInterface.cpp @@ -21,10 +21,14 @@ void ArduinoSerialInterface::resetReceiveState() { _frame_len = 0; rx_len = 0; _last_rx_byte_ms = 0; + _receive_capacity_paused = false; + _receive_capacity_paused_at = 0; } void ArduinoSerialInterface::serviceReceiveTimeout() { + updateReceiveCapacityPause(millis()); if (_state != RECV_STATE_IDLE && _serial != nullptr + && !_receive_capacity_paused && _serial->available() == 0 && (uint32_t)(millis() - _last_rx_byte_ms) >= RX_FRAME_TIMEOUT_MS) { // A truncated length-prefixed frame must not hold the MCU awake forever or @@ -33,6 +37,40 @@ void ArduinoSerialInterface::serviceReceiveTimeout() { } } +bool ArduinoSerialInterface::responseCapacityAvailable() const { + if (!_flow_ctl || _tx_queue_len < TX_QUEUE_SIZE) return true; + // Match enqueueFrame(): a required reply may replace an unsent best-effort + // push, but the prefix of a partly sent frame must remain intact. + for (uint8_t i = _tx_offset == 0 ? 0 : 1; i < _tx_queue_len; ++i) { + const TxFrame& frame = _tx_queue[i]; + if (!mesh::companionFrameRequiresDelivery(&frame.buf[3], frame.len - 3)) { + return true; + } + } + return false; +} + +void ArduinoSerialInterface::updateReceiveCapacityPause(uint32_t now) { + if (_state == RECV_STATE_IDLE) { + _receive_capacity_paused = false; + _receive_capacity_paused_at = 0; + return; + } + const bool pause = !responseCapacityAvailable(); + if (pause && !_receive_capacity_paused) { + // Backpressure must not retroactively extend an already expired frame. + if ((uint32_t)(now - _last_rx_byte_ms) >= RX_FRAME_TIMEOUT_MS) return; + _receive_capacity_paused = true; + _receive_capacity_paused_at = now; + } else if (!pause && _receive_capacity_paused) { + // Bytes retained for reply capacity keep the framing budget they had when + // RX was paused, even if the host takes a long time to resume reading. + _last_rx_byte_ms += (uint32_t)(now - _receive_capacity_paused_at); + _receive_capacity_paused = false; + _receive_capacity_paused_at = 0; + } +} + void ArduinoSerialInterface::resetTransmitState() { _tx_queue_len = 0; _tx_offset = 0; @@ -82,6 +120,7 @@ bool ArduinoSerialInterface::enqueueFrame(const uint8_t src[], size_t len) { frame.buf[1] = (uint8_t)(len & 0xFF); frame.buf[2] = (uint8_t)(len >> 8); memcpy(&frame.buf[3], src, len); + updateReceiveCapacityPause(millis()); return true; } @@ -204,6 +243,7 @@ bool ArduinoSerialInterface::isConnected() const { void ArduinoSerialInterface::loop() { serviceReceiveTimeout(); serviceTransmit(); + updateReceiveCapacityPause(millis()); } bool ArduinoSerialInterface::isReadBusy() const { @@ -240,6 +280,7 @@ size_t ArduinoSerialInterface::writeFrame(const uint8_t src[], size_t len) { } if (!enqueueFrame(src, len)) return 0; serviceTransmit(); + updateReceiveCapacityPause(millis()); return len; } @@ -255,7 +296,12 @@ size_t ArduinoSerialInterface::checkRecvFrame(uint8_t dest[]) { if (_serial == nullptr || dest == nullptr) return 0; serviceReceiveTimeout(); serviceTransmit(); + updateReceiveCapacityPause(millis()); if (_passthroughMode) return 0; + // Immediate command handlers do not retain rejected replies. Leave the + // complete input and any partial parser state untouched until one required + // response can be admitted, rather than executing a command without a reply. + if (!responseCapacityAvailable()) return 0; // Newly arriving bytes belong to the next mesh loop; a continuously writing // host must not keep the radio/watchdog loop trapped in this parser. diff --git a/src/helpers/ArduinoSerialInterface.h b/src/helpers/ArduinoSerialInterface.h index 1492234e1..532956735 100644 --- a/src/helpers/ArduinoSerialInterface.h +++ b/src/helpers/ArduinoSerialInterface.h @@ -38,6 +38,8 @@ private: uint32_t _last_frame_ms; uint32_t _completed_frame_count; uint32_t _last_rx_byte_ms; + uint32_t _receive_capacity_paused_at; + bool _receive_capacity_paused; bool _has_received_frame; Stream* _serial; const char* _controlSequence; @@ -51,6 +53,8 @@ private: bool checkControlLineByte(uint8_t c); void resetReceiveState(); void serviceReceiveTimeout(); + bool responseCapacityAvailable() const; + void updateReceiveCapacityPause(uint32_t now); void resetTransmitState(); bool enqueueFrame(const uint8_t src[], size_t len); void serviceTransmit(); @@ -63,6 +67,7 @@ public: _controlSequenceCursor(nullptr), _secondaryControlSequenceCursor(nullptr), _frame_len(0), rx_len(0), _last_frame_ms(0), _completed_frame_count(0), _last_rx_byte_ms(0), + _receive_capacity_paused_at(0), _receive_capacity_paused(false), _has_received_frame(false), _serial(nullptr), _controlSequence(nullptr), _secondaryControlSequence(nullptr), _conn_check(nullptr), @@ -114,11 +119,13 @@ public: // Optional: queue complete frames and drain one at a time according to the // stream's TX capacity, so short writes cannot discard or interleave bytes. // Report busy while a frame remains queued so bulk streams get paced. + // Defer input, including control lines, while a required reply cannot fit. // Only enable this for streams which really implement availableForWrite() // (USB-CDC does, the Print default returns 0). void enableFlowControl(bool enable) { if (!enable) resetTransmitState(); _flow_ctl = enable; + updateReceiveCapacityPause(millis()); } // BaseSerialInterface methods diff --git a/test/fixtures/arduino_serial_backpressure/test.cpp b/test/fixtures/arduino_serial_backpressure/test.cpp new file mode 100644 index 000000000..082cc12d2 --- /dev/null +++ b/test/fixtures/arduino_serial_backpressure/test.cpp @@ -0,0 +1,364 @@ +#include +#include + +#include +#include +#include +#include +#include +#include + +#define REQUIRE(condition) do { \ + if (!(condition)) { \ + std::fprintf(stderr, "FAIL line %d: %s\n", __LINE__, #condition); \ + std::exit(1); \ + } \ +} while (0) + +using Bytes = std::vector; +static const char START_TOKEN[] = "+++MESHCORE-TERM-START"; +static const char MOTA_TOKEN[] = "ota folder on"; + +class BufferStream : public Stream { +public: + std::deque input; + Bytes output; + int capacity = 0; + size_t max_write = std::numeric_limits::max(); + unsigned reads = 0; + + void push(const Bytes& bytes) { input.insert(input.end(), bytes.begin(), bytes.end()); } + void push(const char* text) { while (*text) input.push_back(uint8_t(*text++)); } + int available() override { return static_cast(input.size()); } + int availableForWrite() override { return capacity; } + int peek() override { return input.empty() ? -1 : input.front(); } + int read() override { + if (input.empty()) return -1; + const uint8_t value = input.front(); + input.pop_front(); + ++reads; + return value; + } + size_t write(uint8_t value) override { return write(&value, 1); } + size_t write(const uint8_t* data, size_t len) override { + const size_t written = std::min(std::min(len, max_write), size_t(std::max(capacity, 0))); + output.insert(output.end(), data, data + written); + capacity -= int(written); + return written; + } +}; + +#define RESP_CODE_OK 0 +#define RESP_CODE_ERR 1 +struct MyMesh { + BaseSerialInterface* _serial; + void writeOKFrame(BaseSerialInterface* route = nullptr); + void writeErrFrame(uint8_t code, BaseSerialInterface* route = nullptr); +}; +// The runner extracts these immediate reply methods from production MyMesh.cpp. +#include "immediate_reply_methods.inc" + +static Bytes framed(const std::vector& frames) { + Bytes result; + for (const Bytes& frame : frames) { + result.push_back('>'); + result.push_back(uint8_t(frame.size())); + result.push_back(uint8_t(frame.size() >> 8)); + result.insert(result.end(), frame.begin(), frame.end()); + } + return result; +} + +struct Fixture { + BufferStream stream; + ArduinoSerialInterface usb; + MultiSerialInterface manager; + MyMesh mesh{&manager}; + uint8_t command[MAX_FRAME_SIZE] = {}; + + Fixture() { + g_mock_millis = 100; + usb.begin(stream, START_TOKEN, MOTA_TOKEN); + usb.enableFlowControl(true); + REQUIRE(manager.addInterface(InterfaceType::USB, &usb)); + manager.enable(); + } + size_t tick() { return manager.checkRecvFrame(command); } + void issue(uint8_t tag) { stream.push(Bytes{'<', 1, 0, tag}); } + void reply() { mesh.writeErrFrame(command[0]); } + void enqueue(const Bytes& frame) { + REQUIRE(usb.writeFrame(frame.data(), frame.size()) == frame.size()); + } + std::vector fillRequired() { + std::vector result; + for (uint8_t i = 0; i < 4; ++i) { + result.push_back(Bytes{1, uint8_t(0xA0 + i)}); + enqueue(result.back()); + } + return result; + } + void flush() { + stream.capacity = 65536; + stream.max_write = std::numeric_limits::max(); + manager.loop(); + REQUIRE(!usb.isWriteBusy()); + } +}; + +static void burst() { + Fixture f; + std::vector expected; + for (uint8_t i = 0; i < 5; ++i) f.issue(uint8_t(0x40 + i)); + for (uint8_t i = 0; i < 4; ++i) { + REQUIRE(f.tick() == 1); + REQUIRE(f.command[0] == 0x40 + i); + f.reply(); + expected.push_back(Bytes{1, uint8_t(0x40 + i)}); + } + const unsigned reads = f.stream.reads; + for (unsigned i = 0; i < 8; ++i) { + REQUIRE(f.tick() == 0); + REQUIRE(f.stream.reads == reads && f.stream.input.size() == 4); + REQUIRE(f.usb.getCompletedFrameCount() == 4); + g_mock_millis += 1000; + } + // Release exactly one complete reply, then accept the waiting command into + // that slot even though every later write is still backpressured. + f.stream.capacity = 5; + REQUIRE(f.tick() == 1 && f.command[0] == 0x44); + f.reply(); + expected.push_back(Bytes{1, 0x44}); + f.flush(); + REQUIRE(f.stream.output == framed(expected)); + REQUIRE(f.stream.input.empty() && !f.manager.hasPendingIO()); +} + +static void partialRxBudget() { + for (uint32_t started : {100U, UINT32_MAX - 500U}) { + for (const Bytes& prefix : {Bytes{'<'}, Bytes{'<', 2}, Bytes{'<', 2, 0, 0x40}}) { + Fixture f; + g_mock_millis = started; + f.stream.push(prefix); + REQUIRE(f.tick() == 0 && f.usb.isReadBusy()); + g_mock_millis += 250; + f.fillRequired(); + g_mock_millis += 5000; + f.manager.loop(); + REQUIRE(f.usb.isReadBusy()); + f.flush(); + g_mock_millis += 749; + f.manager.loop(); + REQUIRE(f.usb.isReadBusy()); + ++g_mock_millis; + f.manager.loop(); + REQUIRE(!f.usb.isReadBusy() && !f.manager.hasPendingIO()); + } + } +} + +static void partialRxResume() { + const Bytes wire{'<', 6, 0, 0x70, '<', '>', '\r', 0, 0xEE}; + for (size_t split = 1; split < wire.size(); ++split) { + Fixture f; + f.stream.push(Bytes(wire.begin(), wire.begin() + split)); + REQUIRE(f.tick() == 0 && f.usb.isReadBusy()); + std::vector expected = f.fillRequired(); + f.stream.push(Bytes(wire.begin() + split, wire.end())); + f.issue(0x72); + const unsigned reads = f.stream.reads; + g_mock_millis += 5000; + REQUIRE(f.tick() == 0 && f.stream.reads == reads); + f.stream.capacity = 5; + REQUIRE(f.tick() == 6); + REQUIRE(Bytes(f.command, f.command + 6) == Bytes(wire.begin() + 3, wire.end())); + f.reply(); + expected.push_back(Bytes{1, 0x70}); + REQUIRE(f.tick() == 0 && f.stream.input.size() == 4); + f.flush(); + REQUIRE(f.tick() == 1 && f.command[0] == 0x72); + f.reply(); + expected.push_back(Bytes{1, 0x72}); + f.flush(); + REQUIRE(f.stream.output == framed(expected)); + } +} + +static void evictableCapacity() { + for (bool pinned : {false, true}) { + Fixture f; + Bytes push(MAX_FRAME_SIZE, 0xA5); + push[0] = 0x88; + f.enqueue(push); + if (pinned) { + f.stream.capacity = 2; + f.manager.loop(); + REQUIRE(f.stream.output.size() == 2); + } + const Bytes required{1, 0xA0}; + for (unsigned i = 0; i < 3; ++i) f.enqueue(required); + f.issue(0x40); + const unsigned reads = f.stream.reads; + if (pinned) { + REQUIRE(f.tick() == 0 && f.stream.reads == reads); + // The partly transmitted best-effort head cannot create reply capacity + // until every byte has left the stream; its suffix must remain intact. + f.stream.capacity = MAX_FRAME_SIZE + 1; + } + REQUIRE(f.tick() == 1 && f.command[0] == 0x40); + f.reply(); + f.flush(); + std::vector expected; + if (pinned) expected.push_back(push); + for (unsigned i = 0; i < 3; ++i) expected.push_back(required); + expected.push_back(Bytes{1, 0x40}); + REQUIRE(f.stream.output == framed(expected)); + } + // An unsent best-effort tail remains evictable behind a partly sent required + // head; admission must not stop merely because all four slots are occupied. + Fixture f; + const Bytes required{1, 0xA0}; + f.enqueue(required); + f.stream.capacity = 2; + f.manager.loop(); + f.enqueue(Bytes{0x88, 0xFF}); + f.enqueue(required); + f.enqueue(required); + f.issue(0x40); + REQUIRE(f.tick() == 1); + f.reply(); + f.flush(); + REQUIRE(f.stream.output == framed({required, required, required, Bytes{1, 0x40}})); +} + +static void controlsAndPassthrough() { + for (bool secondary : {false, true}) { + for (bool partial : {false, true}) { + Fixture f; + const char* token = secondary ? MOTA_TOKEN : START_TOKEN; + if (partial) { + f.stream.push(Bytes(token, token + 5)); + REQUIRE(f.tick() == 0); + token += 5; + } + f.fillRequired(); + f.stream.push(token); + f.stream.push("\rver\r"); + const unsigned reads = f.stream.reads; + REQUIRE(f.tick() == 0 && f.stream.reads == reads); + REQUIRE(!f.usb.takeControlSequence() && !f.usb.takeSecondaryControlSequence()); + f.flush(); + REQUIRE(f.tick() == 0); + REQUIRE(f.usb.takeControlSequence() == !secondary); + REQUIRE(f.usb.takeSecondaryControlSequence() == secondary); + REQUIRE(f.stream.input == std::deque({'v', 'e', 'r', '\r'})); + f.usb.setPassthroughMode(true); + const Bytes before = f.stream.output; + const uint8_t response[] = {0}; + REQUIRE(f.usb.writeFrame(response, sizeof(response)) == sizeof(response)); + REQUIRE(f.tick() == 0 && f.stream.input.size() == 4); + REQUIRE(f.stream.output == before); + } + } +} + +static void resetsAndUncontrolledStreams() { + for (unsigned mode = 0; mode < 4; ++mode) { + Fixture f; + f.stream.push(Bytes{'<', 2, 0, 0x40}); + REQUIRE(f.tick() == 0 && f.usb.isReadBusy()); + f.fillRequired(); + g_mock_millis += 5000; + if (mode == 0) { + f.usb.enableFlowControl(false); + f.stream.push(Bytes{0x41}); + REQUIRE(f.tick() == 2 && f.command[0] == 0x40 && f.command[1] == 0x41); + } else { + if (mode == 1) f.usb.resetSessionState(); + if (mode == 2) { f.usb.setPassthroughMode(true); f.usb.setPassthroughMode(false); } + if (mode == 3) { f.usb.disable(); f.usb.enable(); } + f.issue(0x42); + REQUIRE(f.tick() == 1 && f.command[0] == 0x42); + } + REQUIRE(!f.usb.isReadBusy() && !f.manager.hasPendingIO()); + } + Fixture f; + f.usb.enableFlowControl(false); + f.stream.capacity = 4096; + for (unsigned i = 0; i < 5; ++i) { + f.issue(0x40); + REQUIRE(f.tick() == 1); + f.mesh.writeOKFrame(); + } + REQUIRE(f.stream.output == framed({Bytes{0}, Bytes{0}, Bytes{0}, Bytes{0}, Bytes{0}})); +} + +static void expiredBeforeBackpressure() { + Fixture f; + f.stream.push(Bytes{'<', 2, 0, 0x40}); + REQUIRE(f.tick() == 0 && f.usb.isReadBusy()); + g_mock_millis += 1000; + f.fillRequired(); + f.manager.loop(); + REQUIRE(!f.usb.isReadBusy()); + // The normal incomplete-frame timeout still works when there is no TX + // backpressure and must remain exact across the millis wrap boundary. + Fixture normal; + g_mock_millis = UINT32_MAX - 500U; + normal.stream.push(Bytes{'<', 2}); + REQUIRE(normal.tick() == 0); + g_mock_millis += 999; + normal.manager.loop(); + REQUIRE(normal.usb.isReadBusy()); + ++g_mock_millis; + normal.manager.loop(); + REQUIRE(!normal.usb.isReadBusy()); +} + +class OtherInterface : public BaseSerialInterface { +public: + bool pending = true; + void enable() override {} + void disable() override {} + bool isEnabled() const override { return true; } + bool isConnected() const override { return true; } + bool isReadBusy() const override { return false; } + bool isWriteBusy() const override { return false; } + size_t writeFrame(const uint8_t*, size_t len) override { return len; } + size_t checkRecvFrame(uint8_t* dest) override { + if (!pending) return 0; + pending = false; + dest[0] = 0x79; + return 1; + } +}; + +static void independentRoute() { + Fixture f; + OtherInterface other; + REQUIRE(f.manager.addInterface(InterfaceType::Bluetooth, &other)); + f.fillRequired(); + f.issue(0x40); + REQUIRE(f.tick() == 1 && f.command[0] == 0x79); + REQUIRE(f.manager.captureReplyRoute() == &other); + REQUIRE(f.stream.reads == 0 && f.stream.input.size() == 4); + f.flush(); + REQUIRE(f.tick() == 1 && f.command[0] == 0x40); + REQUIRE(f.manager.captureReplyRoute() == &f.usb); +} + +int main(int argc, char** argv) { + REQUIRE(argc == 2); + const int scenario = std::atoi(argv[1]); + switch (scenario) { + case 0: burst(); break; + case 1: partialRxBudget(); break; + case 2: partialRxResume(); break; + case 3: evictableCapacity(); break; + case 4: controlsAndPassthrough(); break; + case 5: resetsAndUncontrolledStreams(); break; + case 6: expiredBeforeBackpressure(); break; + case 7: independentRoute(); break; + default: REQUIRE(false); + } + std::printf("PASS: Arduino serial backpressure scenario %d\n", scenario); +} diff --git a/test/test_arduino_serial_backpressure.py b/test/test_arduino_serial_backpressure.py new file mode 100644 index 000000000..b6eb3d150 --- /dev/null +++ b/test/test_arduino_serial_backpressure.py @@ -0,0 +1,91 @@ +#!/usr/bin/env python3 +"""Exercise production USB framing and immediate replies under backpressure.""" + +from pathlib import Path +import os +import shutil +import subprocess +import sys +import tempfile +import unittest + +from test_replay_reset_integration import extract_braced + + +ROOT = Path(__file__).resolve().parents[1] +FIXTURE = ROOT / "test/fixtures/arduino_serial_backpressure/test.cpp" +TRANSPORT = ROOT / "src/helpers/ArduinoSerialInterface.cpp" +SANITIZERS = ( + ["-fsanitize=address,undefined", "-fno-sanitize-recover=all", "-fno-pie", "-no-pie"] + if sys.platform.startswith("linux") else [] +) +SCENARIOS = ( + "five-command burst retains fifth command until required reply fits", + "partial headers and payloads preserve timeout budget across rollover", + "every RX split resumes intact without consuming the following command", + "only unsent best-effort frames provide replacement reply capacity", + "terminal and mOTA control tokens preserve their following input", + "session, passthrough and flow-control resets clear backpressure state", + "full TX queue cannot revive an already expired inbound frame", + "blocked USB input does not stop commands from another transport", +) + + +class ArduinoSerialBackpressureTests(unittest.TestCase): + def setUp(self): + self.compiler = os.environ.get("CXX") or shutil.which("g++") or shutil.which("clang++") + if self.compiler is None: + self.skipTest("a host C++17 compiler is required") + + def build(self, directory, transport): + directory = Path(directory) + mesh = (ROOT / "examples/companion_radio/MyMesh.cpp").read_text() + methods = "\n".join(extract_braced(mesh, signature) for signature in ( + "void MyMesh::writeOKFrame(", "void MyMesh::writeErrFrame(", + )) + (directory / "immediate_reply_methods.inc").write_text(methods) + candidate = directory / "ArduinoSerialInterface.cpp" + candidate.write_text(transport) + binary = directory / "serial-backpressure" + built = subprocess.run([ + self.compiler, "-std=c++17", *SANITIZERS, + "-I" + str(ROOT / "test/mocks"), "-I" + str(ROOT / "src"), + "-I" + str(ROOT / "src/helpers"), "-I" + str(directory), + str(FIXTURE), str(candidate), "-o", str(binary), + ], capture_output=True, text=True, timeout=60) + self.assertEqual(built.returncode, 0, built.stdout + built.stderr) + return binary + + def run_scenario(self, binary, scenario): + return subprocess.run([str(binary), str(scenario)], capture_output=True, + text=True, timeout=15) + + def test_production_parser_queue_and_immediate_reply_methods(self): + with tempfile.TemporaryDirectory(prefix="meshcore-serial-backpressure-") as directory: + binary = self.build(directory, TRANSPORT.read_text()) + for scenario, description in enumerate(SCENARIOS): + with self.subTest(scenario=description): + checked = self.run_scenario(binary, scenario) + self.assertEqual(checked.returncode, 0, checked.stdout + checked.stderr) + self.assertIn(f"PASS: Arduino serial backpressure scenario {scenario}", + checked.stdout) + + def test_negative_controls_reproduce_lost_reply_and_partial_frame(self): + source = TRANSPORT.read_text() + controls = ( + (" if (!responseCapacityAvailable()) return 0;\n", "", 0), + (" && !_receive_capacity_paused\n", "", 1), + ) + for old, new, scenario in controls: + with self.subTest(removed=old.strip()): + self.assertEqual(source.count(old), 1) + with tempfile.TemporaryDirectory(prefix="meshcore-serial-negative-") as directory: + binary = self.build(directory, source.replace(old, new)) + checked = self.run_scenario(binary, scenario) + self.assertNotEqual(checked.returncode, 0, + "negative control unexpectedly passed") + self.assertIn("FAIL line", checked.stderr) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/test_companion_usb_logging_reply.py b/test/test_companion_usb_logging_reply.py index 859607611..195878feb 100644 --- a/test/test_companion_usb_logging_reply.py +++ b/test/test_companion_usb_logging_reply.py @@ -196,12 +196,22 @@ int main() { reset(); stream.capacity=0; command("set usb.logging on"); cancelUsbSerialOperations(); usb_serial_interface.resetSessionState(); stream.capacity=4096; serviceUsbLoggingReplyBarrier(); assert(!logging); - // A queue unable to accept the acknowledgement cannot start the handoff. + // A full reply queue leaves the logging command unread until its + // acknowledgement can fit; draining then permits the normal handoff. reset(); stream.capacity=0; const uint8_t required[]={29,1}; for(unsigned i=0;i<4;++i) assert(usb_serial_interface.writeFrame(required,2)==2); - command("set usb.logging on"); + stream.receive("set usb.logging on"); + assert(interface_manager.checkRecvFrame(the_mesh.cmd_frame)==0); + assert(stream.cursor==0 && stream.available()>0); assert(!usb_logging_reply_pending && !logging && ble.tx.empty()); + stream.capacity=4096; + size_t resumed=interface_manager.checkRecvFrame(the_mesh.cmd_frame); + assert(resumed>0 && interface_manager.captureReplyRoute()==&usb_serial_interface); + the_mesh.handleCmdFrame(resumed); + assert(usb_logging_reply_pending && !logging && !the_mesh.terminal); + serviceUsbLoggingReplyBarrier(); + assert(logging && the_mesh.terminal && !usb_logging_reply_pending); // A later explicit setting supersedes an earlier pending on request. reset(); stream.capacity=0; command("set usb.logging on"); the_mesh.applyUsbLoggingState(false); From 6a90229d9a374293e0f5aae80dd03348c283fc39 Mon Sep 17 00:00:00 2001 From: mikecarper Date: Fri, 16 Oct 2026 00:54:49 -0700 Subject: [PATCH 12/14] Preserve folder OTA staging when resume reads fail --- .github/workflows/run-unit-tests.yml | 1 + src/helpers/ota/FolderMotaStore.cpp | 6 +- src/helpers/ota/FolderMotaStore.h | 2 + src/helpers/ota/OtaCli.cpp | 4 +- src/helpers/ota/OtaManager.cpp | 42 +++- src/helpers/ota/OtaStore.h | 6 + test/fixtures/ota_folder_resume.cpp | 338 +++++++++++++++++++++++++++ test/test_ota_folder_resume.py | 71 ++++++ 8 files changed, 460 insertions(+), 10 deletions(-) create mode 100644 test/fixtures/ota_folder_resume.cpp create mode 100644 test/test_ota_folder_resume.py diff --git a/.github/workflows/run-unit-tests.yml b/.github/workflows/run-unit-tests.yml index 14ab4ddda..14f1fafcd 100644 --- a/.github/workflows/run-unit-tests.yml +++ b/.github/workflows/run-unit-tests.yml @@ -243,6 +243,7 @@ jobs: run: | python3 -B test/test_mota_serial_deadline.py -v python3 -B test/test_mota_folder_deadline.py -v + python3 -B test/test_ota_folder_resume.py -v python3 -B test/test_ota_direct_manager.py -v python3 -B test/test_mota_kiss_direct.py -v python3 -B test/test_ota_kiss_direct.py -v diff --git a/src/helpers/ota/FolderMotaStore.cpp b/src/helpers/ota/FolderMotaStore.cpp index 92bf8cd8b..d4b3ab99a 100644 --- a/src/helpers/ota/FolderMotaStore.cpp +++ b/src/helpers/ota/FolderMotaStore.cpp @@ -114,8 +114,12 @@ bool FolderMotaStore::finalize() { } bool FolderMotaStore::reopen() { + _reopen_io_error = false; uint8_t pl[5]; - if (!txn(MS_OP_STAT, _mid, 4, pl, 5)) return false; + if (!txn(MS_OP_STAT, _mid, 4, pl, 5)) { + _reopen_io_error = true; + return false; + } if (pl[0] == 0) return false; // host has no partial/complete file -> start fresh uint32_t total = rd_u32le(pl + 1); if (total < 13) return false; // implausible container diff --git a/src/helpers/ota/FolderMotaStore.h b/src/helpers/ota/FolderMotaStore.h index eaf00d821..e1cc3c066 100644 --- a/src/helpers/ota/FolderMotaStore.h +++ b/src/helpers/ota/FolderMotaStore.h @@ -40,6 +40,7 @@ public: void clear() override { _total = 0; } bool finalize() override; bool reopen() override; // OP_STAT: adopt an existing file for resume + bool reopenHadIoError() const override { return _reopen_io_error; } bool canReconnect() const override { return true; } // A host-backed store is fully random-access, so it needs no pinned-meta RAM page and no layout planning. @@ -58,6 +59,7 @@ private: MotaStreamWritePolicy _write_policy; uint8_t _mid[4] = {0, 0, 0, 0}; uint32_t _total = 0; + bool _reopen_io_error = false; }; } // namespace ota diff --git a/src/helpers/ota/OtaCli.cpp b/src/helpers/ota/OtaCli.cpp index 2a41316d0..147fa9e1c 100644 --- a/src/helpers/ota/OtaCli.cpp +++ b/src/helpers/ota/OtaCli.cpp @@ -740,7 +740,9 @@ static __attribute__((noinline)) bool handle_control_command( return true; } snprintf(reply, 160, "OK %s mid=%s -> %s (primary traffic)", - result == OtaManager::PULL_RESUMED ? "resuming" : "pulling", midhx, dname); + c.manager.fetchState() == OtaManager::PAUSED + ? "paused (folder I/O error; reconnect to resume)" + : result == OtaManager::PULL_RESUMED ? "resuming" : "pulling", midhx, dname); // ---- discard the current session (e.g. a stalled old fetch) to free the slot ---- } else if (is_cmd(a, "drop|cancel|stop", &rest)) { diff --git a/src/helpers/ota/OtaManager.cpp b/src/helpers/ota/OtaManager.cpp index 21d780b23..de836c6a2 100644 --- a/src/helpers/ota/OtaManager.cpp +++ b/src/helpers/ota/OtaManager.cpp @@ -1572,10 +1572,21 @@ OtaManager::PullResult OtaManager::startFetch(const uint8_t* mid, uint32_t targe // A validate pull is a FRESH seed capture, not a resume: the store already holds the seed's payload (not a // real partial), so never adopt it via resumeStaged - always re-begin and run the manifest->leaves->diff. const bool automatic = !_have_desired_mid && _desired_target == 0 && !_archive_fetch; - if (!validate && resumeStagedFor(mid, automatic)) return PULL_RESUMED; // adopt a partial container left in the store memcpy(_fid, mid, 4); _have = 0; _fbc = 0; _ftotal = 0; _fflags = 0; _observed_path_transmissions = 0; // learn the actual source/relay path from this fetch's replies + if (!validate) { + if (resumeStagedFor(mid, automatic)) return PULL_RESUMED; + if (_fetch_error == FETCH_ERROR_STORAGE) { + // A failed host read cannot authorize BEGIN to erase a valid partial. + // IDLE records that adoption itself must be retried on reconnect; no + // fresh-container initialization has been authorized yet. + _fstate = PAUSED; + _paused_from = IDLE; + clearReassembly(); + return PULL_STARTED; + } + } _fstate = WANT_MANIFEST; _mf_total = 0; _mf_mask = 0; _mf_len = 0; _mf_retries = 0; _loop_last_mfmask = 0; // fresh manifest reassembly memset(_mf_buf, 0, sizeof(_mf_buf)); @@ -1616,7 +1627,14 @@ bool OtaManager::pauseFetchForDisconnect() { bool OtaManager::resumeFetchAfterReconnect() { if (_fstate != PAUSED || !_fetch) return false; - if (_paused_from == WANT_MANIFEST || _paused_from == WANT_LEAVES) { + if (_paused_from == IDLE) { + // The first resume probe lost its link before it could establish whether + // staging was usable. Retry it before allowing a fresh initialization. + uint8_t mid[4]; memcpy(mid, _fid, sizeof(mid)); + _fstate = IDLE; + PullResult result = startFetch(mid, _fexpected_target, _validate); + if ((result != PULL_STARTED && result != PULL_RESUMED) || _fstate == PAUSED) return false; + } else if (_paused_from == WANT_MANIFEST || _paused_from == WANT_LEAVES) { // begin/header/manifest/trailer may only have partly reached the host. // Re-request the manifest and run the full initialization transaction. // Leaf-diff also restarts initialization: the reconnected host may have @@ -1859,15 +1877,23 @@ bool OtaManager::resumeStagedFor(const uint8_t* want_mid, bool automatic_resume) const uint32_t expected_target = automatic_resume ? (_migration_target ? _migration_target : _target) : _fexpected_target; - if (!_fetch->reopenFor(want_mid, expected_target)) return false; + auto readFailure = [&]() { + if (_fetch->canReconnect()) _fetch_error = FETCH_ERROR_STORAGE; + return false; + }; + if (!_fetch->reopenFor(want_mid, expected_target)) { + return _fetch->reopenHadIoError() ? readFailure() : false; + } uint32_t total = _fetch->staged_size(); uint8_t hdr[8]; - if (total < 8u + MOTA_MFL + 5u || !_fetch->read(0, hdr, sizeof(hdr)) - || memcmp(hdr, MOTA_MAGIC, 4) != 0 || rd_u32le(hdr + 4) != total) return false; + if (total < 8u + MOTA_MFL + 5u) return false; + if (!_fetch->read(0, hdr, sizeof(hdr))) return readFailure(); + if (memcmp(hdr, MOTA_MAGIC, 4) != 0 || rd_u32le(hdr + 4) != total) return false; // read + parse the stored manifest (everything before leaves[]) to recompute the geometry uint8_t mbuf[MOTA_MFL]; MotaManifest m; - if (!_fetch->read(8, mbuf, sizeof(mbuf)) || !mota_parse_manifest(mbuf, sizeof(mbuf), m)) return false; + if (!_fetch->read(8, mbuf, sizeof(mbuf))) return readFailure(); + if (!mota_parse_manifest(mbuf, sizeof(mbuf), m)) return false; if (want_mid && memcmp(m.merkle_root, want_mid, 4) != 0) return false; // a different fw is staged // A boot-time resume is an automatic fetch decision and therefore belongs // only to this node. An explicit pull retains target=0 as an intentional @@ -1896,8 +1922,8 @@ bool OtaManager::resumeStagedFor(const uint8_t* want_mid, bool automatic_resume) uint32_t payload_off = leaves_off + bc * 4; if ((uint64_t)payload_off + m.payload_size + 5 != total) return false; // geometry must match the header uint8_t trailer[5]; - if (!_fetch->read(total - sizeof(trailer), trailer, sizeof(trailer)) - || memcmp(trailer, MOTA_TRAILER, sizeof(trailer)) != 0) return false; + if (!_fetch->read(total - sizeof(trailer), trailer, sizeof(trailer))) return readFailure(); + if (memcmp(trailer, MOTA_TRAILER, sizeof(trailer)) != 0) return false; _wire_v2_session = _wire_v2_enabled; _wire_v2_confirmed = false; diff --git a/src/helpers/ota/OtaStore.h b/src/helpers/ota/OtaStore.h index de0cfe926..7013a1b60 100644 --- a/src/helpers/ota/OtaStore.h +++ b/src/helpers/ota/OtaStore.h @@ -70,6 +70,12 @@ public: (void)want_mid; (void)expected_target; return reopen(); } + // A failed reopen normally means no usable container, so a new pull may + // begin by erasing it. Reconnectable stores must distinguish an I/O failure: + // an unavailable host has not established that its staged bytes are invalid. + // Query only after reopen()/reopenFor() returned false. + virtual bool reopenHadIoError() const { return false; } + // External host stores can disappear temporarily while retaining their file. // Preserve the selected fetch on an I/O failure until that link reconnects. virtual bool canReconnect() const { return false; } diff --git a/test/fixtures/ota_folder_resume.cpp b/test/fixtures/ota_folder_resume.cpp new file mode 100644 index 000000000..c0ba3ed9f --- /dev/null +++ b/test/fixtures/ota_folder_resume.cpp @@ -0,0 +1,338 @@ +#include +#include +#include +#include +#include +#include "test_ota/mota_vectors.h" +#include +#include +#include +#include + +using namespace mesh::ota; + +namespace mesh { +namespace ota { +struct OtaContext { + OtaManager manager; + OtaStoreRam<32768> fetch_store; + FolderMotaStore* folder_dest = nullptr; + const char* folder_dest_info = "test host"; + bool apply_pending = false, fetch_to_folder = false; +}; + +// @CLI_HELPERS@ + +static bool pull_command(const char* a, char* reply, OtaContext& c) { + const char* rest = nullptr; + if (false) { +// @CLI_PULL@ + } else { + return false; + } + return true; +} +} // namespace ota +} // namespace mesh + +struct HostStream : Stream { + enum Failure { None, Timeout, Truncated, BadChecksum, Error, ShortWrite, WrongOp }; + std::vector bytes{SIM_MOTA_1K, SIM_MOTA_1K + SIM_MOTA_1K_LEN}; + std::vector response; + std::vector operations; + size_t position = 0; + bool present = true; + uint8_t fail_op = MS_OP_SREAD; + uint32_t fail_offset = 0; + Failure failure = None; + unsigned failures_left = 0, begins = 0, writes = 0; + + int available() override { return response.size() - position; } + int read() override { return position < response.size() ? response[position++] : -1; } + int peek() override { return -1; } + void flush() override { assert(false); } + size_t write(uint8_t) override { assert(false); return 0; } + size_t write(const uint8_t* request, size_t length) override { + assert(length >= 4 && request[0] == MOTA_SEEDER_REQ_MAGIC0 + && request[1] == MOTA_SEEDER_REQ_MAGIC1); + uint8_t checksum = 0; + for (size_t i = 2; i < length - 1; ++i) checksum ^= request[i]; + assert(checksum == request[length - 1]); + const uint8_t op = request[2]; + operations.push_back(op); + const bool fail = failures_left && op == fail_op + && (op != MS_OP_SREAD || rd_u32le(request + 7) == fail_offset); + if (fail) --failures_left; + response.clear(); + position = 0; + if (fail && failure == ShortWrite) return length - 1; + if (fail && failure == Timeout) return length; + response = {MOTA_SEEDER_RSP_MAGIC0, MOTA_SEEDER_RSP_MAGIC1, + uint8_t(fail && failure == WrongOp ? op + 1 : op), + uint8_t(fail && failure == Error ? MS_STATUS_ERR : MS_STATUS_OK)}; + if (!(fail && failure == Error)) { + if (op == MS_OP_STAT) { + response.push_back(present ? 1 : 0); + uint8_t size[4]; wr_u32le(size, bytes.size()); + response.insert(response.end(), size, size + sizeof(size)); + } else if (op == MS_OP_SREAD) { + const uint32_t offset = rd_u32le(request + 7); + const uint16_t count = request[11] | (uint16_t(request[12]) << 8); + assert(present && offset <= bytes.size() && count <= bytes.size() - offset); + response.insert(response.end(), bytes.begin() + offset, bytes.begin() + offset + count); + } else if (op == MS_OP_BEGIN) { + ++begins; + present = true; + bytes.assign(rd_u32le(request + 7), 0xFF); + } else if (op == MS_OP_WRITE) { + ++writes; + const uint32_t offset = rd_u32le(request + 7); + const uint16_t count = request[11] | (uint16_t(request[12]) << 8); + assert(present && offset <= bytes.size() && count <= bytes.size() - offset); + std::copy(request + 13, request + 13 + count, bytes.begin() + offset); + } else { + assert(op == MS_OP_FIN); + } + } + checksum = 0; + for (uint8_t value : response) checksum ^= value; + response.push_back(fail && failure == BadChecksum ? uint8_t(checksum ^ 1) : checksum); + if (fail && failure == Truncated) response.resize(4); + return length; + } +}; + +struct Flight { + unsigned manifests = 0; + std::vector requested_blocks; + static bool send(void* context, const uint8_t* bytes, uint16_t length, bool) { + auto& flight = *static_cast(context); + GetManifestMsg manifest{}; + if (decode_get_manifest(bytes, length, manifest)) ++flight.manifests; + ReqWindowMsg request{}; + if (decode_req_window(bytes, length, request)) { + for (uint8_t i = 0; i < request.n_items; ++i) + flight.requested_blocks.push_back(request.items[i].block_idx); + } + return true; + } +}; + +struct Fixture { + HostStream host; + FolderMotaStore store{host, MotaStreamWritePolicy::NoFlush, 4}; + OtaContext context; + Flight flight; + MotaManifest manifest; + + Fixture() { + assert(mota_parse(SIM_MOTA_1K, SIM_MOTA_1K_LEN, manifest)); + context.folder_dest = &store; + context.manager.begin(manifest.target_id, Flight::send, &flight); + context.manager.set_wire_v2_enabled(false); + uint8_t row[OTA_HAVE_ROW_BYTES] = {}; + memcpy(row, manifest.merkle_root, 4); + wr_u32le(row + 4, manifest.target_id); + wr_u32le(row + 8, manifest.fw_version); + row[12] = manifest.codec_id; + row[13] = manifest.flags; + HaveMsg have{}; + have.seeder_id[0] = 1; + have.frag_total = have.n_rows = 1; + have.rows = row; + uint8_t wire[256]; + context.manager.on_message(wire, encode_have(wire, sizeof(wire), have)); + assert(context.manager.catalogCount() == 1); + } + + void pull(bool validate = false) { + char reply[160]; + assert(pull_command(validate ? "pull 1 folder validate" : "pull 1 folder", reply, context)); + assert(strncmp(reply, "OK ", 3) == 0); + if (context.manager.fetchState() == OtaManager::PAUSED) { + assert(strstr(reply, "OK paused (folder I/O error; reconnect to resume)") == reply); + assert(context.fetch_to_folder); + } + } + + void deliver_manifest() { + for (uint32_t offset = 0; offset < MOTA_MFL; offset += OTA_MF_FRAG) { + ManifestMsg message{}; + memcpy(message.manifest_id, manifest.merkle_root, 4); + message.frag_idx = offset / OTA_MF_FRAG; + message.frag_total = (MOTA_MFL + OTA_MF_FRAG - 1) / OTA_MF_FRAG; + message.bytes = manifest.manifest_start + offset; + message.len = std::min(uint32_t(OTA_MF_FRAG), MOTA_MFL - offset); + uint8_t wire[256]; + context.manager.on_message(wire, encode_manifest(wire, sizeof(wire), message)); + } + } + + void finish_verification() { + for (unsigned guard = 0; guard < 100 && context.manager.fetchState() == OtaManager::VERIFYING_STAGED; ++guard) + context.manager.loop(); + assert(context.manager.fetchState() != OtaManager::VERIFYING_STAGED); + } + + void deliver_block(uint32_t block) { + const uint32_t block_size = manifest.block_size(); + const uint32_t length = std::min(block_size, manifest.payload_size - block * block_size); + uint8_t wire[256]; + for (uint32_t offset = 0; offset < length; offset += OTA_FRAG_DATA) { + DataMsg message{}; + memcpy(message.manifest_id, manifest.merkle_root, 4); + message.block_idx = block; + message.frag_off = offset; + message.data = manifest.payload + block * block_size + offset; + message.data_len = std::min(uint32_t(OTA_FRAG_DATA), length - offset); + assert(context.manager.on_message(wire, encode_data(wire, sizeof(wire), message))); + } + std::vector scratch(manifest.block_count * 4); + uint8_t siblings[128]; + ProofMsg proof{}; + memcpy(proof.manifest_id, manifest.merkle_root, 4); + proof.block_idx = block; + proof.n_proof = merkle_gen_proof(manifest.leaves, manifest.block_count, block, + scratch.data(), siblings); + proof.proof = siblings; + assert(context.manager.on_message(wire, encode_proof(wire, sizeof(wire), proof))); + } +}; + +static void transient_resume_failure_preserves_bytes(HostStream::Failure failure, + uint8_t op, uint32_t offset, + bool retry_by_pull) { + Fixture f; + f.host.failure = failure; + f.host.fail_op = op; + f.host.fail_offset = offset; + f.host.failures_left = 2; + const auto saved = f.host.bytes; + f.pull(); + f.deliver_manifest(); // Even a late peer response cannot initialize a paused slot. + if (f.host.bytes != saved) + fputs("FAIL: resume I/O fallback issued BEGIN and erased staging\n", stderr); + assert(f.host.bytes == saved && f.host.begins == 0 && f.host.writes == 0); + assert(f.context.manager.fetchState() == OtaManager::PAUSED); + assert(f.context.manager.fetchError() == OtaManager::FETCH_ERROR_STORAGE); + assert(memcmp(f.context.manager.fetchManifestId(), f.manifest.merkle_root, 4) == 0); + assert(f.flight.manifests == 0 && f.host.begins == 0 && f.host.writes == 0); + f.context.manager.set_clock(1000000); + f.context.manager.loop(); + assert(f.host.bytes == saved && f.flight.manifests == 0); + char reply[160]; + assert(pull_command("pull 1 folder", reply, f.context)); + assert(strncmp(reply, "ERR OTA slot is paused", 22) == 0); + assert(!f.context.manager.resumeFetchAfterReconnect()); + assert(f.context.manager.fetchState() == OtaManager::PAUSED); + assert(f.host.bytes == saved && f.host.begins == 0 && f.flight.manifests == 0); + if (retry_by_pull) { + f.context.manager.reset_session(); // The manager action performed by explicit cancellation. + f.pull(); + } else { + assert(f.context.manager.resumeFetchAfterReconnect()); + } + assert(f.context.manager.fetchState() == OtaManager::VERIFYING_STAGED); + f.finish_verification(); + assert(f.context.manager.fetchState() == OtaManager::COMPLETE); + assert(f.context.manager.blocksHave() == f.manifest.block_count); + assert(f.context.manager.fetchError() == OtaManager::FETCH_ERROR_NONE); + assert(f.host.bytes == saved && f.host.begins == 0 && f.host.writes == 0); +} + +static void known_absent_or_invalid_staging_starts_fresh(unsigned corruption, + bool after_io_failure) { + Fixture f; + if (after_io_failure) { + f.host.failure = HostStream::Timeout; + f.host.fail_op = MS_OP_STAT; + f.host.failures_left = 1; + f.pull(); + assert(f.context.manager.fetchState() == OtaManager::PAUSED); + } + if (corruption == 0) f.host.present = false; + else if (corruption == 1) f.host.bytes[0] ^= 1; + else if (corruption == 2) wr_u32le(f.host.bytes.data() + 4, f.host.bytes.size() + 1); + else if (corruption == 3) f.host.bytes[8] = 0xFF; // Invalid manifest format version. + else if (corruption == 4) f.host.bytes.back() ^= 1; + else if (corruption == 5) f.host.bytes.resize(12); + else if (corruption == 6) f.host.bytes[8 + 20] ^= 1; // Another MID, read successfully. + else wr_u32le(f.host.bytes.data() + 8 + 3, f.manifest.target_id ^ 1u); + if (after_io_failure) assert(f.context.manager.resumeFetchAfterReconnect()); + else f.pull(); + assert(f.context.manager.fetchState() == OtaManager::WANT_MANIFEST); + assert(f.context.manager.fetchError() == OtaManager::FETCH_ERROR_NONE); + assert(!f.store.reopenHadIoError()); + assert(f.flight.manifests == 1 && f.host.begins == 0); + f.deliver_manifest(); + assert(f.context.manager.fetchState() == OtaManager::FETCHING); + assert(f.host.begins == 1); +} + +static void explicit_validate_requests_fresh_initialization() { + Fixture f; + f.host.failure = HostStream::Timeout; + f.host.fail_op = MS_OP_STAT; + f.host.failures_left = 1; + f.pull(true); + assert(f.context.manager.fetchState() == OtaManager::WANT_MANIFEST); + assert(f.host.operations.empty() && f.flight.manifests == 1); + f.deliver_manifest(); + assert(f.host.begins == 1 && f.host.failures_left == 1); +} + +static void transient_probe_failure_resumes_only_missing_blocks() { + Fixture f; + const auto complete = f.host.bytes; + const uint32_t missing = 1; + const uint32_t leaf = f.manifest.leaves - SIM_MOTA_1K + missing * 4; + const uint32_t payload = f.manifest.payload - SIM_MOTA_1K + missing * f.manifest.block_size(); + std::fill(f.host.bytes.begin() + leaf, f.host.bytes.begin() + leaf + 4, 0xFF); + std::fill(f.host.bytes.begin() + payload, + f.host.bytes.begin() + payload + f.manifest.block_size(), 0xFF); + const auto partial = f.host.bytes; + f.host.failure = HostStream::Timeout; + f.host.fail_offset = 8; + f.host.failures_left = 1; + f.pull(); + assert(f.context.manager.fetchState() == OtaManager::PAUSED); + assert(f.context.manager.resumeFetchAfterReconnect()); + f.finish_verification(); + assert(f.context.manager.fetchState() == OtaManager::FETCHING); + assert(f.context.manager.blocksHave() == f.manifest.block_count - 1); + assert(f.host.bytes == partial && f.host.begins == 0 && f.host.writes == 0); + assert(!f.flight.requested_blocks.empty()); + for (uint32_t block : f.flight.requested_blocks) assert(block == missing); + f.deliver_block(missing); + assert(f.context.manager.fetchState() == OtaManager::COMPLETE); + assert(f.host.bytes == complete && f.host.begins == 0); +} + +static void established_capture_still_refuses_replacement_file() { + Fixture f; + f.pull(); + assert(f.context.manager.fetchState() == OtaManager::VERIFYING_STAGED); + assert(f.context.manager.pauseFetchForDisconnect()); + f.host.bytes[0] ^= 1; + assert(!f.context.manager.resumeFetchAfterReconnect()); + assert(f.context.manager.fetchState() == OtaManager::PAUSED); + assert(f.host.begins == 0 && f.flight.manifests == 0); +} + +int main() { + for (auto failure : {HostStream::Timeout, HostStream::Truncated, HostStream::BadChecksum, + HostStream::Error, HostStream::ShortWrite, HostStream::WrongOp}) { + for (bool retry_by_pull : {false, true}) { + transient_resume_failure_preserves_bytes(failure, MS_OP_STAT, 0, retry_by_pull); + for (uint32_t offset : {0u, 8u, SIM_MOTA_1K_LEN - 5u}) + transient_resume_failure_preserves_bytes(failure, MS_OP_SREAD, offset, retry_by_pull); + } + } + for (unsigned corruption = 0; corruption < 8; ++corruption) + for (bool after_io_failure : {false, true}) + known_absent_or_invalid_staging_starts_fresh(corruption, after_io_failure); + explicit_validate_requests_fresh_initialization(); + transient_probe_failure_resumes_only_missing_blocks(); + established_capture_still_refuses_replacement_file(); + puts("PASS: folder resume I/O failures preserve staging and recover safely"); +} diff --git a/test/test_ota_folder_resume.py b/test/test_ota_folder_resume.py new file mode 100644 index 000000000..ef4f2ab05 --- /dev/null +++ b/test/test_ota_folder_resume.py @@ -0,0 +1,71 @@ +#!/usr/bin/env python3 +"""Exercise actual folder store, manager, and CLI pull branch across host failures.""" +from pathlib import Path +import os +import shutil +import subprocess +import tempfile +import unittest + +from test_ota_cli_listing import function + +ROOT = Path(__file__).resolve().parents[1] + + +class OtaFolderResumeTest(unittest.TestCase): + def compile_run(self, manager_source=None, expect_rejection=False): + compiler = os.environ.get("CXX") or shutil.which("g++") or shutil.which("clang++") + self.assertIsNotNone(compiler) + cli = (ROOT / "src/helpers/ota/OtaCli.cpp").read_text() + start = cli.index(' } else if (is_cmd(a, "pull|get|download", &rest)) {') + end = cli.index(' // ---- discard the current session', start) + harness = (ROOT / "test/fixtures/ota_folder_resume.cpp").read_text() + helpers = "\n\n".join(function(cli, name) for name in ("is_cmd", "state_word")) + harness = harness.replace("// @CLI_HELPERS@", helpers).replace("// @CLI_PULL@", cli[start:end]) + with tempfile.TemporaryDirectory(prefix="meshcore-ota-folder-resume-") as temporary: + work = Path(temporary) + source = work / "resume.cpp" + source.write_text(harness) + binary = work / "resume" + sanitizers = (["-fsanitize=address,undefined", "-fno-sanitize-recover=all", + "-fno-pie", "-no-pie"] if os.name != "nt" else []) + manager = ROOT / "src/helpers/ota/OtaManager.cpp" + if manager_source is not None: + manager = work / "OtaManager.cpp" + manager.write_text(manager_source) + sources = ("FolderMotaStore.cpp", "OtaProtocol.cpp", + "MotaContainer.cpp", "MerkleTree.cpp") + built = subprocess.run([ + compiler, "-std=c++17", "-O1", "-Wall", "-Wextra", "-Werror", + "-Wno-unused-parameter", "-Wno-sign-compare", "-Wno-unused-variable", + "-DOTA_SEEDER_ONLY=1", *sanitizers, + "-I", str(ROOT / "src"), "-I", str(ROOT / "test/mocks"), + "-I", str(ROOT / "src/helpers/ota"), + "-I", str(ROOT / "test"), str(source), + str(manager), + *[str(ROOT / "src/helpers/ota" / name) for name in sources], + str(ROOT / "src/Utils.cpp"), "-o", str(binary), + ], capture_output=True, text=True, timeout=90) + self.assertEqual(built.returncode, 0, built.stdout + built.stderr) + checked = subprocess.run([str(binary)], capture_output=True, text=True, timeout=30) + if expect_rejection: + self.assertNotEqual(checked.returncode, 0, checked.stdout + checked.stderr) + self.assertIn("resume I/O fallback issued BEGIN and erased staging", checked.stderr) + self.assertIn("Assertion", checked.stderr) + else: + self.assertEqual(checked.returncode, 0, checked.stdout + checked.stderr) + self.assertIn("PASS: folder resume I/O failures preserve staging", checked.stdout) + + def test_failed_probe_preserves_staging_and_allows_later_resume(self): + self.compile_run() + + def test_reintroduced_fresh_fetch_after_io_failure_is_rejected(self): + source = (ROOT / "src/helpers/ota/OtaManager.cpp").read_text() + unsafe = source.replace(" if (_fetch_error == FETCH_ERROR_STORAGE) {", + " if (false) {", 1) + self.assertNotEqual(source, unsafe) + self.compile_run(unsafe, expect_rejection=True) + + +if __name__ == "__main__": + unittest.main() From 5b464e93a64e641e8429bc3221805b283852ba38 Mon Sep 17 00:00:00 2001 From: mikecarper Date: Fri, 16 Oct 2026 00:55:56 -0700 Subject: [PATCH 13/14] Fit headless Wio Companion while retaining button controls --- test/test_stm32_companion_profile.py | 30 +++++++++++++++++++++------- variants/wio-e5-mini/platformio.ini | 4 ++++ 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/test/test_stm32_companion_profile.py b/test/test_stm32_companion_profile.py index aa82cd8fb..c46ea7c3d 100644 --- a/test/test_stm32_companion_profile.py +++ b/test/test_stm32_companion_profile.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -"""Guard the headless RAK Companion's size recipe and deployed storage contract. +"""Guard headless STM32 Companion size recipes and deployed storage contracts. The stm32-companion-smoke job also links the actual image. These checks prevent fixing its size limit by silently shrinking storage or Companion capacities. @@ -15,11 +15,15 @@ ROOT = Path(__file__).resolve().parents[1] class STM32CompanionProfileTest(unittest.TestCase): + variant = "rak3x72" + target = "RAK_3x72_companion_radio_usb" + base = "rak3x72" + def setUp(self): self.config = configparser.ConfigParser( interpolation=None, inline_comment_prefixes=(";",)) - self.config.read(ROOT / "variants/rak3x72/platformio.ini") - self.companion = self.config["env:RAK_3x72_companion_radio_usb"] + self.config.read(ROOT / "variants" / self.variant / "platformio.ini") + self.companion = self.config["env:" + self.target] def test_unused_timer_wrappers_are_removed_only_from_headless_companion(self): for flag in ("RADIOLIB_TONE_UNSUPPORTED", "HAL_TIM_MODULE_ONLY"): @@ -30,10 +34,10 @@ class STM32CompanionProfileTest(unittest.TestCase): def test_flash_and_filesystem_boundaries_do_not_move(self): self.assertNotIn("board_upload.maximum_size", self.companion) - self.assertEqual(self.config["rak3x72"].getint("board_upload.maximum_size"), + self.assertEqual(self.config[self.base].getint("board_upload.maximum_size"), 229376) self.assertNotIn("LFS_FLASH_TOTAL_SIZE", self.companion["build_flags"]) - self.assertNotIn("LFS_FLASH_TOTAL_SIZE", self.config["rak3x72"]["build_flags"]) + self.assertNotIn("LFS_FLASH_TOTAL_SIZE", self.config[self.base]["build_flags"]) header = (ROOT / "src/helpers/stm32/InternalFileSystem.h").read_text() self.assertRegex(header, r"#define\s+LFS_FLASH_TOTAL_SIZE\s+\(16\s*\*\s*2048\)") self.assertEqual(229376 + 16 * 2048, 256 * 1024) @@ -43,13 +47,25 @@ class STM32CompanionProfileTest(unittest.TestCase): for definition in ("MAX_CONTACTS=100", "MAX_GROUP_CHANNELS=8", "ENABLE_USB_INTERFACE"): self.assertRegex(flags, r"-D\s+" + re.escape(definition) + r"\b") - for flags in (flags, self.config["rak3x72"]["build_flags"]): + for flags in (flags, self.config[self.base]["build_flags"]): self.assertNotRegex(flags, r"COMPANION_FEATURE_\w+=0") self.assertNotRegex(flags, r"MESH_ENABLE_\w+=0") self.assertIn("+<../examples/companion_radio/*.cpp>", self.companion["build_src_filter"]) self.assertIn("-Oz", self.companion["build_flags"]) - self.assertEqual(self.companion["build_unflags"], "-Os") + self.assertEqual(self.companion["build_unflags"].split(), ["-Os"]) + + +class WioSTM32CompanionProfileTest(STM32CompanionProfileTest): + variant = "wio-e5-mini" + target = "wio-e5-mini_companion_radio_usb" + base = "lora_e5_mini" + + def test_headless_button_controls_are_retained(self): + self.assertRegex(self.companion["build_flags"], + r"-D\s+WIO_E5_MINI_HEADLESS_COMPANION_UI=1\b") + self.assertRegex(self.config[self.base]["build_flags"], + r"-D\s+PIN_USER_BTN=USER_BTN\b") if __name__ == "__main__": diff --git a/variants/wio-e5-mini/platformio.ini b/variants/wio-e5-mini/platformio.ini index e2a9203f4..eab2af235 100644 --- a/variants/wio-e5-mini/platformio.ini +++ b/variants/wio-e5-mini/platformio.ini @@ -79,6 +79,10 @@ platform_packages = platformio/toolchain-gccarmnoneeabi@^1.140201.0 build_unflags = -Os build_flags = ${lora_e5_mini.build_flags} + ; Headless button/LED controls use GPIO and millis, not PWM/tone timers. + ; Keep the low-level TIM HAL while omitting unused Arduino timer wrappers. + -D RADIOLIB_TONE_UNSUPPORTED + -D HAL_TIM_MODULE_ONLY -D ED25519_COMPACT_SHA512=1 ; same hash rounds, compact loop preserves the filesystem boundary -D ED25519_COMPACT_SCALAR=1 ; same ref10 arithmetic with fixed loops instead of unrolled code -Oz From 9cf2417d295d0cb28f424e0b29d6b6f3ffaa7aab Mon Sep 17 00:00:00 2001 From: mikecarper Date: Fri, 16 Oct 2026 01:08:46 -0700 Subject: [PATCH 14/14] Use production packet routing types in OTA adapter regression --- test/test_reply_tx_integration.py | 86 +++++++++++++++++++++++++------ 1 file changed, 69 insertions(+), 17 deletions(-) diff --git a/test/test_reply_tx_integration.py b/test/test_reply_tx_integration.py index 932bbe910..576367b9e 100644 --- a/test/test_reply_tx_integration.py +++ b/test/test_reply_tx_integration.py @@ -246,17 +246,18 @@ int main() { #include #include #include -#include +#include +#include namespace mesh { -struct Packet { - bool radio_reply=false, radio_local=true; - uint8_t radio_profile=0, radio_origin=0; - uint32_t radio_generation=0, radio_origin_generation=0; -}; namespace ota { -struct Route { uint32_t generation=0; uint8_t profile=0xFF; }; -struct Context { struct Manager { Route replyRoute() const { return {}; } } manager; }; -Context* ota_context_if_active() { return nullptr; } +struct Context { + struct Manager { + OtaReplyRoute route; + OtaReplyRoute replyRoute() const { return route; } + } manager; +}; +static Context* active = nullptr; +Context* ota_context_if_active() { return active; } } struct Manager { int free=10; int getFreeCount() { return free; } }; static int queued; @@ -274,7 +275,9 @@ struct Mesh { ++allocations; // Deliberately opposite to expected for half the types: classification // must not accidentally depend on the receive call stack or a prior send. - packet.radio_reply=!packet.radio_reply; + const bool previous_reply=packet.radio_reply; + packet=Packet(); + packet.radio_reply=!previous_reply; if (!allocation || !manager.free) return nullptr; --manager.free; return &packet; @@ -326,6 +329,32 @@ int main() { assert(!node.otaSendAdapter(&node,&data,1,true)); assert(node.sends==sends); node.allocation=true; node.admit=false; assert(!node.otaSendAdapter(&node,&data,1,true)); + + // Use the actual Packet and deferred-route definitions. A routed reply + // retains its origin without binding normal reply fan-out. A negotiation + // fallback is not a reply and must stay bound to its original radio. + node.admit=true; node.mask=1; + mesh::ota::Context context; + mesh::ota::active=&context; + for (uint8_t profile : {0,1}) { + context.manager.route.generation=1234+profile; + context.manager.route.profile=profile; + for (uint8_t type : {mesh::ota::OTA_QUERY,mesh::ota::OTA_DATA}) { + assert(node.otaSendAdapter(&node,&type,1,false)); + const bool reply=type==mesh::ota::OTA_DATA; + assert(node.packet.radio_reply==reply && !node.packet.radio_local); + assert(node.packet.radio_profile==profile && node.packet.radio_origin==profile); + assert(node.packet.radio_generation==1234+profile); + assert(node.packet.radio_origin_generation==1234+profile); + assert(node.packet.radio_bound==!reply); + assert(node.packet.tx_radio==(reply ? mesh::RADIO_TX_AUTO + : profile==0 ? mesh::RADIO_TX_PRIMARY : mesh::RADIO_TX_SECONDARY)); + } + } + context.manager.route={}; + assert(node.otaSendAdapter(&node,&data,1,false)); + assert(node.packet.radio_local && !node.packet.radio_bound); + assert(node.packet.tx_radio==mesh::RADIO_TX_AUTO); } ''' with tempfile.TemporaryDirectory() as folder: @@ -334,13 +363,36 @@ int main() { header = (ROOT / 'src/Mesh.h').read_text() credit = re.search(r'#define OTA_EGRESS_QUEUE_CREDIT (\d+)', header).group(1) reserve = re.search(r'#define OTA_EGRESS_MIN_FREE (\d+)', header).group(1) - cpp.write_text(harness.replace('@ADAPTER@', adapter).replace('@PACED@', paced) - .replace('@CREDIT@', credit).replace('@RESERVE@', reserve)) - result = subprocess.run([os.environ.get('CXX', 'g++'), '-std=c++17', - '-Wall', '-Wextra', '-I', str(ROOT / 'src'), str(cpp), '-o', str(exe)], - capture_output=True, text=True) - self.assertEqual(result.returncode, 0, result.stderr) - subprocess.run([str(exe)], check=True) + template = harness.replace('@PACED@', paced).replace('@CREDIT@', credit) + template = template.replace('@RESERVE@', reserve) + sanitizers = ([] if os.name == 'nt' else ['-fsanitize=address,undefined', + '-fno-sanitize-recover=all', '-fno-pie', '-no-pie']) + command = [os.environ.get('CXX', 'g++'), '-std=c++17', + '-Wall', '-Wextra', *sanitizers, '-I', str(ROOT / 'src'), + '-I', str(ROOT / 'test/mocks'), str(cpp), str(ROOT / 'src/Packet.cpp'), + '-o', str(exe)] + variants = [('production', adapter, None)] + for old, new, failure in ( + ('p->radio_bound = true;', 'p->radio_bound = false;', + 'node.packet.radio_bound==!reply'), + ('queued + copies > OTA_EGRESS_QUEUE_CREDIT', 'false', + 'node.otaSendAdapter(&node,&type,1,true)==allowed'), + ): + self.assertEqual(adapter.count(old), 1) + variants.append((old, adapter.replace(old, new), failure)) + for label, candidate, failure in variants: + with self.subTest(adapter=label): + cpp.write_text(template.replace('@ADAPTER@', candidate), encoding='ascii') + result = subprocess.run(command, capture_output=True, text=True, timeout=60) + self.assertEqual(result.returncode, 0, result.stderr) + checked = subprocess.run([str(exe)], capture_output=True, text=True, timeout=15) + if failure is None: + self.assertEqual(checked.returncode, 0, checked.stdout + checked.stderr) + else: + self.assertNotEqual(checked.returncode, 0, + 'negative control unexpectedly passed') + self.assertIn('Assertion', checked.stderr) + self.assertIn(failure, checked.stderr) def test_all_infrastructure_roles_enable_replies_and_mark_delayed_completions(self): common = (ROOT / 'src/helpers/CommonCLI.cpp').read_text()