Cleanup BLEv2Medium

PiperOrigin-RevId: 753288576

internal/platform/BUILD

@@ -356,7 +356,6 @@ cc_library(
         "//:__subpackages__",
     ],
     deps = [
-        ":logging",
         "@com_google_absl//absl/strings",
         "@com_google_absl//absl/strings:str_format",
     ],
@@ -592,6 +591,7 @@ cc_test(
     deps = [
         ":mac_address",
         "@com_github_protobuf_matchers//protobuf-matchers",
+        "@com_google_absl//absl/hash:hash_testing",
         "@com_google_googletest//:gtest_main",
     ],
 )

internal/platform/implementation/windows/ble_gatt_server.cc

@@ -115,7 +115,7 @@ std::string ConvertGattStatusToString(
 BleGattServer::BleGattServer(api::BluetoothAdapter* adapter,
                              api::ble_v2::ServerGattConnectionCallback callback)
     : adapter_(dynamic_cast<BluetoothAdapter*>(adapter)),
-      peripheral_(adapter_->GetMacAddress()),
+      peripheral_(adapter_->mac_address()),
       gatt_connection_callback_(std::move(callback)) {
   DCHECK(adapter_ != nullptr);
 }

internal/platform/implementation/windows/ble_v2.cc

@@ -52,6 +52,7 @@
 #include "internal/platform/implementation/windows/bluetooth_adapter.h"
 #include "internal/platform/implementation/windows/utils.h"
 #include "internal/platform/logging.h"
+#include "internal/platform/mac_address.h"
 #include "internal/platform/prng.h"
 #include "internal/platform/uuid.h"
 #include "winrt/Windows.Devices.Bluetooth.Advertisement.h"
@@ -133,7 +134,7 @@ std::string TxPowerLevelToName(TxPowerLevel tx_power_level) {
 // Use the device MAC address as the advertisement hash so that we have a unique
 // advertisement header for each device.
 BleAdvertisementHeader CreateAdvertisementHeader(
-    const std::string& mac_address,
+    MacAddress mac_address,
     const std::vector<std::string>& service_ids) {
   BloomFilter bloom_filter(
       std::make_unique<BitSetImpl<
@@ -141,13 +142,15 @@ BleAdvertisementHeader CreateAdvertisementHeader(
   for (const auto& service_id : service_ids) {
     bloom_filter.Add(service_id);
   }
+  std::string mac_address_string = mac_address.ToString();
   return BleAdvertisementHeader(
       BleAdvertisementHeader::Version::kV2,
       /*support_extended_advertisement=*/false,
       /*num_slots=*/1, ByteArray(bloom_filter),
       /*advertisement_hash=*/
       connections::Utils::Sha256Hash(
-          mac_address, BleAdvertisementHeader::kAdvertisementHashByteLength),
+          mac_address_string,
+          BleAdvertisementHeader::kAdvertisementHashByteLength),
       /*psm=*/BleAdvertisementHeader::kDefaultPsmValue);
 }
 
@@ -593,9 +596,9 @@ std::unique_ptr<api::ble_v2::GattClient> BleV2Medium::ConnectToGattServer(
             << ", power:" << TxPowerLevelToName(tx_power_level);
 
   try {
+    // In windows, peripheral unique id is the same as the bluetooth address.
     BluetoothLEDevice ble_device =
-        BluetoothLEDevice::FromBluetoothAddressAsync(
-            mac_address_string_to_uint64(peripheral.GetAddress()))
+        BluetoothLEDevice::FromBluetoothAddressAsync(peripheral.GetUniqueId())
             .get();
 
     return std::make_unique<BleGattClient>(ble_device);
@@ -1112,8 +1115,12 @@ void BleV2Medium::AdvertisementReceivedHandler(
   // Advertisement Scan Response packet (containing Copresence UUID 0xFEF3 in
   // 0x16 Service Data) has been received in the handler
   BluetoothLEAdvertisement advertisement = args.Advertisement();
-  std::string bluetooth_address =
-      uint64_to_mac_address_string(args.BluetoothAddress());
+  MacAddress bluetooth_address;
+  if (!MacAddress::FromUint64(args.BluetoothAddress(), bluetooth_address)) {
+    LOG(ERROR) << "Invalid MAC address: 0x"
+               << absl::StrCat(absl::Hex(args.BluetoothAddress()));
+    return;
+  }
   bool has_primary_service_data = false;
   std::vector<std::string> alt_service_ids;
 
@@ -1147,15 +1154,14 @@ void BleV2Medium::AdvertisementReceivedHandler(
         absl::MutexLock lock(&mutex_);
         peripheral_ptr = GetOrCreatePeripheral(bluetooth_address);
         if (peripheral_ptr == nullptr) {
-          LOG(ERROR) << "No BLE peripheral with address: " << bluetooth_address;
           return;
         }
       }
-      LOG(INFO) << "BLE peripheral with address: " << bluetooth_address;
+      LOG(INFO) << "BLE peripheral with address: "
+                << bluetooth_address.ToString();
 
       // Received Advertisement packet
-      LOG(INFO) << "unconsumed_buffer_length: "
-                << static_cast<int>(unconsumed_buffer_length);
+      VLOG(1) << "unconsumed_buffer_length: " << unconsumed_buffer_length;
 
       api::ble_v2::BleAdvertisementData ble_advertisement_data;
       if (unconsumed_buffer_length <= 27) {
@@ -1185,12 +1191,12 @@ void BleV2Medium::AdvertisementReceivedHandler(
       absl::MutexLock lock(&mutex_);
       peripheral_ptr = GetOrCreatePeripheral(bluetooth_address);
       if (peripheral_ptr == nullptr) {
-        LOG(ERROR) << "No BLE peripheral with address: " << bluetooth_address;
         return;
       }
     }
-    LOG(INFO) << "Found BLE peripheral for with address: " << bluetooth_address
-              << " for services: " << absl::StrJoin(alt_service_ids, ",");
+    VLOG(1) << "Found BLE peripheral for with address: "
+            << bluetooth_address.ToString()
+            << " for services: " << absl::StrJoin(alt_service_ids, ",");
     // Create fake advertisement data.
     api::ble_v2::BleAdvertisementData ble_advertisement_data;
     ble_advertisement_data.is_extended_advertisement = false;
@@ -1262,18 +1268,21 @@ void BleV2Medium::AdvertisementFoundHandler(
     return;
   }
   // Save the BleV2Peripheral.
-  std::string bluetooth_address =
-      uint64_to_mac_address_string(args.BluetoothAddress());
+  MacAddress bluetooth_address;
+  if (!MacAddress::FromUint64(args.BluetoothAddress(), bluetooth_address)) {
+    LOG(ERROR) << "Invalid MAC address: " << args.BluetoothAddress();
+    return;
+  }
+
   BleV2Peripheral* peripheral_ptr = nullptr;
   {
     absl::MutexLock lock(&mutex_);
     peripheral_ptr = GetOrCreatePeripheral(bluetooth_address);
     if (peripheral_ptr == nullptr) {
-      LOG(ERROR) << "No BLE peripheral with address: " << bluetooth_address;
       return;
     }
   }
-  LOG(INFO) << "BLE peripheral with address: " << bluetooth_address;
+  VLOG(1) << "BLE peripheral with address: " << bluetooth_address.ToString();
 
   // Invokes callbacks that matches the UUID.
   for (auto service_uuid : service_uuid_list) {
@@ -1293,10 +1302,15 @@ void BleV2Medium::AdvertisementFoundHandler(
 
 bool BleV2Medium::GetRemotePeripheral(const std::string& mac_address,
                                       GetRemotePeripheralCallback callback) {
+  MacAddress bluetooth_address;
+  if (!MacAddress::FromString(mac_address, bluetooth_address)) {
+    LOG(WARNING) << __func__ << ": Invalid MAC address: " << mac_address;
+    return false;
+  }
   BleV2Peripheral* peripheral = nullptr;
   {
     absl::MutexLock lock(&mutex_);
-    peripheral = GetOrCreatePeripheral(mac_address);
+    peripheral = GetOrCreatePeripheral(bluetooth_address);
   }
 
   if (peripheral != nullptr && peripheral->Ok()) {
@@ -1308,10 +1322,16 @@ bool BleV2Medium::GetRemotePeripheral(const std::string& mac_address,
 
 bool BleV2Medium::GetRemotePeripheral(api::ble_v2::BlePeripheral::UniqueId id,
                                       GetRemotePeripheralCallback callback) {
+  MacAddress bluetooth_address;
+  if (!MacAddress::FromUint64(id, bluetooth_address)) {
+    LOG(WARNING) << __func__ << ": Invalid MAC address: 0x"
+                 << absl::StrCat(absl::Hex(id));
+    return false;
+  }
   BleV2Peripheral* peripheral = nullptr;
   {
     absl::MutexLock lock(&mutex_);
-    peripheral = GetPeripheral(id);
+    peripheral = GetPeripheral(bluetooth_address);
   }
 
   if (peripheral == nullptr) {
@@ -1334,33 +1354,30 @@ uint64_t BleV2Medium::GenerateSessionId() {
   return kFailedGenerateSessionId;
 }
 
-BleV2Peripheral* BleV2Medium::GetOrCreatePeripheral(absl::string_view address) {
-  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();
+BleV2Peripheral* BleV2Medium::GetOrCreatePeripheral(MacAddress address) {
+  if (!address.IsSet()) {
+    LOG(WARNING) << __func__ << "empty MAC address is not allowed.";
+    return nullptr;
+  }
+  // For Windows peripheral uniqueId is the same as the address.
+  BleV2Peripheral* peripheral = GetPeripheral(address);
+  if (peripheral != nullptr) {
+    return peripheral;
   }
   RemoveExpiredPeripherals();
   PeripheralInfo peripheral_info{
       .last_access_time = absl::Now(),
       .peripheral = std::make_unique<BleV2Peripheral>(address),
   };
-  BleV2Peripheral* peripheral = peripheral_info.peripheral.get();
-  if (!peripheral->Ok()) {
-    LOG(WARNING) << __func__ << "Invalid MAC address: " << address;
-    return nullptr;
-  }
-  LOG(INFO) << "New BLE peripheral with address: " << address;
+  peripheral = peripheral_info.peripheral.get();
+  VLOG(1) << "New BLE peripheral with address: " << address.ToString();
 
-  peripheral_map_[peripheral->GetUniqueId()] = std::move(peripheral_info);
+  peripheral_map_[address] = std::move(peripheral_info);
   return peripheral;
 }
 
-BleV2Peripheral* BleV2Medium::GetPeripheral(BleV2Peripheral::UniqueId id) {
-  auto it = peripheral_map_.find(id);
+BleV2Peripheral* BleV2Medium::GetPeripheral(MacAddress address) {
+  auto it = peripheral_map_.find(address);
   if (it == peripheral_map_.end()) {
     return nullptr;
   }

internal/platform/implementation/windows/ble_v2.h

@@ -23,9 +23,7 @@
 
 #include "absl/base/thread_annotations.h"
 #include "absl/container/flat_hash_map.h"
-#include "absl/strings/string_view.h"
 #include "absl/synchronization/mutex.h"
-#include "absl/synchronization/notification.h"
 #include "absl/time/clock.h"
 #include "absl/time/time.h"
 #include "internal/platform/cancellation_flag.h"
@@ -34,6 +32,7 @@
 #include "internal/platform/implementation/windows/ble_gatt_server.h"
 #include "internal/platform/implementation/windows/ble_v2_peripheral.h"
 #include "internal/platform/implementation/windows/bluetooth_adapter.h"
+#include "internal/platform/mac_address.h"
 #include "internal/platform/uuid.h"
 #include "winrt/Windows.Devices.Bluetooth.Advertisement.h"
 
@@ -141,11 +140,11 @@ class BleV2Medium : public api::ble_v2::BleMedium {
 
    private:
     absl::Mutex mutex_;
-    bool initialized_ ABSL_GUARDED_BY(mutex_)= false;
-    ::winrt::Windows::Devices::Bluetooth::Advertisement::
+    bool initialized_ ABSL_GUARDED_BY(mutex_) = false;
+    winrt::Windows::Devices::Bluetooth::Advertisement::
         BluetoothLEAdvertisementWatcher watcher_ ABSL_GUARDED_BY(mutex_);
-    ::winrt::event_token watcher_token_ ABSL_GUARDED_BY(mutex_);
-    ::winrt::event_token advertisement_received_token_ ABSL_GUARDED_BY(mutex_);
+    winrt::event_token watcher_token_ ABSL_GUARDED_BY(mutex_);
+    winrt::event_token advertisement_received_token_ ABSL_GUARDED_BY(mutex_);
   };
 
   bool StartBleAdvertising(
@@ -185,10 +184,10 @@ class BleV2Medium : public api::ble_v2::BleMedium {
 
   uint64_t GenerateSessionId() ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
   // Returns nullptr if `address` is invalid.
-  BleV2Peripheral* GetOrCreatePeripheral(absl::string_view address)
+  BleV2Peripheral* GetOrCreatePeripheral(MacAddress address)
       ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
-  // Returns nullptr if `id` does not match a known peripheral.
-  BleV2Peripheral* GetPeripheral(BleV2Peripheral::UniqueId id)
+  // Returns nullptr if `address` does not match a known peripheral.
+  BleV2Peripheral* GetPeripheral(MacAddress address)
       ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
 
   void RemoveExpiredPeripherals() ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
@@ -210,14 +209,14 @@ class BleV2Medium : public api::ble_v2::BleMedium {
       service_uuid_to_session_map_ ABSL_GUARDED_BY(mutex_);
 
   // WinRT objects
-  ::winrt::Windows::Devices::Bluetooth::Advertisement::
+  winrt::Windows::Devices::Bluetooth::Advertisement::
       BluetoothLEAdvertisementPublisher publisher_ ABSL_GUARDED_BY(mutex_) =
           nullptr;
 
   bool is_ble_publisher_started_ ABSL_GUARDED_BY(mutex_) = false;
   bool is_gatt_publisher_started_ ABSL_GUARDED_BY(mutex_) = false;
 
-  ::winrt::event_token publisher_token_ ABSL_GUARDED_BY(mutex_);
+  winrt::event_token publisher_token_ ABSL_GUARDED_BY(mutex_);
 
   BleGattServer* ble_gatt_server_ = nullptr;
   // Map to protect the pointer for BlePeripheral because
@@ -226,7 +225,7 @@ class BleV2Medium : public api::ble_v2::BleMedium {
     absl::Time last_access_time;
     std::unique_ptr<BleV2Peripheral> peripheral;
   };
-  absl::flat_hash_map<BleV2Peripheral::UniqueId, PeripheralInfo> peripheral_map_
+  absl::flat_hash_map<MacAddress, PeripheralInfo> peripheral_map_
       ABSL_GUARDED_BY(mutex_);
   absl::Time cleanup_time_ ABSL_GUARDED_BY(mutex_) = absl::Now();
   // Map of alternative BLE service UUID16s for a given Nearby service.

internal/platform/implementation/windows/ble_v2_peripheral.cc

@@ -15,19 +15,11 @@
 #include "internal/platform/implementation/windows/ble_v2_peripheral.h"
 #include <string>
 
-#include "absl/strings/string_view.h"
-#include "internal/platform/logging.h"
 #include "internal/platform/mac_address.h"
 
 namespace nearby {
 namespace windows {
 
-BleV2Peripheral::BleV2Peripheral(absl::string_view address) {
-  if (!MacAddress::FromString(address, mac_address_)) {
-    LOG(WARNING) << "Create BleV2Peripheral with invalid MAC: " << address;
-  }
-}
-
 std::string BleV2Peripheral::GetAddress() const {
   if (mac_address_.IsSet()) {
     return mac_address_.ToString();

internal/platform/implementation/windows/ble_v2_peripheral.h

@@ -16,8 +16,8 @@
 #define THIRD_PARTY_NEARBY_INTERNAL_PLATFORM_IMPLEMENTATION_WINDOWS_BLE_V2_PERIPHERAL_H_
 
 #include <string>
+#include <utility>
 
-#include "absl/strings/string_view.h"
 #include "internal/platform/implementation/ble_v2.h"
 #include "internal/platform/mac_address.h"
 
@@ -29,7 +29,8 @@ namespace windows {
 class BleV2Peripheral : public api::ble_v2::BlePeripheral {
  public:
   using UniqueId = api::ble_v2::BlePeripheral::UniqueId;
-  explicit BleV2Peripheral(absl::string_view address);
+  explicit BleV2Peripheral(MacAddress address)
+      : mac_address_(std::move(address)) {}
   ~BleV2Peripheral() override = default;
 
   // Returns the MAC address of the peripheral or empty string. The format is in
@@ -42,7 +43,7 @@ class BleV2Peripheral : public api::ble_v2::BlePeripheral {
   explicit operator bool() const { return Ok(); }
 
  private:
-  MacAddress mac_address_;
+  const MacAddress mac_address_;
 };
 
 }  // namespace windows

internal/platform/implementation/windows/ble_v2_peripheral_test.cc

@@ -16,6 +16,7 @@
 
 #include "gtest/gtest.h"
 #include "absl/strings/string_view.h"
+#include "internal/platform/mac_address.h"
 
 namespace nearby {
 namespace windows {
@@ -23,7 +24,9 @@ namespace {
 
 TEST(BleV2Peripheral, Constructor) {
   constexpr absl::string_view kAddress = "F1:F2:F3:F4:F5:F6";
-  BleV2Peripheral ble_peripheral(kAddress);
+  MacAddress address;
+  ASSERT_TRUE(MacAddress::FromString(kAddress, address));
+  BleV2Peripheral ble_peripheral(address);
 
   EXPECT_TRUE(ble_peripheral);
   EXPECT_TRUE(ble_peripheral.Ok());
@@ -32,7 +35,8 @@ TEST(BleV2Peripheral, Constructor) {
 }
 
 TEST(BleV2Peripheral, ConstructFromBadAddress) {
-  BleV2Peripheral ble_peripheral("G1:F2:F3:F4:F5:F6");
+  MacAddress address;
+  BleV2Peripheral ble_peripheral(address);
 
   EXPECT_FALSE(ble_peripheral);
   EXPECT_FALSE(ble_peripheral.Ok());

internal/platform/implementation/windows/bluetooth_adapter.cc

@@ -44,8 +44,8 @@
 #include "internal/platform/feature_flags.h"
 #include "internal/platform/implementation/platform.h"
 #include "internal/platform/implementation/windows/generated/winrt/Windows.Foundation.h"
-#include "internal/platform/implementation/windows/utils.h"
 #include "internal/platform/logging.h"
+#include "internal/platform/mac_address.h"
 
 typedef std::basic_string<TCHAR> tstring;
 
@@ -821,25 +821,32 @@ BluetoothAdapter::GetGenericBluetoothAdapterInstanceID() const {
 }
 
 // Returns BT MAC address assigned to this adapter.
-std::string BluetoothAdapter::GetMacAddress() const {
+MacAddress BluetoothAdapter::mac_address() const {
   if (windows_bluetooth_adapter_ == nullptr) {
     LOG(ERROR) << __func__ << ": No Bluetooth adapter on this device.";
-    return "";
+    return MacAddress();
   }
+  MacAddress mac_address;
   try {
-    return uint64_to_mac_address_string(
-        windows_bluetooth_adapter_.BluetoothAddress());
+    MacAddress::FromUint64(
+            windows_bluetooth_adapter_.BluetoothAddress(), mac_address);
   } catch (std::exception exception) {
     LOG(ERROR) << __func__ << ": exception:" << exception.what();
-    return "";
   } catch (const winrt::hresult_error &ex) {
     LOG(ERROR) << __func__ << ": exception:" << ex.code() << ": "
                << winrt::to_string(ex.message());
-    return "";
   } catch (...) {
     LOG(ERROR) << __func__ << ": unknown error.";
+  }
+  return mac_address;
+}
+
+std::string BluetoothAdapter::GetMacAddress() const {
+  MacAddress address = mac_address();
+  if (!address.IsSet()) {
     return "";
   }
+  return address.ToString();
 }
 
 std::string BluetoothAdapter::GetNameFromRegistry(PHKEY hKey) const {

internal/platform/implementation/windows/bluetooth_adapter.h

@@ -25,10 +25,12 @@
 #include <utility>
 
 #include "absl/functional/any_invocable.h"
+#include "absl/strings/string_view.h"
 #include "internal/platform/implementation/bluetooth_adapter.h"
 #include "internal/platform/implementation/windows/generated/winrt/Windows.Devices.Bluetooth.h"
 #include "internal/platform/implementation/windows/generated/winrt/Windows.Devices.Radios.h"
 #include "internal/platform/implementation/windows/generated/winrt/base.h"
+#include "internal/platform/mac_address.h"
 
 namespace nearby {
 namespace windows {
@@ -83,6 +85,8 @@ class BluetoothAdapter : public api::BluetoothAdapter {
   // Returns BT MAC address assigned to this adapter.
   std::string GetMacAddress() const override;
 
+  MacAddress mac_address() const;
+
   // Returns bluetooth device name from registry
   std::string GetNameFromRegistry(PHKEY hKey) const;
 

internal/platform/mac_address.h

@@ -46,6 +46,15 @@ class MacAddress {
   // Returns true if the MAC address is set.
   bool IsSet() const { return address_ != 0; }
 
+  // Hash function for absl containers.
+  template <typename H>
+  friend H AbslHashValue(H h, const MacAddress& addr) {
+    return H::combine(std::move(h), addr.address_);
+  }
+
+  friend auto operator<=>(const MacAddress& lhs,
+                          const MacAddress& rhs) = default;
+
  private:
   uint64_t address_ = 0;
 };

internal/platform/mac_address_test.cc

@@ -15,6 +15,7 @@
 #include "internal/platform/mac_address.h"
 
 #include "gtest/gtest.h"
+#include "absl/hash/hash_testing.h"
 
 namespace nearby {
 namespace {
@@ -62,11 +63,19 @@ TEST(MacAddressTest, IsSetTrue) {
   EXPECT_TRUE(mac_address.IsSet());
 }
 
-
 TEST(MacAddressTest, IsSetFalse) {
   MacAddress mac_address;
   EXPECT_FALSE(mac_address.IsSet());
 }
 
+TEST(MacAddressTest, Hash) {
+  MacAddress addr1;
+  EXPECT_TRUE(MacAddress::FromUint64(0x00B0D063C226, addr1));
+  EXPECT_TRUE(absl::VerifyTypeImplementsAbslHashCorrectly({
+      MacAddress(),
+      addr1,
+  }));
+}
+
 }  // namespace
 }  // namespace nearby