From 771ce23fa3f06978005fb13091311c01a8298c22 Mon Sep 17 00:00:00 2001 From: edwinwu Date: Mon, 9 May 2022 00:34:25 -0700 Subject: [PATCH] [BLE Refactor] Implements advertisement tracking in Ble medium with DiscoveredPeripheralTracker class. PiperOrigin-RevId: 447397658 --- .../implementation/base_pcp_handler.cc | 14 +- connections/implementation/base_pcp_handler.h | 10 +- connections/implementation/bwu_manager.cc | 9 +- connections/implementation/bwu_manager.h | 4 +- connections/implementation/client_proxy.cc | 9 +- connections/implementation/client_proxy.h | 3 +- connections/implementation/mediums/ble_v2.cc | 117 ++++++++- connections/implementation/mediums/ble_v2.h | 50 +++- .../ble_v2/discovered_peripheral_tracker.cc | 37 +-- .../ble_v2/discovered_peripheral_tracker.h | 16 +- .../discovered_peripheral_tracker_test.cc | 28 +- .../implementation/mediums/ble_v2_test.cc | 243 ++++++++++++++++-- connections/implementation/mediums/webrtc.cc | 22 +- connections/implementation/mediums/webrtc.h | 15 +- internal/platform/ble_v2.cc | 3 +- internal/platform/ble_v2.h | 9 +- internal/platform/ble_v2_test.cc | 5 +- internal/platform/cancelable_alarm.h | 40 ++- internal/platform/cancelable_alarm_test.cc | 17 +- internal/platform/implementation/ble_v2.h | 4 +- internal/platform/implementation/g3/ble_v2.cc | 14 +- internal/platform/medium_environment.cc | 4 +- 22 files changed, 504 insertions(+), 169 deletions(-) diff --git a/connections/implementation/base_pcp_handler.cc b/connections/implementation/base_pcp_handler.cc index af2fae1a..942f81e7 100644 --- a/connections/implementation/base_pcp_handler.cc +++ b/connections/implementation/base_pcp_handler.cc @@ -14,11 +14,14 @@ #include "connections/implementation/base_pcp_handler.h" +#include #include #include #include #include #include +#include +#include #include "securegcm/d2d_connection_context_v1.h" #include "securegcm/ukey2_handshake.h" @@ -27,17 +30,14 @@ #include "absl/types/span.h" #include "connections/implementation/mediums/utils.h" #include "connections/implementation/offline_frames.h" -#include "connections/implementation/pcp_handler.h" #include "internal/platform/base64_utils.h" #include "internal/platform/bluetooth_utils.h" #include "internal/platform/logging.h" -#include "internal/platform/system_clock.h" namespace location { namespace nearby { namespace connections { -using ::location::nearby::proto::connections::Medium; using ::securegcm::UKey2Handshake; constexpr absl::Duration BasePcpHandler::kConnectionRequestReadTimeout; @@ -610,8 +610,8 @@ Status BasePcpHandler::RequestConnection( bool BasePcpHandler::MediumSupportedByClientOptions( const proto::connections::Medium& medium, - const ConnectionOptions& client_options) const { - for (auto supported_medium : client_options.GetMediums()) { + const ConnectionOptions& connection_options) const { + for (auto supported_medium : connection_options.GetMediums()) { if (medium == supported_medium) { return true; } @@ -946,7 +946,7 @@ void BasePcpHandler::OnEndpointDisconnect(ClientProxy* client, auto item = pending_alarms_.find(endpoint_id); if (item != pending_alarms_.end()) { auto& alarm = item->second; - alarm.Cancel(); + alarm->Cancel(); pending_alarms_.erase(item); } ProcessPreConnectionResultFailure(client, @@ -1423,7 +1423,7 @@ void BasePcpHandler::EvaluateConnectionResult(ClientProxy* client, } else { pending_alarms_.emplace( endpoint_id, - CancelableAlarm( + std::make_unique( "BasePcpHandler.evaluateConnectionResult() delayed close", [this, client, endpoint_id]() { endpoint_manager_->DiscardEndpoint(client, endpoint_id); diff --git a/connections/implementation/base_pcp_handler.h b/connections/implementation/base_pcp_handler.h index a62298e0..8499124a 100644 --- a/connections/implementation/base_pcp_handler.h +++ b/connections/implementation/base_pcp_handler.h @@ -18,9 +18,9 @@ #include #include #include +#include #include -#include "securegcm/d2d_connection_context_v1.h" #include "securegcm/ukey2_handshake.h" #include "absl/container/btree_map.h" #include "absl/container/flat_hash_map.h" @@ -34,7 +34,6 @@ #ifdef NO_WEBRTC #include "connections/implementation/mediums/webrtc_stub.h" #else -#include "connections/implementation/mediums/webrtc.h" #endif #include "connections/implementation/pcp.h" #include "connections/implementation/pcp_handler.h" @@ -43,13 +42,11 @@ #include "internal/platform/byte_array.h" #include "internal/platform/prng.h" #include "internal/platform/atomic_boolean.h" -#include "internal/platform/atomic_reference.h" #include "internal/platform/cancelable_alarm.h" #include "internal/platform/count_down_latch.h" #include "internal/platform/future.h" #include "internal/platform/scheduled_executor.h" #include "internal/platform/single_thread_executor.h" -#include "internal/platform/system_clock.h" namespace location { namespace nearby { @@ -477,7 +474,7 @@ class BasePcpHandler : public PcpHandler, const ConnectionOptions& connection_options) const; std::vector GetSupportedConnectionMediumsByPriority( - const ConnectionOptions& local_option); + const ConnectionOptions& local_connection_option); std::string GetStringValueOfSupportedMediums( const ConnectionOptions& connection_options) const; std::string GetStringValueOfSupportedMediums( @@ -518,7 +515,8 @@ class BasePcpHandler : public PcpHandler, // after reading the message (in which case, this alarm should be cancelled // as it's no longer needed), but this alarm is the fallback in case that // doesn't happen. - absl::flat_hash_map pending_alarms_; + absl::flat_hash_map> + pending_alarms_; // The active ClientProxy's connection lifecycle listener. Non-null while // advertising. diff --git a/connections/implementation/bwu_manager.cc b/connections/implementation/bwu_manager.cc index 796460ea..58ed0dbb 100644 --- a/connections/implementation/bwu_manager.cc +++ b/connections/implementation/bwu_manager.cc @@ -36,7 +36,6 @@ #include "internal/platform/count_down_latch.h" #include "internal/platform/feature_flags.h" #include "internal/platform/logging.h" -#include "proto/connections_enums.pb.h" namespace location { namespace nearby { @@ -1261,7 +1260,7 @@ void BwuManager::RetryUpgradesAfterDelay(ClientProxy* client, const std::string& endpoint_id) { absl::Duration delay = CalculateNextRetryDelay(endpoint_id); CancelRetryUpgradeAlarm(endpoint_id); - CancelableAlarm alarm( + auto alarm = std::make_unique( "BWU alarm", [this, client, endpoint_id]() { RunOnBwuManagerThread( @@ -1338,16 +1337,16 @@ void BwuManager::CancelRetryUpgradeAlarm(const std::string& endpoint_id) { auto item = retry_upgrade_alarms_.extract(endpoint_id); if (item.empty()) return; auto& pair = item.mapped(); - pair.first.Cancel(); + pair.first->Cancel(); } void BwuManager::CancelAllRetryUpgradeAlarms() { NEARBY_LOGS(INFO) << "CancelAllRetryUpgradeAlarms invoked"; for (auto& item : retry_upgrade_alarms_) { const std::string& endpoint_id = item.first; - CancelableAlarm& cancellable_alarm = item.second.first; + CancelableAlarm* cancellable_alarm = item.second.first.get(); NEARBY_LOGS(INFO) << "CancelRetryUpgradeAlarm for endpoint " << endpoint_id; - cancellable_alarm.Cancel(); + cancellable_alarm->Cancel(); } retry_upgrade_alarms_.clear(); retry_delays_.clear(); diff --git a/connections/implementation/bwu_manager.h b/connections/implementation/bwu_manager.h index 4654f239..6dea6879 100644 --- a/connections/implementation/bwu_manager.h +++ b/connections/implementation/bwu_manager.h @@ -27,7 +27,6 @@ #include "connections/implementation/client_proxy.h" #include "connections/implementation/endpoint_manager.h" #include "connections/implementation/mediums/mediums.h" -#include "internal/platform/byte_array.h" #include "internal/platform/scheduled_executor.h" namespace location { @@ -216,7 +215,8 @@ class BwuManager : public EndpointManager::FrameProcessor { absl::flat_hash_map in_progress_upgrades_; // Maps endpointId -> timestamp of when the SAFE_TO_CLOSE message was written. absl::flat_hash_map safe_to_close_write_timestamps_; - absl::flat_hash_map> + absl::flat_hash_map< + std::string, std::pair, absl::Duration>> retry_upgrade_alarms_; // Maps endpointId -> duration of delay before bwu retry. // When bwu failed, retry_upgrade_alarms_ will clear the entry before the diff --git a/connections/implementation/client_proxy.cc b/connections/implementation/client_proxy.cc index 3009d447..7e4027b2 100644 --- a/connections/implementation/client_proxy.cc +++ b/connections/implementation/client_proxy.cc @@ -726,7 +726,7 @@ void ClientProxy::ScheduleClearLocalHighVisModeCacheEndpointIdAlarm() { << "; local_high_vis_mode_cache_endpoint_id_=" << local_high_vis_mode_cache_endpoint_id_; clear_local_high_vis_mode_cache_endpoint_id_alarm_ = - CancelableAlarm( + std::make_unique( "clear_high_power_endpoint_id_cache", [this]() { MutexLock lock(&mutex_); @@ -742,9 +742,10 @@ void ClientProxy::ScheduleClearLocalHighVisModeCacheEndpointIdAlarm() { } void ClientProxy::CancelClearLocalHighVisModeCacheEndpointIdAlarm() { - if (clear_local_high_vis_mode_cache_endpoint_id_alarm_.IsValid()) { - clear_local_high_vis_mode_cache_endpoint_id_alarm_.Cancel(); - clear_local_high_vis_mode_cache_endpoint_id_alarm_ = CancelableAlarm(); + if (clear_local_high_vis_mode_cache_endpoint_id_alarm_ && + clear_local_high_vis_mode_cache_endpoint_id_alarm_->IsValid()) { + clear_local_high_vis_mode_cache_endpoint_id_alarm_->Cancel(); + clear_local_high_vis_mode_cache_endpoint_id_alarm_.reset(); } } diff --git a/connections/implementation/client_proxy.h b/connections/implementation/client_proxy.h index 107e37bd..5391a33f 100644 --- a/connections/implementation/client_proxy.h +++ b/connections/implementation/client_proxy.h @@ -270,7 +270,8 @@ class ClientProxy final { // expires. std::string local_high_vis_mode_cache_endpoint_id_; ScheduledExecutor single_thread_executor_; - CancelableAlarm clear_local_high_vis_mode_cache_endpoint_id_alarm_; + std::unique_ptr + clear_local_high_vis_mode_cache_endpoint_id_alarm_; // If not empty, we are currently advertising and accepting connection // requests for the given service_id. diff --git a/connections/implementation/mediums/ble_v2.cc b/connections/implementation/mediums/ble_v2.cc index 7893d6d6..6eea4409 100644 --- a/connections/implementation/mediums/ble_v2.cc +++ b/connections/implementation/mediums/ble_v2.cc @@ -26,8 +26,9 @@ #include "connections/implementation/mediums/ble_v2/bloom_filter.h" #include "connections/implementation/mediums/bluetooth_radio.h" #include "connections/implementation/mediums/utils.h" -#include "connections/implementation/mediums/uuid.h" +#include "connections/power_level.h" #include "internal/platform/byte_array.h" +#include "internal/platform/cancelable_alarm.h" #include "internal/platform/logging.h" #include "internal/platform/mutex_lock.h" @@ -44,11 +45,31 @@ using ::location::nearby::api::ble_v2::PowerMode; constexpr int kMaxAdvertisementLength = 512; constexpr int kDummyServiceIdLength = 128; +// Tell the thread annotation static analysis that `m` is already exclusively +// locked. Used because the analysis is not interprocedural. +void AssumeHeld(Mutex& m) ABSL_ASSERT_EXCLUSIVE_LOCK(m) {} + } // namespace +// These definitions are necessary before C++17. +constexpr absl::Duration BleV2::kPeripheralLostTimeout; + BleV2::BleV2(BluetoothRadio& radio) : radio_(radio), adapter_(radio_.GetBluetoothAdapter()) {} +BleV2::~BleV2() { + // Destructor is not taking locks, but methods it is calling are. + while (!scanned_service_ids_.empty()) { + StopScanning(*scanned_service_ids_.begin()); + } + while (!advertising_service_ids_.empty()) { + StopAdvertising(*advertising_service_ids_.begin()); + } + + serial_executor_.Shutdown(); + alarm_executor_.Shutdown(); +} + bool BleV2::IsAvailable() const { MutexLock lock(&mutex_); @@ -186,8 +207,10 @@ bool BleV2::StartAdvertising( NEARBY_LOGS(ERROR) << "Failed to turn on BLE advertising with advertisement bytes=" << absl::BytesToHexString(advertisement_bytes.data()) + << ", is_fast_advertisement=" << is_fast_advertisement << ", fast advertisement service uuid=" - << fast_advertisement_service_uuid; + << (is_fast_advertisement ? fast_advertisement_service_uuid + : "[empty]"); // If BLE advertising was not successful, stop the advertisement GATT // server. @@ -278,7 +301,9 @@ bool BleV2::StartScanning(const std::string& service_id, PowerLevel power_level, return false; } - // TODO(edwinwu): Start discovered peripheral tracking. + // Start to track the advertisement found for specific `service_id`. + discovered_peripheral_tracker_.StartTracking(service_id, std::move(callback), + fast_advertisement_service_uuid); // Check if scan has been activated, if yes, no need to notify client // to scan again. @@ -298,19 +323,54 @@ bool BleV2::StartScanning(const std::string& service_id, PowerLevel power_level, service_uuids, PowerLevelToPowerMode(power_level), { .advertisement_found_cb = - [](BleV2Peripheral peripheral, - const BleAdvertisementData& advertisement_data) { - // TODO(b/213835576): Move (or Copy at fallback) the - // BleV2Peripheral. - // TODO(b/216629800): Track the found advertisement. + [this](BleV2Peripheral peripheral, + BleAdvertisementData advertisement_data) { + RunOnBleThread([this, peripheral = std::move(peripheral), + advertisement_data]() { + MutexLock lock(&mutex_); + discovered_peripheral_tracker_ + .ProcessFoundBleAdvertisement( + std::move(peripheral), advertisement_data, + { + .fetch_advertisements = + [&](int num_slots, int psm, + const std::vector& + interesting_service_ids, + mediums::AdvertisementReadResult& + advertisement_read_result, + BleV2Peripheral& peripheral) { + // Th`mutex_` is already held here. Use + // `AssumeHeld` tell the thread + // annotation static analysis that + // `mutex_` is already exclusively + // locked. + AssumeHeld(mutex_); + ProcessFetchGattAdvertisementsRequest( + num_slots, psm, + interesting_service_ids, + advertisement_read_result, + peripheral); + }, + }); + }); }, })) { - NEARBY_LOGS(INFO) << "Failed to start client scan of BLE services."; + NEARBY_LOGS(INFO) << "Failed to start scan of BLE services."; + discovered_peripheral_tracker_.StopTracking(service_id); // Erase the service id that is just added. scanned_service_ids_.erase(service_id); return false; } + // Set up lost alarm. + lost_alarm_ = std::make_unique( + "BLE.StartScanning() onLost", + [this]() { + MutexLock lock(&mutex_); + discovered_peripheral_tracker_.ProcessLostGattAdvertisements(); + }, + kPeripheralLostTimeout, &alarm_executor_, /*is_recurring=*/true); + NEARBY_LOGS(INFO) << "Turned on BLE scanning with service id=" << service_id; return true; } @@ -324,17 +384,20 @@ bool BleV2::StopScanning(const std::string& service_id) { return false; } - // TODO(b/213835576): Cancel lost alarm and Stop tracking. - - scanned_service_ids_.erase(service_id); + discovered_peripheral_tracker_.StopTracking(service_id); NEARBY_LOGS(INFO) << "Turned off BLE scanning with service id=" << service_id; + scanned_service_ids_.erase(service_id); // If still has scanner, don't stop the client scanning. if (!scanned_service_ids_.empty()) { return true; } + // If no more scanning activities, then stop client scanning. NEARBY_LOGS(INFO) << "Turned off BLE client scanning"; + if (lost_alarm_->IsValid()) { + lost_alarm_->Cancel(); + } return medium_.StopScanning(); } @@ -424,6 +487,32 @@ bool BleV2::GenerateAdvertisementCharacteristic( return true; } +void BleV2::ProcessFetchGattAdvertisementsRequest( + int num_slots, int psm, + const std::vector& interesting_service_ids, + mediums::AdvertisementReadResult& advertisement_read_result, + BleV2Peripheral& peripheral) { + if (!peripheral.IsValid()) { + NEARBY_LOGS(INFO) << "Can't read from an advertisement GATT server because " + "ble peripheral is null."; + return; + } + + if (!radio_.IsEnabled()) { + NEARBY_LOGS(INFO) << "Can't read from an advertisement GATT server because " + "Bluetooth was never turned on."; + return; + } + + if (!IsAvailableLocked()) { + NEARBY_LOGS(INFO) << "Can't read from an advertisement GATT server because " + "BLE is not available."; + return; + } + + // TODO(edwinwu): Attempt to connect and read some GATT characteristics. +} + bool BleV2::StopAdvertisementGattServerLocked() { if (!IsAdvertisementGattServerRunningLocked()) { NEARBY_LOGS(INFO) << "Unable to stop the advertisement GATT server because " @@ -482,6 +571,10 @@ PowerMode BleV2::PowerLevelToPowerMode(PowerLevel power_level) { } } +void BleV2::RunOnBleThread(Runnable runnable) { + serial_executor_.Execute(std::move(runnable)); +} + } // namespace connections } // namespace nearby } // namespace location diff --git a/connections/implementation/mediums/ble_v2.h b/connections/implementation/mediums/ble_v2.h index de90fbba..7659f427 100644 --- a/connections/implementation/mediums/ble_v2.h +++ b/connections/implementation/mediums/ble_v2.h @@ -22,14 +22,17 @@ #include "absl/container/flat_hash_map.h" #include "absl/container/flat_hash_set.h" -#include "absl/strings/string_view.h" -#include "connections/advertising_options.h" -#include "connections/implementation/mediums/ble_v2/discovered_peripheral_callback.h" +#include "connections/implementation/mediums/ble_v2/advertisement_read_result.h" +#include "connections/implementation/mediums/ble_v2/discovered_peripheral_tracker.h" #include "connections/implementation/mediums/bluetooth_radio.h" +#include "connections/power_level.h" #include "internal/platform/ble_v2.h" #include "internal/platform/byte_array.h" +#include "internal/platform/cancelable_alarm.h" #include "internal/platform/mutex.h" #include "internal/platform/mutex_lock.h" +#include "internal/platform/scheduled_executor.h" +#include "internal/platform/single_thread_executor.h" namespace location { namespace nearby { @@ -43,13 +46,26 @@ class BleV2 final { BleV2Medium::ServerGattConnectionCallback; using DiscoveredPeripheralCallback = mediums::DiscoveredPeripheralCallback; + static constexpr absl::Duration kPeripheralLostTimeout = absl::Seconds(3); + explicit BleV2(BluetoothRadio& bluetooth_radio); + ~BleV2(); // Returns true, if BLE communications are supported by a platform. bool IsAvailable() const ABSL_LOCKS_EXCLUDED(mutex_); // Starts BLE advertising, delivering additional information if the platform // supports it. + // + // service_id - The service ID to track. + // advertisement_bytes - The connections BLE Advertisement used in + // advertising. + // power_level - The power level to use for the advertisement. + // fast_advertisement_service_uuid - The service UUID to look for fast + // advertisements on. + // Note: fast_advertisement_service_uuid can be an empty string to indicate + // that `fast_advertisement_service_uuid` will be ignored for regular + // advertisement. bool StartAdvertising(const std::string& service_id, const ByteArray& advertisement_bytes, PowerLevel power_level, @@ -63,19 +79,27 @@ class BleV2 final { bool IsAdvertising(const std::string& service_id) const ABSL_LOCKS_EXCLUDED(mutex_); - // Enables BLE scanning for a service id. Will report any discoverable + // Enables BLE scanning for a service ID. Will report any discoverable // advertisement data through a callback. // Returns true, if the scanning is successfully enabled, false otherwise. + // + // service_id - The service ID to track. + // power_level - The power level to use for the discovery. + // discovered_peripheral_callback - The callback to invoke for discovery + // events. + // Note: fast_advertisement_service_uuid can be emptry string to indicate that + // `fast_advertisement_service_uuid` will be ignored for regular + // advertisement. bool StartScanning(const std::string& service_id, PowerLevel power_level, DiscoveredPeripheralCallback callback, const std::string& fast_advertisement_service_uuid) ABSL_LOCKS_EXCLUDED(mutex_); - // Disables BLE scanning for a service id. + // Disables BLE scanning for a service ID. // Returns true, if the scanning was previously enabled, false otherwise. bool StopScanning(const std::string& service_id) ABSL_LOCKS_EXCLUDED(mutex_); - // Returns true if the scanning for service id is enabled. + // Returns true if the scanning for service ID is enabled. bool IsScanning(const std::string& service_id) const ABSL_LOCKS_EXCLUDED(mutex_); @@ -106,14 +130,23 @@ class BleV2 final { const ByteArray& gatt_advertisement, GattServer& gatt_server) ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + void ProcessFetchGattAdvertisementsRequest( + int num_slots, int psm, + const std::vector& interesting_service_ids, + mediums::AdvertisementReadResult& advertisement_read_result, + BleV2Peripheral& peripheral) ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_); bool StopAdvertisementGattServerLocked() ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_); ByteArray CreateAdvertisementHeader() ABSL_SHARED_LOCKS_REQUIRED(mutex_); - std::string GenerateAdvertisementUuid(int slot); api::ble_v2::PowerMode PowerLevelToPowerMode(PowerLevel power_level); + void RunOnBleThread(Runnable runnable); + + SingleThreadExecutor serial_executor_; + ScheduledExecutor alarm_executor_; + mutable Mutex mutex_; BluetoothRadio& radio_ ABSL_GUARDED_BY(mutex_); BluetoothAdapter& adapter_ ABSL_GUARDED_BY(mutex_); @@ -126,6 +159,9 @@ class BleV2 final { absl::flat_hash_set hosted_gatt_characteristics_ ABSL_GUARDED_BY(mutex_); absl::flat_hash_set scanned_service_ids_ ABSL_GUARDED_BY(mutex_); + std::unique_ptr lost_alarm_; + mediums::DiscoveredPeripheralTracker discovered_peripheral_tracker_ + ABSL_GUARDED_BY(mutex_); }; } // namespace connections diff --git a/connections/implementation/mediums/ble_v2/discovered_peripheral_tracker.cc b/connections/implementation/mediums/ble_v2/discovered_peripheral_tracker.cc index cbf0b390..b42e366b 100644 --- a/connections/implementation/mediums/ble_v2/discovered_peripheral_tracker.cc +++ b/connections/implementation/mediums/ble_v2/discovered_peripheral_tracker.cc @@ -42,7 +42,7 @@ void DiscoveredPeripheralTracker::StartTracking( .discovered_peripheral_callback = std::move(discovered_peripheral_callback), .lost_entity_tracker = - absl::make_unique>(), + std::make_unique>(), .fast_advertisement_service_uuid = fast_advertisement_service_uuid}; // Replace if key exists. @@ -66,8 +66,7 @@ void DiscoveredPeripheralTracker::StopTracking(const std::string& service_id) { void DiscoveredPeripheralTracker::ProcessFoundBleAdvertisement( BleV2Peripheral peripheral, - const ::location::nearby::api::ble_v2::BleAdvertisementData& - advertisement_data, + ::location::nearby::api::ble_v2::BleAdvertisementData advertisement_data, AdvertisementFetcher advertisement_fetcher) { MutexLock lock(&mutex_); @@ -195,7 +194,7 @@ void DiscoveredPeripheralTracker::HandleAdvertisement( // Process the fast advertisement like we would a GATT advertisement and // insert a placeholder AdvertisementReadResult. advertisement_read_results_.insert( - {advertisement_header, absl::make_unique()}); + {advertisement_header, std::make_unique()}); BleAdvertisementHeader new_advertisement_header = HandleRawGattAdvertisements( advertisement_header, {&advertisement_bytes}, service_uuid); @@ -560,34 +559,22 @@ DiscoveredPeripheralTracker::FetchRawAdvertisements( const BleAdvertisementHeader& advertisement_header, BleV2Peripheral& peripheral, AdvertisementFetcher advertisement_fetcher) { // Fetch the raw GATT advertisements and store the results. - AdvertisementReadResult* advertisement_read_result = nullptr; - const auto it = advertisement_read_results_.find(advertisement_header); - if (it != advertisement_read_results_.end()) { - advertisement_read_result = it->second.get(); + auto& result = advertisement_read_results_[advertisement_header]; + if (result == nullptr) { + result = std::make_unique(); } std::vector service_ids; std::transform(service_id_infos_.begin(), service_id_infos_.end(), std::back_inserter(service_ids), [](auto& kv) { return kv.first; }); - std::unique_ptr read_result = - advertisement_fetcher.fetch_advertisements( - advertisement_header.GetNumSlots(), advertisement_header.GetPsm(), - service_ids, advertisement_read_result, - /*mutated=*/peripheral); - if (!read_result) { - return {}; - } + advertisement_fetcher.fetch_advertisements( + advertisement_header.GetNumSlots(), advertisement_header.GetPsm(), + service_ids, *result, /*mutated=*/peripheral); - auto iterator_and_result_pair = advertisement_read_results_.insert_or_assign( - advertisement_header, std::move(read_result)); - // Take those results and return all the advertisements we were able to read. - std::vector advertisement_bytes_list; - if (iterator_and_result_pair.second) { - advertisement_bytes_list = - iterator_and_result_pair.first->second->GetAdvertisements(); - } - return advertisement_bytes_list; + // Take those results and return all the advertisements we were able to + // read. + return result->GetAdvertisements(); } void DiscoveredPeripheralTracker::UpdateCommonStateForFoundBleAdvertisement( diff --git a/connections/implementation/mediums/ble_v2/discovered_peripheral_tracker.h b/connections/implementation/mediums/ble_v2/discovered_peripheral_tracker.h index b90c8d47..fd560af2 100644 --- a/connections/implementation/mediums/ble_v2/discovered_peripheral_tracker.h +++ b/connections/implementation/mediums/ble_v2/discovered_peripheral_tracker.h @@ -46,14 +46,18 @@ class DiscoveredPeripheralTracker { struct AdvertisementFetcher { // Fetches relevant GATT advertisements for the peripheral found in {@link // DiscoveredPeripheralTracker#ProcessFoundBleAdvertisement(}. - std::function( + // + // `advertisement_read_result` is in/out mutable reference that the caller + // should take of its life cycle and pass a valid reference. + std::function& interesting_service_ids, - AdvertisementReadResult* advertisement_read_result, + mediums::AdvertisementReadResult& advertisement_read_result, BleV2Peripheral& peripheral)> - fetch_advertisements = [](int, int, const std::vector&, - AdvertisementReadResult*, BleV2Peripheral&) - -> std::unique_ptr { return nullptr; }; + fetch_advertisements = + DefaultCallback&, + mediums::AdvertisementReadResult&, + BleV2Peripheral&>(); }; // Starts tracking discoveries for a particular service Id. @@ -85,7 +89,7 @@ class DiscoveredPeripheralTracker { // processed. void ProcessFoundBleAdvertisement( BleV2Peripheral peripheral, - const api::ble_v2::BleAdvertisementData& advertisement_data, + api::ble_v2::BleAdvertisementData advertisement_data, AdvertisementFetcher advertisement_fetcher) ABSL_LOCKS_EXCLUDED(mutex_); // Processes the set of lost GATT advertisements and notifies the client of diff --git a/connections/implementation/mediums/ble_v2/discovered_peripheral_tracker_test.cc b/connections/implementation/mediums/ble_v2/discovered_peripheral_tracker_test.cc index f7ca0374..ed21adf7 100644 --- a/connections/implementation/mediums/ble_v2/discovered_peripheral_tracker_test.cc +++ b/connections/implementation/mediums/ble_v2/discovered_peripheral_tracker_test.cc @@ -168,21 +168,19 @@ class DiscoveredPeripheralTrackerTest : public testing::Test { [this, &fetch_latch, &advertisement_bytes_list]( int num_slots, int psm, const std::vector& interesting_service_ids, - AdvertisementReadResult* arr, BleV2Peripheral& peripheral) - -> std::unique_ptr { - MutexLock lock(&mutex_); - fetch_count_++; - auto advertisement_read_result = - std::make_unique(); - int slot = 0; - for (const auto& advertisement_bytes : advertisement_bytes_list) { - advertisement_read_result->AddAdvertisement(slot++, - advertisement_bytes); - } - advertisement_read_result->RecordLastReadStatus(/*isSuccess=*/true); - fetch_latch.CountDown(); - return advertisement_read_result; - }, + mediums::AdvertisementReadResult& advertisement_read_result, + BleV2Peripheral& peripheral) { + MutexLock lock(&mutex_); + fetch_count_++; + int slot = 0; + for (const auto& advertisement_bytes : advertisement_bytes_list) { + advertisement_read_result.AddAdvertisement(slot++, + advertisement_bytes); + } + advertisement_read_result.RecordLastReadStatus( + /*is_success=*/true); + fetch_latch.CountDown(); + }, }; } diff --git a/connections/implementation/mediums/ble_v2_test.cc b/connections/implementation/mediums/ble_v2_test.cc index 4b5503f1..10cd416a 100644 --- a/connections/implementation/mediums/ble_v2_test.cc +++ b/connections/implementation/mediums/ble_v2_test.cc @@ -16,14 +16,10 @@ #include -#include "gmock/gmock.h" -#include "protobuf-matchers/protocol-buffer-matchers.h" #include "gtest/gtest.h" #include "connections/implementation/mediums/ble_v2/discovered_peripheral_callback.h" #include "connections/implementation/mediums/bluetooth_radio.h" -#include "internal/platform/ble.h" #include "internal/platform/count_down_latch.h" -#include "internal/platform/logging.h" #include "internal/platform/medium_environment.h" namespace location { @@ -31,12 +27,14 @@ namespace nearby { namespace connections { namespace { +constexpr absl::Duration kWaitDuration = absl::Milliseconds(1000); constexpr absl::string_view kServiceIDA{ "com.google.location.nearby.apps.test.a"}; constexpr absl::string_view kServiceIDB{ "com.google.location.nearby.apps.test.b"}; constexpr absl::string_view kAdvertisementString{"\x0a\x0b\x0c\x0d"}; -constexpr absl::string_view kFastAdvertisementServiceUuid{"FAST"}; +constexpr absl::string_view kFastAdvertisementServiceUuid = + "0000FE2C-0000-1000-8000-00805F9B34FB"; class BleV2Test : public testing::Test { protected: @@ -60,40 +58,25 @@ TEST_F(BleV2Test, CanConstructValidObject) { env_.Stop(); } -TEST_F(BleV2Test, CanStartFastAdvertising) { - env_.Start(); - BluetoothRadio radio; - BleV2 ble{radio}; - radio.Enable(); - ByteArray advertisement_bytes{std::string(kAdvertisementString)}; - - EXPECT_TRUE(ble.StartAdvertising(std::string(kServiceIDA), - advertisement_bytes, PowerLevel::kHighPower, - std::string(kFastAdvertisementServiceUuid))); - // Can't advertise twice for the same service_id. - EXPECT_FALSE(ble.StartAdvertising( - std::string(kServiceIDA), advertisement_bytes, PowerLevel::kHighPower, - std::string(kFastAdvertisementServiceUuid))); - EXPECT_TRUE(ble.StopAdvertising(std::string(kServiceIDA))); - env_.Stop(); -} - TEST_F(BleV2Test, CanStartAdvertising) { env_.Start(); BluetoothRadio radio; BleV2 ble{radio}; radio.Enable(); ByteArray advertisement_bytes{std::string(kAdvertisementString)}; - std::string no_fast_advertisement_service_uuid = {}; EXPECT_TRUE(ble.StartAdvertising(std::string(kServiceIDA), advertisement_bytes, PowerLevel::kHighPower, - no_fast_advertisement_service_uuid)); + /*fast_advertisement_service_uuid=*/"")); + // Can't advertise twice for the same service_id. + EXPECT_FALSE(ble.StartAdvertising(std::string(kServiceIDA), + advertisement_bytes, PowerLevel::kHighPower, + /*fast_advertisement_service_uuid=*/"")); EXPECT_TRUE(ble.StopAdvertising(std::string(kServiceIDA))); env_.Stop(); } -TEST_F(BleV2Test, CanStartDiscovery) { +TEST_F(BleV2Test, CanStartScanning) { env_.Start(); BluetoothRadio radio; BleV2 ble{radio}; @@ -115,11 +98,90 @@ TEST_F(BleV2Test, CanStartDiscovery) { // nothing to do for now }, }, - std::string(kFastAdvertisementServiceUuid))); + /*fast_advertisement_service_uuid=*/"")); EXPECT_TRUE(ble.StopScanning(std::string(kServiceIDA))); env_.Stop(); } +TEST_F(BleV2Test, CanStartFastAdvertising) { + env_.Start(); + BluetoothRadio radio_a; + BluetoothRadio radio_b; + BleV2 ble_a{radio_a}; + BleV2 ble_b{radio_b}; + radio_a.Enable(); + radio_b.Enable(); + ByteArray advertisement_bytes{std::string(kAdvertisementString)}; + CountDownLatch found_latch(1); + + ble_b.StartScanning( + std::string(kServiceIDA), PowerLevel::kHighPower, + mediums::DiscoveredPeripheralCallback{ + .peripheral_discovered_cb = + [&found_latch](mediums::BlePeripheral& peripheral, + const std::string& service_id, + const ByteArray& advertisement_bytes, + bool fast_advertisement) { + EXPECT_TRUE(fast_advertisement); + found_latch.CountDown(); + }, + .peripheral_lost_cb = + [](mediums::BlePeripheral& peripheral, + const std::string& service_id) { + // nothing to do for now + }, + }, + std::string(kFastAdvertisementServiceUuid)); + + EXPECT_TRUE(ble_a.StartAdvertising( + std::string(kServiceIDA), advertisement_bytes, PowerLevel::kHighPower, + std::string(kFastAdvertisementServiceUuid))); + EXPECT_TRUE(found_latch.Await(kWaitDuration).result()); + EXPECT_TRUE(ble_a.StopAdvertising(std::string(kServiceIDA))); + ble_b.StopScanning(std::string(kServiceIDA)); + env_.Stop(); +} + +TEST_F(BleV2Test, CanStartFastScanning) { + env_.Start(); + BluetoothRadio radio_a; + BluetoothRadio radio_b; + BleV2 ble_a{radio_a}; + BleV2 ble_b{radio_b}; + radio_a.Enable(); + radio_b.Enable(); + ByteArray advertisement_bytes{std::string(kAdvertisementString)}; + CountDownLatch found_latch(1); + + ble_b.StartAdvertising(std::string(kServiceIDA), advertisement_bytes, + PowerLevel::kHighPower, + std::string(kFastAdvertisementServiceUuid)); + + EXPECT_TRUE(ble_a.StartScanning( + std::string(kServiceIDA), PowerLevel::kHighPower, + mediums::DiscoveredPeripheralCallback{ + .peripheral_discovered_cb = + [&found_latch](mediums::BlePeripheral& peripheral, + const std::string& service_id, + const ByteArray& advertisement_bytes, + bool fast_advertisement) { + EXPECT_TRUE(fast_advertisement); + found_latch.CountDown(); + }, + .peripheral_lost_cb = + [](mediums::BlePeripheral& peripheral, + const std::string& service_id) { + // nothing to do for now + }, + }, + std::string(kFastAdvertisementServiceUuid))); + + EXPECT_TRUE(found_latch.Await(kWaitDuration).result()); + ble_b.StopAdvertising(std::string(kServiceIDA)); + EXPECT_TRUE(ble_a.StopScanning(std::string(kServiceIDA))); + env_.Stop(); +} + TEST_F(BleV2Test, CanStartStopMultipleScanningWithDifferentServiceIds) { env_.Start(); BluetoothRadio radio; @@ -145,6 +207,133 @@ TEST_F(BleV2Test, CanStartStopMultipleScanningWithDifferentServiceIds) { env_.Stop(); } +TEST_F(BleV2Test, DestructWorksForStartAdvertisingAndScanningWithoutStop) { + env_.Start(); + BluetoothRadio radio_a; + BluetoothRadio radio_b; + BleV2 ble_a{radio_a}; + BleV2 ble_b{radio_b}; + + radio_a.Enable(); + radio_b.Enable(); + ByteArray advertisement_bytes{std::string(kAdvertisementString)}; + + // Device A starts advertising with service IDA and IDB. + EXPECT_TRUE(ble_a.StartAdvertising( + std::string(kServiceIDA), advertisement_bytes, PowerLevel::kHighPower, + std::string(kFastAdvertisementServiceUuid))); + EXPECT_TRUE(ble_a.StartAdvertising( + std::string(kServiceIDB), advertisement_bytes, PowerLevel::kHighPower, + std::string(kFastAdvertisementServiceUuid))); + + // Device B starts scanning with service IDA and IDB + EXPECT_TRUE(ble_b.StartScanning(std::string(kServiceIDA), + PowerLevel::kHighPower, + mediums::DiscoveredPeripheralCallback{}, + std::string(kFastAdvertisementServiceUuid))); + EXPECT_TRUE(ble_b.StartScanning(std::string(kServiceIDB), + PowerLevel::kHighPower, + mediums::DiscoveredPeripheralCallback{}, + std::string(kFastAdvertisementServiceUuid))); + env_.Stop(); +} + +TEST_F(BleV2Test, StartScanningDiscoverAndLostPeripheral) { + env_.Start(); + BluetoothRadio radio_a; + BluetoothRadio radio_b; + BleV2 ble_a{radio_a}; + BleV2 ble_b{radio_b}; + radio_a.Enable(); + radio_b.Enable(); + ByteArray advertisement_bytes{std::string(kAdvertisementString)}; + CountDownLatch found_latch(1); + CountDownLatch lost_latch(1); + + ble_b.StartAdvertising(std::string(kServiceIDA), advertisement_bytes, + PowerLevel::kHighPower, + std::string(kFastAdvertisementServiceUuid)); + + ble_a.StartScanning( + std::string(kServiceIDA), PowerLevel::kHighPower, + mediums::DiscoveredPeripheralCallback{ + .peripheral_discovered_cb = + [&found_latch](mediums::BlePeripheral& peripheral, + const std::string& service_id, + const ByteArray& advertisement_bytes, + bool fast_advertisement) { + EXPECT_TRUE(fast_advertisement); + found_latch.CountDown(); + }, + .peripheral_lost_cb = + [&lost_latch](mediums::BlePeripheral& peripheral, + const std::string& service_id) { + lost_latch.CountDown(); + }, + }, + std::string(kFastAdvertisementServiceUuid)); + + EXPECT_TRUE(found_latch.Await(kWaitDuration).result()); + + ble_b.StopAdvertising(std::string(kServiceIDA)); + + // Wait for a while (2 times delay) to let the alaram occur twice and + // `ProcessLostGattAdvertisements` twice to lost periperal. + SystemClock::Sleep(BleV2::kPeripheralLostTimeout * 2); + + EXPECT_TRUE(lost_latch.Await(kWaitDuration).result()); + + ble_a.StopScanning(std::string(kServiceIDA)); + env_.Stop(); +} + +TEST_F(BleV2Test, StartScanningDiscoverButNoPeripheralLostAfterStopScanning) { + env_.Start(); + BluetoothRadio radio_a; + BluetoothRadio radio_b; + BleV2 ble_a{radio_a}; + BleV2 ble_b{radio_b}; + radio_a.Enable(); + radio_b.Enable(); + ByteArray advertisement_bytes{std::string(kAdvertisementString)}; + CountDownLatch found_latch(1); + CountDownLatch lost_latch(1); + + ble_b.StartAdvertising(std::string(kServiceIDA), advertisement_bytes, + PowerLevel::kHighPower, + std::string(kFastAdvertisementServiceUuid)); + + ble_a.StartScanning( + std::string(kServiceIDA), PowerLevel::kHighPower, + mediums::DiscoveredPeripheralCallback{ + .peripheral_discovered_cb = + [&found_latch](mediums::BlePeripheral& peripheral, + const std::string& service_id, + const ByteArray& advertisement_bytes, + bool fast_advertisement) { + EXPECT_TRUE(fast_advertisement); + found_latch.CountDown(); + }, + .peripheral_lost_cb = + [&lost_latch](mediums::BlePeripheral& peripheral, + const std::string& service_id) { + lost_latch.CountDown(); + }, + }, + std::string(kFastAdvertisementServiceUuid)); + + EXPECT_TRUE(found_latch.Await(kWaitDuration).result()); + + ble_b.StopAdvertising(std::string(kServiceIDA)); + ble_a.StopScanning(std::string(kServiceIDA)); + + // Don't receive lost peripheral callback because we have stopped scanning and + // cancelled the alarm. + EXPECT_FALSE(lost_latch.Await(kWaitDuration).result()); + + env_.Stop(); +} + } // namespace } // namespace connections } // namespace nearby diff --git a/connections/implementation/mediums/webrtc.cc b/connections/implementation/mediums/webrtc.cc index 46857dcc..d549c856 100644 --- a/connections/implementation/mediums/webrtc.cc +++ b/connections/implementation/mediums/webrtc.cc @@ -16,20 +16,18 @@ #include #include +#include #include "absl/functional/bind_front.h" -#include "absl/strings/str_cat.h" #include "absl/time/time.h" #include "connections/implementation/mediums/webrtc/session_description_wrapper.h" #include "connections/implementation/mediums/webrtc/signaling_frames.h" #include "connections/implementation/mediums/webrtc_socket.h" #include "internal/platform/byte_array.h" -#include "internal/platform/listeners.h" #include "internal/platform/cancelable_alarm.h" #include "internal/platform/future.h" #include "internal/platform/logging.h" #include "internal/platform/mutex_lock.h" -#include "proto/mediums/web_rtc_signaling_frames.pb.h" #include "webrtc/api/jsep.h" namespace location { @@ -123,11 +121,12 @@ bool WebRtc::StartAcceptingConnections(const std::string& service_id, // We'll automatically disconnect from Tachyon after 60sec. When this alarm // fires, we'll recreate our room so we continue to receive messages. - info.restart_tachyon_receive_messages_alarm = CancelableAlarm( - "restart_receiving_messages_webrtc", - std::bind(&WebRtc::ProcessRestartTachyonReceiveMessages, this, - service_id), - kRestartReceiveMessagesDuration, &single_thread_executor_); + info.restart_tachyon_receive_messages_alarm = + std::make_unique( + "restart_receiving_messages_webrtc", + std::bind(&WebRtc::ProcessRestartTachyonReceiveMessages, this, + service_id), + kRestartReceiveMessagesDuration, &single_thread_executor_); // Now that we're set up to receive messages, we'll save our state and return // a successful result. @@ -156,9 +155,10 @@ void WebRtc::StopAcceptingConnections(const std::string& service_id) { info.signaling_messenger.reset(); // Cancel the scheduled alarm. - if (info.restart_tachyon_receive_messages_alarm.IsValid()) { - info.restart_tachyon_receive_messages_alarm.Cancel(); - info.restart_tachyon_receive_messages_alarm = CancelableAlarm(); + if (info.restart_tachyon_receive_messages_alarm && + info.restart_tachyon_receive_messages_alarm->IsValid()) { + info.restart_tachyon_receive_messages_alarm->Cancel(); + info.restart_tachyon_receive_messages_alarm.reset(); } // If we had any in-progress connections that haven't materialized into full diff --git a/connections/implementation/mediums/webrtc.h b/connections/implementation/mediums/webrtc.h index 58e46c1e..7f2eb9a7 100644 --- a/connections/implementation/mediums/webrtc.h +++ b/connections/implementation/mediums/webrtc.h @@ -21,24 +21,17 @@ #include #include "absl/container/flat_hash_map.h" -#include "absl/container/flat_hash_set.h" -#include "connections/implementation/proto/offline_wire_formats.pb.h" #include "connections/implementation/mediums/webrtc/connection_flow.h" -#include "connections/implementation/mediums/webrtc/data_channel_listener.h" -#include "connections/implementation/mediums/webrtc/local_ice_candidate_listener.h" -#include "connections/implementation/mediums/webrtc/webrtc_socket_impl.h" #include "connections/implementation/mediums/webrtc_peer_id.h" #include "connections/implementation/mediums/webrtc_socket.h" #include "internal/platform/byte_array.h" -#include "internal/platform/cancellation_flag.h" -#include "internal/platform/listeners.h" -#include "internal/platform/runnable.h" -#include "internal/platform/atomic_boolean.h" #include "internal/platform/cancelable_alarm.h" +#include "internal/platform/cancellation_flag.h" #include "internal/platform/future.h" +#include "internal/platform/listeners.h" #include "internal/platform/mutex.h" +#include "internal/platform/runnable.h" #include "internal/platform/scheduled_executor.h" -#include "internal/platform/single_thread_executor.h" #include "internal/platform/webrtc.h" #include "proto/mediums/web_rtc_signaling_frames.pb.h" #include "webrtc/api/jsep.h" @@ -122,7 +115,7 @@ class WebRtc { // streaming rpc times out. The streaming rpc times out after 60s while // advertising. Non-null when listening for WebRTC connections as an // offerer. - CancelableAlarm restart_tachyon_receive_messages_alarm; + std::unique_ptr restart_tachyon_receive_messages_alarm; // Tracks the number of times we've restarted receiving messages after a // failure. We limit the number to prevent endless restarts if we are diff --git a/internal/platform/ble_v2.cc b/internal/platform/ble_v2.cc index e00cb73b..45ea037d 100644 --- a/internal/platform/ble_v2.cc +++ b/internal/platform/ble_v2.cc @@ -15,6 +15,7 @@ #include "internal/platform/ble_v2.h" #include +#include #include "internal/platform/bluetooth_adapter.h" #include "internal/platform/implementation/ble_v2.h" @@ -49,7 +50,7 @@ bool BleV2Medium::StartScanning(const std::vector& service_uuids, { .advertisement_found_cb = [this](api::ble_v2::BlePeripheral& peripheral, - const BleAdvertisementData& advertisement_data) { + BleAdvertisementData advertisement_data) { MutexLock lock(&mutex_); if (peripherals_.contains(&peripheral)) { NEARBY_LOGS(INFO) diff --git a/internal/platform/ble_v2.h b/internal/platform/ble_v2.h index 9922436f..3ad4b3ec 100644 --- a/internal/platform/ble_v2.h +++ b/internal/platform/ble_v2.h @@ -15,17 +15,14 @@ #ifndef PLATFORM_PUBLIC_BLE_V2_H_ #define PLATFORM_PUBLIC_BLE_V2_H_ -#include +#include +#include +#include -#include "absl/container/flat_hash_map.h" #include "internal/platform/bluetooth_adapter.h" #include "internal/platform/byte_array.h" -#include "internal/platform/cancellation_flag.h" #include "internal/platform/implementation/ble_v2.h" -#include "internal/platform/implementation/platform.h" -#include "internal/platform/input_stream.h" #include "internal/platform/mutex.h" -#include "internal/platform/output_stream.h" namespace location { namespace nearby { diff --git a/internal/platform/ble_v2_test.cc b/internal/platform/ble_v2_test.cc index 32272c78..7743e7cb 100644 --- a/internal/platform/ble_v2_test.cc +++ b/internal/platform/ble_v2_test.cc @@ -15,9 +15,8 @@ #include "internal/platform/ble_v2.h" #include +#include -#include "gmock/gmock.h" -#include "protobuf-matchers/protocol-buffer-matchers.h" #include "gtest/gtest.h" #include "internal/platform/medium_environment.h" @@ -154,7 +153,7 @@ TEST_F(BleV2MediumTest, CanStartScanning) { { .advertisement_found_cb = [](BleV2Peripheral peripheral, - const BleAdvertisementData& advertisement_data) { + BleAdvertisementData advertisement_data) { // nothing to do for now }, })); diff --git a/internal/platform/cancelable_alarm.h b/internal/platform/cancelable_alarm.h index 68e0b440..860edac0 100644 --- a/internal/platform/cancelable_alarm.h +++ b/internal/platform/cancelable_alarm.h @@ -19,6 +19,7 @@ #include #include #include +#include #include "internal/platform/cancelable.h" #include "internal/platform/mutex.h" @@ -37,32 +38,45 @@ class CancelableAlarm { public: CancelableAlarm() = default; CancelableAlarm(absl::string_view name, std::function&& runnable, - absl::Duration delay, ScheduledExecutor* scheduled_executor) - : name_(name), - cancelable_(scheduled_executor->Schedule(std::move(runnable), delay)) {} - ~CancelableAlarm() = default; - CancelableAlarm(CancelableAlarm&& other) { *this = std::move(other); } - CancelableAlarm& operator=(CancelableAlarm&& other) { - MutexLock lock(&mutex_); - { - MutexLock other_lock(&other.mutex_); - name_ = std::move(other.name_); - cancelable_ = std::move(other.cancelable_); + absl::Duration delay, ScheduledExecutor* scheduled_executor, + bool is_recurring = false) + : name_(name), scheduled_executor_(scheduled_executor), delay_(delay) { + if (is_recurring) { + runnable_ = std::move(runnable); + Schedule(); + } else { + cancelable_ = scheduled_executor_->Schedule(std::move(runnable), delay_); } - return *this; } + ~CancelableAlarm() = default; bool Cancel() { MutexLock lock(&mutex_); return cancelable_.Cancel(); } - bool IsValid() { return cancelable_.IsValid(); } + bool IsValid() { + MutexLock lock(&mutex_); + return cancelable_.IsValid(); + } private: + void Schedule() { + MutexLock lock(&mutex_); + cancelable_ = scheduled_executor_->Schedule( + [this]() { + runnable_(); + Schedule(); + }, + delay_); + } + Mutex mutex_; std::string name_; Cancelable cancelable_; + ScheduledExecutor* scheduled_executor_; + absl::Duration delay_; + std::function runnable_; }; } // namespace nearby diff --git a/internal/platform/cancelable_alarm_test.cc b/internal/platform/cancelable_alarm_test.cc index 73407cd9..bde8524e 100644 --- a/internal/platform/cancelable_alarm_test.cc +++ b/internal/platform/cancelable_alarm_test.cc @@ -14,11 +14,12 @@ #include "internal/platform/cancelable_alarm.h" -#include "gmock/gmock.h" -#include "protobuf-matchers/protocol-buffer-matchers.h" +#include + #include "gtest/gtest.h" #include "absl/time/time.h" #include "internal/platform/atomic_boolean.h" +#include "internal/platform/atomic_reference.h" #include "internal/platform/scheduled_executor.h" namespace location { @@ -64,6 +65,18 @@ TEST(CancelableAlarmTest, CancelExpiredAlarmFails) { EXPECT_FALSE(alarm.Cancel()); } +TEST(CancelableAlarmTest, CanCreateRecurringAlarm) { + ScheduledExecutor alarm_executor; + AtomicReference count(0); + CancelableAlarm alarm( + "test_alarm", [&count]() { count.Set(count.Get() + 1); }, + absl::Milliseconds(100), &alarm_executor, /*is_recurring=*/true); + // Wait for 2 rounds (>100ms * 2) and expect the `count` = 2. + SystemClock::Sleep(absl::Milliseconds(290)); + alarm.Cancel(); + EXPECT_EQ(count.Get(), 2); +} + } // namespace } // namespace nearby } // namespace location diff --git a/internal/platform/implementation/ble_v2.h b/internal/platform/implementation/ble_v2.h index 937393e6..6d367bf0 100644 --- a/internal/platform/implementation/ble_v2.h +++ b/internal/platform/implementation/ble_v2.h @@ -333,9 +333,9 @@ class BleMedium { // for the whole peripheral(device) connection life cycle. struct ScanCallback { std::function + BleAdvertisementData advertisement_data)> advertisement_found_cb = - DefaultCallback(); + DefaultCallback(); }; // https://developer.android.com/reference/android/bluetooth/le/BluetoothLeScanner.html#startScan(java.util.List%3Candroid.bluetooth.le.ScanFilter%3E,%20android.bluetooth.le.ScanSettings,%20android.bluetooth.le.ScanCallback) diff --git a/internal/platform/implementation/g3/ble_v2.cc b/internal/platform/implementation/g3/ble_v2.cc index 11e17c50..05e5185b 100644 --- a/internal/platform/implementation/g3/ble_v2.cc +++ b/internal/platform/implementation/g3/ble_v2.cc @@ -17,6 +17,7 @@ #include #include #include +#include #include "absl/synchronization/mutex.h" #include "internal/platform/implementation/ble_v2.h" @@ -76,8 +77,19 @@ bool BleV2Medium::StartAdvertising( << ", power_mode=" << PowerModeToName(power_mode); absl::MutexLock lock(&mutex_); + + // Reassemble advertisement data from advertising and scan response + // data. + api::ble_v2::BleAdvertisementData advertisement_data; + if (!advertising_data.service_uuids.empty()) { + advertisement_data.service_uuids = advertising_data.service_uuids; + } else { + advertisement_data.service_uuids = scan_response_data.service_uuids; + } + advertisement_data.service_data = scan_response_data.service_data; + MediumEnvironment::Instance().UpdateBleV2MediumForAdvertising( - /*enabled=*/true, *this, adapter_->GetPeripheralV2(), scan_response_data); + /*enabled=*/true, *this, adapter_->GetPeripheralV2(), advertisement_data); return true; } diff --git a/internal/platform/medium_environment.cc b/internal/platform/medium_environment.cc index 22895e76..8d10bcd8 100644 --- a/internal/platform/medium_environment.cc +++ b/internal/platform/medium_environment.cc @@ -533,7 +533,7 @@ void MediumEnvironment::UpdateBleV2MediumForAdvertising( << ", enabled=" << enabled; for (auto& medium_info : ble_v2_mediums_) { const api::ble_v2::BleMedium* remote_medium = medium_info.first; - const BleV2MediumContext& remote_context = medium_info.second; + BleV2MediumContext& remote_context = medium_info.second; // Do not send notification to the same medium. if (remote_medium == &medium) continue; NEARBY_LOGS(INFO) @@ -541,7 +541,7 @@ void MediumEnvironment::UpdateBleV2MediumForAdvertising( << remote_medium << ", remote_medium_context=" << &remote_context << ", remote_context.peripheral=" << remote_context.ble_peripheral << ". Ready to call OnBleV2PeripheralStateChanged."; - OnBleV2PeripheralStateChanged(enabled, context, + OnBleV2PeripheralStateChanged(enabled, remote_context, context.advertisement_data, *context.ble_peripheral); }