Re: [RFC 1/2] Add g_attrib_send_seq()

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

 



Hi Brian,

On Fri, Dec 17, 2010 at 7:48 PM, Brian Gix <bgix@xxxxxxxxxxxxxx> wrote:
> Add g_attrib_send_seq() as an extension to g_attrib_send().
> Â Â Â Âg_attrib_send_seq() functionally queues an entire
> Â Â Â Â"GATT Procedure" as defined in the BT Core v4.0.
> Â Â Â ÂThe intention is that the full procedure is run
> Â Â Â Âto completion before the next GATT Procedure is
> Â Â Â Âstarted. Subsequent ATT requests to a continuing
> Â Â Â Âprocedure are added to the head of the Attrib queue.
>
> Fix g_attrib_send() to be the degenerative case of g_attrib_send_seq()
> Â Â Â ÂThis function now chains to g_attrib_send_seq() with arguments
> Â Â Â Âindicating that it is *not* a compound (multi-step) GATT
> Â Â Â Âprocedure, but rather that the entire procedure is performed
> Â Â Â Âwith a single ATT opcode request/response.
>
> Fix received_data() to recognize that incoming response is (or isn't)
> Â Â Â Âpart of a compound Procedure, so that it waits for additional
> Â Â Â Ârequests before servicing the Attrib queue.
> ---
> Âattrib/gattrib.c | Â 44 ++++++++++++++++++++++++++++++++++++--------
> Âattrib/gattrib.h | Â Â4 ++++
> Â2 files changed, 40 insertions(+), 8 deletions(-)
>
> diff --git a/attrib/gattrib.c b/attrib/gattrib.c
> index eace01b..8ef5d92 100644
> --- a/attrib/gattrib.c
> +++ b/attrib/gattrib.c
> @@ -58,6 +58,7 @@ struct command {
> Â Â Â Âguint16 len;
> Â Â Â Âguint8 expected;
> Â Â Â Âgboolean sent;
> + Â Â Â gboolean compound;
> Â Â Â ÂGAttribResultFunc func;
> Â Â Â Âgpointer user_data;
> Â Â Â ÂGDestroyNotify notify;
> @@ -285,6 +286,7 @@ static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer data)
> Â Â Â Âuint8_t buf[512], status;
> Â Â Â Âgsize len;
> Â Â Â ÂGIOStatus iostat;
> + Â Â Â gboolean compound = FALSE;
Intialization is not necessary.

>
> Â Â Â Âif (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) {
> Â Â Â Â Â Â Â Âattrib->read_watch = 0;
> @@ -319,6 +321,8 @@ static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer data)
> Â Â Â Â Â Â Â Âreturn attrib->events != NULL;
> Â Â Â Â}
>
> + Â Â Â compound = cmd->compound;
> +
> Â Â Â Âif (buf[0] == ATT_OP_ERROR) {
> Â Â Â Â Â Â Â Âstatus = buf[4];
> Â Â Â Â Â Â Â Âgoto done;
> @@ -332,7 +336,8 @@ static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer data)
> Â Â Â Âstatus = 0;
>
> Âdone:
> - Â Â Â if (attrib->queue && g_queue_is_empty(attrib->queue) == FALSE)
> + Â Â Â if (!compound && attrib->queue &&
> + Â Â Â Â Â Â Â Â Â Â Â g_queue_is_empty(attrib->queue) == FALSE)
> Â Â Â Â Â Â Â Âwake_up_sender(attrib);
>
> Â Â Â Âif (cmd) {
> @@ -340,6 +345,11 @@ done:
> Â Â Â Â Â Â Â Â Â Â Â Âcmd->func(status, buf, len, cmd->user_data);
>
> Â Â Â Â Â Â Â Âcommand_destroy(cmd);
> +
> + Â Â Â Â Â Â Â if (compound && attrib->queue &&
> + Â Â Â Â Â Â Â Â Â Â Â g_queue_is_empty(attrib->queue) == FALSE)
> + Â Â Â Â Â Â Â Â Â Â Â wake_up_sender(attrib);
> +
Any chance to change the logic to avoid duplicated verification?
No mater the value of "compound" wake_up_sender() is always called. Is
the order important?

> Â Â Â Â}
>
> Â Â Â Âreturn TRUE;
> @@ -367,12 +377,16 @@ 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_seq(GAttrib *attrib, gboolean compound, guint id,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â guint8 opcode, const guint8 *pdu, guint16 len,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â GAttribResultFunc func, gpointer user_data,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â GDestroyNotify notify)
> Â{
> Â Â Â Âstruct command *c;
>
> + Â Â Â if (attrib == NULL || attrib->queue == NULL)
> + Â Â Â Â Â Â Â return 0;
> +
Is it necessary to check if queue is NULL?

> Â Â Â Âc = g_try_new0(struct command, 1);
> Â Â Â Âif (c == NULL)
> Â Â Â Â Â Â Â Âreturn 0;
> @@ -385,16 +399,30 @@ 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;
> + Â Â Â c->compound = compound;
>
> - Â Â Â 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);
> + Â Â Â Â Â Â Â if (g_queue_get_length(attrib->queue) == 1)
> + Â Â Â Â Â Â Â Â Â Â Â wake_up_sender(attrib);
> + Â Â Â }
>
> Â Â Â Âreturn c->id;
> Â}
>
> +guint g_attrib_send(GAttrib *attrib, guint8 opcode, const guint8 *pdu,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â guint16 len, GAttribResultFunc func,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â gpointer user_data, GDestroyNotify notify)
> +{
> + Â Â Â return g_attrib_send_seq(attrib, FALSE, 0, opcode,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â pdu, len, func, user_data, notify);
Coding style issue here.

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