2011-07-21 18:19:17

by mike tsai

Subject: [PATCH] Fix Read-Not-Permit in char discovery

Hi,
This patch reads the discovered characteristic's property to
determine if Read is permitted before sending read request. This
avoids the error response from server as "read not permitted".

---
attrib/client.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index 1ef2699..f345a9e 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -850,6 +850,8 @@ static void update_all_chars(gpointer data,
gpointer user_data)
struct primary *prim = user_data;
struct gatt_service *gatt = prim->gatt;

+ if (chr->perm & ATT_CHAR_PROPER_READ)
+ {
qdesc = g_new0(struct query_data, 1);
qdesc->prim = prim;
qdesc->chr = chr;
@@ -864,8 +866,8 @@ static void update_all_chars(gpointer data,
gpointer user_data)

gatt->attrib = g_attrib_ref(gatt->attrib);
gatt_read_char(gatt->attrib, chr->handle, 0, update_char_value, qvalue);
+ }
}
-
static void char_discovered_cb(GSList *characteristics, guint8 status,
gpointer user_data)
{
--
1.7.4.1

2012-01-11 14:29:44

by Vinicius Costa Gomes

Subject: Re: [PATCH] Fix Read-Not-Permit in char discovery

Hi Johan,

On 13:28 Wed 11 Jan, Johan Hedberg wrote:
> Hi,
>
> On Thu, Jul 21, 2011, mike tsai wrote:
> > Hi,
> > This patch reads the discovered characteristic's property to
> > determine if Read is permitted before sending read request. This
> > avoids the error response from server as "read not permitted".
> >
> > ---
> > attrib/client.c | 4 +++-
> > 1 files changed, 3 insertions(+), 1 deletions(-)
> >
> > diff --git a/attrib/client.c b/attrib/client.c
> > index 1ef2699..f345a9e 100644
> > --- a/attrib/client.c
> > +++ b/attrib/client.c
> > @@ -850,6 +850,8 @@ static void update_all_chars(gpointer data,
> > gpointer user_data)
> > struct primary *prim = user_data;
> > struct gatt_service *gatt = prim->gatt;
> >
> > + if (chr->perm & ATT_CHAR_PROPER_READ)
> > + {
> > qdesc = g_new0(struct query_data, 1);
> > qdesc->prim = prim;
> > qdesc->chr = chr;
> > @@ -864,8 +866,8 @@ static void update_all_chars(gpointer data,
> > gpointer user_data)
> >
> > gatt->attrib = g_attrib_ref(gatt->attrib);
> > gatt_read_char(gatt->attrib, chr->handle, 0, update_char_value, qvalue);
> > + }
> > }
> > -
> > static void char_discovered_cb(GSList *characteristics, guint8 status,
>
> Besides the obvious coding-style issue, is this patch still valid?

Yes. The problem the patch is aimed at still exists.

But there is another possible problem with the patch, I think it still
makes sense to try to discover the descriptors associated with a characteristic
even if you are not permitted to read the Value of that characteristic.

>
> Johan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


Cheers,
--
Vinicius

2012-01-11 11:28:54

by Johan Hedberg

Subject: Re: [PATCH] Fix Read-Not-Permit in char discovery

Hi,

On Thu, Jul 21, 2011, mike tsai wrote:
> Hi,
> This patch reads the discovered characteristic's property to
> determine if Read is permitted before sending read request. This
> avoids the error response from server as "read not permitted".
>
> ---
> attrib/client.c | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/attrib/client.c b/attrib/client.c
> index 1ef2699..f345a9e 100644
> --- a/attrib/client.c
> +++ b/attrib/client.c
> @@ -850,6 +850,8 @@ static void update_all_chars(gpointer data,
> gpointer user_data)
> struct primary *prim = user_data;
> struct gatt_service *gatt = prim->gatt;
>
> + if (chr->perm & ATT_CHAR_PROPER_READ)
> + {
> qdesc = g_new0(struct query_data, 1);
> qdesc->prim = prim;
> qdesc->chr = chr;
> @@ -864,8 +866,8 @@ static void update_all_chars(gpointer data,
> gpointer user_data)
>
> gatt->attrib = g_attrib_ref(gatt->attrib);
> gatt_read_char(gatt->attrib, chr->handle, 0, update_char_value, qvalue);
> + }
> }
> -
> static void char_discovered_cb(GSList *characteristics, guint8 status,

Besides the obvious coding-style issue, is this patch still valid?

Johan