Peer Manager silently drop Service Changed indications if an ATT MTU exchange was happening

[Karlis]

Hello,

Using nRF5 SDK v15.3.0 Peer Manager drops Service Changed indications if an ATT MTU exchange is in progress, this hapens because function service_changed_cccd() in gatt_cache_manager.c allways returns NRF_ERROR_NOT_FOUND.

static ret_code_t service_changed_cccd(uint16_t conn_handle, uint16_t * p_cccd)
{
    bool       sc_found = false;
    uint16_t   end_handle;
    ret_code_t err_code = sd_ble_gatts_initial_user_handle_get(&end_handle);
    ASSERT(err_code == NRF_SUCCESS);

    for (uint16_t handle = 1; handle < end_handle; handle++)
    {
        uint16_t uuid;
        ble_gatts_value_t value = {.p_value = (uint8_t *)&uuid, .len = 2, .offset = 0};
        err_code = sd_ble_gatts_value_get(conn_handle, handle, &value);
        if (err_code != NRF_SUCCESS)
        {
            return err_code;
        }
        else if (!sc_found && (uuid == BLE_UUID_GATT_CHARACTERISTIC_SERVICE_CHANGED))
        {
            sc_found = true;
        }
        else if (sc_found && (uuid == BLE_UUID_DESCRIPTOR_CLIENT_CHAR_CONFIG))
        {
            value.p_value = (uint8_t *)p_cccd;
            return sd_ble_gatts_value_get(conn_handle, ++handle, &value);
        }
    }
    return NRF_ERROR_NOT_FOUND;
}

After some digging I found, as far as I understand, that sd_ble_gatts_value_get used in service_changed_cccd() returns handle value which is UUID only for service handles, but for characteristic handles UUID not stored as value.

Is there an other way how to find service changed cccd? 

Maybe it is allways last handle before user handles?

Or handle associated with service changed cccd is allways the same and we can simply check if it exists?

Thank You!

[Vidar Berg]

Hi,

Just to confirm, are you not getting this error when there's no MTU exchange? Also, have you enabled this characteristic in sdk_config.h. You can check if it's present with the nRF connect app on Android. 

Or handle associated with service changed cccd is allways the same and we can simply check if it exists?

Correct, it does not change since there are no optional system attributes above this characteristic. 

[Karlis]

Hi,

Yes, if there no MTU exchange, there are no errors. Characteristic is enabled.

And also if in gatt_cache_manager.c in function service_changed_send_in_evt remove "case NRF_ERROR_INVALID_STATE:" service changed indication works for iOS. Only problem is in fact that it never droped for Android or central witch not enabled service changed indication.

[Vidar Berg]

Hi,

It appears that the service_changed_cccd() function is not implemented correctly, it returns not found even when the SC is present. I will report this internally. A temporary workaround is to add a break statement here in gatt_cache_manager.c (line ~382):

case NRF_ERROR_INVALID_STATE:
{
uint16_t cccd;
err_code = service_changed_cccd(conn_handle, &cccd);
NRF_LOG_INFO("err_code: 0x%x", err_code);
if ((err_code == NRF_SUCCESS) && cccd)
{
// Possible ATT_MTU exchange ongoing.
// Do nothing, treat as busy.
break;
}
else
{
if (err_code != NRF_SUCCESS)
{
NRF_LOG_DEBUG("Unexpected error when looking for service changed CCCD: %s",
nrf_strerror_get(err_code));
}
// CCCDs not enabled or an error happened. Drop indication.
 --// Fallthrough.
break;
}
}

Edit 5/7-19

Please try to replace service_changed_cccd() with the one below instead of removing the fallthrough.

/**@brief Function for getting the value of the CCCD for the service changed characteristic.
 *
 * @details This function will search all system handles consecutively.
 *
 * @param[in]  conn_handle  The connection to check.
 * @param[out] p_cccd       The CCCD value of the service changed characteristic for this link.
 *
 * @return Any error from @ref sd_ble_gatts_value_get.
 */
static ret_code_t service_changed_cccd(uint16_t conn_handle, uint16_t * p_cccd)
{
    bool       sc_found = false;
    bool       generic_att_found = false;
    ble_gatts_attr_md_t attr_md;
    uint16_t   end_handle;
    ret_code_t err_code = sd_ble_gatts_initial_user_handle_get(&end_handle);
    ASSERT(err_code == NRF_SUCCESS);

    for (uint16_t handle = 1; handle < end_handle; handle++)
    {
        ble_uuid_t uuid;
        ble_gatts_value_t value = {.p_value = (uint8_t *)&uuid.uuid, .len = 2, .offset = 0};
        if (!generic_att_found)
        {
            err_code = sd_ble_gatts_value_get(conn_handle, handle, &value);
        }
        else
        {
            err_code = sd_ble_gatts_attr_get(handle, &uuid, NULL); 
        }

        if (err_code != NRF_SUCCESS)
        {
            return err_code;
        }
        else if (uuid.uuid == BLE_UUID_GATT)
        {
            generic_att_found = true;
        }
        else if (!sc_found && (uuid.uuid == BLE_UUID_GATT_CHARACTERISTIC_SERVICE_CHANGED))
        {
            sc_found = true;
        }
        else if (sc_found && (uuid.uuid == BLE_UUID_DESCRIPTOR_CLIENT_CHAR_CONFIG))
        {
            value.p_value = (uint8_t *)p_cccd;
            return sd_ble_gatts_value_get(conn_handle, handle, &value);
        }
    }
    return NRF_ERROR_NOT_FOUND;
}

[Karlis]

Thanks, with new service_changed_cccd() all works!

[Vidar Berg]

Thanks for confirming. I have reported this internally so it should be fixed for the next SDK release.