Serialize BLE access per tag with a per-MAC lock#53
Open
balloob wants to merge 1 commit into
Open
Conversation
BLE access to a tag was not serialized across operations. Only
_async_upload_image self-serialized (cancelling a prior upload_task);
drawcustom/send_image, LED, buzzer and the OTA flow each opened their own
OpenDisplayDevice connection with no coordination, and the library has no
per-address lock. Two automations hitting the same tag raced for the single
BLE link and surfaced a confusing HomeAssistantError("upload_error"). The
reboot-reload path also waited only on upload_task, so it could tear the
connection out from under an in-flight drawcustom/LED/buzzer/OTA.
Add an asyncio.Lock to OpenDisplayRuntimeData (one config entry == one MAC,
so this is a per-MAC lock) and hold it for the full connection lifetime of
every BLE operation: _async_connect_and_run (drawcustom/send_image, upload,
LED, buzzer) and the OTA install flow. Different MACs are not serialized
against each other. The upload_image latest-wins cancel is kept but its
cancel happens before the lock is taken, so it composes without deadlock.
_async_reload_after_reboot and async_unload_entry now drain the same lock
(acquire/release without holding across the reload) to wait for in-flight
BLE ops, replacing the upload_task-only wait.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012g2e8mr132vcizx92WsgiR
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
BLE access to a tag is not serialized across operations, so two automations hitting the same tag race for the single BLE link and surface a confusing
HomeAssistantError("upload_error").Only
_async_upload_imageself-serialized (cancelling a priorupload_task). Butdrawcustom/send_image,activate_led/activate_buzzer, and the OTA install flow each opened their ownOpenDisplayDevice(...)connection with no coordination, and the library has no per-address lock. Separately,_async_reload_after_rebootwaited only onupload_task, so a reboot-triggered reload could tear the connection out from under an in-flight drawcustom / LED / buzzer / OTA.Fix
Introduce a per-MAC
asyncio.Lockand hold it for the full lifetime of every BLE connection to a tag.OpenDisplayRuntimeData.ble_lock: asyncio.Lock(default factory). A config entry maps 1:1 to a MAC (unique_id), so a per-entry lock is a per-MAC lock.services._async_connect_and_run— the single chokepoint for drawcustom / send_image / upload / LED / buzzer — now holdsble_lockaround the wholeasync with OpenDisplayDevice(...)block.update.pyOTA install holds the sameble_lockacross the DFU trigger + AppLoader flash (the GitHub asset download stays outside the lock so it doesn't block other ops needlessly).How the lock is keyed and scoped
OpenDisplayDevice, released when theasync withunwinds, including on error/cancellation.Reconciling with the existing
upload_taskmechanismupload_imagekeeps its latest-wins behaviour (a newer upload cancels an older running one instead of queuing). This composes with the lock without deadlock: the cancel + await happens before_async_send_imageacquiresble_lock, so the cancelled task unwinds itsasync with ble_lockand releases the link before the new task takes it._async_reload_after_rebootandasync_unload_entrynow drain the lock (acquire then immediately release) to wait for any in-flight BLE op — upload, drawcustom, LED, buzzer, or OTA — instead of waiting only onupload_task.Deadlock / concurrency analysis
upload_image→send_image→connect_and_runtakes the lock exactly once; OTA takes it once; LED/buzzer/drawcustom take it once._async_reload_after_rebootdrains (acquire/release) rather than holding the lock acrossasync_reload, becauseasync_reloadcallsasync_unload_entry, which also drains the same lock — holding it would deadlock the reload. The two drains are sequential, never nested.Testing
No pytest harness ships with this integration (it runs inside full Home Assistant, and the
opendisplay/odl_rendererlibraries aren't part of the repo), so a repo test was not added. Instead:🤖 Generated with Claude Code
https://claude.ai/code/session_012g2e8mr132vcizx92WsgiR