[PATCH 0/4] android/gatt: Read/Write improvements

[PATCH 0/4] android/gatt: Read/Write improvements

[Lukasz Rymanowski]

This set improves read/write operations from characteristics managed
by Android server application.


Lukasz Rymanowski (3):
  android/gatt: Improve handling pending read/write requests
  android/gatt: Fix minor style code issue
  android/gatt: Use pending request for write req and prep write

Marcin Kraglak (1):
  android/gatt: Use common framework for processing read_by_group_type
    request

 android/gatt.c | 139 ++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 79 insertions(+), 60 deletions(-)

-- 
1.8.4

[PATCH 1/4] android/gatt: Use common framework for processing read_by_group_type request

[Lukasz Rymanowski]

From: Marcin Kraglak <marcin.kraglak@tieto.com>

Use function process_dev_pending_request to read values of attributes.
---
 android/gatt.c | 19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 89da60d..46df0e1 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -4350,8 +4350,6 @@ static uint8_t read_by_group_type(const uint8_t *cmd, uint16_t cmd_len,
 
 	while (queue_peek_head(q)) {
 		uint16_t handle = PTR_TO_UINT(queue_pop_head(q));
-		uint8_t *value;
-		int value_len;
 		struct pending_request *entry;
 
 		entry = new0(struct pending_request, 1);
@@ -4361,19 +4359,7 @@ static uint8_t read_by_group_type(const uint8_t *cmd, uint16_t cmd_len,
 		}
 
 		entry->handle = handle;
-
-		if (!gatt_db_read(gatt_db, handle, 0, ATT_OP_READ_BY_GROUP_REQ,
-					&device->bdaddr, &value, &value_len))
-			break;
-
-		entry->value = malloc0(value_len);
-		if (!entry->value) {
-			queue_destroy(q, destroy_pending_request);
-			return ATT_ECODE_UNLIKELY;
-		}
-
-		memcpy(entry->value, value, value_len);
-		entry->length = value_len;
+		entry->length = READ_INIT;
 
 		if (!queue_push_tail(device->pending_requests, entry)) {
 			queue_remove_all(device->pending_requests, NULL, NULL,
@@ -4384,8 +4370,7 @@ static uint8_t read_by_group_type(const uint8_t *cmd, uint16_t cmd_len,
 	}
 
 	queue_destroy(q, NULL);
-
-	send_dev_pending_response(device, cmd[0]);
+	process_dev_pending_requests(device, cmd[0]);
 
 	return 0;
 }
-- 
1.8.4

[PATCH 2/4] android/gatt: Improve handling pending read/write requests

[Lukasz Rymanowski]

This patch adds state and error to pending request. It is not easy to
follow the code and we make sure that any error code provided by Android
server application will be send out to remote device.
---
 android/gatt.c | 94 +++++++++++++++++++++++++++++++---------------------------
 1 file changed, 51 insertions(+), 43 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 46df0e1..51fccd9 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -756,9 +756,11 @@ static void disconnect_notify_by_device(void *data, void *user_data)
 		send_app_connect_notify(conn, GATT_FAILURE);
 }
 
-#define READ_INIT -3
-#define READ_PENDING -2
-#define READ_FAILED -1
+enum pend_req_state {
+	REQUEST_INIT,
+	REQUEST_PENDING,
+	REQUEST_DONE,
+};
 
 struct pending_request {
 	uint16_t handle;
@@ -768,6 +770,9 @@ struct pending_request {
 
 	uint8_t *filter_value;
 	uint16_t filter_vlen;
+
+	enum pend_req_state state;
+	uint8_t error;
 };
 
 static void destroy_pending_request(void *data)
@@ -3550,8 +3555,8 @@ static void send_dev_pending_response(struct gatt_device *device,
 	}
 	case ATT_OP_READ_BLOB_REQ:
 		val = queue_pop_head(device->pending_requests);
-		if (!val || val->length < 0) {
-			error = ATT_ECODE_ATTR_NOT_FOUND;
+		if (val->error) {
+			error = val->error;
 			goto done;
 		}
 
@@ -3561,8 +3566,8 @@ static void send_dev_pending_response(struct gatt_device *device,
 		break;
 	case ATT_OP_READ_REQ:
 		val = queue_pop_head(device->pending_requests);
-		if (!val || val->length < 0) {
-			error = ATT_ECODE_ATTR_NOT_FOUND;
+		if (val->error) {
+			error = val->error;
 			goto done;
 		}
 
@@ -3670,9 +3675,9 @@ static void send_dev_pending_response(struct gatt_device *device,
 	}
 	case ATT_OP_EXEC_WRITE_REQ:
 		val = queue_pop_head(device->pending_requests);
-		if (!val) {
-			error = ATT_ECODE_ATTR_NOT_FOUND;
-			break;
+		if (val->error) {
+			error = val->error;
+			goto done;
 		}
 
 		len = enc_exec_write_resp(rsp);
@@ -3680,9 +3685,9 @@ static void send_dev_pending_response(struct gatt_device *device,
 		break;
 	case ATT_OP_WRITE_REQ:
 		val = queue_pop_head(device->pending_requests);
-		if (!val) {
-			error = ATT_ECODE_ATTR_NOT_FOUND;
-			break;
+		if (val->error) {
+			error = val->error;
+			goto done;
 		}
 
 		len = enc_write_resp(rsp);
@@ -3690,9 +3695,9 @@ static void send_dev_pending_response(struct gatt_device *device,
 		break;
 	case ATT_OP_PREP_WRITE_REQ:
 		val = queue_pop_head(device->pending_requests);
-		if (!val) {
-			error = ATT_ECODE_ATTR_NOT_FOUND;
-			break;
+		if (val->error) {
+			error = val->error;
+			goto done;
 		}
 
 		len = enc_prep_write_resp(val->handle, val->offset, val->value,
@@ -3723,7 +3728,7 @@ static bool match_pending_dev_request(const void *data, const void *user_data)
 {
 	const struct pending_request *pending_request = data;
 
-	return pending_request->length == READ_PENDING;
+	return pending_request->state == REQUEST_PENDING;
 }
 
 static bool match_dev_request_by_handle(const void *data, const void *user_data)
@@ -3746,7 +3751,8 @@ static void read_requested_attributes(void *data, void *user_data)
 						process_data->opcode,
 						&process_data->device->bdaddr,
 						&value, &value_len)) {
-		resp_data->length = READ_FAILED;
+		resp_data->state = REQUEST_DONE;
+		resp_data->error = ATT_ECODE_UNLIKELY;
 		return;
 	}
 
@@ -3755,14 +3761,15 @@ static void read_requested_attributes(void *data, void *user_data)
 		resp_data->value = malloc0(value_len);
 		if (!resp_data->value) {
 			/* If data cannot be copied, act like when read fails */
-			resp_data->length = READ_FAILED;
+			resp_data->state = REQUEST_DONE;
+			resp_data->error = ATT_ECODE_INSUFF_RESOURCES;
 			return;
 		}
 
 		memcpy(resp_data->value, value, value_len);
 		resp_data->length = value_len;
-	} else if (resp_data->length == READ_INIT) {
-		resp_data->length = READ_PENDING;
+	} else if (resp_data->state == REQUEST_INIT) {
+		resp_data->state = REQUEST_PENDING;
 	}
 }
 
@@ -3797,9 +3804,6 @@ static void send_gatt_response(uint8_t opcode, uint16_t handle,
 		goto done;
 	}
 
-	if (status)
-		goto done;
-
 	entry = queue_find(dev->pending_requests, match_dev_request_by_handle,
 							UINT_TO_PTR(handle));
 	if (!entry) {
@@ -3810,15 +3814,15 @@ static void send_gatt_response(uint8_t opcode, uint16_t handle,
 	entry->handle = handle;
 	entry->offset = offset;
 	entry->length = len;
+	entry->state = REQUEST_DONE;
+	entry->error = status;
 
 	if (!len)
 		goto done;
 
 	entry->value = malloc0(len);
 	if (!entry->value) {
-		/* send_dev_pending_request on empty queue sends error resp. */
-		queue_remove(dev->pending_requests, entry);
-		destroy_pending_request(entry);
+		entry->error = ATT_ECODE_INSUFF_RESOURCES;
 
 		goto done;
 	}
@@ -4359,7 +4363,7 @@ static uint8_t read_by_group_type(const uint8_t *cmd, uint16_t cmd_len,
 		}
 
 		entry->handle = handle;
-		entry->length = READ_INIT;
+		entry->state = REQUEST_INIT;
 
 		if (!queue_push_tail(device->pending_requests, entry)) {
 			queue_remove_all(device->pending_requests, NULL, NULL,
@@ -4410,7 +4414,7 @@ static uint8_t read_by_type(const uint8_t *cmd, uint16_t cmd_len,
 			return ATT_ECODE_INSUFF_RESOURCES;
 		}
 
-		data->length = READ_INIT;
+		data->state = REQUEST_INIT;
 		data->handle = handle;
 		queue_push_tail(device->pending_requests, data);
 	}
@@ -4455,7 +4459,7 @@ static uint8_t read_request(const uint8_t *cmd, uint16_t cmd_len,
 
 	data->offset = offset;
 	data->handle = handle;
-	data->length = READ_INIT;
+	data->state = REQUEST_INIT;
 	if (!queue_push_tail(dev->pending_requests, data)) {
 		free(data);
 		return ATT_ECODE_INSUFF_RESOURCES;
@@ -4619,7 +4623,7 @@ static uint8_t find_by_type_request(const uint8_t *cmd, uint16_t cmd_len,
 			return ATT_ECODE_INSUFF_RESOURCES;
 		}
 
-		data->length = READ_INIT;
+		data->state = REQUEST_INIT;
 		data->handle = handle;
 		data->filter_vlen = search_vlen;
 		memcpy(data->filter_value, search_value, search_vlen);
@@ -4872,9 +4876,8 @@ static void gap_read_cb(uint16_t handle, uint16_t offset, uint8_t att_opcode,
 
 		entry->value = malloc0(strlen(name));
 		if (!entry->value) {
-			queue_remove(dev->pending_requests, entry);
-			free(entry);
-			return;
+			entry->error = ATT_ECODE_INSUFF_RESOURCES;
+			goto done;
 		}
 
 		entry->length = strlen(name);
@@ -4882,9 +4885,8 @@ static void gap_read_cb(uint16_t handle, uint16_t offset, uint8_t att_opcode,
 	} else if (handle == gap_srvc_data.appear) {
 		entry->value = malloc0(2);
 		if (!entry->value) {
-			queue_remove(dev->pending_requests, entry);
-			free(entry);
-			return;
+			entry->error = ATT_ECODE_INSUFF_RESOURCES;
+			goto done;
 		}
 
 		put_le16(APPEARANCE_GENERIC_PHONE, entry->value);
@@ -4892,16 +4894,20 @@ static void gap_read_cb(uint16_t handle, uint16_t offset, uint8_t att_opcode,
 	} else if (handle == gap_srvc_data.priv) {
 		entry->value = malloc0(1);
 		if (!entry->value) {
-			queue_remove(dev->pending_requests, entry);
-			free(entry);
-			return;
+			entry->error = ATT_ECODE_INSUFF_RESOURCES;
+			goto done;
 		}
 
 		*entry->value = PERIPHERAL_PRIVACY_DISABLE;
 		entry->length = sizeof(uint8_t);
+	} else {
+		entry->error = ATT_ECODE_ATTR_NOT_FOUND;
 	}
 
 	entry->offset = offset;
+
+done:
+	entry->state  = REQUEST_DONE;
 }
 
 static void register_gap_service(void)
@@ -4982,14 +4988,16 @@ static void device_info_read_cb(uint16_t handle, uint16_t offset,
 
 	entry->value = malloc0(strlen(buf));
 	if (!entry->value) {
-		queue_remove(dev->pending_requests, entry);
-		free(entry);
-		return;
+		entry->error = ATT_ECODE_UNLIKELY;
+		goto done;
 	}
 
 	entry->length = strlen(buf);
 	memcpy(entry->value, buf, entry->length);
 	entry->offset = offset;
+
+done:
+	entry->state = REQUEST_DONE;
 }
 
 static void register_device_info_service(void)
-- 
1.8.4

[PATCH 3/4] android/gatt: Fix minor style code issue

[Lukasz Rymanowski]

---
 android/gatt.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/android/gatt.c b/android/gatt.c
index 51fccd9..39314a1 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -3513,7 +3513,6 @@ static void send_dev_pending_response(struct gatt_device *device,
 		int length;
 		struct queue *temp;
 
-
 		temp = queue_new();
 		if (!temp)
 			goto done;
-- 
1.8.4

[PATCH 4/4] android/gatt: Use pending request for write req and prep write

[Lukasz Rymanowski]

This patch add missing pending request for write operations.
Now Android server application can send response to remote device
---
 android/gatt.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/android/gatt.c b/android/gatt.c
index 39314a1..26b2300 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -4658,6 +4658,7 @@ static uint8_t write_req_request(const uint8_t *cmd, uint16_t cmd_len,
 						struct gatt_device *dev)
 {
 	uint8_t value[ATT_DEFAULT_LE_MTU];
+	struct pending_request *data;
 	uint16_t handle;
 	uint16_t len;
 	size_t vlen;
@@ -4666,6 +4667,18 @@ static uint8_t write_req_request(const uint8_t *cmd, uint16_t cmd_len,
 	if (!len)
 		return ATT_ECODE_INVALID_PDU;
 
+	data = new0(struct pending_request, 1);
+	if (!data)
+		return ATT_ECODE_INSUFF_RESOURCES;
+
+	data->handle = handle;
+	data->state = REQUEST_INIT;
+
+	if (!queue_push_tail(dev->pending_requests, data)) {
+		free(data);
+		return ATT_ECODE_INSUFF_RESOURCES;
+	}
+
 	if (!gatt_db_write(gatt_db, handle, 0, value, vlen, cmd[0],
 								&dev->bdaddr))
 		return ATT_ECODE_UNLIKELY;
@@ -4677,6 +4690,7 @@ static uint8_t write_prep_request(const uint8_t *cmd, uint16_t cmd_len,
 						struct gatt_device *dev)
 {
 	uint8_t value[ATT_DEFAULT_LE_MTU];
+	struct pending_request *data;
 	uint16_t handle;
 	uint16_t offset;
 	uint16_t len;
@@ -4687,6 +4701,19 @@ static uint8_t write_prep_request(const uint8_t *cmd, uint16_t cmd_len,
 	if (!len)
 		return ATT_ECODE_INVALID_PDU;
 
+	data = new0(struct pending_request, 1);
+	if (!data)
+		return ATT_ECODE_INSUFF_RESOURCES;
+
+	data->handle = handle;
+	data->offset = offset;
+	data->state = REQUEST_INIT;
+
+	if (!queue_push_tail(dev->pending_requests, data)) {
+		free(data);
+		return ATT_ECODE_INSUFF_RESOURCES;
+	}
+
 	if (!gatt_db_write(gatt_db, handle, offset, value, vlen, cmd[0],
 								&dev->bdaddr))
 		return ATT_ECODE_UNLIKELY;
-- 
1.8.4

Re: [PATCH 0/4] android/gatt: Read/Write improvements

[Szymon Janc]

Hi,

On Thursday 22 of May 2014 13:17:41 Lukasz Rymanowski wrote:
> This set improves read/write operations from characteristics managed
> by Android server application.
> 
> 
> Lukasz Rymanowski (3):
>   android/gatt: Improve handling pending read/write requests
>   android/gatt: Fix minor style code issue
>   android/gatt: Use pending request for write req and prep write
> 
> Marcin Kraglak (1):
>   android/gatt: Use common framework for processing read_by_group_type
>     request
> 
>  android/gatt.c | 139 ++++++++++++++++++++++++++++++++-------------------------
>  1 file changed, 79 insertions(+), 60 deletions(-)
> 

All patches applied, thanks.

-- 
Best regards, 
Szymon Janc