Merge pull request #500 from liamcottle/fix/nrf52-ble-refactor

Refactor nRF52 BLE to use connection callbacks

src/helpers/nrf52/SerialBLEInterface.cpp

@@ -1,6 +1,27 @@
 #include "SerialBLEInterface.h"
 
+static SerialBLEInterface* instance;
+
+void SerialBLEInterface::onConnect(uint16_t connection_handle) {
+  BLE_DEBUG_PRINTLN("SerialBLEInterface: connected");
+  if(instance){
+    instance->_isDeviceConnected = true;
+    // no need to stop advertising on connect, as the ble stack does this automatically
+  }
+}
+
+void SerialBLEInterface::onDisconnect(uint16_t connection_handle, uint8_t reason) {
+  BLE_DEBUG_PRINTLN("SerialBLEInterface: disconnected reason=%d", reason);
+  if(instance){
+    instance->_isDeviceConnected = false;
+    instance->startAdv();
+  }
+}
+
 void SerialBLEInterface::begin(const char* device_name, uint32_t pin_code) {
+
+  instance = this;
+
   char charpin[20];
   sprintf(charpin, "%d", pin_code);
 
@@ -13,12 +34,28 @@ void SerialBLEInterface::begin(const char* device_name, uint32_t pin_code) {
   Bluefruit.Security.setMITM(true);
   Bluefruit.Security.setPIN(charpin);
 
+  Bluefruit.Periph.setConnectCallback(onConnect);
+  Bluefruit.Periph.setDisconnectCallback(onDisconnect);
+
   // To be consistent OTA DFU should be added first if it exists
   //bledfu.begin();
+
+  // Configure and start the BLE Uart service
+  bleuart.setPermission(SECMODE_ENC_WITH_MITM, SECMODE_ENC_WITH_MITM);
+  bleuart.begin();
+  
 }
 
 void SerialBLEInterface::startAdv() {
-  Bluefruit.Advertising.stop(); // always clean restart
+
+  BLE_DEBUG_PRINTLN("SerialBLEInterface: starting advertising");
+  
+  // clean restart if already advertising
+  if(Bluefruit.Advertising.isRunning()){
+    BLE_DEBUG_PRINTLN("SerialBLEInterface: already advertising, stopping to allow clean restart");
+    Bluefruit.Advertising.stop();
+  }
+
   Bluefruit.Advertising.clearData(); // clear advertising data
   Bluefruit.ScanResponse.clearData(); // clear scan response data
   
@@ -42,10 +79,25 @@ void SerialBLEInterface::startAdv() {
    * For recommended advertising interval
    * https://developer.apple.com/library/content/qa/qa1931/_index.html   
    */
-  Bluefruit.Advertising.restartOnDisconnect(false); // don't restart automatically as already beeing done in checkRecvFrame()
+  Bluefruit.Advertising.restartOnDisconnect(false); // don't restart automatically as we handle it in onDisconnect
   Bluefruit.Advertising.setInterval(32, 244);    // in unit of 0.625 ms
   Bluefruit.Advertising.setFastTimeout(30);      // number of seconds in fast mode
   Bluefruit.Advertising.start(0);                // 0 = Don't stop advertising after n seconds
+
+}
+
+void SerialBLEInterface::stopAdv() {
+
+  BLE_DEBUG_PRINTLN("SerialBLEInterface: stopping advertising");
+  
+  // we only want to stop advertising if it's running, otherwise an invalid state error is logged by ble stack
+  if(!Bluefruit.Advertising.isRunning()){
+    return;
+  }
+
+  // stop advertising
+  Bluefruit.Advertising.stop();
+
 }
 
 // ---------- public methods
@@ -56,25 +108,14 @@ void SerialBLEInterface::enable() {
   _isEnabled = true;
   clearBuffers();
 
-  // Configure and start the BLE Uart service
-  bleuart.setPermission(SECMODE_ENC_WITH_MITM, SECMODE_ENC_WITH_MITM);
-  bleuart.begin();
-
   // Start advertising
   startAdv();
-
-  checkAdvRestart = false;
 }
 
 void SerialBLEInterface::disable() {
   _isEnabled = false;
-
   BLE_DEBUG_PRINTLN("SerialBLEInterface::disable");
-
+  stopAdv();
-  Bluefruit.Advertising.stop();
-
-  oldDeviceConnected = deviceConnected = false;
-  checkAdvRestart = false;
 }
 
 size_t SerialBLEInterface::writeFrame(const uint8_t src[], size_t len) {
@@ -83,7 +124,7 @@ size_t SerialBLEInterface::writeFrame(const uint8_t src[], size_t len) {
     return 0;
   }
 
-  if (deviceConnected && len > 0) {
+  if (_isDeviceConnected && len > 0) {
     if (send_queue_len >= FRAME_QUEUE_SIZE) {
       BLE_DEBUG_PRINTLN("writeFrame(), send_queue is full!");
       return 0;
@@ -119,44 +160,14 @@ size_t SerialBLEInterface::checkRecvFrame(uint8_t dest[]) {
   } else {
     int len = bleuart.available();
     if (len > 0) {
-      deviceConnected = true; // should probably use the callback to monitor cx 
       bleuart.readBytes(dest, len);
       BLE_DEBUG_PRINTLN("readBytes: sz=%d, hdr=%d", len, (uint32_t) dest[0]);
       return len;
     }
   }
-
-  if (Bluefruit.connected() == 0)  deviceConnected = false;
-
-  if (deviceConnected != oldDeviceConnected) {
-    if (!deviceConnected) {    // disconnecting
-      clearBuffers();
-
-      BLE_DEBUG_PRINTLN("SerialBLEInterface -> disconnecting...");
-      delay(500); // give the bluetooth stack the chance to get things ready
-
-      checkAdvRestart = true;
-    } else {
-      BLE_DEBUG_PRINTLN("SerialBLEInterface -> stopping advertising");
-      BLE_DEBUG_PRINTLN("SerialBLEInterface -> connecting...");
-      // connecting
-      // do stuff here on connecting
-      Bluefruit.Advertising.stop();
-      checkAdvRestart = false;
-    }
-    oldDeviceConnected = deviceConnected;
-  }
-
-  if (checkAdvRestart) {
-    if (Bluefruit.connected() == 0) {
-      BLE_DEBUG_PRINTLN("SerialBLEInterface -> re-starting advertising");
-      startAdv();
-    }
-    checkAdvRestart = false;
-  }
   return 0;
 }
 
 bool SerialBLEInterface::isConnected() const {
-  return deviceConnected;  //pServer != NULL && pServer->getConnectedCount() > 0;
+  return _isDeviceConnected;
 }

src/helpers/nrf52/SerialBLEInterface.h

@@ -5,10 +5,8 @@
 
 class SerialBLEInterface : public BaseSerialInterface {
   BLEUart bleuart;
-  bool deviceConnected;
-  bool oldDeviceConnected;
-  bool checkAdvRestart;
   bool _isEnabled;
+  bool _isDeviceConnected;
   unsigned long _last_write;
 
   struct Frame {
@@ -21,18 +19,19 @@ class SerialBLEInterface : public BaseSerialInterface {
   Frame send_queue[FRAME_QUEUE_SIZE];
 
   void clearBuffers() { send_queue_len = 0; }
-  void startAdv();
+  static void onConnect(uint16_t connection_handle);
+  static void onDisconnect(uint16_t connection_handle, uint8_t reason);
 
 public:
   SerialBLEInterface() {
-    deviceConnected = false;
-    oldDeviceConnected = false;
-    checkAdvRestart = false;
     _isEnabled = false;
+    _isDeviceConnected = false;
     _last_write = 0;
     send_queue_len = 0;
   }
 
+  void startAdv();
+  void stopAdv();
   void begin(const char* device_name, uint32_t pin_code);
 
   // BaseSerialInterface methods