fix: bleak retry connector warning#38
Conversation
crash0verride11
commented
Mar 17, 2026
crash0verride11
commented
- implement bleak_retry_connector to resolve warnings in HA and potentially improve connections (anecdotally seems more reliable)
* implement bleak_retry_connector to resolve warnings and potentially improve connections
abmantis
left a comment
abmantis
left a comment
There was a problem hiding this comment.
hey @crash0verride11 . Thanks for opening this PR. Can you update the tests too please?
- Patch 'establish_connection' in 'mock_idasen_desk' - Move 'disconnected_callback' capture from 'mock_init' to `mock_establish_conn` - Alias 'mock_desk.connect' to 'mock_establish_connection' to preserve exist. assertions - Initialize 'wakeup' and 'trigger_disconnected_callback' explicitly - Update 'test_double_connect_call_with_different_bledevice' - Fix 'test_reconnect_on_connection_drop' to explicitly reset external 'connect' mock after 'reset_mock()'
* add ruff format
|
Of course, I should have seen that request coming. |
* wakeup() happens before pair() which can an error and connection issue * add wakeup() before move_to_target * add bleakError exceptions
* add wakeup() to tests and move connection alias to conftest.py
|
Thanks for working on this — I ran into the same warning path and ended up with a very similar patch in esphome/esphome#39. In my testing, moving to bleak-retry-connector was necessary, but not sufficient on its own for BLE proxy setups. I also had to move Without that ordering change, my setup still showed:
So I think the main delta between esphome/esphome#38 and esphome/esphome#39 is the pair-before-wakeup sequencing, plus the regression tests that cover that behavior. If it helps, I’m happy to close esphome/esphome#39 and fold the pair-before-wakeup fix into esphome/esphome#38. |
|
Hey @crash0verride11 👋 I'm curious if you also see long/inconsistent times when connecting to the desk controller (between 0.4s all the way to 11s). I filed an issue here, but curious to see if other people are seeing it. |
|
@pedropombeiro That issue with the wakeup() positioning also presented itself to me after a few days. I've been sitting on a commit for that call move and updated tests with wakeup() specific items. It's also why I switched this to a draft pull. I haven't committed yet because I also seem to now be reliably connecting but getting timeouts on movement and stopping. I also had to add a wakeup() to move_to. I've been trying to figure out how much is espHomeBluetooth related vs the Idasen code, so curious if that's also what you're using to connect? |
* caches services to prevent rediscovery on every connect, improving connection responses
* add BleakGATTServiceCollection
* _connection_manager is assigned in Desk.connect() before the callback
* add coverage for failure if move_to is raised before stop, move_to failure, if disconnect during get_height
restore removed guard to fix test.
|
Swapping BleakClient with BleakClientWithServiceCache definitely improved connection reliability for me. It seems to be working normally again (was never ideal with my bluetooth proxy), but I'm going to wait and test this for a while again before inviting a review. I would be interested in your results as well @pedropombeiro . |
|
esphome/esphome#40 has been merged now, and I updated esphome/esphome#39 to be only about BleakClientWithServiceCache and bleak-retry-connector, let's see how that helps. I figured out that the very long (> 5s) connection times were due to a bug in my HA automation, so now the only problem I'm seeing is the initial ~3 secs to connect. |
|
Whatever works, I've moved past bleak retry connect and am testing some other changes, unrelated to your ManagedIdasenDesk that have my desk movements working smoothly without stopping repeatedly. I'll be curious to try layering it on top of yours. |
@crash0verride11 I also managed to make it smoother by making the communications happen more frequently (raised the command issuing frequency from every 200ms to every 50ms). Not sure if that's what you did. UPDATE: If you want to give it a try, you can install my fork with HACS: https://github.com/pedropombeiro/ha-idasen-desk. This will override the built-in integration pointing to my abmantis/idasen-ha fork. |
@pedropombeiro Not the direction I headed at all. I might take a look, but I'm going to stop wasting time here and wait and see the final commit that get's merged here before considering resolving my changes with anything. Looks like abmantis favors your pull. The integration is already/finally working pretty much perfectly for me now — up/down without getting stuck or stopping 90% of the time. So not in any rush. |
This reverts commit 9546c74.
add test coverage
|
@crash0verride11 wonder if there are differences at the controller level that require mine to receive quicker BLE updates. I'm using the Linak DPG1C controller. Are you using a different one? |
|
@pedropombeiro I'm using the one that comes with my Idasen desk, which seems to be an equivalent of the DPG1K. I also came at this from the idea that sending a write and read command to a bluetooth-proxy every 200ms / 10 times a second over the network was untenable and causing the timeout issues and stops, so I pushed towards removing / reducing gatt commands. The idasen-ha library is making the bluetooth device act like a controller for the desk when it really only needs to be a coordinator. It currently reads the desk moving speed to find out when the desk has stopped and sends a stop command if the read command times out, causing all the repeated stops. However we already subscribe to the desk height when we connect (desk sends to us, no need for speed reads at all) and the desk already stops itself when it reaches the correct height. All we really have to do is send the target height and be able to detect when to stop sending the gatt write / the desk reached the target height (which, again, we already receive). I changed move_to so it only sends target height write every 400ms until the height target is reached, and that alone fixed the 90% of the issues. I mixed in the ManagedIdasenDesk from your pull request over the weekend and any last remaining blips are gone. |
@crash0verride11 that does sound like the way to go, great to hear that it is working smoothly for you now! |
|
@crash0verride11 are you thinking of pushing your final version? |
|
@crash0verride11 I've done a similar change on my branch and can confirm that lowering the refresh rate and letting the desk controller do its job works fine (knock on wood) 👍 I've closed my PR and opened a new one (#41) with the complete set of changes. |
…l detection Replace GATT read polling during movement with a write-only loop that sends the target height every 400ms using fire-and-forget writes (response=False). Arrival is detected via BLE notifications from the monitor() subscription instead of reading height/speed each iteration. This eliminates ATT channel contention between reads and writes that caused movement interruptions over Bluetooth proxies. Add a 15-second safety timeout to prevent infinite loops. Ref: abmantis#38 (comment)
…l detection Replace GATT read polling during movement with a write-only loop that sends the target height every 400ms using fire-and-forget writes (response=False). Arrival is detected via BLE notifications from the monitor() subscription instead of reading height/speed each iteration. This eliminates ATT channel contention between reads and writes that caused movement interruptions over Bluetooth proxies. Add a 15-second safety timeout to prevent infinite loops. Ref: abmantis#38 (comment)
Rewrite ManagedIdasenDesk.move_to_target to eliminate GATT reads during movement. Send target height writes every 400ms with response=False (fire-and-forget) and detect arrival via BLE notifications from the monitor() subscription instead of polling get_height_and_speed(). The previous implementation interleaved GATT reads with writes. BLE has a single ATT channel per connection -- reads block the channel until the response arrives. Over Bluetooth proxies, read latency can spike to seconds on timeout/retry, creating unpredictable gaps between reference input writes that cause the desk to stop moving. Add a 15-second safety timeout to prevent infinite move loops. Ref: abmantis#38 (comment)
Rewrite ManagedIdasenDesk.move_to_target to eliminate GATT reads during movement. Send target height writes every 400ms with response=False (fire-and-forget) and detect arrival via BLE notifications from the monitor() subscription instead of polling get_height_and_speed(). The previous implementation interleaved GATT reads with writes. BLE has a single ATT channel per connection -- reads block the channel until the response arrives. Over Bluetooth proxies, read latency can spike to seconds on timeout/retry, creating unpredictable gaps between reference input writes that cause the desk to stop moving. Add a 15-second safety timeout to prevent infinite move loops. Ref: abmantis#38 (comment)
Rewrite ManagedIdasenDesk.move_to_target to eliminate GATT reads during movement. Send target height writes every 200ms with acknowledged delivery and detect arrival via BLE notifications from the monitor() subscription instead of polling get_height_and_speed(). The previous implementation interleaved GATT reads with writes. BLE has a single ATT channel per connection -- reads block the channel until the response arrives. Over Bluetooth proxies, read latency can spike to seconds on timeout/retry, creating unpredictable gaps between reference input writes that cause the desk to stop moving. Send explicit stop commands (COMMAND_STOP + REFERENCE_INPUT_STOP) when the move loop exits to cleanly terminate desk movement. Add a 30-second safety timeout to prevent infinite move loops. Ref: abmantis#38 (comment)
Rewrite ManagedIdasenDesk.move_to_target to eliminate GATT reads during movement. Send target height writes every 200ms with acknowledged delivery and detect arrival via BLE notifications from the monitor() subscription instead of polling get_height_and_speed(). The previous implementation interleaved GATT reads with writes. BLE has a single ATT channel per connection -- reads block the channel until the response arrives. Over Bluetooth proxies, read latency can spike to seconds on timeout/retry, creating unpredictable gaps between reference input writes that cause the desk to stop moving. Send explicit stop commands (COMMAND_STOP + REFERENCE_INPUT_STOP) when the move loop exits to cleanly terminate desk movement. Add a 30-second safety timeout to prevent infinite move loops. Ref: abmantis#38 (comment)
|
@crash0verride11 should we move forward with this PR or the bleak-retry-connector implementation in #41? |
3 participants
