Serialize BLE access per tag with a per-MAC lock

custom_components/opendisplay/__init__.py

@@ -2,7 +2,7 @@
 
 import asyncio
 import contextlib
-from dataclasses import dataclass
+from dataclasses import dataclass, field
 from typing import TYPE_CHECKING
 
 from opendisplay import (
@@ -50,6 +50,11 @@ class OpenDisplayRuntimeData:
     device_config: GlobalConfig
     is_flex: bool
     upload_task: asyncio.Task | None = None
+    # Serializes every BLE connection to this tag (one config entry == one MAC).
+    # The device exposes a single BLE link with no per-address lock in the
+    # library, so drawcustom/upload_image, LED, buzzer and OTA must not open
+    # overlapping connections or they race and surface a confusing upload_error.
+    ble_lock: asyncio.Lock = field(default_factory=asyncio.Lock)
 
 
 type OpenDisplayConfigEntry = ConfigEntry[OpenDisplayRuntimeData]
@@ -185,25 +190,36 @@ async def _async_reload_after_reboot(
     Triggered by the coordinator when the advertised reboot flag goes
     False -> True. Reloading re-runs async_setup_entry, which reconnects (clearing
     the device's reboot flag), re-reads firmware + config, and rebuilds device
-    info and platforms. Defers until any in-progress image upload finishes so an
-    unrelated reboot detection does not abort the user's upload.
+    info and platforms. Defers until any in-flight BLE operation on this tag
+    (upload, drawcustom, LED, buzzer, OTA) finishes so an unrelated reboot
+    detection does not tear the connection out from under it.
     """
     runtime = entry.runtime_data
-    upload_task = runtime.upload_task if runtime is not None else None
-    if upload_task is not None and not upload_task.done():
-        await asyncio.gather(upload_task, return_exceptions=True)
+    if runtime is not None:
+        # Wait for any in-flight BLE op to release the link, then reload. We only
+        # drain (acquire/release) rather than hold the lock across the reload:
+        # async_reload unloads the entry, and async_unload_entry acquires the
+        # same lock, so holding it here would deadlock the reload.
+        async with runtime.ble_lock:
+            pass
     await hass.config_entries.async_reload(entry.entry_id)
 
 
 async def async_unload_entry(
     hass: HomeAssistant, entry: OpenDisplayConfigEntry
 ) -> bool:
     """Unload a config entry."""
-    if (task := entry.runtime_data.upload_task) and not task.done():
+    runtime = entry.runtime_data
+    # Abort an in-flight image upload quickly so unload is not blocked on a long
+    # BLE transfer, then wait for any other in-flight BLE op (LED, buzzer, OTA,
+    # drawcustom) to release the link before tearing the entry down.
+    if (task := runtime.upload_task) and not task.done():
         task.cancel()
         with contextlib.suppress(asyncio.CancelledError):
             await task
+    async with runtime.ble_lock:
+        pass
 
     return await hass.config_entries.async_unload_platforms(
-        entry, _get_platforms(entry.runtime_data)
+        entry, _get_platforms(runtime)
     )

custom_components/opendisplay/services.py

@@ -312,7 +312,13 @@ async def _async_connect_and_run(
         ) from err
 
     try:
-        async with OpenDisplayDevice(
+        # Serialize all BLE access to this tag: the device has a single BLE link
+        # and the library has no per-address lock, so overlapping connections
+        # (two automations, or a drawcustom racing an LED/buzzer/upload on the
+        # same MAC) fail with a confusing upload_error. The lock is held for the
+        # full connection lifetime and released on error. Held per config entry,
+        # i.e. per MAC, so different tags are not serialized against each other.
+        async with entry.runtime_data.ble_lock, OpenDisplayDevice(
             mac_address=address,
             ble_device=ble_device,
             config=entry.runtime_data.device_config,
@@ -383,6 +389,12 @@ async def _async_upload_image(call: ServiceCall) -> None:
             translation_placeholders={"url": image_data},
         )
 
+    # Latest-wins for upload_image specifically: a newer upload cancels an
+    # older still-running one instead of queuing behind it (an automation
+    # pushing a fresh snapshot supersedes the stale one). This composes with the
+    # per-MAC ble_lock without deadlocking: the cancel + await below happens
+    # before _async_send_image acquires the lock, so the cancelled task releases
+    # the lock (its `async with ble_lock` unwinds) before this one takes it.
     current = asyncio.current_task()
     if (prev := entry.runtime_data.upload_task) is not None and not prev.done():
         prev.cancel()

custom_components/opendisplay/update.py

@@ -197,52 +197,59 @@ def _on_log(msg: str) -> None:
                     "Device not reachable over Bluetooth; bring it within range and retry"
                 )
 
-            # Only EFR32BG22 (Silabs AppLoader) is flashed over BLE here — see
-            # _OTA_INSTALL_IC_TYPES. The device is either in app mode (and needs the
-            # DFU trigger) or already in the AppLoader at the same address (a
-            # previous OTA was interrupted, or it browned out before committing).
-            # Try to trigger from app mode, but tolerate the connect failing — then
-            # the device is already in the AppLoader and we flash it directly, so HA
-            # can recover a stuck device instead of failing hard. A stale proxy GATT
-            # cache from a prior interrupted OTA is handled at the connection layer:
-            # BLEConnection.connect() clears it and retries once.
-            try:
-                _on_log("Connecting to trigger DFU bootloader…")
-                async with OpenDisplayDevice(
-                    mac_address=self._ble_address,
-                    ble_device=ble_device,
-                    encryption_key=_get_encryption_key(self._entry),
-                ) as device:
-                    # Clear the (now fresh) app-mode GATT from the proxy cache so the
-                    # post-reboot AppLoader connection re-discovers the OTA service
-                    # instead of these app-firmware handles.
-                    cleared = await device.clear_gatt_cache()
-                    _on_log(f"Proxy GATT cache clear requested: {cleared}")
-                    await device.trigger_dfu_bootloader()
-            except BLEConnectionError as err:
-                _on_log(
-                    f"App-mode connect failed ({err}); device is likely already "
-                    "in the AppLoader — attempting OTA directly."
+            # Hold the per-MAC BLE lock across the whole OTA (DFU trigger +
+            # AppLoader flash) so a drawcustom/upload/LED/buzzer service call on
+            # this tag can't open an overlapping connection mid-flash. This op
+            # only takes the lock here — it never calls back into a locked
+            # service op — so there is no re-entrant deadlock. The download above
+            # runs outside the lock so it doesn't block other BLE ops needlessly.
+            async with self._entry.runtime_data.ble_lock:
+                # Only EFR32BG22 (Silabs AppLoader) is flashed over BLE here — see
+                # _OTA_INSTALL_IC_TYPES. The device is either in app mode (and needs
+                # the DFU trigger) or already in the AppLoader at the same address (a
+                # previous OTA was interrupted, or it browned out before committing).
+                # Try to trigger from app mode, but tolerate the connect failing —
+                # then the device is already in the AppLoader and we flash it
+                # directly, so HA can recover a stuck device instead of failing hard.
+                # A stale proxy GATT cache from a prior interrupted OTA is handled at
+                # the connection layer: BLEConnection.connect() clears it and retries.
+                try:
+                    _on_log("Connecting to trigger DFU bootloader…")
+                    async with OpenDisplayDevice(
+                        mac_address=self._ble_address,
+                        ble_device=ble_device,
+                        encryption_key=_get_encryption_key(self._entry),
+                    ) as device:
+                        # Clear the (now fresh) app-mode GATT from the proxy cache so
+                        # the post-reboot AppLoader connection re-discovers the OTA
+                        # service instead of these app-firmware handles.
+                        cleared = await device.clear_gatt_cache()
+                        _on_log(f"Proxy GATT cache clear requested: {cleared}")
+                        await device.trigger_dfu_bootloader()
+                except BLEConnectionError as err:
+                    _on_log(
+                        f"App-mode connect failed ({err}); device is likely already "
+                        "in the AppLoader — attempting OTA directly."
+                    )
+
+                # AppLoader advertises at the same address; perform_silabs_ota
+                # retries the connection internally until it is ready.
+                ota_device = async_ble_device_from_address(
+                    self.hass, self._ble_address, connectable=True
                 )
-
-            # AppLoader advertises at the same address; perform_silabs_ota retries
-            # the connection internally until it is ready.
-            ota_device = async_ble_device_from_address(
-                self.hass, self._ble_address, connectable=True
-            )
-            if ota_device is None:
-                raise HomeAssistantError(
-                    "Device not reachable in OTA mode; bring it within range and retry"
+                if ota_device is None:
+                    raise HomeAssistantError(
+                        "Device not reachable in OTA mode; bring it within range and retry"
+                    )
+                await perform_silabs_ota(
+                    firmware_bytes,
+                    ota_device,
+                    on_progress=_on_progress,
+                    on_log=_on_log,
                 )
-            await perform_silabs_ota(
-                firmware_bytes,
-                ota_device,
-                on_progress=_on_progress,
-                on_log=_on_log,
-            )
 
-            self._attr_installed_version = tag
-            _LOGGER.info("Firmware updated to %s", tag)
+                self._attr_installed_version = tag
+                _LOGGER.info("Firmware updated to %s", tag)
 
         except (OTAError, BLEConnectionError) as err:
             raise HomeAssistantError(f"Firmware update failed: {err}") from err