RFC: API Change: Bluetooth: Update indication callback parameters

[JordanYates]

Introduction

Updating the signature of the bt_gatt_indicate_func_t callback and the return value of bt_gatt_indicate to allow for tracking the use of the provided struct bt_gatt_indicate_params parameter.

Problem description

The GATT indication function, bt_gatt_indicate is an asynchronous function whose data arguments are packaged up into the struct bt_gatt_indicate_params type. The contents of the struct are not copied on the function call, and therefore must remain valid for the duration of the indication (The struct is still used when the confirmation from the remote side arrives).

Furthermore, it is possible to queue an arbitrary number of indications, which will be sent as the previous indication is acknowledged. As a result it can be necessary to allocate a struct bt_gatt_indicate_params (whether through a simple array, or the mem slab API or similar) to hold the values of indication N until it has been distributed. This also implies that it must be possible to know when a struct bt_gatt_indicate_params is finished with, so that it can be freed.

Currently, the parameters to the indication callback are

1. The connection the indication was sent on
2. The attribute that was indicated
3. Any error codes

It is not possible to recover the original struct bt_gatt_indicate_params instance from any of these parameters.
The current parameters are also inconsistent with other GATT callbacks (bt_gatt_read_func_t, bt_gatt_write_func_t, bt_gatt_notify_func_t, etc), which do provide the original parameter struct.

Additionally, once the struct bt_gatt_indicate_params can be recovered, it is also necessary to determine in the callback whether this is the last usage of this struct. This is because bt_gatt_indicate can take NULL as the connection, in which case it will be distributed to all connected devices that have subscribed to the characteristic. We cannot free any instances of the struct until all callbacks have been run.

Proposed change

Swap the second parameter of bt_gatt_indicate_func_t from the attribute pointer to the original struct bt_gatt_indicate_params used to call this function. Add a field to struct bt_gatt_indicate_params that is populated by the host to allow the callback to know when all indication responses have been received.

Detailed RFC

Proposed change (Detailed)

Callback Update

Update the indication callback typedef from:

typedef void (*bt_gatt_indicate_func_t)(struct bt_conn *conn,
					const struct bt_gatt_attr *attr,
					uint8_t err);

to:

typedef void (*bt_gatt_indicate_func_t)(struct bt_conn *conn,
					struct bt_gatt_indicate_params *params,
					uint8_t err);

As struct bt_gatt_indicate_params contains a pointer to struct bt_gatt_attr, there is no loss to information available in the callback. All current uses of the attr parameter can be kept by simply prepending params-> to the existing use.

Usage Counter

Add the following field to struct bt_gatt_indicate_params:

	/** Host populated counter that signifies the last use of this structure when 0 */
	int usage_count;

Inside bt_gatt_indicate, this value is set to 1 in the case the connection is provided, otherwise incremented in notify_cb for each subscribed connection found.

This value is decremented in the callback function (thread safe as they all run on the system-workqueue), with the intended user callback usage being:

static void indicate_cb(struct bt_conn *conn, struct bt_gatt_indicate_params *params, uint8_t err)
{
    if(params->usage_count == 0) {
        /* Or equivalent free() call */
        k_mem_slab_free(&indication_params_slab, (void **)&params);
    }
}

Dependencies

None

Concerns and Unresolved Questions

None

Alternatives

Not considered