The interactive GATT client of the 'ble_app_interactive' example has multiple issues. As I heavily modified the example, I'm not able to provide a diff against the original source. Anyways I'd like to discuss the issues and the solutions to make it work. I assume the issues haven't been noticed due to usage of very limited test cases, when it comes to interaction with GATT servers.
1. Characteristics discovery for a given service UUID (after service discovery)
Characteristics discovery is mostly handled by the event handler `on_characteristics_discovery_rsp`. This handler seems to work okay, if all data for a call to `sd_ble_gattc_characteristics_discover` is delivered in a single response, but if the responses have to be split, because they don't fit in a single transmission (too many characteristics in the respective service) it totally fails. There are multiple issues causing this:
1.1
successive characteristic requests for the same service (with start_handle adjusted to the successor of the value handle from the previous last characteristic discover response) are written to the wrong offset in the characteristics storage array `m_srv_char.char_data[]`. For each call to the handler, the loop assigning values to the array starts at index 0x00.
Original code:
if ((count + offset) > MAX_CHARACTERISTIC_COUNT) {
bytes_to_copy = MAX_CHARACTERISTIC_COUNT - offset;
NRF_LOG_RAW_INFO("Too many characteristics discovered\r\n");
} else {
bytes_to_copy = count;
}
// Save characteristics data.
for (uint8_t i = offset; i < bytes_to_copy; i++) {
m_srv_char.char_data[i].decl_handle = p_char_disc_rsp_evt->chars[i].handle_decl;
m_srv_char.char_data[i].value_handle = p_char_disc_rsp_evt->chars[i].handle_value;
m_srv_char.char_data[i].uuid = p_char_disc_rsp_evt->chars[i].uuid;
m_srv_char.char_data[i].notify = p_char_disc_rsp_evt->chars[i].char_props.notify;
m_srv_char.char_data[i].cccd_desc_handle = 0;
offset++;
}
Fixed code:
if ((count + offset) > MAX_CHARACTERISTIC_COUNT) {
chars_to_copy = MAX_CHARACTERISTIC_COUNT - offset;
NRF_LOG_RAW_INFO("Too many characteristics discovered\r\n");
} else {
chars_to_copy = count;
}
// Save characteristics data (to correct offset).
for (uint8_t i = 0; i < chars_to_copy; i++) {
m_srv_char.char_data[offset + i].decl_handle = p_char_disc_rsp_evt->chars[i].handle_decl;
m_srv_char.char_data[offset + i].value_handle = p_char_disc_rsp_evt->chars[i].handle_value;
m_srv_char.char_data[offset + i].uuid = p_char_disc_rsp_evt->chars[i].uuid;
m_srv_char.char_data[offset + i].notify = p_char_disc_rsp_evt->chars[i].char_props.notify;
m_srv_char.char_data[offset + i].cccd_desc_handle = 0;
}
1.2
As successive calls to the on characteristics discover response event handler are stateless, the end_handle for a potentially required follow up call to `sd_ble_gattc_characteristics_discover` has to be recalculated (not all characteristics retrieved with former calls). In short words, the end_handler of the handle_range of the resepctive service has to retrieved again. The code doing this has an incomplete logical statement in its condition and thus ALWAYS FETCHES THE END_HANDLE of the first stored service, not the one for which characteristics are currently enumerated.
Wrong code:
// Search for end handle.
for (uint8_t j = 0; j < mp_device_srv[conn_handle]->count; j++) {
if (handle_range.start_handle >
mp_device_srv[conn_handle]->services[j].handle_range.start_handle) {
handle_range.end_handle =
mp_device_srv[conn_handle]->services[j].handle_range.end_handle;
break;
}
}
Code with fixed condition, to select end_handle of correct service:
// Search for end handle.
for (uint8_t j = 0; j < mp_device_srv[conn_handle]->count; j++) {
// added condition to assure end_handle of correct service is fetched, not the first end handle for the first service with a start_handle y "last value_handle"
if (handle_range.start_handle > mp_device_srv[conn_handle]->services[j].handle_range.start_handle &&
handle_range.start_handle <= mp_device_srv[conn_handle]->services[j].handle_range.end_handle) {
NRF_LOG_DEBUG("Used start handle %04x, end handle %04x", handle_range.start_handle, handle_range.end_handle);
handle_range.end_handle = mp_device_srv[conn_handle]->services[j].handle_range.end_handle;
service_number = j;
break;
}
}
1.3 The loop counter which is used to print out the characteristics data from the current response doesn't stop at the last characteristics entry of the response, but at the last valid entry of the internal characteristics array (static `offset` value). For example: 3 characteristics are shipped back from the first response (offset is now at 3), a successive response delivers 2 additional characteristics (offset is now at 5). The output loop tires to fetch characteristics 0 to 4 from the response (i=0; i < offset; i++), but the reseponse only has two entries. This will ultimately lead to a hard fault, because of OOB mem access.
Wrong code:
// Display characteristics data.
for (uint8_t i = 0; i < offset; i++) {
ble_gatt_char_props_t char_param =
p_ble_gattc_evt->params.char_disc_rsp.chars[i].char_props;
NRF_LOG_RAW_INFO("Characteristic UUID: %X\r\n",
p_ble_gattc_evt->params.char_disc_rsp.chars[i].uuid.uuid);
NRF_LOG_RAW_INFO("Parameters:\r\n");
NRF_LOG_RAW_INFO("broadcast: %d ", char_param.broadcast);
...snip...
}
Fixed code:
// Display characteristics data (only of this response, as response data is parsed)
for (uint8_t i = 0; i < chars_to_copy; i++) {
ble_gatt_char_props_t char_param = p_ble_gattc_evt->params.char_disc_rsp.chars[i].char_props;
NRF_LOG_RAW_INFO("Characteristic UUID: %X\r\n",
p_ble_gattc_evt->params.char_disc_rsp.chars[i].uuid.uuid);
NRF_LOG_RAW_INFO("\tParameters:\r\n");
NRF_LOG_RAW_INFO("\tbroadcast: %d ", char_param.broadcast);
...snip...
}
cccd_descriptors_discovery
2. CCC descriptor retrieval is totally messed up, as it doesn't account for callbacks:
After all characteristics data for a single service has been discovered, the `on_characteristics_discovery_rsp` handler calls the `cccd_descriptors_discovery`method to initiate CCCD discovery. From there the `cccd_descriptors_search` method (which itself calls the asynchronous method `sd_ble_gattc_descriptors_discover`) is called in a `for` loop iterating over all the characteristics of the current service, without waiting for the arrival of the `BLE_GATTC_EVT_DESC_DISC_RSP` event. This means only the very first request will be send, the others fail with `NRF_ERROR_BUSY`.
There are multiple other issues int the cccd_*** methods. To ease things up, I completely replaced them call to `cccd_descriptors_discovery` with a direct call to `sd_ble_gattc_descriptors_discover`. Instead of the handle range of each characteristic, the handle range of the whole service is used. This leads to the need for multiple calls `sd_ble_gattc_descriptors_discover` as the results doesn't fit into a single response (in most cases), but the code to handle this is already there. Retrieved CCCD handles onlöy have to be assigned to the correct characteristic handles. I don't give example code for this, as only small changes are required.
3. Characteristic UUIDs vs handles
The CLI commands are using the UUIDs of characteristices to read/write/enable notifications/indications. This is not very convenient. My test device is running HID over GATT, which I think is a common service these days. As all exposed HID reports have the UUID 0x2A4D, only one of those reports can't be accessed (or get notifications enabled). This common example shows, that using the UUID of a characteristic isn't the best selection criteria, as all successive "HID reports" of course share the same UUID and are shadowed by the first one.
As this is a common issue, I changed all method of the GATT CLI client which used a UUID to identify a characteristic, to use the value_handle instead. This requires only minor changes and is required to make the client usable IMO
