LinuxLists.cc - [PATCH bluez 0/2] Add BT 3.0+HS signalling definitions

[permalink] [raw]

Subject: Re: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Hi Chen,

On Mon, Oct 31, 2011 at 3:19 AM, Ganir, Chen <[email protected]> wrote:
>> Ok, I see a trend here :) We are now discussing about the current
>> Generic API implementation and not about doc/attribute-api.txt nor
>> your patch right? Ideally the .txt doc should always reflect the .c
>> code, but currently it is not the case. I suggest we get back to the
>> API discussion, then we can fix the code to behave like documented. Is
>> that ok?
>>
> Unless I misunderstood, the doc/attribute-api.txt describes the generic API? Or is it called somewhat else? I'm talking about the DBUS interface which enables external GATT Clients to be implemented on top of DBUS.

Yes, it is the so called "generic API", although this unofficial alias
is too generic for my taste :).

>> If it was meant to be some cache, I think it could be mentioned on the
>> API document. The reason it is only read once is implementation
>> limitation, not the API.
>>
> Ok. I understand this now. You were responding as if you were totally aware of the problematic current implementation, and this is why it was so strange and difficult to understand your arguments. Now that we agree on the fact that the property "Value" Should always be read, Can we also make sure that the Value property should be omitted from the "get_properties" response, in case there is no connection?

Note I did not actually agree the Value (as a property) should always
be read over-the-air, I was explaining it is not a cache at all, but a
"read once" value. For me it is a sort of limitation, because you
can't use it in all use cases (the same way I explained that ReadValue
method is not ok in all other use cases as well).

I would rather go with Ajay's suggestion for now, and in short term
(as your ReadValue implementation evolves) we drop Value if we indeed
see it as redundant. As long as we don't declare this API stable
(which it is not currently), I see no problem postponing the decision
of whether keeping Value or not.

> What about writing the value? Should we allow the user to set the value with either Write request/Write Command/Write signed or do we really want to keep that internal (like read/read blob) ?

Again, If I were to implement it, I would stick with the core spec
requirements, and checking Characteristic Properties and the link
encryption status for deciding which operation to use. But if you
really think it is useful to allow D-Bus clients "break" spec
requirements some times, you can provide this control. This particular
detail is not my main concern :).

>> Again, you are assuming a Core spec's "client" as a D-Bus client. This
>> may mean we have not been on the same page since long :)
>>
> Why do you think it will not work ? the DBUS for me is just a transparent transport for GATT operations. It should not have too much logic in it. The code mentioned above is also a bit problematic for me, since it hides some of the GATT functionality, and I'm not sure I'm still comfortable with this kind of encapsulation. Why do you think allowing the user to set the security level correctly before reading, or after getting the ATT_ECODE_INSUFF_ENC is wrong ?

If you allow "application A" to set seclevel by itself, it will affect
any other applications (on the same host) which share the same
connection, but are working with other services. Now some may say this
is not common, but it is, if you are able to see IOP results on BT SIG
mailing lists , you can see some interesting combinations of profiles.

On the other hand, if BlueZ takes care of this, it will do what is
best to keep all apps happy.

> What do we benefit from hiding so much from the DBUS user who implements the profile? As I said before, I would rather let the DBUS client implementing the profile as much flexibility and control, to prevent future major changes to API when we realize we forgot something important.

And here I can argue that, on the other hand, giving "full control"
means you need to keep that "complete" API for a long time, and if you
think you made a mistake allowing applications to do that much, you
cannot go back. Extending the API , on the other hand, is easier.

Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

2011-10-31 10:47:54

[permalink] [raw]

Subject: Re: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Hi Ajay,

On Mon, Oct 31, 2011 at 2:38 AM, Ajay Pillai <[email protected]> wrote:
> Hi Anderson,
>
>> What if this value changes on the server itself, and notification is not enabled? The client will need a way to poll the real time >values, and not use a stale or cached value which is a big mistake here.
>
>>(This is not how it works on current implementation, but we already
> know that right?) You just keep reading the Value property in some
> loop in your code:
>
>>while <blah>:
>> ? ?read-Value-property
>
>>If the connection is up, the ATT commands will be sent as soon as
>>possible. If not, connection is triggered, and then the ATT read
>>request/command is sent, and then PropertyChanged signal (should be)
>>sent.
>
> The problem I see with this approach is that the DBUS client does not get to know that the value that it got while doing a getProperty() is
> the value remembered by BlueZ and not the one directly read from the server at that time and hence potentially a stale value. I think it is important because some DBUS clients may just want to ignore any potentially stale value and not process it until it gets a "Property changed" signal. Now in that case, the DBUS client may have to handle the situation where it may never get a "Property Changed" signal (because the connection did not go through), but there would be app specific ways of handling it and it may or may not involve just letting the user know the hard truth.
> Hence I think it is crucial to let the DBUS App know what kind of value it is getting inorder to empower it to make decisions.

You are absolutely right here.

> One alternative would be to let the "value" property always read from cache and a "readValue" API always read from a connection, so that the DBUS application can decide which DBUS operation to use based on its usecase. The "readValue" API must return the value if the connection is active, or else return a status code indicating that connection procedure has been triggered and later (once connection and read procedures are over) emit a signal (maybe "propertyChanged") to signal the read value.

Looks a sane option to me. And it would allow the problem of doing
potentially unnecessary ATT requests while issuing GetProperties(). I
think a Value property is still useful for "read once on every
connection." values such as Tx Power. But both can work without
conflict.

Best Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

2011-10-31 08:04:58

by Ajay Pillai

[permalink] [raw]

Subject: RE: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Hi Chen,

>Can you please try and think of a situation where a client may need the old value, knowing that it may not be the correct one? Why would >anyone needing a value from the server need a cached value which has no meaning at the time of the "get_properties", knowing also that it >should wait for the "property_changed" event ?

I am not sure if we should be making assumptions about if such a "lazy read" would be of use to DBUS Apps. What I am trying to convey is that a design that supports both "lazy reads"(current approach suggested by Anderson which can potentially lead to less over-the-air operations, in multi-App scenarios) and "precise reads" (as described by ReadValue method) would cater to a wider set of usecases.

This also has potential to simplify quite a lot of "Clients" while dealing with servers that do not require polling. The Client just need to write the Client Characteristics Config and just forget about handling notifications or Indications, but just go on reading the value property whenever it needs it, trusting BlueZ to handle the notifications or indications and keeping the "value" property updated. However this would require a change in the way connections are managed as the DBUS Apps would need to be given more control over connections (something like reference counting of connections).

>This design is totally wrong, and may cause problems. I really see no reason for keeping the cached value. The correct behavior can be one >of the two :
>1. ReadValue method, blocking if a connection is up, returning the value, or returning some kind of error to let the client know that a >value can not be read at the moment if the link is down or pending.
>2. ReadValue method invokes a procedure for reading the value. A ReadValueComplete signal/watcher method is then raised when the value was >actually read from the server (connection is successful).

>These should replace the "Value" property, removing it from the list.

>I would prefer method 1, since it's more straight forward, and allows the client to know exactly what is going on.

I see method 2 as just an extension of method 1 where the API behaves likes method1 if the connection persists and if the connection is not present, then it lets the client know (as method1) and then moves on to method2.

Tank you
ajay

Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at http://www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at http://www.csr.com/blog

2011-10-31 07:19:14

[permalink] [raw]

Subject: RE: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Anderson,

> Hi Chen,
>
> We have been talking about consistence inside a single host. What has
> another "xyz" stack to do with making sure apps on the *same host* can
> run together and even access the same services?
>
I agree. Let's keep it simple for now.

> (This is not how it works on current implementation, but we already
> know that right?) You just keep reading the Value property in some
> loop in your code:
>
> while <blah>:
> read-Value-property
>
> If the connection is up, the ATT commands will be sent as soon as
> possible. If not, connection is triggered, and then the ATT read
> request/command is sent, and then PropertyChanged signal (should be)
> sent.
>
That's ok with me. We need to implement it in the DBUS API. We need to try and read the char value and return it as part of the response.

>
> Anyway, in Proximity, the characteristic values are not supposed to be
> polled (TX Power, for instance, should only read it once on each
> connection). So at least for Value reads, it is not useful as example.
>
I agree. Proximity is a bad example. Health thermometer on the other hand is a good example for the need to poll the value.

> Same reason as above. The rationale I gave you is based on how attio
> connection callbacks work, if the Generic API does not use it yet for
> reading values, surely this rationale does not apply to the current
> implementation. We are still discussing the API, right?

Yes, we are. I'm just trying to figure out how it's currently implemented

> Ok, I see a trend here :) We are now discussing about the current
> Generic API implementation and not about doc/attribute-api.txt nor
> your patch right? Ideally the .txt doc should always reflect the .c
> code, but currently it is not the case. I suggest we get back to the
> API discussion, then we can fix the code to behave like documented. Is
> that ok?
>
Unless I misunderstood, the doc/attribute-api.txt describes the generic API? Or is it called somewhat else? I'm talking about the DBUS interface which enables external GATT Clients to be implemented on top of DBUS.

>
> If it was meant to be some cache, I think it could be mentioned on the
> API document. The reason it is only read once is implementation
> limitation, not the API.
>
Ok. I understand this now. You were responding as if you were totally aware of the problematic current implementation, and this is why it was so strange and difficult to understand your arguments. Now that we agree on the fact that the property "Value" Should always be read, Can we also make sure that the Value property should be omitted from the "get_properties" response, in case there is no connection? What about writing the value? Should we allow the user to set the value with either Write request/Write Command/Write signed or do we really want to keep that internal (like read/read blob) ?

>
> It seems you are mapping the Core Spec's "client" definition as a
> D-Bus client. This will *not* work. A client (as per Core spec) is
> BlueZ. If it it provides external D-Bus API, it is out of scope for
> the specification.
>
> BTW, it seems the client.c code already do something like that
> (update_char_value()):
>
> else if (status == ATT_ECODE_INSUFF_ENC) {
> GIOChannel *io = g_attrib_get_channel(gatt->attrib);
>
> if (bt_io_set(io, BT_IO_L2CAP, NULL,
> BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_HIGH,
> BT_IO_OPT_INVALID)) {
> gatt_read_char(gatt->attrib, chr->handle, 0,
> update_char_value, current);
> return;
> }
> }
>
> But for sure it will require review after we agree on how to handle
> this.
>
>
> Again, you are assuming a Core spec's "client" as a D-Bus client. This
> may mean we have not been on the same page since long :)
>
Why do you think it will not work ? the DBUS for me is just a transparent transport for GATT operations. It should not have too much logic in it. The code mentioned above is also a bit problematic for me, since it hides some of the GATT functionality, and I'm not sure I'm still comfortable with this kind of encapsulation. Why do you think allowing the user to set the security level correctly before reading, or after getting the ATT_ECODE_INSUFF_ENC is wrong ? What do we benefit from hiding so much from the DBUS user who implements the profile? As I said before, I would rather let the DBUS client implementing the profile as much flexibility and control, to prevent future major changes to API when we realize we forgot something important.

Best regards,
Chen Ganir.

2011-10-31 06:57:37

[permalink] [raw]

Subject: RE: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Ajay,

> The problem I see with this approach is that the DBUS client does not
> get to know that the value that it got while doing a getProperty() is
> the value remembered by BlueZ and not the one directly read from the
> server at that time and hence potentially a stale value. I think it is
> important because some DBUS clients may just want to ignore any
> potentially stale value and not process it until it gets a "Property
> changed" signal. Now in that case, the DBUS client may have to handle
> the situation where it may never get a "Property Changed" signal
> (because the connection did not go through), but there would be app
> specific ways of handling it and it may or may not involve just letting
> the user know the hard truth.
> Hence I think it is crucial to let the DBUS App know what kind of value
> it is getting inorder to empower it to make decisions.
>
> One alternative would be to let the "value" property always read from
> cache and a "readValue" API always read from a connection, so that the
> DBUS application can decide which DBUS operation to use based on its
> usecase. The "readValue" API must return the value if the connection is
> active, or else return a status code indicating that connection
> procedure has been triggered and later (once connection and read
> procedures are over) emit a signal (maybe "propertyChanged") to signal
> the read value.
>

Can you please try and think of a situation where a client may need the old value, knowing that it may not be the correct one? Why would anyone needing a value from the server need a cached value which has no meaning at the time of the "get_properties", knowing also that it should wait for the "property_changed" event ? This design is totally wrong, and may cause problems. I really see no reason for keeping the cached value. The correct behavior can be one of the two :
1. ReadValue method, blocking if a connection is up, returning the value, or returning some kind of error to let the client know that a value can not be read at the moment if the link is down or pending.
2. ReadValue method invokes a procedure for reading the value. A ReadValueComplete signal/watcher method is then raised when the value was actually read from the server (connection is successful).

These should replace the "Value" property, removing it from the list.

I would prefer method 1, since it's more straight forward, and allows the client to know exactly what is going on.

BR
Chen Ganir.

2011-10-31 06:38:47

by Ajay Pillai

[permalink] [raw]

Subject: RE: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Hi Anderson,

> What if this value changes on the server itself, and notification is not enabled? The client will need a way to poll the real time >values, and not use a stale or cached value which is a big mistake here.

>(This is not how it works on current implementation, but we already
know that right?) You just keep reading the Value property in some
loop in your code:

>while <blah>:
> read-Value-property

>If the connection is up, the ATT commands will be sent as soon as
>possible. If not, connection is triggered, and then the ATT read
>request/command is sent, and then PropertyChanged signal (should be)
>sent.

The problem I see with this approach is that the DBUS client does not get to know that the value that it got while doing a getProperty() is
the value remembered by BlueZ and not the one directly read from the server at that time and hence potentially a stale value. I think it is important because some DBUS clients may just want to ignore any potentially stale value and not process it until it gets a "Property changed" signal. Now in that case, the DBUS client may have to handle the situation where it may never get a "Property Changed" signal (because the connection did not go through), but there would be app specific ways of handling it and it may or may not involve just letting the user know the hard truth.
Hence I think it is crucial to let the DBUS App know what kind of value it is getting inorder to empower it to make decisions.

One alternative would be to let the "value" property always read from cache and a "readValue" API always read from a connection, so that the DBUS application can decide which DBUS operation to use based on its usecase. The "readValue" API must return the value if the connection is active, or else return a status code indicating that connection procedure has been triggered and later (once connection and read procedures are over) emit a signal (maybe "propertyChanged") to signal the read value.

Thank you
ajay

Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at http://www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at http://www.csr.com/blog

2011-10-30 18:02:27

[permalink] [raw]

Subject: Re: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Hi Chen,

On Sun, Oct 30, 2011 at 11:48 AM, Ganir, Chen <[email protected]> wrote:
>> > What happens if this value is written from another client ?
>>
>> A property changed signal is emitted, and any client listening to it
>> will know of the new value written to the server. This can be good for
>> consistency between apps looking at the same service IMHO.
>>
> Anderson - what if this value changes from another client from another device?

We have been talking about consistence inside a single host. What has
another "xyz" stack to do with making sure apps on the *same host* can
run together and even access the same services?

> What if this value changes on the server itself, and notification is not enabled? The client will need a way to poll the real time values, and not use a stale or cached value which is a big mistake here.

(This is not how it works on current implementation, but we already
know that right?) You just keep reading the Value property in some
loop in your code:

while <blah>:
read-Value-property

If the connection is up, the ATT commands will be sent as soon as
possible. If not, connection is triggered, and then the ATT read
request/command is sent, and then PropertyChanged signal (should be)
sent.

>> Polling is still possible, you simply keep reading the Value property
>> periodically. If the connection is up, the value comes in "real time".
>> If not, bluez will arrange re-connection in background.
>>
> This is not reflected in the generic API at all. Maybe this functionality is also missing and only present in the proximity profile ?

Taking a look at the current generic API source code, you are right,
it is only happening at Value writes, not read.

Anyway, in Proximity, the characteristic values are not supposed to be
polled (TX Power, for instance, should only read it once on each
connection). So at least for Value reads, it is not useful as example.

>
>> > I could not find in the upstream code any other gatt_read_char ,
>> except for when connection is established.
>>
>> It only makes sense (and only works) if there is a connection up
>> right? If the connection is already up, the "connected" callback will
>> be called immediately (sort of, it may be called on next main loop
>> iteration).
>>
> I don't really see how this makes sense. If the connection is already up, is it forbidden to re-read the value directly from the server ? I did not see in the generic code "get_properties" function any reference to att_io functionality at all -the function just reads the chr variable, and returnes dbus message with the current properties. No attempt to re-read the value is even made.

Same reason as above. The rationale I gave you is based on how attio
connection callbacks work, if the Generic API does not use it yet for
reading values, surely this rationale does not apply to the current
implementation. We are still discussing the API, right?

>> This means: if you register a *intention* to use the connection, and
>> the link is already up, the connected callback is called "immediately"
>> (on next GLIB mainloop iteration).
>>
> I only wrote this since the only possibility available currently for the generic GATT API to read a value from the server is to reconnect. Otherwise, the gatt_read_... function is never called.

Ok, I see a trend here :) We are now discussing about the current
Generic API implementation and not about doc/attribute-api.txt nor
your patch right? Ideally the .txt doc should always reflect the .c
code, but currently it is not the case. I suggest we get back to the
API discussion, then we can fix the code to behave like documented. Is
that ok?

>> Simply because SetProperty() modifies the property, and
>> PropertyChanged() notifies property changes, this way you can notify
>> other clients that a new value has been written on the peer device, as
>> well as the "writer" client. Other clients can ignore the signal if
>> they are not interested on it.
>>
> Other clients running on the same host, connected to the same DBUS (assuming it actually implements this, which currently it does not). If the other client is running on a separate device, it will not get any signal from it's dbus, and will only wait for notification/indication or simply poll periodicall for the value.

I honestly don't see why this matters. We have been talking about
BlueZ consistency on the same host (two or more applications using the
same API), when did another device enter the game?

>> Again, assuming this can be done just like Proximity Monitor, what are
>> your concerns about it?
>>
> Still, I do not think that returning a cached/stale value is the correct way. You intentionally return a bad value, which you know may no longer be the correct one, and risk a client taking decisions upon your assumption that it's ok to return a bad value. Take the health thermometer example again - if the connection is down - the client MUST know about it, and the value should not be returned until it is read from the server again.

If PropertyChanged() were emitted, there would be no such "cache".
Even now, it is not a cache. It is a "read once" behavior, which of
course is inappropriate for any dynamic service (most are dynamic).

If it was meant to be some cache, I think it could be mentioned on the
API document. The reason it is only read once is implementation
limitation, not the API.

>> In my opinion, giving too much control to all clients may risk they
>> not being able to co-operate, making the D-Bus API any better than
>> letting all apps read/write on bluetooth sockets directly.
> I'd like to allow clients to do it because the GATT profile spec allows you to use ANY GATT Client API's such as write with or without response, write a value when the link is not authenticated, or do whatever you like. It's not like opening a socket (GATT or L2cap - same result). GATT Profile is a high level profile, not like the ATT protocol. Taking control of everything, leaving the client no choice is the best practice to avoid problems, but its also the best way to frustrate them when the need to do something which you did not think of at first place. I think making the API a bit more flexible is not going to strain the DBUS or do any harm.

It will simply allow apps to (intentionally or not) mess with each
other, on the *same host* (just to be clear). But I think I repeated
this too much already :)

Also if the spec says "... shall not ..." I think we should really
consider not allowing it on the (D-Bus) API as well.

>> Yes, this is one point where we can surely improve. The current
>> behavior was simply "cloned" from BR/EDR.
>>
>> I think one of the patches Andre Guedes has been working on is a first
>> step on this (actually it is a bug fix to an issue found on UPF where
>> ATT requests made during SMP pairing may be lost), and we should
>> probably send it this week. But we still need to implement support on
>> ATT layer to increase security level when necessary.
>>
> ATT layer does not need to do such a thing ! This is the responsibility of the profile client, and no one elses. The spec specifically defines the ATT error codes regarding the security , and how clients should handle those errors.

It seems you are mapping the Core Spec's "client" definition as a
D-Bus client. This will *not* work. A client (as per Core spec) is
BlueZ. If it it provides external D-Bus API, it is out of scope for
the specification.

BTW, it seems the client.c code already do something like that
(update_char_value()):

else if (status == ATT_ECODE_INSUFF_ENC) {
GIOChannel *io = g_attrib_get_channel(gatt->attrib);

if (bt_io_set(io, BT_IO_L2CAP, NULL,
BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_HIGH,
BT_IO_OPT_INVALID)) {
gatt_read_char(gatt->attrib, chr->handle, 0,
update_char_value, current);
return;
}
}

But for sure it will require review after we agree on how to handle this.

>> I think that, if the spec only allows certain operations on certain
>> conditions (see the "shall only"), the D-Bus should comply to this.
>> This may help a lot when qualifying BlueZ based products against the
>> Core spec. If apps want full control, again, they can use the socket
>> based API (you can even still use the GATT and ATT functions, just
>> like gatttool).
>>
> Anderson - you keep mentioning the socket API - what is that exactly ? Where is it documented? How do we work with it ?

It is the API used by attrib/client.c itself and attrib/gatttool.c. No
documentation AFAIK.

> D-BUS is not the client here. It should not make decisions of a client. The client is the D-BUS user, which will try to build profiles on top of that API according to the specs.

Again, you are assuming a Core spec's "client" as a D-Bus client. This
may mean we have not been on the same page since long :)

>> If you have any interest on Reliable Writes, feel free to propose some
>> API, so we can discuss :).
>>
> Making the write command a generic one as I suggested on my RFC is exactly that, unifying all the write API's into a single call, with parameters.

For Reliable Writes it should simply not work for all its use cases.
It is not atomic as your proposal suggests.

Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

2011-10-30 15:48:07

[permalink] [raw]

Subject: RE: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Anderson,

> Sorry, my mistake. This is actually how the Proximity Monitor API
> works, not the Generic API. Look for emit_property_changed() in
> proximity/monitor.c. We haven't touched the Generic API in a while
> (last big change was on July).
>
> Assuming this could work just like Proximity Monitor, do you see any
> issues on this approach?
>
Ok, so there are things missing in the generic API behavior.

> > What happens if this value is written from another client ?
>
> A property changed signal is emitted, and any client listening to it
> will know of the new value written to the server. This can be good for
> consistency between apps looking at the same service IMHO.
>
Anderson - what if this value changes from another client from another device? What if this value changes on the server itself, and notification is not enabled? The client will need a way to poll the real time values, and not use a stale or cached value which is a big mistake here.

>
> Polling is still possible, you simply keep reading the Value property
> periodically. If the connection is up, the value comes in "real time".
> If not, bluez will arrange re-connection in background.
>
This is not reflected in the generic API at all. Maybe this functionality is also missing and only present in the proximity profile ?

> > I could not find in the upstream code any other gatt_read_char ,
> except for when connection is established.
>
> It only makes sense (and only works) if there is a connection up
> right? If the connection is already up, the "connected" callback will
> be called immediately (sort of, it may be called on next main loop
> iteration).
>
I don't really see how this makes sense. If the connection is already up, is it forbidden to re-read the value directly from the server ? I did not see in the generic code "get_properties" function any reference to att_io functionality at all -the function just reads the chr variable, and returnes dbus message with the current properties. No attempt to re-read the value is even made.

>
> There is no restriction or requirement on notifications/indications
> here. Notifications/indications are solely handled by the
> RegisterCharacteristicsWatcher() callback.
>
> PropertyChanged() has to be emitted when bluez is finally able to
> *read* the value (after restoring link). But as I said on first
> paragraph, this is currently only implemented in Proximity API, sorry.
>
> > What if the profile requires you to poll ? For example, the health
> thermometer notification/indication is only optional. What will a
> client do if it needs to poll for a new value? Should it disconnect and
> reconnect ?
>
> No. You seem to be confused about the semantics of attio connection
> callbacks. See btd_device_add_attio_callback() in src/device.c, it
> has:
>
> if (device->attrib && cfunc) {
> ...
> g_idle_add(notify_attios, device);
> ...
>
> and notify_attios() has:
>
> ...
> g_slist_foreach(device->attios_offline, attio_connected, device-
> >attrib);
> ...
>
> This means: if you register a *intention* to use the connection, and
> the link is already up, the connected callback is called "immediately"
> (on next GLIB mainloop iteration).
>
I only wrote this since the only possibility available currently for the generic GATT API to read a value from the server is to reconnect. Otherwise, the gatt_read_... function is never called.

>
> Simply because SetProperty() modifies the property, and
> PropertyChanged() notifies property changes, this way you can notify
> other clients that a new value has been written on the peer device, as
> well as the "writer" client. Other clients can ignore the signal if
> they are not interested on it.
>
Other clients running on the same host, connected to the same DBUS (assuming it actually implements this, which currently it does not). If the other client is running on a separate device, it will not get any signal from it's dbus, and will only wait for notification/indication or simply poll periodicall for the value.

>
> Again, assuming this can be done just like Proximity Monitor, what are
> your concerns about it?
>
Still, I do not think that returning a cached/stale value is the correct way. You intentionally return a bad value, which you know may no longer be the correct one, and risk a client taking decisions upon your assumption that it's ok to return a bad value. Take the health thermometer example again - if the connection is down - the client MUST know about it, and the value should not be returned until it is read from the server again.

>
> In this particular case, I don't see why allow the user to use any
> write method, if the properties or security mode does not allow it as
> per spec. If the user wants full control, why not simply open a socket
> and send commands, like gatttool does?
>
> In my opinion, giving too much control to all clients may risk they
> not being able to co-operate, making the D-Bus API any better than
> letting all apps read/write on bluetooth sockets directly.
I'd like to allow clients to do it because the GATT profile spec allows you to use ANY GATT Client API's such as write with or without response, write a value when the link is not authenticated, or do whatever you like. It's not like opening a socket (GATT or L2cap - same result). GATT Profile is a high level profile, not like the ATT protocol. Taking control of everything, leaving the client no choice is the best practice to avoid problems, but its also the best way to frustrate them when the need to do something which you did not think of at first place. I think making the API a bit more flexible is not going to strain the DBUS or do any harm.

>
> >> I can't find such "authenticated write".
> >>
> > Authenticated write is write on an encrypted link. In GATT /GAP LE
> specs, it is not required to establish a link and immediately raise the
> security level. It is assumed that the client can read the ATT error
> code, and figure out that the write needs encryption, so the client may
> initiate security mode change. Currently, we force the link security
> mode upgrade on connection according to bond state.
>
> Yes, this is one point where we can surely improve. The current
> behavior was simply "cloned" from BR/EDR.
>
> I think one of the patches Andre Guedes has been working on is a first
> step on this (actually it is a bug fix to an issue found on UPF where
> ATT requests made during SMP pairing may be lost), and we should
> probably send it this week. But we still need to implement support on
> ATT layer to increase security level when necessary.
>
ATT layer does not need to do such a thing ! This is the responsibility of the profile client, and no one elses. The spec specifically defines the ATT error codes regarding the security , and how clients should handle those errors.

>
> I think that, if the spec only allows certain operations on certain
> conditions (see the "shall only"), the D-Bus should comply to this.
> This may help a lot when qualifying BlueZ based products against the
> Core spec. If apps want full control, again, they can use the socket
> based API (you can even still use the GATT and ATT functions, just
> like gatttool).
>
Anderson - you keep mentioning the socket API - what is that exactly ? Where is it documented? How do we work with it ?
D-BUS is not the client here. It should not make decisions of a client. The client is the D-BUS user, which will try to build profiles on top of that API according to the specs.

>
> I hope you agree the current "generic" Attribute API, as is, is not
> complete nor stable. Marking it "stable", as it is, will certainly not
> help app developers. I hope you guys keep contributing to it until we
> can jointly mark it as stable.
>
> If you have any interest on Reliable Writes, feel free to propose some
> API, so we can discuss :).
>
Making the write command a generic one as I suggested on my RFC is exactly that, unifying all the write API's into a single call, with parameters.

Thanks,
Chen.

2011-10-30 14:23:09

[permalink] [raw]

Subject: Re: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Hi Chen,

On Sun, Oct 30, 2011 at 2:56 AM, Ganir, Chen <[email protected]> wrote:
> Anderson,
>
>> The PropertyChanged signal will be emitted once the value is written.
>> Applications should listen to this signal if they want to know when/if
>> the value has been updated on peer device. So if the application has
>> some requirement to have the value written in, let's say, 20 seconds,
>> it starts a timer when doing a SetProperty(). If the timer expires and
>> PropertyChanged was not received, the value can be considered as "not
>> written yet". It is up to the application what to do later.
>>
> I could not find this in the upstream code. Can you please point me to where this happens?

Sorry, my mistake. This is actually how the Proximity Monitor API
works, not the Generic API. Look for emit_property_changed() in
proximity/monitor.c. We haven't touched the Generic API in a while
(last big change was on July).

Assuming this could work just like Proximity Monitor, do you see any
issues on this approach?

> What happens if this value is written from another client ?

A property changed signal is emitted, and any client listening to it
will know of the new value written to the server. This can be good for
consistency between apps looking at the same service IMHO.

> What happens if this value is
> changed by the server at some unknown time, and the client is responsible for updating this value occasionally by polling the server ?

Polling is still possible, you simply keep reading the Value property
periodically. If the connection is up, the value comes in "real time".
If not, bluez will arrange re-connection in background.

> I could not find in the upstream code any other gatt_read_char , except for when connection is established.

It only makes sense (and only works) if there is a connection up
right? If the connection is already up, the "connected" callback will
be called immediately (sort of, it may be called on next main loop
iteration).

>> The value *can* only be updated once the connection is established. a
>> SetProperty() may trigger this connection. But it will *not* block
>> waiting for the connection establishment (which may be between "now"
>> and "never"). You monitor the PropertyChanged signal to know when/if
>> the value was written.
>>
> Again, you force a specific scenario, where you believe that the server should notify/indicate the client of any changes. What if not ?

There is no restriction or requirement on notifications/indications
here. Notifications/indications are solely handled by the
RegisterCharacteristicsWatcher() callback.

PropertyChanged() has to be emitted when bluez is finally able to
*read* the value (after restoring link). But as I said on first
paragraph, this is currently only implemented in Proximity API, sorry.

> What if the profile requires you to poll ? For example, the health thermometer notification/indication is only optional. What will a client do if it needs to poll for a new value? Should it disconnect and reconnect ?

No. You seem to be confused about the semantics of attio connection
callbacks. See btd_device_add_attio_callback() in src/device.c, it
has:

if (device->attrib && cfunc) {
...
g_idle_add(notify_attios, device);
...

and notify_attios() has:

...
g_slist_foreach(device->attios_offline, attio_connected, device->attrib);
...

This means: if you register a *intention* to use the connection, and
the link is already up, the connected callback is called "immediately"
(on next GLIB mainloop iteration).

>> The value update is started by the time SetProperty() is called. But
>> it is not guaranteed to finish by the time it returns. You should
>> listen to the PropertyChanged signal for that.
>>
> I don't get why you bind setProperty with value change here.

Simply because SetProperty() modifies the property, and
PropertyChanged() notifies property changes, this way you can notify
other clients that a new value has been written on the peer device, as
well as the "writer" client. Other clients can ignore the signal if
they are not interested on it.

>> The "update process" is started by GetProperties(), but it will not
>> block. you have PropertiesChanged for knowing then the "fresh" value
>> has been read.
>>
> I could not find this either in the code.

Same reason as on my first paragraph. I confess I didn't look at the
client.c code lately, and had proximity/monitor.c in mind.

> Nothing here does any of the things you suggest. If I'm wrong, let me know where it does that.

Again, assuming this can be done just like Proximity Monitor, what are
your concerns about it?

>> You need to ask this for the guys who implemented the concept :) But
>> AFAIK it is so to avoid too many D-Bus round trips.
>>
> So to avoid too many round trips I need to get all the information at once ? Characteristic Properties are values that do not change frequently. Char properties, write permissions for example are set and fixed. Value however is changing all the time. I do not see a reason to put all those together - I do not need to get the write modes each time I get a value.

Sorry, I think this thread is splitting into too many discussions.
Feel free to create separate threads if you want to propose any
changes on this regard, maybe others with more experience on D-Bus can
give better answers (they might not be reading all this thread). But
this is what I remember when asking about similar thing in the past.

>> > 3. Writing to the value property needs to be done in one of 4
>> methods. How do you support that in the existing bluez property format
>> ?
>>
>> Based on what the characteristic supports, and the state of the link.
>> See below.
>>
> So again, we take the easy way to decide what's best for the user, instead of giving him control.

In this particular case, I don't see why allow the user to use any
write method, if the properties or security mode does not allow it as
per spec. If the user wants full control, why not simply open a socket
and send commands, like gatttool does?

In my opinion, giving too much control to all clients may risk they
not being able to co-operate, making the D-Bus API any better than
letting all apps read/write on bluetooth sockets directly.

>> I can't find such "authenticated write".
>>
> Authenticated write is write on an encrypted link. In GATT /GAP LE specs, it is not required to establish a link and immediately raise the security level. It is assumed that the client can read the ATT error code, and figure out that the write needs encryption, so the client may initiate security mode change. Currently, we force the link security mode upgrade on connection according to bond state.

Yes, this is one point where we can surely improve. The current
behavior was simply "cloned" from BR/EDR.

I think one of the patches Andre Guedes has been working on is a first
step on this (actually it is a bug fix to an issue found on UPF where
ATT requests made during SMP pairing may be lost), and we should
probably send it this week. But we still need to implement support on
ATT layer to increase security level when necessary.

BTW, we already support Security Request if they come from a slave.

>
>>
>> "This sub-procedure is used to write a Characteristic Value to a server
>> when the
>> client knows the Characteristic Value Handle and the ATT Bearer is not
>> encrypted. This sub-procedure shall only be used if the Characteristic
>> Proper-
>> ties authenticated bit is enabled and the client and server device
>> share a bond
>> as defined in [Vol. 3] Part C, Generic Access Profile."
>>
>> Do you see that all necessary information to know when to use it or
>> not is already known by BlueZ?
>>
> Yes. I see that. However, I'm not sure we need to take control over decisions like this.

I think that, if the spec only allows certain operations on certain
conditions (see the "shall only"), the D-Bus should comply to this.
This may help a lot when qualifying BlueZ based products against the
Core spec. If apps want full control, again, they can use the socket
based API (you can even still use the GATT and ATT functions, just
like gatttool).

>> To me, it seems support for Reliable Writes require a separate set of
>> functions, if we want to support both usecases listed on the spec.
>>
> So we need to be prepared, and make sure the API stays and does not change too much when we do add support for this type of write.

I hope you agree the current "generic" Attribute API, as is, is not
complete nor stable. Marking it "stable", as it is, will certainly not
help app developers. I hope you guys keep contributing to it until we
can jointly mark it as stable.

If you have any interest on Reliable Writes, feel free to propose some
API, so we can discuss :).

>> Note that there is a condition, for "optional" on server. The server
>> cannot add/remove services during its lifetime, or at least cannot
>> change the attribute handles. We don't have this guarantee as well, as
>> handles are allocated dynamically during each plugin initialization.
>>
> Yes. How do we currently handle a request for updating services and characteristics? Do we remove all old object paths and create new ones ?

You mean notifications of Service Changed service from a peer device?
There is no implementation, as I said. We do not even look for this
service during service discovery. Patches are welcome!

Best Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

2011-10-30 06:56:55

[permalink] [raw]

Subject: RE: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Anderson,

> The PropertyChanged signal will be emitted once the value is written.
> Applications should listen to this signal if they want to know when/if
> the value has been updated on peer device. So if the application has
> some requirement to have the value written in, let's say, 20 seconds,
> it starts a timer when doing a SetProperty(). If the timer expires and
> PropertyChanged was not received, the value can be considered as "not
> written yet". It is up to the application what to do later.
>
I could not find this in the upstream code. Can you please point me to where this happens?
What happens if this value is written from another client ? What happens if this value is
changed by the server at some unknown time, and the client is responsible for updating this value occasionally by polling the server ? I could not find in the upstream code any other gatt_read_char , except for when connection is established.

>
> The value *can* only be updated once the connection is established. a
> SetProperty() may trigger this connection. But it will *not* block
> waiting for the connection establishment (which may be between "now"
> and "never"). You monitor the PropertyChanged signal to know when/if
> the value was written.
>
Again, you force a specific scenario, where you believe that the server should notify/indicate the client of any changes. What if not ? What if the profile requires you to poll ? For example, the health thermometer notification/indication is only optional. What will a client do if it needs to poll for a new value? Should it disconnect and reconnect ?

>
> The value update is started by the time SetProperty() is called. But
> it is not guaranteed to finish by the time it returns. You should
> listen to the PropertyChanged signal for that.
>
I don't get why you bind setProperty with value change here.

>
> The "update process" is started by GetProperties(), but it will not
> block. you have PropertiesChanged for knowing then the "fresh" value
> has been read.
>
I could not find this either in the code. Currently, upstream, this is what append_char_dict does :
static void append_char_dict(DBusMessageIter *iter, struct characteristic *chr)
179 {
180 DBusMessageIter dict;
181 const char *name = "";
182 char *uuid;
183
184 dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
185 DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
186 DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
187 DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
188
189 uuid = g_strdup(chr->type);
190 dict_append_entry(&dict, "UUID", DBUS_TYPE_STRING, &uuid);
191 g_free(uuid);
192
193 /* FIXME: Translate UUID to name. */
194 dict_append_entry(&dict, "Name", DBUS_TYPE_STRING, &name);
195
196 if (chr->desc)
197 dict_append_entry(&dict, "Description", DBUS_TYPE_STRING,
198 &chr->desc);
199
200 if (chr->value)
201 dict_append_array(&dict, "Value", DBUS_TYPE_BYTE, &chr->value,
202 chr->vlen);
203
204 /* FIXME: Missing Format, Value and Representation */
205
206 dbus_message_iter_close_container(iter, &dict);
207 }
Nothing here does any of the things you suggest. If I'm wrong, let me know where it does that.

>
> You need to ask this for the guys who implemented the concept :) But
> AFAIK it is so to avoid too many D-Bus round trips.
>
So to avoid too many round trips I need to get all the information at once ? Characteristic Properties are values that do not change frequently. Char properties, write permissions for example are set and fixed. Value however is changing all the time. I do not see a reason to put all those together - I do not need to get the write modes each time I get a value.

> > 3. Writing to the value property needs to be done in one of 4
> methods. How do you support that in the existing bluez property format
> ?
>
> Based on what the characteristic supports, and the state of the link.
> See below.
>
So again, we take the easy way to decide what's best for the user, instead of giving him control.

> I can't find such "authenticated write".
>
Authenticated write is write on an encrypted link. In GATT /GAP LE specs, it is not required to establish a link and immediately raise the security level. It is assumed that the client can read the ATT error code, and figure out that the write needs encryption, so the client may initiate security mode change. Currently, we force the link security mode upgrade on connection according to bond state.

>
> "This sub-procedure is used to write a Characteristic Value to a server
> when the
> client knows the Characteristic Value Handle and the ATT Bearer is not
> encrypted. This sub-procedure shall only be used if the Characteristic
> Proper-
> ties authenticated bit is enabled and the client and server device
> share a bond
> as defined in [Vol. 3] Part C, Generic Access Profile."
>
> Do you see that all necessary information to know when to use it or
> not is already known by BlueZ?
>
Yes. I see that. However, I'm not sure we need to take control over decisions like this.

> Reliable Writes: This is a different beast. According to the spec, it
> can be used in two situations:
>
> * "assurance is required that the correct Characteristic Value is
> going to be written..."
> * "multiple values must be written, in order, in a single operation"
>
> Both usecases cannot be satisfied on current API, and not even on your
> proposal.
>
> To me, it seems support for Reliable Writes require a separate set of
> functions, if we want to support both usecases listed on the spec.
>
So we need to be prepared, and make sure the API stays and does not change too much when we do add support for this type of write.

> Note that there is a condition, for "optional" on server. The server
> cannot add/remove services during its lifetime, or at least cannot
> change the attribute handles. We don't have this guarantee as well, as
> handles are allocated dynamically during each plugin initialization.
>
Yes. How do we currently handle a request for updating services and characteristics? Do we remove all old object paths and create new ones ?

Best regards,
Chen Ganir.

2011-10-28 20:09:01

[permalink] [raw]

Subject: Re: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Hi Brian,

On Fri, Oct 28, 2011 at 12:19 PM, Brian Gix <[email protected]> wrote:
> I am also of the opinion that caching is the wrong thing to do in
> bluez/bluetoothd.

I think you guys should take a look at how the GATT read/write on the
generic D-Bus API is currently implemented (apologies if you done that
already).

I only used the "caching" word in my emails because it appeared at
some point on this thread. What happens is not caching in the "CS"
sense. It is more like "holding on a buffer". The value the client
reads is the last read one, and he gets notified by means of
PropertyChanged signal when the fresh value is read (after connection
establishment). We should not block D-Bus calls where possible, as far
as I know.

For writes, the value is hold by BlueZ while is re-establishes
connection. After the write is effective, a PropertyChanged is emitted
(If I remember correctly). That way, the API is not blocking.

The spec does not forbid the implementation to hold data in a buffer
while the connection is being restored (please provide me reference if
I am mistaken).

> As to moving forward towards more characteristic definitions and profiles, a
> do not think that by not caching, we will create the "poll fest" situation
> that Luiz fears. Applications that have been written with LE in mind should
> be the place where characteristic specific knowledge, such as persistence of
> data over time, should be. ?An App that repeatedly polls a persistent
> characteristic value would be poorly written (the App should do it's own
> caching). In the unlikely event that Multiple Apps are sharing the same
> remote server (which as a LE Use Case, I would consider Rare) you are only
> increasing the Reads of persistent data on a 1 per App basis.

Please take a look at Time, Phone Alert Status and Alert Notification
profiles (specially their UCRDDs). BT SIG usually avoids creating
dependencies between LE profiles, but these profiles are ones where it
makes a lot of sense to be present on the same LE single mode device
(and the corresponding support on the dual mode device).

Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

2011-10-28 16:19:39

by Brian Gix

[permalink] [raw]

Subject: Re: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Hi Chen, Anderson, Luiz,

On 10/27/2011 8:06 AM, Ganir, Chen wrote:
> Anderson,Luiz,
>
>>
>> Regarding caching reads and writes, the way LE works from my POV is
>> that most devices (except ones which rely on Proximity Link Loss) will
>> have Link Supervision Timeout of a few seconds at least. I have
>> already seen devices with 30 seconds LSTO.
>>
>> For these high latency devices (which use conservative connection
>> parameters for saving battery) some caching is good. But we should
>> provide ways to let the D-Bus API user know whether the data has
>> effectively been written, or updated locally. This can be done by
>> means of D-Bus signals. Which point us back to the reason why we have
>> Value as a property in the first place.
>>
>> Also imagine that the API might allow a couple of D-Bus clients
>> talking to the same device, but using different services. In case of
>> temporary link loss (or high latency), the values should be kept by
>> BlueZ, which will then write the value once connection is restored.
>> Otherwise, you would see many D-Bus clients trying to read/write data
>> while the link is not ok.
>>
> Caching seems very wrong to me. A client expects the correct data from the server, and not something that is not correct. Why provide a cached value instead of notifying the client that the link is down ? What if the server changes the value, and the client keeps getting the wrong value ? I believe the client should be notified if the link is down and there is an error reading the correct value.
>
> Regarding the multiple client scenario - Since dbus is still not my expertise, I would be happy to get some ideas here - currently the function is blocking. It means that it will not return until the server replies or the link is down. I assume that the dbus daemon will queue multiple requests to allow bluetoothd to handle each one at it's turn ? If this is not the case, I would be happy if someone could elaborate on this and highlight the correct usage in multiple client scenario.
>

I am also of the opinion that caching is the wrong thing to do in
bluez/bluetoothd.

Caching should only be done when there is established knowledge by
entity doing the caching, that the data persists. There are a few cases
where this is known to be true, such as device/manufacturer text
strings, but for the vast majority of characteristics that have already
been invented, and will be invented in the future bluez should have no
privledged knowledge of.

Temperature readings of course are the perfect example of why we don't
want to make caching the default behavior.

As to moving forward towards more characteristic definitions and
profiles, a do not think that by not caching, we will create the "poll
fest" situation that Luiz fears. Applications that have been written
with LE in mind should be the place where characteristic specific
knowledge, such as persistence of data over time, should be. An App
that repeatedly polls a persistent characteristic value would be poorly
written (the App should do it's own caching). In the unlikely event that
Multiple Apps are sharing the same remote server (which as a LE Use
Case, I would consider Rare) you are only increasing the Reads of
persistent data on a 1 per App basis.

But we should NOT cache values simply as a way to protect ourselves from
poorly behaving (excessively polling) Apps.

Unless we could convince the BT Sig to add a Persistent Property
(perhaps to the Extended Property mask) I would not do any caching in
bluez, with the exception of a few known Characteristrics (such as in
the GATT and GAP service) that bluez itself directly is the "client".

>>
>> Luiz, while I totally agree with the D-Bus error instead of return
>> value (and you know about this stuff better than me :), how could we
>> pass the "custom" ATT error codes to the client? Any profile can
>> define their own ATT error codes, and BlueZ cannot know all of them.
>>
> This is why I wanted it to be integer.
>
>
>>>
>>> Create an array of string WriteMethods which represents the methods
>>> supported by the characteristic.
>>
>> Agreed. They are readonly anyway. and makes it easier to extend.
>>
>> I would just go with "SupportedWriteMethods" of something like that :)
>> (but it is a detail).
>>
> I also agree here. This property will be changed to writableMethods string array, with string>Boolean dictionary.
>
> Thanks for your comments so far.
>
> Chen Ganir.
>

--
Brian Gix
[email protected]
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

2011-10-28 00:23:54

[permalink] [raw]

Subject: RE: [PATCH bluez 0/2] Add BT 3.0+HS signalling definitions

Hi Andrei,

>
> My idea is to use name A2MP when we are talking about
> "AMP Manager Protocol" and use AMP when we are dealing with
> "Alternate MAC PHY" controller.
>
> Best regards
> Andrei Emeltchenko

I'll repost the patchset with the names changed to a2mp.

Peter.

--Peter Krystad
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

2011-10-27 17:32:02

[permalink] [raw]

Subject: Re: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Hi Chen,

On Thu, Oct 27, 2011 at 12:47 PM, Ganir, Chen <[email protected]> wrote:
> Anderson
>
>>
>> If the user tried to read the Value property, it will return
>> immediately with "cached" value if not connected (which is an actual,
>> not invented value read before by this or some other application),
> A value which is cached, can be there for several minutes, hours or days. It is not relevant, since it is not the value from the sever. This could be a value of a patient in a hospital bed, and as far as the client is concerned (doctor), the patient temperature is ok, although the patient may be actually dead (the termperature is updated only at connection or on notification). Notification in health thermometer is only optional, so if the server did not implement notification, and the client only reads the value once on connection, and the client did not disconnect for hours, the value read by the client is invalid.

The PropertyChanged signal will be emitted once the value is written.
Applications should listen to this signal if they want to know when/if
the value has been updated on peer device. So if the application has
some requirement to have the value written in, let's say, 20 seconds,
it starts a timer when doing a SetProperty(). If the timer expires and
PropertyChanged was not received, the value can be considered as "not
written yet". It is up to the application what to do later.

>> then attempt connection. After connection is done, the read request is
>> done, the value is updated, and a PropertyChanged signal is sent.
>>
> So again, basically, what you are saying here is that value is only updated when a connection is established. This is wrong. Value should be updated each time a client requests the value.

The value *can* only be updated once the connection is established. a
SetProperty() may trigger this connection. But it will *not* block
waiting for the connection establishment (which may be between "now"
and "never"). You monitor the PropertyChanged signal to know when/if
the value was written.

>> That is why we use Value as a property and not a method like you are
>> suggesting. Clients should be monitoring the PropertyChanged signals
>> all the time.
>>
> That's fine by me, as long as the value is also updated upon use request as well.

The value update is started by the time SetProperty() is called. But
it is not guaranteed to finish by the time it returns. You should
listen to the PropertyChanged signal for that.

>> And another way is like it is currently :) a Value property for
>> read/writing, ?and PropertyChanged signal for when the new value is
>> read/written. I fail to see a difference between what you suggest
>> above. BlueZ has this "Property" concept for quite some time.
>>
> Properties are great, but need to be update correctly. I don't mind keeping it the way it is, but a fix is required for the following issues:
> 1. Values need to be updated when a user tries to read them (getProperties).

The "update process" is started by GetProperties(), but it will not
block. you have PropertiesChanged for knowing then the "fresh" value
has been read.

> 2. Why should I get all the properties every time I want the value ? Char properties may be constant throughout a session, but it's value may change frequently.

You need to ask this for the guys who implemented the concept :) But
AFAIK it is so to avoid too many D-Bus round trips.

> 3. Writing to the value property needs to be done in one of 4 methods. How do you support that in the existing bluez property format ?

Based on what the characteristic supports, and the state of the link. See below.

>> The only limitation is that we can't control which operation is used
>> for write (write without response vs. write request). But... what if
>> we detect that by the characteristic properties (like we discussed for
>> notification/indication in another message). Again, if both are
>> supported, Write Request takes priority because it is more "reliable".
>>
> What about authenticated write and signed write ? What about reliable write method?

I can't find such "authenticated write".

"Signed write without response" (also known as "authenticated signed
write"): from the spec:

"This sub-procedure is used to write a Characteristic Value to a server when the
client knows the Characteristic Value Handle and the ATT Bearer is not
encrypted. This sub-procedure shall only be used if the Characteristic Proper-
ties authenticated bit is enabled and the client and server device share a bond
as defined in [Vol. 3] Part C, Generic Access Profile."

Do you see that all necessary information to know when to use it or
not is already known by BlueZ?

Reliable Writes: This is a different beast. According to the spec, it
can be used in two situations:

* "assurance is required that the correct Characteristic Value is
going to be written..."
* "multiple values must be written, in order, in a single operation"

Both usecases cannot be satisfied on current API, and not even on your proposal.

To me, it seems support for Reliable Writes require a separate set of
functions, if we want to support both usecases listed on the spec.

>> If by "discover" you mean service discovery, it is already done "for
>> free" during pairing. The core spec says that this information is
>> valid for the time of bonding, and if some service needs to be added,
>> the server shall implement Service Changed. (BTW we don't support
>> Service Changed yet, neither as client nor server).
>>
> The service changed is mandatory for clients, optional for servers. If we intend to keep the cache of services/characteristics, we need to handle service changed notification. Otherwise, we must not cache and read the services on each connection.

Patches implementing the client side are welcome :)

Note that there is a condition, for "optional" on server. The server
cannot add/remove services during its lifetime, or at least cannot
change the attribute handles. We don't have this guarantee as well, as
handles are allocated dynamically during each plugin initialization.

> Regards,
> Chen Ganir.

Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

2011-10-27 16:47:25

[permalink] [raw]

Subject: RE: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Anderson

>
> If the user tried to read the Value property, it will return
> immediately with "cached" value if not connected (which is an actual,
> not invented value read before by this or some other application),
A value which is cached, can be there for several minutes, hours or days. It is not relevant, since it is not the value from the sever. This could be a value of a patient in a hospital bed, and as far as the client is concerned (doctor), the patient temperature is ok, although the patient may be actually dead (the termperature is updated only at connection or on notification). Notification in health thermometer is only optional, so if the server did not implement notification, and the client only reads the value once on connection, and the client did not disconnect for hours, the value read by the client is invalid.

> then attempt connection. After connection is done, the read request is
> done, the value is updated, and a PropertyChanged signal is sent.
>
So again, basically, what you are saying here is that value is only updated when a connection is established. This is wrong. Value should be updated each time a client requests the value.

> That is why we use Value as a property and not a method like you are
> suggesting. Clients should be monitoring the PropertyChanged signals
> all the time.
>
That's fine by me, as long as the value is also updated upon use request as well.

>
> We already do that. The point (which I already mentioned before) is
> that sending errors to the users, which are temporary (so it is under
> debate if they are actually errors), and even if they get these
> errors, they can't do nothing about it, because they can't control the
> connection by themselves.
>
Actually, Connection establishment and termination should be discussed in a different thread, as a continuation of the Bluetooth summit discussion of auto-connection mechanism.

>
> And another way is like it is currently :) a Value property for
> read/writing, and PropertyChanged signal for when the new value is
> read/written. I fail to see a difference between what you suggest
> above. BlueZ has this "Property" concept for quite some time.
>
Properties are great, but need to be update correctly. I don't mind keeping it the way it is, but a fix is required for the following issues:
1. Values need to be updated when a user tries to read them (getProperties).
2. Why should I get all the properties every time I want the value ? Char properties may be constant throughout a session, but it's value may change frequently.
3. Writing to the value property needs to be done in one of 4 methods. How do you support that in the existing bluez property format ?

> The only limitation is that we can't control which operation is used
> for write (write without response vs. write request). But... what if
> we detect that by the characteristic properties (like we discussed for
> notification/indication in another message). Again, if both are
> supported, Write Request takes priority because it is more "reliable".
>
What about authenticated write and signed write ? What about reliable write method?

> If by "discover" you mean service discovery, it is already done "for
> free" during pairing. The core spec says that this information is
> valid for the time of bonding, and if some service needs to be added,
> the server shall implement Service Changed. (BTW we don't support
> Service Changed yet, neither as client nor server).
>
The service changed is mandatory for clients, optional for servers. If we intend to keep the cache of services/characteristics, we need to handle service changed notification. Otherwise, we must not cache and read the services on each connection.

Regards,
Chen Ganir.

2011-10-27 16:46:13

by tim.howes

[permalink] [raw]

Subject: RE: BT 3.0+HS

Hi Timur,

> On 10/26/2011 09:54 AM, Andrei Emeltchenko wrote:
> > Hi Timur,
> >
> > On Wed, Oct 26, 2011 at 02:15:38AM +0200, Timur Mehrvarz wrote:
> >> I'm really just looking for some combination of devices that would
> >> allow me to establish HS/AMP RFCOMM connections myself. Anything
> that
> >> is available today? The new Macbook Air's and iPhone's are said to
> >> support
> >> BT4 (including HS?). Samsung Galaxy2 devices (and the new Galaxy
> >> Nexus
> >> devices) are said to support BT3+HS (true?). Or maybe I should just
> >> order some of these new BT3+HS dongles and try under Windows? Help.
> >
> > I do not know any public device having HS feature fully implemented.
> I
> > would be happy if someone correct me here.
>
> There is really not a single HS capable device out there?

Here's the Bluetooth SIG director demo-ing some phones with HS:

http://www.youtube.com/watch?v=BMdbV8quuL0

Tim

________________________________
This message is for the designated recipient only and may contain privileged, proprietary, or otherwise private information. If you have received it in error, please notify the sender immediately and delete the original. Any other use of the email by you is prohibited.

2011-10-27 16:29:09

[permalink] [raw]

Subject: Re: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Hi Chen,

On Thu, Oct 27, 2011 at 11:48 AM, Ganir, Chen <[email protected]> wrote:
> Anderson,
>
>>
>> Hi Chen,
>>
>> On Thu, Oct 27, 2011 at 11:06 AM, Ganir, Chen <[email protected]>
>> wrote:
>> > Caching seems very wrong to me. A client expects the correct data
>> from the server, and not something that is not correct. Why provide a
>> cached value instead of notifying the client that the link is down ?
>> What if the server changes the value, and the client keeps getting the
>> wrong value ? I believe the client should be notified if the link is
>> down and there is an error reading the correct value.
>>
>> The value is not actually wrong, it is just old. But it is the best
>> data available while connection is not re-established. But as I said
>> before, there should be a way (e.g. specific D-Bus property) to the
>> user know that the value is stale, but it is not an error to have
>> "link down" temporarily.
>>
> If the link is down, value should be empty, not cached. There is no reason showing a false value. Think of the thermometer example - you can not show false information, just to keep the user away from the disconnection error.
> In addition - how will you control when the cached value is refreshed ? Will it be periodically? Upon use request ? Just wait for notification/indication?

If the user tried to read the Value property, it will return
immediately with "cached" value if not connected (which is an actual,
not invented value read before by this or some other application),
then attempt connection. After connection is done, the read request is
done, the value is updated, and a PropertyChanged signal is sent.

That is why we use Value as a property and not a method like you are
suggesting. Clients should be monitoring the PropertyChanged signals
all the time.

>> For some data, the cached value is not bad. For others (e.g.
>> termomether measurements, timestamp information) it is. The API has to
>> be generic and allow to provide the latest available data, but still
>> allow user to know it is not "live" data.
>>
>> Another point here is that we do not want to give to the D-Bus API use
>> the ability to control connections directly. In this scenario, what it
>> would do with a "link down" error?
>>
>> If the BlueZ will try to reconnect on timeout anyway, why the user
>> needs to know about this temporary error? Of course the connection may
>> never be restored, in this case we can use a D-Bus "give up" signal to
>> notify this to the user.
>>
> The bluez should know and prevent such a case. If the link is down, and auto-connection is in progress, no reconnection attempt shall be made in addition to that - and the returned value should be "connection error".

We already do that. The point (which I already mentioned before) is
that sending errors to the users, which are temporary (so it is under
debate if they are actually errors), and even if they get these
errors, they can't do nothing about it, because they can't control the
connection by themselves.

I suppose the end-user is not interested on seeing "Connection error"
or "connection in progress" dialog messages :)

>> Blocking D-Bus is not good as far as I know (Luiz and others may give
>> more details or ideas here, I'm newbie on d-bus as well).
>>
> The other way to do it is to send the command (read, write, discover) and wait for the callback to send back a dbus event. It's possible, but makes the whole thing much more complicated, forcing the application to handle states. This also means registering some kind of agent for generic gatt client, or per service/character, to allow receiving the read/write/discover callbacks, to prevent from broadcasting such events on the bus for everyone. Do we really want to complicate things like that ?

And another way is like it is currently :) a Value property for
read/writing, and PropertyChanged signal for when the new value is
read/written. I fail to see a difference between what you suggest
above. BlueZ has this "Property" concept for quite some time.

The only limitation is that we can't control which operation is used
for write (write without response vs. write request). But... what if
we detect that by the characteristic properties (like we discussed for
notification/indication in another message). Again, if both are
supported, Write Request takes priority because it is more "reliable".

If by "discover" you mean service discovery, it is already done "for
free" during pairing. The core spec says that this information is
valid for the time of bonding, and if some service needs to be added,
the server shall implement Service Changed. (BTW we don't support
Service Changed yet, neither as client nor server).

>
> Thanks,
> Chen Ganir.
>

Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

2011-10-27 15:48:48

[permalink] [raw]

Subject: RE: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Anderson,

>
> Hi Chen,
>
> On Thu, Oct 27, 2011 at 11:06 AM, Ganir, Chen <[email protected]>
> wrote:
> > Caching seems very wrong to me. A client expects the correct data
> from the server, and not something that is not correct. Why provide a
> cached value instead of notifying the client that the link is down ?
> What if the server changes the value, and the client keeps getting the
> wrong value ? I believe the client should be notified if the link is
> down and there is an error reading the correct value.
>
> The value is not actually wrong, it is just old. But it is the best
> data available while connection is not re-established. But as I said
> before, there should be a way (e.g. specific D-Bus property) to the
> user know that the value is stale, but it is not an error to have
> "link down" temporarily.
>
If the link is down, value should be empty, not cached. There is no reason showing a false value. Think of the thermometer example - you can not show false information, just to keep the user away from the disconnection error.
In addition - how will you control when the cached value is refreshed ? Will it be periodically? Upon use request ? Just wait for notification/indication?

> For some data, the cached value is not bad. For others (e.g.
> termomether measurements, timestamp information) it is. The API has to
> be generic and allow to provide the latest available data, but still
> allow user to know it is not "live" data.
>
> Another point here is that we do not want to give to the D-Bus API use
> the ability to control connections directly. In this scenario, what it
> would do with a "link down" error?
>
> If the BlueZ will try to reconnect on timeout anyway, why the user
> needs to know about this temporary error? Of course the connection may
> never be restored, in this case we can use a D-Bus "give up" signal to
> notify this to the user.
>
The bluez should know and prevent such a case. If the link is down, and auto-connection is in progress, no reconnection attempt shall be made in addition to that - and the returned value should be "connection error".

> > Regarding the multiple client scenario - Since dbus is still not my
> expertise, I would be happy to get some ideas here - currently the
> function is blocking. It means that it will not return until the server
> replies or the link is down. I assume that the dbus daemon will queue
> multiple requests to allow bluetoothd to handle each one at it's turn ?
> If this is not the case, I would be happy if someone could elaborate on
> this and highlight the correct usage in multiple client scenario.
>
> Blocking D-Bus is not good as far as I know (Luiz and others may give
> more details or ideas here, I'm newbie on d-bus as well).
>
The other way to do it is to send the command (read, write, discover) and wait for the callback to send back a dbus event. It's possible, but makes the whole thing much more complicated, forcing the application to handle states. This also means registering some kind of agent for generic gatt client, or per service/character, to allow receiving the read/write/discover callbacks, to prevent from broadcasting such events on the bus for everyone. Do we really want to complicate things like that ?

Thanks,
Chen Ganir.

2011-10-27 15:35:51

[permalink] [raw]

Subject: Re: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Hi Chen,

On Thu, Oct 27, 2011 at 11:18 AM, Ganir, Chen <[email protected]> wrote:
>> As a general suggestion, I propose you first get the API changes
>> "approved" and committed upstream, then proceed with code changes.
>> Unless you are comfortable with doing lots of rounds of patch
>> submissions (or you are planning only RFC to get early comments).
>>
> This is the purpose of this document. Early next week I'll send the modified API text with the changes made according to the comments, and get the approval. For now, my implementation is internal, and is mostly to support some other work I need to get done.

Once you are happy with the amount of comments, you should send your
changes in a format suitable for upstream submission (e.g. sent with
git send-email, no sign-off line, "[PATCH BlueZ]" as prefix etc.)

Thanks,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

2011-10-27 15:29:46

[permalink] [raw]

Subject: Re: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Hi Chen,

On Thu, Oct 27, 2011 at 11:06 AM, Ganir, Chen <[email protected]> wrote:
> Caching seems very wrong to me. A client expects the correct data from the server, and not something that is not correct. Why provide a cached value instead of notifying the client that the link is down ? What if the server changes the value, and the client keeps getting the wrong value ? I believe the client should be notified if the link is down and there is an error reading the correct value.

The value is not actually wrong, it is just old. But it is the best
data available while connection is not re-established. But as I said
before, there should be a way (e.g. specific D-Bus property) to the
user know that the value is stale, but it is not an error to have
"link down" temporarily.

For some data, the cached value is not bad. For others (e.g.
termomether measurements, timestamp information) it is. The API has to
be generic and allow to provide the latest available data, but still
allow user to know it is not "live" data.

Another point here is that we do not want to give to the D-Bus API use
the ability to control connections directly. In this scenario, what it
would do with a "link down" error?

If the BlueZ will try to reconnect on timeout anyway, why the user
needs to know about this temporary error? Of course the connection may
never be restored, in this case we can use a D-Bus "give up" signal to
notify this to the user.

> Regarding the multiple client scenario - Since dbus is still not my expertise, I would be happy to get some ideas here - currently the function is blocking. It means that it will not return until the server replies or the link is down. I assume that the dbus daemon will queue multiple requests to allow bluetoothd to handle each one at it's turn ? If this is not the case, I would be happy if someone could elaborate on this and highlight the correct usage in multiple client scenario.

Blocking D-Bus is not good as far as I know (Luiz and others may give
more details or ideas here, I'm newbie on d-bus as well).

Try to model the API on a way which is compatible with:

* high latency (e.g. 30 seconds)
* timeouts are common, but handled automatically by BlueZ.
* Not try to poll, or flood D-Bus with requests which cannot be
satisfied in real time.

>> > Create an array of string WriteMethods which represents the methods
>> > supported by the characteristic.
>>
>> Agreed. They are readonly anyway. and makes it easier to extend.
>>
>> I would just go with "SupportedWriteMethods" of something like that :)
>> (but it is a detail).
>>
> I also agree here. This property will be changed to writableMethods string array, with string>Boolean dictionary.

A dict is not needed here IMHO. If the string is on the array, it is
"true". If it is not, it is "false". I think this is how Luiz imagined
(right?).

Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

2011-10-27 15:18:54

[permalink] [raw]

Subject: RE: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Anderson
>
> Yes, I think read-only property is better. This way, it will mostly be
> used to decide whether RegisterCharacteristicsWatcher() is supported
> or not. Also see the Luiz suggestion of using a list of strings. It
> allows easily extending these GATT properties on future.
>

I will add a string list called ValueChangeMethods and add Notify and Indicate as possible strings.

> Regarding a new RegisterCharacteristicsWatcher() argument, I would
> avoid it if we can for now. We could implicitly have a priority inside
> BlueZ:
>
Ok, I'm good with this as well.

> * if characteristic supports indication, use indication (usually, this
> means the server needs to make sure the value was received, so it can
> e.g. free that value from its internal memory).
> * if characteristic supports notification, use notification
> * else, make RegisterCharacteristicsWatcher() returns a D-Bus error.
>
> That way, if a characteristic happens to support both indication and
> notification, it will always use indication. If in future we find that
> unsuitable for some profile, this can be changed, but let's try to not
> make things too complex now :)
>
I'll do that as well.

> As a general suggestion, I propose you first get the API changes
> "approved" and committed upstream, then proceed with code changes.
> Unless you are comfortable with doing lots of rounds of patch
> submissions (or you are planning only RFC to get early comments).
>
This is the purpose of this document. Early next week I'll send the modified API text with the changes made according to the comments, and get the approval. For now, my implementation is internal, and is mostly to support some other work I need to get done.

Thanks,
Chen Ganir.

2011-10-27 15:13:44

[permalink] [raw]

Subject: Re: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Hi Chen,

On Thu, Oct 27, 2011 at 10:58 AM, Ganir, Chen <[email protected]> wrote:
> Characteristics have the ability to be notified or indicated, according to the profile requirements. We need to reflect those capabilities to the user, which can decide whether the characteristic will be notified or indicated. The bluetoothd will take care of the indication return code, and the watcher will be signaled with the existing mechanism.
>
> Would you rather see the register watcher get a second parameter to tell it the correct method, and change the properties to CanNotify, CanBroadcast to indicate the characteristic capability only (according to the char properties and availability of client char config descriptor)? If this is the case, than we may implicitly force notify or indicate only, not both of them, although the spec does not prohibit such behavior, and manage the watcher method internally.

Yes, I think read-only property is better. This way, it will mostly be
used to decide whether RegisterCharacteristicsWatcher() is supported
or not. Also see the Luiz suggestion of using a list of strings. It
allows easily extending these GATT properties on future.

Regarding a new RegisterCharacteristicsWatcher() argument, I would
avoid it if we can for now. We could implicitly have a priority inside
BlueZ:

* if characteristic supports indication, use indication (usually, this
means the server needs to make sure the value was received, so it can
e.g. free that value from its internal memory).
* if characteristic supports notification, use notification
* else, make RegisterCharacteristicsWatcher() returns a D-Bus error.

That way, if a characteristic happens to support both indication and
notification, it will always use indication. If in future we find that
unsuitable for some profile, this can be changed, but let's try to not
make things too complex now :)

As a general suggestion, I propose you first get the API changes
"approved" and committed upstream, then proceed with code changes.
Unless you are comfortable with doing lots of rounds of patch
submissions (or you are planning only RFC to get early comments).

Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

2011-10-27 15:06:53

[permalink] [raw]

Subject: RE: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Anderson,Luiz,

>
> Regarding caching reads and writes, the way LE works from my POV is
> that most devices (except ones which rely on Proximity Link Loss) will
> have Link Supervision Timeout of a few seconds at least. I have
> already seen devices with 30 seconds LSTO.
>
> For these high latency devices (which use conservative connection
> parameters for saving battery) some caching is good. But we should
> provide ways to let the D-Bus API user know whether the data has
> effectively been written, or updated locally. This can be done by
> means of D-Bus signals. Which point us back to the reason why we have
> Value as a property in the first place.
>
> Also imagine that the API might allow a couple of D-Bus clients
> talking to the same device, but using different services. In case of
> temporary link loss (or high latency), the values should be kept by
> BlueZ, which will then write the value once connection is restored.
> Otherwise, you would see many D-Bus clients trying to read/write data
> while the link is not ok.
>
Caching seems very wrong to me. A client expects the correct data from the server, and not something that is not correct. Why provide a cached value instead of notifying the client that the link is down ? What if the server changes the value, and the client keeps getting the wrong value ? I believe the client should be notified if the link is down and there is an error reading the correct value.

Regarding the multiple client scenario - Since dbus is still not my expertise, I would be happy to get some ideas here - currently the function is blocking. It means that it will not return until the server replies or the link is down. I assume that the dbus daemon will queue multiple requests to allow bluetoothd to handle each one at it's turn ? If this is not the case, I would be happy if someone could elaborate on this and highlight the correct usage in multiple client scenario.

>
> Luiz, while I totally agree with the D-Bus error instead of return
> value (and you know about this stuff better than me :), how could we
> pass the "custom" ATT error codes to the client? Any profile can
> define their own ATT error codes, and BlueZ cannot know all of them.
>
This is why I wanted it to be integer.

> >
> > Create an array of string WriteMethods which represents the methods
> > supported by the characteristic.
>
> Agreed. They are readonly anyway. and makes it easier to extend.
>
> I would just go with "SupportedWriteMethods" of something like that :)
> (but it is a detail).
>
I also agree here. This property will be changed to writableMethods string array, with string>Boolean dictionary.

Thanks for your comments so far.

Chen Ganir.

2011-10-27 14:58:42

[permalink] [raw]

Subject: RE: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Anderson,

> -----Original Message-----
> From: Anderson Lizardo [mailto:[email protected]]
> Sent: Thursday, October 27, 2011 4:03 PM
> To: Luiz Augusto von Dentz
> Cc: Ganir, Chen; Mat Martineau; Claudio Takahasi; linux-
> [email protected]; [email protected]; [email protected]
> Subject: Re: GATT Dbus API on BlueZ - attirbute-api.txt modifications
>
> Hi,
>
> On Thu, Oct 27, 2011 at 9:57 AM, Anderson Lizardo
> <[email protected]> wrote:
> > But, I don't see why have these (writable?) D-Bus properties if we
> > already have RegisterCharacteristicsWatcher(). Are you suggesting
> that
> > confirmations (for indications) should be sent manually by each D-Bus
> > client? Too error-prone IMHO.
>
> To clarify, the question above was to Chen, not to Luiz.
>

Characteristics have the ability to be notified or indicated, according to the profile requirements. We need to reflect those capabilities to the user, which can decide whether the characteristic will be notified or indicated. The bluetoothd will take care of the indication return code, and the watcher will be signaled with the existing mechanism.

Would you rather see the register watcher get a second parameter to tell it the correct method, and change the properties to CanNotify, CanBroadcast to indicate the characteristic capability only (according to the char properties and availability of client char config descriptor)? If this is the case, than we may implicitly force notify or indicate only, not both of them, although the spec does not prohibit such behavior, and manage the watcher method internally.

Chen Ganir.

2011-10-27 14:02:47

[permalink] [raw]

Subject: Re: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Hi,

On Thu, Oct 27, 2011 at 9:57 AM, Anderson Lizardo
<[email protected]> wrote:
> But, I don't see why have these (writable?) D-Bus properties if we
> already have RegisterCharacteristicsWatcher(). Are you suggesting that
> confirmations (for indications) should be sent manually by each D-Bus
> client? Too error-prone IMHO.

To clarify, the question above was to Chen, not to Luiz.

Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

2011-10-27 13:57:15

[permalink] [raw]

Subject: Re: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Hi Luiz,

A few more comments, as Luiz's email reminded me of some other things.

On Thu, Oct 27, 2011 at 9:31 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Chen,
>
> On Thu, Oct 27, 2011 at 11:22 AM, Ganir, Chen <[email protected]> wrote:
>> I do not see any reason why caching of data is required. Data should be read directly from the GATT server. Caching it may cause inconsistency. There is also no reason to read the values on characteristic object creation in the client.c file. I think the decision of reading the value should be by the user and not the system.
>> In addition, Value is not a property of a characteristic.
>
> Actually is the other way round, caching is the only way to maintain
> data consistency otherwise we will never be able to do notifications
> properly because clients can, and probably will, keep polling at
> different rate, also this remove any possibility of doing power safe
> not to me mention that we will have to queue the requests once one is
> pending which can cause other problems like timeouts and etc.

Regarding caching reads and writes, the way LE works from my POV is
that most devices (except ones which rely on Proximity Link Loss) will
have Link Supervision Timeout of a few seconds at least. I have
already seen devices with 30 seconds LSTO.

For these high latency devices (which use conservative connection
parameters for saving battery) some caching is good. But we should
provide ways to let the D-Bus API user know whether the data has
effectively been written, or updated locally. This can be done by
means of D-Bus signals. Which point us back to the reason why we have
Value as a property in the first place.

Also imagine that the API might allow a couple of D-Bus clients
talking to the same device, but using different services. In case of
temporary link loss (or high latency), the values should be kept by
BlueZ, which will then write the value once connection is restored.
Otherwise, you would see many D-Bus clients trying to read/write data
while the link is not ok.

>>> > + ? ? ? ? ? int WriteValue(array{byte} value, int method)
>>> > +
>>> > + ? ? ? ? ? ? ? Write the value of the characteristic, with specified
>>> method :
>>> > + ? ? ? ? ? ? ? 0: Write without response. Always return 0.
>>> > + ? ? ? ? ? ? ? 1: Write with response. Return server response code.
>>> > + ? ? ? ? ? ? ? Other write methods will be added in the future.
>>> > +
>
> D-Bus does not have enum type, so please use string that describes the
> method and the return response, actually for the response it would be
> better to use void if success and use D-Bus error for invalid/error
> responses.

Luiz, while I totally agree with the D-Bus error instead of return
value (and you know about this stuff better than me :), how could we
pass the "custom" ATT error codes to the client? Any profile can
define their own ATT error codes, and BlueZ cannot know all of them.

>>> > + ? ? ? ? ? boolean Indicate [readwrite]
>>> > +
>>> > + ? ? ? ? ? ? ? Indicates whether this characteristic is notified or
>>> not.
>>> > + ? ? ? ? ? ? ? If GATT Client Characteristic Configuration descriptor
>>> > + ? ? ? ? ? ? ? is not available for this characteristic, or if the
>>> characteristic
>>> > + ? ? ? ? ? ? ? properties do not allow this, writing to this property
>>> is not
>>> > + ? ? ? ? ? ? ? allowed.
>>> > +
>>> > + ? ? ? ? ? boolean Notify [readwrite]
>>> > +
>>> > + ? ? ? ? ? ? ? Indicates whether this characteristic is indicated or
>>> not.
>>> > + ? ? ? ? ? ? ? If GATT Client Characteristic Configuration descriptor
>>> > + ? ? ? ? ? ? ? is not available for this characteristic, or if the
>>> characteristic
>>> > + ? ? ? ? ? ? ? properties do not allow this, writing to this property
>>> is not
>>> > + ? ? ? ? ? ? ? allowed.
>
> Notify and Indicate are useless without a way to be notified, so Im
> not sure what is really the point by removing the Value property it
> cannot be signaled via PropertyChanged.

Actually, we have a way to notify which is by registering a
"Characteristics Watcher". See RegisterCharacteristicsWatcher() in
doc/attribute-api.txt.

But, I don't see why have these (writable?) D-Bus properties if we
already have RegisterCharacteristicsWatcher(). Are you suggesting that
confirmations (for indications) should be sent manually by each D-Bus
client? Too error-prone IMHO.

BlueZ knows everything needed to enable notification/indication, and
even send back confirmations in case of indication. If some
characteristic allows either notifications or indications, we could
add a second parameter to RegisterCharacteristicsWatcher().

>
>>> > + ? ? ? ? ? boolean Readable [readonly]
>>> > +
>>> > + ? ? ? ? ? ? ? Indicates wether this characteristic value can be read.
>>> > +
>>> > + ? ? ? ? ? boolean WritableNoRsp [readonly]
>>> > +
>>> > + ? ? ? ? ? ? ? Indicates whether this characteristic value can be
>>> written without
>>> > + ? ? ? ? ? ? ? response.
>>> > +
>>> > + ? ? ? ? ? boolean WritableRsp [readonly]
>>> > +
>>> > + ? ? ? ? ? ? ? Indicates whether this characteristic can be written
>>> with response.
>>> > +
>>> > + ? ? ? ? ? boolean WritableSigned [readonly]
>>
>> The WritableSigned is going to be changed to WritableAuth (this is true for both Authenticated link or signed GATT Write commands).
>>
>>> > +
>>> > + ? ? ? ? ? ? ? Indicates whether this characteristic can be written
>>> with signed
>>> > + ? ? ? ? ? ? ? authentication method.
>>> > +
>>> > + ? ? ? ? ? boolean WritableReliable [readonly]
>>> > +
>>> > + ? ? ? ? ? ? ? Indicates whether this characteristic can be written
>>> with the
>>> > + ? ? ? ? ? ? ? reliable write procedure.
>>> > +
>
> Create an array of string WriteMethods which represents the methods
> supported by the characteristic.

Agreed. They are readonly anyway. and makes it easier to extend.

I would just go with "SupportedWriteMethods" of something like that :)
(but it is a detail).

Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

2011-10-27 13:39:42

[permalink] [raw]

Subject: Re: BT 3.0+HS

On 10/26/2011 09:54 AM, Andrei Emeltchenko wrote:
> Hi Timur,
>
> On Wed, Oct 26, 2011 at 02:15:38AM +0200, Timur Mehrvarz wrote:
>> I'm really just looking for some combination of devices that would allow
>> me to establish HS/AMP RFCOMM connections myself. Anything that is
>> available today? The new Macbook Air's and iPhone's are said to support
>> BT4 (including HS?). Samsung Galaxy2 devices (and the new Galaxy Nexus
>> devices) are said to support BT3+HS (true?). Or maybe I should just
>> order some of these new BT3+HS dongles and try under Windows? Help.
>
> I do not know any public device having HS feature fully implemented. I
> would be happy if someone correct me here.

There is really not a single HS capable device out there?

Timur

2011-10-27 13:32:51

[permalink] [raw]

Subject: Re: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Hi Chen,

On Wed, Oct 26, 2011 at 3:20 AM, Ganir, Chen <[email protected]> wrote:
> + ? ? ? ? ? ? ? int WriteValue(array{byte} value, int method)

I think you meant "uint8" as return value, which corresponds to the
ATT error code type.

> +
> + ? ? ? ? ? ? ? ? ? Write the value of the characteristic, with specified method :
> + ? ? ? ? ? ? ? ? ? 0: Write without response. Always return 0.
> + ? ? ? ? ? ? ? ? ? 1: Write with response. Return server response code.
> + ? ? ? ? ? ? ? ? ? Other write methods will be added in the future.
> +

Besides what others suggested, I would avoid using integers as
enumerations on D-Bus API, specially if the actual values are not
defined by any specification. If you take a look at other API docs in
doc/* you may see examples where string are used. IMHO this make the
client code much more readable and does not require looking at the API
doc all the time. Example:

###
uint8 WriteValue(array{byte} value, string method)

Write the value of the characteristic, with specified method:
"WithoutResponse" or "WithResponse". Other write methods may be added
in the future.
###

An alternative is to use dict, if multiple "flags" are allowed.

Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

2011-10-27 13:31:09

by Luiz Augusto von Dentz

[permalink] [raw]

Subject: Re: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Hi Chen,

On Thu, Oct 27, 2011 at 11:22 AM, Ganir, Chen <[email protected]> wrote:
> Hi Mat.
>
>> -----Original Message-----
>> From: Mat Martineau [mailto:[email protected]]
>> Sent: Thursday, October 27, 2011 9:17 AM
>> Subject: Re: GATT Dbus API on BlueZ - attirbute-api.txt modifications
>>
>> Hi Chen -
>>
>> On Wed, 26 Oct 2011, Ganir, Chen wrote:
>>
>> > Hi.
>> >
>> > Here's my proposal for some modifications to the existing attribute-
>> api.doc file.
>> >
>> > These additions include :
>> > 1. Support for more properties, such as writable, readable, notify
>> > 2. Remove Value from the properties, and handle write/read with
>> specific functions. This was done since currently the value is only
>> read once from the server, and there is no way to refresh the value
>> using the dbus API. In addition, the value can be written in multiple
>> ways (we currently support write with response and write without
>> response, but future write methods include write reliable and write
>> signed, which may be required by profiles in the future).
>> >
>> > Here's the diff from the current docs/attribute-api.txt file :
>> >
>> > ---->8---------------
>> > diff --git a/doc/attribute-api.txt b/doc/attribute-api.txt
>> > index 98d7f30..fbc6957 100644
>> > --- a/doc/attribute-api.txt
>> > +++ b/doc/attribute-api.txt
>> > @@ -112,6 +112,17 @@ Methods ? ? ? ? ? ? ? ?dict GetProperties()
>> >
>> > ? ? ? ? ? ? ? ? ? ? Possible Errors: org.bluez.Error.InvalidArguments
>> >
>> > + ? ? ? ? ? array{byte} ReadValue()
>> > +
>> > + ? ? ? ? ? ? ? Read the value of the characteristic.
>> > +
>>
>> One aspect of the current client implementation is that it caches the
>> read values within bluetoothd. ?Do you prefer to completely eliminate
>> that storage and always have ReadValue fetch from the remote device?
>>
>> If there's still a use case for locally cached values (maybe there
>> isn't), a "method" parameter could specify the source of the data. ?It
>> could also provide some future-proofing for ReadValue.
>>
>
> I do not see any reason why caching of data is required. Data should be read directly from the GATT server. Caching it may cause inconsistency. There is also no reason to read the values on characteristic object creation in the client.c file. I think the decision of reading the value should be by the user and not the system.
> In addition, Value is not a property of a characteristic.

Actually is the other way round, caching is the only way to maintain
data consistency otherwise we will never be able to do notifications
properly because clients can, and probably will, keep polling at
different rate, also this remove any possibility of doing power safe
not to me mention that we will have to queue the requests once one is
pending which can cause other problems like timeouts and etc.

>
>
>> > + ? ? ? ? ? int WriteValue(array{byte} value, int method)
>> > +
>> > + ? ? ? ? ? ? ? Write the value of the characteristic, with specified
>> method :
>> > + ? ? ? ? ? ? ? 0: Write without response. Always return 0.
>> > + ? ? ? ? ? ? ? 1: Write with response. Return server response code.
>> > + ? ? ? ? ? ? ? Other write methods will be added in the future.
>> > +

D-Bus does not have enum type, so please use string that describes the
method and the return response, actually for the response it would be
better to use void if success and use D-Bus error for invalid/error
responses.

>> This addresses the shortcomings we saw with SetProperty, and also
>> gives options for the future - seems like a great idea to me!
>>
>>
>> > Properties ?string UUID [readonly]
>> >
>> > ? ? ? ? ? ? ? ? ? ? UUID128 of this characteristic.
>> > @@ -142,15 +153,58 @@ Properties ? ?string UUID [readonly]
>> > ? ? ? ? ? ? ? ? ? ? ? uint16 | Description: Description of the
>> characteristic defined
>> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| in a high layer profile.
>> >
>> > - ? ? ? ? ? array{byte} Value [readwrite]
>> > -
>> > - ? ? ? ? ? ? ? ? ? Raw value of the Characteristic Value attribute.
>> > -
>> > ? ? ? ? ? ? string Representation (of the binary Value) [readonly]
>> >
>> > ? ? ? ? ? ? ? ? ? ? Friendly representation of the Characteristic Value
>> > ? ? ? ? ? ? ? ? ? ? based on the format attribute.
>> >
>> > + ? ? ? ? ? boolean Broadcast [readwrite]
>> > +
>> > + ? ? ? ? ? ? ? Indicates whether this characteristic is broadcasted or
>> not.
>> > + ? ? ? ? ? ? ? If GATT Server Characteristic Configuration descriptor
>> > + ? ? ? ? ? ? ? is not available for this characteristic, or if the
>> characteristic
>> > + ? ? ? ? ? ? ? properties do not allow this, writing to this property
>> is not
>> > + ? ? ? ? ? ? ? allowed.
>> > +
>> > + ? ? ? ? ? boolean Indicate [readwrite]
>> > +
>> > + ? ? ? ? ? ? ? Indicates whether this characteristic is notified or
>> not.
>> > + ? ? ? ? ? ? ? If GATT Client Characteristic Configuration descriptor
>> > + ? ? ? ? ? ? ? is not available for this characteristic, or if the
>> characteristic
>> > + ? ? ? ? ? ? ? properties do not allow this, writing to this property
>> is not
>> > + ? ? ? ? ? ? ? allowed.
>> > +
>> > + ? ? ? ? ? boolean Notify [readwrite]
>> > +
>> > + ? ? ? ? ? ? ? Indicates whether this characteristic is indicated or
>> not.
>> > + ? ? ? ? ? ? ? If GATT Client Characteristic Configuration descriptor
>> > + ? ? ? ? ? ? ? is not available for this characteristic, or if the
>> characteristic
>> > + ? ? ? ? ? ? ? properties do not allow this, writing to this property
>> is not
>> > + ? ? ? ? ? ? ? allowed.

Notify and Indicate are useless without a way to be notified, so Im
not sure what is really the point by removing the Value property it
cannot be signaled via PropertyChanged.

>> > + ? ? ? ? ? boolean Readable [readonly]
>> > +
>> > + ? ? ? ? ? ? ? Indicates wether this characteristic value can be read.
>> > +
>> > + ? ? ? ? ? boolean WritableNoRsp [readonly]
>> > +
>> > + ? ? ? ? ? ? ? Indicates whether this characteristic value can be
>> written without
>> > + ? ? ? ? ? ? ? response.
>> > +
>> > + ? ? ? ? ? boolean WritableRsp [readonly]
>> > +
>> > + ? ? ? ? ? ? ? Indicates whether this characteristic can be written
>> with response.
>> > +
>> > + ? ? ? ? ? boolean WritableSigned [readonly]
>
> The WritableSigned is going to be changed to WritableAuth (this is true for both Authenticated link or signed GATT Write commands).
>
>> > +
>> > + ? ? ? ? ? ? ? Indicates whether this characteristic can be written
>> with signed
>> > + ? ? ? ? ? ? ? authentication method.
>> > +
>> > + ? ? ? ? ? boolean WritableReliable [readonly]
>> > +
>> > + ? ? ? ? ? ? ? Indicates whether this characteristic can be written
>> with the
>> > + ? ? ? ? ? ? ? reliable write procedure.
>> > +

Create an array of string WriteMethods which represents the methods
supported by the characteristic.

>> > Characteristic Watcher hierarchy
>> > ===============================
>> >
>> > --------8<-----------
>> >
>> > I would appreciate your comments on this.
>>
>> The additional properties also seem like nice additions, and work well
>> to provide clear differentiation between the characteristic value and
>> other metadata.
>>
>> Thanks for the proposal!

Write method is probably fine, and the new properties might be useful,
but IMO we need to keep the caching otherwise this will eventually
became o polling festival, also we have to keep in mind that this is
to be accessible by multiple applications simultaneously and by
application I mean end user application because intermediate layer
will only complicate things even more for no benefit, for Android you
guys should really consider writing a wrapper and exposing the
characteristics directly to the application.

--
Luiz Augusto von Dentz

2011-10-27 11:30:03

by Mat Martineau

[permalink] [raw]

Subject: RE: GATT Dbus API on BlueZ - attirbute-api.txt modifications

On Thu, 27 Oct 2011, Ganir, Chen wrote:

> Hi Mat.
>
>> -----Original Message-----
>> From: Mat Martineau [mailto:[email protected]]
>> Sent: Thursday, October 27, 2011 9:17 AM
>> Subject: Re: GATT Dbus API on BlueZ - attirbute-api.txt modifications
>>
>> Hi Chen -
>>
>> On Wed, 26 Oct 2011, Ganir, Chen wrote:
>>
>>> Hi.
>>>
>>> Here's my proposal for some modifications to the existing attribute-
>> api.doc file.
>>>
>>> These additions include :
>>> 1. Support for more properties, such as writable, readable, notify
>>> 2. Remove Value from the properties, and handle write/read with
>> specific functions. This was done since currently the value is only
>> read once from the server, and there is no way to refresh the value
>> using the dbus API. In addition, the value can be written in multiple
>> ways (we currently support write with response and write without
>> response, but future write methods include write reliable and write
>> signed, which may be required by profiles in the future).
>>>
>>> Here's the diff from the current docs/attribute-api.txt file :
>>>
>>> ---->8---------------
>>> diff --git a/doc/attribute-api.txt b/doc/attribute-api.txt
>>> index 98d7f30..fbc6957 100644
>>> --- a/doc/attribute-api.txt
>>> +++ b/doc/attribute-api.txt
>>> @@ -112,6 +112,17 @@ Methods dict GetProperties()
>>>
>>> Possible Errors: org.bluez.Error.InvalidArguments
>>>
>>> + array{byte} ReadValue()
>>> +
>>> + Read the value of the characteristic.
>>> +
>>
>> One aspect of the current client implementation is that it caches the
>> read values within bluetoothd. Do you prefer to completely eliminate
>> that storage and always have ReadValue fetch from the remote device?
>>
>> If there's still a use case for locally cached values (maybe there
>> isn't), a "method" parameter could specify the source of the data. It
>> could also provide some future-proofing for ReadValue.
>>
>
> I do not see any reason why caching of data is required. Data should be read directly from the GATT server. Caching it may cause inconsistency. There is also no reason to read the values on characteristic object creation in the client.c file. I think the decision of reading the value should be by the user and not the system.
> In addition, Value is not a property of a characteristic.
>
>
>>> + int WriteValue(array{byte} value, int method)
>>> +
>>> + Write the value of the characteristic, with specified
>> method :
>>> + 0: Write without response. Always return 0.
>>> + 1: Write with response. Return server response code.
>>> + Other write methods will be added in the future.
>>> +
>>
>> This addresses the shortcomings we saw with SetProperty, and also
>> gives options for the future - seems like a great idea to me!
>>
>>
>>> Properties string UUID [readonly]
>>>
>>> UUID128 of this characteristic.
>>> @@ -142,15 +153,58 @@ Properties string UUID [readonly]
>>> uint16 | Description: Description of the
>> characteristic defined
>>> | in a high layer profile.
>>>
>>> - array{byte} Value [readwrite]
>>> -
>>> - Raw value of the Characteristic Value attribute.
>>> -
>>> string Representation (of the binary Value) [readonly]
>>>
>>> Friendly representation of the Characteristic Value
>>> based on the format attribute.
>>>
>>> + boolean Broadcast [readwrite]
>>> +
>>> + Indicates whether this characteristic is broadcasted or
>> not.
>>> + If GATT Server Characteristic Configuration descriptor
>>> + is not available for this characteristic, or if the
>> characteristic
>>> + properties do not allow this, writing to this property
>> is not
>>> + allowed.
>>> +
>>> + boolean Indicate [readwrite]
>>> +
>>> + Indicates whether this characteristic is notified or
>> not.
>>> + If GATT Client Characteristic Configuration descriptor
>>> + is not available for this characteristic, or if the
>> characteristic
>>> + properties do not allow this, writing to this property
>> is not
>>> + allowed.
>>> +
>>> + boolean Notify [readwrite]
>>> +
>>> + Indicates whether this characteristic is indicated or
>> not.
>>> + If GATT Client Characteristic Configuration descriptor
>>> + is not available for this characteristic, or if the
>> characteristic
>>> + properties do not allow this, writing to this property
>> is not
>>> + allowed.
>>> +
>>> + boolean Readable [readonly]
>>> +
>>> + Indicates wether this characteristic value can be read.
>>> +
>>> + boolean WritableNoRsp [readonly]
>>> +
>>> + Indicates whether this characteristic value can be
>> written without
>>> + response.
>>> +
>>> + boolean WritableRsp [readonly]
>>> +
>>> + Indicates whether this characteristic can be written
>> with response.
>>> +
>>> + boolean WritableSigned [readonly]
>
> The WritableSigned is going to be changed to WritableAuth (this is true for both Authenticated link or signed GATT Write commands).
>
>>> +
>>> + Indicates whether this characteristic can be written
>> with signed
>>> + authentication method.
>>> +
>>> + boolean WritableReliable [readonly]
>>> +
>>> + Indicates whether this characteristic can be written
>> with the
>>> + reliable write procedure.
>>> +
>>>
>>> Characteristic Watcher hierarchy
>>> ===============================
>>>
>>> --------8<-----------
>>>
>>> I would appreciate your comments on this.
>>
>> The additional properties also seem like nice additions, and work well
>> to provide clear differentiation between the characteristic value and
>> other metadata.
>>
>> Thanks for the proposal!
>>
>
> Thanks for the review. Do these changes break the current implementation of your solution?
>
> If there are no more comments, I think I'll start sending patches for the required changes to support this.

The changes should be compatible with our Android code, provided we
update the DBus method calls to use the new names and properties.

Brian & Inga (cc'd) know all the details and should be available to
comment later today.

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

2011-10-27 08:22:20

[permalink] [raw]

Subject: RE: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Hi Mat.

> -----Original Message-----
> From: Mat Martineau [mailto:[email protected]]
> Sent: Thursday, October 27, 2011 9:17 AM
> Subject: Re: GATT Dbus API on BlueZ - attirbute-api.txt modifications
>
> Hi Chen -
>
> On Wed, 26 Oct 2011, Ganir, Chen wrote:
>
> > Hi.
> >
> > Here's my proposal for some modifications to the existing attribute-
> api.doc file.
> >
> > These additions include :
> > 1. Support for more properties, such as writable, readable, notify
> > 2. Remove Value from the properties, and handle write/read with
> specific functions. This was done since currently the value is only
> read once from the server, and there is no way to refresh the value
> using the dbus API. In addition, the value can be written in multiple
> ways (we currently support write with response and write without
> response, but future write methods include write reliable and write
> signed, which may be required by profiles in the future).
> >
> > Here's the diff from the current docs/attribute-api.txt file :
> >
> > ---->8---------------
> > diff --git a/doc/attribute-api.txt b/doc/attribute-api.txt
> > index 98d7f30..fbc6957 100644
> > --- a/doc/attribute-api.txt
> > +++ b/doc/attribute-api.txt
> > @@ -112,6 +112,17 @@ Methods dict GetProperties()
> >
> > Possible Errors: org.bluez.Error.InvalidArguments
> >
> > + array{byte} ReadValue()
> > +
> > + Read the value of the characteristic.
> > +
>
> One aspect of the current client implementation is that it caches the
> read values within bluetoothd. Do you prefer to completely eliminate
> that storage and always have ReadValue fetch from the remote device?
>
> If there's still a use case for locally cached values (maybe there
> isn't), a "method" parameter could specify the source of the data. It
> could also provide some future-proofing for ReadValue.
>

I do not see any reason why caching of data is required. Data should be read directly from the GATT server. Caching it may cause inconsistency. There is also no reason to read the values on characteristic object creation in the client.c file. I think the decision of reading the value should be by the user and not the system.
In addition, Value is not a property of a characteristic.

> > + int WriteValue(array{byte} value, int method)
> > +
> > + Write the value of the characteristic, with specified
> method :
> > + 0: Write without response. Always return 0.
> > + 1: Write with response. Return server response code.
> > + Other write methods will be added in the future.
> > +
>
> This addresses the shortcomings we saw with SetProperty, and also
> gives options for the future - seems like a great idea to me!
>
>
> > Properties string UUID [readonly]
> >
> > UUID128 of this characteristic.
> > @@ -142,15 +153,58 @@ Properties string UUID [readonly]
> > uint16 | Description: Description of the
> characteristic defined
> > | in a high layer profile.
> >
> > - array{byte} Value [readwrite]
> > -
> > - Raw value of the Characteristic Value attribute.
> > -
> > string Representation (of the binary Value) [readonly]
> >
> > Friendly representation of the Characteristic Value
> > based on the format attribute.
> >
> > + boolean Broadcast [readwrite]
> > +
> > + Indicates whether this characteristic is broadcasted or
> not.
> > + If GATT Server Characteristic Configuration descriptor
> > + is not available for this characteristic, or if the
> characteristic
> > + properties do not allow this, writing to this property
> is not
> > + allowed.
> > +
> > + boolean Indicate [readwrite]
> > +
> > + Indicates whether this characteristic is notified or
> not.
> > + If GATT Client Characteristic Configuration descriptor
> > + is not available for this characteristic, or if the
> characteristic
> > + properties do not allow this, writing to this property
> is not
> > + allowed.
> > +
> > + boolean Notify [readwrite]
> > +
> > + Indicates whether this characteristic is indicated or
> not.
> > + If GATT Client Characteristic Configuration descriptor
> > + is not available for this characteristic, or if the
> characteristic
> > + properties do not allow this, writing to this property
> is not
> > + allowed.
> > +
> > + boolean Readable [readonly]
> > +
> > + Indicates wether this characteristic value can be read.
> > +
> > + boolean WritableNoRsp [readonly]
> > +
> > + Indicates whether this characteristic value can be
> written without
> > + response.
> > +
> > + boolean WritableRsp [readonly]
> > +
> > + Indicates whether this characteristic can be written
> with response.
> > +
> > + boolean WritableSigned [readonly]

The WritableSigned is going to be changed to WritableAuth (this is true for both Authenticated link or signed GATT Write commands).

> > +
> > + Indicates whether this characteristic can be written
> with signed
> > + authentication method.
> > +
> > + boolean WritableReliable [readonly]
> > +
> > + Indicates whether this characteristic can be written
> with the
> > + reliable write procedure.
> > +
> >
> > Characteristic Watcher hierarchy
> > ===============================
> >
> > --------8<-----------
> >
> > I would appreciate your comments on this.
>
> The additional properties also seem like nice additions, and work well
> to provide clear differentiation between the characteristic value and
> other metadata.
>
> Thanks for the proposal!
>

Thanks for the review. Do these changes break the current implementation of your solution?

If there are no more comments, I think I'll start sending patches for the required changes to support this.

Thanks,
Chen Ganir

2011-10-27 08:00:30

by Andrei Emeltchenko

[permalink] [raw]

Subject: Re: [PATCH bluez 0/2] Add BT 3.0+HS signalling definitions

Hi Peter,

On Wed, Oct 26, 2011 at 10:44:51AM -0700, Peter Krystad wrote:
> Hi Andrei,
>
> > Patches looks OK (taking that typedef is a standard way in bluez).
> >
> > The only note: I think that we need to differentiate AMP and A2MP. When we
> > are dealing with A2MP protocol we could name structures / functions as
> > a2mp_*
>
> I tried to follow the convention of the spec, which uses A2MP only when
> referring to the protocol as a whole and when referring to the feature mask
> and the fixed channel. It uses AMP or 'AMP Protocol' in all other places,
> including individual protocol elements.
>
> Consider this, and if you still think it should be a2mp_ I'll do it.

I think we can do a little brainstorming here :-)

My idea is to use name A2MP when we are talking about
"AMP Manager Protocol" and use AMP when we are dealing with
"Alternate MAC PHY" controller.

BTW: in your kernel repository you use correct names
https://www.codeaurora.org/gitweb/quic/la/?p=kernel/msm.git;a=blob;f=include/net/bluetooth/amp.h;h=4b080791ffc74ad17fd504583f7949cd4df1fa82;hb=refs/heads/msm-3.0
...
/* A2MP command codes */
#define A2MP_COMMAND_REJ 0x01
#define A2MP_DISCOVER_REQ 0x02
#define A2MP_DISCOVER_RSP 0x03
...

in your patch "[PATCH bluez 2/2] Add A2MP definitions" you use AMP instead
of A2MP.

Best regards
Andrei Emeltchenko

2011-10-27 07:16:45

by Mat Martineau

[permalink] [raw]

Subject: Re: GATT Dbus API on BlueZ - attirbute-api.txt modifications

Hi Chen -

On Wed, 26 Oct 2011, Ganir, Chen wrote:

> Hi.
>
> Here's my proposal for some modifications to the existing attribute-api.doc file.
>
> These additions include :
> 1. Support for more properties, such as writable, readable, notify
> 2. Remove Value from the properties, and handle write/read with specific functions. This was done since currently the value is only read once from the server, and there is no way to refresh the value using the dbus API. In addition, the value can be written in multiple ways (we currently support write with response and write without response, but future write methods include write reliable and write signed, which may be required by profiles in the future).
>
> Here's the diff from the current docs/attribute-api.txt file :
>
> ---->8---------------
> diff --git a/doc/attribute-api.txt b/doc/attribute-api.txt
> index 98d7f30..fbc6957 100644
> --- a/doc/attribute-api.txt
> +++ b/doc/attribute-api.txt
> @@ -112,6 +112,17 @@ Methods dict GetProperties()
>
> Possible Errors: org.bluez.Error.InvalidArguments
>
> + array{byte} ReadValue()
> +
> + Read the value of the characteristic.
> +

One aspect of the current client implementation is that it caches the
read values within bluetoothd. Do you prefer to completely eliminate
that storage and always have ReadValue fetch from the remote device?

If there's still a use case for locally cached values (maybe there
isn't), a "method" parameter could specify the source of the data. It
could also provide some future-proofing for ReadValue.

> + int WriteValue(array{byte} value, int method)
> +
> + Write the value of the characteristic, with specified method :
> + 0: Write without response. Always return 0.
> + 1: Write with response. Return server response code.
> + Other write methods will be added in the future.
> +

This addresses the shortcomings we saw with SetProperty, and also
gives options for the future - seems like a great idea to me!

> Properties string UUID [readonly]
>
> UUID128 of this characteristic.
> @@ -142,15 +153,58 @@ Properties string UUID [readonly]
> uint16 | Description: Description of the characteristic defined
> | in a high layer profile.
>
> - array{byte} Value [readwrite]
> -
> - Raw value of the Characteristic Value attribute.
> -
> string Representation (of the binary Value) [readonly]
>
> Friendly representation of the Characteristic Value
> based on the format attribute.
>
> + boolean Broadcast [readwrite]
> +
> + Indicates whether this characteristic is broadcasted or not.
> + If GATT Server Characteristic Configuration descriptor
> + is not available for this characteristic, or if the characteristic
> + properties do not allow this, writing to this property is not
> + allowed.
> +
> + boolean Indicate [readwrite]
> +
> + Indicates whether this characteristic is notified or not.
> + If GATT Client Characteristic Configuration descriptor
> + is not available for this characteristic, or if the characteristic
> + properties do not allow this, writing to this property is not
> + allowed.
> +
> + boolean Notify [readwrite]
> +
> + Indicates whether this characteristic is indicated or not.
> + If GATT Client Characteristic Configuration descriptor
> + is not available for this characteristic, or if the characteristic
> + properties do not allow this, writing to this property is not
> + allowed.
> +
> + boolean Readable [readonly]
> +
> + Indicates wether this characteristic value can be read.
> +
> + boolean WritableNoRsp [readonly]
> +
> + Indicates whether this characteristic value can be written without
> + response.
> +
> + boolean WritableRsp [readonly]
> +
> + Indicates whether this characteristic can be written with response.
> +
> + boolean WritableSigned [readonly]
> +
> + Indicates whether this characteristic can be written with signed
> + authentication method.
> +
> + boolean WritableReliable [readonly]
> +
> + Indicates whether this characteristic can be written with the
> + reliable write procedure.
> +
>
> Characteristic Watcher hierarchy
> ===============================
>
> --------8<-----------
>
> I would appreciate your comments on this.

The additional properties also seem like nice additions, and work well
to provide clear differentiation between the characteristic value and
other metadata.

Thanks for the proposal!

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

2011-10-26 17:44:51

[permalink] [raw]

Subject: RE: [PATCH bluez 0/2] Add BT 3.0+HS signalling definitions

Hi Andrei,

> Patches looks OK (taking that typedef is a standard way in bluez).
>
> The only note: I think that we need to differentiate AMP and A2MP. When we
> are dealing with A2MP protocol we could name structures / functions as
> a2mp_*

I tried to follow the convention of the spec, which uses A2MP only when
referring to the protocol as a whole and when referring to the feature mask
and the fixed channel. It uses AMP or 'AMP Protocol' in all other places,
including individual protocol elements.

Consider this, and if you still think it should be a2mp_ I'll do it.

Peter.

--Peter Krystad
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

2011-10-26 07:54:42

by Andrei Emeltchenko

[permalink] [raw]

Subject: Re: Add BT 3.0+HS signalling definitions

Hi Timur,

On Wed, Oct 26, 2011 at 02:15:38AM +0200, Timur Mehrvarz wrote:
> >> When will HS become available to Linux devices (PC/Smartphone)?
> >> Thank you.
> > Upstreaming HS to BlueZ is just getting started, it will take some time.
> > You can check bluetooth.org for existing qualified 3.0+HS listings.
> Thank you very much for your response. I see several '+HS' items there,
> but the info leaves me with even more questions.
>
> It is difficult for me to assess what you mean by some time. But your
> answer implies that HS/AMP will indeed, at some point, be part of BlueZ.
> I have actually expected AMP to be transparent to the stack and to be
> implemented in a layer below. It's hard to get any detailed info on this
> matter. And this goes on for, what, two years? Three?
>
> I'm really just looking for some combination of devices that would allow
> me to establish HS/AMP RFCOMM connections myself. Anything that is
> available today? The new Macbook Air's and iPhone's are said to support
> BT4 (including HS?). Samsung Galaxy2 devices (and the new Galaxy Nexus
> devices) are said to support BT3+HS (true?). Or maybe I should just
> order some of these new BT3+HS dongles and try under Windows? Help.

I do not know any public device having HS feature fully implemented. I
would be happy if someone correct me here.

I have some dongles marked as "BT3+HS" but apparently it is marketing
approach :-( as they do not report via USB level according to specs and
otherwise behave like 2.1 devices.

Best regards
Andrei Emeltchenko

2011-10-26 07:36:44

by Andrei Emeltchenko

[permalink] [raw]

Subject: Re: [PATCH bluez 0/2] Add BT 3.0+HS signalling definitions

Hi Peter,

On Mon, Oct 24, 2011 at 04:06:17PM -0700, Peter Krystad wrote:
> This patchset adds definitions for BT 3.0+HS signalling

Patches looks OK (taking that typedef is a standard way in bluez).

The only note: I think that we need to differentiate AMP and A2MP. When we
are dealing with A2MP protocol we could name structures / functions as
a2mp_*

Best regards
Andrei Emeltchenko

>
> Peter Krystad (2):
> Add L2CAP Create/Move Channel definitions
> Add A2MP definitions
>
> lib/a2mp.h | 131 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> lib/l2cap.h | 44 ++++++++++++++++++++
> 2 files changed, 175 insertions(+), 0 deletions(-)
> create mode 100644 lib/a2mp.h
>
> --
> 1.7.7
>
> --
> Peter Krystad
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
> --
> 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

2011-10-26 07:20:19

[permalink] [raw]

Subject: GATT Dbus API on BlueZ - attirbute-api.txt modifications

SGkuDQoNCkhlcmUncyBteSBwcm9wb3NhbCBmb3Igc29tZSBtb2RpZmljYXRpb25zIHRvIHRoZSBl
eGlzdGluZyBhdHRyaWJ1dGUtYXBpLmRvYyBmaWxlLg0KDQpUaGVzZSBhZGRpdGlvbnMgaW5jbHVk
ZSA6DQoxLiBTdXBwb3J0IGZvciBtb3JlIHByb3BlcnRpZXMsIHN1Y2ggYXMgd3JpdGFibGUsIHJl
YWRhYmxlLCBub3RpZnkNCjIuIFJlbW92ZSBWYWx1ZSBmcm9tIHRoZSBwcm9wZXJ0aWVzLCBhbmQg
aGFuZGxlIHdyaXRlL3JlYWQgd2l0aCBzcGVjaWZpYyBmdW5jdGlvbnMuIFRoaXMgd2FzIGRvbmUg
c2luY2UgY3VycmVudGx5IHRoZSB2YWx1ZSBpcyBvbmx5IHJlYWQgb25jZSBmcm9tIHRoZSBzZXJ2
ZXIsIGFuZCB0aGVyZSBpcyBubyB3YXkgdG8gcmVmcmVzaCB0aGUgdmFsdWUgdXNpbmcgdGhlIGRi
dXMgQVBJLiBJbiBhZGRpdGlvbiwgdGhlIHZhbHVlIGNhbiBiZSB3cml0dGVuIGluIG11bHRpcGxl
IHdheXMgKHdlIGN1cnJlbnRseSBzdXBwb3J0IHdyaXRlIHdpdGggcmVzcG9uc2UgYW5kIHdyaXRl
IHdpdGhvdXQgcmVzcG9uc2UsIGJ1dCBmdXR1cmUgd3JpdGUgbWV0aG9kcyBpbmNsdWRlIHdyaXRl
IHJlbGlhYmxlIGFuZCB3cml0ZSBzaWduZWQsIHdoaWNoIG1heSBiZSByZXF1aXJlZCBieSBwcm9m
aWxlcyBpbiB0aGUgZnV0dXJlKS4NCg0KSGVyZSdzIHRoZSBkaWZmIGZyb20gdGhlIGN1cnJlbnQg
ZG9jcy9hdHRyaWJ1dGUtYXBpLnR4dCBmaWxlIDoNCg0KLS0tLT44LS0tLS0tLS0tLS0tLS0tDQpk
aWZmIC0tZ2l0IGEvZG9jL2F0dHJpYnV0ZS1hcGkudHh0IGIvZG9jL2F0dHJpYnV0ZS1hcGkudHh0
DQppbmRleCA5OGQ3ZjMwLi5mYmM2OTU3IDEwMDY0NA0KLS0tIGEvZG9jL2F0dHJpYnV0ZS1hcGku
dHh0DQorKysgYi9kb2MvYXR0cmlidXRlLWFwaS50eHQNCkBAIC0xMTIsNiArMTEyLDE3IEBAIE1l
dGhvZHMJCWRpY3QgR2V0UHJvcGVydGllcygpDQogDQogCQkJUG9zc2libGUgRXJyb3JzOiBvcmcu
Ymx1ZXouRXJyb3IuSW52YWxpZEFyZ3VtZW50cw0KIA0KKwkJYXJyYXl7Ynl0ZX0gUmVhZFZhbHVl
KCkNCisNCisJCSAgICBSZWFkIHRoZSB2YWx1ZSBvZiB0aGUgY2hhcmFjdGVyaXN0aWMuDQorDQor
CQlpbnQgV3JpdGVWYWx1ZShhcnJheXtieXRlfSB2YWx1ZSwgaW50IG1ldGhvZCkNCisNCisJCSAg
ICBXcml0ZSB0aGUgdmFsdWUgb2YgdGhlIGNoYXJhY3RlcmlzdGljLCB3aXRoIHNwZWNpZmllZCBt
ZXRob2QgOg0KKwkJICAgIDA6IFdyaXRlIHdpdGhvdXQgcmVzcG9uc2UuIEFsd2F5cyByZXR1cm4g
MC4NCisJCSAgICAxOiBXcml0ZSB3aXRoIHJlc3BvbnNlLiBSZXR1cm4gc2VydmVyIHJlc3BvbnNl
IGNvZGUuDQorCQkgICAgT3RoZXIgd3JpdGUgbWV0aG9kcyB3aWxsIGJlIGFkZGVkIGluIHRoZSBm
dXR1cmUuDQorDQogUHJvcGVydGllcyAJc3RyaW5nIFVVSUQgW3JlYWRvbmx5XQ0KIA0KIAkJCVVV
SUQxMjggb2YgdGhpcyBjaGFyYWN0ZXJpc3RpYy4NCkBAIC0xNDIsMTUgKzE1Myw1OCBAQCBQcm9w
ZXJ0aWVzIAlzdHJpbmcgVVVJRCBbcmVhZG9ubHldDQogCQkJICB1aW50MTYgfCBEZXNjcmlwdGlv
bjogRGVzY3JpcHRpb24gb2YgdGhlIGNoYXJhY3RlcmlzdGljIGRlZmluZWQNCiAJCQkgICAgICAg
ICB8IGluIGEgaGlnaCBsYXllciBwcm9maWxlLg0KIA0KLQkJYXJyYXl7Ynl0ZX0gVmFsdWUgW3Jl
YWR3cml0ZV0NCi0NCi0JCQlSYXcgdmFsdWUgb2YgdGhlIENoYXJhY3RlcmlzdGljIFZhbHVlIGF0
dHJpYnV0ZS4NCi0NCiAJCXN0cmluZyBSZXByZXNlbnRhdGlvbiAob2YgdGhlIGJpbmFyeSBWYWx1
ZSkgW3JlYWRvbmx5XQ0KIA0KIAkJCUZyaWVuZGx5IHJlcHJlc2VudGF0aW9uIG9mIHRoZSBDaGFy
YWN0ZXJpc3RpYyBWYWx1ZQ0KIAkJCWJhc2VkIG9uIHRoZSBmb3JtYXQgYXR0cmlidXRlLg0KIA0K
KwkJYm9vbGVhbiBCcm9hZGNhc3QgW3JlYWR3cml0ZV0NCisNCisJCSAgICBJbmRpY2F0ZXMgd2hl
dGhlciB0aGlzIGNoYXJhY3RlcmlzdGljIGlzIGJyb2FkY2FzdGVkIG9yIG5vdC4NCisJCSAgICBJ
ZiBHQVRUIFNlcnZlciBDaGFyYWN0ZXJpc3RpYyBDb25maWd1cmF0aW9uIGRlc2NyaXB0b3INCisJ
CSAgICBpcyBub3QgYXZhaWxhYmxlIGZvciB0aGlzIGNoYXJhY3RlcmlzdGljLCBvciBpZiB0aGUg
Y2hhcmFjdGVyaXN0aWMNCisJCSAgICBwcm9wZXJ0aWVzIGRvIG5vdCBhbGxvdyB0aGlzLCB3cml0
aW5nIHRvIHRoaXMgcHJvcGVydHkgaXMgbm90DQorCQkgICAgYWxsb3dlZC4NCisNCisJCWJvb2xl
YW4gSW5kaWNhdGUgW3JlYWR3cml0ZV0NCisNCisJCSAgICBJbmRpY2F0ZXMgd2hldGhlciB0aGlz
IGNoYXJhY3RlcmlzdGljIGlzIG5vdGlmaWVkIG9yIG5vdC4NCisJCSAgICBJZiBHQVRUIENsaWVu
dCBDaGFyYWN0ZXJpc3RpYyBDb25maWd1cmF0aW9uIGRlc2NyaXB0b3INCisJCSAgICBpcyBub3Qg
YXZhaWxhYmxlIGZvciB0aGlzIGNoYXJhY3RlcmlzdGljLCBvciBpZiB0aGUgY2hhcmFjdGVyaXN0
aWMNCisJCSAgICBwcm9wZXJ0aWVzIGRvIG5vdCBhbGxvdyB0aGlzLCB3cml0aW5nIHRvIHRoaXMg
cHJvcGVydHkgaXMgbm90DQorCQkgICAgYWxsb3dlZC4NCisNCisJCWJvb2xlYW4gTm90aWZ5IFty
ZWFkd3JpdGVdDQorDQorCQkgICAgSW5kaWNhdGVzIHdoZXRoZXIgdGhpcyBjaGFyYWN0ZXJpc3Rp
YyBpcyBpbmRpY2F0ZWQgb3Igbm90Lg0KKwkJICAgIElmIEdBVFQgQ2xpZW50IENoYXJhY3Rlcmlz
dGljIENvbmZpZ3VyYXRpb24gZGVzY3JpcHRvcg0KKwkJICAgIGlzIG5vdCBhdmFpbGFibGUgZm9y
IHRoaXMgY2hhcmFjdGVyaXN0aWMsIG9yIGlmIHRoZSBjaGFyYWN0ZXJpc3RpYw0KKwkJICAgIHBy
b3BlcnRpZXMgZG8gbm90IGFsbG93IHRoaXMsIHdyaXRpbmcgdG8gdGhpcyBwcm9wZXJ0eSBpcyBu
b3QNCisJCSAgICBhbGxvd2VkLg0KKw0KKwkJYm9vbGVhbiBSZWFkYWJsZSBbcmVhZG9ubHldDQor
DQorCQkgICAgSW5kaWNhdGVzIHdldGhlciB0aGlzIGNoYXJhY3RlcmlzdGljIHZhbHVlIGNhbiBi
ZSByZWFkLg0KKw0KKwkJYm9vbGVhbiBXcml0YWJsZU5vUnNwIFtyZWFkb25seV0NCisNCisJCSAg
ICBJbmRpY2F0ZXMgd2hldGhlciB0aGlzIGNoYXJhY3RlcmlzdGljIHZhbHVlIGNhbiBiZSB3cml0
dGVuIHdpdGhvdXQNCisJCSAgICByZXNwb25zZS4NCisNCisJCWJvb2xlYW4gV3JpdGFibGVSc3Ag
W3JlYWRvbmx5XQ0KKw0KKwkJICAgIEluZGljYXRlcyB3aGV0aGVyIHRoaXMgY2hhcmFjdGVyaXN0
aWMgY2FuIGJlIHdyaXR0ZW4gd2l0aCByZXNwb25zZS4NCisNCisJCWJvb2xlYW4gV3JpdGFibGVT
aWduZWQgW3JlYWRvbmx5XQ0KKw0KKwkJICAgIEluZGljYXRlcyB3aGV0aGVyIHRoaXMgY2hhcmFj
dGVyaXN0aWMgY2FuIGJlIHdyaXR0ZW4gd2l0aCBzaWduZWQNCisJCSAgICBhdXRoZW50aWNhdGlv
biBtZXRob2QuDQorDQorCQlib29sZWFuIFdyaXRhYmxlUmVsaWFibGUgW3JlYWRvbmx5XQ0KKw0K
KwkJICAgIEluZGljYXRlcyB3aGV0aGVyIHRoaXMgY2hhcmFjdGVyaXN0aWMgY2FuIGJlIHdyaXR0
ZW4gd2l0aCB0aGUNCisJCSAgICByZWxpYWJsZSB3cml0ZSBwcm9jZWR1cmUuDQorDQogDQogQ2hh
cmFjdGVyaXN0aWMgV2F0Y2hlciBoaWVyYXJjaHkNCiA9PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09DQoNCi0tLS0tLS0tODwtLS0tLS0tLS0tLQ0KDQpJIHdvdWxkIGFwcHJlY2lhdGUgeW91
ciBjb21tZW50cyBvbiB0aGlzLg0KDQpUaGFua3MsDQpDaGVuIEdhbmlyDQo=

2011-10-26 00:15:38

[permalink] [raw]

Subject: Re: Add BT 3.0+HS signalling definitions

On 25.10.2011 16:41, Peter Krystad wrote:
>
> Hi Timur,
>
>> When will HS become available to Linux devices (PC/Smartphone)?
>> Thank you.
>>
>> Timur
>
> Upstreaming HS to BlueZ is just getting started, it will take some time.
>
> You can check bluetooth.org for existing qualified 3.0+HS listings.
>
> Peter.
>

Hi Peter,

Thank you very much for your response. I see several '+HS' items there,
but the info leaves me with even more questions.

It is difficult for me to assess what you mean by some time. But your
answer implies that HS/AMP will indeed, at some point, be part of BlueZ.
I have actually expected AMP to be transparent to the stack and to be
implemented in a layer below. It's hard to get any detailed info on this
matter. And this goes on for, what, two years? Three?

I'm really just looking for some combination of devices that would allow
me to establish HS/AMP RFCOMM connections myself. Anything that is
available today? The new Macbook Air's and iPhone's are said to support
BT4 (including HS?). Samsung Galaxy2 devices (and the new Galaxy Nexus
devices) are said to support BT3+HS (true?). Or maybe I should just
order some of these new BT3+HS dongles and try under Windows? Help.

Timur

2011-10-26 00:06:20

by Claudio Takahasi

[permalink] [raw]

Subject: Re: GATT Dbus API on BlueZ

Hi Chen Ganir,

On Tue, Oct 25, 2011 at 10:19 AM, Ganir, Chen <[email protected]> wrote:
> Claudio
>
>> -----Original Message-----
>> From: Claudio Takahasi [mailto:[email protected]]
>> Sent: Tuesday, October 25, 2011 2:53 PM
>> To: Ganir, Chen
>> Cc: [email protected]
>> Subject: Re: GATT Dbus API on BlueZ
>>
>> Hi Chen Ganir,
>>
>> On Tue, Oct 25, 2011 at 4:38 AM, Ganir, Chen <[email protected]> wrote:
>> > Hi.
>> >
>> > Currently, the DBUS API for GATT Client will allow discovering
>> characteristics and services, connecting to the peer device if
>> required. However, when the operation is done, the link is dropped (no
>> more attio registered). The only way to register a global gatt_service-
>> >attio and keep the connection alive all the time is to register a
>> characteristic watcher. Is there another way to do it ? I'd like to
>> keep the link a live (for example, implementing a proximity link loss
>> profile on top of the DBUS GATT Client) without the need for a watcher.
>> >
>> > Thanks,
>> > Chen Ganir
>> >
>> >
>>
>> At the moment there isn't another way to keep the link up. GATT
>> Profiles have different connection requirements, you will need a
>> dynamic mechanism anyway. The callback registration is the mechanism
>> to inform the core that connection is required. If you keep the
>> connection always up it will be necessary a function to get access to
>> the GAttrib instance and a way to notify that the connection has been
>> established.
>>
>> I tried to remove the association between GAttrib and GIOChannel. The
>> idea was to create GAttrib even if the link is disconnected, but the
>> effort doesn't worth. We will have problems with MTU and GATT
>> abstractions for read long.
>>
>> Do have another suggestion to control connections and control the ATT
>> request/response queue?
>>
>> BR,
>> Claudio
>
> Thanks for the reply.
>
> However, I did not quite understand your response, nor the relevancy to read long.
>
> My basic requirement is to allow a profile to control the connection - meaning that if I implement a proximity or other profile over dbus, I would like to be able to tell it to connect, discover services, discover characteristics, poll from time to time the value of a certain GATT char, or simply idle and wait for disconnection (like proximity does) or notification/indication. When using dbus-send I end up with a disconnected link at the end of the process. Do you think the disconnection results from the termination of the dbus-send application, or is it something else ?
>
> Thanks,
> Chen Ganir.
>
>

You are disabling all the GATT plugins, so my suggestion is: open the
ATT L2CAP socket directly in your JNI bindings. Using this approach
you will have all the control that you need.

We track the users/clients based on the D-Bus connection, if the
client(dbus-send or any other) leaves the bus the core will disconnect
link if watcher list is empty.

BR,
Claudio.

2011-10-25 14:41:38

[permalink] [raw]

Subject: RE: Add BT 3.0+HS signalling definitions

Hi Timur,

> When will HS become available to Linux devices (PC/Smartphone)?
> Thank you.
>
> Timur

Upstreaming HS to BlueZ is just getting started, it will take some time.

You can check bluetooth.org for existing qualified 3.0+HS listings.

Peter.

--Peter Krystad
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

2011-10-25 13:19:05

[permalink] [raw]

Subject: RE: GATT Dbus API on BlueZ

Q2xhdWRpbw0KDQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZyb206IENsYXVkaW8g
VGFrYWhhc2kgW21haWx0bzpjbGF1ZGlvLnRha2FoYXNpQG9wZW5ib3NzYS5vcmddDQo+IFNlbnQ6
IFR1ZXNkYXksIE9jdG9iZXIgMjUsIDIwMTEgMjo1MyBQTQ0KPiBUbzogR2FuaXIsIENoZW4NCj4g
Q2M6IGxpbnV4LWJsdWV0b290aEB2Z2VyLmtlcm5lbC5vcmcNCj4gU3ViamVjdDogUmU6IEdBVFQg
RGJ1cyBBUEkgb24gQmx1ZVoNCj4gDQo+IEhpIENoZW4gR2FuaXIsDQo+IA0KPiBPbiBUdWUsIE9j
dCAyNSwgMjAxMSBhdCA0OjM4IEFNLCBHYW5pciwgQ2hlbiA8Y2hlbi5nYW5pckB0aS5jb20+IHdy
b3RlOg0KPiA+IEhpLg0KPiA+DQo+ID4gQ3VycmVudGx5LCB0aGUgREJVUyBBUEkgZm9yIEdBVFQg
Q2xpZW50IHdpbGwgYWxsb3cgZGlzY292ZXJpbmcNCj4gY2hhcmFjdGVyaXN0aWNzIGFuZCBzZXJ2
aWNlcywgY29ubmVjdGluZyB0byB0aGUgcGVlciBkZXZpY2UgaWYNCj4gcmVxdWlyZWQuIEhvd2V2
ZXIsIHdoZW4gdGhlIG9wZXJhdGlvbiBpcyBkb25lLCB0aGUgbGluayBpcyBkcm9wcGVkIChubw0K
PiBtb3JlIGF0dGlvIHJlZ2lzdGVyZWQpLiBUaGUgb25seSB3YXkgdG8gcmVnaXN0ZXIgYSBnbG9i
YWwgZ2F0dF9zZXJ2aWNlLQ0KPiA+YXR0aW8gYW5kIGtlZXAgdGhlIGNvbm5lY3Rpb24gYWxpdmUg
YWxsIHRoZSB0aW1lIGlzIHRvIHJlZ2lzdGVyIGENCj4gY2hhcmFjdGVyaXN0aWMgd2F0Y2hlci4g
SXMgdGhlcmUgYW5vdGhlciB3YXkgdG8gZG8gaXQgPyBJJ2QgbGlrZSB0bw0KPiBrZWVwIHRoZSBs
aW5rIGEgbGl2ZSAoZm9yIGV4YW1wbGUsIGltcGxlbWVudGluZyBhIHByb3hpbWl0eSBsaW5rIGxv
c3MNCj4gcHJvZmlsZSBvbiB0b3Agb2YgdGhlIERCVVMgR0FUVCBDbGllbnQpIHdpdGhvdXQgdGhl
IG5lZWQgZm9yIGEgd2F0Y2hlci4NCj4gPg0KPiA+IFRoYW5rcywNCj4gPiBDaGVuIEdhbmlyDQo+
ID4NCj4gPg0KPiANCj4gQXQgdGhlIG1vbWVudCB0aGVyZSBpc24ndCBhbm90aGVyIHdheSB0byBr
ZWVwIHRoZSBsaW5rIHVwLiBHQVRUDQo+IFByb2ZpbGVzIGhhdmUgZGlmZmVyZW50IGNvbm5lY3Rp
b24gcmVxdWlyZW1lbnRzLCB5b3Ugd2lsbCBuZWVkIGENCj4gZHluYW1pYyBtZWNoYW5pc20gYW55
d2F5LiBUaGUgY2FsbGJhY2sgcmVnaXN0cmF0aW9uIGlzIHRoZSBtZWNoYW5pc20NCj4gdG8gaW5m
b3JtIHRoZSBjb3JlIHRoYXQgY29ubmVjdGlvbiBpcyByZXF1aXJlZC4gSWYgeW91IGtlZXAgdGhl
DQo+IGNvbm5lY3Rpb24gYWx3YXlzIHVwIGl0IHdpbGwgYmUgbmVjZXNzYXJ5IGEgZnVuY3Rpb24g
dG8gZ2V0IGFjY2VzcyB0bw0KPiB0aGUgR0F0dHJpYiBpbnN0YW5jZSBhbmQgYSB3YXkgdG8gbm90
aWZ5IHRoYXQgdGhlIGNvbm5lY3Rpb24gaGFzIGJlZW4NCj4gZXN0YWJsaXNoZWQuDQo+IA0KPiBJ
IHRyaWVkIHRvIHJlbW92ZSB0aGUgYXNzb2NpYXRpb24gYmV0d2VlbiBHQXR0cmliIGFuZCBHSU9D
aGFubmVsLiBUaGUNCj4gaWRlYSB3YXMgdG8gY3JlYXRlIEdBdHRyaWIgZXZlbiBpZiB0aGUgbGlu
ayBpcyBkaXNjb25uZWN0ZWQsIGJ1dCB0aGUNCj4gZWZmb3J0IGRvZXNuJ3Qgd29ydGguIFdlIHdp
bGwgaGF2ZSBwcm9ibGVtcyB3aXRoIE1UVSBhbmQgR0FUVA0KPiBhYnN0cmFjdGlvbnMgZm9yIHJl
YWQgbG9uZy4NCj4gDQo+IERvIGhhdmUgYW5vdGhlciBzdWdnZXN0aW9uIHRvIGNvbnRyb2wgY29u
bmVjdGlvbnMgYW5kIGNvbnRyb2wgdGhlIEFUVA0KPiByZXF1ZXN0L3Jlc3BvbnNlIHF1ZXVlPw0K
PiANCj4gQlIsDQo+IENsYXVkaW8NCg0KVGhhbmtzIGZvciB0aGUgcmVwbHkuDQoNCkhvd2V2ZXIs
IEkgZGlkIG5vdCBxdWl0ZSB1bmRlcnN0YW5kIHlvdXIgcmVzcG9uc2UsIG5vciB0aGUgcmVsZXZh
bmN5IHRvIHJlYWQgbG9uZy4NCg0KTXkgYmFzaWMgcmVxdWlyZW1lbnQgaXMgdG8gYWxsb3cgYSBw
cm9maWxlIHRvIGNvbnRyb2wgdGhlIGNvbm5lY3Rpb24gLSBtZWFuaW5nIHRoYXQgaWYgSSBpbXBs
ZW1lbnQgYSBwcm94aW1pdHkgb3Igb3RoZXIgcHJvZmlsZSBvdmVyIGRidXMsIEkgd291bGQgbGlr
ZSB0byBiZSBhYmxlIHRvIHRlbGwgaXQgdG8gY29ubmVjdCwgZGlzY292ZXIgc2VydmljZXMsIGRp
c2NvdmVyIGNoYXJhY3RlcmlzdGljcywgcG9sbCBmcm9tIHRpbWUgdG8gdGltZSB0aGUgdmFsdWUg
b2YgYSBjZXJ0YWluIEdBVFQgY2hhciwgb3Igc2ltcGx5IGlkbGUgYW5kIHdhaXQgZm9yIGRpc2Nv
bm5lY3Rpb24gKGxpa2UgcHJveGltaXR5IGRvZXMpIG9yIG5vdGlmaWNhdGlvbi9pbmRpY2F0aW9u
LiBXaGVuIHVzaW5nIGRidXMtc2VuZCBJIGVuZCB1cCB3aXRoIGEgZGlzY29ubmVjdGVkIGxpbmsg
YXQgdGhlIGVuZCBvZiB0aGUgcHJvY2Vzcy4gRG8geW91IHRoaW5rIHRoZSBkaXNjb25uZWN0aW9u
IHJlc3VsdHMgZnJvbSB0aGUgdGVybWluYXRpb24gb2YgdGhlIGRidXMtc2VuZCBhcHBsaWNhdGlv
biwgb3IgaXMgaXQgc29tZXRoaW5nIGVsc2UgPw0KDQpUaGFua3MsDQpDaGVuIEdhbmlyLg0KDQo=

2011-10-25 12:52:48

by Claudio Takahasi

[permalink] [raw]

Subject: Re: GATT Dbus API on BlueZ

Hi Chen Ganir,

On Tue, Oct 25, 2011 at 4:38 AM, Ganir, Chen <[email protected]> wrote:
> Hi.
>
> Currently, the DBUS API for GATT Client will allow discovering characteristics and services, connecting to the peer device if required. However, when the operation is done, the link is dropped (no more attio registered). The only way to register a global gatt_service->attio and keep the connection alive all the time is to register a characteristic watcher. Is there another way to do it ? I'd like to keep the link a live (for example, implementing a proximity link loss profile on top of the DBUS GATT Client) without the need for a watcher.
>
> Thanks,
> Chen Ganir
>
>

At the moment there isn't another way to keep the link up. GATT
Profiles have different connection requirements, you will need a
dynamic mechanism anyway. The callback registration is the mechanism
to inform the core that connection is required. If you keep the
connection always up it will be necessary a function to get access to
the GAttrib instance and a way to notify that the connection has been
established.

I tried to remove the association between GAttrib and GIOChannel. The
idea was to create GAttrib even if the link is disconnected, but the
effort doesn't worth. We will have problems with MTU and GATT
abstractions for read long.

Do have another suggestion to control connections and control the ATT
request/response queue?

BR,
Claudio

2011-10-25 07:38:14

[permalink] [raw]

Subject: GATT Dbus API on BlueZ

Hi.

Currently, the DBUS API for GATT Client will allow discovering characteristics and services, connecting to the peer device if required. However, when the operation is done, the link is dropped (no more attio registered). The only way to register a global gatt_service->attio and keep the connection alive all the time is to register a characteristic watcher. Is there another way to do it ? I'd like to keep the link a live (for example, implementing a proximity link loss profile on top of the DBUS GATT Client) without the need for a watcher.

Thanks,
Chen Ganir

2011-10-25 01:43:26

[permalink] [raw]

Subject: Add BT 3.0+HS signalling definitions

When will HS become available to Linux devices (PC/Smartphone)?
Thank you.

Timur

On 25.10.2011 01:06, Peter Krystad wrote:
> This patchset adds definitions for BT 3.0+HS signalling
>
> Peter Krystad (2):
> Add L2CAP Create/Move Channel definitions
> Add A2MP definitions
>
> lib/a2mp.h | 131 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> lib/l2cap.h | 44 ++++++++++++++++++++
> 2 files changed, 175 insertions(+), 0 deletions(-)
> create mode 100644 lib/a2mp.h
>

2011-10-24 23:06:18

[permalink] [raw]

Subject: [PATCH bluez 1/2] Add L2CAP Create/Move Channel definitions

---
lib/l2cap.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/lib/l2cap.h b/lib/l2cap.h
index 68593d3..6cacc98 100644
--- a/lib/l2cap.h
+++ b/lib/l2cap.h
@@ -82,6 +82,12 @@ struct l2cap_conninfo {
#define L2CAP_ECHO_RSP 0x09
#define L2CAP_INFO_REQ 0x0a
#define L2CAP_INFO_RSP 0x0b
+#define L2CAP_CREATE_REQ 0x0c
+#define L2CAP_CREATE_RSP 0x0d
+#define L2CAP_MOVE_REQ 0x0e
+#define L2CAP_MOVE_RSP 0x0f
+#define L2CAP_MOVE_CFM 0x10
+#define L2CAP_MOVE_CFM_RSP 0x11

/* L2CAP extended feature mask */
#define L2CAP_FEAT_FLOWCTL 0x00000001
@@ -226,6 +232,44 @@ typedef struct {
#define L2CAP_IR_SUCCESS 0x0000
#define L2CAP_IR_NOTSUPP 0x0001

+typedef struct {
+ uint16_t psm;
+ uint16_t scid;
+ uint8_t id;
+} __attribute__ ((packed)) l2cap_create_req;
+#define L2CAP_CREATE_REQ_SIZE 5
+
+typedef struct {
+ uint16_t dcid;
+ uint16_t scid;
+ uint16_t result;
+ uint16_t status;
+} __attribute__ ((packed)) l2cap_create_rsp;
+#define L2CAP_CREATE_RSP_SIZE 8
+
+typedef struct {
+ uint16_t icid;
+ uint8_t id;
+} __attribute__ ((packed)) l2cap_move_req;
+#define L2CAP_MOVE_REQ_SIZE 3
+
+typedef struct {
+ uint16_t icid;
+ uint16_t result;
+} __attribute__ ((packed)) l2cap_move_rsp;
+#define L2CAP_MOVE_RSP_SIZE 4
+
+typedef struct {
+ uint16_t icid;
+ uint16_t result;
+} __attribute__ ((packed)) l2cap_move_cfm;
+#define L2CAP_MOVE_CFM_SIZE 4
+
+typedef struct {
+ uint16_t icid;
+} __attribute__ ((packed)) l2cap_move_cfm_rsp;
+#define L2CAP_MOVE_CFM_RSP_SIZE 2
+
#ifdef __cplusplus
}
#endif
--
1.7.7

--
Peter Krystad
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

2011-10-24 23:06:19

[permalink] [raw]

Subject: [PATCH bluez 2/2] Add A2MP definitions

---
lib/a2mp.h | 131 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 131 insertions(+), 0 deletions(-)
create mode 100644 lib/a2mp.h

diff --git a/lib/a2mp.h b/lib/a2mp.h
new file mode 100644
index 0000000..87babb3
--- /dev/null
+++ b/lib/a2mp.h
@@ -0,0 +1,131 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2010-2011 Code Aurora Forum. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef __A2MP_H
+#define __A2MP_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/* AMP manager codes */
+#define AMP_COMMAND_REJ 0x01
+#define AMP_DISCOVER_REQ 0x02
+#define AMP_DISCOVER_RSP 0x03
+#define AMP_CHANGE_NOTIFY 0x04
+#define AMP_CHANGE_RSP 0x05
+#define AMP_INFO_REQ 0x06
+#define AMP_INFO_RSP 0x07
+#define AMP_ASSOC_REQ 0x08
+#define AMP_ASSOC_RSP 0x09
+#define AMP_LINK_REQ 0x0a
+#define AMP_LINK_RSP 0x0b
+#define AMP_DISCONN_REQ 0x0c
+#define AMP_DISCONN_RSP 0x0d
+
+typedef struct {
+ uint8_t code;
+ uint8_t ident;
+ uint16_t len;
+} __attribute__ ((packed)) amp_mgr_hdr;
+#define AMP_MGR_HDR_SIZE 4
+
+/* AMP ASSOC structure */
+typedef struct {
+ uint8_t type_id;
+ uint16_t len;
+ uint8_t data[0];
+} __attribute__ ((packed)) amp_assoc_tlv;
+
+typedef struct {
+ uint16_t reason;
+} __attribute__ ((packed)) amp_cmd_rej_parms;
+
+typedef struct {
+ uint16_t mtu;
+ uint16_t mask;
+} __attribute__ ((packed)) amp_discover_req_parms;
+
+typedef struct {
+ uint16_t mtu;
+ uint16_t mask;
+ uint8_t controller_list[0];
+} __attribute__ ((packed)) amp_discover_rsp_parms;
+
+typedef struct {
+ uint8_t id;
+} __attribute__ ((packed)) amp_info_req_parms;
+
+typedef struct {
+ uint8_t id;
+ uint8_t status;
+ uint32_t total_bandwidth;
+ uint32_t max_bandwidth;
+ uint32_t min_latency;
+ uint16_t pal_caps;
+ uint16_t assoc_size;
+} __attribute__ ((packed)) amp_info_rsp_parms;
+
+typedef struct {
+ uint8_t id;
+ uint8_t status;
+ amp_assoc_tlv assoc;
+} __attribute__ ((packed)) amp_assoc_rsp_parms;
+
+typedef struct {
+ uint8_t local_id;
+ uint8_t remote_id;
+ amp_assoc_tlv assoc;
+} __attribute__ ((packed)) amp_link_req_parms;
+
+typedef struct {
+ uint8_t local_id;
+ uint8_t remote_id;
+ uint8_t status;
+} __attribute__ ((packed)) amp_link_rsp_parms;
+
+typedef struct {
+ uint8_t local_id;
+ uint8_t remote_id;
+} __attribute__ ((packed)) amp_disconn_req_parms;
+
+#define AMP_COMMAND_NOT_RECOGNIZED 0x0000
+
+/* AMP controller status */
+#define AMP_CT_POWERED_DOWN 0x00
+#define AMP_CT_BLUETOOTH_ONLY 0x01
+#define AMP_CT_NO_CAPACITY 0x02
+#define AMP_CT_LOW_CAPACITY 0x03
+#define AMP_CT_MEDIUM_CAPACITY 0x04
+#define AMP_CT_HIGH_CAPACITY 0x05
+#define AMP_CT_FULL_CAPACITY 0x06
+
+/* AMP response status */
+#define AMP_STATUS_SUCCESS 0x00
+#define AMP_STATUS_INVALID_CTRL_ID 0x01
+#define AMP_STATUS_UNABLE_START_LINK_CREATION 0x02
+#define AMP_STATUS_NO_PHYSICAL_LINK_EXISTS 0x02
+#define AMP_STATUS_COLLISION_OCCURED 0x03
+#define AMP_STATUS_DISCONN_REQ_RECVD 0x04
+#define AMP_STATUS_PHYS_LINK_EXISTS 0x05
+#define AMP_STATUS_SECURITY_VIOLATION 0x06
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __A2MP_H */
--
1.7.7

--
Peter Krystad
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

2011-11-03 16:38:51