-
-
Notifications
You must be signed in to change notification settings - Fork 77
Ensure third party callbacks on BLE notify do not cause disconnect #1212
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
We need a broad exception catch around the callback since it calls back into third party code which may raise up to the protocol handler and disconnect the device.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1212 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 20 20
Lines 2932 2935 +3
=========================================
+ Hits 2932 2935 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging #1212 will not alter performanceComparing Summary
|
Caution Review failedThe pull request is closed. WalkthroughA try-except block was added around the Bluetooth GATT notify callback in the client to catch and log exceptions without propagating them. Additionally, a new asynchronous test verifies that exceptions in the callback are properly handled and logged, ensuring the client connection remains stable even if the callback fails. Changes
Suggested labels
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
What does this implement/fix?
We need a broad exception catch around the callback since it calls back into third party code which may raise up to the protocol handler and disconnect the device.
Types of changes
Related issue or feature (if applicable):
on_bluetooth_gatt_notify_data_response
to prevent downstream libs from being able to break theaioesphomeapi
connection #1211Pull request in esphome (if applicable):
Checklist:
tests/
folder).