Hi Arman,
> bt_gatt_client now registers an incoming PDU handler for notification and
> indication PDUs. The PDU is parsed and routed to the notify handler registered
> with bt_gatt_client_register_notify for the corresponding characteristic value
> handle. A confirmation PDU is sent automatically for received indications.
>
> This patch removes the bt_gatt_register function from shared/gatt-helpers.
> ---
> src/shared/gatt-client.c | 201 ++++++++++++++++++++++++++++++++++------------
> src/shared/gatt-helpers.c | 67 ----------------
> src/shared/gatt-helpers.h | 9 ---
> 3 files changed, 150 insertions(+), 127 deletions(-)
>
> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
> index dfd37f8..74fe9e5 100644
> --- a/src/shared/gatt-client.c
> +++ b/src/shared/gatt-client.c
> @@ -88,6 +88,9 @@ struct bt_gatt_client {
> /* List of registered notification/indication callbacks */
> struct queue *notify_list;
> int next_reg_id;
> + unsigned int not_id, ind_id;
not_id is a bad name for meaning notify_id. I personally do not like ind_id either and we might have to find a more clearer name.
> + bool in_notify;
> + bool need_notify_cleanup;
> };
>
> static bool gatt_client_add_service(struct bt_gatt_client *client,
> @@ -569,6 +572,126 @@ static bool gatt_client_init(struct bt_gatt_client *client, uint16_t mtu)
> return true;
> }
>
> +struct notify_data {
> + struct bt_gatt_client *client;
> + bool removed;
> + unsigned int id;
> + int ref_count;
> + struct chrc_data *chrc;
> + bt_gatt_client_notify_id_callback_t callback;
> + bt_gatt_client_notify_callback_t notify;
> + void *user_data;
> + bt_gatt_client_destroy_func_t destroy;
> +};
> +
> +static struct notify_data *notify_data_ref(struct notify_data *notd)
> +{
> + __sync_fetch_and_add(¬d->ref_count, 1);
> +
> + return notd;
> +}
> +
> +static void notify_data_unref(void *data)
> +{
> + struct notify_data *notd = data;
> +
> + if (__sync_sub_and_fetch(¬d->ref_count, 1))
> + return;
> +
> + if (notd->destroy)
> + notd->destroy(notd->user_data);
> +
> + free(notd);
> +}
> +
> +static bool match_notify_data_id(const void *a, const void *b)
> +{
> + const struct notify_data *notd = a;
> + unsigned int id = PTR_TO_UINT(b);
> +
> + return notd->id == id;
> +}
> +
> +static bool match_notify_data_removed(const void *a, const void *b)
> +{
> + const struct notify_data *notd = a;
> +
> + return notd->removed;
> +}
> +
> +struct pdu_data {
> + const void *pdu;
> + uint16_t length;
> +};
> +
> +static bool notify_data_write_ccc(struct notify_data *notd, bool enable);
> +
Can we avoid forward declaration?
> +static void complete_unregister_notify(void *data)
> +{
> + struct notify_data *notd = data;
> +
> + if (__sync_sub_and_fetch(¬d->chrc->not_ref_count, 1)) {
> + notify_data_unref(notd);
> + return;
> + }
> +
> + notify_data_write_ccc(notd, false);
> +}
> +
> +static void notify_handler(void *data, void *user_data)
> +{
> + struct notify_data *notd = data;
> + struct pdu_data *pdu_data = user_data;
> + uint16_t value_handle;
> + const uint8_t *value = NULL;
> +
> + if (notd->removed)
> + return;
> +
> + value_handle = get_le16(pdu_data->pdu);
> +
> + if (notd->chrc->chrc.value_handle != value_handle)
> + return;
> +
> + if (pdu_data->length > 2)
> + value = pdu_data->pdu + 2;
> +
> + if (notd->notify)
> + notd->notify(value_handle, value, pdu_data->length - 2,
> + notd->user_data);
> +}
> +
> +static void notify_cb(uint8_t opcode, const void *pdu, uint16_t length,
> + void *user_data)
> +{
> + struct bt_gatt_client *client = user_data;
> + struct pdu_data pdu_data;
> +
> + bt_gatt_client_ref(client);
> +
> + client->in_notify = true;
> +
> + memset(&pdu_data, 0, sizeof(pdu_data));
> + pdu_data.pdu = pdu;
> + pdu_data.length = length;
> +
> + queue_foreach(client->notify_list, notify_handler, &pdu_data);
> +
> + client->in_notify = false;
> +
> + if (client->need_notify_cleanup) {
> + queue_remove_all(client->notify_list, match_notify_data_removed,
> + NULL, complete_unregister_notify);
> + client->need_notify_cleanup = false;
> + }
> +
> + if (opcode == BT_ATT_OP_HANDLE_VAL_IND)
> + bt_att_send(client->att, BT_ATT_OP_HANDLE_VAL_CONF, NULL, 0,
> + NULL, NULL, NULL);
> +
> + bt_gatt_client_unref(client);
> +}
> +
> struct bt_gatt_client *bt_gatt_client_new(struct bt_att *att, uint16_t mtu)
> {
> struct bt_gatt_client *client;
> @@ -593,6 +716,23 @@ struct bt_gatt_client *bt_gatt_client_new(struct bt_att *att, uint16_t mtu)
> return NULL;
> }
>
> + client->not_id = bt_att_register(att, BT_ATT_OP_HANDLE_VAL_NOT,
> + notify_cb, client, NULL);
> + if (!client->not_id) {
> + queue_destroy(client->long_write_queue, NULL);
> + free(client);
> + return NULL;
> + }
> +
> + client->ind_id = bt_att_register(att, BT_ATT_OP_HANDLE_VAL_IND,
> + notify_cb, client, NULL);
> + if (!client->ind_id) {
> + bt_att_unregister(att, client->not_id);
> + queue_destroy(client->long_write_queue, NULL);
> + free(client);
> + return NULL;
> + }
> +
> client->att = bt_att_ref(att);
>
> gatt_client_init(client, mtu);
> @@ -626,8 +766,12 @@ void bt_gatt_client_unref(struct bt_gatt_client *client)
> if (client->debug_destroy)
> client->debug_destroy(client->debug_data);
>
> + bt_att_unregister(client->att, client->not_id);
> + bt_att_unregister(client->att, client->ind_id);
> +
> queue_destroy(client->long_write_queue, long_write_op_unref);
> queue_destroy(client->notify_list, notify_data_unref);
> +
> bt_att_unref(client->att);
> free(client);
> }
> @@ -1449,45 +1593,6 @@ bool bt_gatt_client_write_long_value(struct bt_gatt_client *client,
> return true;
> }
>
> -struct notify_data {
> - struct bt_gatt_client *client;
> - unsigned int id;
> - int ref_count;
> - struct chrc_data *chrc;
> - bt_gatt_client_notify_id_callback_t callback;
> - bt_gatt_client_notify_callback_t notify;
> - void *user_data;
> - bt_gatt_client_destroy_func_t destroy;
> -};
> -
> -static struct notify_data *notify_data_ref(struct notify_data *notd)
> -{
> - __sync_fetch_and_add(¬d->ref_count, 1);
> -
> - return notd;
> -}
> -
> -static void notify_data_unref(void *data)
> -{
> - struct notify_data *notd = data;
> -
> - if (__sync_sub_and_fetch(¬d->ref_count, 1))
> - return;
> -
> - if (notd->destroy)
> - notd->destroy(notd->user_data);
> -
> - free(notd);
> -}
> -
> -static bool match_notify_data_id(const void *a, const void *b)
> -{
> - const struct notify_data *notd = a;
> - unsigned int id = PTR_TO_UINT(b);
> -
> - return notd->id == id;
> -}
> -
> static void complete_notify_request(void *data)
> {
> struct notify_data *notd = data;
> @@ -1508,8 +1613,6 @@ static void complete_notify_request(void *data)
> notd->callback(notd->id, 0, notd->user_data);
> }
>
> -static bool notify_data_write_ccc(struct notify_data *notd, bool enable);
> -
> static void enable_ccc_callback(uint8_t opcode, const void *pdu,
> uint16_t length, void *user_data)
> {
> @@ -1688,23 +1791,19 @@ bool bt_gatt_client_unregister_notify(struct bt_gatt_client *client,
>
> notd = queue_find(client->notify_list, match_notify_data_id,
> UINT_TO_PTR(id));
> - if (!notd)
> + if (!notd || notd->removed)
> return false;
>
> assert(notd->chrc->not_ref_count > 0);
> assert(!notd->chrc->ccc_write_id);
>
> - /* TODO: Delay destruction/removal if we're in the middle of processing
> - * a notification.
> - */
> - queue_remove(client->notify_list, notd);
> -
> - if (__sync_sub_and_fetch(¬d->chrc->not_ref_count, 1)) {
> - notify_data_unref(notd);
> + if (!client->in_notify) {
> + queue_remove(client->notify_list, notd);
> + complete_unregister_notify(notd);
> return true;
> }
>
> - notify_data_write_ccc(notd, false);
> -
> + notd->removed = true;
> + client->need_notify_cleanup = true;
> return true;
> }
> diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
> index ede6a67..54876bc 100644
> --- a/src/shared/gatt-helpers.c
> +++ b/src/shared/gatt-helpers.c
> @@ -911,70 +911,3 @@ bool bt_gatt_discover_descriptors(struct bt_att *att,
>
> return true;
> }
> -
> -struct notify_data {
> - struct bt_att *att;
> - bt_gatt_notify_callback_t callback;
> - void *user_data;
> - bt_gatt_destroy_func_t destroy;
> -};
> -
> -static void notify_data_destroy(void *data)
> -{
> - struct notify_data *notd = data;
> -
> - if (notd->destroy)
> - notd->destroy(notd->user_data);
> -
> - free(notd);
> -}
> -
> -static void notify_cb(uint8_t opcode, const void *pdu, uint16_t length,
> - void *user_data)
> -{
> - struct notify_data *data = user_data;
> - uint16_t value_handle;
> - const uint8_t *value = NULL;
> -
> - value_handle = get_le16(pdu);
> -
> - if (length > 2)
> - value = pdu + 2;
> -
> - if (data->callback)
> - data->callback(value_handle, value, length - 2, data->user_data);
> -
> - if (opcode == BT_ATT_OP_HANDLE_VAL_IND)
> - bt_att_send(data->att, BT_ATT_OP_HANDLE_VAL_CONF, NULL, 0,
> - NULL, NULL, NULL);
> -}
> -
> -unsigned int bt_gatt_register(struct bt_att *att, bool indications,
> - bt_gatt_notify_callback_t callback,
> - void *user_data,
> - bt_gatt_destroy_func_t destroy)
> -{
> - struct notify_data *data;
> - uint8_t opcode;
> - unsigned int id;
> -
> - if (!att)
> - return 0;
> -
> - data = new0(struct notify_data, 1);
> - if (!data)
> - return 0;
> -
> - data->att = att;
> - data->callback = callback;
> - data->user_data = user_data;
> - data->destroy = destroy;
> -
> - opcode = indications ? BT_ATT_OP_HANDLE_VAL_IND : BT_ATT_OP_HANDLE_VAL_NOT;
> -
> - id = bt_att_register(att, opcode, notify_cb, data, notify_data_destroy);
> - if (!id)
> - free(data);
> -
> - return id;
> -}
> diff --git a/src/shared/gatt-helpers.h b/src/shared/gatt-helpers.h
> index 75ad4b0..c4a6578 100644
> --- a/src/shared/gatt-helpers.h
> +++ b/src/shared/gatt-helpers.h
> @@ -58,10 +58,6 @@ typedef void (*bt_gatt_discovery_callback_t)(bool success, uint8_t att_ecode,
> struct bt_gatt_result *result,
> void *user_data);
>
> -typedef void (*bt_gatt_notify_callback_t)(uint16_t value_handle,
> - const uint8_t *value, uint16_t length,
> - void *user_data);
> -
> bool bt_gatt_exchange_mtu(struct bt_att *att, uint16_t client_rx_mtu,
> bt_gatt_result_callback_t callback,
> void *user_data,
> @@ -87,8 +83,3 @@ bool bt_gatt_discover_descriptors(struct bt_att *att,
> bt_gatt_discovery_callback_t callback,
> void *user_data,
> bt_gatt_destroy_func_t destroy);
> -
> -unsigned int bt_gatt_register(struct bt_att *att, bool indications,
> - bt_gatt_notify_callback_t callback,
> - void *user_data,
> - bt_gatt_destroy_func_t destroy);
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
