Skip to content
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.

Sign up for GitHub

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

Bluetooth-classic: release BLE memory when BT classic only is requested #8051

Merged
merged 11 commits into from
Nov 29, 2023
Merged

Bluetooth-classic: release BLE memory when BT classic only is requested #8051

Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
68d385e
esp32-hal-bt.c free Bluetooth LE memory if CONFIG_BTDM_CONTROLLER_MOD…
ferbar Apr 8, 2023
8fafe6a
tStart( add error output
ferbar Apr 8, 2023
7d176db
ble mem_release only for esp32
ferbar Apr 9, 2023
1bf5106
disable BLE with BT_MODE define
ferbar Apr 10, 2023
a7d9795
BluetoothSerial add begin()+disableBLE; add memrelease
ferbar Apr 12, 2023
7b715f1
btStart with BT_MODE parameter
ferbar Apr 12, 2023
853b6d2
beautification
ferbar Apr 12, 2023
83ec18d
Merge branch 'master' into patch-2
me-no-dev Oct 6, 2023
cdf224f
Merge branch 'master' into patch-2
ferbar Oct 12, 2023
e813e6d
Update BluetoothSerial.cpp fix wrong merges
ferbar Oct 12, 2023
8d4975d
Merge branch 'master' into patch-2
me-no-dev Nov 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions cores/esp32/esp32-hal-bt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,30 +20,44 @@ bool btInUse(){ return true; }

#include "esp_bt.h"

#ifndef BT_MODE
#ifdef CONFIG_BTDM_CONTROLLER_MODE_BTDM
#define BT_MODE ESP_BT_MODE_BTDM
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does switching defines do anything here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My intention is to tell btStart() that I want to use bluetooth-classic only. CONFIG_BTDM_CONTROLLER_MODE_BTDM is defined in a default aduino-esp32 configuration, so it is the first #if that matches and can not be overridden. When we switch the defines BR_EDR_ONLY can be forced with a -DCONFIG_BTDM_CONTROLLER_MODE_BR_EDR_ONLY.

If you prefer an other #define like ARDUINO_BLUETOOTH_EDR_ONLY or a new function for BT-classic initialization please let me know.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about you instead -DBT_MODE=ESP_BT_MODE_CLASSIC_BT yourself and only #ifndef BT_MODE do the config check.

#elif defined(CONFIG_BTDM_CONTROLLER_MODE_BR_EDR_ONLY)
#define BT_MODE ESP_BT_MODE_CLASSIC_BT
#else
#define BT_MODE ESP_BT_MODE_BLE
#endif
#endif

bool btStarted(){
return (esp_bt_controller_get_status() == ESP_BT_CONTROLLER_STATUS_ENABLED);
}

bool btStart(){
esp_bt_controller_config_t cfg = BT_CONTROLLER_INIT_CONFIG_DEFAULT();
#if BT_MODE == ESP_BT_MODE_CLASSIC_BT
// esp_bt_controller_enable(MODE) This mode must be equal as the mode in “cfg” of esp_bt_controller_init().
esp_bt_controller_mem_release(ESP_BT_MODE_BLE);
#elif BT_MODE == ESP_BT_MODE_BLE
esp_bt_controller_mem_release(ESP_BT_MODE_CLASSIC_BT);
#endif
cfg.mode=BT_MODE;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cfg.mode is only available on ESP32. You should have kept the whole block filtered for ESP32 target. Also as far as I remember, doing mem release of the classic BT will also break BLE. If BT/BLE is not used, that memory is already cleared when Arduino is starting up. So keep just esp_bt_controller_mem_release(ESP_BT_MODE_BLE);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, you were too fast.

  1. esp_bt_controller_mem_release(ESP_BT_MODE_CLASSIC_BT) crashes the esp32 without an error message when using BLE
  2. unexpectedly #if BT_MODE == ESP_BT_MODE_CLASSIC_BT doesn't work. The preprocessor only likes integer values, I will find a solution. Thinking of:
   cfg.mode=BT_MODE;
   if(cfg.mode == ESP_BT_MODE_CLASSIC_BT) {
     esp_bt_controller_mem_release(ESP_BT_MODE_BLE);
   }

tomorrow....

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

#if CONFIG_IDF_TARGET_ESP32
cfg.mode=BT_MODE;
if(cfg.mode == ESP_BT_MODE_CLASSIC_BT) {
  esp_bt_controller_mem_release(ESP_BT_MODE_BLE);
}
#endif

Looks like a good plan 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@me-no-dev Is it not possible to clear memory used by BT_CLASSIC if I'm using only BLE? In the case of Rainmaker, we often run into low memory since both Classic BT and BLE are on during WiFi provisioning. Tagging @shahpiyushv.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sanketwadekar as far as I remember and it seems that @ferbar confirms it above, it is not possible to release classic memory even if you are using just BLE


if(esp_bt_controller_get_status() == ESP_BT_CONTROLLER_STATUS_ENABLED){
return true;
}
esp_err_t ret;
if(esp_bt_controller_get_status() == ESP_BT_CONTROLLER_STATUS_IDLE){
esp_bt_controller_init(&cfg);
if((ret = esp_bt_controller_init(&cfg)) != ESP_OK) {
log_e("initialize controller failed: %s", esp_err_to_name(ret));
return false;
}
while(esp_bt_controller_get_status() == ESP_BT_CONTROLLER_STATUS_IDLE){}
}
if(esp_bt_controller_get_status() == ESP_BT_CONTROLLER_STATUS_INITED){
if (esp_bt_controller_enable(BT_MODE)) {
log_e("BT Enable failed");
if((ret = esp_bt_controller_enable(BT_MODE)) != ESP_OK) {
log_e("BT Enable mode=%d failed %s", BT_MODE, esp_err_to_name(ret));
return false;
}
}
Expand Down