From e00dd88add44dccd511168b2a29b4b10450726f3 Mon Sep 17 00:00:00 2001 From: h2zero Date: Wed, 23 Apr 2025 16:02:34 -0600 Subject: [PATCH] [Bugfix] notify/indicate incorrectly returning success with custom value When sending an array of data with a length value the wrong overload/template was being used and the length value was being interpreted as the connection handle. This updates the template to disable it when an array is passed and will now also report the error. --- src/NimBLECharacteristic.cpp | 48 ++++++++++++++++++++++++------------ src/NimBLECharacteristic.h | 14 +++++++---- 2 files changed, 41 insertions(+), 21 deletions(-) diff --git a/src/NimBLECharacteristic.cpp b/src/NimBLECharacteristic.cpp index 569e00b..42d7f6e 100644 --- a/src/NimBLECharacteristic.cpp +++ b/src/NimBLECharacteristic.cpp @@ -268,17 +268,32 @@ bool NimBLECharacteristic::sendValue(const uint8_t* value, size_t length, bool i int rc = 0; if (value != nullptr && length > 0) { // custom notification value - // Notify all connected peers unless a specific handle is provided - for (const auto& ch : NimBLEDevice::getServer()->getPeerDevices()) { - if (connHandle != BLE_HS_CONN_HANDLE_NONE && ch != connHandle) { - continue; // only send to the specific handle, minor inefficiency but saves code. + os_mbuf* om = nullptr; + + if (connHandle != BLE_HS_CONN_HANDLE_NONE) { // only sending to specific peer + om = ble_hs_mbuf_from_flat(value, length); + if (!om) { + rc = BLE_HS_ENOMEM; + goto done; } + // Null buffer will read the value from the characteristic + if (isNotification) { + rc = ble_gattc_notify_custom(connHandle, m_handle, om); + } else { + rc = ble_gattc_indicate_custom(connHandle, m_handle, om); + } + + goto done; + } + + // Notify all connected peers unless a specific handle is provided + for (const auto& ch : NimBLEDevice::getServer()->getPeerDevices()) { // Must re-create the data buffer on each iteration because it is freed by the calls bellow. - os_mbuf* om = ble_hs_mbuf_from_flat(value, length); + om = ble_hs_mbuf_from_flat(value, length); if (!om) { - NIMBLE_LOGE(LOG_TAG, "<< sendValue: failed to allocate mbuf"); - return false; + rc = BLE_HS_ENOMEM; + goto done; } if (isNotification) { @@ -286,24 +301,25 @@ bool NimBLECharacteristic::sendValue(const uint8_t* value, size_t length, bool i } else { rc = ble_gattc_indicate_custom(ch, m_handle, om); } - - if (rc != 0) { - NIMBLE_LOGE(LOG_TAG, "<< sendValue: failed to send value, rc=%d %s", rc, NimBLEUtils::returnCodeToString(rc)); - break; - } } - } else if (connHandle != BLE_HS_CONN_HANDLE_NONE) { // only sending to specific peer + } else if (connHandle != BLE_HS_CONN_HANDLE_NONE) { // Null buffer will read the value from the characteristic if (isNotification) { - rc = ble_gattc_notify_custom(connHandle, m_handle, NULL); + rc = ble_gattc_notify_custom(connHandle, m_handle, nullptr); } else { - rc = ble_gattc_indicate_custom(connHandle, m_handle, NULL); + rc = ble_gattc_indicate_custom(connHandle, m_handle, nullptr); } } else { // Notify or indicate to all connected peers the characteristic value ble_gatts_chr_updated(m_handle); } - return rc == 0; +done: + if (rc != 0) { + NIMBLE_LOGE(LOG_TAG, "failed to send value, rc=%d %s", rc, NimBLEUtils::returnCodeToString(rc)); + return false; + } + + return true; } // sendValue void NimBLECharacteristic::readEvent(NimBLEConnInfo& connInfo) { diff --git a/src/NimBLECharacteristic.h b/src/NimBLECharacteristic.h index 3ada23a..b26141d 100644 --- a/src/NimBLECharacteristic.h +++ b/src/NimBLECharacteristic.h @@ -87,7 +87,9 @@ class NimBLECharacteristic : public NimBLELocalValueAttribute { # ifdef _DOXYGEN_ bool # else - typename std::enable_if::value && !Has_c_str_length::value && !Has_data_size::value, bool>::type + typename std::enable_if::value && !std::is_array::value && !Has_c_str_length::value && + !Has_data_size::value, + bool>::type # endif notify(const T& v, uint16_t connHandle = BLE_HS_CONN_HANDLE_NONE) const { return notify(reinterpret_cast(&v), sizeof(T), connHandle); @@ -133,7 +135,9 @@ class NimBLECharacteristic : public NimBLELocalValueAttribute { # ifdef _DOXYGEN_ bool # else - typename std::enable_if::value && !Has_c_str_length::value && !Has_data_size::value, bool>::type + typename std::enable_if::value && !std::is_array::value && !Has_c_str_length::value && + !Has_data_size::value, + bool>::type # endif indicate(const T& v, uint16_t connHandle = BLE_HS_CONN_HANDLE_NONE) const { return indicate(reinterpret_cast(&v), sizeof(T), connHandle); @@ -182,8 +186,8 @@ class NimBLECharacteristic : public NimBLELocalValueAttribute { * @note This function is only available if the type T is not a pointer. */ template - typename std::enable_if::value, bool>::type notify(const T& value, - uint16_t connHandle = BLE_HS_CONN_HANDLE_NONE) const { + typename std::enable_if::value && !std::is_array::value, bool>::type notify( + const T& value, uint16_t connHandle = BLE_HS_CONN_HANDLE_NONE) const { if constexpr (Has_data_size::value) { return notify(reinterpret_cast(value.data()), value.size(), connHandle); } else if constexpr (Has_c_str_length::value) { @@ -204,7 +208,7 @@ class NimBLECharacteristic : public NimBLELocalValueAttribute { * @note This function is only available if the type T is not a pointer. */ template - typename std::enable_if::value, bool>::type indicate( + typename std::enable_if::value && !std::is_array::value, bool>::type indicate( const T& value, uint16_t connHandle = BLE_HS_CONN_HANDLE_NONE) const { if constexpr (Has_data_size::value) { return indicate(reinterpret_cast(value.data()), value.size(), connHandle);