Skip to content

Commit ca5708f

Browse files
committed
[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.
1 parent bdeb084 commit ca5708f

File tree

2 files changed

+41
-21
lines changed

2 files changed

+41
-21
lines changed

src/NimBLECharacteristic.cpp

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -268,42 +268,58 @@ bool NimBLECharacteristic::sendValue(const uint8_t* value, size_t length, bool i
268268
int rc = 0;
269269

270270
if (value != nullptr && length > 0) { // custom notification value
271-
// Notify all connected peers unless a specific handle is provided
272-
for (const auto& ch : NimBLEDevice::getServer()->getPeerDevices()) {
273-
if (connHandle != BLE_HS_CONN_HANDLE_NONE && ch != connHandle) {
274-
continue; // only send to the specific handle, minor inefficiency but saves code.
271+
os_mbuf* om = nullptr;
272+
273+
if (connHandle != BLE_HS_CONN_HANDLE_NONE) { // only sending to specific peer
274+
om = ble_hs_mbuf_from_flat(value, length);
275+
if (!om) {
276+
rc = BLE_HS_ENOMEM;
277+
goto done;
278+
}
279+
280+
// Null buffer will read the value from the characteristic
281+
if (isNotification) {
282+
rc = ble_gattc_notify_custom(connHandle, m_handle, om);
283+
} else {
284+
rc = ble_gattc_indicate_custom(connHandle, m_handle, om);
275285
}
276286

287+
goto done;
288+
}
289+
290+
// Notify all connected peers unless a specific handle is provided
291+
for (const auto& ch : NimBLEDevice::getServer()->getPeerDevices()) {
277292
// Must re-create the data buffer on each iteration because it is freed by the calls bellow.
278-
os_mbuf* om = ble_hs_mbuf_from_flat(value, length);
293+
om = ble_hs_mbuf_from_flat(value, length);
279294
if (!om) {
280-
NIMBLE_LOGE(LOG_TAG, "<< sendValue: failed to allocate mbuf");
281-
return false;
295+
rc = BLE_HS_ENOMEM;
296+
goto done;
282297
}
283298

284299
if (isNotification) {
285300
rc = ble_gattc_notify_custom(ch, m_handle, om);
286301
} else {
287302
rc = ble_gattc_indicate_custom(ch, m_handle, om);
288303
}
289-
290-
if (rc != 0) {
291-
NIMBLE_LOGE(LOG_TAG, "<< sendValue: failed to send value, rc=%d %s", rc, NimBLEUtils::returnCodeToString(rc));
292-
break;
293-
}
294304
}
295-
} else if (connHandle != BLE_HS_CONN_HANDLE_NONE) { // only sending to specific peer
305+
} else if (connHandle != BLE_HS_CONN_HANDLE_NONE) {
296306
// Null buffer will read the value from the characteristic
297307
if (isNotification) {
298-
rc = ble_gattc_notify_custom(connHandle, m_handle, NULL);
308+
rc = ble_gattc_notify_custom(connHandle, m_handle, nullptr);
299309
} else {
300-
rc = ble_gattc_indicate_custom(connHandle, m_handle, NULL);
310+
rc = ble_gattc_indicate_custom(connHandle, m_handle, nullptr);
301311
}
302312
} else { // Notify or indicate to all connected peers the characteristic value
303313
ble_gatts_chr_updated(m_handle);
304314
}
305315

306-
return rc == 0;
316+
done:
317+
if (rc != 0) {
318+
NIMBLE_LOGE(LOG_TAG, "failed to send value, rc=%d %s", rc, NimBLEUtils::returnCodeToString(rc));
319+
return false;
320+
}
321+
322+
return true;
307323
} // sendValue
308324

309325
void NimBLECharacteristic::readEvent(NimBLEConnInfo& connInfo) {

src/NimBLECharacteristic.h

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,9 @@ class NimBLECharacteristic : public NimBLELocalValueAttribute {
8787
# ifdef _DOXYGEN_
8888
bool
8989
# else
90-
typename std::enable_if<!std::is_pointer<T>::value && !Has_c_str_length<T>::value && !Has_data_size<T>::value, bool>::type
90+
typename std::enable_if<!std::is_pointer<T>::value && !std::is_array<T>::value && !Has_c_str_length<T>::value &&
91+
!Has_data_size<T>::value,
92+
bool>::type
9193
# endif
9294
notify(const T& v, uint16_t connHandle = BLE_HS_CONN_HANDLE_NONE) const {
9395
return notify(reinterpret_cast<const uint8_t*>(&v), sizeof(T), connHandle);
@@ -133,7 +135,9 @@ class NimBLECharacteristic : public NimBLELocalValueAttribute {
133135
# ifdef _DOXYGEN_
134136
bool
135137
# else
136-
typename std::enable_if<!std::is_pointer<T>::value && !Has_c_str_length<T>::value && !Has_data_size<T>::value, bool>::type
138+
typename std::enable_if<!std::is_pointer<T>::value && !std::is_array<T>::value && !Has_c_str_length<T>::value &&
139+
!Has_data_size<T>::value,
140+
bool>::type
137141
# endif
138142
indicate(const T& v, uint16_t connHandle = BLE_HS_CONN_HANDLE_NONE) const {
139143
return indicate(reinterpret_cast<const uint8_t*>(&v), sizeof(T), connHandle);
@@ -182,8 +186,8 @@ class NimBLECharacteristic : public NimBLELocalValueAttribute {
182186
* @note This function is only available if the type T is not a pointer.
183187
*/
184188
template <typename T>
185-
typename std::enable_if<!std::is_pointer<T>::value, bool>::type notify(const T& value,
186-
uint16_t connHandle = BLE_HS_CONN_HANDLE_NONE) const {
189+
typename std::enable_if<!std::is_pointer<T>::value && !std::is_array<T>::value, bool>::type notify(
190+
const T& value, uint16_t connHandle = BLE_HS_CONN_HANDLE_NONE) const {
187191
if constexpr (Has_data_size<T>::value) {
188192
return notify(reinterpret_cast<const uint8_t*>(value.data()), value.size(), connHandle);
189193
} else if constexpr (Has_c_str_length<T>::value) {
@@ -204,7 +208,7 @@ class NimBLECharacteristic : public NimBLELocalValueAttribute {
204208
* @note This function is only available if the type T is not a pointer.
205209
*/
206210
template <typename T>
207-
typename std::enable_if<!std::is_pointer<T>::value, bool>::type indicate(
211+
typename std::enable_if<!std::is_pointer<T>::value && !std::is_array<T>::value, bool>::type indicate(
208212
const T& value, uint16_t connHandle = BLE_HS_CONN_HANDLE_NONE) const {
209213
if constexpr (Has_data_size<T>::value) {
210214
return indicate(reinterpret_cast<const uint8_t*>(value.data()), value.size(), connHandle);

0 commit comments

Comments
 (0)