diff --git a/src/meshcore/ble_cx.py b/src/meshcore/ble_cx.py index 0a7380f..7cee63f 100644 --- a/src/meshcore/ble_cx.py +++ b/src/meshcore/ble_cx.py @@ -4,6 +4,7 @@ mccli.py : CLI interface to MeschCore BLE companion app import asyncio import logging +from typing import Optional # Make bleak optional - only fail if BLE operations are attempted @@ -52,6 +53,13 @@ class BLEConnection: self.rx_char = None self._disconnect_callback = None self._background_tasks: set[asyncio.Task] = set() + # Serialises write_gatt_char(). Two overlapping writes to the same + # characteristic drop the link outright (observed on macOS/CoreBluetooth: + # "BLE write failed: 19", connection gone). Nothing above this layer + # guarantees callers are sequential -- schedulers, health checks and + # user commands all issue commands independently -- so the transport has + # to enforce it. Lazily created so it binds to the running loop. + self._write_lock: Optional[asyncio.Lock] = None def _spawn_background(self, coro) -> asyncio.Task: """Create a tracked background task (prevents GC of fire-and-forget tasks).""" @@ -199,8 +207,11 @@ class BLEConnection: if not self.rx_char: logger.error("RX characteristic not found") return False + if self._write_lock is None: + self._write_lock = asyncio.Lock() try: - await self.client.write_gatt_char(self.rx_char, bytes(data), response=True) + async with self._write_lock: + await self.client.write_gatt_char(self.rx_char, bytes(data), response=True) except Exception as exc: logger.warning(f"BLE write failed: {exc}") if self._disconnect_callback: diff --git a/tests/unit/test_error_handling.py b/tests/unit/test_error_handling.py index 8be06d1..2470dfe 100644 --- a/tests/unit/test_error_handling.py +++ b/tests/unit/test_error_handling.py @@ -412,3 +412,67 @@ async def test_send_trace_unknown_path_hash_len( assert result.is_error() assert result.payload["reason"] == "invalid_path_format" + + +# ── BLE transport serialises writes ────────────────────────── + +async def test_ble_send_serialises_concurrent_writes(): + """Overlapping write_gatt_char() calls must not interleave. + + Two concurrent writes to the same characteristic drop the link outright + (observed on macOS/CoreBluetooth: "BLE write failed: 19"). Nothing above the + transport guarantees callers are sequential, so the transport must. + """ + from meshcore.ble_cx import BLEConnection + + conn = BLEConnection.__new__(BLEConnection) # bypass bleak availability check + conn._disconnect_callback = None + conn._write_lock = None + conn.rx_char = object() + + overlap = {"current": 0, "max": 0} + + class FakeClient: + async def write_gatt_char(self, char, data, response=True): + overlap["current"] += 1 + overlap["max"] = max(overlap["max"], overlap["current"]) + await asyncio.sleep(0.01) # a real write is not instantaneous + overlap["current"] -= 1 + + conn.client = FakeClient() + + await asyncio.gather(*(conn.send(b"\x01\x02") for _ in range(8))) + + assert overlap["max"] == 1, f"{overlap['max']} writes overlapped" + + +async def test_ble_send_releases_lock_on_write_failure(): + """A failed write must not wedge the lock for every later command.""" + from meshcore.ble_cx import BLEConnection + + conn = BLEConnection.__new__(BLEConnection) + conn._write_lock = None + conn.rx_char = object() + reasons = [] + + async def capture(reason): + reasons.append(reason) + + conn._disconnect_callback = capture + + class BoomThenOK: + def __init__(self): + self.calls = 0 + + async def write_gatt_char(self, char, data, response=True): + self.calls += 1 + if self.calls == 1: + raise RuntimeError("19") + + conn.client = BoomThenOK() + + await conn.send(b"\x01") + assert reasons == ["ble_write_failed: 19"] + # Second write must still be able to acquire the lock. + await asyncio.wait_for(conn.send(b"\x02"), timeout=1.0) + assert conn.client.calls == 2