`attHandle` in `GattUpdatesDisabledCallbackParams` of `onUpdatesEnabled` handler is always not matched with `characteristic->getValueHandle()`

[16L-YT]

Description of defect

When I use in my class ImuService : ble::GattServer::EventHandler the onUpdatesEnabled and onUpdatesDisabled they are called indeed when a subscription/unsubscribing event happens, but the value of attribute handle is always not matching the characteristic's attribute handler, like this:

//...
virtual void onUpdatesDisabled(const GattUpdatesDisabledCallbackParams &params) {
        
        if (params.attHandle == _gyro_data_characteristic->getValueHandle()) {// always false }
//...
}

Although that onDataRead event works like charm for the same characteristic. Form past issues like this and from tracking some code, I think that onUpdatesDisabled is handled differently that onDataRead . So I suspect from a bug somewhere.

Target(s) affected by this defect ?

AMA3B1KK-KBR-B0

Toolchain(s) (name and version) displaying this defect ?

gcc version 10.2.1 20201103 (release) (xPack GNU Arm Embedded GCC, 64-bit)

What version of Mbed-os are you using (tag or sha) ?

mbed-os-6.6.0 @ commit 343b19f
tried with latest too mbed-os-6.9.0

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

Tools that comes with Mbed Studio 1.3.1

How is this defect reproduced ?

#ifndef BLE_IMU_SERVICE_H__
#define BLE_IMU_SERVICE_H__

#include "mbed.h"
#include "ble/BLE.h"
#include "ble/Gap.h"
#include "ble/GattServer.h"
#include <ChainableGattServerEventHandler.h>
#include <rtos.h>

#define power_on_imu()       IMU_EN = 1; \
                             ThisThread::sleep_for(10ms) \

#define power_off_imu()       IMU_EN = 0
                            

I2C * _i2c;

static void imu_interrupt_handler();

class ImuService : ble::GattServer::EventHandler {
public:
    ImuService(ChainableGattServerEventHandler *event_handler) :
      _event_handler(event_handler),
      IMU_EN(IMU_VDD, 0),
      IMU_IRQ1(IMU_INT1),
      IMU_IRQ2(IMU_INT2)
    {
        DigitalOut sda(IMU_SDA, 0);
        DigitalOut scl(IMU_SCL, 0);

        const UUID i2c_addr_UUID(DevUUID::IMU::Characteristic::SCAN_I2C_ADDR);
        _i2c_addr_characteristic = new ReadOnlyGattCharacteristic<uint8_t> (i2c_addr_UUID, &_i2c_addr_value);

        const UUID gyro_data_UUID(DevUUID::IMU::Characteristic::GYRO_DATA);
        _gyro_data_characteristic = new ReadOnlyArrayGattCharacteristic<int16_t,3> (gyro_data_UUID, gyro_data,GattCharacteristic::BLE_GATT_CHAR_PROPERTIES_READ | GattCharacteristic::BLE_GATT_CHAR_PROPERTIES_NOTIFY );

        const UUID acc_data_UUID(DevUUID::IMU::Characteristic::ACC_DATA);
        _acc_data_characteristic = new ReadOnlyArrayGattCharacteristic<int16_t,3> (acc_data_UUID, acc_data,GattCharacteristic::BLE_GATT_CHAR_PROPERTIES_READ | GattCharacteristic::BLE_GATT_CHAR_PROPERTIES_NOTIFY );

        const UUID imu_sett_UUID(DevUUID::IMU::Characteristic::SETTINGS);
        _imu_sett_characteristic = new ReadOnlyArrayGattCharacteristic<char,6> (imu_sett_UUID, _imu_settings);
        

    }

    uint32_t _gyro_data_value = 0 ;
    int16_t acc_data[3],gyro_data[3];

    ReadOnlyArrayGattCharacteristic<int16_t,3> * _gyro_data_characteristic = nullptr;
    ReadOnlyArrayGattCharacteristic<int16_t,3> * _acc_data_characteristic = nullptr; 
    GattServer *_server = nullptr;
    
    ~ImuService(){}

    void sendAccNoti(int16_t * readings)
    {
// some code here
    }

    void sendGyroNoti(int16_t * readings)
    {
// some code here
    }

    void start(BLE &ble, events::EventQueue &event_queue)
    {
        const UUID uuid(Dev::UUID::Service::IMU);             //UUID of the service
        GattCharacteristic* charTable[] = {         //list of characteristics inside service
          _i2c_addr_characteristic,
          _gyro_data_characteristic,
          _acc_data_characteristic,
          _imu_sett_characteristic
        };
        GattService imu_service(uuid, charTable, 4);
        _server = &(ble.gattServer());
        _server->addService(imu_service);

        _event_handler->addEventHandler(this);
        
        _i2c = new I2C(IMU_SDA, IMU_SCL);

        Callback<void()> cb(imu_interrupt_handler);

        IMU_IRQ2.rise(event_queue.event(cb));

    }

private:
    void scan_i2c_addr(){
 
// some code here
    }


    virtual void onDataWritten(const GattWriteCallbackParams &params)
    {

    }

    virtual void onDataRead (const GattReadCallbackParams &params)
    {
      if ((params.handle == _i2c_addr_characteristic->getValueHandle()) ) {
// some code here
      }
      else if ((params.handle == _gyro_data_characteristic->getValueHandle()) ) {
// some code here
      }
      else if ((params.handle == _acc_data_characteristic->getValueHandle()) ) {
// some code here
      }
      else if (params.handle == _imu_sett_characteristic->getValueHandle())
      {     
 // some code here
        }
    }

    virtual void onUpdatesEnabled(const GattUpdatesEnabledCallbackParams &params) {


        if ((params.attHandle == _gyro_data_characteristic->getValueHandle()) )
        {
// some code here
        } 
        else if ((params.attHandle == _acc_data_characteristic->getValueHandle()) ) // Enable once find when the condition can come true
        {
// some code here
        }
    }

    virtual void onUpdatesDisabled(const GattUpdatesDisabledCallbackParams &params) {
        
        if (params.attHandle == _gyro_data_characteristic->getValueHandle())
        { 
            _gyro_data_enabled = 0 ;
        } 
        else if (params.attHandle == _acc_data_characteristic->getValueHandle())
        {
            _acc_data_enabled = 0 ;
        }

        if(_acc_data_enabled == 0 && _gyro_data_enabled == 0) power_off_imu();
    }

private:
    ChainableGattServerEventHandler *_event_handler;

    ReadOnlyGattCharacteristic<uint8_t> * _i2c_addr_characteristic = nullptr;
    ReadOnlyArrayGattCharacteristic<char, 6> * _imu_sett_characteristic = nullptr;

    uint8_t _i2c_addr_value = 0x00;
    uint8_t _acc_data_enabled = 0; // flag to know if user subscribed to Acc notification
    uint8_t _gyro_data_enabled = 0;// flag to know if user subscribed to gyro notification

    char _imu_settings [6];

    DigitalOut IMU_EN;

    InterruptIn IMU_IRQ1;
    InterruptIn IMU_IRQ2;
};

ImuService  * _imuService;


void imu_interrupt_handler()
{
//..  
}


#endif /* #ifndef BLE_IMU_SERVICE_H__*/

[ciarmcom]

Thank you for raising this detailed GitHub issue. I am now notifying our internal issue triagers.
Internal Jira reference: https://jira.arm.com/browse/IOTOSM-3698

[pan-]

@16L-YT Indeed it is not matching the characteristic declaration handle or value handle. However it is matching the CCCD handle. We're not changing the value of attHandle as this would break backward compatibility however PR to add a new field with the value handle are welcome.

On our end, we must fix the documentation. @noonfom Can you take care of it ?

The CCCD handle can be retrieved by searching through the descriptors of the characteristic. In your case, there is no descriptors registered, this make things easier:

    virtual void onUpdatesDisabled(const GattUpdatesDisabledCallbackParams &params) {
        
        if (params.attHandle == _gyro_data_characteristic->getDescriptor(0)->getHandle())
        { 
            _gyro_data_enabled = 0 ;
        } 
        else if (params.attHandle == _acc_data_characteristic->getDescriptor(0)->getHandle())
        {
            _acc_data_enabled = 0 ;
        }

        if(_acc_data_enabled == 0 && _gyro_data_enabled == 0) power_off_imu();
    }