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

Issue 771055 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

ArcBluetoothBridge::DeregisterForGattNotification() should return an error when it fails

Project Member Reported by hashimoto@chromium.org, Oct 3 2017

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.
 
Owner: r...@chromium.org
Status: Assigned (was: Untriaged)
Tentatively assigning to rkc@.
Please forward the ownership to an appropriate person.
Owner: sonnysasaka@chromium.org

Comment 3 by qiyuh@chromium.org, May 11 2018

Owner: qiyuh@chromium.org
Project Member

Comment 4 by bugdroid1@chromium.org, 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

Comment 5 by qiyuh@chromium.org, May 30 2018

Status: Fixed (was: Assigned)

Sign in to add a comment