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

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

[Jakub Tyszkowski]

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

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

[Jakub Tyszkowski]

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

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

[Jakub Tyszkowski]

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

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

[Jakub Tyszkowski]

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

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

[Szymon Janc]

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