From 7eccbfb89afdf02225aacd8d1797d2e5998514ef Mon Sep 17 00:00:00 2001 From: l5y <220195275+l5yth@users.noreply.github.com> Date: Tue, 23 Jun 2026 21:58:18 +0200 Subject: [PATCH] fix: unify Meshtastic custom LoRa label with MeshCore format (#818) --- ACCEPTANCE.md | 10 ++++ data/mesh_ingestor/interfaces/radio.py | 19 +++--- tests/test_interfaces_unit.py | 82 ++++++++++++++++++-------- 3 files changed, 79 insertions(+), 32 deletions(-) diff --git a/ACCEPTANCE.md b/ACCEPTANCE.md index 14d1e19..c0a1c82 100644 --- a/ACCEPTANCE.md +++ b/ACCEPTANCE.md @@ -174,6 +174,16 @@ borrowing a node from another protocol (`findNodeByLongName(longName, nodesById, protocol)` + `chat-entry-renderer.js`). Concrete UI form of SPEC Invariant IV (protocol parity; neither protocol privileged in the data model or UI). +### A4d — Custom radio-config label is protocol-neutral (regression: c8668a7) +```bash +( . .venv/bin/activate && pytest -q tests/test_interfaces_unit.py::TestCustomPresetLabelParity ) +``` +**Expected:** pass. A Meshtastic custom LoRa config (`use_preset=False`) renders +the **same** compact `SF/BW/CR` label as MeshCore's `_derive_modem_preset` for +identical SF/BW/CR — no protocol-specific `"Custom "` prefix — and returns `None` +(not a bare `"Custom"`) when the parameters are unreported, so one radio config +never displays as two different strings depending on protocol (SPEC Invariant IV). + --- ## Layer B — Engineering bar (restated from `CLAUDE.md`) diff --git a/data/mesh_ingestor/interfaces/radio.py b/data/mesh_ingestor/interfaces/radio.py index dfe9295..141926b 100644 --- a/data/mesh_ingestor/interfaces/radio.py +++ b/data/mesh_ingestor/interfaces/radio.py @@ -222,27 +222,30 @@ def _camelcase_enum_name(name: str | None) -> str | None: return "".join(camel_parts) -def _custom_preset_label(lora_message: Any) -> str: +def _custom_preset_label(lora_message: Any) -> str | None: """Return a compact custom-radio label when ``use_preset`` is disabled. Reads ``spread_factor``, ``bandwidth``, and ``coding_rate`` from - *lora_message*. When all three are non-zero the label follows the same - compact convention used by MeshCore (e.g. ``"Custom SF8/BW62/CR6"``). - When any parameter is absent or zero, returns the bare ``"Custom"`` string - so the caller always gets a non-empty label. + *lora_message* and renders them in the **same bare ``SF/BW/CR`` form** that + MeshCore's ``_derive_modem_preset`` emits (e.g. ``"SF8/BW62/CR6"``), so one + radio configuration shows a single spelling across protocols (SPEC + Invariant IV — protocol parity). When any parameter is absent or zero the + configuration is unknown, so — matching MeshCore — this returns ``None`` + rather than inventing a label. Args: lora_message: A LoRa config protobuf message or compatible object. Returns: - A string starting with ``"Custom"``. + A ``"SF{sf}/BW{bw}/CR{cr}"`` string, or ``None`` when any parameter is + absent or zero. """ sf = getattr(lora_message, "spread_factor", None) or None bw = getattr(lora_message, "bandwidth", None) or None cr = getattr(lora_message, "coding_rate", None) or None if sf and bw and cr: - return f"Custom SF{int(sf)}/BW{int(bw)}/CR{int(cr)}" - return "Custom" + return f"SF{int(sf)}/BW{int(bw)}/CR{int(cr)}" + return None def _modem_preset(lora_message: Any) -> str | None: diff --git a/tests/test_interfaces_unit.py b/tests/test_interfaces_unit.py index 996357f..24ded18 100644 --- a/tests/test_interfaces_unit.py +++ b/tests/test_interfaces_unit.py @@ -616,7 +616,7 @@ class TestModemPreset: assert ifaces._modem_preset(msg) is None def test_use_preset_false_full_params(self): - """use_preset=False with all params returns compact custom label.""" + """use_preset=False with all params returns the bare custom label.""" msg = SimpleNamespace( DESCRIPTOR=None, use_preset=False, @@ -624,10 +624,10 @@ class TestModemPreset: bandwidth=62, coding_rate=6, ) - assert ifaces._modem_preset(msg) == "Custom SF8/BW62/CR6" + assert ifaces._modem_preset(msg) == "SF8/BW62/CR6" def test_use_preset_false_partial_params(self): - """use_preset=False with only some params returns bare 'Custom'.""" + """use_preset=False with only some params returns None (config unknown).""" msg = SimpleNamespace( DESCRIPTOR=None, use_preset=False, @@ -635,12 +635,12 @@ class TestModemPreset: bandwidth=62, # coding_rate absent ) - assert ifaces._modem_preset(msg) == "Custom" + assert ifaces._modem_preset(msg) is None def test_use_preset_false_no_params(self): - """use_preset=False with no radio params returns bare 'Custom'.""" + """use_preset=False with no radio params returns None (config unknown).""" msg = SimpleNamespace(DESCRIPTOR=None, use_preset=False) - assert ifaces._modem_preset(msg) == "Custom" + assert ifaces._modem_preset(msg) is None def test_use_preset_true_falls_through_to_enum(self): """use_preset=True is ignored; existing enum logic is used.""" @@ -678,44 +678,78 @@ class TestCustomPresetLabel: """Tests for :func:`interfaces._custom_preset_label`.""" def test_all_params_present(self): - """All three non-zero params produce the compact label.""" + """All three non-zero params produce the bare compact label.""" msg = SimpleNamespace(spread_factor=12, bandwidth=125, coding_rate=5) - assert ifaces._custom_preset_label(msg) == "Custom SF12/BW125/CR5" + assert ifaces._custom_preset_label(msg) == "SF12/BW125/CR5" def test_integer_conversion(self): """Float param values are cast to int in the label.""" msg = SimpleNamespace(spread_factor=8.0, bandwidth=62.5, coding_rate=6.0) - assert ifaces._custom_preset_label(msg) == "Custom SF8/BW62/CR6" + assert ifaces._custom_preset_label(msg) == "SF8/BW62/CR6" - def test_missing_coding_rate_returns_custom(self): - """Absent coding_rate yields the bare 'Custom' fallback.""" + def test_missing_coding_rate_returns_none(self): + """Absent coding_rate yields None (config unknown).""" msg = SimpleNamespace(spread_factor=8, bandwidth=62) - assert ifaces._custom_preset_label(msg) == "Custom" + assert ifaces._custom_preset_label(msg) is None - def test_missing_bandwidth_returns_custom(self): - """Absent bandwidth yields the bare 'Custom' fallback.""" + def test_missing_bandwidth_returns_none(self): + """Absent bandwidth yields None (config unknown).""" msg = SimpleNamespace(spread_factor=8, coding_rate=6) - assert ifaces._custom_preset_label(msg) == "Custom" + assert ifaces._custom_preset_label(msg) is None - def test_missing_spread_factor_returns_custom(self): - """Absent spread_factor yields the bare 'Custom' fallback.""" + def test_missing_spread_factor_returns_none(self): + """Absent spread_factor yields None (config unknown).""" msg = SimpleNamespace(bandwidth=62, coding_rate=6) - assert ifaces._custom_preset_label(msg) == "Custom" + assert ifaces._custom_preset_label(msg) is None def test_zero_bandwidth_treated_as_absent(self): - """A zero bandwidth value is treated as absent, yielding 'Custom'.""" + """A zero bandwidth value is treated as absent, yielding None.""" msg = SimpleNamespace(spread_factor=8, bandwidth=0, coding_rate=6) - assert ifaces._custom_preset_label(msg) == "Custom" + assert ifaces._custom_preset_label(msg) is None def test_zero_spread_factor_treated_as_absent(self): - """A zero spread_factor value is treated as absent, yielding 'Custom'.""" + """A zero spread_factor value is treated as absent, yielding None.""" msg = SimpleNamespace(spread_factor=0, bandwidth=62, coding_rate=6) - assert ifaces._custom_preset_label(msg) == "Custom" + assert ifaces._custom_preset_label(msg) is None def test_no_params_at_all(self): - """A message with none of the param attributes returns 'Custom'.""" + """A message with none of the param attributes returns None.""" msg = SimpleNamespace() - assert ifaces._custom_preset_label(msg) == "Custom" + assert ifaces._custom_preset_label(msg) is None + + +# --------------------------------------------------------------------------- +# Cross-protocol parity for custom radio-config labels +# --------------------------------------------------------------------------- + + +class TestCustomPresetLabelParity: + """Custom radio-config labels must be identical across protocols (Invariant IV). + + Regression guard for the format divergence in c8668a7: Meshtastic emitted a + ``"Custom "``-prefixed label while MeshCore's :func:`_derive_modem_preset` + emits the bare ``SF/BW/CR`` form (and ``None`` when unconfigured). The same + radio parameters must render the same string regardless of protocol, so the + dashboard never shows two spellings of one config. + """ + + def test_matches_meshcore_bare_format(self): + """Full params render the bare SF/BW/CR form, byte-identical to MeshCore.""" + from data.mesh_ingestor.protocols.meshcore.decode import _derive_modem_preset + + msg = SimpleNamespace(spread_factor=8, bandwidth=62, coding_rate=6) + label = ifaces._custom_preset_label(msg) + assert label == "SF8/BW62/CR6" + assert label == _derive_modem_preset(8, 62, 6) + + def test_missing_params_match_meshcore_none(self): + """Absent/zero params return ``None``, matching MeshCore's behaviour.""" + from data.mesh_ingestor.protocols.meshcore.decode import _derive_modem_preset + + assert ifaces._custom_preset_label(SimpleNamespace()) is None + assert ifaces._custom_preset_label(SimpleNamespace()) == _derive_modem_preset( + 0, 0, 0 + ) # ---------------------------------------------------------------------------