Re: [RFC BlueZ] doc/device-api: Replace GattServices with Discovering property

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

 



Hi Andrzej,

On Fri, Mar 11, 2016 at 3:41 PM, Andrzej Kaczmarek
<andrzej.kaczmarek@xxxxxxxxxxx> wrote:
> Hi Luiz,
>
>
> On Fri, Mar 11, 2016 at 2:08 PM, Luiz Augusto von Dentz
> <luiz.dentz@xxxxxxxxx> wrote:
>> Hi Andrzej,
>>
>> On Fri, Mar 11, 2016 at 2:24 PM, Andrzej Kaczmarek
>> <andrzej.kaczmarek@xxxxxxxxxxx> wrote:
>>> Hi Luiz,
>>>
>>>
>>> On Fri, Mar 11, 2016 at 12:23 PM, Luiz Augusto von Dentz
>>> <luiz.dentz@xxxxxxxxx> wrote:
>>>> Hi Andrzej,
>>>>
>>>> On Fri, Mar 11, 2016 at 1:05 PM, Andrzej Kaczmarek
>>>> <andrzej.kaczmarek@xxxxxxxxxxx> wrote:
>>>>> Hi Luiz,
>>>>>
>>>>> On Thu, Mar 10, 2016 at 5:16 PM, Luiz Augusto von Dentz
>>>>> <luiz.dentz@xxxxxxxxx> wrote:
>>>>>> From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
>>>>>>
>>>>>> GattServices is not really doing was it was meant to do which was to
>>>>>> track progress of service discovery since it only worked for the very
>>>>>> first time a device is connected but since we no longer remove the
>>>>>> attributes an application would have the false impression the service are
>>>>>> all resolved by the time it reconnects when in fact the service may have
>>>>>> changed.
>>>>>>
>>>>>> Futhermore object tracking like it is doing has been obsolete by
>>>>>> ObjectManager so this propose to replace the service discovery tracking
>>>>>> with a boolean property which works both with SDP as well as GATT
>>>>>> discovery.
>>>>>> ---
>>>>>>  doc/device-api.txt |  9 +++------
>>>>>>  src/device.c       | 58 +++++++++++-------------------------------------------
>>>>>>  2 files changed, 14 insertions(+), 53 deletions(-)
>>>>>>
>>>>>> diff --git a/doc/device-api.txt b/doc/device-api.txt
>>>>>> index a8076a2..9e58664 100644
>>>>>> --- a/doc/device-api.txt
>>>>>> +++ b/doc/device-api.txt
>>>>>> @@ -212,10 +212,7 @@ Properties string Address [readonly]
>>>>>>                         Service advertisement data. Keys are the UUIDs in
>>>>>>                         string format followed by its byte array value.
>>>>>>
>>>>>> -               array{object} GattServices [readonly, optional]
>>>>>> +               bool Discovering [readonly, optional, experimental]
>>>>>
>>>>> It's not optional - there's no exists method (which is fine).
>>>>
>>>> Indeed, I will fix it.
>>>>
>>>>> I think a better name for this would be "Discovered" which has initial
>>>>> value of false (once device is connected) and then changes to true
>>>>> (once services are ready, either from cache or an actual discovery).
>>>>> It could also change back to false once re-discovery due to Service
>>>>> Changed is in progress which I think is also fine.
>>>>>
>>>>> With "Discovering" the problem is still that once "Connected" changes
>>>>> to true, reading "Discovering" will return false so you don't quite
>>>>> know whether discovery already finished or has not yet started.
>>>>> Application would need to wait for complete sequence to be sure:
>>>>> Connected=false, Discovering=false
>>>>> Connected=true, Discovering=false
>>>>> Connected=false, Discovering=true
>>>>> Connected=false, Discovering=false
>>>>>
>>>>> With "Discovered" this is not a problem since sequence is as follows:
>>>>> Connected=false, Discovered=false
>>>>> Connected=true, Discovered=false
>>>>> Connected=true, Discovered=true
>>>>
>>>> I was actually thinking in using the same terminology we used in the
>>>> source code such as ResolvingServices, but changing to Discovered
>>>> don't actually fix the problem since you may still end up with
>>>> Connected=true, Discovered=true but the flag can still change to
>>>> Discovered=false. Anyway Im currently discussing about this changes
>>>> makes sense in Web Bluetooth, because Service Changed may come in
>>>> later on and there is no API to push the services added/removed to the
>>>> application.
>>>
>>> When Service Changed is received, BlueZ removes old objects and then
>>> adds new ones. And once there's change to Discovered=false it means
>>> device in its current state is not fully discovered, e.g. there's
>>> re-discovery due to Service Changed in this case which makes sense to
>>> me. So what's the problem here?
>>
>> If that was the case, again this is for Web Bluetooth, we wouldn't
>> need to notify this at all because the application would receive the
>> updates directly, but since the Web Bluetooth is not designed in this
>> way you just get the current services if anything is updated later it
>> causes this problem where the services get updated but the application
>> don't realize it has happened. So it is all about when the application
>> actually list the objects, again Im talking about Web Bluetooth
>> applications, if by the time it calls getPrimaryServices the discovery
>> is active there is a chance it will return outdated services.
>> Essentially it is a polling API.
>>
>> You can read more about this issue here:
>>
>> https://github.com/thegecko/web-bluetooth-dfu/issues/12#issuecomment-194973956
>
> In case discovery is in progress, API such as getPrimaryServices()
> could queue response until discovery is done. This is possible with
> both "Discovering" and "Discovered", but the latter has significant
> advantage here: just after connected, you know exactly when initial
> discovery finished, because there will be single transition
> Discovered=false->true. In case of "Discovering" there's problem I
> mentioned before when you start with Discovering=false so there's
> chance application gets services even before initial discovery - we've
> seen such races when application is trying to read/write
> characteristic using existing D-Bus object just after connection, but
> before fixed ATT channel was established in BlueZ.

I think this problem with ATT has been solved actually, so the object
have access to bt_gatt_client immediately after connection so
read/write can be processed. Btw, may latest patch actually contain
some changes so the signal order looks like this:

[CHG] Device ... Connected: yes
[CHG] Device ..  ServicesResolved: yes
[CHG] Device ... ServicesResolved: no
[CHG] Device ... Connected: no

It basically follows svc_refreshed flag.

> As for Service Changed, as you mentioned in comment linked above, this
> cannot be really solved without some kind of notification to
> application. Other possibility (without notifications) is that object
> created by "outdated" getPrimaryServices() call with simply return an
> error forcing application to call getPrimaryServices() again. But this
> has to be solved in Web Bluetooth somehow anyway...

Yep, but I guess this is some unlikely to happen during a connection
that nobody care to add an API for it.

-- 
Luiz Augusto von Dentz
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux