diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f07d79..aca8f4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,68 @@ Format follows [Keep a Changelog](https://keepachangelog.com/) and [Semantic Ver --- +## [1.22.5] - 2026-08-31 โ€” Failed channel discovery no longer erases the channel list + +### Fixed +- ๐Ÿ›  **All channels except Public disappeared after a reboot** + (`ble/worker.py`, `services/channel_discovery.py`): `_discover_channels()` + persisted its result unconditionally. When the device returned ERROR on + every probed slot, the scan aborted, fell back to + `[{"idx": 0, "name": "Public"}]`, and wrote that single entry over the + cached channel names โ€” `DeviceCache.set_channel_names()` replaces the + whole mapping rather than merging into it. Because + `CHANNEL_CACHE_ENABLED` is off by default, those names are the only + channel source at startup, so the loss was permanent and took out both + the Messages and the Archive submenu, which share one `data['channels']` + source. The observed failure left nine cached channel *keys* intact + (written per index) beside a single cached *name*. + +### Added +- `services/channel_discovery.py` โ€” `ChannelDiscoveryResult` dataclass + plus `merge_with_cached_names()`, `name_map_to_channels()` and + `stale_key_indices()`. Pure data transformation, no I/O. + +### Changed +- Channel discovery now records which slots the device answered for and + merges that result into the cached names instead of replacing them. + A slot answered with a name is stored, a slot answered without one is + dropped (so channels deleted on the device do not come back), and a + slot that timed out or returned ERROR keeps its cached name. +- Stale-key cleanup replaces the `len(discovered) >= 2` heuristic with + the same rule: a cached key is only removed when the device + authoritatively reported its slot as vacant. +- The `Public`-only fallback now applies solely when the device confirmed + nothing *and* no cached names exist. +- `CHANNEL_DISCOVERY_ABORT_THRESHOLD` (default 3) replaces the hardcoded + consecutive-error limit in `config.py`. + +### Impact +- No public method signature changed; `_discover_channels()` keeps its + `async def (self) -> None` contract and the cache JSON schema is + untouched. +- A failed scan now leaves `SharedData.channels` populated from cache + rather than collapsing it to Public, so the drawer submenus, the send + selector, the channel backup (which reads the same cached names) and + hashtag key derivation all survive an unresponsive device. +- Channel deletion on the device still propagates: an empty slot the + device confirms is authoritative and removes both name and key. +- Decryption keys are no longer dropped on a failed scan, so messages on + channels the device did not confirm remain decodable. +- `VERSION` bumped `1.22.4` โ†’ `1.22.5` (PATCH: backwards-compatible + bugfix). + +### Rationale +The existing safety net carried the right intent โ€” its comment reads +"so a transient discovery failure does not wipe the cache" โ€” but guarded +only the key cleanup, leaving the name write above it exposed. The +underlying defect is that the code could not tell "the device says this +slot is empty" apart from "the device said nothing"; both ended as an +absent entry in `discovered`. Recording answered slots makes that +distinction explicit and lets one rule serve both the names and the +keys, rather than adding a second heuristic on top of the first. + +--- + ## [1.22.4] - 2026-08-16 โ€” Main-page send selector follows the active channel ### Fixed diff --git a/meshcore_gui/ble/worker.py b/meshcore_gui/ble/worker.py index 585dc38..fd0852e 100644 --- a/meshcore_gui/ble/worker.py +++ b/meshcore_gui/ble/worker.py @@ -41,6 +41,7 @@ import meshcore_gui.config as _config from meshcore_gui.config import ( DEFAULT_TIMEOUT, CHANNEL_CACHE_ENABLED, + CHANNEL_DISCOVERY_ABORT_THRESHOLD, CONTACT_REFRESH_SECONDS, MAX_CHANNELS, RECONNECT_BASE_DELAY, @@ -58,6 +59,12 @@ from meshcore_gui.services.bbs_service import BbsCommandHandler, BbsService from meshcore_gui.services.bbs_config_store import BbsConfigStore from meshcore_gui.services.bot_config_store import BotConfigStore from meshcore_gui.services.cache import DeviceCache +from meshcore_gui.services.channel_discovery import ( + ChannelDiscoveryResult, + merge_with_cached_names, + name_map_to_channels, + stale_key_indices, +) from meshcore_gui.services.dedup import DualDeduplicator from meshcore_gui.services.device_identity import write_device_identity from meshcore_gui.services.pin_store import PinStore @@ -553,10 +560,18 @@ class _BaseWorker(abc.ABC): # โ”€โ”€ channel discovery โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ async def _discover_channels(self) -> None: - """Discover channels and load their keys from the device.""" + """Discover channels and load their keys from the device. + + The scan records which slots the device actually answered for. + Only those slots carry authority over the persisted channel + names: an unanswered slot leaves its cached name untouched, so a + device that is unresponsive โ€” or is not the expected companion + node โ€” can no longer erase the channel list. + """ pfx = self._log_prefix self.shared.set_status("๐Ÿ”„ Discovering channels...") - discovered: List[Dict] = [] + result = ChannelDiscoveryResult() + discovered = result.channels cached_keys = self._cache.get_channel_keys() confirmed: list[str] = [] @@ -570,15 +585,22 @@ class _BaseWorker(abc.ABC): if payload is None: consecutive_errors += 1 - if consecutive_errors >= 3: + if consecutive_errors >= CHANNEL_DISCOVERY_ABORT_THRESHOLD: debug_print( f"Channel discovery: {consecutive_errors} consecutive " f"empty slots at idx {idx}, stopping" ) + # Higher slots were never probed โ€” the scan result is + # partial and must not be treated as authoritative. + result.complete = False break continue consecutive_errors = 0 + # The device responded for this slot, so its verdict is + # authoritative โ€” both for an active channel and for an + # empty slot (a channel deleted on the device). + result.answered.add(idx) name = payload.get("name") or payload.get("channel_name") or "" if not name.strip(): debug_print(f"Channel [{idx}]: response OK but no name โ€” skipping (undefined slot)") @@ -605,45 +627,69 @@ class _BaseWorker(abc.ABC): await asyncio.sleep(0.3) - if not discovered: - discovered = [{"idx": 0, "name": "Public"}] - print(f"{pfx}: โš ๏ธ No channels discovered, using default Public channel") + # Merge with the cached names before publishing anything. Slots + # the device answered for are taken from the scan; slots it + # stayed silent about keep their cached name. A scan that + # confirmed nothing therefore leaves the channel list exactly as + # it was instead of collapsing it to a bare "Public". + cached_names = self._cache.get_channel_names() + merged_names = merge_with_cached_names(result, cached_names) + + if result.is_empty: + retained = len(merged_names) + if retained: + print( + f"{pfx}: โš ๏ธ Channel discovery got no response โ€” " + f"keeping {retained} cached channel(s)" + ) + else: + merged_names = {0: "Public"} + print( + f"{pfx}: โš ๏ธ No channels discovered and no cache, " + f"using default Public channel" + ) + elif not result.complete: + debug_print( + f"Channel discovery incomplete โ€” {len(result.answered)} slot(s) " + f"confirmed, cached names retained for the rest" + ) + + discovered = name_map_to_channels(merged_names) self._channels = discovered self.shared.set_channels(discovered) # Always persist channel names regardless of CHANNEL_CACHE_ENABLED, # so the GUI can display them immediately on next startup. - self._cache.set_channel_names({ch["idx"]: ch["name"] for ch in discovered}) + self._cache.set_channel_names(merged_names) if CHANNEL_CACHE_ENABLED: self._cache.set_channels(discovered) debug_print("Channel list cached to disk") # Post-discovery sync: drop stale cache + decoder entries for slots - # that are no longer active (no name) on the device. Without this, - # firmware-initialised slots and historical entries accumulate - # forever โ€” leading to backups containing 100 channels when the - # device only has 3, and to brute-force decoder pollution that has - # been observed to mis-route messages to vacated slot indices. - # Safety net: only sync when at least one non-Public channel was - # discovered, so a transient discovery failure (which falls back to - # ``[{"idx": 0, "name": "Public"}]`` above) does not wipe the cache. - if len(discovered) >= 2: - valid_indices = {int(ch["idx"]) for ch in discovered} - removed = 0 - for idx_str in list(self._cache.get_channel_keys()): - try: - idx = int(idx_str) - except (ValueError, TypeError): - continue - if idx not in valid_indices: - self._cache.remove_channel_key(idx) - self._decoder.remove_channel_key(idx) - removed += 1 - if removed: - print( - f"{pfx}: ๐Ÿงน Cleaned {removed} stale channel key(s) " - f"from cache and decoder" - ) + # the device reported as vacant. Without this, firmware-initialised + # slots and historical entries accumulate forever โ€” leading to + # backups containing 100 channels when the device only has 3, and to + # brute-force decoder pollution that has been observed to mis-route + # messages to vacated slot indices. + # Only slots the device actually answered for are eligible: a slot + # that timed out or returned ERROR says nothing about whether its + # channel still exists, so its key is left in place. + cached_key_indices: Set[int] = set() + for idx_str in self._cache.get_channel_keys(): + try: + cached_key_indices.add(int(idx_str)) + except (ValueError, TypeError): + continue + + stale = stale_key_indices(result, cached_key_indices) + for idx in stale: + self._cache.remove_channel_key(idx) + self._decoder.remove_channel_key(idx) + if stale: + print( + f"{pfx}: ๐Ÿงน Cleaned {len(stale)} stale channel key(s) " + f"from cache and decoder" + ) print(f"{pfx}: Channels discovered: {[c['name'] for c in discovered]}") print(f"{pfx}: PacketDecoder ready โ€” has_keys={self._decoder.has_keys}") diff --git a/meshcore_gui/config.py b/meshcore_gui/config.py index 3b946c3..78ad65f 100644 --- a/meshcore_gui/config.py +++ b/meshcore_gui/config.py @@ -25,7 +25,7 @@ from typing import Any, Dict, List # ============================================================================== -VERSION: str = "1.22.4" +VERSION: str = "1.22.5" # ============================================================================== @@ -290,6 +290,13 @@ MAX_CHANNELS: int = 255 # regardless of this setting. CHANNEL_CACHE_ENABLED: bool = False +# Number of consecutive unanswered channel slots after which discovery +# aborts. A slot counts as unanswered when the device times out or +# returns ERROR; slots the device answers for (even empty ones) reset +# the counter. Raising this makes discovery more patient with a slow +# device at the cost of a longer startup scan. +CHANNEL_DISCOVERY_ABORT_THRESHOLD: int = 3 + # ============================================================================== # PUBLIC API diff --git a/meshcore_gui/services/channel_discovery.py b/meshcore_gui/services/channel_discovery.py new file mode 100644 index 0000000..cf29155 --- /dev/null +++ b/meshcore_gui/services/channel_discovery.py @@ -0,0 +1,128 @@ +""" +Channel discovery result model and cache-merge logic. + +A channel scan walks the device slot by slot. Each slot can end in one +of three states, and the difference between them is what protects the +persisted channel names: + +- **Answered with a name** โ€” the device confirmed an active channel. +- **Answered without a name** โ€” the device confirmed the slot is empty. + This is authoritative: a channel deleted on the device must disappear + from the cache as well. +- **Not answered** โ€” the request timed out, returned ``ERROR``, or the + slot was never reached because the scan aborted early. Nothing is + known about this slot, so any cached name for it must be kept. + +Treating the last two cases alike is what allowed an unresponsive (or +wrong) device to overwrite the cached channel names with a bare +``{0: "Public"}``. Because ``CHANNEL_CACHE_ENABLED`` is off by default, +those names are the only channel source at startup, so the loss was +permanent. + +No GUI, BLE or filesystem dependencies โ€” pure data transformation, safe +to import from any layer. +""" + +from dataclasses import dataclass, field +from typing import Dict, List, Set + + +@dataclass +class ChannelDiscoveryResult: + """Outcome of a single channel-discovery scan. + + Attributes: + channels: Active channels found on the device, as + ``{"idx": int, "name": str}`` dicts in ascending slot order. + answered: Slot indices for which the device returned a usable + response, whether or not that slot held a channel. Only + these indices carry authority over the cache. + complete: True when the scan ran to a natural end. False when it + aborted early after consecutive unanswered slots, which means + higher indices were never probed. + """ + + channels: List[Dict] = field(default_factory=list) + answered: Set[int] = field(default_factory=set) + complete: bool = True + + @property + def is_empty(self) -> bool: + """True when the device confirmed nothing at all. + + Distinguishes a genuinely channel-less device from a device that + never answered: the former has entries in ``answered``, the + latter does not. + """ + return not self.answered + + def name_map(self) -> Dict[int, str]: + """Return the discovered channels as an ``{idx: name}`` mapping.""" + return {int(ch["idx"]): ch["name"] for ch in self.channels} + + +def merge_with_cached_names( + result: ChannelDiscoveryResult, + cached_names: Dict[int, str], +) -> Dict[int, str]: + """Merge a discovery result with the previously cached channel names. + + The device wins for every slot it answered for; the cache is retained + for every slot it did not. A slot the device reported as empty is + dropped from the result, so channels deleted on the device do not + reappear on the next startup. + + Args: + result: Outcome of the scan. + cached_names: Previously persisted ``{idx: name}`` mapping. + + Returns: + A new ``{idx: name}`` mapping safe to persist. Never smaller + than the set of slots the device actually answered for. + """ + merged: Dict[int, str] = { + idx: name + for idx, name in cached_names.items() + if idx not in result.answered + } + merged.update(result.name_map()) + return merged + + +def name_map_to_channels(names: Dict[int, str]) -> List[Dict]: + """Convert an ``{idx: name}`` mapping to the channel-dict list format. + + Args: + names: Mapping of slot index to channel name. + + Returns: + List of ``{"idx": int, "name": str}`` dicts in ascending index + order โ€” the shape expected by ``SharedData.set_channels()``. + """ + return [{"idx": idx, "name": names[idx]} for idx in sorted(names)] + + +def stale_key_indices( + result: ChannelDiscoveryResult, + cached_key_indices: Set[int], +) -> Set[int]: + """Return cached key indices the device confirmed as vacant. + + A key is only stale when the device answered for its slot and + reported no channel there. Indices the device stayed silent about + are left alone, so a failed scan cannot strip decryption keys for + channels that are still configured. + + Args: + result: Outcome of the scan. + cached_key_indices: Slot indices currently holding a cached key. + + Returns: + Indices whose cached key and decoder entry should be removed. + """ + active = set(result.name_map()) + return { + idx + for idx in cached_key_indices + if idx in result.answered and idx not in active + }