[PATCH 0/2] Implement Compound GATT procedures, Long Reads

[PATCH 0/2] Implement Compound GATT procedures, Long Reads

[Brian Gix]

These patches address the concerns raised by Vinicius and Claudio.

They are based on a rebase as of:

commit 3fd5fa3890e3db3857f212ef6a187f7744d07cba
Author: Claudio Takahasi <claudio.takahasi@openbossa.org>
Date:   Mon Jan 3 18:45:54 2011 -0300


-- 
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[PATCH 1/2] Fix g_attrib_send() to include a new ID parameter

[Brian Gix]

Overall purpose of change is to enable a GATT procedure to be
executed atomically, even if it requires multiple ATT
request/response transactions.

Fix g_attrib_send() to include an ID parameter, if the pkt to
be sent should be added to the Head of the pkt queue.  If the
ID is Zero, legacy functionality is maintained, and the pkt will
be added at the tail of the queuer, and a new ID will be generated,
and returned to the caller. If ID is non-zero, the pkt will be
added to the head of the queue, with the ID value requested, which
will also be returned to the caller.

Fix received_data() to not service the send queue until after the
received data has been processed by calling the cmd->func()
callback, to allow the callback to insert another pkt on the head
of the queue.

Fix all callers of g_attrib_send() to include new parameter.
---
 attrib/client.c     |    2 +-
 attrib/gatt.c       |   12 ++++++------
 attrib/gattrib.c    |   22 +++++++++++++++-------
 attrib/gattrib.h    |    7 ++++---
 attrib/gatttool.c   |    2 +-
 src/attrib-server.c |    2 +-
 6 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index 644cd62..7f72348 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -295,7 +295,7 @@ static void events_handler(const uint8_t *pdu, uint16_t len,
 	switch (pdu[0]) {
 	case ATT_OP_HANDLE_IND:
 		olen = enc_confirmation(opdu, sizeof(opdu));
-		g_attrib_send(gatt->attrib, opdu[0], opdu, olen,
+		g_attrib_send(gatt->attrib, 0, opdu[0], opdu, olen,
 						NULL, NULL, NULL);
 	case ATT_OP_HANDLE_NOTIFY:
 		if (characteristic_set_value(chr, &pdu[3], len - 3) < 0)
diff --git a/attrib/gatt.c b/attrib/gatt.c
index bca8b49..320759f 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -68,7 +68,7 @@ guint gatt_discover_primary(GAttrib *attrib, uint16_t start, uint16_t end,
 	if (plen == 0)
 		return 0;
 
-	return g_attrib_send(attrib, op, pdu, plen, func, user_data, NULL);
+	return g_attrib_send(attrib, 0, op, pdu, plen, func, user_data, NULL);
 }
 
 guint gatt_discover_char(GAttrib *attrib, uint16_t start, uint16_t end,
@@ -93,7 +93,7 @@ guint gatt_read_char_by_uuid(GAttrib *attrib, uint16_t start, uint16_t end,
 	if (plen == 0)
 		return 0;
 
-	return g_attrib_send(attrib, ATT_OP_READ_BY_TYPE_REQ,
+	return g_attrib_send(attrib, 0, ATT_OP_READ_BY_TYPE_REQ,
 					pdu, plen, func, user_data, NULL);
 }
 
@@ -104,7 +104,7 @@ guint gatt_read_char(GAttrib *attrib, uint16_t handle, GAttribResultFunc func,
 	guint16 plen;
 
 	plen = enc_read_req(handle, pdu, sizeof(pdu));
-	return g_attrib_send(attrib, ATT_OP_READ_REQ, pdu, plen, func,
+	return g_attrib_send(attrib, 0, ATT_OP_READ_REQ, pdu, plen, func,
 							user_data, NULL);
 }
 
@@ -115,7 +115,7 @@ guint gatt_write_char(GAttrib *attrib, uint16_t handle, uint8_t *value,
 	guint16 plen;
 
 	plen = enc_write_req(handle, value, vlen, pdu, sizeof(pdu));
-	return g_attrib_send(attrib, ATT_OP_WRITE_REQ, pdu, plen, func,
+	return g_attrib_send(attrib, 0, ATT_OP_WRITE_REQ, pdu, plen, func,
 							user_data, NULL);
 }
 
@@ -129,7 +129,7 @@ guint gatt_find_info(GAttrib *attrib, uint16_t start, uint16_t end,
 	if (plen == 0)
 		return 0;
 
-	return g_attrib_send(attrib, ATT_OP_FIND_INFO_REQ, pdu, plen, func,
+	return g_attrib_send(attrib, 0, ATT_OP_FIND_INFO_REQ, pdu, plen, func,
 							user_data, NULL);
 }
 
@@ -140,6 +140,6 @@ guint gatt_write_cmd(GAttrib *attrib, uint16_t handle, uint8_t *value, int vlen,
 	guint16 plen;
 
 	plen = enc_write_cmd(handle, value, vlen, pdu, sizeof(pdu));
-	return g_attrib_send(attrib, ATT_OP_WRITE_CMD, pdu, plen, NULL,
+	return g_attrib_send(attrib, 0, ATT_OP_WRITE_CMD, pdu, plen, NULL,
 							user_data, notify);
 }
diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index dd7b1d7..779a471 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -294,6 +294,7 @@ static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer data)
 	uint8_t buf[512], status;
 	gsize len;
 	GIOStatus iostat;
+	gboolean qempty;
 
 	if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) {
 		attrib->read_watch = 0;
@@ -341,8 +342,7 @@ static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer data)
 	status = 0;
 
 done:
-	if (attrib->queue && g_queue_is_empty(attrib->queue) == FALSE)
-		wake_up_sender(attrib);
+	qempty = attrib->queue == NULL || g_queue_is_empty(attrib->queue);
 
 	if (cmd) {
 		if (cmd->func)
@@ -351,6 +351,9 @@ done:
 		command_destroy(cmd);
 	}
 
+	if (!qempty)
+		wake_up_sender(attrib);
+
 	return TRUE;
 }
 
@@ -376,9 +379,9 @@ GAttrib *g_attrib_new(GIOChannel *io)
 	return g_attrib_ref(attrib);
 }
 
-guint g_attrib_send(GAttrib *attrib, guint8 opcode, const guint8 *pdu,
-				guint16 len, GAttribResultFunc func,
-				gpointer user_data, GDestroyNotify notify)
+guint g_attrib_send(GAttrib *attrib, guint id, guint8 opcode,
+			const guint8 *pdu, guint16 len, GAttribResultFunc func,
+			gpointer user_data, GDestroyNotify notify)
 {
 	struct command *c;
 
@@ -394,9 +397,14 @@ guint g_attrib_send(GAttrib *attrib, guint8 opcode, const guint8 *pdu,
 	c->func = func;
 	c->user_data = user_data;
 	c->notify = notify;
-	c->id = ++attrib->next_cmd_id;
 
-	g_queue_push_tail(attrib->queue, c);
+	if (id) {
+		c->id = id;
+		g_queue_push_head(attrib->queue, c);
+	} else {
+		c->id = ++attrib->next_cmd_id;
+		g_queue_push_tail(attrib->queue, c);
+	}
 
 	if (g_queue_get_length(attrib->queue) == 1)
 		wake_up_sender(attrib);
diff --git a/attrib/gattrib.h b/attrib/gattrib.h
index 5c50bc9..f25208d 100644
--- a/attrib/gattrib.h
+++ b/attrib/gattrib.h
@@ -52,9 +52,10 @@ gboolean g_attrib_set_disconnect_function(GAttrib *attrib,
 gboolean g_attrib_set_destroy_function(GAttrib *attrib,
 		GDestroyNotify destroy, gpointer user_data);
 
-guint g_attrib_send(GAttrib *attrib, guint8 opcode, const guint8 *pdu,
-				guint16 len, GAttribResultFunc func,
-				gpointer user_data, GDestroyNotify notify);
+guint g_attrib_send(GAttrib *attrib, guint id, guint8 opcode,
+			const guint8 *pdu, guint16 len, GAttribResultFunc func,
+			gpointer user_data, GDestroyNotify notify);
+
 gboolean g_attrib_cancel(GAttrib *attrib, guint id);
 gboolean g_attrib_cancel_all(GAttrib *attrib);
 
diff --git a/attrib/gatttool.c b/attrib/gatttool.c
index a234e36..a6f92db 100644
--- a/attrib/gatttool.c
+++ b/attrib/gatttool.c
@@ -272,7 +272,7 @@ static void events_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
 	olen = enc_confirmation(opdu, sizeof(opdu));
 
 	if (olen > 0)
-		g_attrib_send(attrib, opdu[0], opdu, olen, NULL, NULL, NULL);
+		g_attrib_send(attrib, 0, opdu[0], opdu, olen, NULL, NULL, NULL);
 }
 
 static gboolean listen_start(gpointer user_data)
diff --git a/src/attrib-server.c b/src/attrib-server.c
index cbc01ee..aee2ace 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -694,7 +694,7 @@ done:
 	if (status)
 		length = enc_error_resp(ipdu[0], 0x0000, status, opdu, channel->mtu);
 
-	g_attrib_send(channel->attrib, opdu[0], opdu, length,
+	g_attrib_send(channel->attrib, 0, opdu[0], opdu, length,
 							NULL, NULL, NULL);
 }
 
-- 
1.7.1
-- 
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[PATCH 2/2] Fix gatt_read_char() to support long Attrib Values

[Brian Gix]

Fix gatt_read_char() to support long Attribute Values by recognizing
that results longer that 21 octets may include data beyond
what has been returned with the first read. Extra data is
obtained by issuing READ_BLOB requests until either a
result is returned shorter than 22 octets, or an error
is recieved indicating that no further data is available.
The API for this function has not changed.
---
 attrib/gatt.c |  130 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 128 insertions(+), 2 deletions(-)

diff --git a/attrib/gatt.c b/attrib/gatt.c
index 320759f..ae33211 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -97,15 +97,141 @@ guint gatt_read_char_by_uuid(GAttrib *attrib, uint16_t start, uint16_t end,
 					pdu, plen, func, user_data, NULL);
 }
 
+struct read_long_data {
+	GAttrib *attrib;
+	GAttribResultFunc func;
+	gpointer user_data;
+	guint8 *buffer;
+	guint16 size;
+	guint16 handle;
+	guint id;
+	gint ref;
+};
+
+static void read_long_destroy(gpointer user_data)
+{
+	struct read_long_data *long_read = user_data;
+
+	if (g_atomic_int_dec_and_test(&long_read->ref) == FALSE)
+		return;
+
+	if (long_read->buffer != NULL)
+		g_free(long_read->buffer);
+
+	g_free(long_read);
+}
+
+static void read_blob_helper(guint8 status, const guint8 *rpdu, guint16 rlen,
+							gpointer user_data)
+{
+	struct read_long_data *long_read = user_data;
+	uint8_t pdu[ATT_DEFAULT_MTU];
+	guint8 *tmp;
+	guint16 plen;
+	guint id;
+
+	if (status != 0 || rlen == 1) {
+		status = 0;
+		goto done;
+	}
+
+	tmp = g_try_realloc(long_read->buffer, long_read->size + rlen - 1);
+
+	if (tmp == NULL) {
+		status = ATT_ECODE_INSUFF_RESOURCES;
+		goto done;
+	}
+
+	memcpy(&tmp[long_read->size], &rpdu[1], rlen - 1);
+	long_read->buffer = tmp;
+	long_read->size += rlen - 1;
+
+	if (rlen < ATT_DEFAULT_MTU)
+		goto done;
+
+	plen = enc_read_blob_req(long_read->handle, long_read->size - 1,
+							pdu, sizeof(pdu));
+	id = g_attrib_send(long_read->attrib, long_read->id,
+				ATT_OP_READ_BLOB_REQ, pdu, plen,
+				read_blob_helper, long_read, read_long_destroy);
+
+	if (id != 0) {
+		g_atomic_int_inc(&long_read->ref);
+		return;
+	}
+
+	status = ATT_ECODE_IO;
+
+done:
+	long_read->func(status, long_read->buffer, long_read->size,
+							long_read->user_data);
+}
+
+static void read_char_helper(guint8 status, const guint8 *rpdu,
+					guint16 rlen, gpointer user_data)
+{
+	struct read_long_data *long_read = user_data;
+	uint8_t pdu[ATT_DEFAULT_MTU];
+	guint16 plen;
+	guint id;
+
+	if (status != 0 || rlen < ATT_DEFAULT_MTU)
+		goto done;
+
+	long_read->buffer = g_malloc(rlen);
+
+	if (long_read->buffer == NULL)
+		goto done;
+
+	memcpy(long_read->buffer, rpdu, rlen);
+	long_read->size = rlen;
+
+	plen = enc_read_blob_req(long_read->handle, rlen - 1, pdu, sizeof(pdu));
+	id = g_attrib_send(long_read->attrib, long_read->id,
+			ATT_OP_READ_BLOB_REQ, pdu, plen, read_blob_helper,
+			long_read, read_long_destroy);
+
+	if (id != 0) {
+		g_atomic_int_inc(&long_read->ref);
+		return;
+	}
+
+	status = ATT_ECODE_IO;
+
+done:
+	long_read->func(status, rpdu, rlen, long_read->user_data);
+}
+
 guint gatt_read_char(GAttrib *attrib, uint16_t handle, GAttribResultFunc func,
 							gpointer user_data)
 {
 	uint8_t pdu[ATT_DEFAULT_MTU];
 	guint16 plen;
+	guint id;
+	struct read_long_data *long_read;
+
+	long_read = g_try_new0(struct read_long_data, 1);
+
+	if (long_read == NULL)
+		return 0;
+
+	long_read->attrib = attrib;
+	long_read->func = func;
+	long_read->user_data = user_data;
+	long_read->handle = handle;
 
 	plen = enc_read_req(handle, pdu, sizeof(pdu));
-	return g_attrib_send(attrib, 0, ATT_OP_READ_REQ, pdu, plen, func,
-							user_data, NULL);
+	id = g_attrib_send(attrib, 0, ATT_OP_READ_REQ, pdu, plen,
+				read_char_helper, long_read, read_long_destroy);
+
+	if (id == 0)
+		g_free(long_read);
+	else {
+		g_atomic_int_inc(&long_read->ref);
+		long_read->id = id;
+	}
+
+	return id;
 }
 
 guint gatt_write_char(GAttrib *attrib, uint16_t handle, uint8_t *value,
-- 
1.7.1
-- 
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [PATCH 0/2] Implement Compound GATT procedures, Long Reads

[Johan Hedberg]

Hi Brian,

On Thu, Jan 06, 2011, Brian Gix wrote:
> These patches address the concerns raised by Vinicius and Claudio.
> 
> They are based on a rebase as of:
> 
> commit 3fd5fa3890e3db3857f212ef6a187f7744d07cba
> Author: Claudio Takahasi <claudio.takahasi@openbossa.org>
> Date:   Mon Jan 3 18:45:54 2011 -0300

Thanks. Both patches have been pushed upstream.

Johan

[PATCH 0/2] Implement Compound GATT Procedures and Long Reads

[Brian Gix]

These two patches implement Compound GATT procedures, so that they
are executed atomically, and in the order that they are invoked,
regardless of whether they can be performed in a single or multiple
ATT Req/Resp transaction.

Patch 1 adds an ID parameter to g_attrib_send which shall be Zero for the
first ATT transaction, and shall be the previously returned ID for all
subsequent ATT transactions for that GATT procedure.  The underlying
code will enqueue the requested pkt at the Tail of the queue for Zero
ID'd pkts, and at the Head of the queue for non-zero (continuation)
pkts. The queue is then only services (next item sent) after the
callbacks have been made, to allow any continuation pkts to be created.


Patch 2 then uses this new feature of g_attrib_send to implement
support for Long Attribute Reads (using READ_BLOB).



-- 
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[PATCH 1/2] Fix g_attrib_send() to include a new ID parameter

[Brian Gix]

Overall purpose of change is to enable a GATT procedure to be
	executed atomically, even if it requires multiple ATT
	request/response transactions.

Fix g_attrib_send() to include an ID parameter, if the pkt to
	be sent should be added to the Head of the pkt queue.
	If the ID is Zero, legacy functionality is maintained,
	and the pkt will be added at the tail of the queuer, and
	a new ID will be generated, and returned to the caller. If
	ID is non-zero, the pkt will be added to the head of the
	queue, with the ID value requested, which will also be
	returned to the caller.

Fix received_data() to not service the send queue until after the
	received data has been processed by calling the cmd->func()
	callback, to allow the callback to insert another pkt on
	the head of the queue.

Fix all callers of g_attrib_send() to include new parameter.
---
 attrib/client.c     |    2 +-
 attrib/gatt.c       |   12 ++++++------
 attrib/gattrib.c    |   22 +++++++++++++++-------
 attrib/gattrib.h    |    7 ++++---
 attrib/gatttool.c   |    2 +-
 src/attrib-server.c |    2 +-
 6 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index 10bbf7d..4301082 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -295,7 +295,7 @@ static void events_handler(const uint8_t *pdu, uint16_t len,
 	switch (pdu[0]) {
 	case ATT_OP_HANDLE_IND:
 		olen = enc_confirmation(opdu, sizeof(opdu));
-		g_attrib_send(gatt->attrib, opdu[0], opdu, olen,
+		g_attrib_send(gatt->attrib, 0, opdu[0], opdu, olen,
 						NULL, NULL, NULL);
 	case ATT_OP_HANDLE_NOTIFY:
 		if (characteristic_set_value(chr, &pdu[3], len - 3) < 0)
diff --git a/attrib/gatt.c b/attrib/gatt.c
index bca8b49..320759f 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -68,7 +68,7 @@ guint gatt_discover_primary(GAttrib *attrib, uint16_t start, uint16_t end,
 	if (plen == 0)
 		return 0;
 
-	return g_attrib_send(attrib, op, pdu, plen, func, user_data, NULL);
+	return g_attrib_send(attrib, 0, op, pdu, plen, func, user_data, NULL);
 }
 
 guint gatt_discover_char(GAttrib *attrib, uint16_t start, uint16_t end,
@@ -93,7 +93,7 @@ guint gatt_read_char_by_uuid(GAttrib *attrib, uint16_t start, uint16_t end,
 	if (plen == 0)
 		return 0;
 
-	return g_attrib_send(attrib, ATT_OP_READ_BY_TYPE_REQ,
+	return g_attrib_send(attrib, 0, ATT_OP_READ_BY_TYPE_REQ,
 					pdu, plen, func, user_data, NULL);
 }
 
@@ -104,7 +104,7 @@ guint gatt_read_char(GAttrib *attrib, uint16_t handle, GAttribResultFunc func,
 	guint16 plen;
 
 	plen = enc_read_req(handle, pdu, sizeof(pdu));
-	return g_attrib_send(attrib, ATT_OP_READ_REQ, pdu, plen, func,
+	return g_attrib_send(attrib, 0, ATT_OP_READ_REQ, pdu, plen, func,
 							user_data, NULL);
 }
 
@@ -115,7 +115,7 @@ guint gatt_write_char(GAttrib *attrib, uint16_t handle, uint8_t *value,
 	guint16 plen;
 
 	plen = enc_write_req(handle, value, vlen, pdu, sizeof(pdu));
-	return g_attrib_send(attrib, ATT_OP_WRITE_REQ, pdu, plen, func,
+	return g_attrib_send(attrib, 0, ATT_OP_WRITE_REQ, pdu, plen, func,
 							user_data, NULL);
 }
 
@@ -129,7 +129,7 @@ guint gatt_find_info(GAttrib *attrib, uint16_t start, uint16_t end,
 	if (plen == 0)
 		return 0;
 
-	return g_attrib_send(attrib, ATT_OP_FIND_INFO_REQ, pdu, plen, func,
+	return g_attrib_send(attrib, 0, ATT_OP_FIND_INFO_REQ, pdu, plen, func,
 							user_data, NULL);
 }
 
@@ -140,6 +140,6 @@ guint gatt_write_cmd(GAttrib *attrib, uint16_t handle, uint8_t *value, int vlen,
 	guint16 plen;
 
 	plen = enc_write_cmd(handle, value, vlen, pdu, sizeof(pdu));
-	return g_attrib_send(attrib, ATT_OP_WRITE_CMD, pdu, plen, NULL,
+	return g_attrib_send(attrib, 0, ATT_OP_WRITE_CMD, pdu, plen, NULL,
 							user_data, notify);
 }
diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index 9268001..79ee2e9 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -286,6 +286,7 @@ static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer data)
 	uint8_t buf[512], status;
 	gsize len;
 	GIOStatus iostat;
+	gboolean qempty;
 
 	if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) {
 		attrib->read_watch = 0;
@@ -333,8 +334,7 @@ static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer data)
 	status = 0;
 
 done:
-	if (attrib->queue && g_queue_is_empty(attrib->queue) == FALSE)
-		wake_up_sender(attrib);
+	qempty = attrib->queue == NULL || g_queue_is_empty(attrib->queue);
 
 	if (cmd) {
 		if (cmd->func)
@@ -343,6 +343,9 @@ done:
 		command_destroy(cmd);
 	}
 
+	if (!qempty)
+		wake_up_sender(attrib);
+
 	return TRUE;
 }
 
@@ -368,9 +371,9 @@ GAttrib *g_attrib_new(GIOChannel *io)
 	return g_attrib_ref(attrib);
 }
 
-guint g_attrib_send(GAttrib *attrib, guint8 opcode, const guint8 *pdu,
-				guint16 len, GAttribResultFunc func,
-				gpointer user_data, GDestroyNotify notify)
+guint g_attrib_send(GAttrib *attrib, guint id, guint8 opcode,
+		const guint8 *pdu, guint16 len, GAttribResultFunc func,
+		gpointer user_data, GDestroyNotify notify)
 {
 	struct command *c;
 
@@ -386,9 +389,14 @@ guint g_attrib_send(GAttrib *attrib, guint8 opcode, const guint8 *pdu,
 	c->func = func;
 	c->user_data = user_data;
 	c->notify = notify;
-	c->id = ++attrib->next_cmd_id;
 
-	g_queue_push_tail(attrib->queue, c);
+	if (id) {
+		c->id = id;
+		g_queue_push_head(attrib->queue, c);
+	} else {
+		c->id = ++attrib->next_cmd_id;
+		g_queue_push_tail(attrib->queue, c);
+	}
 
 	if (g_queue_get_length(attrib->queue) == 1)
 		wake_up_sender(attrib);
diff --git a/attrib/gattrib.h b/attrib/gattrib.h
index 0940289..1a966a7 100644
--- a/attrib/gattrib.h
+++ b/attrib/gattrib.h
@@ -50,9 +50,10 @@ gboolean g_attrib_set_disconnect_function(GAttrib *attrib,
 gboolean g_attrib_set_destroy_function(GAttrib *attrib,
 		GDestroyNotify destroy, gpointer user_data);
 
-guint g_attrib_send(GAttrib *attrib, guint8 opcode, const guint8 *pdu,
-				guint16 len, GAttribResultFunc func,
-				gpointer user_data, GDestroyNotify notify);
+guint g_attrib_send(GAttrib *attrib, guint id, guint8 opcode,
+			const guint8 *pdu, guint16 len, GAttribResultFunc func,
+			gpointer user_data, GDestroyNotify notify);
+
 gboolean g_attrib_cancel(GAttrib *attrib, guint id);
 gboolean g_attrib_cancel_all(GAttrib *attrib);
 
diff --git a/attrib/gatttool.c b/attrib/gatttool.c
index a234e36..a6f92db 100644
--- a/attrib/gatttool.c
+++ b/attrib/gatttool.c
@@ -272,7 +272,7 @@ static void events_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
 	olen = enc_confirmation(opdu, sizeof(opdu));
 
 	if (olen > 0)
-		g_attrib_send(attrib, opdu[0], opdu, olen, NULL, NULL, NULL);
+		g_attrib_send(attrib, 0, opdu[0], opdu, olen, NULL, NULL, NULL);
 }
 
 static gboolean listen_start(gpointer user_data)
diff --git a/src/attrib-server.c b/src/attrib-server.c
index cbc01ee..aee2ace 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -694,7 +694,7 @@ done:
 	if (status)
 		length = enc_error_resp(ipdu[0], 0x0000, status, opdu, channel->mtu);
 
-	g_attrib_send(channel->attrib, opdu[0], opdu, length,
+	g_attrib_send(channel->attrib, 0, opdu[0], opdu, length,
 							NULL, NULL, NULL);
 }
 
-- 
1.7.1
-- 
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [PATCH 1/2] Fix g_attrib_send() to include a new ID parameter

[Claudio Takahasi]

Hi Brian,

On Tue, Jan 4, 2011 at 9:01 PM, Brian Gix <bgix@codeaurora.org> wrote:
> Overall purpose of change is to enable a GATT procedure to be
>        executed atomically, even if it requires multiple ATT
>        request/response transactions.
>
> Fix g_attrib_send() to include an ID parameter, if the pkt to
>        be sent should be added to the Head of the pkt queue.
>        If the ID is Zero, legacy functionality is maintained,
>        and the pkt will be added at the tail of the queuer, and
>        a new ID will be generated, and returned to the caller. If
>        ID is non-zero, the pkt will be added to the head of the
>        queue, with the ID value requested, which will also be
>        returned to the caller.
>
> Fix received_data() to not service the send queue until after the
>        received data has been processed by calling the cmd->func()
>        callback, to allow the callback to insert another pkt on
>        the head of the queue.
>

We don't use tabs in the comments.

> Fix all callers of g_attrib_send() to include new parameter.
> ---
>  attrib/client.c     |    2 +-
>  attrib/gatt.c       |   12 ++++++------
>  attrib/gattrib.c    |   22 +++++++++++++++-------
>  attrib/gattrib.h    |    7 ++++---
>  attrib/gatttool.c   |    2 +-
>  src/attrib-server.c |    2 +-
>  6 files changed, 28 insertions(+), 19 deletions(-)
>
> diff --git a/attrib/client.c b/attrib/client.c
> index 10bbf7d..4301082 100644
> --- a/attrib/client.c
> +++ b/attrib/client.c
> @@ -295,7 +295,7 @@ static void events_handler(const uint8_t *pdu, uint16_t len,
>        switch (pdu[0]) {
>        case ATT_OP_HANDLE_IND:
>                olen = enc_confirmation(opdu, sizeof(opdu));
> -               g_attrib_send(gatt->attrib, opdu[0], opdu, olen,
> +               g_attrib_send(gatt->attrib, 0, opdu[0], opdu, olen,
>                                                NULL, NULL, NULL);
>        case ATT_OP_HANDLE_NOTIFY:
>                if (characteristic_set_value(chr, &pdu[3], len - 3) < 0)
> diff --git a/attrib/gatt.c b/attrib/gatt.c
> index bca8b49..320759f 100644
> --- a/attrib/gatt.c
> +++ b/attrib/gatt.c
> @@ -68,7 +68,7 @@ guint gatt_discover_primary(GAttrib *attrib, uint16_t start, uint16_t end,
>        if (plen == 0)
>                return 0;
>
> -       return g_attrib_send(attrib, op, pdu, plen, func, user_data, NULL);
> +       return g_attrib_send(attrib, 0, op, pdu, plen, func, user_data, NULL);
>  }
>
>  guint gatt_discover_char(GAttrib *attrib, uint16_t start, uint16_t end,
> @@ -93,7 +93,7 @@ guint gatt_read_char_by_uuid(GAttrib *attrib, uint16_t start, uint16_t end,
>        if (plen == 0)
>                return 0;
>
> -       return g_attrib_send(attrib, ATT_OP_READ_BY_TYPE_REQ,
> +       return g_attrib_send(attrib, 0, ATT_OP_READ_BY_TYPE_REQ,
>                                        pdu, plen, func, user_data, NULL);
>  }
>
> @@ -104,7 +104,7 @@ guint gatt_read_char(GAttrib *attrib, uint16_t handle, GAttribResultFunc func,
>        guint16 plen;
>
>        plen = enc_read_req(handle, pdu, sizeof(pdu));
> -       return g_attrib_send(attrib, ATT_OP_READ_REQ, pdu, plen, func,
> +       return g_attrib_send(attrib, 0, ATT_OP_READ_REQ, pdu, plen, func,
>                                                        user_data, NULL);
>  }
>
> @@ -115,7 +115,7 @@ guint gatt_write_char(GAttrib *attrib, uint16_t handle, uint8_t *value,
>        guint16 plen;
>
>        plen = enc_write_req(handle, value, vlen, pdu, sizeof(pdu));
> -       return g_attrib_send(attrib, ATT_OP_WRITE_REQ, pdu, plen, func,
> +       return g_attrib_send(attrib, 0, ATT_OP_WRITE_REQ, pdu, plen, func,
>                                                        user_data, NULL);
>  }
>
> @@ -129,7 +129,7 @@ guint gatt_find_info(GAttrib *attrib, uint16_t start, uint16_t end,
>        if (plen == 0)
>                return 0;
>
> -       return g_attrib_send(attrib, ATT_OP_FIND_INFO_REQ, pdu, plen, func,
> +       return g_attrib_send(attrib, 0, ATT_OP_FIND_INFO_REQ, pdu, plen, func,
>                                                        user_data, NULL);
>  }
>
> @@ -140,6 +140,6 @@ guint gatt_write_cmd(GAttrib *attrib, uint16_t handle, uint8_t *value, int vlen,
>        guint16 plen;
>
>        plen = enc_write_cmd(handle, value, vlen, pdu, sizeof(pdu));
> -       return g_attrib_send(attrib, ATT_OP_WRITE_CMD, pdu, plen, NULL,
> +       return g_attrib_send(attrib, 0, ATT_OP_WRITE_CMD, pdu, plen, NULL,
>                                                        user_data, notify);
>  }
> diff --git a/attrib/gattrib.c b/attrib/gattrib.c
> index 9268001..79ee2e9 100644
> --- a/attrib/gattrib.c
> +++ b/attrib/gattrib.c
> @@ -286,6 +286,7 @@ static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer data)
>        uint8_t buf[512], status;
>        gsize len;
>        GIOStatus iostat;
> +       gboolean qempty;
>
>        if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) {
>                attrib->read_watch = 0;
> @@ -333,8 +334,7 @@ static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer data)
>        status = 0;
>
>  done:
> -       if (attrib->queue && g_queue_is_empty(attrib->queue) == FALSE)
> -               wake_up_sender(attrib);
> +       qempty = attrib->queue == NULL || g_queue_is_empty(attrib->queue);
>
>        if (cmd) {
>                if (cmd->func)
> @@ -343,6 +343,9 @@ done:
>                command_destroy(cmd);
>        }
>
> +       if (!qempty)
> +               wake_up_sender(attrib);
> +
>        return TRUE;
>  }
>
> @@ -368,9 +371,9 @@ GAttrib *g_attrib_new(GIOChannel *io)
>        return g_attrib_ref(attrib);
>  }
>
> -guint g_attrib_send(GAttrib *attrib, guint8 opcode, const guint8 *pdu,
> -                               guint16 len, GAttribResultFunc func,
> -                               gpointer user_data, GDestroyNotify notify)
> +guint g_attrib_send(GAttrib *attrib, guint id, guint8 opcode,
> +               const guint8 *pdu, guint16 len, GAttribResultFunc func,
> +               gpointer user_data, GDestroyNotify notify)
Missing tab here.

>  {
>        struct command *c;
>
> @@ -386,9 +389,14 @@ guint g_attrib_send(GAttrib *attrib, guint8 opcode, const guint8 *pdu,
>        c->func = func;
>        c->user_data = user_data;
>        c->notify = notify;
> -       c->id = ++attrib->next_cmd_id;
>
> -       g_queue_push_tail(attrib->queue, c);
> +       if (id) {
> +               c->id = id;
> +               g_queue_push_head(attrib->queue, c);
> +       } else {
> +               c->id = ++attrib->next_cmd_id;
> +               g_queue_push_tail(attrib->queue, c);
> +       }
>
>        if (g_queue_get_length(attrib->queue) == 1)
>                wake_up_sender(attrib);
> diff --git a/attrib/gattrib.h b/attrib/gattrib.h
> index 0940289..1a966a7 100644
> --- a/attrib/gattrib.h
> +++ b/attrib/gattrib.h
> @@ -50,9 +50,10 @@ gboolean g_attrib_set_disconnect_function(GAttrib *attrib,
>  gboolean g_attrib_set_destroy_function(GAttrib *attrib,
>                GDestroyNotify destroy, gpointer user_data);
>
> -guint g_attrib_send(GAttrib *attrib, guint8 opcode, const guint8 *pdu,
> -                               guint16 len, GAttribResultFunc func,
> -                               gpointer user_data, GDestroyNotify notify);
> +guint g_attrib_send(GAttrib *attrib, guint id, guint8 opcode,
> +                       const guint8 *pdu, guint16 len, GAttribResultFunc func,
> +                       gpointer user_data, GDestroyNotify notify);
> +
Missing tab here.

>  gboolean g_attrib_cancel(GAttrib *attrib, guint id);
>  gboolean g_attrib_cancel_all(GAttrib *attrib);
>
> diff --git a/attrib/gatttool.c b/attrib/gatttool.c
> index a234e36..a6f92db 100644
> --- a/attrib/gatttool.c
> +++ b/attrib/gatttool.c
> @@ -272,7 +272,7 @@ static void events_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
>        olen = enc_confirmation(opdu, sizeof(opdu));
>
>        if (olen > 0)
> -               g_attrib_send(attrib, opdu[0], opdu, olen, NULL, NULL, NULL);
> +               g_attrib_send(attrib, 0, opdu[0], opdu, olen, NULL, NULL, NULL);
>  }
>
>  static gboolean listen_start(gpointer user_data)
> diff --git a/src/attrib-server.c b/src/attrib-server.c
> index cbc01ee..aee2ace 100644
> --- a/src/attrib-server.c
> +++ b/src/attrib-server.c
> @@ -694,7 +694,7 @@ done:
>        if (status)
>                length = enc_error_resp(ipdu[0], 0x0000, status, opdu, channel->mtu);
>
> -       g_attrib_send(channel->attrib, opdu[0], opdu, length,
> +       g_attrib_send(channel->attrib, 0, opdu[0], opdu, length,
>                                                        NULL, NULL, NULL);
>  }
>
> --
> 1.7.1
> --
> Brian Gix
> bgix@codeaurora.org
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
>

We need only to avoid calling g_attrib_send outside gatt.c passing id
!=0, otherwise it may break the commands sequence.
Could you please change the the attribute server adding a service with
long attributes?

Regards,
Claudio.