diff --git a/config.yaml.example b/config.yaml.example index 6068b82..ea22b45 100644 --- a/config.yaml.example +++ b/config.yaml.example @@ -417,9 +417,21 @@ identities: # Multi-radio / LoRa backhaul (optional) # --------------------------------------------------------------------------- # Leave ``radios:`` unset to keep the legacy single-radio config below. -# When present, each entry is built with the same fields as the top-level -# single-radio config (radio_type + radio air settings + hardware section). -# Different entries may use different frequency / SF / modem type. +# When present, each entry defines one physical radio. +# +# Top-level radio/hardware sections act as defaults for each entry. +# Mapping sections under radios[] overlay those defaults key-by-key: +# omitted values are inherited, while values explicitly present in an +# entry override the top-level setting. +# +# This allows board-specific settings such as use_dio3_tcxo, +# use_dio2_rf, dio3_tcxo_voltage, en_pin/en_pins, GPIO settings, etc. +# to remain in the top-level hardware configuration unless a particular +# radio needs a different value. +# +# radio_type may also be overridden per entry, so different transport +# types can be mixed in the same fabric. Different entries may use +# different frequency / SF / modem type. # # fabric: # default_radio: local # TX default (first entry if omitted) @@ -515,7 +527,29 @@ identities: # txen_pin: -1 # rxen_pin: -1 # -# Radio hardware type (legacy single-radio; ignored for construction when radios: is set) +# Entries only need the values that differ. The sx1262: section further down +# holds board-wide properties such as TCXO / RF-switch control and enable +# pins; those are inherited by every entry that does not override them: +# +# radios: +# - id: local +# radio: +# frequency: 910525000 +# sx1262: +# cs_id: 0 +# +# - id: link +# radio: +# frequency: 914500000 +# sx1262: +# cs_id: 1 +# reset_pin: 27 +# irq_pin: 18 +# +# Default radio hardware type. +# In legacy single-radio mode this selects the physical radio directly. +# With radios: configured, entries inherit this value unless they specify +# their own radio_type. # Supported: # - sx1262 (Linux spidev + system GPIO) # - sx1262_ch341 (CH341 USB-to-SPI + CH341 GPIO 0-7) diff --git a/repeater/config.py b/repeater/config.py index 6cc13e1..d6af428 100644 --- a/repeater/config.py +++ b/repeater/config.py @@ -1,4 +1,5 @@ import base64 +import copy import inspect import logging import os @@ -9,7 +10,11 @@ from typing import Any, Dict, Optional, overload import yaml from repeater.exceptions import ConfigurationError -from repeater.modem_config import LEGACY_MODEM_RADIO_TYPES, normalize_modem_config +from repeater.modem_config import ( + LEGACY_MODEM_RADIO_TYPES, + normalize_modem_config, + redact_modem_tokens_in_place, +) from repeater.policy_engine import default_policy_engine_config logger = logging.getLogger("Config") @@ -762,11 +767,28 @@ def get_radio_for_board(board_config: dict): ) +# Keys within one section that are alternative spellings of a single setting. +# Whole-section replacement kept these from meeting; key-by-key overlay lets an +# inherited spelling sit next to the entry's, and downstream each pair has a +# fixed winner -- en_pins beats en_pin in SX1262Radio, "address" beats +# "device_address" and "serial_number" beats "serial" in get_radio_for_board. +# So an entry naming one spelling drops the other rather than losing to a +# default it never wrote, which would otherwise fail silently: a radio that +# never powers up, or one opened on the wrong CH341 adapter. +_RADIO_SECTION_ALIASES = { + "sx1262": (("en_pin", "en_pins"),), + "ch341": (("address", "device_address"), ("serial_number", "serial")), +} + + def _merge_radio_entry(global_config: dict, entry: dict) -> dict: """Build a board_config dict for one radios[] entry. - Entry may override radio_type and hardware sections; shared top-level - ``radio`` air settings are inherited unless the entry supplies its own. + Top-level radio and hardware sections act as defaults for multi-radio + entries. Mapping sections are overlaid key-by-key, so an entry only needs + to specify values that differ from the top-level configuration. Explicit + values, including False/None, override inherited values. ``radio_type`` + is a scalar override. """ if not isinstance(entry, dict): raise ValueError("each radios[] entry must be a mapping") @@ -776,8 +798,15 @@ def _merge_radio_entry(global_config: dict, entry: dict) -> dict: raise ValueError("each radios[] entry requires an 'id'") merged = dict(global_config) + + # radio_type is scalar: a per-radio value replaces the global value. + if "radio_type" in entry: + merged["radio_type"] = entry["radio_type"] + + # Configuration sections use shallow overlay semantics. This is + # intentionally one level deep: the known sections currently contain + # scalar values and simple lists rather than nested configuration trees. for key in ( - "radio_type", "radio", "sx1262", "ch341", @@ -785,8 +814,21 @@ def _merge_radio_entry(global_config: dict, entry: dict) -> dict: "modem_tcp", "modem_usb", ): - if key in entry: - merged[key] = entry[key] + if key not in entry: + continue + + base = global_config.get(key) + override = entry[key] + + if isinstance(base, dict) and isinstance(override, dict): + inherited = dict(base) + for group in _RADIO_SECTION_ALIASES.get(key, ()): + if any(alias in override for alias in group): + for alias in group: + inherited.pop(alias, None) + merged[key] = {**inherited, **override} + else: + merged[key] = override if "radio_type" not in entry and "radio_type" not in global_config: raise ValueError(f"radios[] entry {radio_id!r} missing radio_type") @@ -1269,7 +1311,22 @@ def build_metering_profiles(config: dict) -> list: return profiles -def build_radio_status_entries(config: dict) -> list: +def _default_radio_id(config: dict, radio_ids: list) -> Optional[str]: + """The radio a live config save actually retunes. + + Mirrors build_radio_stack's TX default -- ``fabric.default_radio`` when it + names a radio that exists, else the first configured one. A default naming + a radio that is not there cannot be the one that got retuned, so it falls + back rather than freezing every entry. + """ + fabric_cfg = config.get("fabric") if isinstance(config.get("fabric"), dict) else {} + default_radio = fabric_cfg.get("default_radio") or fabric_cfg.get("default_radio_id") + if default_radio and str(default_radio) in radio_ids: + return str(default_radio) + return radio_ids[0] if radio_ids else None + + +def build_radio_status_entries(config: dict, *, applied: Optional[dict] = None) -> list: """Map every radio id to its air settings, for the MQTT status message. Observers join this against the ``rx_radio_id`` / ``tx_radio_ids`` fields @@ -1285,12 +1342,24 @@ def build_radio_status_entries(config: dict) -> list: the air settings it never voids the map: power does not decide which band a packet was on, so an entry without it still attributes the packet correctly. - Every value here is what the node is **configured** with, not a reading from - the hardware. Two consequences worth knowing before trusting a figure: a - driver may clamp what it was given (an SX1262 holds -9..22 dBm), and editing - a ``radios[]`` entry does not reconfigure that radio live -- ``config_manager`` - only applies the top-level ``radio`` section -- so after such an edit the map - leads the hardware until the service restarts. + These are the settings each radio was actually configured with, not a + reading from the hardware -- a driver may still clamp what it was given (an + SX1262 holds -9..22 dBm). + + ``applied`` maps radio id to the entry built when that radio was last really + configured, and makes the map survive a live save. Only the default radio is + retuned without a restart: a change to any other sits in ``radios[]`` marked + restart-required. Because ``radios[]`` entries inherit the top-level + ``radio`` block key by key, a save that edits the default radio also moves + the *configured* values of every radio whose entry omits that key, so + rebuilding all of them from config would publish a bandwidth the hardware + was never given and mis-attribute that radio's packets to a band it is not + on. Entries other than the default therefore keep their applied values until + the service restarts. ``AirtimeBudgets._effective_air`` holds the same line + for duty-cycle metering. + + Passing no ``applied`` map rebuilds every entry from config, which is what a + fresh start wants. Returns an empty list when any radio's settings are unreadable: a partial map would silently attribute a packet to the wrong band, which is worse @@ -1316,9 +1385,37 @@ def build_radio_status_entries(config: dict) -> list: if tx_power is not None: entry["tx_power"] = tx_power entries.append(entry) + + if applied: + default_id = _default_radio_id(config, [entry["id"] for entry in entries]) + entries = [ + entry if entry["id"] == default_id else applied.get(entry["id"], entry) + for entry in entries + ] return entries +_RADIO_LOG_SECTIONS = ("radio", "sx1262", "ch341", "kiss", "modem_tcp", "modem_usb") + + +def _describe_radio_config(board_config: dict) -> str: + """Render a resolved board_config for debug logging, tokens redacted. + + What a radio inherited is otherwise invisible: the config file shows the + entry, not the merge, and a wrong answer surfaces as a radio that does not + transmit. ``modem_tcp.token`` is the one credential these sections carry, + and ``redact_modem_tokens_in_place`` runs over a copy so the live config is + untouched. + """ + snapshot = { + key: copy.deepcopy(board_config[key]) for key in _RADIO_LOG_SECTIONS if key in board_config + } + redact_modem_tokens_in_place(snapshot, replacement="***") + parts = [f"type={board_config.get('radio_type')!r}"] + parts.extend(f"{key}={snapshot[key]!r}" for key in _RADIO_LOG_SECTIONS if key in snapshot) + return " ".join(parts) + + def build_radio_stack(config: dict): """Build single- or multi-radio stack for the repeater. @@ -1367,6 +1464,8 @@ def build_radio_stack(config: dict): for entry in radios_cfg: merged = _merge_radio_entry(config, entry) rid = merged.pop("_radio_id") + if logger.isEnabledFor(logging.DEBUG): + logger.debug("Resolved radio %r: %s", rid, _describe_radio_config(merged)) physical = get_radio_for_board(merged) pairs.append((physical, rid)) profile = _radio_air_profile(merged, rid) diff --git a/repeater/data_acquisition/mqtt_handler.py b/repeater/data_acquisition/mqtt_handler.py index 9979867..e835acf 100644 --- a/repeater/data_acquisition/mqtt_handler.py +++ b/repeater/data_acquisition/mqtt_handler.py @@ -669,6 +669,10 @@ class MeshCoreToMqttPusher: self._shutdown_requested = False self._lock = threading.Lock() self._connect_timers: List[threading.Timer] = [] + # The radio map as built when the radios were last really configured. + # Seeded on the first build, which is this process's start -- see + # _radio_map for why a later rebuild must not replace it wholesale. + self._applied_radio_map: Optional[dict] = None # Initialize brokers list mqtt_brokers_config = config.get("mqtt_brokers", {}) @@ -1048,15 +1052,24 @@ class MeshCoreToMqttPusher: when there is genuinely more than one band to tell apart. Rebuilt per status publish rather than cached at construction, because - air settings can be changed from the web UI while the node runs. + air settings can be changed from the web UI while the node runs. Only + the default radio is retuned by such a save, though, and radios[] entries + inherit the top-level ``radio`` block key by key -- so a rebuild that + took every entry from the current config would report a new bandwidth + for radios still transmitting on the old one, and observers would + attribute their packets to a band they are not on. The first build is + kept and handed back in as ``applied`` so every non-default radio keeps + what it was really configured with until the service restarts. """ from ..config import build_radio_status_entries try: - entries = build_radio_status_entries(self.config) + entries = build_radio_status_entries(self.config, applied=self._applied_radio_map) except Exception as exc: # pragma: no cover - reporting must not break status logger.debug(f"Could not build radio map for status: {exc}") return [] + if self._applied_radio_map is None and entries: + self._applied_radio_map = {entry["id"]: entry for entry in entries} return entries if len(entries) > 1 else [] def publish(self, subtopic: str, payload: dict, retain: bool = False, qos: int = 0): diff --git a/tests/test_multi_radio_stack.py b/tests/test_multi_radio_stack.py index d1efd67..690e8ab 100644 --- a/tests/test_multi_radio_stack.py +++ b/tests/test_multi_radio_stack.py @@ -2,14 +2,18 @@ from __future__ import annotations -from unittest.mock import patch +import sys +import types +from unittest.mock import MagicMock, patch import pytest from repeater.config import ( NullRadio, _apply_fabric_tx_mode, + _describe_radio_config, _merge_radio_entry, + build_metering_profiles, build_radio_stack, fabric_selects_by_ingress_radio, ) @@ -67,26 +71,171 @@ class _FakeRadio: return 5.0 -def test_merge_radio_entry_inherits_and_overrides(): +def test_merge_radio_entry_overlays_radio_sections(): + """A partial entry section inherits the top-level keys it omits.""" global_cfg = { "radio_type": "sx1262", - "radio": {"frequency": 1, "tx_power": 10}, - "sx1262": {"bus_id": 0}, + "radio": { + "frequency": 869618000, + "tx_power": 10, + "bandwidth": 62500, + "spreading_factor": 8, + "coding_rate": 8, + }, + "sx1262": { + "bus_id": 0, + "cs_id": 0, + "use_dio3_tcxo": True, + "dio3_tcxo_voltage": 1.8, + "use_dio2_rf": True, + "en_pins": [12, 13], + }, + } + entry = { + "id": "local", + "radio": { + "frequency": 910525000, + "tx_power": 22, + }, + "sx1262": { + "cs_id": 1, + "reset_pin": 24, + }, + } + + merged = _merge_radio_entry(global_cfg, entry) + + assert merged["_radio_id"] == "local" + assert merged["radio_type"] == "sx1262" + + assert merged["radio"]["frequency"] == 910525000 + assert merged["radio"]["tx_power"] == 22 + assert merged["radio"]["bandwidth"] == 62500 + assert merged["radio"]["spreading_factor"] == 8 + assert merged["radio"]["coding_rate"] == 8 + + assert merged["sx1262"]["cs_id"] == 1 + assert merged["sx1262"]["reset_pin"] == 24 + assert merged["sx1262"]["bus_id"] == 0 + assert merged["sx1262"]["use_dio3_tcxo"] is True + assert merged["sx1262"]["dio3_tcxo_voltage"] == 1.8 + assert merged["sx1262"]["use_dio2_rf"] is True + assert merged["sx1262"]["en_pins"] == [12, 13] + + +def test_merge_radio_entry_inherits_sx1262_tcxo_and_power_settings(): + """RAK6421/RAK1330x regression: pin-only entry keeps TCXO/RF-switch setup.""" + global_cfg = { + "radio_type": "sx1262", + "radio": { + "frequency": 910525000, + "tx_power": 22, + }, + "sx1262": { + "bus_id": 0, + "cs_id": 0, + "cs_pin": -1, + "reset_pin": 16, + "busy_pin": 24, + "irq_pin": 22, + "txen_pin": -1, + "rxen_pin": -1, + "en_pins": [12, 13], + "use_dio2_rf": True, + "use_dio3_tcxo": True, + "dio3_tcxo_voltage": 1.8, + }, + } + entry = { + "id": "local", + "sx1262": { + "bus_id": 0, + "cs_id": 0, + "cs_pin": -1, + "reset_pin": 16, + "busy_pin": 24, + "irq_pin": 22, + "txen_pin": -1, + "rxen_pin": -1, + }, + } + + merged = _merge_radio_entry(global_cfg, entry) + + assert merged["sx1262"]["use_dio3_tcxo"] is True + assert merged["sx1262"]["dio3_tcxo_voltage"] == 1.8 + assert merged["sx1262"]["use_dio2_rf"] is True + assert merged["sx1262"]["en_pins"] == [12, 13] + + +def test_merge_radio_entry_explicit_false_overrides_inherited_true(): + """Presence, not truthiness, decides whether an entry value overrides.""" + global_cfg = { + "radio_type": "sx1262", + "sx1262": { + "use_dio3_tcxo": True, + "use_dio2_rf": True, + "is_waveshare": True, + }, } entry = { "id": "link", - "radio_type": "modem_usb", - "radio": {"frequency": 2, "tx_power": 22}, - "modem_usb": {"port": "/dev/ttyACM0"}, + "sx1262": { + "use_dio3_tcxo": False, + "use_dio2_rf": False, + "is_waveshare": False, + }, } + merged = _merge_radio_entry(global_cfg, entry) - assert merged["_radio_id"] == "link" + + assert merged["sx1262"]["use_dio3_tcxo"] is False + assert merged["sx1262"]["use_dio2_rf"] is False + assert merged["sx1262"]["is_waveshare"] is False + + +def test_merge_radio_entry_can_override_radio_type(): + global_cfg = { + "radio_type": "sx1262", + "radio": { + "frequency": 910525000, + "bandwidth": 62500, + }, + "sx1262": { + "bus_id": 0, + }, + } + entry = { + "id": "backhaul", + "radio_type": "modem_usb", + "modem_usb": { + "port": "/dev/ttyACM0", + }, + } + + merged = _merge_radio_entry(global_cfg, entry) + assert merged["radio_type"] == "modem_usb" - assert merged["radio"]["frequency"] == 2 assert merged["modem_usb"]["port"] == "/dev/ttyACM0" + assert merged["radio"]["frequency"] == 910525000 assert "sx1262" in merged # inherited leftover ok; factory uses radio_type +def test_merge_radio_entry_non_mapping_section_replaces_base(): + global_cfg = { + "radio_type": "sx1262", + "sx1262": {"use_dio3_tcxo": True}, + } + entry = { + "id": "local", + "sx1262": None, + } + + merged = _merge_radio_entry(global_cfg, entry) + + assert merged["sx1262"] is None + + def test_build_radio_stack_legacy_single(): cfg = {"radio_type": "none"} radio, meta = build_radio_stack(cfg) @@ -373,20 +522,38 @@ def test_origin_tx_conflicting_with_former_name_rejected(): factory.assert_not_called() -def test_merge_radio_entry_preserves_per_radio_ch341(): +def test_merge_radio_entry_overlays_per_radio_ch341_selection(): + """Shared adapter parameters stay global; only device identity differs.""" global_cfg = { "radio_type": "sx1262_ch341", - "ch341": {"vid": 0x1A86, "pid": 0x5512, "bus": 1, "address": 5}, - "radio": {"frequency": 1}, + "ch341": { + "vid": 0x1A86, + "pid": 0x5512, + "bus": 1, + "address": 5, + }, + "radio": {"frequency": 869618000}, "sx1262": {"bus_id": 0}, } entry = { "id": "link", - "ch341": {"vid": 0x1A86, "pid": 0x5512, "bus": 1, "address": 8}, - "radio": {"frequency": 2}, + "ch341": { + "address": 8, + }, + "radio": { + "frequency": 864200000, + }, } + merged = _merge_radio_entry(global_cfg, entry) + + assert merged["ch341"]["vid"] == 0x1A86 + assert merged["ch341"]["pid"] == 0x5512 + assert merged["ch341"]["bus"] == 1 assert merged["ch341"]["address"] == 8 + + assert merged["radio"]["frequency"] == 864200000 + assert merged["_ch341_per_instance"] is True @@ -452,3 +619,290 @@ def test_a_current_core_is_given_the_selector_that_routes_by_ingress_radio(): assert fabric.resolve_tx_radio_id(b"frame", rx_radio_id="link") == "local" # Originated here, so there is no side of the bridge to come from. assert fabric.resolve_tx_radio_id(b"frame") == "local" + + +def test_build_radio_stack_multi_inherits_top_level_hardware_defaults(): + """End-to-end: a partial radios[] entry reaches the radio factory complete.""" + seen = [] + + def fake_get(board): + seen.append(dict(board)) + return _FakeRadio(str(len(seen))) + + cfg = { + "radio_type": "sx1262", + "sx1262": { + "bus_id": 0, + "cs_id": 0, + "cs_pin": -1, + "reset_pin": 16, + "busy_pin": 24, + "irq_pin": 22, + "txen_pin": -1, + "rxen_pin": -1, + "en_pins": [12, 13], + "use_dio3_tcxo": True, + "dio3_tcxo_voltage": 1.8, + "use_dio2_rf": True, + }, + "radios": [ + { + "id": "local", + "radio": {"frequency": 910525000}, + "sx1262": {"cs_id": 0, "reset_pin": 16, "busy_pin": 24, "irq_pin": 22}, + }, + { + "id": "remote", + "radio_type": "modem_tcp", + "modem_tcp": {"host": "remote-radio.local"}, + }, + ], + } + + with patch("repeater.config.get_radio_for_board", side_effect=fake_get): + _radio, meta = build_radio_stack(cfg) + + assert meta["radio_ids"] == ["local", "remote"] + + local, remote = seen + assert local["radio_type"] == "sx1262" + assert local["sx1262"]["en_pins"] == [12, 13] + assert local["sx1262"]["use_dio3_tcxo"] is True + assert local["sx1262"]["dio3_tcxo_voltage"] == 1.8 + assert local["sx1262"]["use_dio2_rf"] is True + assert local["radio"]["frequency"] == 910525000 + + assert remote["radio_type"] == "modem_tcp" + assert remote["modem_tcp"]["host"] == "remote-radio.local" + + +def test_merge_radio_entry_does_not_mutate_global_config(): + global_cfg = { + "radio_type": "sx1262", + "sx1262": {"bus_id": 0, "use_dio3_tcxo": True}, + } + entry = {"id": "link", "sx1262": {"cs_id": 1}} + + _merge_radio_entry(global_cfg, entry) + + assert global_cfg["sx1262"] == {"bus_id": 0, "use_dio3_tcxo": True} + assert entry["sx1262"] == {"cs_id": 1} + + +def test_merge_radio_entry_entry_en_pin_displaces_inherited_en_pins(): + """en_pins beats en_pin in SX1262Radio, so an inherited en_pins must not + shadow the single pin this entry asked for.""" + global_cfg = { + "radio_type": "sx1262", + "sx1262": {"bus_id": 0, "en_pins": [12, 13]}, + } + entry = {"id": "link", "sx1262": {"en_pin": 26}} + + merged = _merge_radio_entry(global_cfg, entry) + + assert merged["sx1262"]["en_pin"] == 26 + assert "en_pins" not in merged["sx1262"] + assert merged["sx1262"]["bus_id"] == 0 # unrelated keys still inherited + + +def test_merge_radio_entry_entry_en_pins_displaces_inherited_en_pin(): + global_cfg = { + "radio_type": "sx1262", + "sx1262": {"bus_id": 0, "en_pin": 26}, + } + entry = {"id": "link", "sx1262": {"en_pins": [12, 13]}} + + merged = _merge_radio_entry(global_cfg, entry) + + assert merged["sx1262"]["en_pins"] == [12, 13] + assert "en_pin" not in merged["sx1262"] + + +@pytest.mark.parametrize( + "inherited_key, entry_key", + [ + ("address", "device_address"), + ("device_address", "address"), + ], +) +def test_merge_radio_entry_ch341_address_spelling_displaces_the_other(inherited_key, entry_key): + """get_radio_for_board reads address before device_address, so whichever + spelling the entry uses has to displace the inherited one -- otherwise an + inherited address silently selects the wrong USB adapter.""" + global_cfg = { + "radio_type": "sx1262_ch341", + "ch341": {"vid": 0x1A86, "pid": 0x5512, "bus": 1, inherited_key: 5}, + } + entry = {"id": "link", "ch341": {entry_key: 8}} + + merged = _merge_radio_entry(global_cfg, entry) + + assert merged["ch341"][entry_key] == 8 + assert inherited_key not in merged["ch341"] + assert merged["ch341"]["bus"] == 1 # unrelated keys still inherited + + +@pytest.mark.parametrize( + "inherited_key, entry_key", + [ + ("serial_number", "serial"), + ("serial", "serial_number"), + ], +) +def test_merge_radio_entry_ch341_serial_spelling_displaces_the_other(inherited_key, entry_key): + """A truthy serial_number wins over serial downstream, so an inherited one + would outrank the entry's choice whichever name the entry used.""" + global_cfg = { + "radio_type": "sx1262_ch341", + "ch341": {"vid": 0x1A86, inherited_key: "AAA111"}, + } + entry = {"id": "link", "ch341": {entry_key: "BBB222"}} + + merged = _merge_radio_entry(global_cfg, entry) + + assert merged["ch341"][entry_key] == "BBB222" + assert inherited_key not in merged["ch341"] + + +@pytest.mark.parametrize("cleared", [0, None, ""]) +def test_merge_radio_entry_ch341_falsy_entry_value_still_displaces(cleared): + """Displacement is presence-based. An entry clearing its adapter selector + must not have the inherited one reinstated under the other spelling.""" + global_cfg = { + "radio_type": "sx1262_ch341", + "ch341": {"vid": 0x1A86, "address": 5, "serial_number": "AAA111"}, + } + entry = {"id": "link", "ch341": {"device_address": cleared}} + + merged = _merge_radio_entry(global_cfg, entry) + + assert merged["ch341"]["device_address"] == cleared + assert "address" not in merged["ch341"] + # A different alias group is untouched by this entry, so it still inherits. + assert merged["ch341"]["serial_number"] == "AAA111" + + +def test_merge_radio_entry_ch341_entry_naming_both_spellings_is_left_alone(): + """Both spellings in one entry is a pre-existing accepted configuration. + The merge must not rewrite or reject it -- get_radio_for_board's own + precedence (address before device_address) still decides.""" + global_cfg = { + "radio_type": "sx1262_ch341", + "ch341": {"vid": 0x1A86, "address": 5, "device_address": 6}, + } + entry = {"id": "link", "ch341": {"address": 8, "device_address": 9}} + + merged = _merge_radio_entry(global_cfg, entry) + + assert merged["ch341"]["address"] == 8 + assert merged["ch341"]["device_address"] == 9 + assert merged["ch341"]["vid"] == 0x1A86 + + +def test_merge_radio_entry_keeps_inherited_alias_when_entry_names_neither(): + """The alias guard only fires when the entry actually picks a spelling.""" + global_cfg = { + "radio_type": "sx1262", + "sx1262": {"en_pins": [12, 13], "bus_id": 0}, + } + entry = {"id": "link", "sx1262": {"cs_id": 1}} + + merged = _merge_radio_entry(global_cfg, entry) + + assert merged["sx1262"]["en_pins"] == [12, 13] + + +def test_metering_profile_inherits_top_level_air_settings(): + """A partial radios[] air block used to meter on library defaults rather + than on the node's own top-level radio settings.""" + cfg = { + "radio_type": "sx1262", + "radio": { + "frequency": 906875000, + "bandwidth": 250000, + "spreading_factor": 10, + "coding_rate": 5, + "preamble_length": 16, + "tx_power": 22, + }, + "sx1262": {"bus_id": 0}, + "radios": [{"id": "local", "radio": {"frequency": 910525000}}], + } + + (profile,) = build_metering_profiles(cfg) + + assert profile["radio_id"] == "local" + assert profile["frequency_hz"] == 910525000 + assert profile["bandwidth_hz"] == 250000 + assert profile["spreading_factor"] == 10 + assert profile["coding_rate"] == 5 + assert profile["tx_power"] == 22 + + +def test_describe_radio_config_redacts_modem_token(): + board = { + "radio_type": "modem_tcp", + "radio": {"frequency": 910525000}, + "modem_tcp": {"host": "remote.local", "token": "s3cret-value"}, + } + + described = _describe_radio_config(board) + + assert "s3cret-value" not in described + assert "***" in described + assert "remote.local" in described + assert "type='modem_tcp'" in described + # The live config keeps its token; only the log copy is redacted. + assert board["modem_tcp"]["token"] == "s3cret-value" + + +def test_ch341_alias_displacement_reaches_the_usb_transport(): + """The merge-level tests above pin the dict; this pins the consequence. + + get_radio_for_board selects the adapter with + ``ch341.get("address", ch341.get("device_address"))``, so without + displacement an inherited ``address`` would open a different physical + adapter than the entry asked for, on a node with two CH341 sticks. + """ + ch341_module = types.ModuleType("openhop_core.hardware.transports.ch341_spi_transport") + ch341_module.CH341SPITransport = MagicMock(name="CH341SPITransport") + + config = { + "radio_type": "sx1262_ch341", + "ch341": {"vid": 0x1A86, "pid": 0x5512, "bus": 1, "address": 5}, + "radio": { + "frequency": 869618000, + "tx_power": 14, + "bandwidth": 62500, + "spreading_factor": 8, + "coding_rate": 8, + "preamble_length": 32, + }, + "sx1262": { + "bus_id": 0, + "cs_id": 0, + "cs_pin": 0, + "reset_pin": 1, + "busy_pin": 2, + "irq_pin": 3, + "txen_pin": -1, + "rxen_pin": -1, + }, + "radios": [{"id": "link", "ch341": {"device_address": 8}}], + } + + with patch.dict( + sys.modules, + {"openhop_core.hardware.transports.ch341_spi_transport": ch341_module}, + ): + with patch("openhop_core.hardware.sx1262_wrapper.SX1262Radio"): + with patch("openhop_core.hardware.lora.LoRaRF.SX126x.set_spi_transport") as set_spi: + build_radio_stack(config) + + (call,) = ch341_module.CH341SPITransport.call_args_list + assert call.kwargs["address"] == 8 # the entry's adapter, not the inherited 5 + assert call.kwargs["bus"] == 1 # still inherited + assert call.kwargs["vid"] == 0x1A86 + # Multi-radio must not install the process-global SPI transport. + set_spi.assert_not_called() + assert call.kwargs["set_as_global_gpio"] is False diff --git a/tests/test_observer_radio_attribution.py b/tests/test_observer_radio_attribution.py index a4223f9..cdbd33a 100644 --- a/tests/test_observer_radio_attribution.py +++ b/tests/test_observer_radio_attribution.py @@ -11,6 +11,7 @@ Single-radio nodes must publish exactly what they published before any of this existed. """ +import copy import json import logging from unittest.mock import MagicMock, patch @@ -454,3 +455,113 @@ def test_collector_omits_radio_ids_on_a_single_radio_node(tmp_path): assert "rx_radio_id" not in payload assert "tx_radio_ids" not in payload + + +# -------------------------------------------------------------------- +# A live save must not move a radio that was not retuned +# -------------------------------------------------------------------- +def test_applied_map_holds_non_default_radios_across_a_live_edit(): + """Entries inherit the top-level radio block key by key, so editing the + default radio changes what a partial entry is *configured* with. Only the + default radio is actually retuned, so the rest must keep reporting the band + they are still transmitting on.""" + config = { + "radio_type": "sx1262", + "radio": { + "frequency": 869618000, + "bandwidth": 62500, + "spreading_factor": 8, + "coding_rate": 8, + "preamble_length": 32, + }, + "fabric": {"default_radio": "local"}, + "radios": [ + {"id": "local", "radio": {"frequency": 869618000}}, + # Omits bandwidth/SF/CR, so it inherits them. + {"id": "link", "radio": {"frequency": 864200000}}, + ], + } + + applied = {entry["id"]: entry for entry in build_radio_status_entries(config)} + assert applied["link"]["radio"] == "864.2,62.5,8,8" + + # What api_endpoints._set_radio_param does for the default radio: write the + # target entry, and mirror it into the legacy top-level block. + config["radios"][0]["radio"]["bandwidth"] = 500000 + config["radio"]["bandwidth"] = 500000 + + entries = build_radio_status_entries(config, applied=applied) + by_id = {entry["id"]: entry for entry in entries} + + assert by_id["local"]["radio"] == "869.618,500.0,8,8" # really retuned + assert by_id["link"]["radio"] == "864.2,62.5,8,8" # still on 62.5 kHz + + # Without the applied map, link would follow the mirrored top-level value + # onto a band its hardware was never given -- the bug this guards. + naive = {e["id"]: e["radio"] for e in build_radio_status_entries(config)} + assert naive["link"] == "864.2,500.0,8,8" + + +def test_applied_map_is_ignored_for_a_radio_the_config_has_just_added(): + """A radio with no applied entry has no band to preserve, so it reports + what it is configured with.""" + config = { + "radio_type": "sx1262", + "radio": LOCAL_RADIO["radio"], + "fabric": {"default_radio": "local"}, + "radios": [LOCAL_RADIO, LINK_RADIO], + } + applied = {"local": {"id": "local", "radio": "869.618,62.5,8,8"}} + + entries = build_radio_status_entries(config, applied=applied) + + assert entries[1] == {"id": "link", "radio": "864.2,62.5,11,8"} + + +def test_no_applied_map_rebuilds_every_entry_from_config(): + """A fresh start has nothing to preserve.""" + config = {"radios": [LOCAL_RADIO, LINK_RADIO], "fabric": {"default_radio": "local"}} + + assert build_radio_status_entries(config) == build_radio_status_entries(config, applied=None) + + +def _publish_status_across_a_live_edit(config: dict, mutate) -> tuple: + """Two status publishes from one pusher, with a config edit between them.""" + pusher = MeshCoreToMqttPusher(local_identity=_FakeIdentity("AB" * 32), config=config) + conn = pusher.connections[0] + captured = [] + conn._running = True + conn.client = MagicMock() + conn.client.publish = lambda topic, payload, retain=False, qos=0: captured.append(payload) + + pusher.publish_status(state="online") + mutate(config) + pusher.publish_status(state="online") + + assert len(captured) == 2 + return tuple(json.loads(payload) for payload in captured) + + +def test_published_status_holds_a_non_retuned_radio_across_a_live_edit(): + """The handler-level version of the same guarantee: an observer must not be + told a radio moved band when only the default radio was retuned.""" + local = copy.deepcopy(LOCAL_RADIO) + config = _make_config(radios=[local, {"id": "link", "radio": {"frequency": 864200000}}]) + config["radio"] = copy.deepcopy(LOCAL_RADIO["radio"]) + config["fabric"]["default_radio"] = "local" + + def widen_the_default_radio(cfg): + # Mirrors api_endpoints._set_radio_param for the default radio. + cfg["radios"][0]["radio"]["bandwidth"] = 500000 + cfg["radio"]["bandwidth"] = 500000 + + first, second = _publish_status_across_a_live_edit(config, widen_the_default_radio) + + assert {e["id"]: e["radio"] for e in first["radios"]} == { + "local": "869.618,62.5,8,8", + "link": "864.2,62.5,8,8", + } + assert {e["id"]: e["radio"] for e in second["radios"]} == { + "local": "869.618,500.0,8,8", # retuned + "link": "864.2,62.5,8,8", # untouched, still reported as it is + }