From 8fe2766e012b7dfd92c22665739830dedb89d5bd Mon Sep 17 00:00:00 2001 From: h2zero Date: Sun, 10 Jan 2021 21:54:32 -0700 Subject: [PATCH] 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 | 87 ++++++++++++++++++++++------------------------ src/NimBLEScan.h | 2 +- 2 files changed, 42 insertions(+), 47 deletions(-) diff --git a/src/NimBLEScan.cpp b/src/NimBLEScan.cpp index 73fc7f8..d3ea532 100644 --- a/src/NimBLEScan.cpp +++ b/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) { - NIMBLE_LOGE(LOG_TAG, "Scan stop called, ignoring results."); + if(pScan->m_ignoreResults) { + 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 - // then we should not clear vector or we will connect the same device few times + // Set the flag to ignore the results while we are deleting the vector if(!is_continue) { - clearResults(); + m_ignoreResults = true; } - int rc = 0; - do{ - 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); + int rc = ble_gap_disc(m_own_addr_type, duration, &m_scan_params, + NimBLEScan::handleGapEvent, this); - if (rc != 0 && rc != BLE_HS_EDONE) { - NIMBLE_LOGE(LOG_TAG, "Error initiating GAP discovery procedure; rc=%d, %s", - rc, NimBLEUtils::returnCodeToString(rc)); - m_stopped = true; + switch(rc) { + case 0: + if(!is_continue) { + clearResults(); + } + break; + + case BLE_HS_EALREADY: + break; + + 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)); + break; + } + + m_ignoreResults = false; + NIMBLE_LOGD(LOG_TAG, "<< start()"); + + if(rc != 0 || rc != BLE_HS_EALREADY) { return false; } - - NIMBLE_LOGD(LOG_TAG, "<< start()"); 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; } diff --git a/src/NimBLEScan.h b/src/NimBLEScan.h index 8226290..b55cdf8 100644 --- a/src/NimBLEScan.h +++ b/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;