From ee11f71b91e5b535a03de7c50c40be29a13d0a82 Mon Sep 17 00:00:00 2001 From: Jimmy Durand Wesolowski Date: Thu, 29 Oct 2020 12:58:16 +0100 Subject: [PATCH 1/3] BLERemoteChar: fix descriptor 2902 write for characteristic notifications When registering a notification on a characteristic, the 2902 descriptor (CCCD) value is set to 1 (or 2 for indication). According to the BLUETOOTH CORE SPECIFICATION Version 5.2, Revision Date 2019-12-31, section 4.12.3 "Write Characteristic Descriptors" (page 1588), the characteristic descriptor write must expect a response. Currently, the descriptor write is performed without expecting a reponse, which prevents the notification to be functional with some BLE stacks. This commit modify the write to expect the response. Signed-off-by: Jimmy Durand Wesolowski --- libraries/BLE/src/BLERemoteCharacteristic.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/BLE/src/BLERemoteCharacteristic.cpp b/libraries/BLE/src/BLERemoteCharacteristic.cpp index ba7111c1b05..55e3ad970b7 100644 --- a/libraries/BLE/src/BLERemoteCharacteristic.cpp +++ b/libraries/BLE/src/BLERemoteCharacteristic.cpp @@ -468,7 +468,7 @@ void BLERemoteCharacteristic::registerForNotify(notify_callback notifyCallback, if(!notifications) val[0] = 0x02; BLERemoteDescriptor* desc = getDescriptor(BLEUUID((uint16_t)0x2902)); if (desc != nullptr) - desc->writeValue(val, 2); + desc->writeValue(val, 2, true); } // End Register else { // If we weren't passed a callback function, then this is an unregistration. esp_err_t errRc = ::esp_ble_gattc_unregister_for_notify( @@ -484,7 +484,7 @@ void BLERemoteCharacteristic::registerForNotify(notify_callback notifyCallback, uint8_t val[] = {0x00, 0x00}; BLERemoteDescriptor* desc = getDescriptor((uint16_t)0x2902); if (desc != nullptr) - desc->writeValue(val, 2); + desc->writeValue(val, 2, true); } // End Unregister m_semaphoreRegForNotifyEvt.wait("registerForNotify"); From df6a2cc04b5b23fdbc9f25fecf0de9e3e08ae854 Mon Sep 17 00:00:00 2001 From: Jimmy Durand Wesolowski Date: Thu, 29 Oct 2020 13:49:20 +0100 Subject: [PATCH 2/3] BLERemoteChar: forward GATT client event to characteristic descriptors This commits prevents a permanent wait when calling BLERemoteDescriptor readValue function, on the m_semaphoreReadDescrEvt semaphore. ESP32 BLE stack calls to remote characteristic - notification, - value read - value write and remote characteristic descriptor - value read are asynchronous. When such a call is performed by this library, a semaphore is taken prior to the BLE stack read or write operation, and waited on after it. Releasing the semaphore is done by the characteristic event handling function (gattClientEventHandler), when the appropriate event is received. However, the characteristic descriptor events are discarded, and the value read semaphore is never released. This commits forwards the GATT client events from the remote characteristic down to their remote characteristic descriptor, and implements their event handling. Adding a semaphore for the remote characteristic descriptor value write will be done in a separate commit. Signed-off-by: Jimmy Durand Wesolowski --- libraries/BLE/src/BLERemoteCharacteristic.cpp | 7 +++++++ libraries/BLE/src/BLERemoteDescriptor.cpp | 16 ++++++++++++++++ libraries/BLE/src/BLERemoteDescriptor.h | 2 +- 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/libraries/BLE/src/BLERemoteCharacteristic.cpp b/libraries/BLE/src/BLERemoteCharacteristic.cpp index 55e3ad970b7..c6ab12d42ba 100644 --- a/libraries/BLE/src/BLERemoteCharacteristic.cpp +++ b/libraries/BLE/src/BLERemoteCharacteristic.cpp @@ -237,6 +237,13 @@ void BLERemoteCharacteristic::gattClientEventHandler(esp_gattc_cb_event_t event, break; } // ESP_GATTC_WRITE_CHAR_EVT + case ESP_GATTC_READ_DESCR_EVT: + case ESP_GATTC_WRITE_DESCR_EVT: + for (auto &myPair : m_descriptorMap) { + myPair.second->gattClientEventHandler( + event, gattc_if, evtParam); + } + break; default: break; diff --git a/libraries/BLE/src/BLERemoteDescriptor.cpp b/libraries/BLE/src/BLERemoteDescriptor.cpp index ad506aae9b3..cc895be5cd4 100644 --- a/libraries/BLE/src/BLERemoteDescriptor.cpp +++ b/libraries/BLE/src/BLERemoteDescriptor.cpp @@ -49,6 +49,22 @@ BLEUUID BLERemoteDescriptor::getUUID() { return m_uuid; } // getUUID +void BLERemoteDescriptor::gattClientEventHandler(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if, esp_ble_gattc_cb_param_t* evtParam) { + switch(event) { + case ESP_GATTC_READ_DESCR_EVT: + if (evtParam->read.handle != getHandle()) + break; + m_semaphoreReadDescrEvt.give(); + break; + + case ESP_GATTC_WRITE_DESCR_EVT: + if (evtParam->write.handle != getHandle()) + break; + break; + default: + break; + } +} std::string BLERemoteDescriptor::readValue() { log_v(">> readValue: %s", toString().c_str()); diff --git a/libraries/BLE/src/BLERemoteDescriptor.h b/libraries/BLE/src/BLERemoteDescriptor.h index 29efe573bfe..351ddb99cd6 100644 --- a/libraries/BLE/src/BLERemoteDescriptor.h +++ b/libraries/BLE/src/BLERemoteDescriptor.h @@ -35,7 +35,7 @@ class BLERemoteDescriptor { void writeValue(std::string newValue, bool response = false); void writeValue(uint8_t newValue, bool response = false); void setAuth(esp_gatt_auth_req_t auth); - + void gattClientEventHandler(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if, esp_ble_gattc_cb_param_t* evtParam); private: friend class BLERemoteCharacteristic; From b475eef592958ae33edc2f0b7834577abaf4598f Mon Sep 17 00:00:00 2001 From: Jimmy Durand Wesolowski Date: Thu, 29 Oct 2020 18:19:51 +0100 Subject: [PATCH 3/3] BLERemoteDescriptor: add semaphore to characteristic descriptor write This adds a semaphore to characteristic descriptor value write, to mimic the value read function, and to ensure completion of the operation before we carry on. Signed-off-by: Jimmy Durand Wesolowski --- libraries/BLE/src/BLERemoteDescriptor.cpp | 5 +++++ libraries/BLE/src/BLERemoteDescriptor.h | 1 + 2 files changed, 6 insertions(+) diff --git a/libraries/BLE/src/BLERemoteDescriptor.cpp b/libraries/BLE/src/BLERemoteDescriptor.cpp index cc895be5cd4..b1e0bef221b 100644 --- a/libraries/BLE/src/BLERemoteDescriptor.cpp +++ b/libraries/BLE/src/BLERemoteDescriptor.cpp @@ -60,6 +60,7 @@ void BLERemoteDescriptor::gattClientEventHandler(esp_gattc_cb_event_t event, esp case ESP_GATTC_WRITE_DESCR_EVT: if (evtParam->write.handle != getHandle()) break; + m_semaphoreWriteDescrEvt.give(); break; default: break; @@ -153,6 +154,8 @@ void BLERemoteDescriptor::writeValue(uint8_t* data, size_t length, bool response return; } + m_semaphoreWriteDescrEvt.take("writeValue"); + esp_err_t errRc = ::esp_ble_gattc_write_char_descr( m_pRemoteCharacteristic->getRemoteService()->getClient()->getGattcIf(), m_pRemoteCharacteristic->getRemoteService()->getClient()->getConnId(), @@ -165,6 +168,8 @@ void BLERemoteDescriptor::writeValue(uint8_t* data, size_t length, bool response if (errRc != ESP_OK) { log_e("esp_ble_gattc_write_char_descr: %d", errRc); } + + m_semaphoreWriteDescrEvt.wait("writeValue"); log_v("<< writeValue"); } // writeValue diff --git a/libraries/BLE/src/BLERemoteDescriptor.h b/libraries/BLE/src/BLERemoteDescriptor.h index 351ddb99cd6..ebd847f2c24 100644 --- a/libraries/BLE/src/BLERemoteDescriptor.h +++ b/libraries/BLE/src/BLERemoteDescriptor.h @@ -49,6 +49,7 @@ class BLERemoteDescriptor { std::string m_value; // Last received value of the descriptor. BLERemoteCharacteristic* m_pRemoteCharacteristic; // Reference to the Remote characteristic of which this descriptor is associated. FreeRTOS::Semaphore m_semaphoreReadDescrEvt = FreeRTOS::Semaphore("ReadDescrEvt"); + FreeRTOS::Semaphore m_semaphoreWriteDescrEvt = FreeRTOS::Semaphore("WriteDescrEvt"); esp_gatt_auth_req_t m_auth;