Discovering multiple BLE descriptors problem

[Viktor Venczel]

Dear Nordic Team!

I think I found a bug in the ble_db_discovery module of Nordic SDK 15.3.0, maybe other SDKs are affected too. I am implementing a BLE central device for HID devices(mouse, keyboard). I have a Rapoo mouse which I think supports only BLE 4.0. When the central discovers the services, if more than one descriptors are available under a characteristic, it only finds one descriptor. HID device report characteristics can have two descriptors: Client characteristic configuration and Report reference descriptors. After investigation the issue with a BLE packet sniffer I discovered the problem: The peripheral device sends back only one descriptor info when find information request is sent with multiple handles, and the central device doesn't read the remaining descriptors info:

4025	35.585504	Master_0xba266bbd	Slave_0xba266bbd	ATT	35	Sent Find Information Request, Handles: 0x001c..0x001d
4026	35.585504	Slave_0xba266bbd	Master_0xba266bbd	LE LL	26	Empty PDU
4027	35.615716	Master_0xba266bbd	Slave_0xba266bbd	LE LL	26	Empty PDU
4028	35.615716	Slave_0xba266bbd	Master_0xba266bbd	ATT	36	Rcvd Find Information Response, Handle: 0x001c (Unknown: Report: Client Characteristic Configuration)
4029	35.645973	Master_0xba266bbd	Slave_0xba266bbd	ATT	35	Sent Find Information Request, Handles: 0x0020..0x0021
4030	35.645973	Slave_0xba266bbd	Master_0xba266bbd	LE LL	26	Empty PDU
4031	35.676251	Master_0xba266bbd	Slave_0xba266bbd	LE LL	26	Empty PDU
4032	35.677247	Slave_0xba266bbd	Master_0xba266bbd	ATT	36	Rcvd Find Information Response, Handle: 0x0020 (Unknown: Report: Client Characteristic Configuration)
4033	35.705859	Master_0xba266bbd	Slave_0xba266bbd	ATT	35	Sent Find Information Request, Handles: 0x0024..0x0024
4034	35.706893	Slave_0xba266bbd	Master_0xba266bbd	LE LL	26	Empty PDU
4035	35.735916	Master_0xba266bbd	Slave_0xba266bbd	LE LL	26	Empty PDU
4036	35.735916	Slave_0xba266bbd	Master_0xba266bbd	ATT	36	Rcvd Find Information Response, Handle: 0x0024 (Unknown: Report: Report Reference)
4037	35.796626	Master_0xba266bbd	Slave_0xba266bbd	ATT	35	Sent Find Information Request, Handles: 0x0029..0x0029
4038	35.798618	Slave_0xba266bbd	Master_0xba266bbd	LE LL	26	Empty PDU
4039	35.826564	Master_0xba266bbd	Slave_0xba266bbd	LE LL	26	Empty PDU
4040	35.829556	Slave_0xba266bbd	Master_0xba266bbd	ATT	36	Rcvd Find Information Response, Handle: 0x0029 (Unknown: Boot Keyboard Input Report)

If I connect with my phone to the device, nrfConnect app reads information correctly. it does read the remaining informations:

4986	18.979904	Master_0xaf9ab46d	Slave_0xaf9ab46d	ATT	35	Sent Find Information Request, Handles: 0x001c..0x001d
4987	18.980913	Slave_0xaf9ab46d	Master_0xaf9ab46d	LE LL	26	Empty PDU
4988	18.986918	Master_0xaf9ab46d	Slave_0xaf9ab46d	LE LL	26	Empty PDU
4989	18.987910	Slave_0xaf9ab46d	Master_0xaf9ab46d	ATT	36	Rcvd Find Information Response, Handle: 0x001c (Human Interface Device: Report: Client Characteristic Configuration)
4990	18.994864	Master_0xaf9ab46d	Slave_0xaf9ab46d	ATT	35	Sent Find Information Request, Handles: 0x001d..0x001d
4991	18.995861	Slave_0xaf9ab46d	Master_0xaf9ab46d	LE LL	26	Empty PDU
4992	19.002870	Master_0xaf9ab46d	Slave_0xaf9ab46d	LE LL	26	Empty PDU
4993	19.002870	Slave_0xaf9ab46d	Master_0xaf9ab46d	ATT	36	Rcvd Find Information Response, Handle: 0x001d (Human Interface Device: Report: Report Reference)
4994	19.009824	Master_0xaf9ab46d	Slave_0xaf9ab46d	ATT	35	Sent Find Information Request, Handles: 0x0020..0x0021
4995	19.010821	Slave_0xaf9ab46d	Master_0xaf9ab46d	LE LL	26	Empty PDU
4996	19.017802	Master_0xaf9ab46d	Slave_0xaf9ab46d	LE LL	26	Empty PDU
4997	19.017802	Slave_0xaf9ab46d	Master_0xaf9ab46d	ATT	36	Rcvd Find Information Response, Handle: 0x0020 (Human Interface Device: Report: Client Characteristic Configuration)
4998	19.024784	Master_0xaf9ab46d	Slave_0xaf9ab46d	ATT	35	Sent Find Information Request, Handles: 0x0021..0x0021
4999	19.025781	Slave_0xaf9ab46d	Master_0xaf9ab46d	LE LL	26	Empty PDU
5000	19.032763	Master_0xaf9ab46d	Slave_0xaf9ab46d	LE LL	26	Empty PDU
5001	19.032763	Slave_0xaf9ab46d	Master_0xaf9ab46d	ATT	36	Rcvd Find Information Response, Handle: 0x0021 (Human Interface Device: Report: Report Reference)
5002	19.039776	Master_0xaf9ab46d	Slave_0xaf9ab46d	ATT	35	Sent Find Information Request, Handles: 0x0024..0x0024
5003	19.040740	Slave_0xaf9ab46d	Master_0xaf9ab46d	LE LL	26	Empty PDU
5004	19.047720	Master_0xaf9ab46d	Slave_0xaf9ab46d	LE LL	26	Empty PDU
5005	19.047720	Slave_0xaf9ab46d	Master_0xaf9ab46d	ATT	36	Rcvd Find Information Response, Handle: 0x0024 (Human Interface Device: Report: Report Reference)
5006	19.054702	Master_0xaf9ab46d	Slave_0xaf9ab46d	ATT	35	Sent Find Information Request, Handles: 0x0029..0x0029
5007	19.055699	Slave_0xaf9ab46d	Master_0xaf9ab46d	LE LL	26	Empty PDU
5008	19.062681	Master_0xaf9ab46d	Slave_0xaf9ab46d	LE LL	26	Empty PDU
5009	19.062681	Slave_0xaf9ab46d	Master_0xaf9ab46d	ATT	36	Rcvd Find Information Response, Handle: 0x0029 (Human Interface Device: Boot Keyboard Input Report)

Using NRF as a HID peripheral doesn't have a problem, I think only older devices are affected. 

I made a fix in the ble_db_discovery module however I'd like to know what is your opinion.

Regards

[Einar Thorsrud]

Hi,

It looks like you are right. There are probably not that many use cases where the nRF is a client for HID devices, which may explain why this is. Do you mind sharing how you fixed this in the DB discovery module?

[Viktor Venczel]

Hi,

Sure, here is what I've done:
ble_discovery_db.h:

// Added curr_desc_handle_range 
typedef struct
{
    ble_gatt_db_srv_t   services[BLE_DB_DISCOVERY_MAX_SRV]; /**< Information related to the current service being discovered. This is intended for internal use during service discovery.*/
    uint8_t             srv_count;                          /**< Number of services at the peers GATT database.*/
    uint8_t             curr_char_ind;                      /**< Index of the current characteristic being discovered. This is intended for internal use during service discovery.*/
    uint8_t             curr_srv_ind;                       /**< Index of the current service being discovered. This is intended for internal use during service discovery.*/
    ble_gattc_handle_range_t curr_desc_handle_range;        /**< Handle range of the current descriptors being discovered This is intended for internal use during service discovery.*/
    bool                discovery_in_progress;              /**< Variable to indicate if there is a service discovery in progress. */
    bool                discovery_pending;                  /**< Discovery was requested, but could not start because the SoftDevice was busy. */
    uint8_t             discoveries_count;                  /**< Number of service discoveries made, both successful and unsuccessful. */
    uint16_t            conn_handle;                        /**< Connection handle on which the discovery is started*/
} ble_db_discovery_t;

// Init code added for curr_desc_handle_range
#define DB_INIT()                                                                                   \
{                                                                                                   \
    .services = {},                                                                                 \
    .srv_count             = 0,                                                                     \
    .curr_char_ind         = 0,                                                                     \
    .curr_srv_ind          = 0,                                                                     \
    .curr_desc_handle_range= {},                                                                    \
    .discovery_in_progress = 0,                                                                     \
    .discovery_pending     = 0,                                                                     \
    .discoveries_count     = 0,                                                                     \
    .conn_handle           = BLE_CONN_HANDLE_INVALID,                                               \
},

ble_db_discovery.c:

// Added code to the end of the descriptor_discover function
static uint32_t descriptors_discover(ble_db_discovery_t * p_db_discovery,
                                     bool               * p_raise_discov_complete,
                                     uint16_t             conn_handle)
{
//... begining of the function implementation

    if (!is_discovery_reqd)
    {
        // No more descriptor discovery required. Discovery is complete.
        // This informs the caller that a discovery complete event can be triggered.
        *p_raise_discov_complete = true;
        
        // Added code: No descriptor handles
        p_db_discovery->curr_desc_handle_range.start_handle = 0;
        p_db_discovery->curr_desc_handle_range.end_handle = 0;

        return NRF_SUCCESS;
    }

    *p_raise_discov_complete = false;

    // Added code: Save descriptor handle range for later use
    p_db_discovery->curr_desc_handle_range.start_handle = handle_range.start_handle;
    p_db_discovery->curr_desc_handle_range.end_handle = handle_range.end_handle;

    return sd_ble_gattc_descriptors_discover(conn_handle, &handle_range);
}


static void on_descriptor_discovery_rsp(ble_db_discovery_t * const    p_db_discovery,
                                        const ble_gattc_evt_t * const p_ble_gattc_evt)
{
    const ble_gattc_evt_desc_disc_rsp_t * p_desc_disc_rsp_evt;
    ble_gatt_db_srv_t                   * p_srv_being_discovered;

    if (p_ble_gattc_evt->conn_handle != p_db_discovery->conn_handle)
    {
        return;
    }

    p_srv_being_discovered = &(p_db_discovery->services[p_db_discovery->curr_srv_ind]);

    p_desc_disc_rsp_evt = &(p_ble_gattc_evt->params.desc_disc_rsp);

    ble_gatt_db_char_t * p_char_being_discovered =
        &(p_srv_being_discovered->charateristics[p_db_discovery->curr_char_ind]);

    if (p_ble_gattc_evt->gatt_status == BLE_GATT_STATUS_SUCCESS)
    {
        // The descriptor was found at the peer.
        // Iterate through and collect CCCD, Extended Properties,
        // User Description & Report Reference descriptor handles.
        for (uint32_t i = 0; i < p_desc_disc_rsp_evt->count; i++)
        {
            switch (p_desc_disc_rsp_evt->descs[i].uuid.uuid)
            {
                case BLE_UUID_DESCRIPTOR_CLIENT_CHAR_CONFIG:
                    p_char_being_discovered->cccd_handle =
                        p_desc_disc_rsp_evt->descs[i].handle;
                    break;

                case BLE_UUID_DESCRIPTOR_CHAR_EXT_PROP:
                    p_char_being_discovered->ext_prop_handle =
                        p_desc_disc_rsp_evt->descs[i].handle;
                    break;

                case BLE_UUID_DESCRIPTOR_CHAR_USER_DESC:
                    p_char_being_discovered->user_desc_handle =
                        p_desc_disc_rsp_evt->descs[i].handle;
                    break;

                case BLE_UUID_REPORT_REF_DESCR:
                    p_char_being_discovered->report_ref_handle =
                        p_desc_disc_rsp_evt->descs[i].handle;
                    break;
            }

            /* Break if we've found all the descriptors we are looking for. */
            if (p_char_being_discovered->cccd_handle       != BLE_GATT_HANDLE_INVALID &&
                p_char_being_discovered->ext_prop_handle   != BLE_GATT_HANDLE_INVALID &&
                p_char_being_discovered->user_desc_handle  != BLE_GATT_HANDLE_INVALID &&
                p_char_being_discovered->report_ref_handle != BLE_GATT_HANDLE_INVALID)
            {
                break;
            }
        }
    }

    // Added code begins
    // Check if we have descriptors more than one
    if(p_db_discovery->curr_desc_handle_range.start_handle != p_db_discovery->curr_desc_handle_range.end_handle)
    {
        uint16_t start_handle = p_db_discovery->curr_desc_handle_range.start_handle + p_desc_disc_rsp_evt->count;
    
        if(start_handle > p_db_discovery->curr_desc_handle_range.end_handle)
        {
            // No more descriptor remaining
        }
        else
        {
            // Discovery the remaining descriptors for the current characteristic.
            uint32_t err_code;
            
            //Update handle range
            p_db_discovery->curr_desc_handle_range.start_handle = start_handle;

            err_code = sd_ble_gattc_descriptors_discover(p_ble_gattc_evt->conn_handle,
                                            &(p_db_discovery->curr_desc_handle_range));

            if (err_code != NRF_SUCCESS)
            {
                p_db_discovery->discovery_in_progress = false;

                // Error with discovering the service.
                // Indicate the error to the registered user application.
                discovery_error_evt_trigger(p_db_discovery, err_code, p_ble_gattc_evt->conn_handle);

                m_pending_user_evts[0].evt.evt_type    = BLE_DB_DISCOVERY_AVAILABLE;
                m_pending_user_evts[0].evt.conn_handle = p_ble_gattc_evt->conn_handle;
            }
            
            // Return if we have more descriptors to discover or error occured
            return;
        }
    }
    // Added code ends
    
    // ... function code continues here
}

I hope it is clear. I excluded some part of the functions that are not relevant now.
I haven't tested it thoroughly yet, but it works with my devices.

[Einar Thorsrud]

Thanks for sharing. I have created an internal ticket so that the SDK team can consider fixing this in the future. I cannot  guarantee anything though, particularly as the nRF5 SDK is in maintenance mode.

[Viktor Venczel]

Hi, after some testing I found a problem when on_descriptor_discovery_rsp is called with error code. So I have updated the code.

This:

//Check if we have descriptors more than one
if(p_db_discovery->curr_desc_handle_range.start_handle != p_db_discovery->curr_desc_handle_range.end_handle)
{
 //...
}

to this:

//Check if we have descriptors more than one
if(p_ble_gattc_evt->gatt_status == BLE_GATT_STATUS_SUCCESS && p_db_discovery->curr_desc_handle_range.start_handle != p_db_discovery->curr_desc_handle_range.end_handle)
{
 //...
}