diff --git a/docs/lora_ota_automation.md b/docs/lora_ota_automation.md index a6a03dbd3..7050d36c6 100644 --- a/docs/lora_ota_automation.md +++ b/docs/lora_ota_automation.md @@ -479,6 +479,13 @@ The lab test default is 909.5 MHz, 500 kHz bandwidth, SF5, and CR5. The frequenc North American example: choose a legal frequency supported by every participating radio and appropriate to your location. Older radios that do not support SF5 require a complete replacement tuple passed with `--temp-radio`. +The fifth value is the TempRadio window in minutes. To change only that value, +pass `--temp-radio-minutes 90`; this works with `--relay` and keeps the chosen +frequency, bandwidth, SF, and CR. The hard minimum is 20 minutes. The 120-minute +default and the runner's worst-case timeout estimate are guidance. A window +below the estimate prints a warning and continues; the radio lease can expire +before a slow OTA finishes. +The separate three-minute path rehearsal and its identity checks still run. Before that long window is allowed, the live runner performs a mandatory independent three-minute rehearsal. Its exact normal-channel `ota status` and diff --git a/tools/lora_ota/lora_ota.py b/tools/lora_ota/lora_ota.py index 256d4c530..dc21e1956 100755 --- a/tools/lora_ota/lora_ota.py +++ b/tools/lora_ota/lora_ota.py @@ -65,8 +65,9 @@ LEGACY_TARGET_MAX_BLOCK_SIZE = 1024 MOTA_MAX_BLOCK_SIZE = 2048 # Lab/HIL OTA tests use one explicit tuple on every participating radio. # This is a bench default, not a regional or production firmware preset. -# Override --temp-radio only when the hardware or local rules require it. +# Use --temp-radio for the tuple and --temp-radio-minutes for only its window. DEFAULT_LAB_TEMP_RADIO = "909.5,500,5,5,120" +MIN_TEMP_RADIO_WINDOW_MINUTES = 20 TRANSMISSION_RETRY_LIMIT = 3 TRANSMISSION_RETRY_WINDOW_SECONDS = 90 TRANSMISSION_RETRY_DELAY_SECONDS = 2 @@ -2951,8 +2952,12 @@ def parse_temp_radio(value: str) -> tuple[float, float, int, int, int]: 7.8, 10.4, 15.6, 20.8, 31.25, 41.7, 62.5, 125.0, 250.0, 500.0, ) bandwidth_valid = any(abs(bandwidth - allowed) <= 0.001 for allowed in valid_bandwidths) - if not 150 <= freq <= 2500 or not 5 <= sf <= 12 or not 5 <= cr <= 8 or minutes <= 0: + if not 150 <= freq <= 2500 or not 5 <= sf <= 12 or not 5 <= cr <= 8 or not 1 <= minutes <= 10080: raise argparse.ArgumentTypeError("invalid TempRadio range") + if minutes < MIN_TEMP_RADIO_WINDOW_MINUTES: + raise argparse.ArgumentTypeError( + f"TempRadio window must be at least {MIN_TEMP_RADIO_WINDOW_MINUTES} minutes" + ) if not bandwidth_valid: raise argparse.ArgumentTypeError( "bandwidth must be one of 7.8,10.4,15.6,20.8,31.25,41.7,62.5,125,250,500" @@ -2960,6 +2965,55 @@ def parse_temp_radio(value: str) -> tuple[float, float, int, int, int]: return freq, bandwidth, sf, cr, minutes +def set_temp_radio_minutes(args: argparse.Namespace) -> None: + """Override only the lease length while retaining the selected radio tuple.""" + minutes = getattr(args, "temp_radio_minutes", None) + if minutes is not None: + if minutes < MIN_TEMP_RADIO_WINDOW_MINUTES or minutes > 10080: + raise argparse.ArgumentTypeError( + f"TempRadio window must be at least {MIN_TEMP_RADIO_WINDOW_MINUTES} minutes (maximum 10080)" + ) + args.temp_radio = args.temp_radio.rsplit(",", 1)[0] + f",{minutes}" + + +def recommended_temp_radio_minutes( + args: argparse.Namespace, *, include_install: bool = True, +) -> int: + """Worst-case timeout budget, offered as guidance rather than a lease minimum.""" + relays = len(args.relay) + remote_setup_seconds = (1 + relays) * args.reply_timeout + source_setup_seconds = ( + 0 if args.source_already_temp or args.source_shares_controller else 30 + ) + final_reply_count = (4 + relays) if include_install else 1 + required_seconds = ( + remote_setup_seconds + + source_setup_seconds + + TEMP_RADIO_SWITCH_DELAY_SECONDS + + args.seeder_start_wait + + args.discovery_timeout + + args.transfer_timeout_minutes * 60 + + adaptive_poll_ceiling(args.poll_seconds) + + args.reply_timeout * final_reply_count + + relays * RELAY_TIMING_COMMANDS_PER_RELAY * args.reply_timeout + ) + return required_seconds // 60 + 1 + + +def warn_short_temp_radio_window( + args: argparse.Namespace, *, include_install: bool = True, +) -> None: + recommended = recommended_temp_radio_minutes(args, include_install=include_install) + selected = args.temp_values[4] + if selected < recommended: + print( + f"warning: TempRadio window is {selected} minutes; {recommended} minutes " + "covers the combined worst-case setup, discovery, transfer, and " + "final checks. Continuing; the lease may expire before OTA completes.", + file=sys.stderr, + ) + + 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", "") @@ -5339,10 +5393,13 @@ def parse_source_temp_radio_status( text, re.IGNORECASE, ) + # Newer firmware adds the wire preamble and optional semicolon-delimited + # timing diagnostics; older firmware ends immediately after the tuple. active_matches = re.findall( prefix + r"TempRadio (active|pending):\s*" r"([0-9]+(?:\.[0-9]+)?),([0-9]+(?:\.[0-9]+)?)," - r"(\d+),(\d+)(?:\s+\d+s left)?" + suffix, + r"(\d+),(\d+)(?:\s+\d+s left)?" + r"(?:,preamble=\d+)?(?:;[^\n;]+)*" + suffix, text, re.IGNORECASE, ) @@ -7162,7 +7219,11 @@ def build_parser() -> argparse.ArgumentParser: ) parser.add_argument( "--temp-radio", default=DEFAULT_LAB_TEMP_RADIO, - help="frequency,bw,sf,cr,minutes", + help="frequency,bw,sf,cr,window-minutes (minimum 20m; default 120m)", + ) + parser.add_argument( + "--temp-radio-minutes", type=int, metavar="MINUTES", + help="change only the TempRadio window (minimum 20m); keeps frequency,bw,sf,cr", ) parser.add_argument( "--base", type=Path, @@ -7320,6 +7381,7 @@ def serial_paths_match(first: str, second: str) -> bool: def validate_args(args: argparse.Namespace, parser: argparse.ArgumentParser) -> None: try: + set_temp_radio_minutes(args) args.temp_values = parse_temp_radio(args.temp_radio) except argparse.ArgumentTypeError as exc: parser.error(f"--temp-radio: {exc}") @@ -7448,35 +7510,7 @@ def validate_args(args: argparse.Namespace, parser: argparse.ArgumentParser) -> if getattr(args, name) <= 0: parser.error(f"--{name.replace('_', '-')} must be positive") if not args.prepare_only: - remote_setup_seconds = (1 + len(args.relay)) * args.reply_timeout - source_setup_seconds = ( - 0 if args.source_already_temp or args.source_shares_controller else 30 - ) - final_reply_count = 1 if args.no_install else 4 + len(args.relay) - relay_timing_seconds = ( - len(args.relay) - * RELAY_TIMING_COMMANDS_PER_RELAY - * args.reply_timeout - ) - required_temp_seconds = ( - remote_setup_seconds - + source_setup_seconds - + TEMP_RADIO_SWITCH_DELAY_SECONDS - + args.seeder_start_wait - + args.discovery_timeout - + args.transfer_timeout_minutes * 60 - + adaptive_poll_ceiling(args.poll_seconds) - + args.reply_timeout * final_reply_count - + relay_timing_seconds - ) - temp_seconds = args.temp_values[4] * 60 - if required_temp_seconds >= temp_seconds: - minimum_minutes = required_temp_seconds // 60 + 1 - parser.error( - f"TempRadio must last at least {minimum_minutes} minutes for " - "remote setup, seeder startup, discovery, transfer, and one " - "final poll plus install checks" - ) + warn_short_temp_radio_window(args, include_install=not args.no_install) def require_command(command: str, label: str) -> None: diff --git a/tools/lora_ota/rak3401_mota_chain.py b/tools/lora_ota/rak3401_mota_chain.py index b69c7401c..98545ddd5 100755 --- a/tools/lora_ota/rak3401_mota_chain.py +++ b/tools/lora_ota/rak3401_mota_chain.py @@ -2472,7 +2472,14 @@ def build_parser() -> argparse.ArgumentParser: default=ota.DEFAULT_RELAY_TX_DELAY, help="temporary flood txdelay for managed intermediate relays", ) - parser.add_argument("--temp-radio", default=ota.DEFAULT_LAB_TEMP_RADIO) + parser.add_argument( + "--temp-radio", default=ota.DEFAULT_LAB_TEMP_RADIO, + help="frequency,bw,sf,cr,window-minutes (minimum 20m; default 120m)", + ) + parser.add_argument( + "--temp-radio-minutes", type=int, metavar="MINUTES", + help="change only the TempRadio window (minimum 20m); keeps frequency,bw,sf,cr", + ) parser.add_argument( "--ota-hops", type=int, @@ -2586,26 +2593,13 @@ def validate_args(args: argparse.Namespace, parser: argparse.ArgumentParser) -> if not 0 <= args.ota_hops <= 8: parser.error("--ota-hops must be from 0 through 8") try: + ota.set_temp_radio_minutes(args) temp_values = ota.parse_temp_radio(args.temp_radio) except argparse.ArgumentTypeError as exc: parser.error(f"--temp-radio: {exc}") args.temp_values = temp_values - remote_setup_seconds = (1 + len(args.relay)) * args.reply_timeout - required_seconds = ( - remote_setup_seconds - + (0 if args.source_already_temp or args.source_shares_controller else 30) - + ota.TEMP_RADIO_SWITCH_DELAY_SECONDS - + args.seeder_start_wait - + args.discovery_timeout - + args.transfer_timeout_minutes * 60 - + ota.adaptive_poll_ceiling(args.poll_seconds) - + args.reply_timeout * (4 + len(args.relay)) - + len(args.relay) * ota.RELAY_TIMING_COMMANDS_PER_RELAY * args.reply_timeout - ) - if not args.verify_only and temp_values[4] * 60 <= required_seconds: - parser.error( - "TempRadio window is too short for the selected relay count and timeouts" - ) + if not (args.verify_only or args.preflight_only): + ota.warn_short_temp_radio_window(args) def confirm_chain( diff --git a/tools/lora_ota/test_lora_ota.py b/tools/lora_ota/test_lora_ota.py index d02678778..6c71da19d 100644 --- a/tools/lora_ota/test_lora_ota.py +++ b/tools/lora_ota/test_lora_ota.py @@ -359,6 +359,9 @@ class FormatTests(unittest.TestCase): ) with self.assertRaisesRegex(argparse.ArgumentTypeError, "bandwidth must be"): ota.parse_temp_radio("909.950,200,5,5,120") + self.assertEqual(ota.parse_temp_radio("909.950,250,5,5,20")[-1], 20) + with self.assertRaisesRegex(argparse.ArgumentTypeError, "at least 20 minutes"): + ota.parse_temp_radio("909.950,250,5,5,19") def test_ota_runners_use_the_same_lab_tuple(self) -> None: generic = ota.build_parser().parse_args(["release.mota", "remote"]) @@ -3847,6 +3850,11 @@ class TempRadioPreflightTests(unittest.TestCase): if source_active(): if failure == "temp_source_tuple": return "TempRadio active: 908.000,250.00,5,5 170s left" + if failure == "source_status_preamble": + return ( + " -> TempRadio active: 909.950,250.00,5,5 170s left" + ",preamble=128; timing self-test pending\r\n> " + ) return " -> TempRadio active: 909.950,250.00,5,5 170s left\r\n> " return "TempRadio inactive\r\n> " if command == "normalradio": @@ -3943,6 +3951,55 @@ class TempRadioPreflightTests(unittest.TestCase): "TempRadio active: 909.950,250.00,5,5 170s left\r\n> " ) + def test_source_temp_radio_status_with_optional_preamble(self) -> None: + expected = ota.RadioSettings(909.5, 500.0, 5, 5, False) + cases = ( + ("TempRadio active: 909.500,500.00,5,5 168s left", "active"), + ( + "TempRadio active: 909.500,500.00,5,5 168s left,preamble=128", + "active", + ), + ( + "TempRadio active: 909.500,500.00,5,5 168s left,preamble=128" + "; WARN recommended preamble: radio=128; switch=7000us", + "active", + ), + ("TempRadio pending: 909.500,500.00,5,5", "pending"), + ( + "TempRadio pending: 909.500,500.00,5,5,preamble=128" + "; timing self-test pending", + "pending", + ), + ) + for reply, state in cases: + with self.subTest(reply=reply): + self.assertEqual( + ota.parse_source_temp_radio_status(f" -> {reply}\r\n> "), + (state, expected), + ) + for reply in ( + "TempRadio active: 909.500,500.00,5,5 168s left,preamble=", + "TempRadio active: 909.500,500.00,5,5 168s left,preamble=fast", + "TempRadio active: 909.500,500.00,5,5 168s left,unknown=128", + ): + with self.subTest(reply=reply), self.assertRaisesRegex( + ota.OtaError, "invalid TempRadio status" + ): + ota.parse_source_temp_radio_status(reply) + + def test_rehearsal_accepts_source_status_with_preamble(self) -> None: + controller, source_calls, _clock, state, error = self.run_rehearsal( + failure="source_status_preamble", + capture_error=True, + ) + self.assertIsNone(error) + self.assertTrue(controller.all_nodes_normal()) + self.assertFalse(bool(state["active"])) + self.assertGreaterEqual( + sum(command == "tempradio" for command, _ in source_calls), + 2, + ) + def test_optional_source_command_never_hides_transport_or_silence(self) -> None: args = self.args() with mock.patch.object( @@ -5101,6 +5158,30 @@ class ReliabilityTests(unittest.TestCase): reply = controller._remote_command_once("remote", "ota status", "secret") self.assertTrue(reply.startswith("OTA |")) + def test_remote_relay_command_uses_bound_public_key(self) -> None: + controller = object.__new__(ota.Controller) + controller.reply_timeout = 20 + controller._authenticated_targets = set() + key = "d8" * 32 + observed: list[str] = [] + + def run_marked(commands: list[str], _label: str, _marker: str): + observed.extend(commands) + return ( + [{"adv_name": "Renamed relay", "public_key": key}, + {"login_success": True}], + [{"txt_type": 1, "text": "OK", "pubkey_prefix": key[:12]}], + ) + + controller._run_marked = run_marked + self.assertEqual( + controller._remote_command_once(key, "get rxdelay", "relay-password"), + "OK", + ) + self.assertEqual(observed[:2], ["contact_info", key]) + self.assertEqual(observed[2:5], ["login", key, "relay-password"]) + self.assertEqual(observed[8:11], ["cmd", key, "get rxdelay"]) + def test_matching_admin_reply_recovers_lost_login_acknowledgement(self) -> None: controller = object.__new__(ota.Controller) controller.reply_timeout = 20 @@ -5832,26 +5913,78 @@ class ReliabilityTests(unittest.TestCase): package, None, old_hash, ) - def test_temp_window_includes_setup_and_discovery_budget(self) -> None: + def test_temp_window_budget_is_advisory_with_a_relay(self) -> None: parser = ota.build_parser() default_args = parser.parse_args([ "release.mota", "remote", "--controller-serial", "/dev/controller", "--source-serial", "/dev/source", + "--relay", "a" * 64, ]) - ota.validate_args(default_args, parser) + warning = io.StringIO() + with contextlib.redirect_stderr(warning): + ota.validate_args(default_args, parser) + self.assertEqual(default_args.temp_values[4], 120) + self.assertIn("Continuing", warning.getvalue()) + self.assertIn("minutes covers the combined worst-case", warning.getvalue()) + self.assertGreater(ota.recommended_temp_radio_minutes(default_args), 120) args = parser.parse_args([ "release.mota", "remote", "--controller-serial", "/dev/controller", "--source-serial", "/dev/source", - "--temp-radio", "909.950,250,5,5,114", + "--temp-radio", "909.950,250,5,5,120", + "--temp-radio-minutes", "30", ]) - with ( - contextlib.redirect_stderr(io.StringIO()), - self.assertRaises(SystemExit), - ): + with contextlib.redirect_stderr(io.StringIO()): ota.validate_args(args, parser) + self.assertEqual(args.temp_radio, "909.950,250,5,5,30") + self.assertEqual(args.temp_values, (909.95, 250.0, 5, 5, 30)) + + invalid = parser.parse_args([ + "release.mota", "remote", "--controller-serial", "/dev/controller", + "--source-serial", "/dev/source", "--temp-radio-minutes", "0", + ]) + with contextlib.redirect_stderr(io.StringIO()), self.assertRaises(SystemExit): + ota.validate_args(invalid, parser) + + for option in ( + ["--temp-radio-minutes", "19"], + ["--temp-radio", "909.5,500,5,5,19"], + ): + below_minimum = parser.parse_args([ + "release.mota", "remote", "--controller-serial", "/dev/controller", + "--source-serial", "/dev/source", *option, + ]) + error = io.StringIO() + with contextlib.redirect_stderr(error), self.assertRaises(SystemExit): + ota.validate_args(below_minimum, parser) + self.assertIn("20 minutes", error.getvalue()) + + def test_chain_temp_window_can_override_only_minutes(self) -> None: + parser = rak_chain.build_parser() + args = parser.parse_args([ + "--source-serial", "/dev/source", "--relay", "a" * 64, + "--temp-radio-minutes", "30", + ]) + warning = io.StringIO() + with contextlib.redirect_stderr(warning): + rak_chain.validate_args(args, parser) + self.assertEqual(args.temp_radio, "909.5,500,5,5,30") + self.assertEqual(args.temp_values[4], 30) + self.assertIn("Continuing", warning.getvalue()) + + for option in ( + ["--temp-radio-minutes", "19"], + ["--temp-radio", "909.5,500,5,5,19"], + ): + below_minimum = parser.parse_args([ + "--source-serial", "/dev/source", *option, + ]) + error = io.StringIO() + with contextlib.redirect_stderr(error), self.assertRaises(SystemExit): + rak_chain.validate_args(below_minimum, parser) + self.assertIn("20 minutes", error.getvalue()) def test_stage_cleanup_shortens_target_and_relays(self) -> None: class Controller: diff --git a/tools/lora_ota/test_lora_ota_bootloader.py b/tools/lora_ota/test_lora_ota_bootloader.py index 0c5d71e9d..46b113f72 100644 --- a/tools/lora_ota/test_lora_ota_bootloader.py +++ b/tools/lora_ota/test_lora_ota_bootloader.py @@ -105,11 +105,17 @@ class BootloaderStagingTests(unittest.TestCase): dict(platform="esp32"), dict(boot_target_id=None), dict(boot_target_id=self.package.target_id ^ 1), dict(boot_hw_id="wrong"), dict(bootloader_abi=2), dict(bootloader_codecs=4), - dict(boot_storage=14), dict(nrf_qspi=True), dict(max_block_size=512), + dict(boot_storage=14), dict(max_block_size=512), ): with self.subTest(change=change): self.assertFalse(ota.compatible_mota(self.package, replace(live, **change))[0]) + def test_internal_rak_bootloader_allows_qspi_application_storage(self): + # The application may stage OTA data in QSPI while the bootloader + # update itself remains an exact-board internal-flash package. + live = replace(boot_target(self.package), nrf_qspi=True) + self.assertEqual(ota.compatible_mota(self.package, live), (True, "")) + def test_stage_only_is_required_even_with_yes(self): args = argparse.Namespace(no_install=False, yes=True) with self.assertRaisesRegex(ota.OtaError, "use --no-install"): diff --git a/tools/lora_ota/test_lora_ota_contact_import.py b/tools/lora_ota/test_lora_ota_contact_import.py index 385ee07f1..2687307f3 100644 --- a/tools/lora_ota/test_lora_ota_contact_import.py +++ b/tools/lora_ota/test_lora_ota_contact_import.py @@ -14,6 +14,7 @@ from test_lora_ota_source_identity import banner, connection SOURCE = "a5" * 32 TARGET = "b6" * 32 CONTROLLER = "c7" * 32 +RELAY = "d8" * 32 NAME = "Source \U0001f4e1" @@ -104,6 +105,16 @@ class ContactImportTests(unittest.TestCase): def bind(self): ota.bind_contact_selectors(self.controller, self.args) + def test_relay_public_key_binds_existing_contact(self): + self.args.relay_values = [(RELAY.upper(), "relay-password")] + self.args.source_shares_controller = True + self.controller._run.side_effect = [[ + {**self.before, RELAY: entry(RELAY, "Relay renamed later")} + ]] + self.bind() + self.assertEqual(self.args.relay_values, [(RELAY, "relay-password")]) + self.controller._execute.assert_not_called() + def test_approved_import_verified_then_continue_with_full_key(self): self.bind() self.assertEqual(self.args.source_contact_value, SOURCE)