2011-02-16 18:13:39

by Elvis Pfutzenreuter

[permalink] [raw]
Subject: [PATCH 1/3] Add UUID property to GATT service object

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

diff --git a/attrib/client.c b/attrib/client.c
index ec5fcf2..0f9ba3e 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -941,6 +941,7 @@ static DBusMessage *prim_get_properties(DBusConnection *conn, DBusMessage *msg,
DBusMessageIter dict;
GSList *l;
char **chars;
+ const char *uuid;
int i;

reply = dbus_message_new_method_return(msg);
@@ -963,6 +964,9 @@ static DBusMessage *prim_get_properties(DBusConnection *conn, DBusMessage *msg,

dict_append_array(&dict, "Characteristics", DBUS_TYPE_OBJECT_PATH,
&chars, i);
+ uuid = prim->att->uuid;
+ dict_append_entry(&dict, "UUID", DBUS_TYPE_STRING, &uuid);
+
g_free(chars);

dbus_message_iter_close_container(&iter, &dict);
--
1.7.1



2011-02-21 21:24:18

by Elvis Pfutzenreuter

[permalink] [raw]
Subject: Re: [PATCH 2/3] Add generic descriptor support to Attribute API document


On 21 Feb 2011, at 18:14 , Claudio Takahasi wrote:

> Hi Elvis,
>
> On Mon, Feb 21, 2011 at 7:39 PM, Elvis Pf?tzenreuter <[email protected]> wrote:
>>
>> On 21 Feb 2011, at 16:14 , Claudio Takahasi wrote:
>>
>>> Hi,
>>>
>>>
>>> On Wed, Feb 16, 2011 at 6:13 PM, Elvis Pf?tzenreuter <[email protected]> wrote:
>>>> This patch proposes extensions to Attribute API, giving access to all
>>>> characteristic descriptors (beyond 'Description' and 'Format').
>>>> ---
>>>> doc/attribute-api.txt | 28 ++++++++++++++++++++++++++++
>>>> 1 files changed, 28 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/doc/attribute-api.txt b/doc/attribute-api.txt
>>>> index 23808e6..5ee189e 100644
>>>> --- a/doc/attribute-api.txt
>>>> +++ b/doc/attribute-api.txt
>>>> @@ -104,6 +104,14 @@ Methods dict GetProperties()
>>>>
>>>> Possible Errors: org.bluez.Error.InvalidArguments
>>>>
>>>> + void SetDescriptorValue(object descriptor, array{byte} value)
>>>> +
>>>> + Sets descriptor value, provided that it is writable.
>>>> +
>>>> + Possible Errors: org.bluez.Error.InvalidArguments
>>>> + org.bluez.Error.NotAuthorized
>>>> +
>>>> +
>>>> Properties string UUID [readonly]
>>>>
>>>> UUID128 of this characteristic.
>>>> @@ -143,6 +151,26 @@ Properties string UUID [readonly]
>>>> Friendly representation of the Characteristic Value
>>>> based on the format attribute.
>>>>
>>>> + dict Descriptors [readonly]
>>>> +
>>>> + List of descriptors for this characteristic.
>>>> +
>>>> + This list contains only the descriptors not already
>>>> + covered by other properties (v.g. Description, Format).
>>>> +
>>>> + Each descriptor is mapped to an unique object path,
>>>> + which is the key for the dict.
>>>> +
>>>> + Each dict value is, in turn, a dict with at least
>>>> + the following keys:
>>>> +
>>>> + {
>>>> + "UUID": string (descriptor UUID - mandatory),
>>>> + "Value": array of bytes (raw descriptor value -
>>>> + optional, shows up when value can be
>>>> + fetched)
>>>> + }
>>>> +
>>>>
>>>> Characteristic Watcher hierarchy
>>>> ===============================
>>>> --
>>>> 1.7.1
>>>>
>>>> --
>>>> 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
>>>>
>>>
>>> In my opinion is not a good approach to allow applications(D-Bus
>>> clients) to change a behavior/value of the same characteristic
>>> descriptor. bluetoothd could manage automatically writing the client
>>> characteristic configuration and server characteristic configuration
>>> based on the registered watchers and supported client profiles.
>>
>> I'm not sure I follow, but let's break it down:
>>
>> 1) Note that this API is not just for client (and server) characteristic configuration descriptors. It is good for any kind of descriptor (like Valid Range, that client is supposed to be able to read, and set the related characteristic within its limits).
>
> The API already has SetProperty and GetProperties that could expose
> the descriptors that you need.

Yes, but note that biggest part of proposal is exactly to add one more
item to be returned by GetProperties() :)

If you mean that a separate method to update descriptors is not ok, and
using SetProperty() to do that is the way, I understand.

>
>>
>> 2) Updating automatically CCC/SCC descriptors based on Watchers sounds nice. We just need to add some kind of flag to agent registering in order to tell if we want notification or indication.
>
> This is our initial ideia, but it is not written in the stone yet. We
> need to investigate a little bit more and see which/how the Profiles
> are using these characteristics descriptors.

Ok, but I think it is near perfect.

>>
>> 3) In terms of client profiles, the GATT API looks generic to me, not profile-specific. As there is a way to access characteristic values in a generic way, so there is a need to do the same for descriptors.
>
> For some descriptors I agree that we need to expose through the D-Bus
> API. However for others such as CCC and SCC I don't think that we need
> to allow the apps to change it. If necessary we will also have a
> plugin or a service specific interface, one example is Proximity
> Profile. Do you have a good use case/example in health profiles using
> CCC and SCC?

Yes, I will send you the details.

2011-02-21 21:14:15

by Claudio Takahasi

[permalink] [raw]
Subject: Re: [PATCH 2/3] Add generic descriptor support to Attribute API document

Hi Elvis,

On Mon, Feb 21, 2011 at 7:39 PM, Elvis Pfützenreuter <[email protected]> wrote:
>
> On 21 Feb 2011, at 16:14 , Claudio Takahasi wrote:
>
>> Hi,
>>
>>
>> On Wed, Feb 16, 2011 at 6:13 PM, Elvis Pfützenreuter <[email protected]> wrote:
>>> This patch proposes extensions to Attribute API, giving access to all
>>> characteristic descriptors (beyond 'Description' and 'Format').
>>> ---
>>>  doc/attribute-api.txt |   28 ++++++++++++++++++++++++++++
>>>  1 files changed, 28 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/doc/attribute-api.txt b/doc/attribute-api.txt
>>> index 23808e6..5ee189e 100644
>>> --- a/doc/attribute-api.txt
>>> +++ b/doc/attribute-api.txt
>>> @@ -104,6 +104,14 @@ Methods            dict GetProperties()
>>>
>>>                        Possible Errors: org.bluez.Error.InvalidArguments
>>>
>>> +               void SetDescriptorValue(object descriptor, array{byte} value)
>>> +
>>> +                       Sets descriptor value, provided that it is writable.
>>> +
>>> +                       Possible Errors: org.bluez.Error.InvalidArguments
>>> +                                       org.bluez.Error.NotAuthorized
>>> +
>>> +
>>>  Properties     string UUID [readonly]
>>>
>>>                        UUID128 of this characteristic.
>>> @@ -143,6 +151,26 @@ Properties         string UUID [readonly]
>>>                        Friendly representation of the Characteristic Value
>>>                        based on the format attribute.
>>>
>>> +               dict Descriptors [readonly]
>>> +
>>> +                       List of descriptors for this characteristic.
>>> +
>>> +                       This list contains only the descriptors not already
>>> +                       covered by other properties (v.g. Description, Format).
>>> +
>>> +                       Each descriptor is mapped to an unique object path,
>>> +                       which is the key for the dict.
>>> +
>>> +                       Each dict value is, in turn, a dict with at least
>>> +                       the following keys:
>>> +
>>> +                       {
>>> +                               "UUID": string (descriptor UUID - mandatory),
>>> +                               "Value": array of bytes (raw descriptor value -
>>> +                                        optional, shows up when value can be
>>> +                                        fetched)
>>> +                       }
>>> +
>>>
>>>  Characteristic Watcher hierarchy
>>>  ===============================
>>> --
>>> 1.7.1
>>>
>>> --
>>> 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
>>>
>>
>> In my opinion is not a good approach to allow applications(D-Bus
>> clients) to change a behavior/value of the same characteristic
>> descriptor. bluetoothd could manage automatically writing the client
>> characteristic configuration and server characteristic configuration
>> based on the registered watchers and supported client profiles.
>
> I'm not sure I follow, but let's break it down:
>
> 1) Note that this API is not just for client (and server) characteristic configuration descriptors. It is good for any kind of descriptor (like Valid Range, that client is supposed to be able to read, and set the related characteristic within its limits).

The API already has SetProperty and GetProperties that could expose
the descriptors that you need.

>
> 2) Updating automatically CCC/SCC descriptors based on Watchers sounds nice. We just need to add some kind of flag to agent registering in order to tell if we want notification or indication.

This is our initial ideia, but it is not written in the stone yet. We
need to investigate a little bit more and see which/how the Profiles
are using these characteristics descriptors.

>
> 3) In terms of client profiles, the GATT API looks generic to me, not profile-specific. As there is a way to access characteristic values in a generic way, so there is a need to do the same for descriptors.

For some descriptors I agree that we need to expose through the D-Bus
API. However for others such as CCC and SCC I don't think that we need
to allow the apps to change it. If necessary we will also have a
plugin or a service specific interface, one example is Proximity
Profile. Do you have a good use case/example in health profiles using
CCC and SCC?

BR,
Claudio.

2011-02-21 19:39:21

by Elvis Pfutzenreuter

[permalink] [raw]
Subject: Re: [PATCH 2/3] Add generic descriptor support to Attribute API document


On 21 Feb 2011, at 16:14 , Claudio Takahasi wrote:

> Hi,
>
>
> On Wed, Feb 16, 2011 at 6:13 PM, Elvis Pf?tzenreuter <[email protected]> wrote:
>> This patch proposes extensions to Attribute API, giving access to all
>> characteristic descriptors (beyond 'Description' and 'Format').
>> ---
>> doc/attribute-api.txt | 28 ++++++++++++++++++++++++++++
>> 1 files changed, 28 insertions(+), 0 deletions(-)
>>
>> diff --git a/doc/attribute-api.txt b/doc/attribute-api.txt
>> index 23808e6..5ee189e 100644
>> --- a/doc/attribute-api.txt
>> +++ b/doc/attribute-api.txt
>> @@ -104,6 +104,14 @@ Methods dict GetProperties()
>>
>> Possible Errors: org.bluez.Error.InvalidArguments
>>
>> + void SetDescriptorValue(object descriptor, array{byte} value)
>> +
>> + Sets descriptor value, provided that it is writable.
>> +
>> + Possible Errors: org.bluez.Error.InvalidArguments
>> + org.bluez.Error.NotAuthorized
>> +
>> +
>> Properties string UUID [readonly]
>>
>> UUID128 of this characteristic.
>> @@ -143,6 +151,26 @@ Properties string UUID [readonly]
>> Friendly representation of the Characteristic Value
>> based on the format attribute.
>>
>> + dict Descriptors [readonly]
>> +
>> + List of descriptors for this characteristic.
>> +
>> + This list contains only the descriptors not already
>> + covered by other properties (v.g. Description, Format).
>> +
>> + Each descriptor is mapped to an unique object path,
>> + which is the key for the dict.
>> +
>> + Each dict value is, in turn, a dict with at least
>> + the following keys:
>> +
>> + {
>> + "UUID": string (descriptor UUID - mandatory),
>> + "Value": array of bytes (raw descriptor value -
>> + optional, shows up when value can be
>> + fetched)
>> + }
>> +
>>
>> Characteristic Watcher hierarchy
>> ===============================
>> --
>> 1.7.1
>>
>> --
>> 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
>>
>
> In my opinion is not a good approach to allow applications(D-Bus
> clients) to change a behavior/value of the same characteristic
> descriptor. bluetoothd could manage automatically writing the client
> characteristic configuration and server characteristic configuration
> based on the registered watchers and supported client profiles.

I'm not sure I follow, but let's break it down:

1) Note that this API is not just for client (and server) characteristic configuration descriptors. It is good for any kind of descriptor (like Valid Range, that client is supposed to be able to read, and set the related characteristic within its limits).

2) Updating automatically CCC/SCC descriptors based on Watchers sounds nice. We just need to add some kind of flag to agent registering in order to tell if we want notification or indication.

3) In terms of client profiles, the GATT API looks generic to me, not profile-specific. As there is a way to access characteristic values in a generic way, so there is a need to do the same for descriptors.

2011-02-21 19:14:20

by Claudio Takahasi

[permalink] [raw]
Subject: Re: [PATCH 2/3] Add generic descriptor support to Attribute API document

SGksCgoKT24gV2VkLCBGZWIgMTYsIDIwMTEgYXQgNjoxMyBQTSwgRWx2aXMgUGbDvHR6ZW5yZXV0
ZXIgPGVweEBzaWdub3ZlLmNvbT4gd3JvdGU6Cj4gVGhpcyBwYXRjaCBwcm9wb3NlcyBleHRlbnNp
b25zIHRvIEF0dHJpYnV0ZSBBUEksIGdpdmluZyBhY2Nlc3MgdG8gYWxsCj4gY2hhcmFjdGVyaXN0
aWMgZGVzY3JpcHRvcnMgKGJleW9uZCAnRGVzY3JpcHRpb24nIGFuZCAnRm9ybWF0JykuCj4gLS0t
Cj4gwqBkb2MvYXR0cmlidXRlLWFwaS50eHQgfCDCoCAyOCArKysrKysrKysrKysrKysrKysrKysr
KysrKysrCj4gwqAxIGZpbGVzIGNoYW5nZWQsIDI4IGluc2VydGlvbnMoKyksIDAgZGVsZXRpb25z
KC0pCj4KPiBkaWZmIC0tZ2l0IGEvZG9jL2F0dHJpYnV0ZS1hcGkudHh0IGIvZG9jL2F0dHJpYnV0
ZS1hcGkudHh0Cj4gaW5kZXggMjM4MDhlNi4uNWVlMTg5ZSAxMDA2NDQKPiAtLS0gYS9kb2MvYXR0
cmlidXRlLWFwaS50eHQKPiArKysgYi9kb2MvYXR0cmlidXRlLWFwaS50eHQKPiBAQCAtMTA0LDYg
KzEwNCwxNCBAQCBNZXRob2RzIMKgIMKgIMKgIMKgIMKgIMKgZGljdCBHZXRQcm9wZXJ0aWVzKCkK
Pgo+IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgUG9zc2libGUgRXJyb3JzOiBv
cmcuYmx1ZXouRXJyb3IuSW52YWxpZEFyZ3VtZW50cwo+Cj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDC
oCB2b2lkIFNldERlc2NyaXB0b3JWYWx1ZShvYmplY3QgZGVzY3JpcHRvciwgYXJyYXl7Ynl0ZX0g
dmFsdWUpCj4gKwo+ICsgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgU2V0cyBkZXNj
cmlwdG9yIHZhbHVlLCBwcm92aWRlZCB0aGF0IGl0IGlzIHdyaXRhYmxlLgo+ICsKPiArIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIFBvc3NpYmxlIEVycm9yczogb3JnLmJsdWV6LkVy
cm9yLkludmFsaWRBcmd1bWVudHMKPiArIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIG9yZy5ibHVlei5FcnJvci5Ob3RBdXRob3JpemVkCj4g
Kwo+ICsKPiDCoFByb3BlcnRpZXMgwqAgwqAgc3RyaW5nIFVVSUQgW3JlYWRvbmx5XQo+Cj4gwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqBVVUlEMTI4IG9mIHRoaXMgY2hhcmFjdGVy
aXN0aWMuCj4gQEAgLTE0Myw2ICsxNTEsMjYgQEAgUHJvcGVydGllcyDCoCDCoCDCoCDCoCBzdHJp
bmcgVVVJRCBbcmVhZG9ubHldCj4gwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqBG
cmllbmRseSByZXByZXNlbnRhdGlvbiBvZiB0aGUgQ2hhcmFjdGVyaXN0aWMgVmFsdWUKPiDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoGJhc2VkIG9uIHRoZSBmb3JtYXQgYXR0cmli
dXRlLgo+Cj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCBkaWN0IERlc2NyaXB0b3JzIFtyZWFkb25s
eV0KPiArCj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCBMaXN0IG9mIGRlc2Ny
aXB0b3JzIGZvciB0aGlzIGNoYXJhY3RlcmlzdGljLgo+ICsKPiArIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIFRoaXMgbGlzdCBjb250YWlucyBvbmx5IHRoZSBkZXNjcmlwdG9ycyBu
b3QgYWxyZWFkeQo+ICsgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgY292ZXJlZCBi
eSBvdGhlciBwcm9wZXJ0aWVzICh2LmcuIERlc2NyaXB0aW9uLCBGb3JtYXQpLgo+ICsKPiArIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIEVhY2ggZGVzY3JpcHRvciBpcyBtYXBwZWQg
dG8gYW4gdW5pcXVlIG9iamVjdCBwYXRoLAo+ICsgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgd2hpY2ggaXMgdGhlIGtleSBmb3IgdGhlIGRpY3QuCj4gKwo+ICsgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgRWFjaCBkaWN0IHZhbHVlIGlzLCBpbiB0dXJuLCBhIGRpY3Qg
d2l0aCBhdCBsZWFzdAo+ICsgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgdGhlIGZv
bGxvd2luZyBrZXlzOgo+ICsKPiArIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIHsK
PiArIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgICJVVUlEIjog
c3RyaW5nIChkZXNjcmlwdG9yIFVVSUQgLSBtYW5kYXRvcnkpLAo+ICsgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgIlZhbHVlIjogYXJyYXkgb2YgYnl0ZXMgKHJh
dyBkZXNjcmlwdG9yIHZhbHVlIC0KPiArIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgb3B0aW9uYWwsIHNob3dzIHVwIHdoZW4gdmFsdWUg
Y2FuIGJlCj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoGZldGNoZWQpCj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCB9Cj4gKwo+Cj4gwqBDaGFyYWN0ZXJpc3RpYyBXYXRjaGVyIGhpZXJhcmNoeQo+IMKgPT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PQo+IC0tCj4gMS43LjEKPgo+IC0tCj4gVG8gdW5zdWJz
Y3JpYmUgZnJvbSB0aGlzIGxpc3Q6IHNlbmQgdGhlIGxpbmUgInVuc3Vic2NyaWJlIGxpbnV4LWJs
dWV0b290aCIgaW4KPiB0aGUgYm9keSBvZiBhIG1lc3NhZ2UgdG8gbWFqb3Jkb21vQHZnZXIua2Vy
bmVsLm9yZwo+IE1vcmUgbWFqb3Jkb21vIGluZm8gYXQgwqBodHRwOi8vdmdlci5rZXJuZWwub3Jn
L21ham9yZG9tby1pbmZvLmh0bWwKPgoKSW4gbXkgb3BpbmlvbiBpcyBub3QgYSBnb29kIGFwcHJv
YWNoIHRvIGFsbG93IGFwcGxpY2F0aW9ucyhELUJ1cwpjbGllbnRzKSB0byBjaGFuZ2UgYSBiZWhh
dmlvci92YWx1ZSBvZiB0aGUgc2FtZSBjaGFyYWN0ZXJpc3RpYwpkZXNjcmlwdG9yLiBibHVldG9v
dGhkIGNvdWxkIG1hbmFnZSBhdXRvbWF0aWNhbGx5IHdyaXRpbmcgdGhlIGNsaWVudApjaGFyYWN0
ZXJpc3RpYyBjb25maWd1cmF0aW9uIGFuZCBzZXJ2ZXIgY2hhcmFjdGVyaXN0aWMgY29uZmlndXJh
dGlvbgpiYXNlZCBvbiB0aGUgcmVnaXN0ZXJlZCB3YXRjaGVycyBhbmQgc3VwcG9ydGVkIGNsaWVu
dCBwcm9maWxlcy4KCkJSLApDbGF1ZGlvLgo=

2011-02-18 15:10:41

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH 1/3] Add UUID property to GATT service object

Hi Elvis,

On Wed, Feb 16, 2011, Elvis Pf??tzenreuter wrote:
> ---
> attrib/client.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)

This patch has been pushed upstream. For the others I'm still waiting
for some feedback from the other LE developers.

Johan

2011-02-16 18:13:40

by Elvis Pfutzenreuter

[permalink] [raw]
Subject: [PATCH 2/3] Add generic descriptor support to Attribute API document

This patch proposes extensions to Attribute API, giving access to all
characteristic descriptors (beyond 'Description' and 'Format').
---
doc/attribute-api.txt | 28 ++++++++++++++++++++++++++++
1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/doc/attribute-api.txt b/doc/attribute-api.txt
index 23808e6..5ee189e 100644
--- a/doc/attribute-api.txt
+++ b/doc/attribute-api.txt
@@ -104,6 +104,14 @@ Methods dict GetProperties()

Possible Errors: org.bluez.Error.InvalidArguments

+ void SetDescriptorValue(object descriptor, array{byte} value)
+
+ Sets descriptor value, provided that it is writable.
+
+ Possible Errors: org.bluez.Error.InvalidArguments
+ org.bluez.Error.NotAuthorized
+
+
Properties string UUID [readonly]

UUID128 of this characteristic.
@@ -143,6 +151,26 @@ Properties string UUID [readonly]
Friendly representation of the Characteristic Value
based on the format attribute.

+ dict Descriptors [readonly]
+
+ List of descriptors for this characteristic.
+
+ This list contains only the descriptors not already
+ covered by other properties (v.g. Description, Format).
+
+ Each descriptor is mapped to an unique object path,
+ which is the key for the dict.
+
+ Each dict value is, in turn, a dict with at least
+ the following keys:
+
+ {
+ "UUID": string (descriptor UUID - mandatory),
+ "Value": array of bytes (raw descriptor value -
+ optional, shows up when value can be
+ fetched)
+ }
+

Characteristic Watcher hierarchy
===============================
--
1.7.1


2011-02-16 18:13:41

by Elvis Pfutzenreuter

[permalink] [raw]
Subject: [PATCH 3/3] Implement generic descriptor access to Attribute API

---
attrib/client.c | 302 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 291 insertions(+), 11 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index 0f9ba3e..cfc8535 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -79,6 +79,15 @@ struct primary {
GSList *watchers;
};

+struct descriptor {
+ char *path;
+ uint16_t handle;
+ uuid_t uuid;
+ char type[MAX_LEN_UUID_STR + 1];
+ uint8_t *value;
+ size_t vlen;
+};
+
struct characteristic {
struct primary *prim;
char *path;
@@ -91,12 +100,14 @@ struct characteristic {
struct format *format;
uint8_t *value;
size_t vlen;
+ GSList *descriptors;
};

struct query_data {
struct primary *prim;
struct characteristic *chr;
uint16_t handle;
+ struct descriptor *descr;
};

struct watcher {
@@ -110,10 +121,22 @@ static GSList *gatt_services = NULL;

static DBusConnection *connection;

+static void descriptor_free(void *user_data)
+{
+ struct descriptor *descr = user_data;
+
+ g_free(descr->path);
+ g_free(descr->value);
+
+ g_free(descr);
+}
+
static void characteristic_free(void *user_data)
{
struct characteristic *chr = user_data;

+ g_slist_foreach(chr->descriptors, (GFunc) descriptor_free, NULL);
+
g_free(chr->path);
g_free(chr->desc);
g_free(chr->format);
@@ -188,11 +211,90 @@ static int watcher_cmp(gconstpointer a, gconstpointer b)
return g_strcmp0(watcher->path, match->path);
}

+static void add_descriptor_dict(DBusMessageIter *entry,
+ struct descriptor *descr)
+{
+ DBusMessageIter descr_dict_var;
+ DBusMessageIter descr_dict;
+ const char *uuid;
+ const uint8_t *value;
+ size_t vlen;
+
+ value = descr->value;
+ vlen = descr->vlen;
+ uuid = descr->type;
+
+ dbus_message_iter_open_container(entry, DBUS_TYPE_VARIANT,
+ DBUS_TYPE_ARRAY_AS_STRING
+ DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+ DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
+ DBUS_DICT_ENTRY_END_CHAR_AS_STRING,
+ &descr_dict_var);
+
+ dbus_message_iter_open_container(&descr_dict_var, DBUS_TYPE_ARRAY,
+ DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+ DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
+ DBUS_DICT_ENTRY_END_CHAR_AS_STRING,
+ &descr_dict);
+
+ dict_append_entry(&descr_dict, "UUID", DBUS_TYPE_STRING, &uuid);
+
+ if (value)
+ dict_append_array(&descr_dict, "Value", DBUS_TYPE_BYTE,
+ &value, vlen);
+
+ dbus_message_iter_close_container(&descr_dict_var, &descr_dict);
+ dbus_message_iter_close_container(entry, &descr_dict_var);
+}
+
+static void add_descriptors_dict(DBusMessageIter *outer_entry,
+ struct characteristic *chr)
+{
+ DBusMessageIter descrs_dict_var;
+ DBusMessageIter descrs_dict;
+ GSList *l;
+
+ dbus_message_iter_open_container(outer_entry, DBUS_TYPE_VARIANT,
+ DBUS_TYPE_ARRAY_AS_STRING
+ DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+ DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
+ DBUS_DICT_ENTRY_END_CHAR_AS_STRING,
+ &descrs_dict_var);
+
+ dbus_message_iter_open_container(&descrs_dict_var, DBUS_TYPE_ARRAY,
+ DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+ DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
+ DBUS_DICT_ENTRY_END_CHAR_AS_STRING,
+ &descrs_dict);
+
+ for (l = chr->descriptors; l; l = l->next) {
+ struct descriptor *descr = l->data;
+ const char *key;
+ DBusMessageIter entry;
+
+ key = descr->path;
+
+ dbus_message_iter_open_container(&descrs_dict,
+ DBUS_TYPE_DICT_ENTRY, NULL, &entry);
+
+ dbus_message_iter_append_basic(&entry, DBUS_TYPE_STRING, &key);
+
+ add_descriptor_dict(&entry, descr);
+
+ dbus_message_iter_close_container(&descrs_dict, &entry);
+ }
+
+ dbus_message_iter_close_container(&descrs_dict_var, &descrs_dict);
+ dbus_message_iter_close_container(outer_entry, &descrs_dict_var);
+}
+
static void append_char_dict(DBusMessageIter *iter, struct characteristic *chr)
{
DBusMessageIter dict;
+ DBusMessageIter entry;
const char *name = "";
char *uuid;
+ const char *key;

dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
@@ -214,6 +316,19 @@ static void append_char_dict(DBusMessageIter *iter, struct characteristic *chr)
dict_append_array(&dict, "Value", DBUS_TYPE_BYTE, &chr->value,
chr->vlen);

+ /* Descriptors entry */
+
+ dbus_message_iter_open_container(&dict, DBUS_TYPE_DICT_ENTRY,
+ NULL, &entry);
+
+ key = "Descriptors";
+
+ dbus_message_iter_append_basic(&entry, DBUS_TYPE_STRING, &key);
+
+ add_descriptors_dict(&entry, chr);
+
+ dbus_message_iter_close_container(&dict, &entry);
+
/* FIXME: Missing Format, Value and Representation */

dbus_message_iter_close_container(iter, &dict);
@@ -245,6 +360,19 @@ static int characteristic_set_value(struct characteristic *chr,
return 0;
}

+static int descriptor_set_value(struct descriptor *descr,
+ const uint8_t *value, size_t vlen)
+{
+ descr->value = g_try_realloc(descr->value, vlen);
+ if (descr->value == NULL)
+ return -ENOMEM;
+
+ memcpy(descr->value, value, vlen);
+ descr->vlen = vlen;
+
+ return 0;
+}
+
static void update_watchers(gpointer data, gpointer user_data)
{
struct watcher *w = data;
@@ -483,6 +611,76 @@ static DBusMessage *set_value(DBusConnection *conn, DBusMessage *msg,
return dbus_message_new_method_return(msg);
}

+static int descriptor_path_cmp(gconstpointer a, gconstpointer b)
+{
+ const struct descriptor *descr = a;
+ const char *path = b;
+
+ return strcmp(descr->path, path);
+}
+
+static struct descriptor *find_descriptor_by_path(struct characteristic *chr,
+ const char *path)
+{
+ GSList *ldescr;
+
+ ldescr = g_slist_find_custom(chr->descriptors, path,
+ descriptor_path_cmp);
+
+ if (ldescr)
+ return ldescr->data;
+
+ return NULL;
+}
+
+static int descriptor_handle_cmp(gconstpointer a, gconstpointer b)
+{
+ const struct descriptor *descr = a;
+ uint16_t handle = GPOINTER_TO_UINT(b);
+
+ return descr->handle - handle;
+}
+
+static struct descriptor *find_descriptor_by_handle(struct characteristic *chr,
+ guint handle)
+{
+ GSList *ldescr;
+
+ ldescr = g_slist_find_custom(chr->descriptors, GUINT_TO_POINTER(handle),
+ descriptor_handle_cmp);
+
+ if (ldescr)
+ return ldescr->data;
+
+ return NULL;
+}
+
+static DBusMessage *set_descriptor(DBusConnection *conn, DBusMessage *msg,
+ struct characteristic *chr, const char *path,
+ uint8_t *value, int len)
+{
+ struct gatt_service *gatt = chr->prim->gatt;
+ GError *gerr = NULL;
+ struct descriptor *descr;
+
+ descr = find_descriptor_by_path(chr, path);
+
+ if (!descr)
+ return btd_error_invalid_args(msg);
+
+ if (l2cap_connect(gatt, &gerr, FALSE) < 0) {
+ DBusMessage *reply = btd_error_failed(msg, gerr->message);
+ g_error_free(gerr);
+ return reply;
+ }
+
+ gatt_write_cmd(gatt->attrib, descr->handle, value, len, NULL, NULL);
+
+ descriptor_set_value(descr, value, len);
+
+ return dbus_message_new_method_return(msg);
+}
+
static DBusMessage *get_properties(DBusConnection *conn, DBusMessage *msg,
void *data)
{
@@ -529,10 +727,43 @@ static DBusMessage *set_property(DBusConnection *conn,
return btd_error_invalid_args(msg);
}

+static DBusMessage *set_descriptor_front(DBusConnection *conn,
+ DBusMessage *msg, void *data)
+{
+ struct characteristic *chr = data;
+ DBusMessageIter iter;
+ DBusMessageIter sub;
+ const char *descriptor_path;
+ uint8_t *value;
+ int len;
+
+ if (!dbus_message_iter_init(msg, &iter))
+ return btd_error_invalid_args(msg);
+
+ if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_OBJECT_PATH)
+ return btd_error_invalid_args(msg);
+
+ dbus_message_iter_get_basic(&iter, &descriptor_path);
+ dbus_message_iter_next(&iter);
+
+ if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_ARRAY)
+ return btd_error_invalid_args(msg);
+
+ if (dbus_message_iter_get_element_type(&iter) != DBUS_TYPE_BYTE)
+ return btd_error_invalid_args(msg);
+
+ dbus_message_iter_recurse(&iter, &sub);
+ dbus_message_iter_get_fixed_array(&sub, &value, &len);
+
+ return set_descriptor(conn, msg, chr, descriptor_path, value, len);
+}
+
static GDBusMethodTable char_methods[] = {
{ "GetProperties", "", "a{sv}", get_properties },
{ "SetProperty", "sv", "", set_property,
G_DBUS_METHOD_FLAG_ASYNC},
+ { "SetDescriptorValue", "oay", "", set_descriptor_front,
+ G_DBUS_METHOD_FLAG_ASYNC},
{ }
};

@@ -737,6 +968,26 @@ done:
g_free(current);
}

+static void update_descriptor(guint8 status, const guint8 *pdu, guint16 len,
+ gpointer user_data)
+{
+ struct query_data *current = user_data;
+ struct gatt_service *gatt = current->prim->gatt;
+ struct descriptor *descr = current->descr;
+
+ if (status != 0)
+ goto done;
+
+ if (len < 2)
+ goto done;
+
+ descriptor_set_value(descr, pdu + 1, len - 1);
+
+done:
+ g_attrib_unref(gatt->attrib);
+ g_free(current);
+}
+
static void update_char_value(guint8 status, const guint8 *pdu,
guint16 len, gpointer user_data)
{
@@ -762,6 +1013,31 @@ static void update_char_value(guint8 status, const guint8 *pdu,
g_free(current);
}

+static struct descriptor *char_add_descriptor(struct characteristic *chr,
+ uint16_t handle, uuid_t uuid)
+{
+ struct descriptor *descr;
+
+ descr = find_descriptor_by_handle(chr, handle);
+
+ if (!descr) {
+ descr = g_new0(struct descriptor, 1);
+ descr->path = g_strdup_printf("%s/descriptor%04x",
+ chr->path, handle);
+ descr->handle = handle;
+ chr->descriptors = g_slist_append(chr->descriptors, descr);
+ }
+
+ if (sdp_uuid_cmp(&uuid, &descr->uuid) != 0) {
+ char *uuidstr = bt_uuid2string(&uuid);
+ strcpy(descr->type, uuidstr);
+ g_free(uuidstr);
+ descr->uuid = uuid;
+ }
+
+ return descr;
+}
+
static int uuid_desc16_cmp(uuid_t *uuid, guint16 desc)
{
uuid_t u16;
@@ -783,7 +1059,7 @@ static void descriptor_cb(guint8 status, const guint8 *pdu, guint16 plen,
if (status != 0)
goto done;

- DBG("Find Information Response received");
+ DBG("Discover Descriptors Response received");

list = dec_find_info_resp(pdu, plen, &format);
if (list == NULL)
@@ -797,15 +1073,14 @@ static void descriptor_cb(guint8 status, const guint8 *pdu, guint16 plen,

handle = att_get_u16(info);

- if (format == 0x01) {
+ if (format == 0x01)
sdp_uuid16_create(&uuid, att_get_u16(&info[2]));
- } else {
- /* Currently, only "user description" and "presentation
- * format" descriptors are used, and both have 16-bit
- * UUIDs. Therefore there is no need to support format
- * 0x02 yet. */
+ else if (format == 0x02)
+ // FIXME use att_get_u128
+ sdp_uuid128_create(&uuid, &info[2]);
+ else
continue;
- }
+
qfmt = g_new0(struct query_data, 1);
qfmt->prim = current->prim;
qfmt->chr = current->chr;
@@ -819,8 +1094,13 @@ static void descriptor_cb(guint8 status, const guint8 *pdu, guint16 plen,
gatt->attrib = g_attrib_ref(gatt->attrib);
gatt_read_char(gatt->attrib, handle,
update_char_format, qfmt);
- } else
- g_free(qfmt);
+ } else {
+ qfmt->descr = char_add_descriptor(current->chr, handle,
+ uuid);
+ gatt->attrib = g_attrib_ref(gatt->attrib);
+ gatt_read_char(gatt->attrib, handle, update_descriptor,
+ qfmt);
+ }
}

att_data_list_free(list);
@@ -888,7 +1168,7 @@ static void char_discovered_cb(GSList *characteristics, guint8 status,
strncpy(chr->type, current_chr->uuid, sizeof(chr->type));

if (previous_end)
- *previous_end = current_chr->handle;
+ *previous_end = current_chr->handle - 1;

previous_end = &chr->end;

--
1.7.1