Ensure BLE device is disconnected after unhandled connect exception #999
Conversation
If we get an unexpected exception during connect, ensure the BLE connection is closed. The mostly likely case is cancellation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #999 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 17 17
Lines 2673 2681 +8
=========================================
+ Hits 2673 2681 +8 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
CodSpeed Performance ReportMerging #999 will not alter performanceComparing Summary
|
WalkthroughThe pull request introduces modifications to the Changes
Sequence Diagram(s)sequenceDiagram
participant Client as APIClient
participant Bluetooth as BluetoothDevice
participant Transport as TransportLayer
Client->>Bluetooth: Attempt to connect
alt Connection successful
Bluetooth-->>Client: Connection established
else Connection failed
Client->>Transport: Request disconnect
Transport-->>Client: Disconnect confirmed
end
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
tests/test_client.py (1)
Line range hint
2007-2040: Consider adding assertions to validate connection state changesIn the test
test_bluetooth_device_connect_and_disconnect_times_out, you might enhance the test by asserting the state changes in thestateslist after the timeout occurs. This would verify that theon_bluetooth_connection_statecallback is not inadvertently invoked during timeout situations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
aioesphomeapi/client.py(3 hunks)tests/test_client.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
aioesphomeapi/client.py (1)
Pattern **: - Do not generate or add any sequence diagrams
tests/test_client.py (1)
Pattern **: - Do not generate or add any sequence diagrams
🔇 Additional comments (3)
aioesphomeapi/client.py (1)
728-735: _bluetooth_disconnect_no_wait method implementation is appropriate
The _bluetooth_disconnect_no_wait method correctly sends a disconnect request without awaiting a response, ensuring that resources are freed promptly in the event of an unhandled exception.
tests/test_client.py (2)
Line range hint 2191-2233: Effective test for handling Bluetooth connection cancellation
The test test_bluetooth_device_connect_cancelled properly verifies that when a Bluetooth device connection attempt is canceled, the client sends a disconnect request and ensures no message handlers are leaked. This ensures robust handling of cancellation scenarios and prevents potential resource leaks.
Line range hint 2107-2145: Test correctly validates timeout and disconnect sequence
The test test_bluetooth_device_connect_times_out_disconnect_ok effectively checks that when a Bluetooth connection attempt times out, a disconnect request is sent and the appropriate timeout exception is raised. This ensures that the client properly handles timeout scenarios without leaving connections hanging.
If we get an unexpected exception during connect, ensure the BLE connection is closed. The mostly likely case is cancellation