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

Issue 606011 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Chrome
Pri: 1
Type: Bug

Blocking:
issue 600655



Sign in to add a comment

Change GATT client attribute discovery in Chrome.

Project Member Reported by r...@chromium.org, Apr 22 2016

Issue description

Currently we add services as they are discovered, traversing their characteristics by following the GattService1->Characteristics property. This property is being removed in BlueZ 5.39.

If we don't use this property, the regular mechanism of reading all known characteristics doesn't work because BlueZ can return stale attributes to the client.

Since we cannot use a service while discovery is active, the suggested solution is to wait till discovery is complete, and then read all the available attributes on a remote device. WebBluetooth already does this and switching to this model in Chrome will allow current functionality to keep working while still allowing us to BlueZ 5.39.


Assigning to myself tentatively, but if anyone is able to take this over, feel free to jump on it.

 

Comment 1 by r...@chromium.org, Apr 25 2016

Components: OS>Systems>Bluetooth

Comment 2 by ortuno@chromium.org, Apr 25 2016

To add a bit more context: Both Chrome Apps[1] and Proximity Auth.[2] rely on GattDiscoveryCompleteForService[3] being called once all children of a Service have been discovered. This function is called when the 'Characteristics'[4] property of a Service changes.[5]

Now that the 'Characteristics' property has been removed from Bluez[6] we need to either (1) find a new trigger for this function or (2) refactor our code to only expose services once all their children have been discovered.

(1) is fairly straightforward. Bluez recently introduced a 'ServicesResolved'[7] property that changes once all services, characteristics, and descriptors are discovered. We can use this new property to trigger 'GattDiscoveryCompleteForService'. This would only require a change to device/bluetooth

(2) is more involved. As rkc mentioned, we shouldn't be exposing stale services when clients call device->GetServices(). I believe the reason we are currently exposing these stale services is because BluetoothDeviceBluez populates its services map  from bluez when constructing itself[8] not when they are exposed through ObjectAdded. We need to investigate if ObjectAdded is called only after all children of a service have been discovered and if it is called for cached services. If both of those conditions are true then we can remote GattDiscoveryCompleteForService and new clients can simply rely on GattServiceAdded[9]. For old clients we could keep GattDiscoveryCompleteForService or we could refactor clients to just use GattServiceAdded. If either of these conditions doesn't apply then we need to refactor bluetooth_device_bluez to only populate it's services map once discovery has completed.

[1] https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/extensions/api/bluetooth_low_energy/bluetooth_low_energy_event_router.cc&l=854&rcl=1461583037
[2] https://code.google.com/p/chromium/codesearch#chromium/src/components/proximity_auth/ble/bluetooth_low_energy_characteristics_finder.cc&l=69&rcl=1461583037
[3] https://code.google.com/p/chromium/codesearch#chromium/src/device/bluetooth/bluetooth_adapter.h&l=136&rcl=1461583037
[4] http://git.kernel.org/cgit/bluetooth/bluez.git/tree/doc/gatt-api.txt?id=262cb18e2b26077a8caba43cbade1f2533f3b423#n48
[5] https://code.google.com/p/chromium/codesearch#chromium/src/device/bluetooth/bluez/bluetooth_remote_gatt_service_bluez.cc&sq=package:chromium&type=cs&l=155&rcl=1461583037
[6] http://git.kernel.org/cgit/bluetooth/bluez.git/commit/?id=2f3c4b2ff2dcd087ae328efdc86b35234a4ed25e
[7] http://git.kernel.org/cgit/bluetooth/bluez.git/tree/doc/device-api.txt#n215
[8] https://code.google.com/p/chromium/codesearch#chromium/src/device/bluetooth/bluez/bluetooth_device_bluez.cc&sq=package:chromium&type=cs&l=159
[9] https://code.google.com/p/chromium/codesearch#chromium/src/device/bluetooth/bluetooth_adapter.h&l=116

Comment 3 by ortuno@chromium.org, Apr 25 2016

 Issue 599520  has been merged into this issue.

Comment 4 by ortuno@chromium.org, Apr 25 2016

Components: IO>Bluetooth

Comment 5 by ortuno@chromium.org, Apr 25 2016

Components: -OS>Systems>Bluetooth

Comment 6 by r...@chromium.org, May 4 2016

Blocking: -601935 596742

Comment 7 by r...@chromium.org, May 4 2016

Blocking: -596742 600655
After some experimentation with BlueZ 5.39 and bluetoothctl I concluded that both conditions are false:

1. ObjectAdded calls are done in a prefix order meaning Service -> Characteristic -> Descriptor.
2. ObjectAdded is not called for cached attributes.

This means we can't really rely on ObjectAdded/ObjectRemoved for GattDiscoveryCompleteForService and we need to change the way we create our GATT Attributes.
Status: Started (was: Assigned)
Owner: mcchou@chromium.org
Cc: r...@chromium.org

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

Cc: puthik@chromium.org
3 days from branch. Please remove M-52 label from issues, or schedule for 53, as appropriate.
Project Member

Comment 14 by bugdroid1@chromium.org, May 17 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fe2a8f2b78fcf7f617df7ab287cd7922a4e969a8

commit fe2a8f2b78fcf7f617df7ab287cd7922a4e969a8
Author: mcchou <mcchou@chromium.org>
Date: Tue May 17 01:28:07 2016

Address property removals of GATT service and characteristic interfaces

This removes the deprecated Characteristics property of org.bluez.GattService1
interface and Descriptors property of org.bluez.GattCharacteristic1 interface.

BUG= 606011 
TEST=build and run tests

Review-Url: https://codereview.chromium.org/1984493002
Cr-Commit-Position: refs/heads/master@{#394006}

[modify] https://crrev.com/fe2a8f2b78fcf7f617df7ab287cd7922a4e969a8/device/bluetooth/bluez/bluetooth_gatt_bluez_unittest.cc
[modify] https://crrev.com/fe2a8f2b78fcf7f617df7ab287cd7922a4e969a8/device/bluetooth/bluez/bluetooth_remote_gatt_service_bluez.cc
[modify] https://crrev.com/fe2a8f2b78fcf7f617df7ab287cd7922a4e969a8/device/bluetooth/dbus/bluetooth_gatt_characteristic_client.cc
[modify] https://crrev.com/fe2a8f2b78fcf7f617df7ab287cd7922a4e969a8/device/bluetooth/dbus/bluetooth_gatt_characteristic_client.h
[modify] https://crrev.com/fe2a8f2b78fcf7f617df7ab287cd7922a4e969a8/device/bluetooth/dbus/bluetooth_gatt_service_client.cc
[modify] https://crrev.com/fe2a8f2b78fcf7f617df7ab287cd7922a4e969a8/device/bluetooth/dbus/bluetooth_gatt_service_client.h
[modify] https://crrev.com/fe2a8f2b78fcf7f617df7ab287cd7922a4e969a8/device/bluetooth/dbus/fake_bluetooth_gatt_characteristic_client.cc
[modify] https://crrev.com/fe2a8f2b78fcf7f617df7ab287cd7922a4e969a8/device/bluetooth/dbus/fake_bluetooth_gatt_service_client.cc

Project Member

Comment 15 by bugdroid1@chromium.org, May 18 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/da9047813fef5c79e536420281860c62606e8201

commit da9047813fef5c79e536420281860c62606e8201
Author: mcchou <mcchou@chromium.org>
Date: Wed May 18 07:00:36 2016

Invoke GattDiscoveryCompleteForService by observing ServicesResolved property

This changes how GattDiscoveryCompleteForService is invoked by observing the
toggle of ServicesResolved property, which is added in BlueZ5.39, of interface
org.bluez.Device1 instead of depending GattServiceAdded. Although BlueZ toggles
this property not only on the first fully-discovered event but also on the
following addition/removal events of service. GattDiscoveryCompleteForService
should be called only once after a remote device is created.

BUG= 606011 
TEST=build and run tests

Review-Url: https://codereview.chromium.org/1979633004
Cr-Commit-Position: refs/heads/master@{#394345}

[modify] https://crrev.com/da9047813fef5c79e536420281860c62606e8201/device/bluetooth/bluez/bluetooth_device_bluez.cc
[modify] https://crrev.com/da9047813fef5c79e536420281860c62606e8201/device/bluetooth/bluez/bluetooth_device_bluez.h
[modify] https://crrev.com/da9047813fef5c79e536420281860c62606e8201/device/bluetooth/bluez/bluetooth_gatt_bluez_unittest.cc
[modify] https://crrev.com/da9047813fef5c79e536420281860c62606e8201/device/bluetooth/bluez/bluetooth_remote_gatt_service_bluez.cc
[modify] https://crrev.com/da9047813fef5c79e536420281860c62606e8201/device/bluetooth/bluez/bluetooth_remote_gatt_service_bluez.h
[modify] https://crrev.com/da9047813fef5c79e536420281860c62606e8201/device/bluetooth/dbus/fake_bluetooth_device_client.cc
[modify] https://crrev.com/da9047813fef5c79e536420281860c62606e8201/device/bluetooth/dbus/fake_bluetooth_device_client.h
[modify] https://crrev.com/da9047813fef5c79e536420281860c62606e8201/device/bluetooth/dbus/fake_bluetooth_gatt_service_client.cc
[modify] https://crrev.com/da9047813fef5c79e536420281860c62606e8201/device/bluetooth/dbus/fake_bluetooth_gatt_service_client.h

Status: Fixed (was: Started)
Status: Verified (was: Fixed)
bulk verify of io>bluetooth gatt server bugs

Sign in to add a comment