From 8c59396ec80a30ea9b7ab7fb579160fe3f69d571 Mon Sep 17 00:00:00 2001 From: l5y <220195275+l5yth@users.noreply.github.com> Date: Sun, 5 Apr 2026 13:46:04 +0200 Subject: [PATCH] fix: derive channel probe bound from device max_channels (#701) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the hardcoded max_idx=8 parameter on _ensure_channel_names with a DEVICE_INFO query (send_device_query → max_channels) so the full range of configured channels is always probed regardless of firmware variant. Falls back to _CHANNEL_PROBE_FALLBACK_MAX (32) when the query fails or the device returns an older firmware that omits max_channels. Also removes always=True from the warning-severity channel failure log (redundant — only debug-severity is gated behind the DEBUG flag) and adds a deferred-import comment in _ensure_channel_names. --- data/mesh_ingestor/protocols/meshcore.py | 42 ++++++-- tests/test_provider_unit.py | 122 ++++++++++++++++------- 2 files changed, 120 insertions(+), 44 deletions(-) diff --git a/data/mesh_ingestor/protocols/meshcore.py b/data/mesh_ingestor/protocols/meshcore.py index ac34dea..215239d 100644 --- a/data/mesh_ingestor/protocols/meshcore.py +++ b/data/mesh_ingestor/protocols/meshcore.py @@ -442,32 +442,55 @@ class _MeshcoreInterface: thread.join(timeout=5.0) +# Fallback upper bound for channel index probing when the device query fails +# or returns an older firmware version that omits ``max_channels``. +_CHANNEL_PROBE_FALLBACK_MAX = 32 + # --------------------------------------------------------------------------- # Channel name resolution # --------------------------------------------------------------------------- -async def _ensure_channel_names(mc: object, max_idx: int = 8) -> None: +async def _ensure_channel_names(mc: object) -> None: """Probe channel names from the device and populate the channel cache. - Iterates indices 0 through *max_idx* - 1, requesting each via + Queries the device for its authoritative channel count via + :meth:`~meshcore.MeshCore.commands.send_device_query` (``max_channels`` + field of the ``DEVICE_INFO`` response), then iterates every index from 0 + through ``max_channels - 1``, requesting each via :meth:`~meshcore.MeshCore.commands.get_channel`. The responses arrive as :attr:`~meshcore.EventType.CHANNEL_INFO` events and are registered into the shared channel cache via :func:`~data.mesh_ingestor.channels.register_channel`. - Probes every index in ``range(max_idx)`` without early-stopping on - consecutive ``ERROR`` responses, so sparse configurations (e.g. slots 0 - and 5 configured, slots 1-4 empty) are handled correctly. Only a hard - exception (connection loss, timeout) aborts the loop early. + Falls back to a probe bound of :data:`_CHANNEL_PROBE_FALLBACK_MAX` when the + device query fails or returns an older firmware that omits ``max_channels``. + + Probes every index without early-stopping on ``ERROR`` responses, so sparse + configurations (e.g. slots 0 and 5 configured, slots 1–4 empty) are handled + correctly. Only a hard exception (connection loss, timeout) aborts the loop. Parameters: mc: Connected :class:`~meshcore.MeshCore` instance. - max_idx: Upper bound (exclusive) for channel indices to probe. - MeshCore companion firmware typically configures at most 8 - channels (indices 0–7). """ + # Deferred — see _make_event_handlers for the circular-dependency note. from .. import channels as _channels + max_idx = _CHANNEL_PROBE_FALLBACK_MAX + try: + dev_evt = await mc.commands.send_device_query() + if dev_evt.type == EventType.DEVICE_INFO: + reported = (dev_evt.payload or {}).get("max_channels") + if isinstance(reported, int) and reported > 0: + max_idx = reported + except Exception as exc: + config._debug_log( + "Device query failed; using fallback channel probe bound", + context="meshcore.channels", + severity="warning", + fallback_max=max_idx, + error=str(exc), + ) + for idx in range(max_idx): try: evt = await mc.commands.get_channel(idx) @@ -840,7 +863,6 @@ async def _run_meshcore( "Failed to fetch channel names", context="meshcore.channels", severity="warning", - always=True, error=str(exc), ) diff --git a/tests/test_provider_unit.py b/tests/test_provider_unit.py index 072e3d5..f994456 100644 --- a/tests/test_provider_unit.py +++ b/tests/test_provider_unit.py @@ -35,6 +35,7 @@ from data.mesh_ingestor.connection import parse_tcp_target # noqa: E402 - path from data.mesh_ingestor.protocols.meshcore import ( # noqa: E402 - path setup EventType, MeshcoreProvider, + _CHANNEL_PROBE_FALLBACK_MAX, _MeshcoreInterface, _contact_to_node_dict, _derive_message_id, @@ -1186,16 +1187,26 @@ def test_derive_modem_preset_none_on_missing(): # --------------------------------------------------------------------------- -def _make_fake_mc_for_channels(channel_map: dict): +def _make_fake_mc_for_channels(channel_map: dict, max_channels: int | None = None): """Build a minimal fake MeshCore instance for channel-probe tests. Parameters: - channel_map: Mapping of channel_idx → channel_name string, or - ``None`` to simulate an ERROR response for that index. + channel_map: Mapping of channel_idx → channel_name string; absent keys + simulate an ERROR response for that index. + max_channels: Value to return in the DEVICE_INFO ``max_channels`` field. + When ``None`` the device query returns an ERROR so the fallback bound + is used. """ - import asyncio class _FakeCommands: + async def send_device_query(self): + if max_channels is None: + return types.SimpleNamespace(type=EventType.ERROR, payload={}) + return types.SimpleNamespace( + type=EventType.DEVICE_INFO, + payload={"max_channels": max_channels}, + ) + async def get_channel(self, idx): name = channel_map.get(idx) if name is None: @@ -1207,8 +1218,7 @@ def _make_fake_mc_for_channels(channel_map: dict): payload={"channel_idx": idx, "channel_name": name}, ) - mc = types.SimpleNamespace(commands=_FakeCommands()) - return mc + return types.SimpleNamespace(commands=_FakeCommands()) def test_ensure_channel_names_populates_cache(monkeypatch): @@ -1220,8 +1230,8 @@ def test_ensure_channel_names_populates_cache(monkeypatch): _channels._reset_channel_cache() monkeypatch.setattr(_mod.config, "_debug_log", lambda *_a, **_k: None) - fake_mc = _make_fake_mc_for_channels({0: "LongFast", 1: "Chat"}) - asyncio.run(_ensure_channel_names(fake_mc, max_idx=4)) + fake_mc = _make_fake_mc_for_channels({0: "LongFast", 1: "Chat"}, max_channels=4) + asyncio.run(_ensure_channel_names(fake_mc)) assert _channels.channel_name(0) == "LongFast" assert _channels.channel_name(1) == "Chat" @@ -1238,8 +1248,8 @@ def test_ensure_channel_names_tolerates_error_response(monkeypatch): monkeypatch.setattr(_mod.config, "_debug_log", lambda *_a, **_k: None) # Index 0 returns ERROR; index 1 returns a valid name. - fake_mc = _make_fake_mc_for_channels({1: "Chat"}) - asyncio.run(_ensure_channel_names(fake_mc, max_idx=4)) + fake_mc = _make_fake_mc_for_channels({1: "Chat"}, max_channels=4) + asyncio.run(_ensure_channel_names(fake_mc)) assert _channels.channel_name(0) is None assert _channels.channel_name(1) == "Chat" @@ -1260,25 +1270,9 @@ def test_ensure_channel_names_probes_all_indices_on_sparse_config(monkeypatch): monkeypatch.setattr(_mod.config, "_debug_log", lambda *_a, **_k: None) # Slots 0-4 return ERROR; slot 5 is configured. - channel_map = {5: "Admin"} - - class _FakeCommands: - async def get_channel(self, idx): - name = channel_map.get(idx) - if name is None: - return types.SimpleNamespace( - type=EventType.ERROR, payload={"reason": "not_found"} - ) - return types.SimpleNamespace( - type=EventType.CHANNEL_INFO, - payload={"channel_idx": idx, "channel_name": name}, - ) - - asyncio.run( - _ensure_channel_names( - types.SimpleNamespace(commands=_FakeCommands()), max_idx=8 - ) - ) + # Device reports max_channels=8 so all indices are probed. + fake_mc = _make_fake_mc_for_channels({5: "Admin"}, max_channels=8) + asyncio.run(_ensure_channel_names(fake_mc)) # Slot 5 must be registered despite the preceding empty slots. assert _channels.channel_name(5) == "Admin" @@ -1301,15 +1295,16 @@ def test_ensure_channel_names_stops_on_exception(monkeypatch): ) class _FakeCommands: + async def send_device_query(self): + return types.SimpleNamespace( + type=EventType.DEVICE_INFO, payload={"max_channels": 4} + ) + async def get_channel(self, idx): raise OSError("serial port disconnected") # Must complete without raising. - asyncio.run( - _ensure_channel_names( - types.SimpleNamespace(commands=_FakeCommands()), max_idx=4 - ) - ) + asyncio.run(_ensure_channel_names(types.SimpleNamespace(commands=_FakeCommands()))) assert "warning" in logged _channels._reset_channel_cache() @@ -1333,6 +1328,59 @@ def test_on_channel_info_handler_registers_channel(monkeypatch): _channels._reset_channel_cache() +def test_ensure_channel_names_uses_device_max_channels(monkeypatch): + """max_channels from DEVICE_INFO must bound the probe, not the fallback.""" + import asyncio + import data.mesh_ingestor.protocols.meshcore as _mod + import data.mesh_ingestor.channels as _channels + + _channels._reset_channel_cache() + monkeypatch.setattr(_mod.config, "_debug_log", lambda *_a, **_k: None) + + probed: list[int] = [] + + class _FakeCommands: + async def send_device_query(self): + # Device reports exactly 3 channels. + return types.SimpleNamespace( + type=EventType.DEVICE_INFO, payload={"max_channels": 3} + ) + + async def get_channel(self, idx): + probed.append(idx) + return types.SimpleNamespace(type=EventType.ERROR, payload={}) + + asyncio.run(_ensure_channel_names(types.SimpleNamespace(commands=_FakeCommands()))) + + assert probed == [0, 1, 2] + _channels._reset_channel_cache() + + +def test_ensure_channel_names_falls_back_when_device_query_fails(monkeypatch): + """When DEVICE_INFO is unavailable the fallback bound must be used.""" + import asyncio + import data.mesh_ingestor.protocols.meshcore as _mod + import data.mesh_ingestor.channels as _channels + + _channels._reset_channel_cache() + monkeypatch.setattr(_mod.config, "_debug_log", lambda *_a, **_k: None) + + probed: list[int] = [] + + class _FakeCommands: + async def send_device_query(self): + return types.SimpleNamespace(type=EventType.ERROR, payload={}) + + async def get_channel(self, idx): + probed.append(idx) + return types.SimpleNamespace(type=EventType.ERROR, payload={}) + + asyncio.run(_ensure_channel_names(types.SimpleNamespace(commands=_FakeCommands()))) + + assert len(probed) == _CHANNEL_PROBE_FALLBACK_MAX + _channels._reset_channel_cache() + + # --------------------------------------------------------------------------- # _process_contacts # --------------------------------------------------------------------------- @@ -1656,6 +1704,12 @@ def _make_fake_meshcore_mod( ) class _FakeCommands: + async def send_device_query(self): + # Return minimal DEVICE_INFO — channel probing is not under test here. + return types.SimpleNamespace( + type=EventType.DEVICE_INFO, payload={"max_channels": 1} + ) + async def get_channel(self, idx): # Return ERROR for all channels — channel probing is not under test here. return types.SimpleNamespace(type=EventType.ERROR, payload={})