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

Hi Luiz,


On Fri, Mar 11, 2016 at 2:08 PM, Luiz Augusto von Dentz
[off-list ref] wrote:

Hi Andrzej,

On Fri, Mar 11, 2016 at 2:24 PM, Andrzej Kaczmarek
[off-list ref] wrote:

quoted

Hi Luiz,


On Fri, Mar 11, 2016 at 12:23 PM, Luiz Augusto von Dentz
[off-list ref] wrote:

quoted

Hi Andrzej,

On Fri, Mar 11, 2016 at 1:05 PM, Andrzej Kaczmarek
[off-list ref] wrote:

quoted

Hi Luiz,

On Thu, Mar 10, 2016 at 5:16 PM, Luiz Augusto von Dentz
[off-list ref] wrote:

quoted

From: Luiz Augusto von Dentz <redacted>

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.

quoted

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.

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...

BR,
Andrzej