Skip to content

Commit 6f19dea

Browse files
pan-pan-
committed
BLE: Fix GattServer::write on Nordic targets.
GattServer::write on Nordic's targets use sd_ble_gatts_hvx to send an handle value Notification or Indication; This function can fail if the connection handle is invalid or if Updates are not enabled for this connection. This patch workaround those limitations.
1 parent 4e22295 commit 6f19dea

File tree

4 files changed

+160
-85
lines changed

4 files changed

+160
-85
lines changed

‎features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/source/nRF5xGattServer.cpp

Lines changed: 75 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,34 @@
2626

2727
#include "nRF5xn.h"
2828

29+
namespace {
30+
31+
static ble_error_t set_attribute_value(
32+
Gap::Handle_t connectionHandle,
33+
GattAttribute::Handle_t attributeHandle,
34+
ble_gatts_value_t *value
35+
) {
36+
uint32_t err = sd_ble_gatts_value_set(connectionHandle, attributeHandle, value);
37+
switch(err) {
38+
case NRF_SUCCESS:
39+
return BLE_ERROR_NONE;
40+
case NRF_ERROR_INVALID_ADDR:
41+
case NRF_ERROR_INVALID_PARAM:
42+
return BLE_ERROR_INVALID_PARAM;
43+
case NRF_ERROR_NOT_FOUND:
44+
case NRF_ERROR_DATA_SIZE:
45+
case BLE_ERROR_INVALID_CONN_HANDLE:
46+
case BLE_ERROR_GATTS_INVALID_ATTR_TYPE:
47+
return BLE_ERROR_PARAM_OUT_OF_RANGE;
48+
case NRF_ERROR_FORBIDDEN:
49+
return BLE_ERROR_OPERATION_NOT_PERMITTED;
50+
default:
51+
return BLE_ERROR_UNSPECIFIED;
52+
}
53+
}
54+
55+
} // end of anonymous namespace
56+
2957
/**************************************************************************/
3058
/*!
3159
@brief Adds a new service to the GATT table on the peripheral
@@ -244,53 +272,52 @@ ble_error_t nRF5xGattServer::write(Gap::Handle_t connectionHandle, GattAttribute
244272
nRF5xGap &gap = (nRF5xGap &) nRF5xn::Instance(BLE::DEFAULT_INSTANCE).getGap();
245273
connectionHandle = gap.getConnectionHandle();
246274
}
247-
error_t error = (error_t) sd_ble_gatts_hvx(connectionHandle, &hvx_params);
248-
if (error != ERROR_NONE) {
249-
switch (error) {
250-
case ERROR_BLE_NO_TX_BUFFERS: /* Notifications consume application buffers. The return value can be used for resending notifications. */
251-
case ERROR_BUSY:
252-
returnValue = BLE_STACK_BUSY;
253-
break;
254-
255-
case ERROR_INVALID_STATE:
256-
case ERROR_BLEGATTS_SYS_ATTR_MISSING:
257-
returnValue = BLE_ERROR_INVALID_STATE;
258-
break;
259275

260-
default :
261-
ASSERT_INT( ERROR_NONE,
262-
sd_ble_gatts_value_set(connectionHandle, attributeHandle, &value),
263-
BLE_ERROR_PARAM_OUT_OF_RANGE );
276+
bool updatesEnabled = false;
277+
if (connectionHandle != BLE_CONN_HANDLE_INVALID) {
278+
ble_error_t err = areUpdatesEnabled(connectionHandle, attributeHandle, &updatesEnabled);
279+
// FIXME: The softdevice allocates and populates CCCD when the client
280+
// interract with them. Checking for updates may return an out of
281+
// range error in such case.
282+
if(err && err != BLE_ERROR_PARAM_OUT_OF_RANGE) {
283+
return err;
284+
}
285+
}
264286

265-
/* Notifications consume application buffers. The return value can
266-
* be used for resending notifications. */
267-
returnValue = BLE_STACK_BUSY;
268-
break;
287+
if (updatesEnabled) {
288+
error_t error = (error_t) sd_ble_gatts_hvx(connectionHandle, &hvx_params);
289+
if (error != ERROR_NONE) {
290+
switch (error) {
291+
case ERROR_BLE_NO_TX_BUFFERS: /* Notifications consume application buffers. The return value can be used for resending notifications. */
292+
case ERROR_BUSY:
293+
returnValue = BLE_STACK_BUSY;
294+
break;
295+
296+
case ERROR_INVALID_STATE:
297+
case ERROR_BLEGATTS_SYS_ATTR_MISSING:
298+
returnValue = BLE_ERROR_INVALID_STATE;
299+
break;
300+
301+
default :
302+
ASSERT_INT( ERROR_NONE,
303+
sd_ble_gatts_value_set(connectionHandle, attributeHandle, &value),
304+
BLE_ERROR_PARAM_OUT_OF_RANGE );
305+
306+
if (connectionHandle == BLE_CONN_HANDLE_INVALID) {
307+
returnValue = BLE_ERROR_NONE;
308+
} else {
309+
/* Notifications consume application buffers. The return value can
310+
* be used for resending notifications. */
311+
returnValue = BLE_STACK_BUSY;
312+
}
313+
break;
314+
}
269315
}
316+
} else {
317+
returnValue = set_attribute_value(connectionHandle, attributeHandle, &value);
270318
}
271319
} else {
272-
uint32_t err = sd_ble_gatts_value_set(connectionHandle, attributeHandle, &value);
273-
switch(err) {
274-
case NRF_SUCCESS:
275-
returnValue = BLE_ERROR_NONE;
276-
break;
277-
case NRF_ERROR_INVALID_ADDR:
278-
case NRF_ERROR_INVALID_PARAM:
279-
returnValue = BLE_ERROR_INVALID_PARAM;
280-
break;
281-
case NRF_ERROR_NOT_FOUND:
282-
case NRF_ERROR_DATA_SIZE:
283-
case BLE_ERROR_INVALID_CONN_HANDLE:
284-
case BLE_ERROR_GATTS_INVALID_ATTR_TYPE:
285-
returnValue = BLE_ERROR_PARAM_OUT_OF_RANGE;
286-
break;
287-
case NRF_ERROR_FORBIDDEN:
288-
returnValue = BLE_ERROR_OPERATION_NOT_PERMITTED;
289-
break;
290-
default:
291-
returnValue = BLE_ERROR_UNSPECIFIED;
292-
break;
293-
}
320+
returnValue = set_attribute_value(connectionHandle, attributeHandle, &value);
294321
}
295322

296323
return returnValue;
@@ -305,7 +332,12 @@ ble_error_t nRF5xGattServer::areUpdatesEnabled(const GattCharacteristic &charact
305332

306333
ble_error_t nRF5xGattServer::areUpdatesEnabled(Gap::Handle_t connectionHandle, const GattCharacteristic &characteristic, bool *enabledP)
307334
{
308-
int characteristicIndex = resolveValueHandleToCharIndex(characteristic.getValueHandle());
335+
return areUpdatesEnabled(connectionHandle, characteristic.getValueHandle(), enabledP);
336+
}
337+
338+
ble_error_t nRF5xGattServer::areUpdatesEnabled(Gap::Handle_t connectionHandle, GattAttribute::Handle_t attributeHandle, bool *enabledP)
339+
{
340+
int characteristicIndex = resolveValueHandleToCharIndex(attributeHandle);
309341
if (characteristicIndex == -1) {
310342
return BLE_ERROR_INVALID_PARAM;
311343
}

‎features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/source/nRF5xGattServer.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,15 @@ class nRF5xGattServer : public GattServer
7979
return -1;
8080
}
8181

82+
/**
83+
* Query if updates of a characteristics are enabled for a given connection.
84+
*/
85+
ble_error_t areUpdatesEnabled(
86+
Gap::Handle_t connectionHandle,
87+
GattAttribute::Handle_t valueHandle,
88+
bool *enabledP
89+
);
90+
8291
private:
8392
GattCharacteristic *p_characteristics[BLE_TOTAL_CHARACTERISTICS];
8493
ble_gatts_char_handles_t nrfCharacteristicHandles[BLE_TOTAL_CHARACTERISTICS];

‎features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xGattServer.cpp

Lines changed: 67 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,32 @@ static const ble_gatts_rw_authorize_reply_params_t write_auth_invalid_reply = {
6565
}
6666
};
6767

68+
static ble_error_t set_attribute_value(
69+
Gap::Handle_t connectionHandle,
70+
GattAttribute::Handle_t attributeHandle,
71+
ble_gatts_value_t *value
72+
) {
73+
uint32_t err = sd_ble_gatts_value_set(connectionHandle, attributeHandle, value);
74+
switch(err) {
75+
case NRF_SUCCESS:
76+
return BLE_ERROR_NONE;
77+
case NRF_ERROR_INVALID_ADDR:
78+
case NRF_ERROR_INVALID_PARAM:
79+
return BLE_ERROR_INVALID_PARAM;
80+
case NRF_ERROR_NOT_FOUND:
81+
case NRF_ERROR_DATA_SIZE:
82+
case BLE_ERROR_INVALID_CONN_HANDLE:
83+
case BLE_ERROR_GATTS_INVALID_ATTR_TYPE:
84+
return BLE_ERROR_PARAM_OUT_OF_RANGE;
85+
case NRF_ERROR_FORBIDDEN:
86+
return BLE_ERROR_OPERATION_NOT_PERMITTED;
87+
default:
88+
return BLE_ERROR_UNSPECIFIED;
89+
}
6890
}
6991

92+
} // end of anonymous namespace
93+
7094
/**************************************************************************/
7195
/*!
7296
@brief Adds a new service to the GATT table on the peripheral
@@ -285,53 +309,49 @@ ble_error_t nRF5xGattServer::write(Gap::Handle_t connectionHandle, GattAttribute
285309
nRF5xGap &gap = (nRF5xGap &) nRF5xn::Instance(BLE::DEFAULT_INSTANCE).getGap();
286310
connectionHandle = gap.getConnectionHandle();
287311
}
288-
error_t error = (error_t) sd_ble_gatts_hvx(connectionHandle, &hvx_params);
289-
if (error != ERROR_NONE) {
290-
switch (error) {
291-
case ERROR_BLE_NO_TX_BUFFERS: /* Notifications consume application buffers. The return value can be used for resending notifications. */
292-
case ERROR_BUSY:
293-
returnValue = BLE_STACK_BUSY;
294-
break;
295312

296-
case ERROR_INVALID_STATE:
297-
case ERROR_BLEGATTS_SYS_ATTR_MISSING:
298-
returnValue = BLE_ERROR_INVALID_STATE;
299-
break;
313+
bool updatesEnabled = false;
314+
if (connectionHandle != BLE_CONN_HANDLE_INVALID) {
315+
ble_error_t err = areUpdatesEnabled(connectionHandle, attributeHandle, &updatesEnabled);
300316

301-
default :
302-
ASSERT_INT( ERROR_NONE,
303-
sd_ble_gatts_value_set(connectionHandle, attributeHandle, &value),
304-
BLE_ERROR_PARAM_OUT_OF_RANGE );
317+
// FIXME: The softdevice allocates and populates CCCD when the client
318+
// interract with them. Checking for updates may return an out of
319+
// range error in such case.
320+
if(err && err != BLE_ERROR_PARAM_OUT_OF_RANGE) {
321+
return err;
322+
}
323+
}
305324

306-
/* Notifications consume application buffers. The return value can
307-
* be used for resending notifications. */
308-
returnValue = BLE_STACK_BUSY;
309-
break;
325+
if (updatesEnabled) {
326+
error_t error = (error_t) sd_ble_gatts_hvx(connectionHandle, &hvx_params);
327+
if (error != ERROR_NONE) {
328+
switch (error) {
329+
case ERROR_BLE_NO_TX_BUFFERS: /* Notifications consume application buffers. The return value can be used for resending notifications. */
330+
case ERROR_BUSY:
331+
returnValue = BLE_STACK_BUSY;
332+
break;
333+
334+
case ERROR_INVALID_STATE:
335+
case ERROR_BLEGATTS_SYS_ATTR_MISSING:
336+
returnValue = BLE_ERROR_INVALID_STATE;
337+
break;
338+
339+
default :
340+
ASSERT_INT( ERROR_NONE,
341+
sd_ble_gatts_value_set(connectionHandle, attributeHandle, &value),
342+
BLE_ERROR_PARAM_OUT_OF_RANGE );
343+
344+
/* Notifications consume application buffers. The return value can
345+
* be used for resending notifications. */
346+
returnValue = BLE_STACK_BUSY;
347+
break;
348+
}
310349
}
350+
} else {
351+
returnValue = set_attribute_value(connectionHandle, attributeHandle, &value);
311352
}
312353
} else {
313-
uint32_t err = sd_ble_gatts_value_set(connectionHandle, attributeHandle, &value);
314-
switch(err) {
315-
case NRF_SUCCESS:
316-
returnValue = BLE_ERROR_NONE;
317-
break;
318-
case NRF_ERROR_INVALID_ADDR:
319-
case NRF_ERROR_INVALID_PARAM:
320-
returnValue = BLE_ERROR_INVALID_PARAM;
321-
break;
322-
case NRF_ERROR_NOT_FOUND:
323-
case NRF_ERROR_DATA_SIZE:
324-
case BLE_ERROR_INVALID_CONN_HANDLE:
325-
case BLE_ERROR_GATTS_INVALID_ATTR_TYPE:
326-
returnValue = BLE_ERROR_PARAM_OUT_OF_RANGE;
327-
break;
328-
case NRF_ERROR_FORBIDDEN:
329-
returnValue = BLE_ERROR_OPERATION_NOT_PERMITTED;
330-
break;
331-
default:
332-
returnValue = BLE_ERROR_UNSPECIFIED;
333-
break;
334-
}
354+
returnValue = set_attribute_value(connectionHandle, attributeHandle, &value);
335355
}
336356

337357
return returnValue;
@@ -346,7 +366,12 @@ ble_error_t nRF5xGattServer::areUpdatesEnabled(const GattCharacteristic &charact
346366

347367
ble_error_t nRF5xGattServer::areUpdatesEnabled(Gap::Handle_t connectionHandle, const GattCharacteristic &characteristic, bool *enabledP)
348368
{
349-
int characteristicIndex = resolveValueHandleToCharIndex(characteristic.getValueHandle());
369+
return areUpdatesEnabled(connectionHandle, characteristic.getValueHandle(), enabledP);
370+
}
371+
372+
ble_error_t nRF5xGattServer::areUpdatesEnabled(Gap::Handle_t connectionHandle, GattAttribute::Handle_t attributeHandle, bool *enabledP)
373+
{
374+
int characteristicIndex = resolveValueHandleToCharIndex(attributeHandle);
350375
if (characteristicIndex == -1) {
351376
return BLE_ERROR_INVALID_PARAM;
352377
}

‎features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xGattServer.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,15 @@ class nRF5xGattServer : public GattServer
133133
*/
134134
void releaseAllWriteRequests();
135135

136+
/**
137+
* Query if updates of a characteristics are enabled for a given connection.
138+
*/
139+
ble_error_t areUpdatesEnabled(
140+
Gap::Handle_t connectionHandle,
141+
GattAttribute::Handle_t valueHandle,
142+
bool *enabledP
143+
);
144+
136145
private:
137146
GattCharacteristic *p_characteristics[BLE_TOTAL_CHARACTERISTICS];
138147
ble_gatts_char_handles_t nrfCharacteristicHandles[BLE_TOTAL_CHARACTERISTICS];

0 commit comments

Comments
 (0)