-
Notifications
You must be signed in to change notification settings - Fork 7.7k
fix(ble): Fix descriptor loading and add client multiconnect example #11978
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
base: master
Are you sure you want to change the base?
Conversation
👋 Hello lucasssvaz, we appreciate your contribution to this project! 📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more. 🖊️ Please also make sure you have read and signed the Contributor License Agreement for this project. Click to see more instructions ...
Review and merge process you can expect ...
|
Test Results 76 files 76 suites 14m 19s ⏱️ Results for commit b062cc8. ♻️ This comment has been updated with latest results. |
Memory usage test (comparing PR against master branch)The table below shows the summary of memory usage change (decrease - increase) in bytes and percentage for each target.
Click to expand the detailed deltas report [usage change in BYTES]
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good! Thanks @lucasssvaz
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements lazy loading of BLE descriptors for NimBLE to avoid deadlock issues during characteristic construction and adds a new multi-connection client example.
Key changes:
- Removed
m_endHandlefromBLERemoteCharacteristicand replaced withgetRemoteService()->getEndHandle()calls - Implemented lazy loading of descriptors in
getDescriptors(),getDescriptor(), andsetNotify()methods - Added a new
Client_multiconnectexample demonstrating simultaneous connections to multiple BLE servers
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| libraries/BLE/src/BLERemoteCharacteristic.h | Removed unused m_endHandle member variable from NimBLE implementation |
| libraries/BLE/src/BLERemoteCharacteristic.cpp | Implemented lazy descriptor loading and replaced m_endHandle with service's end handle |
| libraries/BLE/examples/Client_multiconnect/ci.yml | Added CI configuration for the new multi-connection example |
| libraries/BLE/examples/Client_multiconnect/Client_multiconnect.ino | Added new example demonstrating multi-server BLE client connections |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ffa08d4 to
d0d84b7
Compare
Description of Change
This pull request introduces a new example for connecting to multiple BLE servers simultaneously and refactors how BLE characteristic descriptors are managed in the NimBLE client implementation. The main improvements include lazy-loading of descriptors to avoid deadlocks, on-demand retrieval for notifications, and a new example demonstrating multi-server client logic.
New Example: Multi-Server BLE Client
Client_multiconnect.inoexample, which demonstrates how to scan for, connect to, and interact with up to three BLE servers at once, including handling notifications and reconnections.ci.ymlconfiguration for the new example, specifying build requirements for BLE support.Descriptor Management Refactor (NimBLE Client):
BLERemoteCharacteristicto lazily retrieve descriptors only when needed (e.g., for notifications or explicit requests), instead of during construction. This avoids potential deadlocks with NimBLE and improves reliability. [1] [2]getDescriptors(),getDescriptor(), andsetNotify()to automatically retrieve descriptors on-demand if they have not been loaded yet. This ensures correct behavior for notification registration and descriptor access. [1] [2] [3]retrieveDescriptors()to use the remote service's end handle, ensuring all relevant descriptors are found.Test Scenarios
Tested Locally
Related links
Closes #11796
Closes #11811