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

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<BluetoothAdapter*>(adapter)) {
-  DCHECK_NE(adapter_, nullptr);
-  peripheral_.SetAddress(adapter_->GetMacAddress());
-  gatt_connection_callback_ = std::move(callback);
-}
+    : adapter_(dynamic_cast<BluetoothAdapter*>(adapter)),
+      peripheral_(adapter_->GetMacAddress()),
+      gatt_connection_callback_(std::move(callback)) {}
 
 absl::optional<api::ble_v2::GattCharacteristic>
 BleGattServer::CreateCharacteristic(

internal/platform/implementation/windows/ble_v2.cc

@@ -14,6 +14,7 @@
 
 #include "internal/platform/implementation/windows/ble_v2.h"
 
+#include <algorithm>
 #include <array>
 #include <exception>
 #include <iostream>
@@ -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<BleV2Peripheral>();
-      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<BleV2Peripheral>();
-    } 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<BleV2Peripheral>(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

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<std::string, std::unique_ptr<BleV2Peripheral>>
-  absl::flat_hash_map<std::string, std::unique_ptr<BleV2Peripheral>>
-      address_to_peripheral_map_ ABSL_GUARDED_BY(map_mutex_);
   // std::map<Uuid, std::map<uint64_t, ScanningCallback>>
   absl::flat_hash_map<Uuid, absl::flat_hash_map<uint64_t, ScanningCallback>>
       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<BleV2Peripheral::UniqueId,
-                      std::unique_ptr<BleV2Peripheral>>
-      peripheral_map_ ABSL_GUARDED_BY(peripheral_map_mutex_);
-  absl::flat_hash_map<std::string, BleV2Peripheral::UniqueId>
-      mac_address_to_peripheral_id_map_ ABSL_GUARDED_BY(peripheral_map_mutex_);
+  struct PeripheralInfo {
+    absl::Time last_access_time;
+    std::unique_ptr<BleV2Peripheral> peripheral;
+  };
+  absl::flat_hash_map<BleV2Peripheral::UniqueId, PeripheralInfo> peripheral_map_
+      ABSL_GUARDED_BY(peripheral_map_mutex_);
+  absl::Time cleanup_time_ ABSL_GUARDED_BY(peripheral_map_mutex_) = absl::Now();
 };
 
 }  // namespace windows

internal/platform/implementation/windows/ble_v2_peripheral.cc

@@ -17,6 +17,7 @@
 #include <string>
 
 #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<UniqueId>(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) {

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

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