-
Notifications
You must be signed in to change notification settings - Fork 7.7k
Fix error #8176: simple_ble_start enable controller failed 259 #11167
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
Open
+9
−24
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 Jason2866, 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 11m 58s ⏱️
38 tests 38 ✅ 0 💤 0 ❌
241 runs 241 ✅ 0 💤 0 ❌
Results for commit 3b971f2.
♻️ 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]
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This is the classic BT memory being released, not BLE, so I wonder why you see the issue that you do. Maybe we can rethink this so that provisioning will cause it to not be released?
@me-no-dev
me-no-dev
added
the
Status: Needs investigation
We need to do some research before taking next steps on this issue
label
Mar 24, 2025
@me-no-dev Isnt the provided PR just doing this? Keep the allocated memory for BT / BLE needs. Imho the weak if here does not work always as it should
from the docs
esp_bt_controller_mem_release(ESP_BT_MODE_BTDM)could release the BSS and data consumed by both Classic Bluetooth and BLE Controller to heap.
So doing this call affects classic BT and BLE https://docs.espressif.com/projects/esp-idf/en/stable/esp32/api-reference/bluetooth/controller_vhci.html
@Jason2866 it is keeping it always, regardless if BT is used or not. The point of the commented code is to free that memory if BT is not in use. You in effect never free that memory with this PR
So the if with the "weak" is not working. Honestly idk why not working looking in https://github.com/espressif/arduino-esp32/blob/master/cores/esp32/esp32-hal-bt.c
Imho btInUse() is always true since
#if SOC_BT_SUPPORTED
#ifdef CONFIG_BT_ENABLED
#if CONFIG_IDF_TARGET_ESP32
is always set when using a BT / BLE sketch. Wrong include order?
We might need to rework/revisit this, especially with moving to NimBLE. What chip is failing @Jason2866 ?
@me-no-dev Have tested with an esp32. Probably other MCU types fail too.
Removed the in pioarduino non working weak stuff define. The allocating and release BT logic should be carefully revisited when moving to NimBLE.
1 task
This was referenced Mar 27, 2025
The PR is not needed for pioarduino. The use of weak definitions in Arduino core can be forced in Platformio / pioarduino by adding lib_archive = no in platformio.ini
The example has this setting added and is working now.
The compile Arduino as an component of IDF still fails, since the weak define does not work and needs a change as suggested in this PR
1 task
Sign up for free
to join this conversation on GitHub.
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.
Uh oh!
There was an error while loading. Please reload this page.
which happens when provisioning example is build via Arduino as an component of IDF.
This PR fixes this bug by removing the override possiblity via the weak define
Fix #11181
Edit: Platformio / pioarduino does generate correct code when the setting
lib_archive = noin platformio.ini is added.