Skip to content

BLE.begin() -> BLE.end() -> BLE.begin() [again] #243

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

Closed
ASabovic opened this issue Jul 4, 2022 · 34 comments · Fixed by #260
Closed

BLE.begin() -> BLE.end() -> BLE.begin() [again] #243

ASabovic opened this issue Jul 4, 2022 · 34 comments · Fixed by #260
Labels
conclusion: resolved Issue was resolved topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project

Comments

@ASabovic
Copy link

ASabovic commented Jul 4, 2022

Hi guys,

I am wondering if it is possible to turn off BLE when I do not need to send some data, and turn it on again when data is ready to be sent? I am trying to enable that part, but every time when BLE.end() function is called seems that I am not able to start the BLE again after some sleep time. In my case, I want to send the simple message from Arduino to my PC, and it works when BLE is constantly ON, but I want to turn it off in order to keep my device in a sleep state, and in that case, decrease the current consumption. Is this implementation possible with the Arduino BLE library?

P.S.
In my case, the BLE service, characteristics, advertising,... are defined in the setup function. Thanks in advance. Cheers! :)

@rmlearney-digicatapult
Copy link

#65 covers what you're describing.

Implementing this without a power cycle needs changes to the underlying CordioHCIHook

@ASabovic
Copy link
Author

I have already checked this thread and also checked if src/utility/HCICordioTransport.cpp is properly changed in my case and it's same as in #92 but still I am only able to perform 2 cycles and the code breaks.

@ASabovic
Copy link
Author

Is there something related to my board as I am using the Arduino Nano 33 BLE Sense based on the ublox chip? I literally changed everything based on this thread but still after only 2 cycles the device breaks and I am not able to continue executing my code. Please help as this is the most important part of my project.

@baconcheese113
Copy link

Facing the same issue with Nano 33 IoT, a blank sketch with just BLE.begin() in setup increases power consumption from 22mA to 85mA so being able to turn it off is very important.

I'm trying to turn it on every 10 seconds to do a 1 second scan by calling BLE.begin() and BLE.end() a second later. This seems to work, however when setting back up a service BLE.addService() it crashes

@ASabovic
Copy link
Author

In my case, I want to call BLE.begin() when data transfer is needed and after that part is finished call BLE.end() in order to put back my device into sleep state. I am not planning to use BLE very often so this feature is a great opportunity for me to keep BLE "inactive" when I do not need to use it. I tried the solution from these two threads #65 and #92 by changing HCICordioTransport.cpp file with parts that they suggested and confirmed that it works but in my case the code crashes after two cycles and it's not able to back up service and other configuration parts of BLE.

@baconcheese113
Copy link

@ASabovic My issue is described here #246
If you just call BLE.begin() and BLE.end() on repeat without adding services, does it work for you indefinitely? Because that works for me, I just can't add event handlers or services after having called BLE.end()

@ASabovic
Copy link
Author

I have the same situation, if I just call BLE.begin() and BLE.end() the code runs indefenitely, but if I add anything else after two cycles it will crash. I am able to send the message two times and put the BLE OFF, but after that it just stops and the code crashes. Maybe you can also try this solution proposed in the above mentioned threads and let me know if that method works in your case.

@baconcheese113
Copy link

I'm using the latest ArduinoBLE version: 1.3.1 which does include @facchinm 's PR, however the Nano 33 IoT doesn't use HCICordioTransport (ARDUINO_ARCH_MBED is undefined) so no changes to that file would impact my issue.

When trying the BLE.end()/BLE.begin() method, my program crashes trying to add the service. Specifically on this line.

In this loop from GATT.cpp, characteristic->descriptorCount() and descriptor->_refCount return random high numbers right before the crash on the retain() line. Seems like they're trying to read from a corrupted memory address.

for (unsigned int j = 0; j < characteristic->descriptorCount(); j++) {
  BLELocalDescriptor* descriptor = characteristic->descriptor(j);

  descriptor->retain();
  _attributes.add(descriptor);
  descriptor->setHandle(attributeCount());
}

@per1234 per1234 added type: imperfection Perceived defect in any part of project topic: code Related to content of the project itself labels Jul 13, 2022
@ASabovic
Copy link
Author

Here I am not sure what can cause an issue. Is it possible to change the memory address or to define the specific one where all parameters will be saved? Based on #64 in Nano 33 BLE family the BLE functionality is inside the chip, so there's no need for that extra steps. Furthermore, it doesn't use an UART based transport but Cordio's one. Now, I am bit confused as you said that in your case to change HCICordioTransport will not change anything...I am using the Arduino Nano 33 BLE Sense so maybe in my case I also do not have anything from these changes? Now, I am able to reach 3 cycles but then when my device is connected it crashes and that's the end of the execution. @facchinm @rmlearney-digicatapult any suggestions how to fix this issue?

@rmlearney-digicatapult
Copy link

rmlearney-digicatapult commented Jul 13, 2022

I'm really sorry but I'm just a hobbyist when it comes to Arduino.

One of the Arduino team like @facchinm would be more appropriate.

@ASabovic
Copy link
Author

But in #92 you @rmlearney-digicatapult confirmed that the code works for you when you tried to begin and end BLE in cycles. What did you change to make it able to work without breaks? Do you maybe have the code saved somewhere? I think that here you tested the same board that I am using right now (Arduino Nano 33 BLE Sense).

@rmlearney-digicatapult
Copy link

rmlearney-digicatapult commented Jul 13, 2022

Ah. I only did it the once during testing and then realised that cycling the bluetooth connection wasn't a necessary feature of my application so have just left it on ever since. Sorry :(

Try the sketch in #65 on a Nano 33 BLE (not the Sense version) - it might cycle more than once?

@ASabovic
Copy link
Author

Aha okay, I understand. Maybe then @facchinm can give as the final answer is this feasible on Nano 33 BLE Sense or not. I chose Sense version as I saw that the ML part is feasible on it. Does Nano 33 BLE (without Sense) support the machine learning part (specifically in my case the person detection model)?

@ASabovic
Copy link
Author

I just checked this part should work without issues on the Arduino Nano 33 BLE Sense as the only difference between these two is number of sensors (https://create.arduino.cc/projecthub/thatiotguy/arduino-nano-33-ble-v-s-33-ble-sense-v-s-33-iot-07d2bf). It's weird that it does not work if it worked before in your case. Can you maybe just send me your HCI Cordio Transport file just to check if everything matches?

@rmlearney-digicatapult
Copy link

I've just been continuously updating my libraries from Arduino, and haven't used that demo code from #65 for about 2 years now.

I'm afraid that I've since packed away my Ardunio stuff from my desk moved onto different things. Sorry I can't help.

@baconcheese113
Copy link

@ASabovic see if this works for you

@ASabovic
Copy link
Author

ASabovic commented Jul 15, 2022

@baconcheese113 I am trying like this but it does not work. NINA_RESETN is always out of scope.

#include <ArduinoBLE.h>
unsigned long prevNow = 0;


const char* uuidOfBatteryLevelChar = "2d2F88c4-f244-5a80-21f1-ee0224e80658";
const char* uuidOfService = "180F";
const char* nameofPeripheral = "BatteryMonitor";

bool wasConnected = false;
uint32_t genericTimer = 0;
BLEService batteryService(uuidOfService);

BLEUnsignedCharCharacteristic batteryLevelChar(uuidOfBatteryLevelChar, BLEIndicate); 


void setup() {
  Serial.begin(9600);
  while (!Serial);
  pinMode(NINA_RESETN, OUTPUT);
  initBLE();
}


void blePeripheralConnectHandler(BLEDevice central) {
  Serial.println("Connected event, central: ");
  Serial.println(central.address());
}

void blePeripheralDisconnectHandler(BLEDevice central) {
  Serial.println("Disconnected event, central: ");
  Serial.println(central.address());
}


void loop() {

  BLE.poll();
  long now = millis();
  if (now - prevNow >= 5000) {
    prevNow = now;
    if (BLE.connected()) {
      Serial.println("BLE device is connected!");
      int rssiValue = BLE.rssi();
      Serial.println(rssiValue);
      Serial.println("Connected to central");
      updateBatteryLevel();
      digitalWrite(NINA_RESETN, LOW)
      //BLE.disconnect();
      //BLE.end();
      wasConnected=true;
    }
  
    else {
      Serial.println("Not connected to central");
    }
  }

  
  if(wasConnected){
    Serial.println("Central is not connected and must be connected again!");
    //long now2 = millis();
    genericTimer = millis();
    while(millis() - genericTimer <= 10000){} // Wait 15 seconds after disconnect
    wasConnected = false;
    Serial.println("I have to initialize BLE again!");
    //initBLE();
    digitalWrite(NINA_RESETN, HIGH);
    delay(750);
  }
}


void initBLE(void){
  if (BLE.begin()){
    Serial.println("BLE will begin now!");
  }
  BLE.setLocalName(nameofPeripheral);
  BLE.setAdvertisedService(batteryService);
  batteryService.addCharacteristic(batteryLevelChar); // add the battery level characteristic
  BLE.addService(batteryService);
  BLE.setEventHandler(BLEConnected, blePeripheralConnectHandler);
  BLE.setEventHandler(BLEDisconnected, blePeripheralDisconnectHandler);
  BLE.advertise();
  
}


void updateBatteryLevel() {

  int prediction = 1;
  sprintf(prediction_status, "%d", prediction);

  //Serial.println("Waiting to send data!");
  batteryLevelChar.writeValue(prediction_status[0]); 
  
  Serial.println("Data is sent!");
}

Can you maybe suggest me where the mistake is (or how to implement it using your approach)?

@baconcheese113
Copy link

baconcheese113 commented Jul 15, 2022

You'd just call disconnect the regular way

BLE.disconnect();

And when everything is disconnected, you've stopped scanning/advertising, and you're ready to turn off your BLE module call

digitalWrite(NINA_RESETN, LOW);

And when the 15 seconds is up you'd turn back on the module

while (millis() - genericTimer <= 10000) {} // Wait 15 seconds after disconnect
digitalWrite(NINA_RESETN, HIGH);
delay(750);
HCI.setEventMask(0x3FFFFFFFFFFFFFFF);
wasConnected = false;

NINA_RESETN is defined in the the variant.h file for my Nano 33 IoT, you'll need to figure out what it's defined as for your board.

@ASabovic
Copy link
Author

Hi @baconcheese113, I edited the code above. You added the complete library where NINA_RESETN is defined or just header file? I am not able to find a way how to define it for my board. Maybe you can help me?

@baconcheese113
Copy link

After looking into your Nano 33 BLE board a bit more I noticed that the BLE functionality is integrated into the microcontroller inside the NINA-B306, as opposed to boards like the 33 IoT where the microcontroller is separate from the NINA module.

That might make it simpler in your case just using the LowPower.idle() or LowPower.sleep() functions since sleeping the microcontroller would also take care of the BLE for your board.

Also, after looking at your code, is your goal actually to turn off BLE to save power? Or just to stop scanning/advertising for 15 seconds? Are you sure the BLE functionality is drawing significant power?

@ASabovic
Copy link
Author

Yes, in the meantime I checked, and everything related to the BLE is integrated inside the NINA B306 module. Based on what @rmlearney-digicatapult wrote above, I also looked for the Arduino Nano 33 BLE (classic board without SENSE) but based on the datasheet they are the same (SENSE and without SENSE) so maybe there is no sense to order it and test BLE begin/end implementation with HCI transport code.

Are you sure that I can use LowPowerIdle() and sleep() functions in order to control my MCU? I think that I tried that already but the Arduino IDE was not able to recognize the specific library for my device. Which library do I need to consider in case I want to use these functions?

This is just the test code, for my real project application, I will use BLE to send images and simple messages (1-byte char) to the Cloud so I will not use the BLE so often. That is the main reason why I am trying to enable this begin/end functionality in order to keep BLE off when I do not need to use it. Based on my measurements, when the behind jumper is removed (hardware changes in order to decrease the current), my device is consuming around 3mA and I want to keep it in the sleep state where I am able to reach around 300microAmpere. So, basically, I just want to use BLE when I need to send the data and then turn it off and go to a sleep state or even execute another task in the flow, which will not be related to BLE.

The code I provided above is just for testing the BLE begin/end functionality, and other ways how to enable BLE to be off during the time of my experiments when I am not planning to use it. I hope you can understand the concept that I want to reach and maybe provide some answers based on this "little story" :)

@rmlearney-digicatapult
Copy link

Have you considered an alternative technology to BLE for your use case such as LoRa?

@ASabovic
Copy link
Author

Yes, I thought about LoRa but I do not think it’s possible to send a large amount of data such as image using this technology. So, in the end, I decided to take a try with BLE.

@per1234
Copy link
Contributor

per1234 commented Jul 17, 2022

Hi all. Thanks so much for submitting this issue and for the valuable information that has been shared here.

I will request that we keep this thread focused tightly on any deficiencies or defects in the ArduinoBLE library code base related to the original report:

when BLE.end() function is called seems that I am not able to start the BLE again after some sleep time.

General discussion and project assistance will be more appropriate on the Arduino Forum:

https://forum.arduino.cc/

@ASabovic
Copy link
Author

@per1234 yes but I think that is related to the BLE library as it controls this part when we want to play with BLE and enable some things related to it. For the rest I understand but BLE begin/end issue is directly related to this library and this is the 2 years old issue that is never properly solved.

@ASabovic
Copy link
Author

ASabovic commented Aug 9, 2022

@baconcheese113 @rmlearney-digicatapult Hi guys, me again after some time, during my holidays I ordered Nano 33 BLE (without Sense part) and tested it, but again only two successful cycles are possible after which the device is stuck again. Do you maybe know if there were some changes in the Arduino BLE library that removed this feature? Should I also try with some older versions of same library? Thanks in advance :)

@ASabovic
Copy link
Author

ASabovic commented Aug 25, 2022

@facchinm based on your solution before 2 years I think, I also tried to test this feature, but it does not work as I expected. I am able to run 2 cycles after which my code breaks, the device connects to my computer for the third time, but after that, I am not able to do anything. The code stops, the data is not sent, and the only what I can do at that moment is reset the device. Do you maybe have the solution for this problem? I thought that your changes were not merged into this new release of the BLE library, but the guys told me that they have been already implemented. With changes I mean the thread #92 :)

Here is my code:

#include <ArduinoBLE.h>
unsigned long prevNow = 0;

const char* uuidOfBatteryLevelChar = "2d2F88c4-f244-5a80-21f1-ee0224e80658";
const char* uuidOfService = "180F";
const char* nameofPeripheral = "BatteryMonitor";

bool wasConnected = false;
uint32_t genericTimer = 0;

BLEService batteryService(uuidOfService);

BLEUnsignedCharCharacteristic batteryLevelChar(uuidOfBatteryLevelChar, BLEIndicate); 

char prediction_status[] = "";

void setup() {
  Serial.begin(9600);
  while (!Serial);
  initBLE();
}

void blePeripheralConnectHandler(BLEDevice central) {
  Serial.println("Connected event, central: ");
  Serial.println(central.address());
}

void blePeripheralDisconnectHandler(BLEDevice central) {
  Serial.println("Disconnected event, central: ");
  Serial.println(central.address());
}

void loop() {

  BLE.poll();
  long now = millis();
  if (now - prevNow >= 5000) {
    prevNow = now;
    if (BLE.connected()) {
      Serial.println("BLE device is connected!");
      int rssiValue = BLE.rssi();
      Serial.println(rssiValue);
      Serial.println("Connected to central");
      int prediction = 1;
      sprintf(prediction_status, "%d", prediction);
      batteryLevelChar.writeValue(prediction_status[0]);
      BLE.disconnect();
      BLE.end();
      wasConnected=true;
    }
  
    else {
      Serial.println("Not connected to central");
    }
  }

  
  if(wasConnected){
    Serial.println("Central is not connected and must be connected again!");
    genericTimer = millis();
    while(millis() - genericTimer <= 10000){} 
    wasConnected = false;
    Serial.println("I have to initialize BLE again!");
    initBLE();
  }
}


void initBLE(void){
  if (BLE.begin()){
    Serial.println("BLE will begin now!");
  }
  BLE.setLocalName(nameofPeripheral);
  Serial.println("First step is done!");
  BLE.setAdvertisedService(batteryService);
  Serial.println("Second step is done!");
  batteryService.addCharacteristic(batteryLevelChar); // add the battery level characteristic
  Serial.println("Third step is done!");
  BLE.addService(batteryService);
  Serial.println("Fourth step is done!");
  BLE.setEventHandler(BLEConnected, blePeripheralConnectHandler);
  Serial.println("Fifth step is done!");
  BLE.setEventHandler(BLEDisconnected, blePeripheralDisconnectHandler);
  Serial.println("Sixth step is done!");
  Serial.println("Before seventh step and immediately after sixth step!");
  Serial.println("Seventh step is done!");
  BLE.advertise();
}

And this is the output:

BLE will begin now!
First step is done!
Second step is done!
Third step is done!
Fourth step is done!
Fifth step is done!
Sixth step is done!
Before seventh step and immediately after sixth step!
Seventh step is done!
Not connected to central
Connected event, central: 
3c:6a:a7:42:cc:0e
BLE device is connected!
-37
Connected to central
Central is not connected and must be connected again!
I have to initialize BLE again!
BLE will begin now!
First step is done!
Second step is done!
Third step is done!
Fourth step is done!
Fifth step is done!
Sixth step is done!
Before seventh step and immediately after sixth step!
Seventh step is done!
Not connected to central
Connected event, central: 
3c:6a:a7:42:cc:0e
BLE device is connected!
-41
Connected to central
Central is not connected and must be connected again!
I have to initialize BLE again!
BLE will begin now!
First step is done!
Second step is done!
Third step is done!
Fourth step is done!
Fifth step is done!
Sixth step is done!
Before seventh step and immediately after sixth step!
Seventh step is done!
Not connected to central
Connected event, central: 
3c:6a:a7:42:cc:0e

@dominsch
Copy link
Contributor

Just tried your code directly on esp32. It crashed after the second connect as well and it reported a higher memory usage. Needs further investigation.

@dominsch
Copy link
Contributor

Your code is a mess but here is a quick fix:

void initBLE(void){
  BLEService batteryService(uuidOfService); // <-- move declaration here to fix memory leak
  if (BLE.begin()){
    Serial.println("BLE will begin now!");
  }
  BLE.setLocalName(nameofPeripheral);
  BLE.addService(batteryService); // <-- add Service before advertising to fix crash
  BLE.setAdvertisedService(batteryService);
  batteryService.addCharacteristic(batteryLevelChar);
  BLE.setEventHandler(BLEConnected, blePeripheralConnectHandler);
  BLE.setEventHandler(BLEDisconnected, blePeripheralDisconnectHandler);
  BLE.advertise();
}

No issues with ArduinoBLE as far as I can tell

@ASabovic
Copy link
Author

Yes, I agree, it is a complete mess, but I was just using it for testing, and many commented things should be removed. Sorry anyway! I just tried your implementation with my board, but it does not work. I mean, the board is able to execute multiple cycles, but the data never arrived at the receiver, which is the computer in my case. On the computer side, access is constantly denied if things are moved in places you did in the code above. I went through #92 and #65 again and made some changes in the BLE library (HCICordioTransport.cpp and GATT.cpp files), after which it started to work without issues. Now, I am able to execute an indefinite number of begin->end cycles with the data received on the computer side.

Have you also tested your code with the received node as well or just run it on your board to see if it will be able to execute more cycles without breaking?

@facchinm
Copy link
Contributor

Hi @ASabovic @dominsch @baconcheese113 @rmlearney-digicatapult
I just pushed #260 to try and fix the end() -> begin() -> end() memory corruption; after testing in various scenarios it look like the leak is gone and there's no need to instantiate the BLEService inside a function (but it still works and is supported)

If you could test with your sketches and report the results it would be very useful, thanks!

@per1234 per1234 added the status: waiting for information More information must be provided before work can proceed label Aug 31, 2022
@dominsch
Copy link
Contributor

Works on esp32 and nRF connect for android. Might be a good time for another release.

@ASabovic
Copy link
Author

ASabovic commented Sep 5, 2022

@facchinm I confirm it works with the Arduino Nano 33 BLE and Bleak (library that enables BLE on PC) without issues.

@facchinm
Copy link
Contributor

facchinm commented Sep 5, 2022

Super, thanks @dominsch and @ASabovic for testing, merging and releasing

@per1234 per1234 added conclusion: resolved Issue was resolved and removed status: waiting for information More information must be provided before work can proceed labels Sep 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conclusion: resolved Issue was resolved topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants