From 64c715e53135b346d6fbc1a82ca91dabeaff8442 Mon Sep 17 00:00:00 2001 From: Janusz Sobczak Date: Tue, 6 Jun 2023 18:43:53 -0700 Subject: [PATCH] Improve BLE peripheral lifecycle Keep all discovered and explicitly created BLE peripherals in the same collection. The peripherals are removed when they haven't been used for a while. PiperOrigin-RevId: 538349622 --- .../implementation/windows/ble_gatt_server.cc | 8 +- .../platform/implementation/windows/ble_v2.cc | 137 ++++++++++-------- .../platform/implementation/windows/ble_v2.h | 23 +-- .../windows/ble_v2_peripheral.cc | 7 + .../windows/ble_v2_peripheral.h | 13 +- .../windows/ble_v2_peripheral_test.cc | 40 ++++- 6 files changed, 146 insertions(+), 82 deletions(-) diff --git a/internal/platform/implementation/windows/ble_gatt_server.cc b/internal/platform/implementation/windows/ble_gatt_server.cc index bfb14e6e..99061dfa 100644 --- a/internal/platform/implementation/windows/ble_gatt_server.cc +++ b/internal/platform/implementation/windows/ble_gatt_server.cc @@ -104,11 +104,9 @@ std::string ConvertGattStatusToString( BleGattServer::BleGattServer(api::BluetoothAdapter* adapter, api::ble_v2::ServerGattConnectionCallback callback) - : adapter_(dynamic_cast(adapter)) { - DCHECK_NE(adapter_, nullptr); - peripheral_.SetAddress(adapter_->GetMacAddress()); - gatt_connection_callback_ = std::move(callback); -} + : adapter_(dynamic_cast(adapter)), + peripheral_(adapter_->GetMacAddress()), + gatt_connection_callback_(std::move(callback)) {} absl::optional BleGattServer::CreateCharacteristic( diff --git a/internal/platform/implementation/windows/ble_v2.cc b/internal/platform/implementation/windows/ble_v2.cc index 4e0d5d90..09047a79 100644 --- a/internal/platform/implementation/windows/ble_v2.cc +++ b/internal/platform/implementation/windows/ble_v2.cc @@ -14,6 +14,7 @@ #include "internal/platform/implementation/windows/ble_v2.h" +#include #include #include #include @@ -27,6 +28,7 @@ #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" #include "absl/synchronization/mutex.h" +#include "absl/time/time.h" #include "internal/platform/bluetooth_adapter.h" #include "internal/platform/byte_array.h" #include "internal/platform/cancellation_flag.h" @@ -107,6 +109,10 @@ static constexpr uint64_t kGenerateSessionIdRetryLimit = 3; // Indicating failed to generate unused session id. static constexpr uint64_t kFailedGenerateSessionId = 0; +// Remove lost/unused peripherals after a timeout. +constexpr absl::Duration kPeripheralExpiryTime = absl::Minutes(15); +// Prevent too frequent cleanup tasks. +constexpr absl::Duration kMaxPeripheralCleanupFrequency = absl::Minutes(3); } // namespace BleV2Medium::BleV2Medium(api::BluetoothAdapter& adapter) @@ -209,11 +215,6 @@ bool BleV2Medium::StartScanning(const Uuid& service_uuid, service_uuid_ = service_uuid; tx_power_level_ = tx_power_level; scan_callback_ = std::move(callback); - { - absl::MutexLock lock(&peripheral_map_mutex_); - peripheral_map_.clear(); - mac_address_to_peripheral_id_map_.clear(); - } watcher_ = BluetoothLEAdvertisementWatcher(); watcher_token_ = watcher_.Stopped({this, &BleV2Medium::WatcherHandler}); @@ -919,33 +920,16 @@ void BleV2Medium::AdvertisementReceivedHandler( advertisement_data.AsStringView()) << "(" << advertisement_data.size() << ")"; - std::string peripheral_name = + std::string bluetooth_address = uint64_to_mac_address_string(args.BluetoothAddress()); - - auto peripheral = std::make_unique(); - std::string mac_address_string = - uint64_to_mac_address_string(args.BluetoothAddress()); - peripheral->SetAddress(mac_address_string); - BleV2Peripheral* peripheral_ptr = nullptr; - { - absl::MutexLock lock(&peripheral_map_mutex_); - if (mac_address_to_peripheral_id_map_.contains(mac_address_string)) { - peripheral_map_[mac_address_to_peripheral_id_map_[mac_address_string]] - ->SetAddress( - uint64_to_mac_address_string(args.BluetoothAddress())); - } else { - mac_address_to_peripheral_id_map_[mac_address_string] = - peripheral->GetUniqueId(); - peripheral_map_[peripheral->GetUniqueId()] = std::move(peripheral); - } - peripheral_ptr = - peripheral_map_ - [mac_address_to_peripheral_id_map_[mac_address_string]] - .get(); + BleV2Peripheral* peripheral_ptr = + GetOrCreatePeripheral(bluetooth_address); + if (peripheral_ptr == nullptr) { + NEARBY_LOGS(ERROR) << "No BLE peripheral with address: " + << bluetooth_address; + return; } - - NEARBY_LOGS(VERBOSE) << "New BLE peripheral: " << peripheral_ptr - << ", address: " << peripheral_ptr->GetAddress(); + NEARBY_LOGS(INFO) << "BLE peripheral with address: " << bluetooth_address; // Received Advertisement packet NEARBY_LOGS(INFO) << "unconsumed_buffer_length: " @@ -1026,23 +1010,13 @@ void BleV2Medium::AdvertisementFoundHandler( // Save the BleV2Peripheral. std::string bluetooth_address = uint64_to_mac_address_string(args.BluetoothAddress()); - BleV2Peripheral* peripheral_ptr = nullptr; - { - absl::MutexLock lock(&map_mutex_); - if (address_to_peripheral_map_.find(bluetooth_address) == - address_to_peripheral_map_.end()) { - NEARBY_LOGS(INFO) << "New BLE peripheral with address: " - << bluetooth_address; - address_to_peripheral_map_[bluetooth_address] = - std::make_unique(); - } else { - NEARBY_LOGS(INFO) << "Already existing BLE peripheral with address: " - << bluetooth_address; - } - address_to_peripheral_map_[bluetooth_address]->SetAddress( - bluetooth_address); - peripheral_ptr = address_to_peripheral_map_[bluetooth_address].get(); + BleV2Peripheral* peripheral_ptr = GetOrCreatePeripheral(bluetooth_address); + if (peripheral_ptr == nullptr) { + NEARBY_LOGS(ERROR) << "No BLE peripheral with address: " + << bluetooth_address; + return; } + NEARBY_LOGS(INFO) << "BLE peripheral with address: " << bluetooth_address; // Invokes callbacks that matches the UUID. for (auto service_uuid : service_uuid_list) { @@ -1062,27 +1036,22 @@ void BleV2Medium::AdvertisementFoundHandler( bool BleV2Medium::GetRemotePeripheral(const std::string& mac_address, GetRemotePeripheralCallback callback) { - absl::MutexLock lock(&peripheral_map_mutex_); - for (auto& item : peripheral_map_) { - if (item.second->GetAddress() == mac_address) { - NEARBY_LOGS(WARNING) << __func__ << ": No matched peripheral device."; - callback(*(item.second)); - return true; - } + BleV2Peripheral* peripheral = GetOrCreatePeripheral(mac_address); + if (peripheral != nullptr && peripheral->Ok()) { + callback(*peripheral); + return true; } - return false; } bool BleV2Medium::GetRemotePeripheral(api::ble_v2::BlePeripheral::UniqueId id, GetRemotePeripheralCallback callback) { - absl::MutexLock lock(&peripheral_map_mutex_); - auto it = peripheral_map_.find(id); - if (it == peripheral_map_.end()) { + BleV2Peripheral* peripheral = GetPeripheral(id); + if (peripheral == nullptr) { NEARBY_LOGS(WARNING) << __func__ << ": No matched peripheral device."; return false; } - callback(*(it->second)); + callback(*peripheral); return true; } @@ -1099,5 +1068,57 @@ uint64_t BleV2Medium::GenerateSessionId() { return kFailedGenerateSessionId; } +BleV2Peripheral* BleV2Medium::GetOrCreatePeripheral(absl::string_view address) { + absl::MutexLock lock(&peripheral_map_mutex_); + auto it = std::find_if( + peripheral_map_.begin(), peripheral_map_.end(), [&](const auto& item) { + return item.second.peripheral->GetAddress() == address; + }); + if (it != peripheral_map_.end()) { + it->second.last_access_time = absl::Now(); + return it->second.peripheral.get(); + } + RemoveExpiredPeripherals(); + PeripheralInfo peripheral_info{ + .last_access_time = absl::Now(), + .peripheral = std::make_unique(address), + }; + BleV2Peripheral* peripheral = peripheral_info.peripheral.get(); + if (!peripheral->Ok()) { + NEARBY_LOGS(WARNING) << __func__ << "Invalid MAC address: " << address; + return nullptr; + } + NEARBY_LOGS(INFO) << "New BLE peripheral with address: " << address; + + peripheral_map_[peripheral->GetUniqueId()] = std::move(peripheral_info); + return peripheral; +} + +BleV2Peripheral* BleV2Medium::GetPeripheral(BleV2Peripheral::UniqueId id) { + absl::MutexLock lock(&peripheral_map_mutex_); + auto it = peripheral_map_.find(id); + if (it == peripheral_map_.end()) { + return nullptr; + } + it->second.last_access_time = absl::Now(); + return it->second.peripheral.get(); +} + +void BleV2Medium::RemoveExpiredPeripherals() { + absl::Time now = absl::Now(); + if (cleanup_time_ + kMaxPeripheralCleanupFrequency < now) { + return; + } + cleanup_time_ = now; + absl::Time cut_off_time = now - kPeripheralExpiryTime; + for (auto it = peripheral_map_.begin(); it != peripheral_map_.end();) { + if (it->second.last_access_time < cut_off_time) { + peripheral_map_.erase(it++); + } else { + ++it; + } + } +} + } // namespace windows } // namespace nearby diff --git a/internal/platform/implementation/windows/ble_v2.h b/internal/platform/implementation/windows/ble_v2.h index a369a7ef..5e6f8d53 100644 --- a/internal/platform/implementation/windows/ble_v2.h +++ b/internal/platform/implementation/windows/ble_v2.h @@ -112,6 +112,13 @@ class BleV2Medium : public api::ble_v2::BleMedium { BluetoothLEAdvertisementWatcherStoppedEventArgs args); uint64_t GenerateSessionId(); + // Returns nullptr if `address` is invalid. + BleV2Peripheral* GetOrCreatePeripheral(absl::string_view address); + // Returns nullptr if `id` does not match a known peripheral. + BleV2Peripheral* GetPeripheral(BleV2Peripheral::UniqueId id); + + void RemoveExpiredPeripherals() + ABSL_EXCLUSIVE_LOCKS_REQUIRED(peripheral_map_mutex_); BluetoothAdapter* adapter_; Uuid service_uuid_; @@ -119,9 +126,6 @@ class BleV2Medium : public api::ble_v2::BleMedium { ScanCallback scan_callback_; absl::Mutex map_mutex_; - // std::map> - absl::flat_hash_map> - address_to_peripheral_map_ ABSL_GUARDED_BY(map_mutex_); // std::map> absl::flat_hash_map> service_uuid_to_session_map_ ABSL_GUARDED_BY(map_mutex_); @@ -141,15 +145,16 @@ class BleV2Medium : public api::ble_v2::BleMedium { ::winrt::event_token advertisement_received_token_; BleGattServer* ble_gatt_server_ = nullptr; - // Map to protect the pointer for BlePeripheral because // DiscoveredPeripheralCallback only keeps the pointer to the object absl::Mutex peripheral_map_mutex_; - absl::flat_hash_map> - peripheral_map_ ABSL_GUARDED_BY(peripheral_map_mutex_); - absl::flat_hash_map - mac_address_to_peripheral_id_map_ ABSL_GUARDED_BY(peripheral_map_mutex_); + struct PeripheralInfo { + absl::Time last_access_time; + std::unique_ptr peripheral; + }; + absl::flat_hash_map peripheral_map_ + ABSL_GUARDED_BY(peripheral_map_mutex_); + absl::Time cleanup_time_ ABSL_GUARDED_BY(peripheral_map_mutex_) = absl::Now(); }; } // namespace windows diff --git a/internal/platform/implementation/windows/ble_v2_peripheral.cc b/internal/platform/implementation/windows/ble_v2_peripheral.cc index b1e3597d..c6972c48 100644 --- a/internal/platform/implementation/windows/ble_v2_peripheral.cc +++ b/internal/platform/implementation/windows/ble_v2_peripheral.cc @@ -17,6 +17,7 @@ #include #include "absl/strings/string_view.h" +#include "internal/platform/bluetooth_utils.h" #include "internal/platform/logging.h" namespace nearby { @@ -25,6 +26,12 @@ namespace { constexpr int kMacAddressLength = 17; } +BleV2Peripheral::BleV2Peripheral(absl::string_view address) { + if (SetAddress(address)) { + unique_id_ = static_cast(BluetoothUtils::ToNumber(address)); + } +} + bool BleV2Peripheral::SetAddress(absl::string_view address) { // The address must be in format "00:B0:D0:63:C2:26". if (address.size() != kMacAddressLength) { diff --git a/internal/platform/implementation/windows/ble_v2_peripheral.h b/internal/platform/implementation/windows/ble_v2_peripheral.h index 6c890d9c..6486cb2b 100644 --- a/internal/platform/implementation/windows/ble_v2_peripheral.h +++ b/internal/platform/implementation/windows/ble_v2_peripheral.h @@ -19,7 +19,6 @@ #include "absl/strings/string_view.h" #include "internal/platform/implementation/ble_v2.h" -#include "internal/platform/prng.h" namespace nearby { namespace windows { @@ -28,23 +27,25 @@ namespace windows { // about a particular BLE device to connect to its GATT server. class BleV2Peripheral : public api::ble_v2::BlePeripheral { public: - BleV2Peripheral() { unique_id_ = Prng().NextInt64(); } + using UniqueId = api::ble_v2::BlePeripheral::UniqueId; + explicit BleV2Peripheral(absl::string_view address); ~BleV2Peripheral() override = default; // Returns the MAC address of the peripheral. The format is in // "00:B0:D0:63:C2:26". std::string GetAddress() const override { return address_; } - api::ble_v2::BlePeripheral::UniqueId GetUniqueId() const override { - return unique_id_; - } + UniqueId GetUniqueId() const override { return unique_id_; } // Sets the MAC address of the peripheral. The address format must be in // pattern of "00:B0:D0:63:C2:26". bool SetAddress(absl::string_view address); + bool Ok() const { return unique_id_ != 0; } + explicit operator bool() const { return Ok(); } + private: std::string address_; - api::ble_v2::BlePeripheral::UniqueId unique_id_; + UniqueId unique_id_ = 0; }; } // namespace windows diff --git a/internal/platform/implementation/windows/ble_v2_peripheral_test.cc b/internal/platform/implementation/windows/ble_v2_peripheral_test.cc index a1ed3843..8902a21c 100644 --- a/internal/platform/implementation/windows/ble_v2_peripheral_test.cc +++ b/internal/platform/implementation/windows/ble_v2_peripheral_test.cc @@ -20,8 +20,18 @@ namespace nearby { namespace windows { namespace { +TEST(BleV2Peripheral, Constructor) { + constexpr absl::string_view kAddress = "F1:F2:F3:F4:F5:F6"; + BleV2Peripheral ble_peripheral(kAddress); + + EXPECT_TRUE(ble_peripheral); + EXPECT_TRUE(ble_peripheral.Ok()); + EXPECT_NE(ble_peripheral.GetUniqueId(), 0); + EXPECT_EQ(ble_peripheral.GetAddress(), kAddress); +} + TEST(BleV2Peripheral, SetMacAddress) { - BleV2Peripheral ble_peripheral; + BleV2Peripheral ble_peripheral("F1:F2:F3:F4:F5:F6"); EXPECT_TRUE(ble_peripheral.SetAddress("00:B0:D0:63:C2:26")); EXPECT_FALSE(ble_peripheral.SetAddress("00:B0:D0:6T:C2:26")); EXPECT_FALSE(ble_peripheral.SetAddress("00:B0:D0:63:C2:2")); @@ -30,9 +40,31 @@ TEST(BleV2Peripheral, SetMacAddress) { } TEST(BleV2Peripheral, SetAndGetMacAddress) { - BleV2Peripheral ble_peripheral; - EXPECT_TRUE(ble_peripheral.SetAddress("00:B0:D0:63:C2:26")); - EXPECT_EQ(ble_peripheral.GetAddress(), "00:B0:D0:63:C2:26"); + constexpr absl::string_view kChangedAddress = "00:B0:D0:63:C2:26"; + BleV2Peripheral ble_peripheral("F1:F2:F3:F4:F5:F6"); + + EXPECT_TRUE(ble_peripheral.SetAddress(kChangedAddress)); + EXPECT_EQ(ble_peripheral.GetAddress(), kChangedAddress); +} + +TEST(BleV2Peripheral, SetAddressDoesNotChangeUniqueId) { + constexpr absl::string_view kChangedAddress = "00:B0:D0:63:C2:26"; + BleV2Peripheral ble_peripheral("F1:F2:F3:F4:F5:F6"); + BleV2Peripheral::UniqueId unique_id = ble_peripheral.GetUniqueId(); + + EXPECT_NE(unique_id, 0); + EXPECT_TRUE(ble_peripheral.SetAddress(kChangedAddress)); + EXPECT_EQ(ble_peripheral.GetAddress(), kChangedAddress); + EXPECT_EQ(ble_peripheral.GetUniqueId(), unique_id); +} + +TEST(BleV2Peripheral, ConstructFromBadAddress) { + BleV2Peripheral ble_peripheral("G1:F2:F3:F4:F5:F6"); + + EXPECT_FALSE(ble_peripheral); + EXPECT_FALSE(ble_peripheral.Ok()); + EXPECT_EQ(ble_peripheral.GetUniqueId(), 0); + EXPECT_EQ(ble_peripheral.GetAddress(), ""); } } // namespace