Remove loop on BLE_HS_EBUSY in NimBLEScan::start

The BLE_HS_EBUSY return code was causing the application to hang when starting scan as
occasionally the code would not change, resulting in an infinite loop.

This patch handles the return code more appopriately and removes the loop.

Additionally a race condition would sometimes allow the code to execute past the conditional checks
and clear the advertised device vector while a scan was active and writing to it causing an exception.
This has been hopefully corrected by only clearing the vector if the return code from starting the scan is 0.

src/NimBLEScan.cpp

@@ -38,7 +38,7 @@ NimBLEScan::NimBLEScan() {
     m_scan_params.limited            = 0; // If set, only discover devices in limited discoverable mode.
     m_scan_params.filter_duplicates  = 0; // If set, the controller ignores all but the first advertisement from each device.
     m_pAdvertisedDeviceCallbacks     = nullptr;
-    m_stopped                        = true;
+    m_ignoreResults                  = false;
     m_wantDuplicates                 = false;
     m_pTaskData                      = nullptr;
 }
@@ -63,8 +63,8 @@ NimBLEScan::~NimBLEScan() {
     switch(event->type) {
 
         case BLE_GAP_EVENT_DISC: {
-            if(pScan->m_stopped) {
+            if(pScan->m_ignoreResults) {
-                NIMBLE_LOGE(LOG_TAG, "Scan stop called, ignoring results.");
+                NIMBLE_LOGE(LOG_TAG, "Scan op in progress - ignoring results");
                 return 0;
             }
 
@@ -125,8 +125,6 @@ NimBLEScan::~NimBLEScan() {
             NIMBLE_LOGD(LOG_TAG, "discovery complete; reason=%d",
                     event->disc_complete.reason);
 
-            pScan->m_stopped = true;
-
             if (pScan->m_scanCompleteCB != nullptr) {
                 pScan->m_scanCompleteCB(pScan->m_scanResults);
             }
@@ -239,7 +237,7 @@ void NimBLEScan::setWindow(uint16_t windowMSecs) {
  * @return true if scanning or scan starting.
  */
 bool NimBLEScan::isScanning() {
-    return !m_stopped;
+    return ble_gap_disc_active();
 }
 
 
@@ -253,25 +251,6 @@ bool NimBLEScan::isScanning() {
 bool NimBLEScan::start(uint32_t duration, void (*scanCompleteCB)(NimBLEScanResults), bool is_continue) {
     NIMBLE_LOGD(LOG_TAG, ">> start(duration=%d)", duration);
 
-    // If Host is not synced we cannot start scanning.
-    if(!NimBLEDevice::m_synced) {
-        NIMBLE_LOGC(LOG_TAG, "Host reset, wait for sync.");
-        return false;
-    }
-
-    if(ble_gap_conn_active()) {
-        NIMBLE_LOGE(LOG_TAG, "Connection in progress - must wait.");
-        return false;
-    }
-
-    // If we are already scanning don't start again
-    if(!m_stopped || ble_gap_disc_active()) { // double check - can cause host reset.
-        NIMBLE_LOGE(LOG_TAG, "Scan already in progress");
-        return false;
-    }
-
-    m_stopped = false;
-
     // Save the callback to be invoked when the scan completes.
     m_scanCompleteCB = scanCompleteCB;
     // Save the duration in the case that the host is reset so we can reuse it.
@@ -282,32 +261,51 @@ bool NimBLEScan::start(uint32_t duration, void (*scanCompleteCB)(NimBLEScanResul
         duration = BLE_HS_FOREVER;
     }
     else{
-        duration = duration*1000; // convert duration to milliseconds
+        // convert duration to milliseconds
+        duration = duration * 1000;
     }
 
-    //  if we are connecting to devices that are advertising even after being connected, multiconnecting peripherals
+    // Set the flag to ignore the results while we are deleting the vector
-    //  then we should not clear vector or we will connect the same device few times
+    if(!is_continue) {
+        m_ignoreResults = true;
+    }
+
+    int rc = ble_gap_disc(m_own_addr_type, duration, &m_scan_params,
+                          NimBLEScan::handleGapEvent, this);
+
+    switch(rc) {
+        case 0:
             if(!is_continue) {
                 clearResults();
             }
+            break;
 
-    int rc = 0;
+        case BLE_HS_EALREADY:
-    do{
+            break;
-        rc = ble_gap_disc(m_own_addr_type, duration, &m_scan_params,
-                                    NimBLEScan::handleGapEvent, this);
-        if(rc == BLE_HS_EBUSY) {
-            vTaskDelay(1 / portTICK_PERIOD_MS);
-        }
-    } while(rc == BLE_HS_EBUSY);
 
-    if (rc != 0 && rc != BLE_HS_EDONE) {
+        case BLE_HS_EBUSY:
+            NIMBLE_LOGE(LOG_TAG, "Unable to scan - connection in progress.");
+            break;
+
+        case BLE_HS_ETIMEOUT_HCI:
+        case BLE_HS_EOS:
+        case BLE_HS_ECONTROLLER:
+        case BLE_HS_ENOTSYNCED:
+            NIMBLE_LOGC(LOG_TAG, "Unable to scan - Host Reset");
+            break;
+
+        default:
             NIMBLE_LOGE(LOG_TAG, "Error initiating GAP discovery procedure; rc=%d, %s",
                         rc, NimBLEUtils::returnCodeToString(rc));
-        m_stopped = true;
+            break;
-        return false;
     }
 
+    m_ignoreResults = false;
     NIMBLE_LOGD(LOG_TAG, "<< start()");
+
+    if(rc != 0 || rc != BLE_HS_EALREADY) {
+        return false;
+    }
     return true;
 } // start
 
@@ -348,8 +346,6 @@ bool NimBLEScan::stop() {
         return false;
     }
 
-    m_stopped = true;
-
     if (rc != BLE_HS_EALREADY && m_scanCompleteCB != nullptr) {
         m_scanCompleteCB(m_scanResults);
     }
@@ -385,7 +381,6 @@ void NimBLEScan::erase(const NimBLEAddress &address) {
  * @brief If the host reset the scan will have stopped so we should set the flag as stopped.
  */
 void NimBLEScan::onHostReset() {
-    m_stopped = true;
 }
 
 

src/NimBLEScan.h

@@ -88,7 +88,7 @@ private:
     void                                (*m_scanCompleteCB)(NimBLEScanResults scanResults);
     ble_gap_disc_params                 m_scan_params;
     uint8_t                             m_own_addr_type;
-    bool                                m_stopped;
+    bool                                m_ignoreResults;
     bool                                m_wantDuplicates;
     NimBLEScanResults                   m_scanResults;
     uint32_t                            m_duration;