2014-05-22 10:54:30

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCHv5 1/4] android/gatt: Send Exchange MTU request on connect

This adds sending exchange MTU request on connection.

This is needed to pass TC_GAC_CL_BV_01_C.
---
android/gatt.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 71 insertions(+)

diff --git a/android/gatt.c b/android/gatt.c
index 89da60d..82e8dc1 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -979,6 +979,73 @@ static void send_app_connect_notifications(void *data, void *user_data)

static void att_handler(const uint8_t *ipdu, uint16_t len, gpointer user_data);

+static void exchange_mtu_cb(guint8 status, const guint8 *pdu, guint16 plen,
+ gpointer user_data)
+{
+ struct gatt_device *device = user_data;
+ GIOChannel *io;
+ GError *gerr = NULL;
+ uint16_t rmtu, mtu, imtu;
+
+ if (status) {
+ error("gatt: MTU exchange: %s", att_ecode2str(status));
+ goto failed;
+ }
+
+ if (!dec_mtu_resp(pdu, plen, &rmtu)) {
+ error("gatt: MTU exchange: protocol error");
+ goto failed;
+ }
+
+ if (rmtu < ATT_DEFAULT_LE_MTU) {
+ error("gatt: MTU exchange: mtu error");
+ goto failed;
+ }
+
+ io = g_attrib_get_channel(device->attrib);
+
+ bt_io_get(io, &gerr, BT_IO_OPT_IMTU, &imtu, BT_IO_OPT_INVALID);
+ if (gerr) {
+ error("gatt: Could not get imtu: %s", gerr->message);
+ g_error_free(gerr);
+
+ return;
+ }
+
+ mtu = MIN(rmtu, imtu);
+ if (mtu != imtu && !g_attrib_set_mtu(device->attrib, mtu)) {
+ error("gatt: MTU exchange failed");
+ goto failed;
+ }
+
+ DBG("MTU exchange succeeded: rmtu:%d, old mtu:%d, new mtu:%d", rmtu,
+ imtu, mtu);
+
+failed:
+ device_unref(device);
+}
+
+static void send_exchange_mtu_request(struct gatt_device *device)
+{
+ GIOChannel *io;
+ GError *gerr = NULL;
+ uint16_t imtu;
+
+ io = g_attrib_get_channel(device->attrib);
+
+ bt_io_get(io, &gerr, BT_IO_OPT_IMTU, &imtu, BT_IO_OPT_INVALID);
+ if (gerr) {
+ error("gatt: Could not get imtu: %s", gerr->message);
+ g_error_free(gerr);
+
+ return;
+ }
+
+ if (!gatt_exchange_mtu(device->attrib, imtu, exchange_mtu_cb,
+ device_ref(device)))
+ device_unref(device);
+}
+
static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
{
struct gatt_device *dev = user_data;
@@ -1024,6 +1091,10 @@ static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)

device_set_state(dev, DEVICE_CONNECTED);

+ /* Send exchange mtu request as we assume being client and server */
+ /* TODO: Dont exchange mtu if no client apps */
+ send_exchange_mtu_request(dev);
+
status = GATT_SUCCESS;

reply:
--
1.9.3



2014-05-22 14:49:32

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCHv5 1/4] android/gatt: Send Exchange MTU request on connect

Hi Jakub,

On Thursday 22 of May 2014 12:54:30 Jakub Tyszkowski wrote:
> This adds sending exchange MTU request on connection.
>
> This is needed to pass TC_GAC_CL_BV_01_C.
> ---
> android/gatt.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 71 insertions(+)
>
> diff --git a/android/gatt.c b/android/gatt.c
> index 89da60d..82e8dc1 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -979,6 +979,73 @@ static void send_app_connect_notifications(void *data, void *user_data)
>
> static void att_handler(const uint8_t *ipdu, uint16_t len, gpointer user_data);
>
> +static void exchange_mtu_cb(guint8 status, const guint8 *pdu, guint16 plen,
> + gpointer user_data)
> +{
> + struct gatt_device *device = user_data;
> + GIOChannel *io;
> + GError *gerr = NULL;
> + uint16_t rmtu, mtu, imtu;
> +
> + if (status) {
> + error("gatt: MTU exchange: %s", att_ecode2str(status));
> + goto failed;
> + }
> +
> + if (!dec_mtu_resp(pdu, plen, &rmtu)) {
> + error("gatt: MTU exchange: protocol error");
> + goto failed;
> + }
> +
> + if (rmtu < ATT_DEFAULT_LE_MTU) {
> + error("gatt: MTU exchange: mtu error");
> + goto failed;
> + }
> +
> + io = g_attrib_get_channel(device->attrib);
> +
> + bt_io_get(io, &gerr, BT_IO_OPT_IMTU, &imtu, BT_IO_OPT_INVALID);
> + if (gerr) {
> + error("gatt: Could not get imtu: %s", gerr->message);
> + g_error_free(gerr);
> +
> + return;
> + }
> +
> + mtu = MIN(rmtu, imtu);
> + if (mtu != imtu && !g_attrib_set_mtu(device->attrib, mtu)) {
> + error("gatt: MTU exchange failed");
> + goto failed;
> + }
> +
> + DBG("MTU exchange succeeded: rmtu:%d, old mtu:%d, new mtu:%d", rmtu,
> + imtu, mtu);
> +
> +failed:
> + device_unref(device);
> +}
> +
> +static void send_exchange_mtu_request(struct gatt_device *device)
> +{
> + GIOChannel *io;
> + GError *gerr = NULL;
> + uint16_t imtu;
> +
> + io = g_attrib_get_channel(device->attrib);
> +
> + bt_io_get(io, &gerr, BT_IO_OPT_IMTU, &imtu, BT_IO_OPT_INVALID);
> + if (gerr) {
> + error("gatt: Could not get imtu: %s", gerr->message);
> + g_error_free(gerr);
> +
> + return;
> + }
> +
> + if (!gatt_exchange_mtu(device->attrib, imtu, exchange_mtu_cb,
> + device_ref(device)))
> + device_unref(device);
> +}
> +
> static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
> {
> struct gatt_device *dev = user_data;
> @@ -1024,6 +1091,10 @@ static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
>
> device_set_state(dev, DEVICE_CONNECTED);
>
> + /* Send exchange mtu request as we assume being client and server */
> + /* TODO: Dont exchange mtu if no client apps */
> + send_exchange_mtu_request(dev);
> +
> status = GATT_SUCCESS;
>
> reply:

Patch 1/4 is now applied but rest need rebase. Thanks.


--
Best regards,
Szymon Janc

2014-05-22 10:54:32

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCHv5 3/4] android/gatt: Make att_handler use g_attrib buffer

This makes use of g_attrib buffer for output and removes some unused
read_by_group_type function parameters.
---
android/gatt.c | 33 +++++++++++++++------------------
1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index c67fc97..4a9fd56 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -4398,8 +4398,6 @@ static const struct ipc_handler cmd_handlers[] = {
};

static uint8_t read_by_group_type(const uint8_t *cmd, uint16_t cmd_len,
- uint8_t *rsp, size_t rsp_size,
- uint16_t *length,
struct gatt_device *device)
{
uint16_t start, end;
@@ -4783,24 +4781,22 @@ static uint8_t write_prep_request(const uint8_t *cmd, uint16_t cmd_len,
static void att_handler(const uint8_t *ipdu, uint16_t len, gpointer user_data)
{
struct gatt_device *dev = user_data;
- uint8_t opdu[ATT_DEFAULT_LE_MTU];
uint8_t status;
- uint16_t length = 0;
- size_t vlen;
- uint8_t *value = g_attrib_get_buffer(dev->attrib, &vlen);
+ uint16_t resp_length = 0;
+ size_t length;
+ uint8_t *opdu = g_attrib_get_buffer(dev->attrib, &length);

DBG("op 0x%02x", ipdu[0]);

- if (len > vlen) {
- error("gatt: Too much data on ATT socket %p", value);
+ if (len > length) {
+ error("gatt: Too much data on ATT socket %p", opdu);
status = ATT_ECODE_INVALID_PDU;
goto done;
}

switch (ipdu[0]) {
case ATT_OP_READ_BY_GROUP_REQ:
- status = read_by_group_type(ipdu, len, opdu, sizeof(opdu),
- &length, dev);
+ status = read_by_group_type(ipdu, len, dev);
break;
case ATT_OP_READ_BY_TYPE_REQ:
status = read_by_type(ipdu, len, dev);
@@ -4810,12 +4806,12 @@ static void att_handler(const uint8_t *ipdu, uint16_t len, gpointer user_data)
status = read_request(ipdu, len, dev);
break;
case ATT_OP_MTU_REQ:
- status = mtu_att_handle(ipdu, len, opdu, sizeof(opdu), dev,
- &length);
+ status = mtu_att_handle(ipdu, len, opdu, length, dev,
+ &resp_length);
break;
case ATT_OP_FIND_INFO_REQ:
- status = find_info_handle(ipdu, len, opdu, sizeof(opdu),
- &length);
+ status = find_info_handle(ipdu, len, opdu, length,
+ &resp_length);
break;
case ATT_OP_WRITE_REQ:
status = write_req_request(ipdu, len, dev);
@@ -4850,11 +4846,12 @@ static void att_handler(const uint8_t *ipdu, uint16_t len, gpointer user_data)

done:
if (status)
- length = enc_error_resp(ipdu[0], 0x0000, status, opdu,
- ATT_DEFAULT_LE_MTU);
+ resp_length = enc_error_resp(ipdu[0], 0x0000, status, opdu,
+ length);

- if (length)
- g_attrib_send(dev->attrib, 0, opdu, length, NULL, NULL, NULL);
+ if (resp_length)
+ g_attrib_send(dev->attrib, 0, opdu, resp_length, NULL, NULL,
+ NULL);
}

static void create_listen_connections(void *data, void *user_data)
--
1.9.3


2014-05-22 10:54:33

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCHv5 4/4] android/gatt: Use default mtu on mtu exchange response

We should be setting new mtu after sending response using old mtu. This
also moves sending the response from att_handler to mtu_att_handle.
---
android/gatt.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 4a9fd56..d42fd0b 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -4554,14 +4554,14 @@ static uint8_t read_request(const uint8_t *cmd, uint16_t cmd_len,
}

static uint8_t mtu_att_handle(const uint8_t *cmd, uint16_t cmd_len,
- uint8_t *rsp, size_t rsp_size,
- struct gatt_device *dev,
- uint16_t *length)
+ struct gatt_device *dev)
{
- uint16_t mtu, imtu, omtu;
+ uint16_t mtu, imtu;
+ size_t omtu;
GIOChannel *io;
GError *gerr = NULL;
uint16_t len;
+ uint8_t *rsp;

DBG("");

@@ -4576,7 +4576,6 @@ static uint8_t mtu_att_handle(const uint8_t *cmd, uint16_t cmd_len,

bt_io_get(io, &gerr,
BT_IO_OPT_IMTU, &imtu,
- BT_IO_OPT_OMTU, &omtu,
BT_IO_OPT_INVALID);
if (gerr) {
error("bt_io_get: %s", gerr->message);
@@ -4584,16 +4583,18 @@ static uint8_t mtu_att_handle(const uint8_t *cmd, uint16_t cmd_len,
return ATT_ECODE_UNLIKELY;
}

- /* Limit OMTU to received value */
- mtu = MIN(mtu, omtu);
- g_attrib_set_mtu(dev->attrib, mtu);
+ rsp = g_attrib_get_buffer(dev->attrib, &omtu);

/* Respond with our IMTU */
- len = enc_mtu_resp(imtu, rsp, rsp_size);
+ len = enc_mtu_resp(imtu, rsp, omtu);
if (!len)
return ATT_ECODE_UNLIKELY;

- *length = len;
+ g_attrib_send(dev->attrib, 0, rsp, len, NULL, NULL, NULL);
+
+ /* Limit OMTU to received value */
+ mtu = MIN(mtu, omtu);
+ g_attrib_set_mtu(dev->attrib, mtu);

return 0;
}
@@ -4806,8 +4807,7 @@ static void att_handler(const uint8_t *ipdu, uint16_t len, gpointer user_data)
status = read_request(ipdu, len, dev);
break;
case ATT_OP_MTU_REQ:
- status = mtu_att_handle(ipdu, len, opdu, length, dev,
- &resp_length);
+ status = mtu_att_handle(ipdu, len, dev);
break;
case ATT_OP_FIND_INFO_REQ:
status = find_info_handle(ipdu, len, opdu, length,
--
1.9.3


2014-05-22 10:54:31

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCHv5 2/4] android/gatt: Use g_attrib buffer where possible for att

This replaces fixed size pdu usage with g_attrib buffer when possible.
When only received packets are decoded we use dynamic allocation with
current mtu.
---
android/gatt.c | 42 ++++++++++++++++++++++++------------------
1 file changed, 24 insertions(+), 18 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 82e8dc1..c67fc97 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -3567,7 +3567,8 @@ static bool is_service(const bt_uuid_t *type)
static void send_dev_pending_response(struct gatt_device *device,
uint8_t opcode)
{
- uint8_t rsp[ATT_DEFAULT_LE_MTU];
+ size_t mtu;
+ uint8_t *rsp = g_attrib_get_buffer(device->attrib, &mtu);
struct pending_request *val;
uint16_t len = 0;
uint8_t error = 0;
@@ -3612,7 +3613,7 @@ static void send_dev_pending_response(struct gatt_device *device,
val = queue_pop_head(temp);
}

- len = enc_read_by_type_resp(adl, rsp, sizeof(rsp));
+ len = enc_read_by_type_resp(adl, rsp, mtu);

att_data_list_free(adl);
queue_destroy(temp, destroy_pending_request);
@@ -3627,7 +3628,7 @@ static void send_dev_pending_response(struct gatt_device *device,
}

len = enc_read_blob_resp(val->value, val->length, val->offset,
- rsp, sizeof(rsp));
+ rsp, mtu);
destroy_pending_request(val);
break;
case ATT_OP_READ_REQ:
@@ -3637,7 +3638,7 @@ static void send_dev_pending_response(struct gatt_device *device,
goto done;
}

- len = enc_read_resp(val->value, val->length, rsp, sizeof(rsp));
+ len = enc_read_resp(val->value, val->length, rsp, mtu);
destroy_pending_request(val);
break;
case ATT_OP_READ_BY_GROUP_REQ: {
@@ -3683,7 +3684,7 @@ static void send_dev_pending_response(struct gatt_device *device,
val = queue_pop_head(temp);
}

- len = enc_read_by_grp_resp(adl, rsp, sizeof(rsp));
+ len = enc_read_by_grp_resp(adl, rsp, mtu);

att_data_list_free(adl);
queue_destroy(temp, destroy_pending_request);
@@ -3731,7 +3732,7 @@ static void send_dev_pending_response(struct gatt_device *device,
}

if (list && !error)
- len = enc_find_by_type_resp(list, rsp, sizeof(rsp));
+ len = enc_find_by_type_resp(list, rsp, mtu);
else
error = ATT_ECODE_ATTR_NOT_FOUND;

@@ -3767,7 +3768,7 @@ static void send_dev_pending_response(struct gatt_device *device,
}

len = enc_prep_write_resp(val->handle, val->offset, val->value,
- val->length, rsp, sizeof(rsp));
+ val->length, rsp, mtu);
destroy_pending_request(val);
break;
default:
@@ -3776,8 +3777,7 @@ static void send_dev_pending_response(struct gatt_device *device,

done:
if (!len)
- len = enc_error_resp(opcode, 0x0000, error, rsp,
- ATT_DEFAULT_LE_MTU);
+ len = enc_error_resp(opcode, 0x0000, error, rsp, mtu);

g_attrib_send(device->attrib, 0, rsp, len, NULL, NULL, NULL);

@@ -4213,10 +4213,11 @@ failed:
static void handle_server_send_indication(const void *buf, uint16_t len)
{
const struct hal_cmd_gatt_server_send_indication *cmd = buf;
- uint8_t pdu[ATT_DEFAULT_LE_MTU];
struct app_connection *conn;
uint8_t status;
uint16_t length;
+ uint8_t *pdu;
+ size_t mtu;

DBG("");

@@ -4227,15 +4228,17 @@ static void handle_server_send_indication(const void *buf, uint16_t len)
goto reply;
}

+ pdu = g_attrib_get_buffer(conn->device->attrib, &mtu);
+
if (cmd->confirm)
/* TODO: Add data to track confirmation for this request */
length = enc_indication(cmd->attribute_handle,
- (uint8_t *)cmd->value, cmd->len,
- pdu, sizeof(pdu));
+ (uint8_t *)cmd->value, cmd->len, pdu,
+ mtu);
else
length = enc_notification(cmd->attribute_handle,
(uint8_t *)cmd->value, cmd->len,
- pdu, sizeof(pdu));
+ pdu, mtu);

g_attrib_send(conn->device->attrib, 0, pdu, length, NULL, NULL, NULL);

@@ -4667,7 +4670,7 @@ static uint8_t find_info_handle(const uint8_t *cmd, uint16_t cmd_len,
static uint8_t find_by_type_request(const uint8_t *cmd, uint16_t cmd_len,
struct gatt_device *device)
{
- uint8_t search_value[ATT_DEFAULT_LE_MTU];
+ uint8_t search_value[cmd_len];
size_t search_vlen;
uint16_t start, end;
uint16_t handle;
@@ -4725,7 +4728,7 @@ static uint8_t find_by_type_request(const uint8_t *cmd, uint16_t cmd_len,
static void write_cmd_request(const uint8_t *cmd, uint16_t cmd_len,
struct gatt_device *dev)
{
- uint8_t value[ATT_DEFAULT_LE_MTU];
+ uint8_t value[cmd_len];
uint16_t handle;
uint16_t len;
size_t vlen;
@@ -4740,7 +4743,7 @@ static void write_cmd_request(const uint8_t *cmd, uint16_t cmd_len,
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];
+ uint8_t value[cmd_len];
uint16_t handle;
uint16_t len;
size_t vlen;
@@ -4759,7 +4762,7 @@ static uint8_t write_req_request(const uint8_t *cmd, uint16_t cmd_len,
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];
+ uint8_t value[cmd_len];
uint16_t handle;
uint16_t offset;
uint16_t len;
@@ -5141,9 +5144,10 @@ static void gatt_srvc_change_register_cb(uint16_t handle, uint16_t offset,
bdaddr_t *bdaddr,
void *user_data)
{
- uint8_t pdu[ATT_DEFAULT_LE_MTU];
struct gatt_device *dev;
uint16_t length;
+ size_t mtu;
+ uint8_t *pdu;

dev = find_device_by_addr(bdaddr);
if (!dev) {
@@ -5151,6 +5155,8 @@ static void gatt_srvc_change_register_cb(uint16_t handle, uint16_t offset,
return;
}

+ pdu = g_attrib_get_buffer(dev->attrib, &mtu);
+
/* TODO handle CCC */

/* Set services changed notification flag */
--
1.9.3