All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] Add included services discovery
@ 2014-10-07  7:19 Marcin Kraglak
  2014-10-07  7:19 ` [PATCH 1/6] shared/gatt: Distinguish Primary from Secondary services Marcin Kraglak
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Marcin Kraglak @ 2014-10-07  7:19 UTC (permalink / raw)
  To: linux-bluetooth

This patch set implements searching of included services.
It is based on previous patches to gatt-helper.
Additionally flag primary is introduced in bt_gatt_service_t
to know type of service. It will be needed in Android
gatt part too.
Included service discovery start right after primary discovery
finish. If include service declaration was found, and it was
not found in primary discovery, it's just simply added as
secondary service.
There is TODO part: we should check if included services
have other includes too, because nested includes are
allowed SPEC. We can consider recursive calls.

Comments are welcome
Marcin

Marcin Kraglak (6):
  shared/gatt: Distinguish Primary from Secondary services
  tools/btgatt-client: Print type of service
  shared/gatt: Discover included services
  shared/gatt: Add Secondary services to service_list
  shared/gatt: Add gatt-client include service iterator
  tools/btgatt-client: Print found include services

 src/shared/gatt-client.c | 203 +++++++++++++++++++++++++++++++++++++++++++++--
 src/shared/gatt-client.h |  18 +++++
 tools/btgatt-client.c    |  17 +++-
 3 files changed, 231 insertions(+), 7 deletions(-)

-- 
1.9.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/6] shared/gatt: Distinguish Primary from Secondary services
  2014-10-07  7:19 [PATCH 0/6] Add included services discovery Marcin Kraglak
@ 2014-10-07  7:19 ` Marcin Kraglak
  2014-10-07  7:19 ` [PATCH 2/6] tools/btgatt-client: Print type of service Marcin Kraglak
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Marcin Kraglak @ 2014-10-07  7:19 UTC (permalink / raw)
  To: linux-bluetooth

Add flag primary to distinguish service as primary or secondary.
---
 src/shared/gatt-client.c | 7 +++++--
 src/shared/gatt-client.h | 1 +
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 03d722f..724fde2 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -202,7 +202,8 @@ static void mark_notify_data_invalid_if_in_range(void *data, void *user_data)
 
 static bool service_list_add_service(struct service_list **head,
 						struct service_list **tail,
-						uint16_t start, uint16_t end,
+						bool primary, uint16_t start,
+						uint16_t end,
 						uint8_t uuid[BT_GATT_UUID_SIZE])
 {
 	struct service_list *list;
@@ -211,6 +212,7 @@ static bool service_list_add_service(struct service_list **head,
 	if (!list)
 		return false;
 
+	list->service.primary = primary;
 	list->service.start_handle = start;
 	list->service.end_handle = end;
 	memcpy(list->service.uuid, uuid, UUID_BYTES);
@@ -668,7 +670,8 @@ static void discover_primary_cb(bool success, uint8_t att_ecode,
 
 		/* Store the service */
 		if (!service_list_add_service(&op->result_head,
-					&op->result_tail, start, end, uuid)) {
+					&op->result_tail, true, start, end,
+					uuid)) {
 			util_debug(client->debug_callback, client->debug_data,
 						"Failed to store service");
 			success = false;
diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
index 6807f6b..22d4dc0 100644
--- a/src/shared/gatt-client.h
+++ b/src/shared/gatt-client.h
@@ -75,6 +75,7 @@ bool bt_gatt_client_set_debug(struct bt_gatt_client *client,
 					bt_gatt_client_destroy_func_t destroy);
 
 typedef struct {
+	bool primary;
 	uint16_t start_handle;
 	uint16_t end_handle;
 	uint8_t uuid[BT_GATT_UUID_SIZE];
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/6] tools/btgatt-client: Print type of service
  2014-10-07  7:19 [PATCH 0/6] Add included services discovery Marcin Kraglak
  2014-10-07  7:19 ` [PATCH 1/6] shared/gatt: Distinguish Primary from Secondary services Marcin Kraglak
@ 2014-10-07  7:19 ` Marcin Kraglak
  2014-10-07  7:19 ` [PATCH 3/6] shared/gatt: Discover included services Marcin Kraglak
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Marcin Kraglak @ 2014-10-07  7:19 UTC (permalink / raw)
  To: linux-bluetooth

---
 tools/btgatt-client.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
index d900e08..5c692ff 100644
--- a/tools/btgatt-client.c
+++ b/tools/btgatt-client.c
@@ -182,8 +182,9 @@ static void print_service(const bt_gatt_service_t *service)
 		return;
 	}
 
-	printf(COLOR_RED "service" COLOR_OFF " - start: 0x%04x, "
+	printf(COLOR_RED "service %s" COLOR_OFF " - start: 0x%04x, "
 				"end: 0x%04x, uuid: ",
+				service->primary ? "primary" : "second.",
 				service->start_handle, service->end_handle);
 	print_uuid(service->uuid);
 
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/6] shared/gatt: Discover included services
  2014-10-07  7:19 [PATCH 0/6] Add included services discovery Marcin Kraglak
  2014-10-07  7:19 ` [PATCH 1/6] shared/gatt: Distinguish Primary from Secondary services Marcin Kraglak
  2014-10-07  7:19 ` [PATCH 2/6] tools/btgatt-client: Print type of service Marcin Kraglak
@ 2014-10-07  7:19 ` Marcin Kraglak
  2014-10-07  7:19 ` [PATCH 4/6] shared/gatt: Add Secondary services to service_list Marcin Kraglak
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Marcin Kraglak @ 2014-10-07  7:19 UTC (permalink / raw)
  To: linux-bluetooth

---
 src/shared/gatt-client.c | 116 +++++++++++++++++++++++++++++++++++++++++++++--
 src/shared/gatt-client.h |   7 +++
 2 files changed, 119 insertions(+), 4 deletions(-)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 724fde2..16b1630 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -69,6 +69,8 @@ struct service_list {
 	bt_gatt_service_t service;
 	struct chrc_data *chrcs;
 	size_t num_chrcs;
+	bt_gatt_included_service_t *includes;
+	size_t num_includes;
 	struct service_list *next;
 };
 
@@ -240,6 +242,14 @@ static void service_destroy_characteristics(struct service_list *service)
 	free(service->chrcs);
 }
 
+static void service_destroy_includes(struct service_list *service)
+{
+	free(service->includes);
+
+	service->includes = NULL;
+	service->num_includes = 0;
+}
+
 static void service_list_clear(struct service_list **head,
 						struct service_list **tail)
 {
@@ -252,6 +262,7 @@ static void service_list_clear(struct service_list **head,
 
 	while (l) {
 		service_destroy_characteristics(l);
+		service_destroy_includes(l);
 		tmp = l;
 		l = tmp->next;
 		free(tmp);
@@ -280,6 +291,7 @@ static void service_list_clear_range(struct service_list **head,
 		}
 
 		service_destroy_characteristics(cur);
+		service_destroy_includes(cur);
 
 		if (!prev)
 			*head = cur->next;
@@ -413,6 +425,101 @@ static int uuid_cmp(const uint8_t uuid128[16], uint16_t uuid16)
 	return memcmp(uuid128, rhs_uuid, sizeof(rhs_uuid));
 }
 
+static void discover_incl_cb(bool success, uint8_t att_ecode,
+		struct bt_gatt_result *result,
+		void *user_data)
+{
+	struct discovery_op *op = user_data;
+	struct bt_gatt_client *client = op->client;
+	struct bt_gatt_iter iter;
+	char uuid_str[MAX_LEN_UUID_STR];
+	bt_gatt_included_service_t *includes;
+	unsigned int includes_count, i;
+
+	if (!success) {
+		if (att_ecode == BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND) {
+			success = true;
+			goto next;
+		}
+
+		goto done;
+	}
+
+	if (!result || !bt_gatt_iter_init(&iter, result)) {
+		success = false;
+		goto done;
+	}
+
+	includes_count = bt_gatt_result_included_count(result);
+	if (includes_count == 0) {
+		success = false;
+		goto done;
+	}
+
+	includes = new0(bt_gatt_included_service_t, includes_count);
+	if (!includes) {
+		success = false;
+		goto done;
+	}
+
+	util_debug(client->debug_callback, client->debug_data,
+						"Included services found: %u",
+						includes_count);
+
+	i = 0;
+	while (bt_gatt_iter_next_included_service(&iter, &includes[i].handle,
+						&includes[i].start_handle,
+						&includes[i].end_handle,
+						includes[i].uuid)) {
+		uuid_to_string(includes[i].uuid, uuid_str);
+		util_debug(client->debug_callback, client->debug_data,
+				"handle: 0x%04x, start: 0x%04x, end: 0x%04x,"
+				"uuid: %s", includes[i].handle,
+				includes[i].start_handle,
+				includes[i].end_handle, uuid_str);
+
+		/* TODO check if included service is on list */
+		i++;
+	}
+
+	op->cur_service->includes = includes;
+	op->cur_service->num_includes = includes_count;
+
+next:
+	if (!op->cur_service->next) {
+		op->cur_service = op->result_head;
+		if (bt_gatt_discover_characteristics(client->att,
+					op->cur_service->service.start_handle,
+					op->cur_service->service.end_handle,
+					discover_chrcs_cb,
+					discovery_op_ref(op),
+					discovery_op_unref))
+			return;
+
+		util_debug(client->debug_callback, client->debug_data,
+				"Failed to start characteristic discovery");
+		discovery_op_unref(op);
+		success = false;
+		goto done;
+	}
+
+	op->cur_service = op->cur_service->next;
+	if (bt_gatt_discover_included_services(client->att,
+				op->cur_service->service.start_handle,
+				op->cur_service->service.end_handle,
+				discover_incl_cb,
+				discovery_op_ref(op),
+				discovery_op_unref))
+		return;
+	util_debug(client->debug_callback, client->debug_data,
+					"Failed to start included discovery");
+	discovery_op_unref(op);
+	success = false;
+
+done:
+	op->complete_func(op, success, att_ecode);
+}
+
 static void discover_descs_cb(bool success, uint8_t att_ecode,
 						struct bt_gatt_result *result,
 						void *user_data)
@@ -517,6 +624,7 @@ done:
 	op->complete_func(op, success, att_ecode);
 }
 
+
 static void discover_chrcs_cb(bool success, uint8_t att_ecode,
 						struct bt_gatt_result *result,
 						void *user_data)
@@ -686,18 +794,18 @@ static void discover_primary_cb(bool success, uint8_t att_ecode,
 	if (!op->result_head)
 		goto done;
 
-	/* Sequentially discover the characteristics of all services */
+	/* Sequentially discover included services */
 	op->cur_service = op->result_head;
-	if (bt_gatt_discover_characteristics(client->att,
+	if (bt_gatt_discover_included_services(client->att,
 					op->cur_service->service.start_handle,
 					op->cur_service->service.end_handle,
-					discover_chrcs_cb,
+					discover_incl_cb,
 					discovery_op_ref(op),
 					discovery_op_unref))
 		return;
 
 	util_debug(client->debug_callback, client->debug_data,
-				"Failed to start characteristic discovery");
+				"Failed to start included services discovery");
 	discovery_op_unref(op);
 	success = false;
 
diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
index 22d4dc0..05b4838 100644
--- a/src/shared/gatt-client.h
+++ b/src/shared/gatt-client.h
@@ -96,6 +96,13 @@ typedef struct {
 	size_t num_descs;
 } bt_gatt_characteristic_t;
 
+typedef struct {
+	uint16_t handle;
+	uint16_t start_handle;
+	uint16_t end_handle;
+	uint8_t uuid[BT_GATT_UUID_SIZE];
+} bt_gatt_included_service_t;
+
 struct bt_gatt_service_iter {
 	struct bt_gatt_client *client;
 	void *ptr;
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/6] shared/gatt: Add Secondary services to service_list
  2014-10-07  7:19 [PATCH 0/6] Add included services discovery Marcin Kraglak
                   ` (2 preceding siblings ...)
  2014-10-07  7:19 ` [PATCH 3/6] shared/gatt: Discover included services Marcin Kraglak
@ 2014-10-07  7:19 ` Marcin Kraglak
  2014-10-07 23:04   ` Arman Uguray
  2014-10-07  7:19 ` [PATCH 5/6] shared/gatt: Add gatt-client include service iterator Marcin Kraglak
  2014-10-07  7:19 ` [PATCH 6/6] tools/btgatt-client: Print found include services Marcin Kraglak
  5 siblings, 1 reply; 10+ messages in thread
From: Marcin Kraglak @ 2014-10-07  7:19 UTC (permalink / raw)
  To: linux-bluetooth

If included service was found and doesn't exist on service list,
create Secondary Service and insert it.
---
 src/shared/gatt-client.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 16b1630..54e6fec 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -471,6 +471,7 @@ static void discover_incl_cb(bool success, uint8_t att_ecode,
 						&includes[i].start_handle,
 						&includes[i].end_handle,
 						includes[i].uuid)) {
+		struct service_list *service;
 		uuid_to_string(includes[i].uuid, uuid_str);
 		util_debug(client->debug_callback, client->debug_data,
 				"handle: 0x%04x, start: 0x%04x, end: 0x%04x,"
@@ -478,7 +479,56 @@ static void discover_incl_cb(bool success, uint8_t att_ecode,
 				includes[i].start_handle,
 				includes[i].end_handle, uuid_str);
 
-		/* TODO check if included service is on list */
+
+		/*
+		 * Check if included service was found previously in this
+		 * session
+		 */
+		for (service = op->result_head; service; service =
+								service->next) {
+			if (service->service.start_handle ==
+						includes[i].start_handle)
+				break;
+		}
+
+		/*
+		 * Check if included service was found in previous session
+		 */
+		if (!service) {
+			for (service = client->svc_head; service; service =
+								service->next) {
+				if (service->service.start_handle ==
+						includes[i].start_handle)
+					break;
+			}
+		}
+
+		if (!service) {
+			service = new0(struct service_list, 1);
+			if (!service) {
+				free(includes);
+				success = false;
+				goto done;
+			}
+
+			service->service.primary = false;
+			service->service.start_handle =
+						includes[i].start_handle;
+			service->service.end_handle = includes[i].end_handle;
+			memcpy(service->service.uuid, includes[i].uuid,
+								UUID_BYTES);
+
+			service_list_insert_services(&op->result_head,
+							&op->result_tail,
+							service, service);
+
+			/*
+			 * TODO: Newly created Secondary Service can contain
+			 * Included Services too. They should be discovered
+			 * before characteristic discovery.
+			 */
+		}
+
 		i++;
 	}
 
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/6] shared/gatt: Add gatt-client include service iterator
  2014-10-07  7:19 [PATCH 0/6] Add included services discovery Marcin Kraglak
                   ` (3 preceding siblings ...)
  2014-10-07  7:19 ` [PATCH 4/6] shared/gatt: Add Secondary services to service_list Marcin Kraglak
@ 2014-10-07  7:19 ` Marcin Kraglak
  2014-10-07 23:08   ` Arman Uguray
  2014-10-07  7:19 ` [PATCH 6/6] tools/btgatt-client: Print found include services Marcin Kraglak
  5 siblings, 1 reply; 10+ messages in thread
From: Marcin Kraglak @ 2014-10-07  7:19 UTC (permalink / raw)
  To: linux-bluetooth

---
 src/shared/gatt-client.c | 30 ++++++++++++++++++++++++++++++
 src/shared/gatt-client.h | 10 ++++++++++
 2 files changed, 40 insertions(+)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 54e6fec..b823c28 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -1635,6 +1635,36 @@ bool bt_gatt_characteristic_iter_next(struct bt_gatt_characteristic_iter *iter,
 	return true;
 }
 
+bool bt_gatt_include_service_iter_init(struct bt_gatt_include_service_iter *i,
+					const bt_gatt_service_t *service)
+{
+	if (!i || !service)
+		return false;
+
+	memset(i, 0, sizeof(*i));
+	i->service = (struct service_list *) service;
+
+	return true;
+}
+
+bool bt_gatt_include_service_iter_next(struct bt_gatt_include_service_iter *i,
+					const bt_gatt_included_service_t **incl)
+{
+	struct service_list *service;
+
+	if (!i || !incl)
+		return false;
+
+	service = i->service;
+
+	if (i->pos >= service->num_includes)
+		return false;
+
+	*incl = &service->includes[i->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 05b4838..df6515e 100644
--- a/src/shared/gatt-client.h
+++ b/src/shared/gatt-client.h
@@ -113,6 +113,11 @@ struct bt_gatt_characteristic_iter {
 	size_t pos;
 };
 
+struct bt_gatt_include_service_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,
@@ -129,6 +134,11 @@ bool bt_gatt_characteristic_iter_init(struct bt_gatt_characteristic_iter *iter,
 bool bt_gatt_characteristic_iter_next(struct bt_gatt_characteristic_iter *iter,
 					const bt_gatt_characteristic_t **chrc);
 
+bool bt_gatt_include_service_iter_init(struct bt_gatt_include_service_iter *i,
+					const bt_gatt_service_t *service);
+bool bt_gatt_include_service_iter_next(struct bt_gatt_include_service_iter *i,
+					const bt_gatt_included_service_t **inc);
+
 typedef void (*bt_gatt_client_read_callback_t)(bool success, uint8_t att_ecode,
 					const uint8_t *value, uint16_t length,
 					void *user_data);
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 6/6] tools/btgatt-client: Print found include services
  2014-10-07  7:19 [PATCH 0/6] Add included services discovery Marcin Kraglak
                   ` (4 preceding siblings ...)
  2014-10-07  7:19 ` [PATCH 5/6] shared/gatt: Add gatt-client include service iterator Marcin Kraglak
@ 2014-10-07  7:19 ` Marcin Kraglak
  5 siblings, 0 replies; 10+ messages in thread
From: Marcin Kraglak @ 2014-10-07  7:19 UTC (permalink / raw)
  To: linux-bluetooth

---
 tools/btgatt-client.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
index 5c692ff..a217819 100644
--- a/tools/btgatt-client.c
+++ b/tools/btgatt-client.c
@@ -174,7 +174,9 @@ 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;
+	struct bt_gatt_include_service_iter include_iter;
 	const bt_gatt_characteristic_t *chrc;
+	const bt_gatt_included_service_t *incl;
 	size_t i;
 
 	if (!bt_gatt_characteristic_iter_init(&iter, service)) {
@@ -182,12 +184,24 @@ static void print_service(const bt_gatt_service_t *service)
 		return;
 	}
 
+	if (!bt_gatt_include_service_iter_init(&include_iter, service)) {
+		PRLOG("Failed to initialize include service iterator\n");
+		return;
+	}
 	printf(COLOR_RED "service %s" COLOR_OFF " - start: 0x%04x, "
 				"end: 0x%04x, uuid: ",
 				service->primary ? "primary" : "second.",
 				service->start_handle, service->end_handle);
 	print_uuid(service->uuid);
 
+	while (bt_gatt_include_service_iter_next(&include_iter, &incl)) {
+		printf("\t  " COLOR_GREEN "include" COLOR_OFF " - handle: "
+					"0x%04x, - start: 0x%04x, end: 0x%04x,"
+					"uuid: ", incl->handle,
+					incl->start_handle, incl->end_handle);
+			print_uuid(incl->uuid);
+	}
+
 	while (bt_gatt_characteristic_iter_next(&iter, &chrc)) {
 		printf("\t  " COLOR_YELLOW "charac" COLOR_OFF
 				" - start: 0x%04x, end: 0x%04x, "
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/6] shared/gatt: Add Secondary services to service_list
  2014-10-07  7:19 ` [PATCH 4/6] shared/gatt: Add Secondary services to service_list Marcin Kraglak
@ 2014-10-07 23:04   ` Arman Uguray
  2014-10-08  5:30     ` Marcin Kraglak
  0 siblings, 1 reply; 10+ messages in thread
From: Arman Uguray @ 2014-10-07 23:04 UTC (permalink / raw)
  To: Marcin Kraglak; +Cc: BlueZ development

Hi Marcin,


> If included service was found and doesn't exist on service list,
> create Secondary Service and insert it.

I guess this is somewhat of an optimization but wouldn't it be more
robust to actually perform secondary service discovery? I mean after
discovering all the primary services, should we also send a bunch of
Read By Group Type requests with the secondary service UUID? You could
modify bt_gatt_discover_primary_services so that it accepts a boolean
parameter (perhaps rename it to bt_gatt_discover_services) and uses
the correct UUID based on that. Do you think that would be a better
approach?


> ---
>  src/shared/gatt-client.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
> index 16b1630..54e6fec 100644
> --- a/src/shared/gatt-client.c
> +++ b/src/shared/gatt-client.c
> @@ -471,6 +471,7 @@ static void discover_incl_cb(bool success, uint8_t att_ecode,
>                                                 &includes[i].start_handle,
>                                                 &includes[i].end_handle,
>                                                 includes[i].uuid)) {
> +               struct service_list *service;
>                 uuid_to_string(includes[i].uuid, uuid_str);
>                 util_debug(client->debug_callback, client->debug_data,
>                                 "handle: 0x%04x, start: 0x%04x, end: 0x%04x,"
> @@ -478,7 +479,56 @@ static void discover_incl_cb(bool success, uint8_t att_ecode,
>                                 includes[i].start_handle,
>                                 includes[i].end_handle, uuid_str);
>
> -               /* TODO check if included service is on list */
> +
> +               /*
> +                * Check if included service was found previously in this
> +                * session
> +                */
> +               for (service = op->result_head; service; service =
> +                                                               service->next) {
> +                       if (service->service.start_handle ==
> +                                               includes[i].start_handle)
> +                               break;
> +               }
> +
> +               /*
> +                * Check if included service was found in previous session
> +                */
> +               if (!service) {
> +                       for (service = client->svc_head; service; service =
> +                                                               service->next) {
> +                               if (service->service.start_handle ==
> +                                               includes[i].start_handle)
> +                                       break;
> +                       }
> +               }
> +
> +               if (!service) {
> +                       service = new0(struct service_list, 1);
> +                       if (!service) {
> +                               free(includes);
> +                               success = false;
> +                               goto done;
> +                       }
> +
> +                       service->service.primary = false;
> +                       service->service.start_handle =
> +                                               includes[i].start_handle;
> +                       service->service.end_handle = includes[i].end_handle;
> +                       memcpy(service->service.uuid, includes[i].uuid,
> +                                                               UUID_BYTES);
> +
> +                       service_list_insert_services(&op->result_head,
> +                                                       &op->result_tail,
> +                                                       service, service);
> +
> +                       /*
> +                        * TODO: Newly created Secondary Service can contain
> +                        * Included Services too. They should be discovered
> +                        * before characteristic discovery.
> +                        */
> +               }
> +
>                 i++;
>         }
>
> --
> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 5/6] shared/gatt: Add gatt-client include service iterator
  2014-10-07  7:19 ` [PATCH 5/6] shared/gatt: Add gatt-client include service iterator Marcin Kraglak
@ 2014-10-07 23:08   ` Arman Uguray
  0 siblings, 0 replies; 10+ messages in thread
From: Arman Uguray @ 2014-10-07 23:08 UTC (permalink / raw)
  To: Marcin Kraglak; +Cc: BlueZ development

Hi Marcin,


> @@ -1635,6 +1635,36 @@ bool bt_gatt_characteristic_iter_next(struct bt_gatt_characteristic_iter *iter,
>         return true;
>  }
>
> +bool bt_gatt_include_service_iter_init(struct bt_gatt_include_service_iter *i,
> +                                       const bt_gatt_service_t *service)

I would use "iter" instead of "i" to keep it consistent with the rest
of the code (here and elsewhere).

Thanks,
Arman

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/6] shared/gatt: Add Secondary services to service_list
  2014-10-07 23:04   ` Arman Uguray
@ 2014-10-08  5:30     ` Marcin Kraglak
  0 siblings, 0 replies; 10+ messages in thread
From: Marcin Kraglak @ 2014-10-08  5:30 UTC (permalink / raw)
  To: Arman Uguray; +Cc: BlueZ development

Hi Arman,

On 8 October 2014 01:04, Arman Uguray <armansito@chromium.org> wrote:
> Hi Marcin,
>
>
>> If included service was found and doesn't exist on service list,
>> create Secondary Service and insert it.
>
> I guess this is somewhat of an optimization but wouldn't it be more
> robust to actually perform secondary service discovery? I mean after
> discovering all the primary services, should we also send a bunch of
> Read By Group Type requests with the secondary service UUID? You could
> modify bt_gatt_discover_primary_services so that it accepts a boolean
> parameter (perhaps rename it to bt_gatt_discover_services) and uses
> the correct UUID based on that. Do you think that would be a better
> approach?

If we discover secondary first, then we won't have to recursive search
for include services.
I didn't implement this just because it wasn't listed in gatt feature
requirements in SPEC.
But I think that is good approach. I'll send patches

>
>
>> ---
>>  src/shared/gatt-client.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 51 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
>> index 16b1630..54e6fec 100644
>> --- a/src/shared/gatt-client.c
>> +++ b/src/shared/gatt-client.c
>> @@ -471,6 +471,7 @@ static void discover_incl_cb(bool success, uint8_t att_ecode,
>>                                                 &includes[i].start_handle,
>>                                                 &includes[i].end_handle,
>>                                                 includes[i].uuid)) {
>> +               struct service_list *service;
>>                 uuid_to_string(includes[i].uuid, uuid_str);
>>                 util_debug(client->debug_callback, client->debug_data,
>>                                 "handle: 0x%04x, start: 0x%04x, end: 0x%04x,"
>> @@ -478,7 +479,56 @@ static void discover_incl_cb(bool success, uint8_t att_ecode,
>>                                 includes[i].start_handle,
>>                                 includes[i].end_handle, uuid_str);
>>
>> -               /* TODO check if included service is on list */
>> +
>> +               /*
>> +                * Check if included service was found previously in this
>> +                * session
>> +                */
>> +               for (service = op->result_head; service; service =
>> +                                                               service->next) {
>> +                       if (service->service.start_handle ==
>> +                                               includes[i].start_handle)
>> +                               break;
>> +               }
>> +
>> +               /*
>> +                * Check if included service was found in previous session
>> +                */
>> +               if (!service) {
>> +                       for (service = client->svc_head; service; service =
>> +                                                               service->next) {
>> +                               if (service->service.start_handle ==
>> +                                               includes[i].start_handle)
>> +                                       break;
>> +                       }
>> +               }
>> +
>> +               if (!service) {
>> +                       service = new0(struct service_list, 1);
>> +                       if (!service) {
>> +                               free(includes);
>> +                               success = false;
>> +                               goto done;
>> +                       }
>> +
>> +                       service->service.primary = false;
>> +                       service->service.start_handle =
>> +                                               includes[i].start_handle;
>> +                       service->service.end_handle = includes[i].end_handle;
>> +                       memcpy(service->service.uuid, includes[i].uuid,
>> +                                                               UUID_BYTES);
>> +
>> +                       service_list_insert_services(&op->result_head,
>> +                                                       &op->result_tail,
>> +                                                       service, service);
>> +
>> +                       /*
>> +                        * TODO: Newly created Secondary Service can contain
>> +                        * Included Services too. They should be discovered
>> +                        * before characteristic discovery.
>> +                        */
>> +               }
>> +
>>                 i++;
>>         }
>>
>> --
>> 1.9.3
>>

BR
Marcin
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2014-10-08  5:30 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-07  7:19 [PATCH 0/6] Add included services discovery Marcin Kraglak
2014-10-07  7:19 ` [PATCH 1/6] shared/gatt: Distinguish Primary from Secondary services Marcin Kraglak
2014-10-07  7:19 ` [PATCH 2/6] tools/btgatt-client: Print type of service Marcin Kraglak
2014-10-07  7:19 ` [PATCH 3/6] shared/gatt: Discover included services Marcin Kraglak
2014-10-07  7:19 ` [PATCH 4/6] shared/gatt: Add Secondary services to service_list Marcin Kraglak
2014-10-07 23:04   ` Arman Uguray
2014-10-08  5:30     ` Marcin Kraglak
2014-10-07  7:19 ` [PATCH 5/6] shared/gatt: Add gatt-client include service iterator Marcin Kraglak
2014-10-07 23:08   ` Arman Uguray
2014-10-07  7:19 ` [PATCH 6/6] tools/btgatt-client: Print found include services Marcin Kraglak

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.