+9,910 −3,201
Conversation
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
👋 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 12m 45s ⏱️ Results for commit 6100cb5. ♻️ This comment has been updated with latest results. |
982fbbd to 4931b99 Compare 2e34cb7 to 45ff5a8 Compare 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]
| |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
54b16c2 to 97650b0 Compare Co-authored-by: h2zero <[email protected]>
97650b0 to 6100cb5 Compare There was a problem hiding this comment.
Pull Request Overview
This PR adds support for the NimBLE Bluetooth stack alongside the existing Bluedroid implementation in the ESP32 BLE library. The changes update conditional compilation macros, add NimBLE-specific includes and function implementations, and update documentation across multiple files.
- Updated conditional compilation from CONFIG_BLUEDROID_ENABLED to include CONFIG_NIMBLE_ENABLED.
- Introduced NimBLE-specific functions and macros in core BLE files (e.g., BLEValue, BLEUUID, BLEDescriptor, etc.).
- Updated documentation headers and maintained backward compatibility with Bluedroid.
Reviewed Changes
Copilot reviewed 63 out of 72 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| libraries/BLE/src/BLEValue.h/.cpp | Updated conditional macros and added common includes for NimBLE |
| libraries/BLE/src/BLEUUID.h/.cpp | Added NimBLE-specific handling in UUID constructors and methods |
| libraries/BLE/src/BLEUtils.h | Modified includes and macros to support both stacks |
| libraries/BLE/src/BLEService*.h/.cpp | Added NimBLE public declarations and adjusted error handling |
| libraries/BLE/src/BLEEddystone*.h/.cpp | Updated headers and conditional compilation for NimBLE support |
| ... | Similar conditional updates and NimBLE function implementations in other BLE classes |
Comments suppressed due to low confidence (2)
libraries/BLE/src/BLEDescriptor.cpp:172
- Consider adding a more detailed comment or documentation note explaining that manual handle setting is not supported in NimBLE and that this behavior is intentional. This will help future maintainers understand why the request is ignored in NimBLE builds.
#endif
libraries/BLE/src/BLEHIDDevice.cpp:75
- [nitpick] In BLEHIDDevice.cpp, the battery level descriptor is added only in the Bluedroid branch. Consider either adding equivalent descriptor registration for NimBLE or clarifying in comments that NimBLE handles battery level notifications internally. This will avoid potential confusion about unsupported functionality in NimBLE builds.
#endif
Sign up for free to join this conversation on . 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.
Description of Change
This pull request introduces support (ExtAdv still not implemented) for the NimBLE Bluetooth stack alongside the existing Bluedroid stack in the ESP32 BLE library. It includes updates to ensure compatibility with both stacks, adds new functionality, and improves documentation. Below is a summary of the most important changes grouped by theme.
Compatibility with NimBLE and Bluedroid
cores/esp32/esp32-hal-bt.cto support bothCONFIG_BLUEDROID_ENABLEDandCONFIG_NIMBLE_ENABLED. This ensures the Bluetooth stack is selected dynamically based on the configuration.BLE2901,BLE2902, andBLE2904classes to work with both NimBLE and Bluedroid, including adding new NimBLE-specific logic for notifications and indications.New Features and Improvements
BLE2902class with NimBLE-specific methods for getting and setting notifications and indications, while marking the class as deprecated for NimBLE users since such descriptors are automatically managed.BLE2904class to ensure consistency across both stacks.Documentation Updates
README.mdto clarify the origins of the library, acknowledge contributions from h2zero (also added as co-author) for NimBLE, and provide guidance on where to raise issues.Example and Configuration Changes
ci.jsonfiles) to requireCONFIG_BLUEDROID_ENABLEDfor compatibility with features not yet supported by NimBLE.Maintenance and Metadata
library.propertiestolucasssvaz, reflecting updated project ownership.These changes collectively improve the flexibility and usability of the ESP32 BLE library by supporting multiple Bluetooth stacks while maintaining backward compatibility.
Parts of the NimBLE implementation are based on older versions of NimBLE-Arduino. In future versions where breaking API changes are allowed, NimBLE-Arduino can be used as a submodule if optimal.
Tests scenarios
Examples successfully tested with NimBLE (mostly using ESP32-S3 and C6):
Related links
#9878
espressif/esp32-arduino-lib-builder#296