[PATCH 1/6] android/gatt: Remove redundant code

[PATCH 1/6] android/gatt: Remove redundant code

[Andrzej Kaczmarek]

cache_all_srvc_chars is called only with empty queue so it's redundant
to handle this condition again inside function.
---
 android/gatt.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index e339789..59bcd23 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -1739,10 +1739,6 @@ static void cache_all_srvc_chars(GSList *characteristics, struct queue *q)
 	uint16_t inst_id = 0;
 	bt_uuid_t uuid;
 
-	/* Refresh characteristics cache if already exist */
-	if (!queue_isempty(q))
-		queue_remove_all(q, NULL, NULL, destroy_characteristic);
-
 	for (; characteristics; characteristics = characteristics->next) {
 		struct characteristic *ch;
 
-- 
1.9.2

[PATCH 2/6] android/gatt: Cache end handle for characteristics

[Andrzej Kaczmarek]

---
 android/gatt.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 59bcd23..67bb9b5 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -78,6 +78,7 @@ struct descriptor {
 struct characteristic {
 	struct element_id id;
 	struct gatt_char ch;
+	uint16_t end_handle;
 
 	struct queue *descriptors;
 };
@@ -1734,7 +1735,8 @@ static void send_client_char_notify(const struct characteristic *ch,
 					sizeof(ev), &ev);
 }
 
-static void cache_all_srvc_chars(GSList *characteristics, struct queue *q)
+static void cache_all_srvc_chars(struct service *srvc,
+							GSList *characteristics)
 {
 	uint16_t inst_id = 0;
 	bt_uuid_t uuid;
@@ -1765,7 +1767,16 @@ static void cache_all_srvc_chars(GSList *characteristics, struct queue *q)
 		 */
 		ch->id.instance = ++inst_id;
 
-		if (!queue_push_tail(q, ch)) {
+		/* Store end handle to use later for descriptors discovery */
+		if (characteristics->next) {
+			struct gatt_char *next = characteristics->next->data;
+			ch->end_handle = next->handle - 1;
+		} else {
+			ch->end_handle = srvc->primary ? srvc->prim.range.end :
+							srvc->incl.range.end;
+		}
+
+		if (!queue_push_tail(srvc->chars, ch)) {
 			error("gatt: Error while caching characteristic");
 			destroy_characteristic(ch);
 		}
@@ -1783,7 +1794,7 @@ static void discover_char_cb(uint8_t status, GSList *characteristics,
 	struct discover_char_data *data = user_data;
 
 	if (queue_isempty(data->service->chars))
-		cache_all_srvc_chars(characteristics, data->service->chars);
+		cache_all_srvc_chars(data->service, characteristics);
 
 	send_client_char_notify(queue_peek_head(data->service->chars),
 						data->conn_id, data->service);
-- 
1.9.2

[PATCH 3/6] android/gatt: Simplify discover_char_cb

[Andrzej Kaczmarek]

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

diff --git a/android/gatt.c b/android/gatt.c
index 67bb9b5..18f1a37 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -1792,12 +1792,13 @@ static void discover_char_cb(uint8_t status, GSList *characteristics,
 								void *user_data)
 {
 	struct discover_char_data *data = user_data;
+	struct service *srvc = data->service;
 
-	if (queue_isempty(data->service->chars))
-		cache_all_srvc_chars(data->service, characteristics);
+	if (queue_isempty(srvc->chars))
+		cache_all_srvc_chars(srvc, characteristics);
 
-	send_client_char_notify(queue_peek_head(data->service->chars),
-						data->conn_id, data->service);
+	send_client_char_notify(queue_peek_head(srvc->chars), data->conn_id,
+									srvc);
 
 	free(data);
 }
-- 
1.9.2

[PATCH 4/6] android/gatt: Use cached handle for descriptors discovery

[Andrzej Kaczmarek]

We can now just use cached end handle from characteristic instead of
calculating it.
---
 android/gatt.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 18f1a37..15107b9 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -1979,21 +1979,11 @@ static bool build_descr_cache(int32_t conn_id, struct gatt_device *dev,
 					struct characteristic *ch)
 {
 	struct discover_desc_data *cb_data;
-	struct characteristic *next_ch;
 	uint16_t start, end;
 
 	/* Clip range to given characteristic */
 	start = ch->ch.value_handle + 1;
-	end = srvc->primary ? srvc->prim.range.end : srvc->incl.range.end;
-
-	/* Use next characteristic start as end. If there is none -
-	 * service end is valid end.
-	 * TODO: we should cache char end handle to avoid this search
-	 */
-	next_ch = queue_find(srvc->chars, match_char_by_higher_inst_id,
-					INT_TO_PTR(ch->id.instance));
-	if (next_ch)
-		end = next_ch->ch.handle - 1;
+	end = ch->end_handle;
 
 	/* If there are no descriptors, notify with fail status. */
 	if (start > end)
-- 
1.9.2

[PATCH 5/6] android/gatt: Simplify descriptor discovery structs

[Andrzej Kaczmarek]

There's no need to pass/keep e.g. both gatt_device and conn_id since
we can retrieve one from another, so let's just use strutcures which
hold more data we can reuse later.
---
 android/gatt.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 15107b9..fbe16c2 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -1947,35 +1947,37 @@ static void cache_all_descr(const uint8_t *pdu, guint16 len,
 }
 
 struct discover_desc_data {
-	int32_t conn_id;
-	const struct element_id *srvc_id;
-	const struct characteristic *ch;
-	uint8_t primary;
+	struct gatt_device *dev;
+	struct service *srvc;
+	struct characteristic *ch;
 };
 
 static void gatt_discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
 							gpointer user_data)
 {
 	struct discover_desc_data *data = user_data;
+	struct gatt_device *dev = data->dev;
+	struct service *srvc = data->srvc;
+	struct characteristic *ch = data->ch;
 	struct descriptor *descr;
 
 	if (status)
 		error("gatt: Discover all char descriptors failed: %s",
 							att_ecode2str(status));
-	else if (queue_isempty(data->ch->descriptors))
-		cache_all_descr(pdu, len, data->ch->descriptors);
+	else if (queue_isempty(ch->descriptors))
+		cache_all_descr(pdu, len, ch->descriptors);
 
-	descr = queue_peek_head(data->ch->descriptors);
+	descr = queue_peek_head(ch->descriptors);
 
-	send_client_descr_notify(status, data->conn_id, data->primary,
-						data->srvc_id, &data->ch->id,
+	send_client_descr_notify(status, dev->conn_id, srvc->primary, &srvc->id,
+						&ch->id,
 						descr ? &descr->id : NULL);
 
 	free(data);
 }
 
-static bool build_descr_cache(int32_t conn_id, struct gatt_device *dev,
-					struct service *srvc, uint8_t primary,
+static bool build_descr_cache(struct gatt_device *dev,
+					struct service *srvc,
 					struct characteristic *ch)
 {
 	struct discover_desc_data *cb_data;
@@ -1993,10 +1995,9 @@ static bool build_descr_cache(int32_t conn_id, struct gatt_device *dev,
 	if (!cb_data)
 		return false;
 
-	cb_data->conn_id = conn_id;
-	cb_data->srvc_id = &srvc->id;
+	cb_data->dev = dev;
+	cb_data->srvc = srvc;
 	cb_data->ch = ch;
-	cb_data->primary = primary;
 
 	if (!gatt_discover_char_desc(dev->attrib, start, end,
 					gatt_discover_desc_cb, cb_data)) {
@@ -2053,7 +2054,7 @@ static void handle_client_get_descriptor(const void *buf, uint16_t len)
 	}
 
 	if (queue_isempty(ch->descriptors)) {
-		if (build_descr_cache(conn_id, dev, srvc, primary, ch)) {
+		if (build_descr_cache(dev, srvc, ch)) {
 			ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT,
 					HAL_OP_GATT_CLIENT_GET_DESCRIPTOR,
 					HAL_STATUS_SUCCESS);
-- 
1.9.2

[PATCH 6/6] android/gatt: Fix descriptors discovery

[Andrzej Kaczmarek]

gatt_discover_char_desc() may not return all of characteristic's
descriptors in single call since they may not fit into single
response (i.e. there are simply too many of them or there are both 16-
and 128-bit UUID descriptors which cannot be sent in single response).
We need to check if all descriptors were already returned by using
characteristic handles range and/or status error and request remaining
if possible.

Retrieved descriptors are stored on temporary queue which is later just
swapped with characteristic descriptors queue once finished. This it to
make update process atomic-like.
---
 android/gatt.c | 39 +++++++++++++++++++++++++++++++++------
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index fbe16c2..183fb02 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -1896,22 +1896,22 @@ static void send_client_descr_notify(int32_t status, int32_t conn_id,
 			HAL_EV_GATT_CLIENT_GET_DESCRIPTOR, sizeof(ev), &ev);
 }
 
-static void cache_all_descr(const uint8_t *pdu, guint16 len,
+static uint16_t parse_descrs(const uint8_t *pdu, guint16 len,
 							struct queue *cache)
 {
 	struct att_data_list *list;
 	guint8 format;
+	uint16_t handle = 0xffff;
 	int i;
 
 	list = dec_find_info_resp(pdu, len, &format);
 	if (!list)
-		return;
+		return handle;
 
 	for (i = 0; i < list->num; i++) {
 		char uuidstr[MAX_LEN_UUID_STR];
 		struct descriptor *descr;
 		bt_uuid_t uuid128;
-		uint16_t handle;
 		uint8_t *value;
 		bt_uuid_t uuid;
 
@@ -1944,12 +1944,16 @@ static void cache_all_descr(const uint8_t *pdu, guint16 len,
 	}
 
 	att_data_list_free(list);
+
+	return handle;
 }
 
 struct discover_desc_data {
 	struct gatt_device *dev;
 	struct service *srvc;
 	struct characteristic *ch;
+
+	struct queue *result;
 };
 
 static void gatt_discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
@@ -1961,11 +1965,27 @@ static void gatt_discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
 	struct characteristic *ch = data->ch;
 	struct descriptor *descr;
 
-	if (status)
+	if (!status) {
+		uint16_t last_handle = parse_descrs(pdu, len, data->result);
+
+		if (last_handle >= ch->end_handle)
+			goto reply;
+
+		if (!gatt_discover_char_desc(dev->attrib, last_handle + 1,
+						ch->end_handle,
+						gatt_discover_desc_cb, data))
+			goto reply;
+
+		/* Do not reply yet, wait for next piece */
+		return;
+	} else if (status != ATT_ECODE_ATTR_NOT_FOUND) {
 		error("gatt: Discover all char descriptors failed: %s",
 							att_ecode2str(status));
-	else if (queue_isempty(ch->descriptors))
-		cache_all_descr(pdu, len, ch->descriptors);
+	}
+
+reply:
+	queue_destroy(ch->descriptors, free);
+	ch->descriptors = data->result;
 
 	descr = queue_peek_head(ch->descriptors);
 
@@ -1998,9 +2018,16 @@ static bool build_descr_cache(struct gatt_device *dev,
 	cb_data->dev = dev;
 	cb_data->srvc = srvc;
 	cb_data->ch = ch;
+	cb_data->result = queue_new();
+
+	if (!cb_data->result) {
+		free(cb_data);
+		return false;
+	}
 
 	if (!gatt_discover_char_desc(dev->attrib, start, end,
 					gatt_discover_desc_cb, cb_data)) {
+		queue_destroy(cb_data->result, NULL);
 		free(cb_data);
 		return false;
 	}
-- 
1.9.2

Re: [PATCH 1/6] android/gatt: Remove redundant code

[Szymon Janc]

Hi Andrzej,

On Friday 18 of April 2014 02:40:19 Andrzej Kaczmarek wrote:
> cache_all_srvc_chars is called only with empty queue so it's redundant
> to handle this condition again inside function.
> ---
>  android/gatt.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/android/gatt.c b/android/gatt.c
> index e339789..59bcd23 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -1739,10 +1739,6 @@ static void cache_all_srvc_chars(GSList *characteristics, struct queue *q)
>  	uint16_t inst_id = 0;
>  	bt_uuid_t uuid;
>  
> -	/* Refresh characteristics cache if already exist */
> -	if (!queue_isempty(q))
> -		queue_remove_all(q, NULL, NULL, destroy_characteristic);
> -
>  	for (; characteristics; characteristics = characteristics->next) {
>  		struct characteristic *ch;
>  
> 

All patches applied, thanks.

-- 
Best regards, 
Szymon Janc