2014-05-26 07:13:14

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCHv5 1/3] 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 70cd5fe..2ae3669 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -3569,7 +3569,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;
@@ -3613,7 +3614,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);
@@ -3628,7 +3629,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:
@@ -3638,7 +3639,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: {
@@ -3684,7 +3685,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);
@@ -3732,7 +3733,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;

@@ -3768,7 +3769,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:
@@ -3777,8 +3778,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);

@@ -4217,10 +4217,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("");

@@ -4231,15 +4232,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);

@@ -4659,7 +4662,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;
@@ -4717,7 +4720,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;
@@ -4732,7 +4735,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];
struct pending_request *data;
uint16_t handle;
uint16_t len;
@@ -4764,7 +4767,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];
struct pending_request *data;
uint16_t handle;
uint16_t offset;
@@ -5172,9 +5175,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) {
@@ -5182,6 +5186,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



2014-05-26 08:55:04

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCHv5 1/3] android/gatt: Use g_attrib buffer where possible for att

Hi Jakub,

On Monday 26 of May 2014 09:13:14 Jakub Tyszkowski wrote:
> 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 70cd5fe..2ae3669 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -3569,7 +3569,8 @@ static bool is_service(const bt_uuid_t *type)
> static void send_dev_pending_response(struct gatt_device *device,

Applied, thanks.

--
Best regards,
Szymon Janc

2014-05-26 07:13:16

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCHv5 3/3] 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 5da6ffa..c8af09c 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -4546,14 +4546,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("");

@@ -4568,7 +4568,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);
@@ -4576,16 +4575,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;
}
@@ -4825,8 +4826,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-26 07:13:15

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCHv5 2/3] 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 2ae3669..5da6ffa 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -4402,8 +4402,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;
@@ -4802,24 +4800,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);
@@ -4829,12 +4825,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);
@@ -4877,11 +4873,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