ArcBluetoothBridge::DeregisterForGattNotification() should return an error when it fails |
||||
Issue description(cont. from https://chromium-review.googlesource.com/c/chromium/src/+/694403) Currently, ArcBluetoothBridge::DeregisterForGattNotification() can fail silently when it's called without getting the result of RegisterForGattNotification() (i.e. before calling it, or before waiting for the callback being called asynchronously). When it cannot deregister, it should return an error code to notify the caller code.
,
Oct 3 2017
,
May 11 2018
,
May 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c7b259dbcbe6380dec5a1770146794df27159e69 commit c7b259dbcbe6380dec5a1770146794df27159e69 Author: Qiyu Hu <qiyuh@google.com> Date: Wed May 30 17:39:26 2018 arc: bluetooth: Rewrite the code for notification/indication Subscribing to notification/indication in android is a two step process, in the following order: a) RegisterForGattNotification() b) WriteGattDescriptor() to modify CCC descriptor to either 1 or 2 This patch does the following: - Refrain from calling StartNotifySession() in RegisterForGattNotification(). It's legal for a client to execute a) w/o b). Before CCC descriptor is modified, no notification/indication ought to be sent as CCC descriptor is still DISABLE_NOTIFICATION_VALUE. - Allow a client to change CCC descriptor as long as it once registers. - Explicitly check the valid CCC descriptor value. - Call the callback when (De)RegisterForGattNotification() fails. Do not fail silently. The golden rule in ARC++ bridge is to call |callback| for each request, with only one exception: |error_callback| is called. And a heads-up: As you notice, there is no difference for now between ENABLE_NOTIFICATION_VALUE and ENABLE_INDICATION_VALUE. This is due to a bug in blueZ. BlueZ ignores the CCC descriptor value provided by the client and infers it from the characteristic property. This is incorrect and will be fixed in future patches. Bug:771055,622832 Test:Still being able to pass CtsVerifier test on notifcation Change-Id: I53e62a6aaddf08f27be23b413f3621979a357265 Reviewed-on: https://chromium-review.googlesource.com/1066920 Commit-Queue: Qiyu Hu <qiyuh@google.com> Reviewed-by: Miao-chen Chou <mcchou@chromium.org> Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org> Cr-Commit-Position: refs/heads/master@{#562895} [modify] https://crrev.com/c7b259dbcbe6380dec5a1770146794df27159e69/chrome/browser/chromeos/arc/bluetooth/arc_bluetooth_bridge.cc
,
May 30 2018
|
||||
►
Sign in to add a comment |
||||
Comment 1 by hashimoto@chromium.org
, Oct 3 2017Status: Assigned (was: Untriaged)