[Bugfix] Incorrect Client connection state tracking and self delete

This adds better client state tracking so that functions like NimBLEDevice::getDisconnectedClient get a more accurate state and will not return a connecting client.

This also fixes the client self delete on connection error where function call errors did not delete the client

Author: h2zero

src/NimBLEClient.cpp

@@ -68,6 +68,7 @@ NimBLEClient::NimBLEClient(const NimBLEAddress& peerAddress)
       m_terminateFailCount{0},
       m_asyncSecureAttempt{0},
       m_config{},
+      m_connStatus{DISCONNECTED},
 # if MYNEWT_VAL(BLE_EXT_ADV)
       m_phyMask{BLE_GAP_LE_PHY_1M_MASK | BLE_GAP_LE_PHY_2M_MASK | BLE_GAP_LE_PHY_CODED_MASK},
 # endif
@@ -171,35 +172,30 @@ bool NimBLEClient::connect(bool deleteAttributes, bool asyncConnect, bool exchan
  */
 bool NimBLEClient::connect(const NimBLEAddress& address, bool deleteAttributes, bool asyncConnect, bool exchangeMTU) {
     NIMBLE_LOGD(LOG_TAG, ">> connect(%s)", address.toString().c_str());
+    NimBLETaskData    taskData(this);
+    const ble_addr_t* peerAddr = address.getBase();
+    int               rc       = 0;
 
     if (!NimBLEDevice::m_synced) {
         NIMBLE_LOGE(LOG_TAG, "Host reset, wait for sync.");
-        return false;
+        rc = BLE_HS_ECONTROLLER;
+        goto error;
     }
 
     if (isConnected()) {
         NIMBLE_LOGE(LOG_TAG, "Client already connected");
-        return false;
-    }
-
-    const ble_addr_t* peerAddr = address.getBase();
-    if (ble_gap_conn_find_by_addr(peerAddr, NULL) == 0) {
-        NIMBLE_LOGE(LOG_TAG, "A connection to %s already exists", address.toString().c_str());
-        return false;
+        rc = BLE_HS_EREJECT;
+        goto error;
     }
 
     if (address.isNull()) {
         NIMBLE_LOGE(LOG_TAG, "Invalid peer address; (NULL)");
-        return false;
-    } else {
-        m_peerAddress = address;
-    }
-
-    if (deleteAttributes) {
-        deleteServices();
+        rc = BLE_HS_EINVAL;
+        goto error;
     }
 
-    int rc                = 0;
+    m_connStatus          = CONNECTING;
+    m_peerAddress         = address;
     m_config.asyncConnect = asyncConnect;
     m_config.exchangeMTU  = exchangeMTU;
 
@@ -259,16 +255,18 @@ bool NimBLEClient::connect(const NimBLEAddress& address, bool deleteAttributes,
 
     } while (rc == BLE_HS_EBUSY);
 
+    if (deleteAttributes) {
+        deleteServices();
+    }
+
     if (rc != 0) {
-        m_lastErr = rc;
-        return false;
+        goto error;
     }
 
     if (m_config.asyncConnect) {
         return true;
     }
 
-    NimBLETaskData taskData(this);
     m_pTaskData = &taskData;
 
     // Wait for the connect timeout time +1 second for the connection to complete
@@ -288,17 +286,21 @@ bool NimBLEClient::connect(const NimBLEAddress& address, bool deleteAttributes,
     rc          = taskData.m_flags;
     if (rc != 0) {
         NIMBLE_LOGE(LOG_TAG, "Connection failed; status=%d %s", rc, NimBLEUtils::returnCodeToString(rc));
-        m_lastErr = rc;
-        if (m_config.deleteOnConnectFail) {
-            NimBLEDevice::deleteClient(this);
-        }
-        return false;
+        goto error;
     }
 
     m_pClientCallbacks->onConnect(this);
     NIMBLE_LOGD(LOG_TAG, "<< connect()");
     // Check if still connected before returning
     return isConnected();
+
+error:
+    m_connStatus = DISCONNECTED;
+    m_lastErr    = rc;
+    if (m_config.deleteOnConnectFail) {
+        NimBLEDevice::deleteClient(this);
+    }
+    return false;
 } // connect
 
 /**
@@ -358,6 +360,7 @@ bool NimBLEClient::disconnect(uint8_t reason) {
         return false;
     }
 
+    m_connStatus = DISCONNECTING;
     return true;
 } // disconnect
 
@@ -976,6 +979,7 @@ int NimBLEClient::handleGapEvent(struct ble_gap_event* event, void* arg) {
                 pClient->m_pClientCallbacks->onDisconnect(pClient, rc);
             }
 
+            pClient->m_connStatus = DISCONNECTED;
             pClient->m_connHandle = BLE_HS_CONN_HANDLE_NONE;
 
             if (pClient->m_config.deleteOnDisconnect) {
@@ -1005,6 +1009,7 @@ int NimBLEClient::handleGapEvent(struct ble_gap_event* event, void* arg) {
             }
 
             if (rc == 0) {
+                pClient->m_connStatus = CONNECTED;
                 pClient->m_connHandle = event->connect.conn_handle;
 
                 if (pClient->m_config.asyncConnect) {
@@ -1020,6 +1025,7 @@ int NimBLEClient::handleGapEvent(struct ble_gap_event* event, void* arg) {
                     return 0; // return as we may have a task waiting for the MTU before releasing it.
                 }
             } else {
+                pClient->m_connStatus = DISCONNECTED;
                 pClient->m_connHandle = BLE_HS_CONN_HANDLE_NONE;
 
                 if (pClient->m_config.asyncConnect) {
@@ -1258,7 +1264,7 @@ int NimBLEClient::handleGapEvent(struct ble_gap_event* event, void* arg) {
  * @return True if we are connected and false if we are not connected.
  */
 bool NimBLEClient::isConnected() const {
-    return m_connHandle != BLE_HS_CONN_HANDLE_NONE;
+    return m_connStatus != DISCONNECTED;
 } // isConnected
 
 /**

src/NimBLEClient.h

@@ -113,6 +113,8 @@ class NimBLEClient {
     void   setConfig(Config config);
 
   private:
+    enum ConnStatus : uint8_t { CONNECTED, DISCONNECTED, CONNECTING, DISCONNECTING };
+
     NimBLEClient(const NimBLEAddress& peerAddress);
     ~NimBLEClient();
     NimBLEClient(const NimBLEClient&)            = delete;
@@ -136,6 +138,7 @@ class NimBLEClient {
     uint8_t                           m_terminateFailCount;
     mutable uint8_t                   m_asyncSecureAttempt;
     Config                            m_config;
+    ConnStatus                        m_connStatus;
 
 # if MYNEWT_VAL(BLE_EXT_ADV)
     uint8_t m_phyMask;