diff --git a/connections/implementation/mediums/ble_v2.cc b/connections/implementation/mediums/ble_v2.cc index 6cb4e463..d0e22137 100644 --- a/connections/implementation/mediums/ble_v2.cc +++ b/connections/implementation/mediums/ble_v2.cc @@ -14,6 +14,7 @@ #include "connections/implementation/mediums/ble_v2.h" +#include #include #include #include @@ -607,15 +608,8 @@ void BleV2::ProcessFetchGattAdvertisementsRequest( return; } - // Always use kCopresenceServiceUuid for service uuid. - if (!gatt_client->DiscoverService( - mediums::bleutils::kCopresenceServiceUuid)) { - NEARBY_LOGS(WARNING) << "GATT client can't discover service."; - advertisement_read_result.RecordLastReadStatus(false); - return; - } - - // Read all advertisements from all slots that we haven't read from yet. + // Collect service_uuid and its associated characteristic_uuids. + absl::flat_hash_map slot_characteristic_uuids = {}; for (int slot = 0; slot < num_slots; ++slot) { // Make sure we haven't already read this advertisement before. if (advertisement_read_result.HasAdvertisement(slot)) { @@ -632,8 +626,36 @@ void BleV2::ProcessFetchGattAdvertisementsRequest( if (!advertiement_uuid.has_value()) { continue; } + slot_characteristic_uuids.insert({slot, *advertiement_uuid}); + } + if (slot_characteristic_uuids.empty()) { + // TODO(b/222392304): More test coverage. + NEARBY_LOGS(WARNING) << "Edwin GATT client doesn't have characteristics."; + advertisement_read_result.RecordLastReadStatus(false); + return; + } + + // Discover service and characteristics. + std::vector characteristic_uuids; + std::transform(slot_characteristic_uuids.begin(), + slot_characteristic_uuids.end(), + std::back_inserter(characteristic_uuids), + [](auto& kv) { return kv.second; }); + if (!gatt_client->DiscoverServiceAndCharacteristics( + mediums::bleutils::kCopresenceServiceUuid, characteristic_uuids)) { + // TODO(b/222392304): More test coverage. + NEARBY_LOGS(WARNING) << "Edwin GATT client doesn't have characteristics."; + advertisement_read_result.RecordLastReadStatus(false); + return; + } + + // Read all advertisements from all characteristics that we haven't read from + // yet. + for (const auto& it : slot_characteristic_uuids) { + int slot = it.first; + Uuid characteristic_uuid = it.second; auto gatt_characteristic = gatt_client->GetCharacteristic( - mediums::bleutils::kCopresenceServiceUuid, *advertiement_uuid); + mediums::bleutils::kCopresenceServiceUuid, characteristic_uuid); if (!gatt_characteristic.has_value()) { continue; } diff --git a/internal/platform/ble_v2.h b/internal/platform/ble_v2.h index 368f16f4..db9c298e 100644 --- a/internal/platform/ble_v2.h +++ b/internal/platform/ble_v2.h @@ -192,8 +192,11 @@ class GattClient final { std::unique_ptr client_gatt_connection) : impl_(std::move(client_gatt_connection)) {} - bool DiscoverService(const Uuid& service_uuid) { - return impl_->DiscoverService(service_uuid); + bool DiscoverServiceAndCharacteristics( + const Uuid& service_uuid, + const std::vector& characteristic_uuids) { + return impl_->DiscoverServiceAndCharacteristics(service_uuid, + characteristic_uuids); } // NOLINTNEXTLINE(google3-legacy-absl-backports) diff --git a/internal/platform/ble_v2_test.cc b/internal/platform/ble_v2_test.cc index 935a71ef..9da17d90 100644 --- a/internal/platform/ble_v2_test.cc +++ b/internal/platform/ble_v2_test.cc @@ -387,10 +387,10 @@ TEST_F(BleV2MediumTest, GattClientConnectToGattServerWorks) { ASSERT_NE(gatt_client, nullptr); - // Discover service. - EXPECT_TRUE(gatt_client->DiscoverService(service_uuid)); + // Discover service and characteristics. + EXPECT_TRUE(gatt_client->DiscoverServiceAndCharacteristics( + service_uuid, {characteristic_uuid})); - // Discover characteristic. // NOLINTNEXTLINE(google3-legacy-absl-backports) absl::optional client_characteristic = gatt_client->GetCharacteristic(service_uuid, characteristic_uuid); diff --git a/internal/platform/implementation/ble_v2.h b/internal/platform/implementation/ble_v2.h index bc1da958..f9162f2b 100644 --- a/internal/platform/implementation/ble_v2.h +++ b/internal/platform/implementation/ble_v2.h @@ -137,15 +137,17 @@ class GattClient { // Returns whether or not discovery finished successfully. // // This function should block until discovery has finished. - virtual bool DiscoverService(const Uuid& service_uuid) = 0; + virtual bool DiscoverServiceAndCharacteristics( + const Uuid& service_uuid, + const std::vector& characteristic_uuids) = 0; // https://developer.android.com/reference/android/bluetooth/BluetoothGatt.html#getService(java.util.UUID) // https://developer.android.com/reference/android/bluetooth/BluetoothGattService.html#getCharacteristic(java.util.UUID) // // Retrieves a GATT characteristic. On error, does not return a value. // - // DiscoverServices() should be called before this method to fetch all - // available services and characteristics first. + // DiscoverServiceAndCharacteristics() should be called before this method to + // fetch all available services and characteristics first. // // It is okay for duplicate services to exist, as long as the specified // characteristic UUID is unique among all services of the same UUID. diff --git a/internal/platform/implementation/g3/ble_v2.cc b/internal/platform/implementation/g3/ble_v2.cc index 8782dbc8..2add49bf 100644 --- a/internal/platform/implementation/g3/ble_v2.cc +++ b/internal/platform/implementation/g3/ble_v2.cc @@ -306,17 +306,18 @@ void BleV2Medium::GattServer::Stop() { MediumEnvironment::Instance().ClearBleV2MediumGattCharacteristics(); } -bool BleV2Medium::GattClient::DiscoverService(const Uuid& service_uuid) { +bool BleV2Medium::GattClient::DiscoverServiceAndCharacteristics( + const Uuid& service_uuid, const std::vector& characteristic_uuids) { absl::MutexLock lock(&mutex_); - NEARBY_LOGS(INFO) << "G3 Ble GattClient DiscoverService, service_uuid=" - << service_uuid.Get16BitAsString(); + NEARBY_LOGS(INFO) + << "G3 Ble GattClient DiscoverServiceAndCharacteristics, service_uuid=" + << service_uuid.Get16BitAsString(); if (!is_connection_alive_) { return false; } - // Search if the service exists. - return MediumEnvironment::Instance().ContainsBleV2MediumGattCharacteristics( - service_uuid, /*characteristic_uuid=*/{}); + return MediumEnvironment::Instance().DiscoverBleV2MediumGattCharacteristics( + service_uuid, characteristic_uuids); } std::optional @@ -331,13 +332,20 @@ BleV2Medium::GattClient::GetCharacteristic(const Uuid& service_uuid, return std::nullopt; } - // Search gatt_characteristic by uuid and if found return the - // gatt_characteristic. - api::ble_v2::GattCharacteristic characteristic; - if (MediumEnvironment::Instance().ContainsBleV2MediumGattCharacteristics( - service_uuid, characteristic_uuid)) { - characteristic = {.uuid = characteristic_uuid, - .service_uuid = service_uuid}; + // clang-format off + api::ble_v2::GattCharacteristic characteristic = { + .uuid = characteristic_uuid, + .service_uuid = service_uuid}; + // clang-format on + ByteArray value = + MediumEnvironment::Instance().ReadBleV2MediumGattCharacteristics( + characteristic); + if (value.Empty()) { + NEARBY_LOGS(WARNING) + << "G3 Ble GattClient GetCharacteristic, can't find characteristic=(" + << characteristic.service_uuid.Get16BitAsString() << "," + << std::string(characteristic.uuid) << ")"; + return std::nullopt; } NEARBY_LOGS(INFO) << "G3 Ble GattClient GetCharacteristic, found characteristic=(" @@ -375,6 +383,8 @@ void BleV2Medium::GattClient::Disconnect() { absl::MutexLock lock(&mutex_); NEARBY_LOGS(INFO) << "G3 Ble GattClient Disconnect"; is_connection_alive_ = false; + MediumEnvironment::Instance() + .ClearBleV2MediumGattCharacteristicsForDiscovery(); } std::unique_ptr BleV2Medium::OpenServerSocket( diff --git a/internal/platform/implementation/g3/ble_v2.h b/internal/platform/implementation/g3/ble_v2.h index 9def4a74..8b4becd5 100644 --- a/internal/platform/implementation/g3/ble_v2.h +++ b/internal/platform/implementation/g3/ble_v2.h @@ -216,7 +216,9 @@ class BleV2Medium : public api::ble_v2::BleMedium { // A concrete implemenation for GattClient. class GattClient : public api::ble_v2::GattClient { public: - bool DiscoverService(const Uuid& service_uuid) override; + bool DiscoverServiceAndCharacteristics( + const Uuid& service_uuid, + const std::vector& characteristic_uuids) override; std::optional GetCharacteristic( const Uuid& service_uuid, const Uuid& characteristic_uuid) override; diff --git a/internal/platform/medium_environment.cc b/internal/platform/medium_environment.cc index c443aa12..b8e0f9ad 100644 --- a/internal/platform/medium_environment.cc +++ b/internal/platform/medium_environment.cc @@ -14,6 +14,7 @@ #include "internal/platform/medium_environment.h" +#include #include #include #include @@ -615,31 +616,34 @@ void MediumEnvironment::InsertBleV2MediumGattCharacteristics( latch.Await(); } -bool MediumEnvironment::ContainsBleV2MediumGattCharacteristics( - const Uuid& service_uuid, const Uuid& characteristic_uuid) { +void MediumEnvironment::ClearBleV2MediumGattCharacteristics() { + if (!enabled_) return; + RunOnMediumEnvironmentThread([this]() { gatt_advertisement_bytes_.clear(); }); +} + +bool MediumEnvironment::DiscoverBleV2MediumGattCharacteristics( + const Uuid& service_uuid, const std::vector& characteristic_uuids) { if (!enabled_) return false; - bool found_characteristic = false; CountDownLatch latch(1); - RunOnMediumEnvironmentThread([this, &latch, &service_uuid, - &characteristic_uuid, &found_characteristic]() { - for (const auto& item : gatt_advertisement_bytes_) { - if (item.first.service_uuid == service_uuid) { - if (characteristic_uuid.IsEmpty()) { - // Found the service uuid and no need to search characteristic - // uuid. - found_characteristic = true; - break; + RunOnMediumEnvironmentThread( + [this, &latch, &service_uuid, &characteristic_uuids]() { + for (const auto& item : gatt_advertisement_bytes_) { + if (item.first.service_uuid == service_uuid) { + Uuid char_uuid_key = item.first.uuid; + auto it = std::find_if(characteristic_uuids.rbegin(), + characteristic_uuids.rend(), + [char_uuid_key](const auto& char_uuid) { + return char_uuid == char_uuid_key; + }); + if (it != characteristic_uuids.rend()) { + discovered_gatt_advertisement_bytes_[item.first] = item.second; + } + } } - if (item.first.uuid == characteristic_uuid) { - found_characteristic = true; - break; - } - } - } - latch.CountDown(); - }); + latch.CountDown(); + }); latch.Await(); - return found_characteristic; + return true; } ByteArray MediumEnvironment::ReadBleV2MediumGattCharacteristics( @@ -649,8 +653,8 @@ ByteArray MediumEnvironment::ReadBleV2MediumGattCharacteristics( CountDownLatch latch(1); RunOnMediumEnvironmentThread( [this, &latch, &characteristic, &gatt_advertisement_byte]() { - auto it = gatt_advertisement_bytes_.find(characteristic); - if (it != gatt_advertisement_bytes_.end()) { + auto it = discovered_gatt_advertisement_bytes_.find(characteristic); + if (it != discovered_gatt_advertisement_bytes_.end()) { gatt_advertisement_byte = it->second; } latch.CountDown(); @@ -659,9 +663,10 @@ ByteArray MediumEnvironment::ReadBleV2MediumGattCharacteristics( return gatt_advertisement_byte; } -void MediumEnvironment::ClearBleV2MediumGattCharacteristics() { +void MediumEnvironment::ClearBleV2MediumGattCharacteristicsForDiscovery() { if (!enabled_) return; - RunOnMediumEnvironmentThread([this]() { gatt_advertisement_bytes_.clear(); }); + RunOnMediumEnvironmentThread( + [this]() { discovered_gatt_advertisement_bytes_.clear(); }); } void MediumEnvironment::UnregisterBleV2Medium(api::ble_v2::BleMedium& medium) { diff --git a/internal/platform/medium_environment.h b/internal/platform/medium_environment.h index 2b186986..b0e51b3a 100644 --- a/internal/platform/medium_environment.h +++ b/internal/platform/medium_environment.h @@ -31,12 +31,12 @@ #endif #include "internal/platform/byte_array.h" #include "internal/platform/feature_flags.h" -#include "internal/platform/implementation/wifi_lan.h" -#include "internal/platform/wifi_hotspot_credential.h" #include "internal/platform/implementation/wifi_hotspot.h" +#include "internal/platform/implementation/wifi_lan.h" +#include "internal/platform/mutex.h" #include "internal/platform/nsd_service_info.h" #include "internal/platform/single_thread_executor.h" -#include "internal/platform/mutex.h" +#include "internal/platform/wifi_hotspot_credential.h" namespace location { namespace nearby { @@ -241,19 +241,22 @@ class MediumEnvironment { const api::ble_v2::GattCharacteristic& characteristic, const ByteArray& gatt_advertisement_byte); - // Check if `service_uuid` and `characteristic_uuid` exists in the map. - // - // `characteristic_uuid` can be empty and to check `service_uuid` only. - bool ContainsBleV2MediumGattCharacteristics(const Uuid& service_uuid, - const Uuid& characteristic_uuid); + // Clears the map `gatt_advertisement_bytes_`. + void ClearBleV2MediumGattCharacteristics(); + + // Discover `service_uuid` and `characteristic_uuids`. This is to save the + // matched `gatt_advertisement_bytes_` to the + // `discovered_gatt_advertisement_bytes_`. + bool DiscoverBleV2MediumGattCharacteristics( + const Uuid& service_uuid, const std::vector& characteristic_uuids); // Reads the BLE GATT characteristic value. If the GATT characteristic is not // existed, return empty byte array. ByteArray ReadBleV2MediumGattCharacteristics( const api::ble_v2::GattCharacteristic& characteristic); - // Clears the map `gatt_advertisement_bytes_`. - void ClearBleV2MediumGattCharacteristics(); + // Clears the map `discovered_gatt_advertisement_bytes_`. + void ClearBleV2MediumGattCharacteristicsForDiscovery(); // Removes medium-related info. This should correspond to device power off. void UnregisterBleV2Medium(api::ble_v2::BleMedium& mediumum); @@ -403,6 +406,9 @@ class MediumEnvironment { absl::flat_hash_map gatt_advertisement_bytes_; + absl::flat_hash_map + discovered_gatt_advertisement_bytes_; #ifndef NO_WEBRTC // Maps peer id to callback for receiving signaling messages.