New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 611427 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Last visit > 30 days ago
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Feature



Sign in to add a comment

Retrieve Gatt Service information for device::BluetoothAdapter::NotifyGattDiscoveryComplete in BlueZ implementation

Project Member Reported by mcchou@chromium.org, May 12 2016

Issue description

After BlueZ 5.39 uprev, ServicesResolved[1] property is added to org.bluez.Device1 interface, and it toggles whenever the services associated with the device are fully discovered. Instead of rely on GattServiceAdded()[2], NotifyGattDiscoveryComplete[3] should be invoked whenever ServicesResolved property changes to true. However, DevicePropertyChanged[4] does not provide the information of the newly-added/newly-removed service, so there is no proper way to call NotifyGattDiscoveryComplete with the corresponding device::BluetoothRemoteGattService pointer.

[1] http://git.kernel.org/cgit/bluetooth/bluez.git/tree/doc/device-api.txt#n215
[2] https://code.google.com/p/chromium/codesearch#chromium/src/device/bluetooth/dbus/bluetooth_gatt_service_client.h&q=GattServiceAdded&sq=package:chromium&type=cs&l=56
[3] https://code.google.com/p/chromium/codesearch#chromium/src/device/bluetooth/bluetooth_adapter.h&q=NotifyGattDiscoveryComplete&sq=package:chromium&type=cs&l=441
[4] https://code.google.com/p/chromium/codesearch#chromium/src/device/bluetooth/dbus/bluetooth_device_client.h&q=DevicePropertyChanged&sq=package:chromium&type=cs&l=114

This probably requires BlueZ changes and also the corresponding changes in Chrome APIs.
 

Comment 1 by ortuno@chromium.org, May 12 2016

If our goal is to match the behavior before 5.39 I think we can just iterate through our list of services and call NotifyGattDiscoveryComplete for each service for which we haven't called NotifyGattDiscoveryComplete before. We would do this every time the ServicesResolved property is toggled to true.

Note that clients of device/bluetooth expect GattDiscoveryCompleteForService to only be called once.

Comment 2 by r...@chromium.org, May 12 2016

Owner: mcchou@chromium.org
Status: Assigned (was: Unconfirmed)

Comment 3 by r...@chromium.org, May 12 2016

Status: WontFix (was: Assigned)
So looking at the CL, this is not needed. I agree with ortuno@. ServicesResolved will only be called once for a device (if it is called more than once, we should fix that). Once it is called, just call discovery complete for service on all services.

Comment 4 by ortuno@chromium.org, May 12 2016

"if it is called more than once, we should fix that". ServicesResolved being called more than once is normal behavior: A device can send a services change notification which would trigger a services rediscovery which will toggle the property. So our code does indeed need to keep track of services for which we've notify of discovery.

I'm aware that it's weird that a we don't notify clients if a service discovery has been re triggered but my goal here is to keep the behavior for the clients that currently depend on the GattDiscoveryCompleteForService the same. I think long term we want to get rid of that event completely.

Comment 5 by r...@chromium.org, May 12 2016

This is true.
In either case, it doesn't effect this bug. We should still call GattDiscoveryCompleteForService for all services once we receive the services resolved notification.

Comment 6 by mcchou@chromium.org, May 17 2016

As ortuno@ mentioned in comment #4, ServicesResolved will be triggered again for new objects. So we agree that NotifyGattDiscoveryComplete() should be invoked multiple time with newly-added services when the ServicesResolved becomes true.

Sign in to add a comment