mirror of
https://github.com/pe1hvh/meshcore-gui.git
synced 2026-10-31 01:42:04 +01:00
fix(channels): keep cached channel names when discovery fails
WHAT
_discover_channels() recorded no distinction between a slot the device
reported as empty and a slot it never answered for; both ended as an
absent entry, and the result was written over the cached channel names
wholesale. A device that returned ERROR on every slot therefore reduced
the cache to {0: "Public"}, permanently — those names are the only
channel source at startup while CHANNEL_CACHE_ENABLED is off.
The scan now records answered slots in a ChannelDiscoveryResult and
merges into the cached names: answered slots win, unanswered slots keep
their cached entry. Stale-key cleanup follows the same rule, replacing
the len(discovered) >= 2 heuristic.
WHY
The existing safety net guarded only the key cleanup, not the name write
above it. Recording answered slots addresses the defect itself rather
than adding a second heuristic, and lets one rule serve names and keys.
NOTES
No breaking changes. Cache JSON schema unchanged; _discover_channels()
keeps its signature. New config key CHANNEL_DISCOVERY_ABORT_THRESHOLD
(default 3) replaces the hardcoded consecutive-error limit.
This commit is contained in:
1 parent
16ed9f5469
commit
4b6d3a2d78
4 files changed
+276
-33
No files matched your search
@@ -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
|
||||
|
||||
+78
-32
@@ -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}")
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
}
|
||||
Reference in new issue
Block a user