2014-09-09 01:03:04

by Arman Uguray

[permalink] [raw]
Subject: [PATCH BlueZ 0/7] shared/gatt-client: Handle notifications.

This patch set adds handle value notification/indication handling to
shared/gatt-client. Introduced are bt_gatt_client_register_notify and
bt_gatt_client_unregister_notify functions which register a user provided
callback for a specific characteristic value handle to be invoked when a
notification or indication is received from that characteristic.

The functions automatically write to the Client Characteristic Configuration
descriptor to enable or disable notifications/indications based on the
characteristic's properties bit field. An internal per-characteristic count is
kept, in which the CCC descriptor is only written to when the count transitions
from 0 -> 1 or 1 -> 0.

Also added the "register-notify" and "unregister-notify" commands to
tools/btgatt-client to test the above feature.

Arman Uguray (7):
shared/gatt-client: Introduce struct bt_gatt_characteristic_iter.
shared/gatt-client: Implement bt_gatt_client_register_notify.
shared/gatt-client: Implement bt_gatt_client_unregister_notify.
shared/gatt-client: Handle incoming not/ind PDUs.
tools/btgatt-client: Add the "register-notify" command.
tools/btgatt-client: Add "unregister-notify" command.
TODO: Reference counted notify functions implemented

TODO | 10 -
src/shared/gatt-client.c | 521 ++++++++++++++++++++++++++++++++++++++++++----
src/shared/gatt-client.h | 57 +++--
src/shared/gatt-helpers.c | 67 ------
src/shared/gatt-helpers.h | 9 -
tools/btgatt-client.c | 139 +++++++++++--
6 files changed, 657 insertions(+), 146 deletions(-)

--
2.1.0.rc2.206.gedb03e5



2014-09-09 15:52:11

by Arman Uguray

[permalink] [raw]
Subject: Re: [PATCH BlueZ 2/7] shared/gatt-client: Implement bt_gatt_client_register_notify.

Hi Marcel,


>> Without the cast the code will generate a compiler warning, since
>> |descs| is declared as "const bt_descriptor_t *". I'd rather keep it
>> as const since external code will access it but I need the cast to
>> avoid the warning. I could cast it to void* if that makes it shorter.
>
> ouch. We should not be freeing const pointers. If we allocate the pointer, then it should not be const.
>

Yeah, my idea here was that the pointer is allocated internally by
bt_gatt_client and exposed externally as a const pointer. I put the
const there mainly to make it explicit that individual
bt_gatt_descriptor_t entries should not be modified and because I
wanted to avoid yet another nested iterator.

Now that I think of it, I can work around this by keeping the pointer
inside bt_gatt_characteristic_t const and have that point to an
internal non-const parameter in struct chrc_data. This should be OK
since the whole point of struct chrc_data here is to encapsulate
internal variables that should not be accessed or modified externally.

-Arman

2014-09-09 15:23:43

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH BlueZ 2/7] shared/gatt-client: Implement bt_gatt_client_register_notify.

Hi Arman,

>>> + for (i = 0; i < service->num_chrcs; i++) {
>>> + free((bt_gatt_descriptor_t *) service->chrcs[i].chrc.descs);
>>
>> I must have overlooked these before, but that casting for free() should not be needed at all.
>>
>
> Without the cast the code will generate a compiler warning, since
> |descs| is declared as "const bt_descriptor_t *". I'd rather keep it
> as const since external code will access it but I need the cast to
> avoid the warning. I could cast it to void* if that makes it shorter.

ouch. We should not be freeing const pointers. If we allocate the pointer, then it should not be const.

>>> +
>>> + be16 = htons(uuid16);
>>> + memcpy(rhs_uuid + 2, &be16, sizeof(be16));
>>
>> You might away with that the be16 and rhs_uuid are properly aligned here. However that is a dangerous game and I would rather see that you use put_be16() here.
>>
>
> Makes sense. FYI, I took this straight from bt_uuid16_to_uuid128 in lib/uuid.c.

Not everything in lib/uuid.c is a good example ;)

Regards

Marcel


2014-09-09 15:04:31

by Arman Uguray

[permalink] [raw]
Subject: Re: [PATCH BlueZ 2/7] shared/gatt-client: Implement bt_gatt_client_register_notify.

Hi Marcel,


>> + for (i = 0; i < service->num_chrcs; i++) {
>> + free((bt_gatt_descriptor_t *) service->chrcs[i].chrc.descs);
>
> I must have overlooked these before, but that casting for free() should not be needed at all.
>

Without the cast the code will generate a compiler warning, since
|descs| is declared as "const bt_descriptor_t *". I'd rather keep it
as const since external code will access it but I need the cast to
avoid the warning. I could cast it to void* if that makes it shorter.


>> +
>> + be16 = htons(uuid16);
>> + memcpy(rhs_uuid + 2, &be16, sizeof(be16));
>
> You might away with that the be16 and rhs_uuid are properly aligned here. However that is a dangerous game and I would rather see that you use put_be16() here.
>

Makes sense. FYI, I took this straight from bt_uuid16_to_uuid128 in lib/uuid.c.

-Arman

2014-09-09 14:09:41

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH BlueZ 4/7] shared/gatt-client: Handle incoming not/ind PDUs.

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(&notd->ref_count, 1);
> +
> + return notd;
> +}
> +
> +static void notify_data_unref(void *data)
> +{
> + struct notify_data *notd = data;
> +
> + if (__sync_sub_and_fetch(&notd->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(&notd->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(&notd->ref_count, 1);
> -
> - return notd;
> -}
> -
> -static void notify_data_unref(void *data)
> -{
> - struct notify_data *notd = data;
> -
> - if (__sync_sub_and_fetch(&notd->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(&notd->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


2014-09-09 14:07:10

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH BlueZ 3/7] shared/gatt-client: Implement bt_gatt_client_unregister_notify.

Hi Arman,

> This patch implements bt_gatt_client_unregister_notify, which is used to remove
> a previous registered notification/indication handler. This function decreases
> the per-characteristic count of handlers, and if the count drops to 0, a write
> request is sent to the CCC descriptor to disable notifications/indications.
> ---
> src/shared/gatt-client.c | 52 +++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 49 insertions(+), 3 deletions(-)
>
> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
> index d1b5c12..dfd37f8 100644
> --- a/src/shared/gatt-client.c
> +++ b/src/shared/gatt-client.c
> @@ -1480,6 +1480,14 @@ static void notify_data_unref(void *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;
> @@ -1541,13 +1549,27 @@ static void enable_ccc_callback(uint8_t opcode, const void *pdu,
> static void disable_ccc_callback(uint8_t opcode, const void *pdu,
> uint16_t length, void *user_data)
> {
> - /* TODO */
> + struct notify_data *notd = user_data;
> +
> + assert(!notd->chrc->not_ref_count);
> + assert(notd->chrc->ccc_write_id);
> +
> + notd->chrc->ccc_write_id = 0;
> +
> + /* This is a best effort procedure, so ignore errors and process any
> + * queued requests.
> + */
> + while ((notd = queue_pop_head(notd->chrc->register_not_queue))) {
> + if (notify_data_write_ccc(notd, true))
> + return;
> + }

while (1) {
notd = queue_pop_head(..)
if (!notd)
break;

if (!notify..)
break;

}

That said, I still find the notd naming and the overwriting notd with an entry from notd->chrc->.. highly confusing. You need to put in comments here on why this is all sane (if it is) and that we do not leak memory.

> }
>
> static bool notify_data_write_ccc(struct notify_data *notd, bool enable)
> {
> uint8_t pdu[4];
>
> + assert(notd->chrc->ccc_handle);
> memset(pdu, 0, sizeof(pdu));
> put_le16(notd->chrc->ccc_handle, pdu);
>
> @@ -1659,6 +1681,30 @@ bool bt_gatt_client_register_notify(struct bt_gatt_client *client,
> bool bt_gatt_client_unregister_notify(struct bt_gatt_client *client,
> unsigned int id)
> {
> - /* TODO */
> - return false;
> + struct notify_data *notd;
> +
> + if (!client || !id)
> + return false;
> +
> + notd = queue_find(client->notify_list, match_notify_data_id,
> + UINT_TO_PTR(id));
> + if (!notd)
> + 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(&notd->chrc->not_ref_count, 1)) {
> + notify_data_unref(notd);
> + return true;
> + }
> +
> + notify_data_write_ccc(notd, false);
> +
> + return true;
> }

Regards

Marcel


2014-09-09 14:03:45

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH BlueZ 2/7] shared/gatt-client: Implement bt_gatt_client_register_notify.

Hi Arman,

> This patch introduces the bt_gatt_client_register_notify and
> bt_gatt_client_unregister_notify functions and implements the latter. The
> bt_gatt_client_register_notify function does the following:
>
> - If the given characteristic has a CCC descriptor, it writes to it based on
> which notify/indicate properties are present in the characteristic
> properties bit field.
>
> - It maintains a per-characteristic count of how many callbacks have been
> registered, so that the CCC descriptor is written to only if the count is 0.
>
> - It stores the passed in callback and user data in bt_gatt_client, which will
> be invoked when a notification/indication is received from the corresponding
> characteristic.
>
> - It queues register requests if the count is 0 and a CCC write is pending,
> and processes them once the write request has completed.
> ---
> src/shared/gatt-client.c | 321 +++++++++++++++++++++++++++++++++++++++++++----
> src/shared/gatt-client.h | 25 ++++
> 2 files changed, 323 insertions(+), 23 deletions(-)
>
> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
> index fd866b2..d1b5c12 100644
> --- a/src/shared/gatt-client.c
> +++ b/src/shared/gatt-client.c
> @@ -28,6 +28,9 @@
> #include "src/shared/util.h"
> #include "src/shared/queue.h"
>
> +#include <assert.h>
> +#include <limits.h>
> +
> #ifndef MAX
> #define MAX(a, b) ((a) > (b) ? (a) : (b))
> #endif
> @@ -38,9 +41,21 @@
>
> #define UUID_BYTES (BT_GATT_UUID_SIZE * sizeof(uint8_t))
>
> +struct chrc_data {
> + bt_gatt_characteristic_t chrc;
> + uint16_t ccc_handle;
> + int not_ref_count;
> +
> + /* Pending calls to register_notify are queued here so that they can be
> + * processed after a write that modifies the CCC descriptor.
> + */
> + struct queue *register_not_queue;

this is not really a good variable name. Using not as shortcut for notify is a pretty bad idea.

> + unsigned int ccc_write_id;
> +};
> +
> struct service_list {
> bt_gatt_service_t service;
> - bt_gatt_characteristic_t *chrcs;
> + struct chrc_data *chrcs;
> size_t num_chrcs;
> struct service_list *next;
> };
> @@ -69,6 +84,10 @@ struct bt_gatt_client {
> */
> struct queue *long_write_queue;
> bool in_long_write;
> +
> + /* List of registered notification/indication callbacks */
> + struct queue *notify_list;
> + int next_reg_id;
> };
>
> static bool gatt_client_add_service(struct bt_gatt_client *client,
> @@ -95,12 +114,17 @@ static bool gatt_client_add_service(struct bt_gatt_client *client,
> return true;
> }
>
> +static void notify_data_unref(void *data);
> +
> static void service_destroy_characteristics(struct service_list *service)
> {
> unsigned int i;
>
> - for (i = 0; i < service->num_chrcs; i++)
> - free((bt_gatt_descriptor_t *) service->chrcs[i].descs);
> + for (i = 0; i < service->num_chrcs; i++) {
> + free((bt_gatt_descriptor_t *) service->chrcs[i].chrc.descs);

I must have overlooked these before, but that casting for free() should not be needed at all.

> + queue_destroy(service->chrcs[i].register_not_queue,
> + notify_data_unref);
> + }
>
> free(service->chrcs);
> }
> @@ -124,7 +148,7 @@ static void gatt_client_clear_services(struct bt_gatt_client *client)
> struct discovery_op {
> struct bt_gatt_client *client;
> struct service_list *cur_service;
> - bt_gatt_characteristic_t *cur_chrc;
> + struct chrc_data *cur_chrc;
> int cur_chrc_index;
> int ref_count;
> };
> @@ -176,6 +200,20 @@ static void discover_chrcs_cb(bool success, uint8_t att_ecode,
> struct bt_gatt_result *result,
> void *user_data);
>
> +static int uuid_cmp(const uint8_t uuid128[16], uint16_t uuid16)
> +{
> + uint16_t be16;
> + uint8_t rhs_uuid[16] = {
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00,
> + 0x80, 0x00, 0x00, 0x80, 0x5F, 0x9B, 0x34, 0xFB
> + };

> +
> + be16 = htons(uuid16);
> + memcpy(rhs_uuid + 2, &be16, sizeof(be16));

You might away with that the be16 and rhs_uuid are properly aligned here. However that is a dangerous game and I would rather see that you use put_be16() here.

> +
> + return memcmp(uuid128, rhs_uuid, sizeof(rhs_uuid));
> +}
> +
> static void discover_descs_cb(bool success, uint8_t att_ecode,
> struct bt_gatt_result *result,
> void *user_data)
> @@ -225,22 +263,26 @@ static void discover_descs_cb(bool success, uint8_t att_ecode,
> util_debug(client->debug_callback, client->debug_data,
> "handle: 0x%04x, uuid: %s",
> descs[i].handle, uuid_str);
> +
> + if (uuid_cmp(descs[i].uuid, GATT_CLIENT_CHARAC_CFG_UUID) == 0)
> + op->cur_chrc->ccc_handle = descs[i].handle;
> +
> i++;
> }
>
> - op->cur_chrc->num_descs = desc_count;
> - op->cur_chrc->descs = descs;
> + op->cur_chrc->chrc.num_descs = desc_count;
> + op->cur_chrc->chrc.descs = descs;
>
> for (i = op->cur_chrc_index + 1; i < op->cur_service->num_chrcs; i++) {
> op->cur_chrc_index = i;
> op->cur_chrc++;
> - desc_start = op->cur_chrc->value_handle + 1;
> - if (desc_start > op->cur_chrc->end_handle)
> + desc_start = op->cur_chrc->chrc.value_handle + 1;
> + if (desc_start > op->cur_chrc->chrc.end_handle)
> continue;
>
> if (bt_gatt_discover_descriptors(client->att,
> desc_start,
> - op->cur_chrc->end_handle,
> + op->cur_chrc->chrc.end_handle,
> discover_descs_cb,
> discovery_op_ref(op),
> discovery_op_unref))
> @@ -288,7 +330,7 @@ static void discover_chrcs_cb(bool success, uint8_t att_ecode,
> unsigned int chrc_count;
> unsigned int i;
> uint16_t desc_start;
> - bt_gatt_characteristic_t *chrcs;
> + struct chrc_data *chrcs;
>
> if (!success) {
> if (att_ecode == BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND) {
> @@ -311,25 +353,35 @@ static void discover_chrcs_cb(bool success, uint8_t att_ecode,
> if (chrc_count == 0)
> goto next;
>
> - chrcs = new0(bt_gatt_characteristic_t, chrc_count);
> + chrcs = new0(struct chrc_data, chrc_count);
> if (!chrcs) {
> success = false;
> goto done;
> }
>
> i = 0;
> - while (bt_gatt_iter_next_characteristic(&iter, &chrcs[i].start_handle,
> - &chrcs[i].end_handle,
> - &chrcs[i].value_handle,
> - &chrcs[i].properties,
> - chrcs[i].uuid)) {
> - uuid_to_string(chrcs[i].uuid, uuid_str);
> + while (bt_gatt_iter_next_characteristic(&iter,
> + &chrcs[i].chrc.start_handle,
> + &chrcs[i].chrc.end_handle,
> + &chrcs[i].chrc.value_handle,
> + &chrcs[i].chrc.properties,
> + chrcs[i].chrc.uuid)) {
> + uuid_to_string(chrcs[i].chrc.uuid, uuid_str);
> util_debug(client->debug_callback, client->debug_data,
> "start: 0x%04x, end: 0x%04x, value: 0x%04x, "
> "props: 0x%02x, uuid: %s",
> - chrcs[i].start_handle, chrcs[i].end_handle,
> - chrcs[i].value_handle, chrcs[i].properties,
> + chrcs[i].chrc.start_handle,
> + chrcs[i].chrc.end_handle,
> + chrcs[i].chrc.value_handle,
> + chrcs[i].chrc.properties,
> uuid_str);
> +
> + chrcs[i].register_not_queue = queue_new();
> + if (!chrcs[i].register_not_queue) {
> + success = false;
> + goto done;
> + }
> +
> i++;
> }
>
> @@ -339,12 +391,13 @@ static void discover_chrcs_cb(bool success, uint8_t att_ecode,
> for (i = 0; i < chrc_count; i++) {
> op->cur_chrc_index = i;
> op->cur_chrc = chrcs + i;
> - desc_start = chrcs[i].value_handle + 1;
> - if (desc_start > chrcs[i].end_handle)
> + desc_start = chrcs[i].chrc.value_handle + 1;
> + if (desc_start > chrcs[i].chrc.end_handle)
> continue;
>
> if (bt_gatt_discover_descriptors(client->att,
> - desc_start, chrcs[i].end_handle,
> + desc_start,
> + chrcs[i].chrc.end_handle,
> discover_descs_cb,
> discovery_op_ref(op),
> discovery_op_unref))
> @@ -533,6 +586,13 @@ struct bt_gatt_client *bt_gatt_client_new(struct bt_att *att, uint16_t mtu)
> return NULL;
> }
>
> + client->notify_list = queue_new();
> + if (!client->notify_list) {
> + queue_destroy(client->long_write_queue, NULL);
> + free(client);
> + return NULL;
> + }
> +
> client->att = bt_att_ref(att);
>
> gatt_client_init(client, mtu);
> @@ -567,6 +627,7 @@ void bt_gatt_client_unref(struct bt_gatt_client *client)
> client->debug_destroy(client->debug_data);
>
> 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);
> }
> @@ -700,7 +761,7 @@ bool bt_gatt_characteristic_iter_next(struct bt_gatt_characteristic_iter *iter,
> if (iter->pos >= service->num_chrcs)
> return false;
>
> - *chrc = service->chrcs + iter->pos++;
> + *chrc = &service->chrcs[iter->pos++].chrc;
>
> return true;
> }
> @@ -1387,3 +1448,217 @@ 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(&notd->ref_count, 1);
> +
> + return notd;
> +}
> +
> +static void notify_data_unref(void *data)
> +{
> + struct notify_data *notd = data;
> +
> + if (__sync_sub_and_fetch(&notd->ref_count, 1))
> + return;
> +
> + if (notd->destroy)
> + notd->destroy(notd->user_data);
> +
> + free(notd);
> +}
> +
> +static void complete_notify_request(void *data)
> +{
> + struct notify_data *notd = data;
> +
> + /* Increment the per-characteristic ref count of notify handlers */
> + __sync_fetch_and_add(&notd->chrc->not_ref_count, 1);
> +
> + /* Add the handler to the bt_gatt_client's general list */
> + queue_push_tail(notd->client->notify_list, notify_data_ref(notd));
> +
> + /* Assign an ID to the handler and notify the caller that it was
> + * successfully registered.
> + */
> + if (notd->client->next_reg_id < 1)
> + notd->client->next_reg_id = 1;
> +
> + notd->id = notd->client->next_reg_id++;
> + notd->callback(notd->id, 0, notd->user_data);
> +}
> +
> +static bool notify_data_write_ccc(struct notify_data *notd, bool enable);
> +

Is there no way to shuffle the functions around to avoid this forward declaration?

> +static void enable_ccc_callback(uint8_t opcode, const void *pdu,
> + uint16_t length, void *user_data)
> +{
> + struct notify_data *notd = user_data;
> + uint16_t att_ecode;
> +
> + assert(!notd->chrc->not_ref_count);
> + assert(notd->chrc->ccc_write_id);
> +
> + notd->chrc->ccc_write_id = 0;
> +
> + if (opcode == BT_ATT_OP_ERROR_RSP) {
> + att_ecode = process_error(pdu, length);
> +
> + /* Failed to enable. Complete the current request and move on to
> + * the next one in the queue. If there was an error sending the
> + * write request, then just move on to the next queued entry.
> + */
> + notd->callback(0, att_ecode, notd->user_data);
> +
> + while ((notd = queue_pop_head(
> + notd->chrc->register_not_queue))) {
> +
> + if (notify_data_write_ccc(notd, true))
> + return;
> + }
> +
> + return;
> + }
> +
> + /* Success! Report success for all remaining requests. */
> + complete_notify_request(notd);
> + queue_remove_all(notd->chrc->register_not_queue, NULL, NULL,
> + complete_notify_request);
> +}
> +
> +static void disable_ccc_callback(uint8_t opcode, const void *pdu,
> + uint16_t length, void *user_data)
> +{
> + /* TODO */
> +}
> +
> +static bool notify_data_write_ccc(struct notify_data *notd, bool enable)
> +{
> + uint8_t pdu[4];
> +
> + memset(pdu, 0, sizeof(pdu));
> + put_le16(notd->chrc->ccc_handle, pdu);
> +
> + if (enable) {
> + /* Try to enable notifications and/or indications based on
> + * whatever the characteristic supports.
> + */
> + if (notd->chrc->chrc.properties & BT_GATT_CHRC_PROP_NOTIFY)
> + pdu[2] = 0x01;
> +
> + if (notd->chrc->chrc.properties & BT_GATT_CHRC_PROP_INDICATE)
> + pdu[2] |= 0x02;
> +
> + if (!pdu[2])
> + return false;
> + }
> +
> + notd->chrc->ccc_write_id = bt_att_send(notd->client->att,
> + BT_ATT_OP_WRITE_REQ,
> + pdu, sizeof(pdu),
> + (enable ? enable_ccc_callback : disable_ccc_callback),
> + notd, notify_data_unref);
> +
> + return !!notd->chrc->ccc_write_id;
> +}
> +
> +bool bt_gatt_client_register_notify(struct bt_gatt_client *client,
> + uint16_t chrc_value_handle,
> + 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)
> +{
> + struct notify_data *notd;
> + struct service_list *svc_data = NULL;
> + struct chrc_data *chrc = NULL;
> + struct bt_gatt_service_iter iter;
> + const bt_gatt_service_t *service;
> + size_t i;
> +
> + if (!client || !chrc_value_handle || !callback)
> + return false;
> +
> + if (!bt_gatt_client_is_ready(client))
> + return false;
> +
> + /* Check that chrc_value_handle belongs to a known characteristic */
> + if (!bt_gatt_service_iter_init(&iter, client))
> + return false;
> +
> + while (bt_gatt_service_iter_next(&iter, &service)) {
> + if (chrc_value_handle >= service->start_handle &&
> + chrc_value_handle <= service->end_handle) {
> + svc_data = (struct service_list *) service;
> + break;
> + }
> + }
> +
> + if (!svc_data)
> + return false;
> +
> + for (i = 0; i < svc_data->num_chrcs; i++) {
> + if (svc_data->chrcs[i].chrc.value_handle == chrc_value_handle) {
> + chrc = svc_data->chrcs + i;
> + break;
> + }
> + }
> +
> + /* Check that the characteristic supports notifications/indications */
> + if (!chrc || !chrc->ccc_handle || chrc->not_ref_count == INT_MAX)
> + return false;
> +
> + notd = new0(struct notify_data, 1);
> + if (!notd)
> + return false;
> +
> + notd->client = client;
> + notd->ref_count = 1;
> + notd->chrc = chrc;
> + notd->callback = callback;
> + notd->notify = notify;
> + notd->user_data = user_data;
> + notd->destroy = destroy;
> +
> + /* If a write to the CCC descriptor is in progress, then queue this
> + * request.
> + */
> + if (chrc->ccc_write_id) {
> + queue_push_tail(chrc->register_not_queue, notd);
> + return true;
> + }
> +
> + /* If the ref count is not zero, then notifications are already enabled.
> + */
> + if (chrc->not_ref_count > 0) {
> + complete_notify_request(notd);
> + return true;
> + }
> +
> + /* Write to the CCC descriptor */
> + if (!notify_data_write_ccc(notd, true)) {
> + free(notd);
> + return false;
> + }
> +
> + return true;
> +}
> +
> +bool bt_gatt_client_unregister_notify(struct bt_gatt_client *client,
> + unsigned int id)
> +{
> + /* TODO */
> + return false;
> +}
> diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
> index 417d964..7612a6e 100644
> --- a/src/shared/gatt-client.h
> +++ b/src/shared/gatt-client.h
> @@ -27,6 +27,16 @@
>
> #define BT_GATT_UUID_SIZE 16
>
> +#define BT_GATT_CHRC_PROP_BROADCAST 0x01
> +#define BT_GATT_CHRC_PROP_READ 0x02
> +#define BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP 0x04
> +#define BT_GATT_CHRC_PROP_WRITE 0x08
> +#define BT_GATT_CHRC_PROP_NOTIFY 0x10
> +#define BT_GATT_CHRC_PROP_INDICATE 0x20
> +#define BT_GATT_CHRC_PROP_AUTH 0x40
> +#define BT_GATT_CHRC_PROP_EXT_PROP 0x80
> +
> +/* Client Characteristic Configuration bit field */
> struct bt_gatt_client;
>
> struct bt_gatt_client *bt_gatt_client_new(struct bt_att *att, uint16_t mtu);
> @@ -41,6 +51,12 @@ typedef void (*bt_gatt_client_debug_func_t)(const char *str, void *user_data);
> typedef void (*bt_gatt_client_write_long_callback_t)(bool success,
> bool reliable_error, uint8_t att_ecode,
> void *user_data);
> +typedef void (*bt_gatt_client_notify_callback_t)(uint16_t value_handle,
> + const uint8_t *value, uint16_t length,
> + void *user_data);
> +typedef void (*bt_gatt_client_notify_id_callback_t)(unsigned int id,
> + uint16_t att_ecode,
> + void *user_data);
>
> bool bt_gatt_client_is_ready(struct bt_gatt_client *client);
> bool bt_gatt_client_set_ready_handler(struct bt_gatt_client *client,
> @@ -131,3 +147,12 @@ bool bt_gatt_client_write_long_value(struct bt_gatt_client *client,
> bt_gatt_client_write_long_callback_t callback,
> void *user_data,
> bt_gatt_client_destroy_func_t destroy);
> +
> +bool bt_gatt_client_register_notify(struct bt_gatt_client *client,
> + uint16_t chrc_value_handle,
> + 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);
> +bool bt_gatt_client_unregister_notify(struct bt_gatt_client *client,
> + unsigned int id);

Regards

Marcel


2014-09-09 01:03:11

by Arman Uguray

[permalink] [raw]
Subject: [PATCH BlueZ 7/7] TODO: Reference counted notify functions implemented

---
TODO | 10 ----------
1 file changed, 10 deletions(-)

diff --git a/TODO b/TODO
index 5277e39..3530de4 100644
--- a/TODO
+++ b/TODO
@@ -152,16 +152,6 @@ ATT/GATT (new shared stack)
Priority: Medium
Complexity: C1

-- Define API functions in shared/gatt-client for enabling/disabling
- characteristic handle-value notifications/indications on a reference counted
- basis. Code that is no longer interested in notifications from a
- characteristic should be able to unregister any callbacks without writing to
- the "Client Characteristic Configuration" descriptor if there are others who
- are still interested.
-
- Priority: Medium
- Complexity: C2
-
- Introduce a handler interface to shared/gatt-client which can be used by the
upper layer to determine when the link has been disconnected or an ATT
protocol request times out.
--
2.1.0.rc2.206.gedb03e5


2014-09-09 01:03:08

by Arman Uguray

[permalink] [raw]
Subject: [PATCH BlueZ 4/7] shared/gatt-client: Handle incoming not/ind PDUs.

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;
+ 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(&notd->ref_count, 1);
+
+ return notd;
+}
+
+static void notify_data_unref(void *data)
+{
+ struct notify_data *notd = data;
+
+ if (__sync_sub_and_fetch(&notd->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);
+
+static void complete_unregister_notify(void *data)
+{
+ struct notify_data *notd = data;
+
+ if (__sync_sub_and_fetch(&notd->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(&notd->ref_count, 1);
-
- return notd;
-}
-
-static void notify_data_unref(void *data)
-{
- struct notify_data *notd = data;
-
- if (__sync_sub_and_fetch(&notd->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(&notd->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);
--
2.1.0.rc2.206.gedb03e5


2014-09-09 01:03:10

by Arman Uguray

[permalink] [raw]
Subject: [PATCH BlueZ 6/7] tools/btgatt-client: Add "unregister-notify" command.

This patch adds the "unregister-notify" command which can be used to test the
bt_gatt_client_unregister_notify function.
---
tools/btgatt-client.c | 38 ++++++++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)

diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
index f95cc9c..d2d997e 100644
--- a/tools/btgatt-client.c
+++ b/tools/btgatt-client.c
@@ -770,6 +770,42 @@ static void cmd_register_notify(struct client *cli, char *cmd_str)
printf("\n");
}

+static void unregister_notify_usage(void)
+{
+ printf("Usage: unregister-notify <notify id>\n");
+}
+
+static void cmd_unregister_notify(struct client *cli, char *cmd_str)
+{
+ char *argv[2];
+ int argc = 0;
+ unsigned int id;
+ char *endptr = NULL;
+
+ if (!bt_gatt_client_is_ready(cli->gatt)) {
+ printf("GATT client not initialized\n");
+ return;
+ }
+
+ if (!parse_args(cmd_str, 1, argv, &argc) || argc != 1) {
+ unregister_notify_usage();
+ return;
+ }
+
+ id = strtol(argv[0], &endptr, 10);
+ if (!endptr || *endptr != '\0' || !id) {
+ printf("Invalid notify id: %s\n", argv[0]);
+ return;
+ }
+
+ if (!bt_gatt_client_unregister_notify(cli->gatt, id)) {
+ printf("Failed to unregister notify handler with id: %u\n", id);
+ return;
+ }
+
+ printf("Unregistered notify handler with id: %u\n", id);
+}
+
static void cmd_help(struct client *cli, char *cmd_str);

typedef void (*command_func_t)(struct client *cli, char *cmd_str);
@@ -791,6 +827,8 @@ static struct {
"Write long characteristic or descriptor value" },
{ "register-notify", cmd_register_notify,
"\tSubscribe to not/ind from a characteristic" },
+ { "unregister-notify", cmd_unregister_notify,
+ "Unregister a not/ind session"},
{ }
};

--
2.1.0.rc2.206.gedb03e5


2014-09-09 01:03:09

by Arman Uguray

[permalink] [raw]
Subject: [PATCH BlueZ 5/7] tools/btgatt-client: Add the "register-notify" command.

This patch adds the "register-notify" command, which registers a
notification/indication handler for a given characteristic value handle.
---
tools/btgatt-client.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 70 insertions(+)

diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
index 8bde1ee..f95cc9c 100644
--- a/tools/btgatt-client.c
+++ b/tools/btgatt-client.c
@@ -702,6 +702,74 @@ static void cmd_write_long_value(struct client *cli, char *cmd_str)
free(value);
}

+static void register_notify_usage(void)
+{
+ printf("Usage: register-notify <chrc value handle>\n");
+}
+
+static void notify_cb(uint16_t value_handle, const uint8_t *value,
+ uint16_t length, void *user_data)
+{
+ int i;
+
+ printf("\n\tHandle Value Not/Ind: 0x%04x - ", value_handle);
+
+ if (length == 0) {
+ PRLOG("(0 bytes)\n");
+ return;
+ }
+
+ printf("(%u bytes): ", length);
+
+ for (i = 0; i < length; i++)
+ printf("%02x ", value[i]);
+
+ PRLOG("\n");
+}
+
+static void register_notify_cb(unsigned int id, uint16_t att_ecode,
+ void *user_data)
+{
+ if (!id) {
+ PRLOG("Failed to register notify handler "
+ "- error code: 9x%02x\n", att_ecode);
+ return;
+ }
+
+ PRLOG("Registered notify handler with id: %u\n", id);
+}
+
+static void cmd_register_notify(struct client *cli, char *cmd_str)
+{
+ char *argv[2];
+ int argc = 0;
+ uint16_t value_handle;
+ char *endptr = NULL;
+
+ if (!bt_gatt_client_is_ready(cli->gatt)) {
+ printf("GATT client not initialized\n");
+ return;
+ }
+
+ if (!parse_args(cmd_str, 1, argv, &argc) || argc != 1) {
+ register_notify_usage();
+ return;
+ }
+
+ value_handle = strtol(argv[0], &endptr, 16);
+ if (!endptr || *endptr != '\0' || !value_handle) {
+ printf("Invalid value handle: %s\n", argv[0]);
+ return;
+ }
+
+ if (!bt_gatt_client_register_notify(cli->gatt, value_handle,
+ register_notify_cb,
+ notify_cb, NULL, NULL))
+ printf("Failed to register notify handler\n");
+
+ printf("\n");
+}
+
static void cmd_help(struct client *cli, char *cmd_str);

typedef void (*command_func_t)(struct client *cli, char *cmd_str);
@@ -721,6 +789,8 @@ static struct {
"\tWrite a characteristic or descriptor value" },
{ "write-long-value", cmd_write_long_value,
"Write long characteristic or descriptor value" },
+ { "register-notify", cmd_register_notify,
+ "\tSubscribe to not/ind from a characteristic" },
{ }
};

--
2.1.0.rc2.206.gedb03e5


2014-09-09 01:03:07

by Arman Uguray

[permalink] [raw]
Subject: [PATCH BlueZ 3/7] shared/gatt-client: Implement bt_gatt_client_unregister_notify.

This patch implements bt_gatt_client_unregister_notify, which is used to remove
a previous registered notification/indication handler. This function decreases
the per-characteristic count of handlers, and if the count drops to 0, a write
request is sent to the CCC descriptor to disable notifications/indications.
---
src/shared/gatt-client.c | 52 +++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 49 insertions(+), 3 deletions(-)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index d1b5c12..dfd37f8 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -1480,6 +1480,14 @@ static void notify_data_unref(void *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;
@@ -1541,13 +1549,27 @@ static void enable_ccc_callback(uint8_t opcode, const void *pdu,
static void disable_ccc_callback(uint8_t opcode, const void *pdu,
uint16_t length, void *user_data)
{
- /* TODO */
+ struct notify_data *notd = user_data;
+
+ assert(!notd->chrc->not_ref_count);
+ assert(notd->chrc->ccc_write_id);
+
+ notd->chrc->ccc_write_id = 0;
+
+ /* This is a best effort procedure, so ignore errors and process any
+ * queued requests.
+ */
+ while ((notd = queue_pop_head(notd->chrc->register_not_queue))) {
+ if (notify_data_write_ccc(notd, true))
+ return;
+ }
}

static bool notify_data_write_ccc(struct notify_data *notd, bool enable)
{
uint8_t pdu[4];

+ assert(notd->chrc->ccc_handle);
memset(pdu, 0, sizeof(pdu));
put_le16(notd->chrc->ccc_handle, pdu);

@@ -1659,6 +1681,30 @@ bool bt_gatt_client_register_notify(struct bt_gatt_client *client,
bool bt_gatt_client_unregister_notify(struct bt_gatt_client *client,
unsigned int id)
{
- /* TODO */
- return false;
+ struct notify_data *notd;
+
+ if (!client || !id)
+ return false;
+
+ notd = queue_find(client->notify_list, match_notify_data_id,
+ UINT_TO_PTR(id));
+ if (!notd)
+ 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(&notd->chrc->not_ref_count, 1)) {
+ notify_data_unref(notd);
+ return true;
+ }
+
+ notify_data_write_ccc(notd, false);
+
+ return true;
}
--
2.1.0.rc2.206.gedb03e5


2014-09-09 01:03:06

by Arman Uguray

[permalink] [raw]
Subject: [PATCH BlueZ 2/7] shared/gatt-client: Implement bt_gatt_client_register_notify.

This patch introduces the bt_gatt_client_register_notify and
bt_gatt_client_unregister_notify functions and implements the latter. The
bt_gatt_client_register_notify function does the following:

- If the given characteristic has a CCC descriptor, it writes to it based on
which notify/indicate properties are present in the characteristic
properties bit field.

- It maintains a per-characteristic count of how many callbacks have been
registered, so that the CCC descriptor is written to only if the count is 0.

- It stores the passed in callback and user data in bt_gatt_client, which will
be invoked when a notification/indication is received from the corresponding
characteristic.

- It queues register requests if the count is 0 and a CCC write is pending,
and processes them once the write request has completed.
---
src/shared/gatt-client.c | 321 +++++++++++++++++++++++++++++++++++++++++++----
src/shared/gatt-client.h | 25 ++++
2 files changed, 323 insertions(+), 23 deletions(-)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index fd866b2..d1b5c12 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -28,6 +28,9 @@
#include "src/shared/util.h"
#include "src/shared/queue.h"

+#include <assert.h>
+#include <limits.h>
+
#ifndef MAX
#define MAX(a, b) ((a) > (b) ? (a) : (b))
#endif
@@ -38,9 +41,21 @@

#define UUID_BYTES (BT_GATT_UUID_SIZE * sizeof(uint8_t))

+struct chrc_data {
+ bt_gatt_characteristic_t chrc;
+ uint16_t ccc_handle;
+ int not_ref_count;
+
+ /* Pending calls to register_notify are queued here so that they can be
+ * processed after a write that modifies the CCC descriptor.
+ */
+ struct queue *register_not_queue;
+ unsigned int ccc_write_id;
+};
+
struct service_list {
bt_gatt_service_t service;
- bt_gatt_characteristic_t *chrcs;
+ struct chrc_data *chrcs;
size_t num_chrcs;
struct service_list *next;
};
@@ -69,6 +84,10 @@ struct bt_gatt_client {
*/
struct queue *long_write_queue;
bool in_long_write;
+
+ /* List of registered notification/indication callbacks */
+ struct queue *notify_list;
+ int next_reg_id;
};

static bool gatt_client_add_service(struct bt_gatt_client *client,
@@ -95,12 +114,17 @@ static bool gatt_client_add_service(struct bt_gatt_client *client,
return true;
}

+static void notify_data_unref(void *data);
+
static void service_destroy_characteristics(struct service_list *service)
{
unsigned int i;

- for (i = 0; i < service->num_chrcs; i++)
- free((bt_gatt_descriptor_t *) service->chrcs[i].descs);
+ for (i = 0; i < service->num_chrcs; i++) {
+ free((bt_gatt_descriptor_t *) service->chrcs[i].chrc.descs);
+ queue_destroy(service->chrcs[i].register_not_queue,
+ notify_data_unref);
+ }

free(service->chrcs);
}
@@ -124,7 +148,7 @@ static void gatt_client_clear_services(struct bt_gatt_client *client)
struct discovery_op {
struct bt_gatt_client *client;
struct service_list *cur_service;
- bt_gatt_characteristic_t *cur_chrc;
+ struct chrc_data *cur_chrc;
int cur_chrc_index;
int ref_count;
};
@@ -176,6 +200,20 @@ static void discover_chrcs_cb(bool success, uint8_t att_ecode,
struct bt_gatt_result *result,
void *user_data);

+static int uuid_cmp(const uint8_t uuid128[16], uint16_t uuid16)
+{
+ uint16_t be16;
+ uint8_t rhs_uuid[16] = {
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00,
+ 0x80, 0x00, 0x00, 0x80, 0x5F, 0x9B, 0x34, 0xFB
+ };
+
+ be16 = htons(uuid16);
+ memcpy(rhs_uuid + 2, &be16, sizeof(be16));
+
+ return memcmp(uuid128, rhs_uuid, sizeof(rhs_uuid));
+}
+
static void discover_descs_cb(bool success, uint8_t att_ecode,
struct bt_gatt_result *result,
void *user_data)
@@ -225,22 +263,26 @@ static void discover_descs_cb(bool success, uint8_t att_ecode,
util_debug(client->debug_callback, client->debug_data,
"handle: 0x%04x, uuid: %s",
descs[i].handle, uuid_str);
+
+ if (uuid_cmp(descs[i].uuid, GATT_CLIENT_CHARAC_CFG_UUID) == 0)
+ op->cur_chrc->ccc_handle = descs[i].handle;
+
i++;
}

- op->cur_chrc->num_descs = desc_count;
- op->cur_chrc->descs = descs;
+ op->cur_chrc->chrc.num_descs = desc_count;
+ op->cur_chrc->chrc.descs = descs;

for (i = op->cur_chrc_index + 1; i < op->cur_service->num_chrcs; i++) {
op->cur_chrc_index = i;
op->cur_chrc++;
- desc_start = op->cur_chrc->value_handle + 1;
- if (desc_start > op->cur_chrc->end_handle)
+ desc_start = op->cur_chrc->chrc.value_handle + 1;
+ if (desc_start > op->cur_chrc->chrc.end_handle)
continue;

if (bt_gatt_discover_descriptors(client->att,
desc_start,
- op->cur_chrc->end_handle,
+ op->cur_chrc->chrc.end_handle,
discover_descs_cb,
discovery_op_ref(op),
discovery_op_unref))
@@ -288,7 +330,7 @@ static void discover_chrcs_cb(bool success, uint8_t att_ecode,
unsigned int chrc_count;
unsigned int i;
uint16_t desc_start;
- bt_gatt_characteristic_t *chrcs;
+ struct chrc_data *chrcs;

if (!success) {
if (att_ecode == BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND) {
@@ -311,25 +353,35 @@ static void discover_chrcs_cb(bool success, uint8_t att_ecode,
if (chrc_count == 0)
goto next;

- chrcs = new0(bt_gatt_characteristic_t, chrc_count);
+ chrcs = new0(struct chrc_data, chrc_count);
if (!chrcs) {
success = false;
goto done;
}

i = 0;
- while (bt_gatt_iter_next_characteristic(&iter, &chrcs[i].start_handle,
- &chrcs[i].end_handle,
- &chrcs[i].value_handle,
- &chrcs[i].properties,
- chrcs[i].uuid)) {
- uuid_to_string(chrcs[i].uuid, uuid_str);
+ while (bt_gatt_iter_next_characteristic(&iter,
+ &chrcs[i].chrc.start_handle,
+ &chrcs[i].chrc.end_handle,
+ &chrcs[i].chrc.value_handle,
+ &chrcs[i].chrc.properties,
+ chrcs[i].chrc.uuid)) {
+ uuid_to_string(chrcs[i].chrc.uuid, uuid_str);
util_debug(client->debug_callback, client->debug_data,
"start: 0x%04x, end: 0x%04x, value: 0x%04x, "
"props: 0x%02x, uuid: %s",
- chrcs[i].start_handle, chrcs[i].end_handle,
- chrcs[i].value_handle, chrcs[i].properties,
+ chrcs[i].chrc.start_handle,
+ chrcs[i].chrc.end_handle,
+ chrcs[i].chrc.value_handle,
+ chrcs[i].chrc.properties,
uuid_str);
+
+ chrcs[i].register_not_queue = queue_new();
+ if (!chrcs[i].register_not_queue) {
+ success = false;
+ goto done;
+ }
+
i++;
}

@@ -339,12 +391,13 @@ static void discover_chrcs_cb(bool success, uint8_t att_ecode,
for (i = 0; i < chrc_count; i++) {
op->cur_chrc_index = i;
op->cur_chrc = chrcs + i;
- desc_start = chrcs[i].value_handle + 1;
- if (desc_start > chrcs[i].end_handle)
+ desc_start = chrcs[i].chrc.value_handle + 1;
+ if (desc_start > chrcs[i].chrc.end_handle)
continue;

if (bt_gatt_discover_descriptors(client->att,
- desc_start, chrcs[i].end_handle,
+ desc_start,
+ chrcs[i].chrc.end_handle,
discover_descs_cb,
discovery_op_ref(op),
discovery_op_unref))
@@ -533,6 +586,13 @@ struct bt_gatt_client *bt_gatt_client_new(struct bt_att *att, uint16_t mtu)
return NULL;
}

+ client->notify_list = queue_new();
+ if (!client->notify_list) {
+ queue_destroy(client->long_write_queue, NULL);
+ free(client);
+ return NULL;
+ }
+
client->att = bt_att_ref(att);

gatt_client_init(client, mtu);
@@ -567,6 +627,7 @@ void bt_gatt_client_unref(struct bt_gatt_client *client)
client->debug_destroy(client->debug_data);

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);
}
@@ -700,7 +761,7 @@ bool bt_gatt_characteristic_iter_next(struct bt_gatt_characteristic_iter *iter,
if (iter->pos >= service->num_chrcs)
return false;

- *chrc = service->chrcs + iter->pos++;
+ *chrc = &service->chrcs[iter->pos++].chrc;

return true;
}
@@ -1387,3 +1448,217 @@ 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(&notd->ref_count, 1);
+
+ return notd;
+}
+
+static void notify_data_unref(void *data)
+{
+ struct notify_data *notd = data;
+
+ if (__sync_sub_and_fetch(&notd->ref_count, 1))
+ return;
+
+ if (notd->destroy)
+ notd->destroy(notd->user_data);
+
+ free(notd);
+}
+
+static void complete_notify_request(void *data)
+{
+ struct notify_data *notd = data;
+
+ /* Increment the per-characteristic ref count of notify handlers */
+ __sync_fetch_and_add(&notd->chrc->not_ref_count, 1);
+
+ /* Add the handler to the bt_gatt_client's general list */
+ queue_push_tail(notd->client->notify_list, notify_data_ref(notd));
+
+ /* Assign an ID to the handler and notify the caller that it was
+ * successfully registered.
+ */
+ if (notd->client->next_reg_id < 1)
+ notd->client->next_reg_id = 1;
+
+ notd->id = notd->client->next_reg_id++;
+ 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)
+{
+ struct notify_data *notd = user_data;
+ uint16_t att_ecode;
+
+ assert(!notd->chrc->not_ref_count);
+ assert(notd->chrc->ccc_write_id);
+
+ notd->chrc->ccc_write_id = 0;
+
+ if (opcode == BT_ATT_OP_ERROR_RSP) {
+ att_ecode = process_error(pdu, length);
+
+ /* Failed to enable. Complete the current request and move on to
+ * the next one in the queue. If there was an error sending the
+ * write request, then just move on to the next queued entry.
+ */
+ notd->callback(0, att_ecode, notd->user_data);
+
+ while ((notd = queue_pop_head(
+ notd->chrc->register_not_queue))) {
+
+ if (notify_data_write_ccc(notd, true))
+ return;
+ }
+
+ return;
+ }
+
+ /* Success! Report success for all remaining requests. */
+ complete_notify_request(notd);
+ queue_remove_all(notd->chrc->register_not_queue, NULL, NULL,
+ complete_notify_request);
+}
+
+static void disable_ccc_callback(uint8_t opcode, const void *pdu,
+ uint16_t length, void *user_data)
+{
+ /* TODO */
+}
+
+static bool notify_data_write_ccc(struct notify_data *notd, bool enable)
+{
+ uint8_t pdu[4];
+
+ memset(pdu, 0, sizeof(pdu));
+ put_le16(notd->chrc->ccc_handle, pdu);
+
+ if (enable) {
+ /* Try to enable notifications and/or indications based on
+ * whatever the characteristic supports.
+ */
+ if (notd->chrc->chrc.properties & BT_GATT_CHRC_PROP_NOTIFY)
+ pdu[2] = 0x01;
+
+ if (notd->chrc->chrc.properties & BT_GATT_CHRC_PROP_INDICATE)
+ pdu[2] |= 0x02;
+
+ if (!pdu[2])
+ return false;
+ }
+
+ notd->chrc->ccc_write_id = bt_att_send(notd->client->att,
+ BT_ATT_OP_WRITE_REQ,
+ pdu, sizeof(pdu),
+ (enable ? enable_ccc_callback : disable_ccc_callback),
+ notd, notify_data_unref);
+
+ return !!notd->chrc->ccc_write_id;
+}
+
+bool bt_gatt_client_register_notify(struct bt_gatt_client *client,
+ uint16_t chrc_value_handle,
+ 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)
+{
+ struct notify_data *notd;
+ struct service_list *svc_data = NULL;
+ struct chrc_data *chrc = NULL;
+ struct bt_gatt_service_iter iter;
+ const bt_gatt_service_t *service;
+ size_t i;
+
+ if (!client || !chrc_value_handle || !callback)
+ return false;
+
+ if (!bt_gatt_client_is_ready(client))
+ return false;
+
+ /* Check that chrc_value_handle belongs to a known characteristic */
+ if (!bt_gatt_service_iter_init(&iter, client))
+ return false;
+
+ while (bt_gatt_service_iter_next(&iter, &service)) {
+ if (chrc_value_handle >= service->start_handle &&
+ chrc_value_handle <= service->end_handle) {
+ svc_data = (struct service_list *) service;
+ break;
+ }
+ }
+
+ if (!svc_data)
+ return false;
+
+ for (i = 0; i < svc_data->num_chrcs; i++) {
+ if (svc_data->chrcs[i].chrc.value_handle == chrc_value_handle) {
+ chrc = svc_data->chrcs + i;
+ break;
+ }
+ }
+
+ /* Check that the characteristic supports notifications/indications */
+ if (!chrc || !chrc->ccc_handle || chrc->not_ref_count == INT_MAX)
+ return false;
+
+ notd = new0(struct notify_data, 1);
+ if (!notd)
+ return false;
+
+ notd->client = client;
+ notd->ref_count = 1;
+ notd->chrc = chrc;
+ notd->callback = callback;
+ notd->notify = notify;
+ notd->user_data = user_data;
+ notd->destroy = destroy;
+
+ /* If a write to the CCC descriptor is in progress, then queue this
+ * request.
+ */
+ if (chrc->ccc_write_id) {
+ queue_push_tail(chrc->register_not_queue, notd);
+ return true;
+ }
+
+ /* If the ref count is not zero, then notifications are already enabled.
+ */
+ if (chrc->not_ref_count > 0) {
+ complete_notify_request(notd);
+ return true;
+ }
+
+ /* Write to the CCC descriptor */
+ if (!notify_data_write_ccc(notd, true)) {
+ free(notd);
+ return false;
+ }
+
+ return true;
+}
+
+bool bt_gatt_client_unregister_notify(struct bt_gatt_client *client,
+ unsigned int id)
+{
+ /* TODO */
+ return false;
+}
diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
index 417d964..7612a6e 100644
--- a/src/shared/gatt-client.h
+++ b/src/shared/gatt-client.h
@@ -27,6 +27,16 @@

#define BT_GATT_UUID_SIZE 16

+#define BT_GATT_CHRC_PROP_BROADCAST 0x01
+#define BT_GATT_CHRC_PROP_READ 0x02
+#define BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP 0x04
+#define BT_GATT_CHRC_PROP_WRITE 0x08
+#define BT_GATT_CHRC_PROP_NOTIFY 0x10
+#define BT_GATT_CHRC_PROP_INDICATE 0x20
+#define BT_GATT_CHRC_PROP_AUTH 0x40
+#define BT_GATT_CHRC_PROP_EXT_PROP 0x80
+
+/* Client Characteristic Configuration bit field */
struct bt_gatt_client;

struct bt_gatt_client *bt_gatt_client_new(struct bt_att *att, uint16_t mtu);
@@ -41,6 +51,12 @@ typedef void (*bt_gatt_client_debug_func_t)(const char *str, void *user_data);
typedef void (*bt_gatt_client_write_long_callback_t)(bool success,
bool reliable_error, uint8_t att_ecode,
void *user_data);
+typedef void (*bt_gatt_client_notify_callback_t)(uint16_t value_handle,
+ const uint8_t *value, uint16_t length,
+ void *user_data);
+typedef void (*bt_gatt_client_notify_id_callback_t)(unsigned int id,
+ uint16_t att_ecode,
+ void *user_data);

bool bt_gatt_client_is_ready(struct bt_gatt_client *client);
bool bt_gatt_client_set_ready_handler(struct bt_gatt_client *client,
@@ -131,3 +147,12 @@ bool bt_gatt_client_write_long_value(struct bt_gatt_client *client,
bt_gatt_client_write_long_callback_t callback,
void *user_data,
bt_gatt_client_destroy_func_t destroy);
+
+bool bt_gatt_client_register_notify(struct bt_gatt_client *client,
+ uint16_t chrc_value_handle,
+ 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);
+bool bt_gatt_client_unregister_notify(struct bt_gatt_client *client,
+ unsigned int id);
--
2.1.0.rc2.206.gedb03e5


2014-09-09 01:03:05

by Arman Uguray

[permalink] [raw]
Subject: [PATCH BlueZ 1/7] shared/gatt-client: Introduce struct bt_gatt_characteristic_iter.

shared/gatt-client currently defines a service iterator which returns service
data in a copy. The user then accesses the service's characteristics by manually
going through an array of bt_gatt_characteristic_t.

This patch changes this by restricting access to individual characteristic
entries via a new characteristic iterator. This is done so that gatt-client code
can internally store private data on each characteristic (e.g. reference count
for notification sessions) which shouldn't be exposed to external code.

The code also changes the service iterator functions to return a pointer to an
internally stored bt_gatt_service_t structure rather than returning its contents
in a copy.
---
src/shared/gatt-client.c | 59 ++++++++++++++++++++++++++++++++++++------------
src/shared/gatt-client.h | 32 ++++++++++++++++----------
tools/btgatt-client.c | 31 ++++++++++++++-----------
3 files changed, 83 insertions(+), 39 deletions(-)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 1a157ec..fd866b2 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -40,6 +40,8 @@

struct service_list {
bt_gatt_service_t service;
+ bt_gatt_characteristic_t *chrcs;
+ size_t num_chrcs;
struct service_list *next;
};

@@ -93,14 +95,14 @@ static bool gatt_client_add_service(struct bt_gatt_client *client,
return true;
}

-static void service_destroy_characteristics(bt_gatt_service_t *service)
+static void service_destroy_characteristics(struct service_list *service)
{
unsigned int i;

for (i = 0; i < service->num_chrcs; i++)
free((bt_gatt_descriptor_t *) service->chrcs[i].descs);

- free((bt_gatt_characteristic_t *) service->chrcs);
+ free(service->chrcs);
}

static void gatt_client_clear_services(struct bt_gatt_client *client)
@@ -110,7 +112,7 @@ static void gatt_client_clear_services(struct bt_gatt_client *client)
l = client->svc_head;

while (l) {
- service_destroy_characteristics(&l->service);
+ service_destroy_characteristics(l);
tmp = l;
l = tmp->next;
free(tmp);
@@ -229,8 +231,7 @@ static void discover_descs_cb(bool success, uint8_t att_ecode,
op->cur_chrc->num_descs = desc_count;
op->cur_chrc->descs = descs;

- for (i = op->cur_chrc_index + 1;
- i < op->cur_service->service.num_chrcs; i++) {
+ for (i = op->cur_chrc_index + 1; i < op->cur_service->num_chrcs; i++) {
op->cur_chrc_index = i;
op->cur_chrc++;
desc_start = op->cur_chrc->value_handle + 1;
@@ -332,8 +333,8 @@ static void discover_chrcs_cb(bool success, uint8_t att_ecode,
i++;
}

- op->cur_service->service.chrcs = chrcs;
- op->cur_service->service.num_chrcs = chrc_count;
+ op->cur_service->chrcs = chrcs;
+ op->cur_service->num_chrcs = chrc_count;

for (i = 0; i < chrc_count; i++) {
op->cur_chrc_index = i;
@@ -627,7 +628,7 @@ bool bt_gatt_service_iter_init(struct bt_gatt_service_iter *iter,
}

bool bt_gatt_service_iter_next(struct bt_gatt_service_iter *iter,
- bt_gatt_service_t *service)
+ const bt_gatt_service_t **service)
{
struct service_list *l;

@@ -644,18 +645,18 @@ bool bt_gatt_service_iter_next(struct bt_gatt_service_iter *iter,
if (!l)
return false;

- *service = l->service;
+ *service = &l->service;
iter->ptr = l;

return true;
}

bool bt_gatt_service_iter_next_by_handle(struct bt_gatt_service_iter *iter,
- uint16_t start_handle,
- bt_gatt_service_t *service)
+ uint16_t start_handle,
+ const bt_gatt_service_t **service)
{
while (bt_gatt_service_iter_next(iter, service)) {
- if (service->start_handle == start_handle)
+ if ((*service)->start_handle == start_handle)
return true;
}

@@ -664,16 +665,46 @@ bool bt_gatt_service_iter_next_by_handle(struct bt_gatt_service_iter *iter,

bool bt_gatt_service_iter_next_by_uuid(struct bt_gatt_service_iter *iter,
const uint8_t uuid[BT_GATT_UUID_SIZE],
- bt_gatt_service_t *service)
+ const bt_gatt_service_t **service)
{
while (bt_gatt_service_iter_next(iter, service)) {
- if (memcmp(service->uuid, uuid, UUID_BYTES) == 0)
+ if (memcmp((*service)->uuid, uuid, UUID_BYTES) == 0)
return true;
}

return false;
}

+bool bt_gatt_characteristic_iter_init(struct bt_gatt_characteristic_iter *iter,
+ const bt_gatt_service_t *service)
+{
+ if (!iter || !service)
+ return false;
+
+ memset(iter, 0, sizeof(*iter));
+ iter->service = (struct service_list *) service;
+
+ return true;
+}
+
+bool bt_gatt_characteristic_iter_next(struct bt_gatt_characteristic_iter *iter,
+ const bt_gatt_characteristic_t **chrc)
+{
+ struct service_list *service;
+
+ if (!iter || !chrc)
+ return false;
+
+ service = iter->service;
+
+ if (iter->pos >= service->num_chrcs)
+ return false;
+
+ *chrc = service->chrcs + iter->pos++;
+
+ return true;
+}
+
struct read_op {
bt_gatt_client_read_callback_t callback;
void *user_data;
diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
index 8b0d334..417d964 100644
--- a/src/shared/gatt-client.h
+++ b/src/shared/gatt-client.h
@@ -53,6 +53,12 @@ bool bt_gatt_client_set_debug(struct bt_gatt_client *client,
bt_gatt_client_destroy_func_t destroy);

typedef struct {
+ uint16_t start_handle;
+ uint16_t end_handle;
+ uint8_t uuid[BT_GATT_UUID_SIZE];
+} bt_gatt_service_t;
+
+typedef struct {
uint16_t handle;
uint8_t uuid[BT_GATT_UUID_SIZE];
} bt_gatt_descriptor_t;
@@ -67,29 +73,31 @@ typedef struct {
size_t num_descs;
} bt_gatt_characteristic_t;

-typedef struct {
- uint16_t start_handle;
- uint16_t end_handle;
- uint8_t uuid[BT_GATT_UUID_SIZE];
- const bt_gatt_characteristic_t *chrcs;
- size_t num_chrcs;
-} bt_gatt_service_t;
-
struct bt_gatt_service_iter {
struct bt_gatt_client *client;
void *ptr;
};

+struct bt_gatt_characteristic_iter {
+ void *service;
+ size_t pos;
+};
+
bool bt_gatt_service_iter_init(struct bt_gatt_service_iter *iter,
struct bt_gatt_client *client);
bool bt_gatt_service_iter_next(struct bt_gatt_service_iter *iter,
- bt_gatt_service_t *service);
+ const bt_gatt_service_t **service);
bool bt_gatt_service_iter_next_by_handle(struct bt_gatt_service_iter *iter,
- uint16_t start_handle,
- bt_gatt_service_t *service);
+ uint16_t start_handle,
+ const bt_gatt_service_t **service);
bool bt_gatt_service_iter_next_by_uuid(struct bt_gatt_service_iter *iter,
const uint8_t uuid[BT_GATT_UUID_SIZE],
- bt_gatt_service_t *service);
+ const bt_gatt_service_t **service);
+
+bool bt_gatt_characteristic_iter_init(struct bt_gatt_characteristic_iter *iter,
+ const bt_gatt_service_t *service);
+bool bt_gatt_characteristic_iter_next(struct bt_gatt_characteristic_iter *iter,
+ const bt_gatt_characteristic_t **chrc);

typedef void (*bt_gatt_client_read_callback_t)(bool success, uint8_t att_ecode,
const uint8_t *value, uint16_t length,
diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
index d1395b2..8bde1ee 100644
--- a/tools/btgatt-client.c
+++ b/tools/btgatt-client.c
@@ -169,16 +169,21 @@ static void print_uuid(const uint8_t uuid[16])

static void print_service(const bt_gatt_service_t *service)
{
+ struct bt_gatt_characteristic_iter iter;
const bt_gatt_characteristic_t *chrc;
- size_t i, j;
+ size_t i;
+
+ if (!bt_gatt_characteristic_iter_init(&iter, service)) {
+ PRLOG("Failed to initialize characteristic iterator\n");
+ return;
+ }

printf(COLOR_RED "service" COLOR_OFF " - start: 0x%04x, "
"end: 0x%04x, uuid: ",
service->start_handle, service->end_handle);
print_uuid(service->uuid);

- for (i = 0; i < service->num_chrcs; i++) {
- chrc = service->chrcs + i;
+ while (bt_gatt_characteristic_iter_next(&iter, &chrc)) {
printf("\t " COLOR_YELLOW "charac" COLOR_OFF
" - start: 0x%04x, end: 0x%04x, "
"value: 0x%04x, props: 0x%02x, uuid: ",
@@ -186,13 +191,13 @@ static void print_service(const bt_gatt_service_t *service)
chrc->end_handle,
chrc->value_handle,
chrc->properties);
- print_uuid(service->chrcs[i].uuid);
+ print_uuid(chrc->uuid);

- for (j = 0; j < chrc->num_descs; j++) {
+ for (i = 0; i < chrc->num_descs; i++) {
printf("\t\t " COLOR_MAGENTA "descr" COLOR_OFF
" - handle: 0x%04x, uuid: ",
- chrc->descs[j].handle);
- print_uuid(chrc->descs[j].uuid);
+ chrc->descs[i].handle);
+ print_uuid(chrc->descs[i].uuid);
}
}

@@ -202,7 +207,7 @@ static void print_service(const bt_gatt_service_t *service)
static void print_services(struct client *cli)
{
struct bt_gatt_service_iter iter;
- bt_gatt_service_t service;
+ const bt_gatt_service_t *service;

if (!bt_gatt_service_iter_init(&iter, cli->gatt)) {
PRLOG("Failed to initialize service iterator\n");
@@ -212,13 +217,13 @@ static void print_services(struct client *cli)
printf("\n");

while (bt_gatt_service_iter_next(&iter, &service))
- print_service(&service);
+ print_service(service);
}

static void print_services_by_uuid(struct client *cli, const bt_uuid_t *uuid)
{
struct bt_gatt_service_iter iter;
- bt_gatt_service_t service;
+ const bt_gatt_service_t *service;

if (!bt_gatt_service_iter_init(&iter, cli->gatt)) {
PRLOG("Failed to initialize service iterator\n");
@@ -229,13 +234,13 @@ static void print_services_by_uuid(struct client *cli, const bt_uuid_t *uuid)

while (bt_gatt_service_iter_next_by_uuid(&iter, uuid->value.u128.data,
&service))
- print_service(&service);
+ print_service(service);
}

static void print_services_by_handle(struct client *cli, uint16_t handle)
{
struct bt_gatt_service_iter iter;
- bt_gatt_service_t service;
+ const bt_gatt_service_t *service;

if (!bt_gatt_service_iter_init(&iter, cli->gatt)) {
PRLOG("Failed to initialize service iterator\n");
@@ -245,7 +250,7 @@ static void print_services_by_handle(struct client *cli, uint16_t handle)
printf("\n");

while (bt_gatt_service_iter_next_by_handle(&iter, handle, &service))
- print_service(&service);
+ print_service(service);
}

static void ready_cb(bool success, uint8_t att_ecode, void *user_data)
--
2.1.0.rc2.206.gedb03e5