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

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

[Marcin Kraglak]

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

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

[Marcin Kraglak]

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

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

[Marcin Kraglak]

---
 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

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

[Marcin Kraglak]

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

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

[Arman Uguray]

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

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

[Marcin Kraglak]

Hi Arman,

On 3 October 2014 05:35, Arman Uguray <armansito@chromium.org> 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

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

[Arman Uguray]

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