`ruff format .` across the repo: 153 files, no behaviour change. This is the second gate CONTRIBUTING.md promises, and until now it reported 154 files needing reformat — so a first contributor ran the command the guide told them to run and could not tell their own diff from the background. Landed on its own, with nothing else in it, because a whole-repo reformat is only safe to skip past in blame if it is provably nothing but layout. It was checked that way rather than assumed: every changed file was formatted in memory and compared by full `ast.dump()`, and 152 of 153 come out structurally identical (the one exception was a docstring opening on a quote character, fixed in the commit before this one). The string and comment token streams were diffed separately, so a quote flip or an edited comment could not hide inside a re-layout — five quote-style normalisations and sixteen implicit-concatenation joins, all value-identical, and one commented-out line gaining the space after its `#`. `meshterm specimen` is byte-identical on both platforms, and the suite is 1888 green, gallery gate and `test_theme16` included. What actually changed, in order of how much of the diff it is: wrapped calls and signatures rejoin onto one line where they fit inside the hundred columns; the magic trailing comma expands what it is there to expand; complex slices gain their spaces (`[len(noise) :]`); a few redundant parens go. The tables that are laid out to be read were fenced in the previous commit and are untouched here. The aligned trailing-comment column is gone from about ninety lanes that were not fenced — including the `heat.*` ladder in `MESH_THEME`, which cannot be fenced where it sits. Every comment stays on the entry it describes, so nothing there reads wrong; it just no longer lines up. Add this commit to `.git-blame-ignore-revs`, which follows immediately. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
308 lines
12 KiB
Python
308 lines
12 KiB
Python
"""Tests for the BLE connect path's ownership of its meshcore client.
|
|
|
|
The bug these pin down: ``MeshCore.create_ble`` builds a client, calls ``connect()`` on it,
|
|
and returns it *only on success*. A connect that raises — which is exactly how a
|
|
PIN-protected companion answers an unbonded notify-subscribe — therefore left the client,
|
|
and the bleak link it had already opened, orphaned inside the library. MeshTerm's own
|
|
``disconnect`` was a no-op (``_mc`` was never assigned), Windows held the ACL link for the
|
|
life of the process, and the peripheral, still believing it had a peer, stopped advertising.
|
|
The PIN dialog that opened next then asked for a code it could no longer deliver.
|
|
|
|
Owning the client is only half of it, and the fakes here model the other half. The library's
|
|
``ConnectionManager.disconnect`` closes its transport only ``if self._is_connected`` — a flag
|
|
set *after* ``connection.connect()`` returns. A connect that raised never set it, so a
|
|
graceful ``mc.disconnect()`` walks away from a live ``BleakClient`` believing there is
|
|
nothing to close. That is why ``_discard_meshcore`` also closes the transport directly, and
|
|
why the assertions below are about the *transport*, not about ``mc.disconnect`` being called.
|
|
|
|
Everything runs against stand-ins for ``meshcore.MeshCore`` and ``meshcore.BLEConnection``;
|
|
no radio, no real address, and no real pairing code is involved.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import asyncio
|
|
|
|
import pytest
|
|
|
|
from meshterm.core import connection as conn_mod
|
|
from meshterm.core.connection import MeshCoreDevice
|
|
|
|
# A stand-in address. Deliberately not a real companion's: a test that names one turns a
|
|
# personal device into repository content, and nothing here needs hardware to run.
|
|
_ADDR = "00:11:22:33:44:55"
|
|
|
|
|
|
class _FakeTransport:
|
|
"""Stands in for ``meshcore.BLEConnection`` — what actually holds the bleak client.
|
|
|
|
``link_open`` is the thing that matters: it stands for the OS-level link that, left up,
|
|
makes the peripheral believe it still has a peer and stop advertising.
|
|
"""
|
|
|
|
last: _FakeTransport | None = None
|
|
|
|
def __init__(self, address=None, device=None, pin=None) -> None:
|
|
self.address = address
|
|
self.device = device
|
|
self.pin = pin
|
|
self.link_open = False
|
|
self.disconnect_calls = 0
|
|
_FakeTransport.last = self
|
|
|
|
async def disconnect(self) -> None:
|
|
self.disconnect_calls += 1
|
|
self.link_open = False
|
|
|
|
|
|
class _FakeConnectionManager:
|
|
"""Mirrors the library's guard: it will not close a transport it never saw connect."""
|
|
|
|
def __init__(self, connection: _FakeTransport) -> None:
|
|
self.connection = connection
|
|
self._is_connected = False
|
|
|
|
async def disconnect(self) -> None:
|
|
if self._is_connected: # the trap: False whenever connection.connect() raised
|
|
await self.connection.disconnect()
|
|
self._is_connected = False
|
|
|
|
|
|
class _FakeMeshCore:
|
|
"""Stands in for the ``meshcore.MeshCore`` *class*, one instance per connect attempt.
|
|
|
|
``outcomes`` drives each successive ``connect()``: an exception instance is raised (with
|
|
the link already up, as the real GATT failure leaves it), ``"none"`` returns ``None``
|
|
(transport up, no identity reply), ``"slow"`` hangs for a caller to cancel, and anything
|
|
else is returned as a successful handshake.
|
|
"""
|
|
|
|
#: Every instance built during a test, in order (a retry builds a second one).
|
|
built: list[_FakeMeshCore] = []
|
|
#: Consumed one entry per connect attempt.
|
|
outcomes: list = []
|
|
|
|
def __init__(self, cx, *, default_timeout=None, auto_reconnect=False, **kwargs) -> None:
|
|
self.cx = cx
|
|
self.connection_manager = _FakeConnectionManager(cx)
|
|
self.default_timeout = default_timeout
|
|
self.auto_reconnect = auto_reconnect
|
|
self.graceful_calls = 0
|
|
self.stop_calls = 0
|
|
type(self).built.append(self)
|
|
|
|
async def connect(self):
|
|
outcome = type(self).outcomes.pop(0)
|
|
self.cx.link_open = True # bleak is connected by the time anything below fails
|
|
if isinstance(outcome, BaseException):
|
|
# The GATT subscribe fails here — after the link is up and *before* the manager
|
|
# records it. Nothing downstream will close it unless someone reaches the cx.
|
|
raise outcome
|
|
if outcome == "slow":
|
|
await asyncio.sleep(30) # a handshake the caller's wait_for will cancel
|
|
self.connection_manager._is_connected = True
|
|
return None if outcome == "none" else {"ok": True}
|
|
|
|
async def disconnect(self) -> None:
|
|
self.graceful_calls += 1
|
|
await self.connection_manager.disconnect()
|
|
|
|
def stop(self) -> None:
|
|
self.stop_calls += 1
|
|
|
|
@classmethod
|
|
def reset(cls, *outcomes) -> None:
|
|
cls.built = []
|
|
cls.outcomes = list(outcomes)
|
|
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def _fake_meshcore(monkeypatch: pytest.MonkeyPatch):
|
|
"""Make ``from meshcore import BLEConnection`` inside the connect path resolve to a fake."""
|
|
import sys
|
|
import types
|
|
|
|
module = types.ModuleType("meshcore")
|
|
module.BLEConnection = _FakeTransport
|
|
module.MeshCore = _FakeMeshCore
|
|
monkeypatch.setitem(sys.modules, "meshcore", module)
|
|
_FakeTransport.last = None
|
|
yield
|
|
|
|
|
|
def _device(pin: str | None = None) -> MeshCoreDevice:
|
|
return MeshCoreDevice(transport="ble", address=_ADDR, pin=pin, connect_timeout=5.0)
|
|
|
|
|
|
class _AuthError(Exception):
|
|
"""A stand-in for the GATT failure a PIN-protected companion answers with."""
|
|
|
|
|
|
def test_a_raising_connect_releases_the_link_it_left_open() -> None:
|
|
"""THE bug: a connect that raises must not strand an open link.
|
|
|
|
``create_ble`` would have swallowed the reference here. Owning the client lets the
|
|
failure path put the link down before the exception continues on its way — so the
|
|
peripheral stops holding a phantom peer and goes on advertising for the PIN retry.
|
|
"""
|
|
_FakeMeshCore.reset(_AuthError("Insufficient Authentication"))
|
|
dev = _device()
|
|
|
|
with pytest.raises(_AuthError):
|
|
asyncio.run(dev._connect_owned_ble(_FakeMeshCore))
|
|
|
|
assert len(_FakeMeshCore.built) == 1
|
|
transport = _FakeMeshCore.built[0].cx
|
|
assert transport.link_open is False, "the link was left up — the peripheral goes deaf"
|
|
assert transport.disconnect_calls == 1
|
|
|
|
|
|
def test_the_graceful_disconnect_alone_would_not_have_been_enough() -> None:
|
|
"""The regression guard for the *first* attempt at this fix, which still leaked.
|
|
|
|
``ConnectionManager.disconnect`` is a no-op after a raising connect, because the flag it
|
|
guards on is set only once ``connection.connect()`` has returned. Calling ``mc.disconnect``
|
|
and stopping there looks like a teardown and closes nothing — so this asserts the graceful
|
|
call really is inert on this path, and that the link went down anyway.
|
|
"""
|
|
_FakeMeshCore.reset(_AuthError("Insufficient Authentication"))
|
|
dev = _device()
|
|
|
|
with pytest.raises(_AuthError):
|
|
asyncio.run(dev._connect_owned_ble(_FakeMeshCore))
|
|
|
|
client = _FakeMeshCore.built[0]
|
|
assert client.graceful_calls == 1 # it was tried…
|
|
assert client.connection_manager._is_connected is False # …and the guard refused it
|
|
assert client.cx.link_open is False # …so the direct transport close is what saved us
|
|
assert client.stop_calls == 1 # and the dispatcher was cancelled rather than awaited
|
|
|
|
|
|
def test_an_unanswered_handshake_closes_the_link_too() -> None:
|
|
"""Transport up, no identity reply: reported as "not a companion", link still released."""
|
|
_FakeMeshCore.reset("none")
|
|
dev = _device()
|
|
|
|
assert asyncio.run(dev._connect_owned_ble(_FakeMeshCore)) is None
|
|
assert _FakeMeshCore.built[0].cx.link_open is False
|
|
|
|
|
|
def test_a_good_connect_hands_the_client_over_still_open() -> None:
|
|
"""The success path must not close anything — the caller owns the live client."""
|
|
_FakeMeshCore.reset("ok")
|
|
dev = _device()
|
|
|
|
client = asyncio.run(dev._connect_owned_ble(_FakeMeshCore))
|
|
|
|
assert client is _FakeMeshCore.built[0]
|
|
assert client.graceful_calls == 0
|
|
assert client.cx.link_open is True
|
|
|
|
|
|
def test_the_connection_is_built_from_this_device_s_own_endpoint() -> None:
|
|
"""Address, discovered ``BLEDevice`` and PIN all reach the connection we construct."""
|
|
_FakeMeshCore.reset("ok")
|
|
sentinel = object()
|
|
dev = MeshCoreDevice(
|
|
transport="ble",
|
|
address=_ADDR,
|
|
pin="000000",
|
|
ble_device=sentinel,
|
|
connect_timeout=7.5,
|
|
)
|
|
|
|
asyncio.run(dev._connect_owned_ble(_FakeMeshCore))
|
|
|
|
cx = _FakeTransport.last
|
|
assert (cx.address, cx.device, cx.pin) == (_ADDR, sentinel, "000000")
|
|
client = _FakeMeshCore.built[0]
|
|
# auto_reconnect stays off: MeshTerm drives reconnection itself.
|
|
assert (client.default_timeout, client.auto_reconnect) == (7.5, False)
|
|
|
|
|
|
def test_a_cancelled_handshake_still_closes_the_link() -> None:
|
|
"""A probe's ``wait_for`` expiring mid-handshake must not leak what it cancelled.
|
|
|
|
This is the second way in, and the reason the teardown is shielded: a plain ``await``
|
|
would itself be cancelled the moment it suspended, abandoning the close.
|
|
"""
|
|
_FakeMeshCore.reset("slow")
|
|
dev = _device()
|
|
|
|
async def scenario():
|
|
with pytest.raises(asyncio.TimeoutError):
|
|
await asyncio.wait_for(dev._connect_owned_ble(_FakeMeshCore), timeout=0.1)
|
|
# The shielded close runs on after the cancellation propagates; give it a tick.
|
|
await asyncio.sleep(0.2)
|
|
return _FakeMeshCore.built[0]
|
|
|
|
client = asyncio.run(scenario())
|
|
assert client.cx.link_open is False, "a cancelled handshake stranded its link"
|
|
|
|
|
|
def test_a_failing_teardown_never_masks_the_real_error() -> None:
|
|
"""Teardown is best-effort: the connect's own failure is what the caller must see."""
|
|
|
|
class _Stubborn(_FakeMeshCore):
|
|
async def disconnect(self):
|
|
self.graceful_calls += 1
|
|
raise RuntimeError("teardown exploded")
|
|
|
|
_Stubborn.reset(_AuthError("Insufficient Authentication"))
|
|
dev = _device()
|
|
|
|
with pytest.raises(_AuthError):
|
|
asyncio.run(dev._connect_owned_ble(_Stubborn))
|
|
# The graceful half blew up, and the forced close still ran and did the job.
|
|
assert _Stubborn.built[0].cx.link_open is False
|
|
|
|
|
|
def test_a_wedged_teardown_is_not_waited_out_forever() -> None:
|
|
"""A dispatcher stop that deadlocks is abandoned, and the transport closed regardless."""
|
|
|
|
class _Wedged(_FakeMeshCore):
|
|
async def disconnect(self):
|
|
self.graceful_calls += 1
|
|
await asyncio.sleep(30) # the library's queue.join() deadlock
|
|
|
|
_Wedged.reset(_AuthError("Insufficient Authentication"))
|
|
dev = _device()
|
|
|
|
async def scenario():
|
|
with pytest.raises(_AuthError):
|
|
await dev._connect_owned_ble(_Wedged)
|
|
return _Wedged.built[0]
|
|
|
|
with pytest.MonkeyPatch.context() as mp:
|
|
mp.setattr(conn_mod, "_DISCARD_TIMEOUT_S", 0.1)
|
|
client = asyncio.run(scenario())
|
|
assert client.cx.link_open is False
|
|
|
|
|
|
def test_the_retry_loop_never_reuses_a_torn_down_client() -> None:
|
|
"""A retried link-open builds a fresh client, and the abandoned one was released."""
|
|
_FakeMeshCore.reset(ConnectionError("Failed to connect to device"), "ok")
|
|
dev = _device()
|
|
|
|
client = asyncio.run(dev._create_ble_with_retry(_FakeMeshCore))
|
|
|
|
assert len(_FakeMeshCore.built) == 2, "the retry did not build its own client"
|
|
assert _FakeMeshCore.built[0].cx.link_open is False, "the failed attempt leaked"
|
|
assert client is _FakeMeshCore.built[1]
|
|
assert client.cx.link_open is True
|
|
|
|
|
|
def test_every_link_open_attempt_failing_raises_the_last_error() -> None:
|
|
"""Exhausting the retries surfaces the link failure — with nothing left open."""
|
|
_FakeMeshCore.reset(
|
|
ConnectionError("Failed to connect to device"),
|
|
ConnectionError("Failed to connect to device"),
|
|
)
|
|
dev = _device()
|
|
|
|
with pytest.raises(ConnectionError):
|
|
asyncio.run(dev._create_ble_with_retry(_FakeMeshCore))
|
|
|
|
assert len(_FakeMeshCore.built) == conn_mod._BLE_CONNECT_ATTEMPTS
|
|
assert all(not c.cx.link_open for c in _FakeMeshCore.built)
|