Fix LoRa OTA TempRadio compatibility and lease handling

Accept preamble-bearing TempRadio status from newer firmware while retaining legacy replies. Keep the 20-minute hard minimum but make the 120-minute and timeout estimates advisory, with a window-only override. Add parser, rehearsal, relay identity, and RAK bootloader regression coverage.
This commit is contained in:
mikecarper committed 2026-09-29 07:48:01 -07:00
1 parent 84429ae520
commit 21d42aa57f
6 files changed
+243 -58

No files matched your search

+7
View File
@@ -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
+67 -33
View File
@@ -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:
+11 -17
View File
@@ -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(
+140 -7
View File
@@ -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:
+7 -1
View File
@@ -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"):
@@ -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)