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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Brian,

On Tue, Jan 4, 2011 at 9:01 PM, Brian Gix <bgix@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx
> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux