[PATCH 00/15] android/gatt: Gatt client improvements

[PATCH 00/15] android/gatt: Gatt client improvements

[Lukasz Rymanowski]

This patch set adds:
* caching primary and included sevices.
* fix in hanling included services
* add list of disconnected devices - thanks to this we keep 
cached services and next connect is faster.
* Fix  instance id for primary and included services

Lukasz Rymanowski (15):
  android/gatt: Change name of gatt_primary in service structure
  android/gatt: Add flag primary to struct service
  android/gatt: Add helper functions to send all primary services
  android/gatt: Use cached primary services if possible
  android/gatt: Add create_device helper function
  android/gatt: Add list of disconnected device
  android/gatt: Minor change in get_include_data
  android/gatt: Add msg size check for get included service
  android/gatt: Prepare to cache included services
  android/gatt: Add helper function to create_service
  android/gatt: Cache include services
  android/gatt: Add helper to send included service
  android/gatt: Change label name from failed to reply
  android/gatt: Use cache for included services
  android/gatt: Fix instance id for primary and included service

 android/gatt.c | 509 +++++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 346 insertions(+), 163 deletions(-)

-- 
1.8.4

[PATCH 01/15] android/gatt: Change name of gatt_primary in service structure

[Lukasz Rymanowski]

---
 android/gatt.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 4f0a656..d7228a3 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -84,7 +84,7 @@ struct characteristic {
 
 struct service {
 	struct element_id id;
-	struct gatt_primary primary;
+	struct gatt_primary prim;
 
 	struct queue *chars;
 };
@@ -525,7 +525,7 @@ static void primary_cb(uint8_t status, GSList *services, void *user_data)
 		}
 
 		/* Put primary service to our local list */
-		memcpy(&p->primary, prim, sizeof(p->primary));
+		memcpy(&p->prim, prim, sizeof(p->prim));
 		if (!queue_push_tail(dev->services, p)) {
 			error("gatt: Cannot push primary service to the list");
 			free(p);
@@ -1137,7 +1137,7 @@ static bool match_service_by_uuid(const void *data, const void *user_data)
 	const bt_uuid_t *uuid = user_data;
 	bt_uuid_t service_uuid;
 
-	if (bt_string_to_uuid(&service_uuid, service->primary.uuid) < 0)
+	if (bt_string_to_uuid(&service_uuid, service->prim.uuid) < 0)
 		return false;
 
 	return !bt_uuid_cmp(uuid, &service_uuid);
@@ -1171,7 +1171,7 @@ static void get_included_cb(uint8_t status, GSList *included, void *user_data)
 		return;
 	}
 
-	bt_string_to_uuid(&uuid, service->primary.uuid);
+	bt_string_to_uuid(&uuid, service->prim.uuid);
 
 	/* TODO store included services in device->services list */
 	for (; included; included = included->next) {
@@ -1249,8 +1249,8 @@ static void handle_client_get_included_service(const void *buf, uint16_t len)
 	data->service = service;
 	data->device = device;
 
-	gatt_find_included(device->attrib, service->primary.range.start,
-				service->primary.range.end, get_included_cb,
+	gatt_find_included(device->attrib, service->prim.range.start,
+				service->prim.range.end, get_included_cb,
 				data);
 
 	status = HAL_STATUS_SUCCESS;
@@ -1408,8 +1408,8 @@ static void handle_client_get_characteristic(const void *buf, uint16_t len)
 		cb_data->service = srvc;
 		cb_data->conn_id = dev->conn_id;
 
-		if (!gatt_discover_char(dev->attrib, srvc->primary.range.start,
-					srvc->primary.range.end, NULL,
+		if (!gatt_discover_char(dev->attrib, srvc->prim.range.start,
+					srvc->prim.range.end, NULL,
 					discover_char_cb, cb_data)) {
 			free(cb_data);
 
@@ -1547,7 +1547,7 @@ static bool build_descr_cache(int32_t conn_id, struct gatt_device *dev,
 
 	/* Clip range to given characteristic */
 	start = ch->ch.value_handle + 1;
-	end = srvc->primary.range.end;
+	end = srvc->prim.range.end;
 
 	/* Use next characteristic start as end. If there is none -
 	 * service end is valid end.
-- 
1.8.4

[PATCH 02/15] android/gatt: Add flag primary to struct service

[Lukasz Rymanowski]

Since we are going to use this struct also for included services lets
add here flag saying if this is primary service or not.
---
 android/gatt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/android/gatt.c b/android/gatt.c
index d7228a3..8b0833c 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -85,6 +85,7 @@ struct characteristic {
 struct service {
 	struct element_id id;
 	struct gatt_primary prim;
+	bool primary;
 
 	struct queue *chars;
 };
@@ -526,6 +527,8 @@ static void primary_cb(uint8_t status, GSList *services, void *user_data)
 
 		/* Put primary service to our local list */
 		memcpy(&p->prim, prim, sizeof(p->prim));
+		p->primary = true;
+
 		if (!queue_push_tail(dev->services, p)) {
 			error("gatt: Cannot push primary service to the list");
 			free(p);
-- 
1.8.4

[PATCH 03/15] android/gatt: Add helper functions to send all primary services

[Lukasz Rymanowski]

This patch adds helper functions to send all the primary services.
This will be useful for following patches where we want to send to
client cached data.
---
 android/gatt.c | 64 +++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 45 insertions(+), 19 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 8b0833c..f4cf569 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -479,29 +479,67 @@ failed:
 					HAL_OP_GATT_CLIENT_UNREGISTER, status);
 }
 
-static void primary_cb(uint8_t status, GSList *services, void *user_data)
+static void send_client_primary_notify(void *data, void *user_data)
+{
+	struct hal_ev_gatt_client_search_result ev;
+	struct service *p = data;
+	int32_t conn_id = PTR_TO_INT(user_data);
+
+	/* In service queue we will have also included services */
+	if (!p->primary)
+		return;
+
+	ev.conn_id  = conn_id;
+	element_id_to_hal_srvc_id(&p->id, 1, &ev.srvc_id);
+
+	uuid2android(&p->id.uuid, ev.srvc_id.uuid);
+
+	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
+					HAL_EV_GATT_CLIENT_SEARCH_RESULT,
+					sizeof(ev), &ev);
+}
+
+static void send_client_all_primary(int32_t status, struct queue *services,
+							int32_t conn_id)
 {
 	struct hal_ev_gatt_client_search_complete ev;
+
+	if (status)
+		goto done;
+
+	queue_foreach(services, send_client_primary_notify,
+						INT_TO_PTR(conn_id));
+
+done:
+	ev.status = status;
+	ev.conn_id = conn_id;
+	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
+			HAL_EV_GATT_CLIENT_SEARCH_COMPLETE, sizeof(ev), &ev);
+
+}
+
+static void primary_cb(uint8_t status, GSList *services, void *user_data)
+{
 	struct gatt_device *dev = user_data;
 	GSList *l;
+	int32_t gatt_status;
 
 	DBG("Status %d", status);
 
 	if (status) {
 		error("gatt: Discover all primary services failed: %s",
 							att_ecode2str(status));
-		ev.status = GATT_FAILURE;
+		gatt_status = GATT_FAILURE;
 		goto done;
 	}
 
 	if (!services) {
 		info("gatt: No primary services found");
-		ev.status = GATT_SUCCESS;
+		gatt_status = GATT_SUCCESS;
 		goto done;
 	}
 
 	for (l = services; l; l = l->next) {
-		struct hal_ev_gatt_client_search_result ev_res;
 		struct gatt_primary *prim = l->data;
 		struct service *p;
 
@@ -518,8 +556,6 @@ static void primary_cb(uint8_t status, GSList *services, void *user_data)
 			continue;
 		}
 
-		memset(&ev_res, 0, sizeof(ev_res));
-
 		if (bt_string_to_uuid(&p->id.uuid, prim->uuid) < 0) {
 			error("gatt: Cannot convert string to uuid");
 			continue;
@@ -536,23 +572,13 @@ static void primary_cb(uint8_t status, GSList *services, void *user_data)
 		}
 
 		DBG("attr handle = 0x%04x, end grp handle = 0x%04x uuid: %s",
-				prim->range.start, prim->range.end, prim->uuid);
-
-		/* Set event data */
-		ev_res.conn_id  = dev->conn_id;
-		element_id_to_hal_srvc_id(&p->id, 1, &ev_res.srvc_id);
-
-		ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT ,
-					HAL_EV_GATT_CLIENT_SEARCH_RESULT,
-					sizeof(ev_res), &ev_res);
+			prim->range.start, prim->range.end, prim->uuid);
 	}
 
-	ev.status = GATT_SUCCESS;
+	gatt_status = GATT_SUCCESS;
 
 done:
-	ev.conn_id = dev->conn_id;
-	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
-			HAL_EV_GATT_CLIENT_SEARCH_COMPLETE, sizeof(ev), &ev);
+	send_client_all_primary(gatt_status, dev->services, dev->conn_id);
 }
 
 static void connection_cleanup(struct gatt_device *device)
-- 
1.8.4

[PATCH 04/15] android/gatt: Use cached primary services if possible

[Lukasz Rymanowski]

---
 android/gatt.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/android/gatt.c b/android/gatt.c
index f4cf569..eb99d54 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -1148,6 +1148,14 @@ static void handle_client_search_service(const void *buf, uint16_t len)
 
 	/*TODO:  Handle filter uuid */
 
+	/* Use cache if possible */
+	if (!queue_isempty(dev->services)) {
+		status = HAL_STATUS_SUCCESS;
+		send_client_all_primary(GATT_SUCCESS, dev->services,
+								dev->conn_id);
+		goto reply;
+	}
+
 	if (!gatt_discover_primary(dev->attrib, NULL, primary_cb, dev)) {
 		status = HAL_STATUS_FAILED;
 		goto reply;
-- 
1.8.4

[PATCH 05/15] android/gatt: Add create_device helper function

[Lukasz Rymanowski]

---
 android/gatt.c | 47 ++++++++++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 19 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index eb99d54..7e083e1 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -938,6 +938,33 @@ static struct gatt_device *find_device_by_conn_id(int32_t conn_id)
 	return queue_find(conn_list, match_dev_by_conn_id, INT_TO_PTR(conn_id));
 }
 
+static struct gatt_device *create_device(bdaddr_t *addr)
+{
+	struct gatt_device *dev;
+
+	dev = new0(struct gatt_device, 1);
+	if (!dev)
+		return NULL;
+
+	memcpy(&dev->bdaddr, addr, sizeof(bdaddr_t));
+
+	/* Create queue to keep list of clients for given device*/
+	dev->clients = queue_new();
+	if (!dev->clients) {
+		error("gatt: Cannot create client queue");
+		return NULL;
+	}
+
+	dev->services = queue_new();
+	if (!dev->services) {
+		error("gatt: Cannot create services queue");
+		queue_destroy(dev->clients, NULL);
+		return NULL;
+	}
+
+	return dev;
+}
+
 static void handle_client_connect(const void *buf, uint16_t len)
 {
 	const struct hal_cmd_gatt_client_connect *cmd = buf;
@@ -992,30 +1019,12 @@ static void handle_client_connect(const void *buf, uint16_t len)
 	/* Lets create new gatt device and put it on conn_wait_queue.
 	  * Once it is connected we move it to conn_list
 	  */
-	dev = new0(struct gatt_device, 1);
+	dev = create_device(&addr);
 	if (!dev) {
 		status = HAL_STATUS_FAILED;
 		goto reply;
 	}
 
-	memcpy(&dev->bdaddr, &addr, sizeof(bdaddr_t));
-
-	/* Create queue to keep list of clients for given device*/
-	dev->clients = queue_new();
-	if (!dev->clients) {
-		error("gatt: Cannot create client queue");
-		status = HAL_STATUS_FAILED;
-		goto reply;
-	}
-
-	dev->services = queue_new();
-	if (!dev->services) {
-		error("gatt: Cannot create services queue");
-		queue_destroy(dev->clients, NULL);
-		status = HAL_STATUS_FAILED;
-		goto reply;
-	}
-
 	/* Update client list of device */
 	if (!queue_push_tail(dev->clients, INT_TO_PTR(cmd->client_if))) {
 		error("gatt: Cannot push client on the client queue!?");
-- 
1.8.4

[PATCH 06/15] android/gatt: Add list of disconnected device

[Lukasz Rymanowski]

This patch add disconnected device list. It is in order to not lose
the cache if we disconnect from LE device.

Once device is disconnected we move it to disc_dev_list. When Client
wants to connect we first check if we know device, if so we use it.
---
 android/gatt.c | 38 +++++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 7e083e1..6259176 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -124,6 +124,7 @@ static struct queue *gatt_clients = NULL;
 static struct queue *gatt_servers = NULL;
 static struct queue *conn_list	= NULL;		/* Connected devices */
 static struct queue *conn_wait_queue = NULL;	/* Devs waiting to connect */
+static struct queue *disc_dev_list = NULL;	/* Disconnected devices */
 
 static void bt_le_discovery_stop_cb(void);
 
@@ -679,6 +680,13 @@ connect:
 	bt_le_discovery_stop(bt_le_discovery_stop_cb);
 }
 
+static void put_device_on_disc_list(struct gatt_device *dev)
+{
+	dev->conn_id = 0;
+	queue_remove_all(dev->clients, NULL, NULL, NULL);
+	queue_push_tail(disc_dev_list, dev);
+}
+
 static gboolean disconnected_cb(GIOChannel *io, GIOCondition cond,
 							gpointer user_data)
 {
@@ -713,7 +721,9 @@ done:
 	connection_cleanup(dev);
 
 	queue_foreach(dev->clients, client_disconnect_notify, dev);
-	destroy_device(dev);
+
+	/* Reset conn_id and put on disconnected list.*/
+	put_device_on_disc_list(dev);
 
 	return FALSE;
 }
@@ -1016,13 +1026,15 @@ static void handle_client_connect(const void *buf, uint16_t len)
 		goto reply;
 	}
 
-	/* Lets create new gatt device and put it on conn_wait_queue.
-	  * Once it is connected we move it to conn_list
-	  */
-	dev = create_device(&addr);
+	/* Let's check if we know device already */
+	dev = queue_remove_if(disc_dev_list, match_dev_by_bdaddr, &addr);
 	if (!dev) {
-		status = HAL_STATUS_FAILED;
-		goto reply;
+		/* New device, create it. */
+		dev = create_device(&addr);
+		if (!dev) {
+			status = HAL_STATUS_FAILED;
+			goto reply;
+		}
 	}
 
 	/* Update client list of device */
@@ -1121,7 +1133,7 @@ reply:
 	/* If this is last client do more cleaning */
 	connection_cleanup(dev);
 	dev = queue_remove_if(conn_list, match_dev_by_bdaddr, &dev->bdaddr);
-	destroy_device(dev);
+	put_device_on_disc_list(dev);
 }
 
 static void handle_client_listen(const void *buf, uint16_t len)
@@ -2730,8 +2742,10 @@ bool bt_gatt_register(struct ipc *ipc, const bdaddr_t *addr)
 	conn_wait_queue = queue_new();
 	gatt_clients = queue_new();
 	gatt_servers = queue_new();
+	disc_dev_list = queue_new();
 
-	if (!conn_list || !conn_wait_queue || !gatt_clients || !gatt_servers) {
+	if (!conn_list || !conn_wait_queue || !gatt_clients || !gatt_servers ||
+							!disc_dev_list) {
 		error("gatt: Failed to allocate memory for queues");
 
 		queue_destroy(gatt_servers, NULL);
@@ -2746,6 +2760,9 @@ bool bt_gatt_register(struct ipc *ipc, const bdaddr_t *addr)
 		queue_destroy(conn_wait_queue, NULL);
 		conn_wait_queue = NULL;
 
+		queue_destroy(disc_dev_list, NULL);
+		disc_dev_list = NULL;
+
 		return false;
 	}
 
@@ -2777,4 +2794,7 @@ void bt_gatt_unregister(void)
 
 	queue_destroy(conn_wait_queue, destroy_device);
 	conn_wait_queue = NULL;
+
+	queue_destroy(disc_dev_list, destroy_device);
+	disc_dev_list = NULL;
 }
-- 
1.8.4

[PATCH 07/15] android/gatt: Minor change in get_include_data

[Lukasz Rymanowski]

Change service to prim so it is more readable that this is primary
service
---
 android/gatt.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 6259176..6ac930d 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -1208,7 +1208,7 @@ static struct service *find_service_by_uuid(struct gatt_device *device,
 }
 
 struct get_included_data {
-	struct service *service;
+	struct service *prim;
 	struct gatt_device *device;
 };
 
@@ -1217,7 +1217,7 @@ static void get_included_cb(uint8_t status, GSList *included, void *user_data)
 	struct hal_ev_gatt_client_get_inc_service ev;
 	struct get_included_data *data = user_data;
 	struct gatt_device *device = data->device;
-	struct service *service = data->service;
+	struct service *service = data->prim;
 	bt_uuid_t uuid;
 
 	DBG("");
@@ -1304,7 +1304,7 @@ static void handle_client_get_included_service(const void *buf, uint16_t len)
 		goto failed;
 	}
 
-	data->service = service;
+	data->prim = service;
 	data->device = device;
 
 	gatt_find_included(device->attrib, service->prim.range.start,
-- 
1.8.4

[PATCH 08/15] android/gatt: Add msg size check for get included service

[Lukasz Rymanowski]

---
 android/gatt.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/android/gatt.c b/android/gatt.c
index 6ac930d..b95962d 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -1272,6 +1272,13 @@ static void handle_client_get_included_service(const void *buf, uint16_t len)
 
 	DBG("");
 
+	if (len != sizeof(*cmd) + (cmd->number * sizeof(cmd->srvc_id[0]))) {
+		error("Invalid get incl services size (%u bytes), terminating",
+									len);
+		raise(SIGTERM);
+		return;
+	}
+
 	device = find_device_by_conn_id(cmd->conn_id);
 	if (!device) {
 		status = HAL_STATUS_FAILED;
-- 
1.8.4

[PATCH 09/15] android/gatt: Prepare to cache included services

[Lukasz Rymanowski]

With this patch struct service is enhanced to handle caching include
services
---
 android/gatt.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index b95962d..f88c72a 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -85,6 +85,8 @@ struct characteristic {
 struct service {
 	struct element_id id;
 	struct gatt_primary prim;
+	struct gatt_included incl;
+
 	bool primary;
 
 	struct queue *chars;
@@ -1194,8 +1196,13 @@ static bool match_service_by_uuid(const void *data, const void *user_data)
 	const struct service *service = data;
 	const bt_uuid_t *uuid = user_data;
 	bt_uuid_t service_uuid;
+	int res;
 
-	if (bt_string_to_uuid(&service_uuid, service->prim.uuid) < 0)
+	if (service->primary)
+		res = bt_string_to_uuid(&service_uuid, service->prim.uuid);
+	else
+		res = bt_string_to_uuid(&service_uuid, service->incl.uuid);
+	if (res < 0)
 		return false;
 
 	return !bt_uuid_cmp(uuid, &service_uuid);
@@ -1461,6 +1468,8 @@ static void handle_client_get_characteristic(const void *buf, uint16_t len)
 
 	/* Discover all characteristics for services if not cached yet */
 	if (queue_isempty(srvc->chars)) {
+		struct att_range range;
+
 		struct discover_char_data *cb_data =
 					new0(struct discover_char_data, 1);
 
@@ -1473,9 +1482,11 @@ static void handle_client_get_characteristic(const void *buf, uint16_t len)
 		cb_data->service = srvc;
 		cb_data->conn_id = dev->conn_id;
 
-		if (!gatt_discover_char(dev->attrib, srvc->prim.range.start,
-					srvc->prim.range.end, NULL,
-					discover_char_cb, cb_data)) {
+		range = srvc->primary ? srvc->prim.range: srvc->incl.range;
+
+		if (!gatt_discover_char(dev->attrib, range.start, range.end,
+							NULL, discover_char_cb,
+							cb_data)) {
 			free(cb_data);
 
 			status = HAL_STATUS_FAILED;
-- 
1.8.4

[PATCH 10/15] android/gatt: Add helper function to create_service

[Lukasz Rymanowski]

This function will be useful when we start cache also included
services
---
 android/gatt.c | 56 ++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 36 insertions(+), 20 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index f88c72a..3500cd4 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -521,6 +521,40 @@ done:
 
 }
 
+static struct service *create_service(bool primary, char *uuid, void *data)
+{
+	struct service *s;
+
+	s = new0(struct service, 1);
+	if (!s) {
+		error("gatt: Cannot allocate memory for gatt_primary");
+		return NULL;
+	}
+
+	s->chars = queue_new();
+	if (!s->chars) {
+		error("gatt: Cannot allocate memory for char cache");
+		free(s);
+		return NULL;
+	}
+
+	if (bt_string_to_uuid(&s->id.uuid, uuid) < 0) {
+		error("gatt: Cannot convert string to uuid");
+		queue_destroy(s->chars, NULL);
+		free(s);
+		return NULL;
+	}
+
+	/* Put primary service to our local list */
+	s->primary = primary;
+	if (s->primary)
+		memcpy(&s->prim, data, sizeof(s->prim));
+	else
+		memcpy(&s->incl, data, sizeof(s->incl));
+
+	return s;
+}
+
 static void primary_cb(uint8_t status, GSList *services, void *user_data)
 {
 	struct gatt_device *dev = user_data;
@@ -546,27 +580,9 @@ static void primary_cb(uint8_t status, GSList *services, void *user_data)
 		struct gatt_primary *prim = l->data;
 		struct service *p;
 
-		p = new0(struct service, 1);
-		if (!p) {
-			error("gatt: Cannot allocate memory for gatt_primary");
-			continue;
-		}
-
-		p->chars = queue_new();
-		if (!p->chars) {
-			error("gatt: Cannot allocate memory for char cache");
-			free(p);
-			continue;
-		}
-
-		if (bt_string_to_uuid(&p->id.uuid, prim->uuid) < 0) {
-			error("gatt: Cannot convert string to uuid");
+		p = create_service(true, prim->uuid, prim);
+		if (!p)
 			continue;
-		}
-
-		/* Put primary service to our local list */
-		memcpy(&p->prim, prim, sizeof(p->prim));
-		p->primary = true;
 
 		if (!queue_push_tail(dev->services, p)) {
 			error("gatt: Cannot push primary service to the list");
-- 
1.8.4

[PATCH 11/15] android/gatt: Cache include services

[Lukasz Rymanowski]

This patch adds caching included services.
Note that included service in put on two queues.
1. On main services queue on the device, together with primary services.
2. On special queue inside the primary service.

We need 1) to have easy search of service when Android framework request
for characteristics etc.

We need 2) to handle iteration on included services done by Android
framework for a given primary service
---
 android/gatt.c | 39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/android/gatt.c b/android/gatt.c
index 3500cd4..06dbf8b 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -90,6 +90,7 @@ struct service {
 	bool primary;
 
 	struct queue *chars;
+	struct queue *included;	/* Valid only for primary services */
 };
 
 struct notification_data {
@@ -204,6 +205,16 @@ static void destroy_service(void *data)
 		return;
 
 	queue_destroy(srvc->chars, destroy_characteristic);
+
+	/* Included services we keep on two queues.
+	 * 1. On the same queue with primary services.
+	 * 2. On the queue inside primary service.
+	 * So we need to free service memory only once but we need to destroy
+	 * two queues
+	 */
+	if (srvc->primary)
+		queue_destroy(srvc->included, NULL);
+
 	free(srvc);
 }
 
@@ -552,6 +563,17 @@ static struct service *create_service(bool primary, char *uuid, void *data)
 	else
 		memcpy(&s->incl, data, sizeof(s->incl));
 
+	if (!s->primary)
+		goto done;
+
+	/* For primary service allocate queue for included services */
+	s->included = queue_new();
+	if (!s->included) {
+		queue_destroy(s->chars, NULL);
+		free(s);
+		return NULL;
+	}
+done:
 	return s;
 }
 
@@ -1254,11 +1276,26 @@ static void get_included_cb(uint8_t status, GSList *included, void *user_data)
 
 	bt_string_to_uuid(&uuid, service->prim.uuid);
 
-	/* TODO store included services in device->services list */
 	for (; included; included = included->next) {
 		struct gatt_included *included_service = included->data;
 		bt_uuid_t included_uuid;
+		struct service *incl;
+
+		incl = create_service(false, included_service->uuid,
+							included_service);
+		if (!incl)
+			continue;
 
+		/* Lets keep included service on two queues.
+		 * 1. on services queue together with primary service
+		 * 2. on special queue inside primary service
+		 */
+		if (!queue_push_tail(service->included, incl) ||
+				!queue_push_tail(device->services, incl)) {
+			error("gatt: Cannot push incl service to the list");
+			free(incl);
+			continue;
+		}
 		ev.conn_id = device->conn_id;
 		ev.status = GATT_SUCCESS;
 
-- 
1.8.4

[PATCH 12/15] android/gatt: Add helper to send included service

[Lukasz Rymanowski]

---
 android/gatt.c | 57 +++++++++++++++++++++++++++++++++------------------------
 1 file changed, 33 insertions(+), 24 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 06dbf8b..1b48730 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -1252,6 +1252,28 @@ static struct service *find_service_by_uuid(struct gatt_device *device,
 	return queue_find(device->services, match_service_by_uuid, uuid);
 }
 
+static void send_client_incl_service_notify(const struct service *prim,
+						const struct service *incl,
+						int32_t conn_id,
+						int32_t status)
+{
+	struct hal_ev_gatt_client_get_inc_service ev;
+
+	memset(&ev, 0, sizeof(ev));
+
+	ev.conn_id = conn_id;
+	ev.status = status;
+
+	element_id_to_hal_srvc_id(&prim->id, 1, &ev.srvc_id);
+
+	if (incl)
+		element_id_to_hal_srvc_id(&incl->id, 0, &ev.incl_srvc_id);
+
+	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT ,
+					HAL_EV_GATT_CLIENT_GET_INC_SERVICE,
+					sizeof(ev), &ev);
+}
+
 struct get_included_data {
 	struct service *prim;
 	struct gatt_device *device;
@@ -1259,11 +1281,11 @@ struct get_included_data {
 
 static void get_included_cb(uint8_t status, GSList *included, void *user_data)
 {
-	struct hal_ev_gatt_client_get_inc_service ev;
 	struct get_included_data *data = user_data;
 	struct gatt_device *device = data->device;
 	struct service *service = data->prim;
 	bt_uuid_t uuid;
+	struct service *incl;
 
 	DBG("");
 
@@ -1278,8 +1300,6 @@ static void get_included_cb(uint8_t status, GSList *included, void *user_data)
 
 	for (; included; included = included->next) {
 		struct gatt_included *included_service = included->data;
-		bt_uuid_t included_uuid;
-		struct service *incl;
 
 		incl = create_service(false, included_service->uuid,
 							included_service);
@@ -1296,30 +1316,19 @@ static void get_included_cb(uint8_t status, GSList *included, void *user_data)
 			free(incl);
 			continue;
 		}
-		ev.conn_id = device->conn_id;
-		ev.status = GATT_SUCCESS;
-
-		ev.srvc_id.inst_id = 0;
-		uuid2android(&uuid, ev.srvc_id.uuid);
-
-		ev.incl_srvc_id.inst_id = 0;
-		bt_string_to_uuid(&included_uuid, included_service->uuid);
-		uuid2android(&included_uuid, ev.incl_srvc_id.uuid);
-
-		ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT ,
-					HAL_EV_GATT_CLIENT_GET_INC_SERVICE,
-					sizeof(ev), &ev);
 	}
 
-	/* Android expects notification with error status in the end */
-	ev.conn_id = device->conn_id;
-	ev.status = GATT_FAILURE;
-	ev.srvc_id.inst_id = 0;
-	uuid2android(&uuid, ev.srvc_id.uuid);
+	/* Notify upper layer about first included service.
+	 * Android framework will iterate for next one.
+	 */
+	incl = queue_peek_head(service->included);
 
-	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT ,
-					HAL_EV_GATT_CLIENT_GET_INC_SERVICE,
-					sizeof(ev), &ev);
+	if (incl)
+		send_client_incl_service_notify(service,incl, device->conn_id,
+								GATT_SUCCESS);
+	else
+		send_client_incl_service_notify(service, NULL, device->conn_id,
+								GATT_FAILURE);
 }
 
 static void handle_client_get_included_service(const void *buf, uint16_t len)
-- 
1.8.4

[PATCH 13/15] android/gatt: Change label name from failed to reply

[Lukasz Rymanowski]

Next patch will use this label in success case therefore let's change
this label
---
 android/gatt.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 1b48730..58b228d 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -1351,12 +1351,12 @@ static void handle_client_get_included_service(const void *buf, uint16_t len)
 	device = find_device_by_conn_id(cmd->conn_id);
 	if (!device) {
 		status = HAL_STATUS_FAILED;
-		goto failed;
+		goto reply;
 	}
 
 	if (queue_isempty(device->services)) {
 		status = HAL_STATUS_FAILED;
-		goto failed;
+		goto reply;
 	}
 
 	if (!cmd->number) {
@@ -1370,14 +1370,14 @@ static void handle_client_get_included_service(const void *buf, uint16_t len)
 
 	if (!service) {
 		status = HAL_STATUS_FAILED;
-		goto failed;
+		goto reply;
 	}
 
 	data = new0(struct get_included_data, 1);
 	if (!data) {
 		error("gatt: failed to allocate memory for included_data");
 		status = HAL_STATUS_FAILED;
-		goto failed;
+		goto reply;
 	}
 
 	data->prim = service;
@@ -1389,7 +1389,7 @@ static void handle_client_get_included_service(const void *buf, uint16_t len)
 
 	status = HAL_STATUS_SUCCESS;
 
-failed:
+reply:
 	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT,
 					HAL_OP_GATT_CLIENT_GET_INCLUDED_SERVICE,
 					status);
-- 
1.8.4

[PATCH 14/15] android/gatt: Use cache for included services

[Lukasz Rymanowski]

We should used cached included services as ofter as possible

This patch do bit refactor of handling included service.
Removes some unsed helper functions used to match in queue.
Also moves up find_service function as it is need now higier in the
code.
---
 android/gatt.c | 175 +++++++++++++++++++++++++++++++--------------------------
 1 file changed, 94 insertions(+), 81 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 58b228d..d1deae4 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -91,6 +91,7 @@ struct service {
 
 	struct queue *chars;
 	struct queue *included;	/* Valid only for primary services */
+	bool incl_search_done;
 };
 
 struct notification_data {
@@ -294,6 +295,16 @@ static bool match_srvc_by_element_id(const void *data, const void *user_data)
 	return false;
 }
 
+static bool match_srvc_by_higher_inst_id(const void *data,
+							const void *user_data)
+{
+	const struct service *s = data;
+	uint8_t inst_id = PTR_TO_INT(user_data);
+
+	/* For now we match inst_id as it is unique */
+	return inst_id < s->id.instance;
+}
+
 static bool match_char_by_higher_inst_id(const void *data,
 							const void *user_data)
 {
@@ -1229,29 +1240,6 @@ reply:
 			HAL_OP_GATT_CLIENT_SEARCH_SERVICE, status);
 }
 
-static bool match_service_by_uuid(const void *data, const void *user_data)
-{
-	const struct service *service = data;
-	const bt_uuid_t *uuid = user_data;
-	bt_uuid_t service_uuid;
-	int res;
-
-	if (service->primary)
-		res = bt_string_to_uuid(&service_uuid, service->prim.uuid);
-	else
-		res = bt_string_to_uuid(&service_uuid, service->incl.uuid);
-	if (res < 0)
-		return false;
-
-	return !bt_uuid_cmp(uuid, &service_uuid);
-}
-
-static struct service *find_service_by_uuid(struct gatt_device *device,
-								bt_uuid_t *uuid)
-{
-	return queue_find(device->services, match_service_by_uuid, uuid);
-}
-
 static void send_client_incl_service_notify(const struct service *prim,
 						const struct service *incl,
 						int32_t conn_id,
@@ -1284,7 +1272,6 @@ static void get_included_cb(uint8_t status, GSList *included, void *user_data)
 	struct get_included_data *data = user_data;
 	struct gatt_device *device = data->device;
 	struct service *service = data->prim;
-	bt_uuid_t uuid;
 	struct service *incl;
 
 	DBG("");
@@ -1296,7 +1283,9 @@ static void get_included_cb(uint8_t status, GSList *included, void *user_data)
 		return;
 	}
 
-	bt_string_to_uuid(&uuid, service->prim.uuid);
+	/* Remember that we already search included services.*/
+	service->incl_search_done = true;
+
 
 	for (; included; included = included->next) {
 		struct gatt_included *included_service = included->data;
@@ -1331,12 +1320,59 @@ static void get_included_cb(uint8_t status, GSList *included, void *user_data)
 								GATT_FAILURE);
 }
 
-static void handle_client_get_included_service(const void *buf, uint16_t len)
+static bool search_included_services(struct gatt_device *dev,
+							struct service *prim)
 {
-	const struct hal_cmd_gatt_client_get_included_service *cmd = buf;
 	struct get_included_data *data;
+
+	data = new0(struct get_included_data, 1);
+	if (!data) {
+		error("gatt: failed to allocate memory for included_data");
+		return false;
+	}
+
+	data->prim = prim;
+	data->device = dev;
+
+	gatt_find_included(dev->attrib, prim->prim.range.start,
+							prim->prim.range.end,
+							get_included_cb, data);
+	return true;
+}
+
+static bool find_service(int32_t conn_id, struct element_id *service_id,
+				struct gatt_device **dev, struct service **srvc)
+{
 	struct gatt_device *device;
 	struct service *service;
+
+	device = find_device_by_conn_id(conn_id);
+	if (!device) {
+		error("gatt: conn_id=%d not found", conn_id);
+		return false;
+	}
+
+	service = queue_find(device->services, match_srvc_by_element_id,
+								service_id);
+	if (!service) {
+		error("gatt: Service with inst_id: %d not found",
+							service_id->instance);
+		return false;
+	}
+
+	*dev = device;
+	*srvc = service;
+
+	return true;
+}
+
+static void handle_client_get_included_service(const void *buf, uint16_t len)
+{
+	const struct hal_cmd_gatt_client_get_included_service *cmd = buf;
+	struct gatt_device *device;
+	struct service *prim_service;
+	struct service *incl_service;
+	struct element_id match_id;
 	uint8_t status;
 
 	DBG("");
@@ -1348,44 +1384,40 @@ static void handle_client_get_included_service(const void *buf, uint16_t len)
 		return;
 	}
 
-	device = find_device_by_conn_id(cmd->conn_id);
-	if (!device) {
-		status = HAL_STATUS_FAILED;
-		goto reply;
-	}
-
-	if (queue_isempty(device->services)) {
+	hal_srvc_id_to_element_id(&cmd->srvc_id[0], &match_id);
+	if (!find_service(cmd->conn_id, &match_id, &device, &prim_service)) {
 		status = HAL_STATUS_FAILED;
 		goto reply;
 	}
 
-	if (!cmd->number) {
-		service = queue_peek_head(device->services);
-	} else {
-		bt_uuid_t uuid;
-
-		android2uuid(cmd->srvc_id->uuid, &uuid);
-		service = find_service_by_uuid(device, &uuid);
-	}
+	if (!prim_service->incl_search_done) {
+		if (search_included_services(device, prim_service))
+			status = HAL_STATUS_SUCCESS;
+		else
+			status = HAL_STATUS_FAILED;
 
-	if (!service) {
-		status = HAL_STATUS_FAILED;
 		goto reply;
 	}
 
-	data = new0(struct get_included_data, 1);
-	if (!data) {
-		error("gatt: failed to allocate memory for included_data");
-		status = HAL_STATUS_FAILED;
-		goto reply;
+	/* Try to use cache here */
+	if (cmd->number == 1) {
+		incl_service = queue_peek_head(prim_service->included);
+	} else {
+		uint8_t inst_id = cmd->srvc_id[1].inst_id;
+		incl_service = queue_find(prim_service->included,
+						match_srvc_by_higher_inst_id,
+						INT_TO_PTR(inst_id));
 	}
 
-	data->prim = service;
-	data->device = device;
-
-	gatt_find_included(device->attrib, service->prim.range.start,
-				service->prim.range.end, get_included_cb,
-				data);
+	/* Note that Android framework expects failure notification
+	 * which is treat as the end of included services
+	 */
+	if (!incl_service)
+		send_client_incl_service_notify(prim_service, NULL,
+						device->conn_id, GATT_FAILURE);
+	else
+		send_client_incl_service_notify(prim_service, incl_service,
+						device->conn_id, GATT_SUCCESS);
 
 	status = HAL_STATUS_SUCCESS;
 
@@ -1393,6 +1425,13 @@ reply:
 	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT,
 					HAL_OP_GATT_CLIENT_GET_INCLUDED_SERVICE,
 					status);
+
+	/* In case of error in handling request we need to send event with
+	 * Android framework is stupid and do not check status of response
+	 */
+	if (status)
+		send_client_incl_service_notify(prim_service, NULL,
+						device->conn_id, GATT_FAILURE);
 }
 
 static void send_client_char_notify(const struct characteristic *ch,
@@ -1478,32 +1517,6 @@ static void discover_char_cb(uint8_t status, GSList *characteristics,
 	free(data);
 }
 
-static bool find_service(int32_t conn_id, struct element_id *service_id,
-				struct gatt_device **dev, struct service **srvc)
-{
-	struct gatt_device *device;
-	struct service *service;
-
-	device = find_device_by_conn_id(conn_id);
-	if (!device) {
-		error("gatt: conn_id=%d not found", conn_id);
-		return false;
-	}
-
-	service = queue_find(device->services, match_srvc_by_element_id,
-								service_id);
-	if (!service) {
-		error("gatt: Service with inst_id: %d not found",
-							service_id->instance);
-		return false;
-	}
-
-	*dev = device;
-	*srvc = service;
-
-	return true;
-}
-
 static void handle_client_get_characteristic(const void *buf, uint16_t len)
 {
 	const struct hal_cmd_gatt_client_get_characteristic *cmd = buf;
-- 
1.8.4

[PATCH 15/15] android/gatt: Fix instance id for primary and included service

[Lukasz Rymanowski]

Since there might be multiple services with same uuid, we should
make sure that each primary and included service has unique instance
id.

This patch adds this.
---
 android/gatt.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index d1deae4..7f15459 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -543,7 +543,8 @@ done:
 
 }
 
-static struct service *create_service(bool primary, char *uuid, void *data)
+static struct service *create_service(uint8_t id, bool primary, char *uuid,
+								void *data)
 {
 	struct service *s;
 
@@ -567,6 +568,8 @@ static struct service *create_service(bool primary, char *uuid, void *data)
 		return NULL;
 	}
 
+	s->id.instance = id;
+
 	/* Put primary service to our local list */
 	s->primary = primary;
 	if (s->primary)
@@ -593,6 +596,7 @@ static void primary_cb(uint8_t status, GSList *services, void *user_data)
 	struct gatt_device *dev = user_data;
 	GSList *l;
 	int32_t gatt_status;
+	uint8_t instance_id;
 
 	DBG("Status %d", status);
 
@@ -609,11 +613,16 @@ static void primary_cb(uint8_t status, GSList *services, void *user_data)
 		goto done;
 	}
 
+	/* There might be multiply services with same uuid. Therefore make sure
+	 * each primary service one has unique instance_id
+	 */
+	instance_id = 0;
+
 	for (l = services; l; l = l->next) {
 		struct gatt_primary *prim = l->data;
 		struct service *p;
 
-		p = create_service(true, prim->uuid, prim);
+		p = create_service(instance_id++, true, prim->uuid, prim);
 		if (!p)
 			continue;
 
@@ -1267,12 +1276,20 @@ struct get_included_data {
 	struct gatt_device *device;
 };
 
+static uint8_t get_inst_id_of_prim_services(const struct gatt_device *dev)
+{
+	struct service *s = queue_peek_tail(dev->services);
+
+	return s->id.instance;
+}
+
 static void get_included_cb(uint8_t status, GSList *included, void *user_data)
 {
 	struct get_included_data *data = user_data;
 	struct gatt_device *device = data->device;
 	struct service *service = data->prim;
 	struct service *incl;
+	uint8_t instance_id;
 
 	DBG("");
 
@@ -1286,11 +1303,18 @@ static void get_included_cb(uint8_t status, GSList *included, void *user_data)
 	/* Remember that we already search included services.*/
 	service->incl_search_done = true;
 
+	/* There might be multiply services with same uuid. Therefore make sure
+	 * each service has unique instance id. Let's take the latest instance
+	 * id of primary service and start iterate included services from this
+	 * point.
+	 */
+	instance_id = get_inst_id_of_prim_services(device);
 
 	for (; included; included = included->next) {
 		struct gatt_included *included_service = included->data;
 
-		incl = create_service(false, included_service->uuid,
+		incl = create_service(++instance_id, false,
+							included_service->uuid,
 							included_service);
 		if (!incl)
 			continue;
-- 
1.8.4

Re: [PATCH 00/15] android/gatt: Gatt client improvements

[Szymon Janc]

Hi Łukasz,

On Tuesday 08 April 2014 11:22:17 Lukasz Rymanowski wrote:
> This patch set adds:
> * caching primary and included sevices.
> * fix in hanling included services
> * add list of disconnected devices - thanks to this we keep
> cached services and next connect is faster.
> * Fix  instance id for primary and included services
> 
> Lukasz Rymanowski (15):
>   android/gatt: Change name of gatt_primary in service structure
>   android/gatt: Add flag primary to struct service
>   android/gatt: Add helper functions to send all primary services
>   android/gatt: Use cached primary services if possible
>   android/gatt: Add create_device helper function
>   android/gatt: Add list of disconnected device
>   android/gatt: Minor change in get_include_data
>   android/gatt: Add msg size check for get included service
>   android/gatt: Prepare to cache included services
>   android/gatt: Add helper function to create_service
>   android/gatt: Cache include services
>   android/gatt: Add helper to send included service
>   android/gatt: Change label name from failed to reply
>   android/gatt: Use cache for included services
>   android/gatt: Fix instance id for primary and included service
> 
>  android/gatt.c | 509
> +++++++++++++++++++++++++++++++++++++++------------------ 1 file changed,
> 346 insertions(+), 163 deletions(-)

All patches applied (including v2) with some changes discussed offline, 
thanks.

-- 
Szymon K. Janc
szymon.janc@gmail.com