diff --git a/meshterm/context.py b/meshterm/context.py index 923b5a4..f14be7f 100644 --- a/meshterm/context.py +++ b/meshterm/context.py @@ -116,6 +116,11 @@ class AppContext: _active_transport: Optional[str] = field(default=None, init=False, repr=False) _active_address: Optional[str] = field(default=None, init=False, repr=False) _active_endpoint: Optional[str] = field(default=None, init=False, repr=False) + #: A live ``bleak.BLEDevice`` handed to :meth:`reconnect` by the flow that just heard the + #: companion advertise again. Preferred over the startup scan's handle when reopening the + #: link, because a device that dropped and came back is a *new* peripheral to the OS and + #: the older handle no longer names it. Typed ``object`` so ``bleak`` stays optional. + _ble_handle: Optional[object] = field(default=None, init=False, repr=False) unpair_on_exit: bool = field(default=False, init=False, repr=False) #: Set by the config editor just before it sends a reboot command, so the session's #: disconnect watcher can label the ensuing (expected) link drop as a reboot in @@ -464,12 +469,14 @@ class AppContext: # resolution, and no re-scan needed to reconnect to a known address. ble_address, ble_pin = self._resolve_ble_endpoint() if ble_address: - # When this session's scan discovered the device, hand its live BLEDevice to the + # When a scan has produced a live BLEDevice for this address, hand it to the # connection so bleak opens it directly instead of re-discovering the address (a # fresh internal scan that intermittently misses a slow-advertising companion). # A remembered/explicit address has no scan result and connects by address alone. + # A handle from the reconnect watch wins over the startup picker's: it was scanned + # seconds ago, after the drop, so it names the peripheral that is on the air *now*. selected = self.selected_device - ble_device = ( + ble_device = self._ble_handle or ( selected.ble_device if selected is not None and selected.is_ble and selected.address == ble_address else None @@ -614,31 +621,27 @@ class AppContext: if restored: self.log.info("channels: restored %d remembered channel(s) to the device", restored) - async def reconnect(self) -> None: - """Drop a lost device connection and rebuild it, restoring live services. + async def release_link(self) -> None: + """Tear down the dead connection and the services riding on it, keeping what to resume. - Called after the companion link is detected as gone (see - :func:`~meshterm.core.connection.is_connection_lost`). Tears down the dead - connection and the services riding on it, opens a fresh connection to the same - device, then restarts whatever was running before — so passive monitoring and chat - recording resume transparently across a replug. + The first half of :meth:`reconnect`, split out because it is also worth doing *before* + going looking for the device. Over Bluetooth that ordering is load-bearing: a companion + only advertises while nothing is connected to it, so a peripheral we are still holding + a link to — however dead we believe that link to be — stays off the air and can never + be found by the scan waiting for it to come back. Releasing first puts it back on the + air, and is harmless on the transports that don't care. - Raises: - Exception: If a new connection could not be opened (e.g. the device is still - absent); the caller can surface it and offer to retry. + What was running is captured *once* and held across retries: this stops the services, + so a later attempt would otherwise read the now-idle flags and restore nothing. The + intent is cleared only after :meth:`reconnect` actually succeeds. Idempotent — calling + it again with nothing connected is a no-op. """ - # Remember what was live so it can be restored after the reconnect — but capture it - # only *once*, and hold it across retries. The teardown below stops the services, so - # a second attempt (after the first failed because the device wasn't back yet) would - # otherwise read the now-idle flags and restore nothing. The intent is cleared only - # after a reconnect actually succeeds. if self._resume_intent is None: self._resume_intent = ( self._events is not None and self._events.active, self._monitor is not None and self._monitor.active, self._chat is not None and self._chat.active, ) - resume_events, resume_monitor, resume_chat = self._resume_intent # Release the stale hub/service subscriptions and discard the dead device. The # subscriptions are in-process (to the hub), so they survive the link drop and must @@ -661,6 +664,32 @@ class AppContext: if self._devstate is not None: self._devstate.reset() + async def reconnect(self, *, ble_device: Optional[object] = None) -> None: + """Drop a lost device connection and rebuild it, restoring live services. + + Called after the companion link is detected as gone (see + :func:`~meshterm.core.connection.is_connection_lost`). Tears down the dead + connection and the services riding on it, opens a fresh connection to the same + device, then restarts whatever was running before — so passive monitoring and chat + recording resume transparently across a replug. + + Args: + ble_device: The live ``bleak.BLEDevice`` the caller just scanned for this address, + when it has one (see :func:`~meshterm.core.discovery.find_ble_device`). Any + previously-held handle is dropped either way: the peripheral that comes back + from a power-cycle is a new one to the OS, so the scan result the session + opened with is not merely stale but wrong, and reusing it fails the connect + on Windows while the device sits there advertising. Passing ``None`` (serial, + TCP, or a BLE retry with nothing scanned) reconnects by address alone. + + Raises: + Exception: If a new connection could not be opened (e.g. the device is still + absent); the caller can surface it and offer to retry. + """ + self._ble_handle = ble_device + await self.release_link() + resume_events, resume_monitor, resume_chat = self._resume_intent + # Open a fresh connection (raises if the device still can't be reached, leaving the # remembered intent in place for the next attempt), then restart whatever was running # before it dropped and clear the intent now that we're back. @@ -672,6 +701,9 @@ class AppContext: if resume_chat: await self.chat.start() self._resume_intent = None + # The handle has been opened; it names a link that is now live rather than a device to + # go find, so the next drop starts from a clean slate and scans afresh. + self._ble_handle = None async def _node_name(self) -> str: """Return the connected device's own mesh node name, or ``""`` if unavailable.""" diff --git a/meshterm/core/discovery.py b/meshterm/core/discovery.py index e5d9cbd..457cb15 100644 --- a/meshterm/core/discovery.py +++ b/meshterm/core/discovery.py @@ -49,6 +49,12 @@ BLE_NAME_PREFIX = "MeshCore" #: advertisement, short enough not to stall the startup splash noticeably. BLE_SCAN_TIMEOUT_S = 5.0 +#: Default seconds to watch for one *known* companion's advertisement while waiting for it to +#: come back (see :func:`find_ble_device`). Deliberately longer than the startup scan: nothing +#: is waiting on it, the watch ends the instant the device is heard, and a companion that just +#: powered on can take several advertising intervals to be picked up by the OS. +BLE_PRESENCE_TIMEOUT_S = 8.0 + # USB vendor IDs frequently seen on MeshCore/Meshtastic companion hardware and the # USB-UART bridges they ship with. Used only to name, flag, and sort likely devices. KNOWN_LORA_VIDS: dict[int, str] = { @@ -406,6 +412,49 @@ async def discover_ble_devices(timeout: float = BLE_SCAN_TIMEOUT_S) -> list[Disc return devices +async def find_ble_device( + address: str, timeout: float = BLE_PRESENCE_TIMEOUT_S +) -> Optional[object]: + """Watch for one known companion's advertisement, returning its live ``BLEDevice``. + + The Bluetooth counterpart to :func:`~meshterm.core.connection.serial_port_present`, and it + answers the same question the reconnect flow asks of a serial port: *is the device back + yet?* Unlike a serial port there is no OS registry to consult — a peripheral exists only + while it is advertising — so the probe is a scan. ``find_device_by_address`` returns the + **moment** the address is heard rather than at the end of the window, so a generous + ``timeout`` costs nothing when the device is present and simply keeps listening when it + isn't. + + The returned handle matters as much as the answer. A companion that has been power-cycled + is a *new* peripheral to the OS: the ``BLEDevice`` captured by an earlier scan names a + connection endpoint that no longer resolves, and on Windows handing bleak that stale object + fails the connect outright. Answering the presence question with a freshly-scanned handle + means the reconnect that follows opens the device that is actually there (see + :meth:`~meshterm.context.AppContext.reconnect`). + + Never raises: a missing ``bleak``, a disabled adapter, or a driver hiccup reads as "not + advertising", which is the truthful answer for a device we cannot currently reach. + + Args: + address: The Bluetooth address to listen for (as reported by discovery). + timeout: Seconds to keep listening before giving up on this round. + + Returns: + The live ``bleak.BLEDevice`` if the address advertised within ``timeout``, else + ``None``. Typed ``object`` so ``bleak`` stays an optional dependency. + """ + try: + from bleak import BleakScanner + except ImportError: # bleak not installed → BLE simply unavailable + return None + + try: + return await BleakScanner.find_device_by_address(address, timeout=timeout) + except Exception as exc: # noqa: BLE001 - no adapter / OS Bluetooth off / driver hiccup + _log.debug("BLE presence scan for %s unavailable: %s", address, exc) + return None + + async def discover_all( *, ble: bool = True, ble_timeout: float = BLE_SCAN_TIMEOUT_S ) -> list[DiscoveredDevice]: diff --git a/meshterm/ui/menu.py b/meshterm/ui/menu.py index f73eb2a..f539ea0 100644 --- a/meshterm/ui/menu.py +++ b/meshterm/ui/menu.py @@ -63,6 +63,16 @@ _LIVENESS_POLL_S = 2.0 #: can't fire a false "disconnected" prompt (seconds). _LIVENESS_CONFIRM_S = 0.4 +#: How long each round of the reconnect flow listens for a dropped Bluetooth companion's +#: advertisement (seconds). The scan ends the instant the address is heard, so this is a +#: ceiling on a *miss*, not a cost paid on success — long enough to cover several advertising +#: intervals of a companion that has just powered on. +_BLE_PRESENCE_SCAN_S = 8.0 + +#: Pause between BLE presence scans (seconds). The scan window is the real wait; this only +#: stops the loop spinning when scanning fails instantly (no adapter, Bluetooth switched off). +_BLE_RESCAN_PAUSE_S = 1.0 + def _arm_exit_watchdog(seconds: float = _EXIT_WATCHDOG_S) -> None: """Guarantee the process terminates even if the exit path wedges. @@ -991,34 +1001,59 @@ async def _animate_dialog(session: TuiSession, dialog: ReconnectDialog) -> None: async def _auto_reconnect(ctx: AppContext, dialog: ReconnectDialog) -> None: """Poll for the device to return, reconnect when it does, then dismiss ``dialog``. - For serial, waits for the OS to re-enumerate the port the connection was opened on before - each attempt, so a reconnect is only tried once there's a device to reach. For Bluetooth - there is no cheap "is it back yet" probe short of a full scan, so it simply attempts a - reconnect each interval — ``create_ble`` connects directly by address and fails fast when - the peripheral isn't in range. Either way a failed attempt (the endpoint is back but the - board isn't ready yet, or it dropped again) just loops and retries. On the first success - the dialog's future is resolved with ``"reconnected"``, dismissing the popup. Runs until - it succeeds or the task is cancelled (the user quit). + The dead link is released up front (:meth:`~meshterm.context.AppContext.release_link`) so + the peripheral is free to advertise while we wait for it. Each transport then waits for its + own evidence that the device is reachable before touching the radio, so an attempt is only ever made against something that is actually there. Serial + waits for the OS to re-enumerate the port the connection was opened on. Bluetooth listens + for the companion's advertisement (:func:`~meshterm.core.discovery.find_ble_device`), which + is the same question asked of the only registry BLE has, and hands the resulting live + handle to :meth:`~meshterm.context.AppContext.reconnect` — a power-cycled peripheral is a + new one to the OS, so scanning is both how we notice it came back and how we get something + that can open it. Blind-retrying instead, as this used to, was doubly wrong: it drove + bleak's own internal address lookup into a device that wasn't advertising (a slow failure + that overlapped the window the device returned in) and, when one finally landed, it landed + with the startup scan's dead handle. TCP has no such probe and simply retries the connect, + which fails fast against an absent endpoint. + + A failed attempt (the device is back but the board isn't ready yet, or it dropped again) + just loops and retries. On the first success the dialog's future is resolved with + ``"reconnected"``, dismissing the popup. Runs until it succeeds or the task is cancelled + (the user quit). Args: ctx: The shared application context. dialog: The reconnect dialog to dismiss once the link is back. """ from ..core.connection import serial_port_present + from ..core.discovery import find_ble_device + + # Put the dead link down before going looking for the device. A Bluetooth companion we are + # still nominally connected to does not advertise, so the scan below would wait out a + # peripheral that is powered on and silent purely because we never let go of it. + await ctx.release_link() while True: - # Serial: wait for the port to re-appear before touching the radio. BLE/TCP (and an - # unknown/deferred serial port): skip the wait and just retry the reconnect itself — - # ``create_ble``/``create_tcp`` connect by address/endpoint and fail fast when absent. - if ctx.active_transport == "serial": + ble_device: object | None = None + transport = ctx.active_transport + if transport == "serial": port = ctx.active_port # Off the loop: the probe enumerates ports, which blocks long enough to stall # the very spinner this dialog is showing while it waits. if port is not None and not await asyncio.to_thread(serial_port_present, port): await asyncio.sleep(_LIVENESS_POLL_S) continue + elif transport == "ble": + address = ctx.active_address + if address is not None: + ble_device = await find_ble_device(address, timeout=_BLE_PRESENCE_SCAN_S) + if ble_device is None: + # The scan window is itself the wait — it just spent several seconds + # listening — so only a brief pause is needed to keep a scan that fails + # instantly (Bluetooth switched off mid-session) from spinning. + await asyncio.sleep(_BLE_RESCAN_PAUSE_S) + continue try: - await ctx.reconnect() + await ctx.reconnect(ble_device=ble_device) except Exception: # noqa: BLE001 - not reachable yet; keep the popup up and retry await asyncio.sleep(_LIVENESS_POLL_S) continue diff --git a/tests/test_disconnect.py b/tests/test_disconnect.py index 43ec13d..8b02116 100644 --- a/tests/test_disconnect.py +++ b/tests/test_disconnect.py @@ -16,12 +16,14 @@ from types import SimpleNamespace import pytest from rich.console import Console +from meshterm import context as context_module from meshterm.context import AppContext -from meshterm.core import connection +from meshterm.core import connection, discovery from meshterm.core.admin_store import AdminStore from meshterm.core.config import Settings from meshterm.core.connection import DeviceCommandError, is_connection_lost from meshterm.core.device_store import DeviceStore +from meshterm.core.discovery import DiscoveredDevice from meshterm.persistence.repository import Repository from meshterm.ui import menu @@ -743,7 +745,7 @@ async def test_handle_disconnect_auto_reconnects_when_port_returns( polls["n"] += 1 return polls["n"] >= 2 # absent on the first poll, back thereafter - async def fake_reconnect(self: AppContext) -> None: + async def fake_reconnect(self: AppContext, *, ble_device: object | None = None) -> None: self._device = object() # a fresh connection monkeypatch.setattr(connection, "serial_port_present", fake_present) @@ -957,3 +959,152 @@ async def test_disconnect_graceful_path_needs_no_force(monkeypatch) -> None: assert healthy.disconnected assert not healthy.force_stopped assert dev._mc is None + + +class _FakeBleDevice: + """A stand-in for a connected BLE :class:`Device` whose link has already dropped.""" + + transport = "ble" + + def __init__(self) -> None: + self.disconnected = False + + async def link_present(self) -> bool: + return False + + async def connect(self) -> None: + return None + + async def disconnect(self) -> None: + self.disconnected = True + + async def get_self_info(self) -> dict: + return {} + + async def get_device_info(self) -> dict: + return {} + + +def _ble_ctx(tmp_path: Path) -> AppContext: + """A context standing in for a live BLE session whose companion has just dropped.""" + ctx = _make_ctx(tmp_path) + ctx.mock = False + ctx._device = _FakeBleDevice() + ctx._active_transport = "ble" + ctx._active_address = "AA:BB:00:00:00:01" + return ctx + + +async def test_ble_reconnect_waits_for_the_advertisement_and_hands_over_the_fresh_handle( + tmp_path: Path, monkeypatch +) -> None: + """BLE reconnects on hearing the companion, opening it with the handle just scanned.""" + ctx = _ble_ctx(tmp_path) + fresh = object() # the live BLEDevice the scan produced + scans: list[str] = [] + + async def fake_find(address: str, timeout: float = 0.0) -> object | None: + scans.append(address) + return fresh if len(scans) >= 2 else None # silent on the first round, then heard + + opened: list[object | None] = [] + + async def fake_reconnect(self: AppContext, *, ble_device: object | None = None) -> None: + opened.append(ble_device) + self._device = _FakeBleDevice() + + monkeypatch.setattr(discovery, "find_ble_device", fake_find) + monkeypatch.setattr(AppContext, "reconnect", fake_reconnect) + monkeypatch.setattr(menu, "_BLE_RESCAN_PAUSE_S", 0.0) + monkeypatch.setattr(menu, "spinner_interval", lambda: 0.0) + + session = _FakeSession() + try: + quit_chosen = await asyncio.wait_for( + menu._handle_disconnect(ctx, session), timeout=2.0 + ) + assert quit_chosen is False # reconnected, not quit + # It waited for the device to be heard rather than blind-retrying the connect... + assert scans == [ctx._active_address, ctx._active_address] + assert len(opened) == 1 + # ...and reopened it with the handle that scan produced, not a stale one. + assert opened == [fresh] + finally: + ctx.repo.close() + + +async def test_ble_reconnect_releases_the_dead_link_before_scanning( + tmp_path: Path, monkeypatch +) -> None: + """The old link is put down first, so the peripheral is free to advertise again.""" + ctx = _ble_ctx(tmp_path) + dead = ctx._device + held_at_scan: list[bool] = [] + + async def fake_find(address: str, timeout: float = 0.0) -> object | None: + held_at_scan.append(ctx._device is not None) + return object() + + async def fake_reconnect(self: AppContext, *, ble_device: object | None = None) -> None: + self._device = _FakeBleDevice() + + monkeypatch.setattr(discovery, "find_ble_device", fake_find) + monkeypatch.setattr(AppContext, "reconnect", fake_reconnect) + monkeypatch.setattr(menu, "spinner_interval", lambda: 0.0) + + session = _FakeSession() + try: + await asyncio.wait_for(menu._handle_disconnect(ctx, session), timeout=2.0) + assert dead.disconnected # the dead companion was let go... + assert held_at_scan == [False] # ...before we ever listened for it + finally: + ctx.repo.close() + + +async def test_reconnect_prefers_the_freshly_scanned_ble_handle( + tmp_path: Path, monkeypatch +) -> None: + """A handle passed to reconnect opens the link; the startup scan's stale one is not reused.""" + address = "AA:BB:00:00:00:01" + ctx = _make_ctx(tmp_path) + ctx.mock = False + ctx.ble_override = address + stale = object() # what the picker's scan produced when the session opened + fresh = object() # what the reconnect flow just heard advertising + ctx.selected_device = DiscoveredDevice( + transport="ble", address=address, name="MeshCore-Homestead", ble_device=stale + ) + handles: list[object | None] = [] + + def fake_make_device(**kwargs: object): # noqa: ANN202 - a stub stands in for the radio + handles.append(kwargs.get("ble_device")) + return _FakeBleDevice() + + monkeypatch.setattr(context_module, "make_device", fake_make_device) + try: + await ctx.reconnect(ble_device=fresh) + assert handles == [fresh] # the freshly-scanned peripheral, never the stale handle + assert ctx._ble_handle is None # consumed by the connection it opened + + # With nothing scanned, the picker's handle is still the best available guess. + await ctx.reconnect() + assert handles == [fresh, stale] + finally: + ctx.repo.close() + + +async def test_release_link_is_idempotent_and_holds_the_resume_intent(tmp_path: Path) -> None: + """Releasing twice is harmless, and what was running is still restored on reconnect.""" + ctx = _make_ctx(tmp_path) + try: + await ctx.events.start() + await ctx.monitor.start() + await ctx.release_link() + assert not ctx.monitor.active # the services rode the link down with it + await ctx.release_link() # a second release finds nothing to do and says nothing + + await ctx.reconnect() + + assert ctx.events.active and ctx.monitor.active # the held intent survived both + finally: + await ctx.aclose()