Skip to content

Commit

Permalink
Revert "[Fast Pair] Migrate to modern GATT service discovery API."
Browse files Browse the repository at this point in the history
This reverts commit 06ea495.

Reason for revert: For some reason, the migration to the new GATT
API is not finding the associated Fast Pair service. We need to
investigate this further to determine the underlying cause, which
is likely on the platform/API side.See: b/260142871

Original change's description:
> [Fast Pair] Migrate to modern GATT service discovery API.
>
> In the FastPairGattClientImpl class, GattServicesDiscovered is
> implemented and GattDiscoveryCompleteForService is removed.
> The former replaces the deprecated latter.
>
> Follow-up CLs related to the Bug:
> - A new Handshake failure that the device is
> missing Fast Pair service even if the GATT services discovery is
> complete needs to be surfaced.
> - The addition of a GATT operation timeout for GATT connection separate
> from the GATT service discovery to help us distinguish failure points.
>
> TEST=
> - Initial pair, subsequent pair, and retroactive pair scenarios
>   were tested on DUT.
> - The newly added unit test tests the case in which the Fast Pair
>   Service isn't found on the device; all other test cases test the case
>   that the Service is found. This covers the scenarios for the new API.
>
> Bug: b/257099968
> Change-Id: Ia46360ec9bc0b6ac851161a33efcd4d71d8d0ec2
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4015079
> Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
> Reviewed-by: Juliet Lévesque <julietlevesque@google.com>
> Commit-Queue: Brando Socarras <brandosocarras@google.com>
> Cr-Commit-Position: refs/heads/main@{#1073644}

Bug: b/257099968
Change-Id: I44455e320a502d4cda81a09a4b86626371a7b6dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4049134
Reviewed-by: Juliet Lévesque <julietlevesque@google.com>
Auto-Submit: Jack Shira <jackshira@google.com>
Commit-Queue: Juliet Lévesque <julietlevesque@google.com>
Cr-Commit-Position: refs/heads/main@{#1075159}
  • Loading branch information
Jack Shira authored and Chromium LUCI CQ committed Nov 23, 2022
1 parent d000634 commit 801a929
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 175 deletions.
Expand Up @@ -217,41 +217,19 @@ void FastPairGattServiceClientImpl::NotifyWriteAccountKeyError(
std::move(write_account_key_callback_).Run(error);
}

void FastPairGattServiceClientImpl::GattServicesDiscovered(
void FastPairGattServiceClientImpl::GattDiscoveryCompleteForService(
device::BluetoothAdapter* adapter,
device::BluetoothDevice* device) {
// The Bluetooth adapter |this| observes notifies its observers to call this
// function for all devices known to the adapter once all GATT services of
// each device are ready. One FastPairGattServiceClientImpl is created per
// device handshake, so each is associated with a device. Before considering
// |device|'s discovered services, this client must ensure that it is the same
// as the associated device.
if (device_address_ != device->GetAddress())
return;

device::BluetoothRemoteGattService* service) {
gatt_service_discovery_timer_.Stop();

gatt_service_ = device->GetGattService(kFastPairBluetoothUuid.value());
if (!gatt_service_) {
QP_LOG(WARNING) << __func__
<< ": Fast Pair GATT service expected but unavailable; "
<< "GATT connection will be abandoned.";

// TODO(b/257099968): Emit a metric "expected GATT service *not* found".

// Since the Fast Pair GATT service is not found, this connection
// is fatal; |this| will be destroyed.
// The FastPairHandshake that owns |this| and the FastPairPairer that owns
// the FastPairHandshake will be notified of the failed connection.
NotifyInitializedError(PairFailure::kGattServiceDiscovery);
return;
// Verify that the discovered service and device are the ones we care about.
if (service->GetUUID() == kFastPairBluetoothUuid &&
service->GetDevice()->GetAddress() == device_address_) {
QP_LOG(INFO) << __func__
<< ": Completed discovery for Fast Pair GATT service";
gatt_service_ = service;
FindGattCharacteristicsAndStartNotifySessions();
}

// TODO(b/257099968): Emit a metric "expected GATT service found".

QP_LOG(INFO) << __func__
<< ": Completed discovery for Fast Pair GATT service";
FindGattCharacteristicsAndStartNotifySessions();
}

std::vector<device::BluetoothRemoteGattCharacteristic*>
Expand Down
Expand Up @@ -131,12 +131,13 @@ class FastPairGattServiceClientImpl : public FastPairGattServiceClient {
void ClearCurrentState();

// BluetoothAdapter::Observer
void GattDiscoveryCompleteForService(
device::BluetoothAdapter* adapter,
device::BluetoothRemoteGattService* service) override;
void GattCharacteristicValueChanged(
device::BluetoothAdapter* adapter,
device::BluetoothRemoteGattCharacteristic* characteristic,
const std::vector<uint8_t>& value) override;
void GattServicesDiscovered(device::BluetoothAdapter* adapter,
device::BluetoothDevice* device) override;

void FindGattCharacteristicsAndStartNotifySessions();

Expand Down

0 comments on commit 801a929

Please sign in to comment.