Set Client connect/disconnect data before stack calls.

Connect should set m_pTaskData before calling ble_gap_connect in case of an early event.

Disconnect should check if the timer has already started before starting the timer so it does not reset it.
The timer should also be started before calling ble_gap_terminate in case of an early event that would cause the timer to start
after disconnection, resetting the host unnecessarily.

Author: h2zero

src/NimBLEClient.cpp

@@ -182,25 +182,30 @@ bool NimBLEClient::connect(const NimBLEAddress &address, bool deleteAttibutes) {
         return false;
     }
 
-    if(isConnected() || m_pTaskData != nullptr) {
+    if(isConnected() || m_connEstablished || m_pTaskData != nullptr) {
         NIMBLE_LOGE(LOG_TAG, "Client busy, connected to %s, id=%d",
                     std::string(m_peerAddress).c_str(), getConnId());
         return false;
     }
 
+    ble_addr_t peerAddr_t;
+    memcpy(&peerAddr_t.val, address.getNative(),6);
+    peerAddr_t.type = address.getType();
+    if(ble_gap_conn_find_by_addr(&peerAddr_t, NULL) == 0) {
+        NIMBLE_LOGE(LOG_TAG, "A connection to %s already exists",
+                    address.toString().c_str());
+        return false;
+    }
+
     if(address == NimBLEAddress("")) {
         NIMBLE_LOGE(LOG_TAG, "Invalid peer address;(NULL)");
         return false;
-    } else if(m_peerAddress != address) {
+    } else {
         m_peerAddress = address;
     }
 
-    ble_addr_t peerAddr_t;
-    memcpy(&peerAddr_t.val, m_peerAddress.getNative(),6);
-    peerAddr_t.type = m_peerAddress.getType();
-
-
     ble_task_data_t taskData = {this, xTaskGetCurrentTaskHandle(), 0, nullptr};
+    m_pTaskData = &taskData;
     int rc = 0;
 
     /* Try to connect the the advertiser.  Allow 30 seconds (30000 ms) for
@@ -213,39 +218,43 @@ bool NimBLEClient::connect(const NimBLEAddress &address, bool deleteAttibutes) {
                              NimBLEClient::handleGapEvent, this);
         switch (rc) {
             case 0:
-                m_pTaskData = &taskData;
                 break;
 
             case BLE_HS_EBUSY:
                 // Scan was still running, stop it and try again
                 if (!NimBLEDevice::getScan()->stop()) {
-                    return false;
+                    rc = BLE_HS_EUNKNOWN;
                 }
                 break;
 
             case BLE_HS_EDONE:
                 // A connection to this device already exists, do not connect twice.
                 NIMBLE_LOGE(LOG_TAG, "Already connected to device; addr=%s",
                             std::string(m_peerAddress).c_str());
-                return false;
+                break;
 
             case BLE_HS_EALREADY:
                 // Already attemting to connect to this device, cancel the previous
                 // attempt and report failure here so we don't get 2 connections.
                 NIMBLE_LOGE(LOG_TAG, "Already attempting to connect to %s - cancelling",
                             std::string(m_peerAddress).c_str());
                 ble_gap_conn_cancel();
-                return false;
+                break;
 
             default:
                 NIMBLE_LOGE(LOG_TAG, "Failed to connect to %s, rc=%d; %s",
                             std::string(m_peerAddress).c_str(),
                             rc, NimBLEUtils::returnCodeToString(rc));
-                return false;
+                break;
         }
 
     } while (rc == BLE_HS_EBUSY);
 
+    if(rc != 0) {
+        m_pTaskData = nullptr;
+        return false;
+    }
+
     // Wait for the connect timeout time +1 second for the connection to complete
     if(ulTaskNotifyTake(pdTRUE, pdMS_TO_TICKS(m_connectTimeout + 1000)) == pdFALSE) {
         m_pTaskData = nullptr;
@@ -255,7 +264,7 @@ bool NimBLEClient::connect(const NimBLEAddress &address, bool deleteAttibutes) {
             disconnect();
         } else {
         // workaround; if the controller doesn't cancel the connection
-        // at the timeout cancel it here.
+        // at the timeout, cancel it here.
             NIMBLE_LOGE(LOG_TAG, "Connect timeout - cancelling");
             ble_gap_conn_cancel();
         }
@@ -269,7 +278,7 @@ bool NimBLEClient::connect(const NimBLEAddress &address, bool deleteAttibutes) {
         // If the failure was not a result of a disconnection
         // make sure we disconnect now to avoid dangling connections
         if(isConnected()) {
-            ble_gap_terminate(m_conn_id, BLE_ERR_REM_USER_CONN_TERM);
+            disconnect();
         }
         return false;
     } else {
@@ -325,26 +334,35 @@ bool NimBLEClient::secureConnection() {
  */
 int NimBLEClient::disconnect(uint8_t reason) {
     NIMBLE_LOGD(LOG_TAG, ">> disconnect()");
-
     int rc = 0;
-    if(isConnected()){
+    if(isConnected()) {
+        // If the timer was already started, ignore this call.
+        if(ble_npl_callout_is_active(&m_dcTimer)) {
+            NIMBLE_LOGI(LOG_TAG, "Already disconnecting, timer started");
+            return BLE_HS_EALREADY;
+        }
+
+        ble_gap_conn_desc desc;
+        if(ble_gap_conn_find(m_conn_id, &desc) != 0){
+            NIMBLE_LOGI(LOG_TAG, "Connection ID not found");
+            return BLE_HS_EALREADY;
+        }
+
+        // We use a timer to detect a controller error in the event that it does
+        // not inform the stack when disconnection is complete.
+        // This is a common error in certain esp-idf versions.
+        // The disconnect timeout time is the supervison timeout time + 1 second.
+        // In the case that the event happenss shortly after the supervision timeout
+        // we don't want to prematurely reset the host.
+        ble_npl_time_t ticks;
+        ble_npl_time_ms_to_ticks((desc.supervision_timeout + 100) * 10, &ticks);
+        ble_npl_callout_reset(&m_dcTimer, ticks);
+
         rc = ble_gap_terminate(m_conn_id, reason);
-        if (rc == 0) {
-            ble_addr_t peerAddr_t;
-            memcpy(&peerAddr_t.val, m_peerAddress.getNative(),6);
-            peerAddr_t.type = m_peerAddress.getType();
-
-            // Set the disconnect timeout to the supervison timeout time + 1 second
-            // In case the event triggers shortly after the supervision timeout.
-            // We don't want to prematurely reset the host.
-            ble_gap_conn_desc desc;
-            if(ble_gap_conn_find_by_addr(&peerAddr_t, &desc) == 0){
-                ble_npl_time_t ticks;
-                ble_npl_time_ms_to_ticks((desc.supervision_timeout + 100) * 10, &ticks);
-                ble_npl_callout_reset(&m_dcTimer, ticks);
-                NIMBLE_LOGD(LOG_TAG, "DC TIMEOUT = %dms", (desc.supervision_timeout + 100) * 10);
+        if (rc != 0) {
+            if(rc != BLE_HS_EALREADY) {
+                ble_npl_callout_stop(&m_dcTimer);
             }
-        } else if (rc != BLE_HS_EALREADY) {
             NIMBLE_LOGE(LOG_TAG, "ble_gap_terminate failed: rc=%d %s",
                         rc, NimBLEUtils::returnCodeToString(rc));
         }
@@ -797,14 +815,15 @@ uint16_t NimBLEClient::getMTU() {
                     break;
             }
 
-            client->m_conn_id = BLE_HS_CONN_HANDLE_NONE;
-
             // Stop the disconnect timer since we are now disconnected.
             ble_npl_callout_stop(&client->m_dcTimer);
 
             // Remove the device from ignore list so we will scan it again
             NimBLEDevice::removeIgnored(client->m_peerAddress);
 
+            // No longer connected, clear the connection ID.
+            client->m_conn_id = BLE_HS_CONN_HANDLE_NONE;
+
             // If we received a connected event but did not get established (no PDU)
             // then a disconnect event will be sent but we should not send it to the
             // app for processing. Instead we will ensure the task is released