2014-10-02 08:25:34

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCH 1/4] shared/gatt: Add initial implementation of discover_included_services

Current implementation allow to discover included services with
16 bit UUID only.
---
src/shared/gatt-helpers.c | 108 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 106 insertions(+), 2 deletions(-)

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index 55e6992..3cebbe7 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -668,6 +668,82 @@ bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
return result;
}

+static void discover_included_cb(uint8_t opcode, const void *pdu,
+ uint16_t length, void *user_data)
+{
+ struct bt_gatt_result *final_result = NULL;
+ struct discovery_op *op = user_data;
+ struct bt_gatt_result *cur_result;
+ uint8_t att_ecode = 0;
+ uint16_t last_handle;
+ size_t data_length;
+ bool success;
+
+ if (opcode == BT_ATT_OP_ERROR_RSP) {
+ success = false;
+ att_ecode = process_error(pdu, length);
+
+ if (att_ecode == BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND &&
+ op->result_head)
+
+ goto success;
+
+ goto done;
+ }
+
+ if (opcode != BT_ATT_OP_READ_BY_TYPE_RSP || !pdu || length < 6) {
+ success = false;
+ goto done;
+ }
+
+ data_length = ((uint8_t *) pdu)[0];
+
+ if ((length - 1) % data_length || data_length != 6) {
+ success = false;
+ goto done;
+ }
+
+ cur_result = result_create(opcode, pdu + 1, length - 1,
+ data_length, op);
+ if (!cur_result) {
+ success = false;
+ goto done;
+ }
+
+ if (!op->result_head)
+ op->result_head = op->result_tail = cur_result;
+ else {
+ op->result_tail->next = cur_result;
+ op->result_tail = cur_result;
+ }
+
+ last_handle = get_le16(pdu + length - data_length);
+ if (last_handle != op->end_handle) {
+ uint8_t pdu[6];
+
+ put_le16(last_handle + 1, pdu);
+ put_le16(op->end_handle, pdu + 2);
+ put_le16(GATT_INCLUDE_UUID, pdu + 4);
+
+ if (bt_att_send(op->att, BT_ATT_OP_READ_BY_TYPE_REQ,
+ pdu, sizeof(pdu),
+ discover_included_cb,
+ discovery_op_ref(op),
+ discovery_op_unref))
+ return;
+
+ discovery_op_unref(op);
+ success = false;
+ }
+success:
+ success = true;
+ final_result = op->result_head;
+
+done:
+ if (op->callback)
+ op->callback(success, att_ecode, final_result, op->user_data);
+}
+
bool bt_gatt_discover_included_services(struct bt_att *att,
uint16_t start, uint16_t end,
bt_uuid_t *uuid,
@@ -675,8 +751,36 @@ bool bt_gatt_discover_included_services(struct bt_att *att,
void *user_data,
bt_gatt_destroy_func_t destroy)
{
- /* TODO */
- return false;
+ struct discovery_op *op;
+ uint8_t pdu[6];
+
+ if (!att)
+ return false;
+
+ op = new0(struct discovery_op, 1);
+ if (!op)
+ return false;
+
+ op->att = att;
+ op->callback = callback;
+ op->user_data = user_data;
+ op->destroy = destroy;
+ op->end_handle = end;
+
+ put_le16(start, pdu);
+ put_le16(end, pdu + 2);
+ put_le16(GATT_INCLUDE_UUID, pdu + 4);
+
+ if (!bt_att_send(att, BT_ATT_OP_READ_BY_TYPE_REQ,
+ pdu, sizeof(pdu),
+ discover_included_cb,
+ discovery_op_ref(op),
+ discovery_op_unref)) {
+ free(op);
+ return false;
+ }
+
+ return true;
}

static void discover_chrcs_cb(uint8_t opcode, const void *pdu,
--
1.9.3



2014-10-03 15:37:58

by Arman Uguray

[permalink] [raw]
Subject: Re: [PATCH 3/4] shared/gatt: Add included service iterator

Hi Marcin,

>>> +bool bt_gatt_iter_next_included_service(struct bt_gatt_iter *iter,
>>> + uint16_t *handle, uint16_t *start_handle,
>>> + uint16_t *end_handle, uint8_t uuid[16])
>>> +{
>>> + struct bt_gatt_result *read_result;
>>> + const void *pdu_ptr;
>>> + int i = 0;
>>> +
>>> + if (!iter || !iter->result || !handle || !start_handle || !end_handle
>>> + || !uuid)
>>> + return false;
>>> +
>>> + if (iter->result->opcode != BT_ATT_OP_READ_BY_TYPE_RSP)
>>> + return false;
>>
>> Remember that characteristics are also obtained via a Read By Type
>> request. I think we should make it so that
>> bt_gatt_iter_next_included_service and
>> bt_gatt_iter_next_characteristic return false if called on the
>> incorrect PDU. So it might make sense to add a flag to bt_gatt_result
>> such as "is_include" and check against that in both functions.
>>
>
> I think we can use data_length instead of flag. For characteristics
> data_len is 7 or 21, for included it is 6 or 8.
> Is it ok?

Yeah, that would work too (probably might be better actually, since
it's consistent with the ATT protocol PDU approach in those
iterators). So yeah, let's go with your suggestion.

Cheers,
Arman

2014-10-03 06:35:36

by Marcin Kraglak

[permalink] [raw]
Subject: Re: [PATCH 3/4] shared/gatt: Add included service iterator

Hi Arman,

On 3 October 2014 05:35, Arman Uguray <[email protected]> wrote:
> Hi Marcin,
>
>
>> +bool bt_gatt_iter_next_included_service(struct bt_gatt_iter *iter,
>> + uint16_t *handle, uint16_t *start_handle,
>> + uint16_t *end_handle, uint8_t uuid[16])
>> +{
>> + struct bt_gatt_result *read_result;
>> + const void *pdu_ptr;
>> + int i = 0;
>> +
>> + if (!iter || !iter->result || !handle || !start_handle || !end_handle
>> + || !uuid)
>> + return false;
>> +
>> + if (iter->result->opcode != BT_ATT_OP_READ_BY_TYPE_RSP)
>> + return false;
>
> Remember that characteristics are also obtained via a Read By Type
> request. I think we should make it so that
> bt_gatt_iter_next_included_service and
> bt_gatt_iter_next_characteristic return false if called on the
> incorrect PDU. So it might make sense to add a flag to bt_gatt_result
> such as "is_include" and check against that in both functions.
>

I think we can use data_length instead of flag. For characteristics
data_len is 7 or 21, for included it is 6 or 8.
Is it ok?
> Thanks,
> Arman

BR
Marcin

2014-10-03 03:35:11

by Arman Uguray

[permalink] [raw]
Subject: Re: [PATCH 3/4] shared/gatt: Add included service iterator

Hi Marcin,


> +bool bt_gatt_iter_next_included_service(struct bt_gatt_iter *iter,
> + uint16_t *handle, uint16_t *start_handle,
> + uint16_t *end_handle, uint8_t uuid[16])
> +{
> + struct bt_gatt_result *read_result;
> + const void *pdu_ptr;
> + int i = 0;
> +
> + if (!iter || !iter->result || !handle || !start_handle || !end_handle
> + || !uuid)
> + return false;
> +
> + if (iter->result->opcode != BT_ATT_OP_READ_BY_TYPE_RSP)
> + return false;

Remember that characteristics are also obtained via a Read By Type
request. I think we should make it so that
bt_gatt_iter_next_included_service and
bt_gatt_iter_next_characteristic return false if called on the
incorrect PDU. So it might make sense to add a flag to bt_gatt_result
such as "is_include" and check against that in both functions.

Thanks,
Arman

2014-10-02 08:25:35

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCH 2/4] shared/gatt: Discover includ services with 128 bit UUIDS

If included services has 128 bit UUID, it won't be returned in
READ_BY_TYPE_RSP. To get UUID READ_REQUEST is used.
This procedure is described in CORE SPEC 4.5.1 "Find Included Services".
---
src/shared/gatt-helpers.c | 170 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 169 insertions(+), 1 deletion(-)

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index 3cebbe7..12930a0 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -668,6 +668,160 @@ bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
return result;
}

+struct read_incl_data {
+ struct discovery_op *op;
+ struct bt_gatt_result *result;
+ int pos;
+ int ref_count;
+};
+
+static struct read_incl_data *new_read_included(struct bt_gatt_result *res)
+{
+ struct read_incl_data *data;
+
+ data = new0(struct read_incl_data, 1);
+ if (!data)
+ return NULL;
+
+ data->op = discovery_op_ref(res->op);
+ data->result = res;
+
+ return data;
+};
+
+static struct read_incl_data *read_included_ref(struct read_incl_data *data)
+{
+ __sync_fetch_and_add(&data->ref_count, 1);
+
+ return data;
+}
+
+static void read_included_unref(void *data)
+{
+ struct read_incl_data *read_data = data;
+
+ if (__sync_sub_and_fetch(&read_data->ref_count, 1))
+ return;
+
+ discovery_op_unref(read_data->op);
+
+ free(read_data);
+}
+
+static void discover_included_cb(uint8_t opcode, const void *pdu,
+ uint16_t length, void *user_data);
+
+static void read_included_cb(uint8_t opcode, const void *pdu,
+ uint16_t length, void *user_data)
+{
+ struct read_incl_data *data = user_data;
+ struct bt_gatt_result *final_result = NULL;
+ struct discovery_op *op = data->op;
+ struct bt_gatt_result *cur_result;
+ uint8_t att_ecode = 0;
+ uint16_t handle;
+ uint8_t read_pdu[2];
+ bool success;
+
+ if (opcode == BT_ATT_OP_ERROR_RSP) {
+ success = false;
+ att_ecode = process_error(pdu, length);
+ goto done;
+ }
+
+ if (opcode != BT_ATT_OP_READ_RSP || (!pdu && length)) {
+ success = false;
+ goto done;
+ }
+
+ if (length != 16) {
+ success = false;
+ goto done;
+ }
+ cur_result = result_create(opcode, pdu, length, length, op);
+ if (!cur_result) {
+ success = false;
+ goto done;
+ }
+
+ if (!op->result_head)
+ op->result_head = op->result_tail = cur_result;
+ else {
+ op->result_tail->next = cur_result;
+ op->result_tail = cur_result;
+ }
+
+ if (data->pos == data->result->pdu_len) {
+ uint16_t last_handle, pdu_len, data_len;
+ uint8_t pdu[6];
+
+ pdu_len = data->result->pdu_len;
+ data_len = data->result->data_len;
+ last_handle = get_le16(data->result->pdu + pdu_len - data_len);
+ if (last_handle == op->end_handle) {
+ final_result = op->result_head;
+ success = true;
+ goto done;
+ }
+
+ put_le16(last_handle + 1, pdu);
+ put_le16(op->end_handle, pdu + 2);
+ put_le16(GATT_INCLUDE_UUID, pdu + 4);
+
+ if (bt_att_send(op->att, BT_ATT_OP_READ_BY_TYPE_REQ,
+ pdu, sizeof(pdu),
+ discover_included_cb,
+ discovery_op_ref(op),
+ discovery_op_unref))
+ return;
+
+ discovery_op_unref(op);
+ success = false;
+ goto done;
+ }
+
+ handle = get_le16(data->result->pdu + data->pos + 2);
+ put_le16(handle, read_pdu);
+
+ data->pos += data->result->data_len;
+
+ if (bt_att_send(op->att, BT_ATT_OP_READ_REQ, read_pdu, sizeof(read_pdu),
+ read_included_cb,
+ read_included_ref(data),
+ read_included_unref))
+ return;
+
+ read_included_unref(data);
+ success = false;
+
+done:
+ if (op->callback)
+ op->callback(success, att_ecode, final_result, op->user_data);
+}
+
+static void read_included(struct read_incl_data *data)
+{
+ struct discovery_op *op = data->op;
+ uint16_t handle;
+ uint8_t pdu[2];
+
+ handle = get_le16(data->result->pdu + 2);
+ put_le16(handle, pdu);
+
+ data->pos += data->result->data_len;
+
+ if (bt_att_send(op->att, BT_ATT_OP_READ_REQ, pdu, sizeof(pdu),
+ read_included_cb,
+ read_included_ref(data),
+ read_included_unref))
+ return;
+
+ read_included_unref(data);
+
+ if (op->callback)
+ op->callback(false, 0, NULL, data->op->user_data);
+}
+
static void discover_included_cb(uint8_t opcode, const void *pdu,
uint16_t length, void *user_data)
{
@@ -698,7 +852,8 @@ static void discover_included_cb(uint8_t opcode, const void *pdu,

data_length = ((uint8_t *) pdu)[0];

- if ((length - 1) % data_length || data_length != 6) {
+ if (((length - 1) % data_length) ||
+ (data_length != 6 && data_length != 8)) {
success = false;
goto done;
}
@@ -717,6 +872,19 @@ static void discover_included_cb(uint8_t opcode, const void *pdu,
op->result_tail = cur_result;
}

+ if (data_length == 6) {
+ struct read_incl_data *data;
+
+ data = new_read_included(cur_result);
+ if (!data) {
+ success = false;
+ goto done;
+ }
+
+ read_included(data);
+ return;
+ }
+
last_handle = get_le16(pdu + length - data_length);
if (last_handle != op->end_handle) {
uint8_t pdu[6];
--
1.9.3


2014-10-02 08:25:36

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCH 3/4] shared/gatt: Add included service iterator

---
src/shared/gatt-helpers.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++
src/shared/gatt-helpers.h | 3 +++
2 files changed, 61 insertions(+)

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index 12930a0..ee96011 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -182,6 +182,64 @@ struct discovery_op {
bt_gatt_destroy_func_t destroy;
};

+bool bt_gatt_iter_next_included_service(struct bt_gatt_iter *iter,
+ uint16_t *handle, uint16_t *start_handle,
+ uint16_t *end_handle, uint8_t uuid[16])
+{
+ struct bt_gatt_result *read_result;
+ const void *pdu_ptr;
+ int i = 0;
+
+ if (!iter || !iter->result || !handle || !start_handle || !end_handle
+ || !uuid)
+ return false;
+
+ if (iter->result->opcode != BT_ATT_OP_READ_BY_TYPE_RSP)
+ return false;
+
+ if (iter->result->data_len != 8 && iter->result->data_len != 6)
+ return false;
+
+ pdu_ptr = iter->result->pdu + iter->pos;
+
+ if (iter->result->data_len == 8) {
+ *handle = get_le16(pdu_ptr);
+ *start_handle = get_le16(pdu_ptr + 2);
+ *end_handle = get_le16(pdu_ptr + 4);
+ convert_uuid_le(pdu_ptr + 6, 2, uuid);
+
+ iter->pos += iter->result->data_len;
+
+ if (iter->pos == iter->result->pdu_len) {
+ iter->result = iter->result->next;
+ iter->pos = 0;
+ }
+
+ return true;
+ }
+
+ *handle = get_le16(pdu_ptr);
+ *start_handle = get_le16(pdu_ptr + 2);
+ *end_handle = get_le16(pdu_ptr + 4);
+ read_result = iter->result;
+
+ do {
+ read_result = read_result->next;
+ } while (read_result && i++ < (iter->pos / iter->result->data_len));
+
+ if (!read_result)
+ return false;
+
+ convert_uuid_le(read_result->pdu, read_result->data_len, uuid);
+ iter->pos += iter->result->data_len;
+ if (iter->pos == iter->result->pdu_len) {
+ iter->result = read_result->next;
+ iter->pos = 0;
+ }
+
+ return true;
+}
+
bool bt_gatt_iter_next_service(struct bt_gatt_iter *iter,
uint16_t *start_handle, uint16_t *end_handle,
uint8_t uuid[16])
diff --git a/src/shared/gatt-helpers.h b/src/shared/gatt-helpers.h
index f6f4b62..da409fa 100644
--- a/src/shared/gatt-helpers.h
+++ b/src/shared/gatt-helpers.h
@@ -49,6 +49,9 @@ bool bt_gatt_iter_next_characteristic(struct bt_gatt_iter *iter,
uint8_t uuid[16]);
bool bt_gatt_iter_next_descriptor(struct bt_gatt_iter *iter, uint16_t *handle,
uint8_t uuid[16]);
+bool bt_gatt_iter_next_included_service(struct bt_gatt_iter *iter,
+ uint16_t *handle, uint16_t *start_handle,
+ uint16_t *end_handle, uint8_t uuid[16]);

typedef void (*bt_gatt_destroy_func_t)(void *user_data);

--
1.9.3


2014-10-02 08:25:37

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCH 4/4] shared/gatt: Remove not needed function parameter

According to SPEC 4.1 4.5.1 "Find Included Services" there is no need
to pass UUID to discover_included_services() function.
---
src/shared/gatt-helpers.c | 1 -
src/shared/gatt-helpers.h | 1 -
2 files changed, 2 deletions(-)

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index ee96011..acb8e39 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -972,7 +972,6 @@ done:

bool bt_gatt_discover_included_services(struct bt_att *att,
uint16_t start, uint16_t end,
- bt_uuid_t *uuid,
bt_gatt_discovery_callback_t callback,
void *user_data,
bt_gatt_destroy_func_t destroy)
diff --git a/src/shared/gatt-helpers.h b/src/shared/gatt-helpers.h
index da409fa..abd218c 100644
--- a/src/shared/gatt-helpers.h
+++ b/src/shared/gatt-helpers.h
@@ -77,7 +77,6 @@ bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
bt_gatt_destroy_func_t destroy);
bool bt_gatt_discover_included_services(struct bt_att *att,
uint16_t start, uint16_t end,
- bt_uuid_t *uuid,
bt_gatt_discovery_callback_t callback,
void *user_data,
bt_gatt_destroy_func_t destroy);
--
1.9.3