From 026864e031c12a7d77bcd6e79e77270528f0375d Mon Sep 17 00:00:00 2001 From: h2zero Date: Wed, 20 Jan 2021 12:28:07 -0700 Subject: [PATCH] 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. --- src/NimBLEClient.cpp | 85 +++++++++++++++++++++++++++----------------- 1 file changed, 52 insertions(+), 33 deletions(-) diff --git a/src/NimBLEClient.cpp b/src/NimBLEClient.cpp index ddd3abe..1919a06 100644 --- a/src/NimBLEClient.cpp +++ b/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,13 +218,12 @@ 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; @@ -227,7 +231,7 @@ bool NimBLEClient::connect(const NimBLEAddress &address, bool deleteAttibutes) { // 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 @@ -235,17 +239,22 @@ bool NimBLEClient::connect(const NimBLEAddress &address, bool deleteAttibutes) { 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()){ - 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(); + 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; + } - // 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); + 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) { + 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