From 946b9717501a56ea4f5ddb7b701f820915ab0347 Mon Sep 17 00:00:00 2001 From: h2zero Date: Mon, 17 May 2021 14:37:03 -0600 Subject: [PATCH] Properly find the remote characteristic end handle for descriptor discovery. Previously we used the service end handle or, if available, the handle of the next characteristic in the service as the end handle when retrieving the descriptors of a characteristic. This process would fail when connecting to some devices if the characteristics were retrieved individually instead of all together. The cause of failure was requesting a handle range that is out of characteristic scope which is also out of line with BLE specifications. The fix included in this is to set the end handles of the characteristics either after retrieving all the characteristics in a service by using the next charactertistic definition handle or, if the characteristic was retrieved separately, we retrieve the next characteristic just before retrieving the descriptors. --- src/NimBLERemoteCharacteristic.cpp | 96 ++++++++++++++++++++++-------- src/NimBLERemoteCharacteristic.h | 3 + src/NimBLERemoteService.cpp | 31 +++++----- src/NimBLERemoteService.h | 1 - 4 files changed, 89 insertions(+), 42 deletions(-) diff --git a/src/NimBLERemoteCharacteristic.cpp b/src/NimBLERemoteCharacteristic.cpp index 8bc5090..03b9cc5 100644 --- a/src/NimBLERemoteCharacteristic.cpp +++ b/src/NimBLERemoteCharacteristic.cpp @@ -55,6 +55,7 @@ static const char* LOG_TAG = "NimBLERemoteCharacteristic"; m_handle = chr->val_handle; m_defHandle = chr->def_handle; + m_endHandle = 0; m_charProp = chr->properties; m_pRemoteService = pRemoteService; m_notifyCallback = nullptr; @@ -146,31 +147,25 @@ int NimBLERemoteCharacteristic::descriptorDiscCB(uint16_t conn_handle, const struct ble_gatt_dsc *dsc, void *arg) { - NIMBLE_LOGD(LOG_TAG,"Descriptor Discovered >> status: %d handle: %d", - error->status, (error->status == 0) ? dsc->handle : -1); + int rc = error->status; + NIMBLE_LOGD(LOG_TAG, "Descriptor Discovered >> status: %d handle: %d", + rc, (rc == 0) ? dsc->handle : -1); desc_filter_t *filter = (desc_filter_t*)arg; const NimBLEUUID *uuid_filter = filter->uuid; ble_task_data_t *pTaskData = (ble_task_data_t*)filter->task_data; NimBLERemoteCharacteristic *characteristic = (NimBLERemoteCharacteristic*)pTaskData->pATT; - int rc=0; - if(characteristic->getRemoteService()->getClient()->getConnId() != conn_handle){ + if (characteristic->getRemoteService()->getClient()->getConnId() != conn_handle){ return 0; } - switch (error->status) { + switch (rc) { case 0: { - if(dsc->uuid.u.type == BLE_UUID_TYPE_16 && dsc->uuid.u16.value == uint16_t(0x2803)) { - NIMBLE_LOGD(LOG_TAG,"Descriptor NOT found - end of Characteristic definintion"); - rc = BLE_HS_EDONE; - break; - } - if(uuid_filter != nullptr) { - if(ble_uuid_cmp(&uuid_filter->getNative()->u, &dsc->uuid.u) != 0) { + if (uuid_filter != nullptr) { + if (ble_uuid_cmp(&uuid_filter->getNative()->u, &dsc->uuid.u) != 0) { return 0; } else { - NIMBLE_LOGD(LOG_TAG,"Descriptor Found"); rc = BLE_HS_EDONE; } } @@ -180,11 +175,10 @@ int NimBLERemoteCharacteristic::descriptorDiscCB(uint16_t conn_handle, break; } default: - rc = error->status; break; } - /** If rc == BLE_HS_EDONE, resume the task with a success error code and stop the discovery process. + /* If rc == BLE_HS_EDONE, resume the task with a success error code and stop the discovery process. * Else if rc == 0, just return 0 to continue the discovery until we get BLE_HS_EDONE. * If we get any other error code tell the application to abort by returning non-zero in the rc. */ @@ -202,6 +196,38 @@ int NimBLERemoteCharacteristic::descriptorDiscCB(uint16_t conn_handle, } +/** + * @brief callback from NimBLE when the next characteristic of the service is discovered. + */ +int NimBLERemoteCharacteristic::nextCharCB(uint16_t conn_handle, + const struct ble_gatt_error *error, + const struct ble_gatt_chr *chr, void *arg) +{ + int rc = error->status; + NIMBLE_LOGD(LOG_TAG, "Next Characteristic >> status: %d handle: %d", + rc, (rc == 0) ? chr->val_handle : -1); + + ble_task_data_t *pTaskData = (ble_task_data_t*)arg; + NimBLERemoteCharacteristic *pChar = (NimBLERemoteCharacteristic*)pTaskData->pATT; + + if (pChar->getRemoteService()->getClient()->getConnId() != conn_handle) { + return 0; + } + + if (rc == 0) { + pChar->m_endHandle = chr->def_handle - 1; + rc = BLE_HS_EDONE; + } else if (rc == BLE_HS_EDONE) { + pChar->m_endHandle = pChar->getRemoteService()->getEndHandle(); + } else { + pTaskData->rc = rc; + } + + xTaskNotifyGive(pTaskData->task); + return rc; +} + + /** * @brief Populate the descriptors (if any) for this characteristic. * @param [in] the end handle of the characteristic, or the service, whichever comes first. @@ -209,35 +235,57 @@ int NimBLERemoteCharacteristic::descriptorDiscCB(uint16_t conn_handle, bool NimBLERemoteCharacteristic::retrieveDescriptors(const NimBLEUUID *uuid_filter) { NIMBLE_LOGD(LOG_TAG, ">> retrieveDescriptors() for characteristic: %s", getUUID().toString().c_str()); - uint16_t endHandle = getRemoteService()->getEndHandle(this); - if(m_handle >= endHandle) { - return false; + // If this is the last handle then there are no descriptors + if (m_handle == getRemoteService()->getEndHandle()) { + return true; } int rc = 0; ble_task_data_t taskData = {this, xTaskGetCurrentTaskHandle(), 0, nullptr}; + + // If we don't know the end handle of this characteristic retrieve the next one in the service + // The end handle is the next characteristic definition handle -1. + if (m_endHandle == 0) { + rc = ble_gattc_disc_all_chrs(getRemoteService()->getClient()->getConnId(), + m_handle, + getRemoteService()->getEndHandle(), + NimBLERemoteCharacteristic::nextCharCB, + &taskData); + if (rc != 0) { + NIMBLE_LOGE(LOG_TAG, "Error getting end handle rc=%d", rc); + return false; + } + + ulTaskNotifyTake(pdTRUE, portMAX_DELAY); + + if (taskData.rc != 0) { + NIMBLE_LOGE(LOG_TAG, "Could not retrieve end handle rc=%d", taskData.rc); + return false; + } + } + desc_filter_t filter = {uuid_filter, &taskData}; rc = ble_gattc_disc_all_dscs(getRemoteService()->getClient()->getConnId(), m_handle, - endHandle, + m_endHandle, NimBLERemoteCharacteristic::descriptorDiscCB, &filter); if (rc != 0) { - NIMBLE_LOGE(LOG_TAG, "ble_gattc_disc_all_chrs: rc=%d %s", rc, NimBLEUtils::returnCodeToString(rc)); + NIMBLE_LOGE(LOG_TAG, "ble_gattc_disc_all_dscs: rc=%d %s", rc, NimBLEUtils::returnCodeToString(rc)); return false; } ulTaskNotifyTake(pdTRUE, portMAX_DELAY); - if(taskData.rc != 0) { - NIMBLE_LOGE(LOG_TAG, "ble_gattc_disc_all_chrs: startHandle:%d endHandle:%d taskData.rc=%d %s", m_handle, endHandle, taskData.rc, NimBLEUtils::returnCodeToString(0x0100+taskData.rc)); - return false; + if (taskData.rc != 0) { + NIMBLE_LOGE(LOG_TAG, "Failed to retrieve descriptors; startHandle:%d endHandle:%d taskData.rc=%d", + m_handle, m_endHandle, taskData.rc); } - return true; NIMBLE_LOGD(LOG_TAG, "<< retrieveDescriptors(): Found %d descriptors.", m_descriptorVector.size()); + return (taskData.rc == 0); } // retrieveDescriptors diff --git a/src/NimBLERemoteCharacteristic.h b/src/NimBLERemoteCharacteristic.h index 720a4da..39e6d40 100644 --- a/src/NimBLERemoteCharacteristic.h +++ b/src/NimBLERemoteCharacteristic.h @@ -148,12 +148,15 @@ private: static int descriptorDiscCB(uint16_t conn_handle, const struct ble_gatt_error *error, uint16_t chr_val_handle, const struct ble_gatt_dsc *dsc, void *arg); + static int nextCharCB(uint16_t conn_handle, const struct ble_gatt_error *error, + const struct ble_gatt_chr *chr, void *arg); // Private properties NimBLEUUID m_uuid; uint8_t m_charProp; uint16_t m_handle; uint16_t m_defHandle; + uint16_t m_endHandle; NimBLERemoteService* m_pRemoteService; std::string m_value; notify_callback m_notifyCallback; diff --git a/src/NimBLERemoteService.cpp b/src/NimBLERemoteService.cpp index cb20e0b..efb6662 100644 --- a/src/NimBLERemoteService.cpp +++ b/src/NimBLERemoteService.cpp @@ -223,6 +223,20 @@ bool NimBLERemoteService::retrieveCharacteristics(const NimBLEUUID *uuid_filter) ulTaskNotifyTake(pdTRUE, portMAX_DELAY); if(taskData.rc == 0){ + if (uuid_filter == nullptr) { + if (m_characteristicVector.size() > 1) { + for (auto it = m_characteristicVector.begin(); it != m_characteristicVector.end(); ++it ) { + auto nx = std::next(it, 1); + if (nx == m_characteristicVector.end()) { + break; + } + (*it)->m_endHandle = (*nx)->m_defHandle - 1; + } + } + + m_characteristicVector.back()->m_endHandle = getEndHandle(); + } + NIMBLE_LOGD(LOG_TAG, "<< retrieveCharacteristics()"); return true; } @@ -249,23 +263,6 @@ uint16_t NimBLERemoteService::getEndHandle() { return m_endHandle; } // getEndHandle -/** - * @brief Get the end handle of specified NimBLERemoteCharacteristic. - */ - -uint16_t NimBLERemoteService::getEndHandle(NimBLERemoteCharacteristic *pCharacteristic) { - uint16_t endHandle = m_endHandle; - - for(auto &it: m_characteristicVector) { - uint16_t defHandle = it->getDefHandle() - 1; - if(defHandle > pCharacteristic->getDefHandle() && endHandle > defHandle) { - endHandle = defHandle; - } - } - - return endHandle; -} // getEndHandle - /** * @brief Get the service start handle. diff --git a/src/NimBLERemoteService.h b/src/NimBLERemoteService.h index 4920844..751c9ef 100644 --- a/src/NimBLERemoteService.h +++ b/src/NimBLERemoteService.h @@ -70,7 +70,6 @@ private: uint16_t getStartHandle(); uint16_t getEndHandle(); - uint16_t getEndHandle(NimBLERemoteCharacteristic *pCharacteristic); void releaseSemaphores(); // Properties