2014-05-16 11:00:39

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCH 1/4] android/gatt: Handle Androids read/write responses in response queue

This is needed for reads like "read by type" to support Android
responses, callback reads and direct db reads in a single request.
---
android/gatt.c | 88 ++++++++++++++++++++++++++++++++++++++--------------------
1 file changed, 58 insertions(+), 30 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 9bbcc48..20850af 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -3610,6 +3610,37 @@ static void send_dev_pending_response(struct gatt_device *device,

break;
}
+ case ATT_OP_EXEC_WRITE_REQ:
+ val = queue_pop_head(device->pending_requests);
+ if (!val) {
+ error = ATT_ECODE_ATTR_NOT_FOUND;
+ break;
+ }
+
+ len = enc_exec_write_resp(rsp);
+ destroy_pending_request(val);
+ break;
+ case ATT_OP_WRITE_REQ:
+ val = queue_pop_head(device->pending_requests);
+ if (!val) {
+ error = ATT_ECODE_ATTR_NOT_FOUND;
+ break;
+ }
+
+ len = enc_write_resp(rsp);
+ destroy_pending_request(val);
+ break;
+ case ATT_OP_PREP_WRITE_REQ:
+ val = queue_pop_head(device->pending_requests);
+ if (!val) {
+ error = ATT_ECODE_ATTR_NOT_FOUND;
+ break;
+ }
+
+ len = enc_prep_write_resp(val->handle, val->offset, val->value,
+ val->length, rsp, sizeof(rsp));
+ destroy_pending_request(val);
+ break;
default:
break;
}
@@ -3698,48 +3729,45 @@ static void send_gatt_response(uint8_t opcode, uint16_t handle,
bdaddr_t *bdaddr)
{
struct gatt_device *dev;
- uint16_t length;
- uint8_t pdu[ATT_DEFAULT_LE_MTU];
-
- DBG("");
+ struct pending_request *entry;

dev = find_device_by_addr(bdaddr);
if (!dev) {
error("gatt: send_gatt_response, could not find dev");
- return;
+ goto done;
}

- if (!status) {
- length = enc_error_resp(opcode, handle, status, pdu,
- sizeof(pdu));
+ if (status)
goto done;
- }

- switch (opcode) {
- case ATT_OP_EXEC_WRITE_REQ:
- length = enc_exec_write_resp(pdu);
- break;
- case ATT_OP_WRITE_REQ:
- length = enc_write_resp(pdu);
- break;
- case ATT_OP_PREP_WRITE_REQ:
- length = enc_prep_write_resp(handle, offset, data, len, pdu,
- sizeof(pdu));
- break;
- case ATT_OP_READ_BLOB_REQ:
- length = enc_read_blob_resp((uint8_t *)data, len, offset, pdu,
- sizeof(pdu));
- break;
- case ATT_OP_READ_REQ:
- length = enc_read_resp((uint8_t *)data, len, pdu, sizeof(pdu));
- break;
- default:
- error("gatt: Unexpected opcode");
+ entry = queue_find(dev->pending_requests, match_dev_request_by_handle,
+ UINT_TO_PTR(handle));
+ if (!entry) {
+ DBG("No pending response found! Bogus android response?");
return;
}

+ entry->handle = handle;
+ entry->offset = offset;
+ entry->length = len;
+
+ 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);
+
+ goto done;
+ }
+
+ memcpy(entry->value, data, len);
+
done:
- g_attrib_send(dev->attrib, 0, pdu, length, NULL, NULL, NULL);
+ if (!queue_find(dev->pending_requests, match_pending_dev_request, NULL))
+ send_dev_pending_response(dev, opcode);
}

static void set_trans_id(struct gatt_app *app, unsigned int id, int8_t opcode)
--
1.9.3



2014-05-18 12:01:56

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH 1/4] android/gatt: Handle Androids read/write responses in response queue

Hi Jakub,

On Friday 16 of May 2014 13:00:39 Jakub Tyszkowski wrote:
> This is needed for reads like "read by type" to support Android
> responses, callback reads and direct db reads in a single request.
> ---
> android/gatt.c | 88
> ++++++++++++++++++++++++++++++++++++++-------------------- 1 file changed,
> 58 insertions(+), 30 deletions(-)
>
> diff --git a/android/gatt.c b/android/gatt.c
> index 9bbcc48..20850af 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -3610,6 +3610,37 @@ static void send_dev_pending_response(struct
> gatt_device *device,
>
> break;
> }
> + case ATT_OP_EXEC_WRITE_REQ:
> + val = queue_pop_head(device->pending_requests);
> + if (!val) {
> + error = ATT_ECODE_ATTR_NOT_FOUND;
> + break;
> + }
> +
> + len = enc_exec_write_resp(rsp);
> + destroy_pending_request(val);
> + break;
> + case ATT_OP_WRITE_REQ:
> + val = queue_pop_head(device->pending_requests);
> + if (!val) {
> + error = ATT_ECODE_ATTR_NOT_FOUND;
> + break;
> + }
> +
> + len = enc_write_resp(rsp);
> + destroy_pending_request(val);
> + break;
> + case ATT_OP_PREP_WRITE_REQ:
> + val = queue_pop_head(device->pending_requests);
> + if (!val) {
> + error = ATT_ECODE_ATTR_NOT_FOUND;
> + break;
> + }
> +
> + len = enc_prep_write_resp(val->handle, val->offset, val->value,
> + val->length, rsp, sizeof(rsp));
> + destroy_pending_request(val);
> + break;
> default:
> break;
> }
> @@ -3698,48 +3729,45 @@ static void send_gatt_response(uint8_t opcode,
> uint16_t handle, bdaddr_t *bdaddr)
> {
> struct gatt_device *dev;
> - uint16_t length;
> - uint8_t pdu[ATT_DEFAULT_LE_MTU];
> -
> - DBG("");
> + struct pending_request *entry;
>
> dev = find_device_by_addr(bdaddr);
> if (!dev) {
> error("gatt: send_gatt_response, could not find dev");
> - return;
> + goto done;
> }
>
> - if (!status) {
> - length = enc_error_resp(opcode, handle, status, pdu,
> - sizeof(pdu));
> + if (status)
> goto done;
> - }
>
> - switch (opcode) {
> - case ATT_OP_EXEC_WRITE_REQ:
> - length = enc_exec_write_resp(pdu);
> - break;
> - case ATT_OP_WRITE_REQ:
> - length = enc_write_resp(pdu);
> - break;
> - case ATT_OP_PREP_WRITE_REQ:
> - length = enc_prep_write_resp(handle, offset, data, len, pdu,
> - sizeof(pdu));
> - break;
> - case ATT_OP_READ_BLOB_REQ:
> - length = enc_read_blob_resp((uint8_t *)data, len, offset, pdu,
> - sizeof(pdu));
> - break;
> - case ATT_OP_READ_REQ:
> - length = enc_read_resp((uint8_t *)data, len, pdu, sizeof(pdu));
> - break;
> - default:
> - error("gatt: Unexpected opcode");
> + entry = queue_find(dev->pending_requests, match_dev_request_by_handle,
> + UINT_TO_PTR(handle));
> + if (!entry) {
> + DBG("No pending response found! Bogus android response?");
> return;
> }
>
> + entry->handle = handle;
> + entry->offset = offset;
> + entry->length = len;
> +
> + 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);
> +
> + goto done;
> + }
> +
> + memcpy(entry->value, data, len);
> +
> done:
> - g_attrib_send(dev->attrib, 0, pdu, length, NULL, NULL, NULL);
> + if (!queue_find(dev->pending_requests, match_pending_dev_request, NULL))
> + send_dev_pending_response(dev, opcode);
> }
>
> static void set_trans_id(struct gatt_app *app, unsigned int id, int8_t
> opcode)

All patches are now applied, thanks.

--
BR
Szymon Janc

2014-05-16 11:00:42

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCH 4/4] android/hal: Fix not seting mode in health hal

This fixes the following issue:
==8505== Syscall param socketcall.sendmsg(msg.msg_iov[i]) points to
uninitialised byte(s)
==8505== at 0x534133D: ??? (syscall-template.S:82)
==8505== by 0x7756346: hal_ipc_cmd (hal-ipc.c:359)
==8505== by 0x7750EB2: init (hal-health.c:206)
==8505== by 0x40DF7F: init_p (if-hl.c:86)
==8505== by 0x401961: main (haltest.c:417)
==8505== Address 0x7fefffb81 is on thread 1's stack
---
android/hal-health.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/android/hal-health.c b/android/hal-health.c
index 344f55c..427d4c9 100644
--- a/android/hal-health.c
+++ b/android/hal-health.c
@@ -202,6 +202,7 @@ static bt_status_t init(bthl_callbacks_t *callbacks)
sizeof(ev_handlers)/sizeof(ev_handlers[0]));

cmd.service_id = HAL_SERVICE_ID_HEALTH;
+ cmd.mode = HAL_MODE_DEFAULT;

ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
sizeof(cmd), &cmd, 0, NULL, NULL);
--
1.9.3


2014-05-16 11:00:41

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCH 3/4] android/client: Support sending GATT Server responses

We prepare response by filling btgatt_response_t union's first member
only. The same thing is done by JNI in Android, which ignores second
member. There is also no parameter to tell which union member is set.
More to this, second union member seams to be redundant duplicate of
first member's inner member (broken API?). JNI method is always called
with auth_req set to 0 thus this parameter is omitted in haltest.
---
android/client/if-gatt.c | 39 +++++++++++++++++++++++++++++++++++++--
1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/android/client/if-gatt.c b/android/client/if-gatt.c
index ebf9c59..252e89d 100644
--- a/android/client/if-gatt.c
+++ b/android/client/if-gatt.c
@@ -64,6 +64,9 @@ const btgatt_interface_t *if_gatt = NULL;
#define VERIFY_CLIENT_IF(n, v) VERIFY_INT_ARG(n, v, "No client_if specified\n")
#define VERIFY_SERVER_IF(n, v) VERIFY_INT_ARG(n, v, "No server_if specified\n")
#define VERIFY_CONN_ID(n, v) VERIFY_INT_ARG(n, v, "No conn_if specified\n")
+#define VERIFY_TRANS_ID(n, v) VERIFY_INT_ARG(n, v, "No trans_id specified\n")
+#define VERIFY_STATUS(n, v) VERIFY_INT_ARG(n, v, "No status specified\n")
+#define VERIFY_OFFSET(n, v) VERIFY_INT_ARG(n, v, "No offset specified\n")
#define VERIFY_HANDLE(n, v) VERIFY_HEX_ARG(n, v, "No "#v" specified\n")
#define VERIFY_SERVICE_HANDLE(n, v) VERIFY_HANDLE(n, v)

@@ -1751,7 +1754,38 @@ static void gatts_send_indication_p(int argc, const char *argv[])

static void gatts_send_response_p(int argc, const char *argv[])
{
- haltest_warn("%s is not implemented yet\n", __func__);
+ int conn_id;
+ int trans_id;
+ int status;
+ btgatt_response_t data;
+
+ memset(&data, 0, sizeof(data));
+
+ RETURN_IF_NULL(if_gatt);
+
+ VERIFY_CONN_ID(2, conn_id);
+ VERIFY_TRANS_ID(3, trans_id);
+ VERIFY_STATUS(4, status);
+ VERIFY_HANDLE(5, data.attr_value.handle);
+ VERIFY_OFFSET(6, data.attr_value.offset);
+
+ data.attr_value.auth_req = 0;
+ data.attr_value.len = 0;
+
+ if (argc <= 7) {
+ haltest_error("No data specified\n");
+ return;
+ }
+
+ data.attr_value.len = strlen(argv[7]);
+ scan_field(argv[7], data.attr_value.len, data.attr_value.value,
+ sizeof(data.attr_value.value));
+
+
+ haltest_info("conn_id %d, trans_id %d, status %d", conn_id, trans_id,
+ status);
+
+ EXEC(if_gatt->server->send_response, conn_id, trans_id, status, &data);
}

#define GATTS_METHODH(n, h) METHOD(#n, gatts_##n##_p, NULL, h)
@@ -1775,7 +1809,8 @@ static struct method server_methods[] = {
GATTS_METHODCH(delete_service, "<server_if> <service_handle>"),
GATTS_METHODH(send_indication,
"<server_if> <attr_handle> <conn_id> <confirm> [<data>]"),
- GATTS_METHODH(send_response, "<conn_id> <trans_id> <status>"),
+ GATTS_METHODH(send_response,
+ "<conn_id> <trans_id> <status> <handle> <offset> [<data>]"),
END_METHOD
};

--
1.9.3


2014-05-16 11:00:40

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCH 2/4] android/hal: Fix sending not initialised data

As we always send whole gatt_response_t struct through IPC,
but copy only cmd->len bytes, the rest should be initilised to 0.

This fixes the following issue:
==30585== Syscall param socketcall.sendmsg(msg.msg_iov[i]) points
to uninitialised byte(s)
==30585== at 0x534133D: ??? (syscall-template.S:82)
==30585== by 0x7756336: hal_ipc_cmd (hal-ipc.c:359)
==30585== by 0x77546DF: send_response.part.0 (hal-gatt.c:1247)
==30585== by 0x408119: gatts_send_response_p (if-gatt.c:1777)
==30585== by 0x40219F: process_line (haltest.c:293)
==30585== by 0x402552: terminal_action_enter (terminal.c:666)
==30585== by 0x403184: terminal_process_char (terminal.c:781)
==30585== by 0x401B90: stdin_handler (haltest.c:308)
==30585== by 0x402261: poll_dispatch_loop (pollhandler.c:60)
==30585== by 0x401870: main (haltest.c:441)
==30585== Address 0x7fefff2a3 is on thread 1's stack
---
android/hal-gatt.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/android/hal-gatt.c b/android/hal-gatt.c
index e1faccb..93dc066 100644
--- a/android/hal-gatt.c
+++ b/android/hal-gatt.c
@@ -1231,6 +1231,8 @@ static bt_status_t send_response(int conn_id, int trans_id, int status,
struct hal_cmd_gatt_server_send_response *cmd = (void *) buf;
size_t cmd_len = sizeof(*cmd) + sizeof(*response);

+ memset(buf, 0 , IPC_MTU);
+
if (!interface_ready())
return BT_STATUS_NOT_READY;

--
1.9.3