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

[Claudio Takahasi <claudio.takahasi@xxxxxxxxxxxxx>]

Hi Brian,

On Tue, Jan 4, 2011 at 9:01 PM, Brian Gix <bgix@xxxxxxxxxxxxxx> wrote:
> 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.
No tabs here.

> ---
> Âattrib/gatt.c | Â134 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> Â1 files changed, 132 insertions(+), 2 deletions(-)
>
> diff --git a/attrib/gatt.c b/attrib/gatt.c
> index 320759f..304c2b1 100644
> --- a/attrib/gatt.c
> +++ b/attrib/gatt.c
> @@ -97,15 +97,145 @@ 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;
> + Â Â Â guint8 ref;
> +};
> +
> +static void read_long_destroy(gpointer user_data)
> +{
> + Â Â Â struct read_long_data *long_read = user_data;
> +
> + Â Â Â if (--long_read->ref)
> + Â Â Â Â Â Â Â return;
use g_atomic_int_dec_and_test

> +
> + Â Â Â 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 == ATT_ECODE_ATTR_NOT_LONG ||
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â status == ATT_ECODE_INVALID_OFFSET) {
> + Â Â Â Â Â Â Â status = 0;
> + Â Â Â Â Â Â Â goto done;
> + Â Â Â }
> +
> + Â Â Â if (status != 0 || rlen == 1)
> + Â Â Â Â Â Â Â 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;
Maybe using GByteArray will make your code simpler.

> +
> + Â Â Â if (rlen < ATT_DEFAULT_MTU)
> + Â Â Â Â Â Â Â goto done;

For now it is fine for testing. But it needs to be fixed later, MTU
can change after the MTU negotiation, the new value could be accessed
using a getsockopt, passing the value to gatt_* functions or using a
GAttrib function.

> +
> + Â Â Â 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) {
> + Â Â Â Â Â Â Â 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) {
> + Â Â Â Â Â Â Â long_read->ref++;
Use g_atomic_int_x function


> + Â Â Â Â Â Â Â 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 {
> + Â Â Â Â Â Â Â long_read->ref++;
same here.

Claudio
> + Â Â Â Â Â Â Â long_read->id = id;
> + Â Â Â }
> +
> + Â Â Â return id;
> Â}
>
> Âguint gatt_write_char(GAttrib *attrib, uint16_t handle, uint8_t *value,
> --
> 1.7.1
> --
> Brian Gix
> bgix@xxxxxxxxxxxxxx
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
>
ÿô.nlj·Ÿ®‰­†+%ŠË±é¥Šwÿº{.nlj·¥Š{±ý¶â^n‡r¡öë¨è&£ûz¹Þúzf£¢·hšˆ§~†­†Ûÿÿïÿ‘ê_èæ+v‰¨þ)ßø