diff --git a/custom_components/opendisplay/__init__.py b/custom_components/opendisplay/__init__.py index 3bfb97d..2a0b18d 100644 --- a/custom_components/opendisplay/__init__.py +++ b/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,13 +190,18 @@ 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) @@ -199,11 +209,17 @@ 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) ) diff --git a/custom_components/opendisplay/services.py b/custom_components/opendisplay/services.py index 6ca5651..8c0564b 100644 --- a/custom_components/opendisplay/services.py +++ b/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() diff --git a/custom_components/opendisplay/update.py b/custom_components/opendisplay/update.py index 2937aa1..74eb411 100644 --- a/custom_components/opendisplay/update.py +++ b/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