From eff0e2d3c66930927f2f0c7bfdcfaaae0dcee7a7 Mon Sep 17 00:00:00 2001 From: Anay Wadhera Date: Mon, 10 Jul 2023 11:34:27 -0700 Subject: [PATCH] Commonize callback construction for discovery. PiperOrigin-RevId: 546934102 --- .../implementation/p2p_cluster_pcp_handler.cc | 107 +++++++++--------- .../implementation/p2p_cluster_pcp_handler.h | 13 +-- .../p2p_cluster_pcp_handler_test.cc | 84 ++++++++++++++ internal/platform/bluetooth_classic.cc | 3 + 4 files changed, 143 insertions(+), 64 deletions(-) diff --git a/connections/implementation/p2p_cluster_pcp_handler.cc b/connections/implementation/p2p_cluster_pcp_handler.cc index e9779d0b..7ca3014e 100644 --- a/connections/implementation/p2p_cluster_pcp_handler.cc +++ b/connections/implementation/p2p_cluster_pcp_handler.cc @@ -946,16 +946,7 @@ BasePcpHandler::StartOperationResult P2pClusterPcpHandler::StartDiscoveryImpl( if (discovery_options.allowed.wifi_lan) { location::nearby::proto::connections::Medium wifi_lan_medium = - StartWifiLanDiscovery( - { - .service_discovered_cb = absl::bind_front( - &P2pClusterPcpHandler::WifiLanServiceDiscoveredHandler, - this, client), - .service_lost_cb = absl::bind_front( - &P2pClusterPcpHandler::WifiLanServiceLostHandler, this, - client), - }, - client, service_id); + StartWifiLanDiscovery(client, service_id); if (wifi_lan_medium != location::nearby::proto::connections::UNKNOWN_MEDIUM) { NEARBY_LOGS(INFO) @@ -967,17 +958,6 @@ BasePcpHandler::StartOperationResult P2pClusterPcpHandler::StartDiscoveryImpl( if (discovery_options.allowed.bluetooth) { location::nearby::proto::connections::Medium bluetooth_medium = StartBluetoothDiscovery( - { - .device_discovered_cb = absl::bind_front( - &P2pClusterPcpHandler::BluetoothDeviceDiscoveredHandler, - this, client, service_id), - .device_name_changed_cb = absl::bind_front( - &P2pClusterPcpHandler::BluetoothNameChangedHandler, this, - client, service_id), - .device_lost_cb = absl::bind_front( - &P2pClusterPcpHandler::BluetoothDeviceLostHandler, this, - client, service_id), - }, client, service_id); if (bluetooth_medium != location::nearby::proto::connections::UNKNOWN_MEDIUM) { @@ -992,14 +972,6 @@ BasePcpHandler::StartOperationResult P2pClusterPcpHandler::StartDiscoveryImpl( config_package_nearby::nearby_connections_feature::kEnableBleV2)) { location::nearby::proto::connections::Medium ble_v2_medium = StartBleV2Scanning( - { - .peripheral_discovered_cb = absl::bind_front( - &P2pClusterPcpHandler::BleV2PeripheralDiscoveredHandler, - this, client), - .peripheral_lost_cb = absl::bind_front( - &P2pClusterPcpHandler::BleV2PeripheralLostHandler, this, - client), - }, client, service_id, discovery_options); if (ble_v2_medium != location::nearby::proto::connections::UNKNOWN_MEDIUM) { @@ -1009,17 +981,8 @@ BasePcpHandler::StartOperationResult P2pClusterPcpHandler::StartDiscoveryImpl( } } else { location::nearby::proto::connections::Medium ble_medium = - StartBleScanning( - { - .peripheral_discovered_cb = absl::bind_front( - &P2pClusterPcpHandler::BlePeripheralDiscoveredHandler, - this, client), - .peripheral_lost_cb = absl::bind_front( - &P2pClusterPcpHandler::BlePeripheralLostHandler, this, - client), - }, - client, service_id, - discovery_options.fast_advertisement_service_uuid); + StartBleScanning(client, service_id, + discovery_options.fast_advertisement_service_uuid); if (ble_medium != location::nearby::proto::connections::UNKNOWN_MEDIUM) { NEARBY_LOGS(INFO) << "P2pClusterPcpHandler::StartDiscoveryImpl: Ble added."; @@ -1498,11 +1461,20 @@ P2pClusterPcpHandler::StartBluetoothAdvertising( } location::nearby::proto::connections::Medium -P2pClusterPcpHandler::StartBluetoothDiscovery( - BluetoothDiscoveredDeviceCallback callback, ClientProxy* client, - const std::string& service_id) { +P2pClusterPcpHandler::StartBluetoothDiscovery(ClientProxy* client, + const std::string& service_id) { if (bluetooth_radio_.Enable() && - bluetooth_medium_.StartDiscovery(std::move(callback))) { + bluetooth_medium_.StartDiscovery({ + .device_discovered_cb = absl::bind_front( + &P2pClusterPcpHandler::BluetoothDeviceDiscoveredHandler, this, + client, service_id), + .device_name_changed_cb = absl::bind_front( + &P2pClusterPcpHandler::BluetoothNameChangedHandler, this, client, + service_id), + .device_lost_cb = absl::bind_front( + &P2pClusterPcpHandler::BluetoothDeviceLostHandler, this, client, + service_id), + })) { NEARBY_LOGS(INFO) << "In StartBluetoothDiscovery(), client=" << client->GetClientId() << " started scanning for Bluetooth for service_id=" @@ -1714,12 +1686,19 @@ P2pClusterPcpHandler::StartBleAdvertising( location::nearby::proto::connections::Medium P2pClusterPcpHandler::StartBleScanning( - BleDiscoveredPeripheralCallback callback, ClientProxy* client, - const std::string& service_id, + ClientProxy* client, const std::string& service_id, const std::string& fast_advertisement_service_uuid) { if (bluetooth_radio_.Enable() && - ble_medium_.StartScanning(service_id, fast_advertisement_service_uuid, - std::move(callback))) { + ble_medium_.StartScanning( + service_id, fast_advertisement_service_uuid, + { + .peripheral_discovered_cb = absl::bind_front( + &P2pClusterPcpHandler::BlePeripheralDiscoveredHandler, this, + client), + .peripheral_lost_cb = absl::bind_front( + &P2pClusterPcpHandler::BlePeripheralLostHandler, this, + client), + })) { NEARBY_LOGS(INFO) << "In StartBleScanning(), client=" << client->GetClientId() << " started scanning for BLE advertisements for service_id=" @@ -1923,13 +1902,21 @@ P2pClusterPcpHandler::StartBleV2Advertising( location::nearby::proto::connections::Medium P2pClusterPcpHandler::StartBleV2Scanning( - BleV2DiscoveredPeripheralCallback callback, ClientProxy* client, - const std::string& service_id, const DiscoveryOptions& discovery_options) { + ClientProxy* client, const std::string& service_id, + const DiscoveryOptions& discovery_options) { PowerLevel power_level = discovery_options.low_power ? PowerLevel::kLowPower : PowerLevel::kHighPower; if (bluetooth_radio_.Enable() && - ble_v2_medium_.StartScanning(service_id, power_level, - std::move(callback))) { + ble_v2_medium_.StartScanning( + service_id, power_level, + { + .peripheral_discovered_cb = absl::bind_front( + &P2pClusterPcpHandler::BleV2PeripheralDiscoveredHandler, this, + client), + .peripheral_lost_cb = absl::bind_front( + &P2pClusterPcpHandler::BleV2PeripheralLostHandler, this, + client), + })) { NEARBY_LOGS(INFO) << "In StartBleScanning(), client=" << client->GetClientId() << " started scanning for BLE advertisements for service_id=" @@ -2086,10 +2073,18 @@ P2pClusterPcpHandler::StartWifiLanAdvertising( } location::nearby::proto::connections::Medium -P2pClusterPcpHandler::StartWifiLanDiscovery( - WifiLanDiscoveredServiceCallback callback, ClientProxy* client, - const std::string& service_id) { - if (wifi_lan_medium_.StartDiscovery(service_id, std::move(callback))) { +P2pClusterPcpHandler::StartWifiLanDiscovery(ClientProxy* client, + const std::string& service_id) { + if (wifi_lan_medium_.StartDiscovery( + service_id, + { + .service_discovered_cb = absl::bind_front( + &P2pClusterPcpHandler::WifiLanServiceDiscoveredHandler, this, + client), + .service_lost_cb = absl::bind_front( + &P2pClusterPcpHandler::WifiLanServiceLostHandler, this, + client), + })) { NEARBY_LOGS(INFO) << "In StartWifiLanDiscovery(), client=" << client->GetClientId() << " started scanning for Wifi devices for service_id=" diff --git a/connections/implementation/p2p_cluster_pcp_handler.h b/connections/implementation/p2p_cluster_pcp_handler.h index 93ae269b..16946610 100644 --- a/connections/implementation/p2p_cluster_pcp_handler.h +++ b/connections/implementation/p2p_cluster_pcp_handler.h @@ -181,8 +181,7 @@ class P2pClusterPcpHandler : public BasePcpHandler { const ByteArray& service_id_hash, const std::string& local_endpoint_id, const ByteArray& local_endpoint_info, WebRtcState web_rtc_state); location::nearby::proto::connections::Medium StartBluetoothDiscovery( - BluetoothDiscoveredDeviceCallback callback, ClientProxy* client, - const std::string& service_id); + ClientProxy* client, const std::string& service_id); BasePcpHandler::ConnectImplResult BluetoothConnectImpl( ClientProxy* client, BluetoothEndpoint* endpoint); @@ -207,8 +206,7 @@ class P2pClusterPcpHandler : public BasePcpHandler { const ByteArray& local_endpoint_info, const AdvertisingOptions& advertising_options, WebRtcState web_rtc_state); location::nearby::proto::connections::Medium StartBleScanning( - BleDiscoveredPeripheralCallback callback, ClientProxy* client, - const std::string& service_id, + ClientProxy* client, const std::string& service_id, const std::string& fast_advertisement_service_uuid); BasePcpHandler::ConnectImplResult BleConnectImpl(ClientProxy* client, BleEndpoint* endpoint); @@ -237,8 +235,8 @@ class P2pClusterPcpHandler : public BasePcpHandler { const ByteArray& local_endpoint_info, const AdvertisingOptions& advertising_options, WebRtcState web_rtc_state); location::nearby::proto::connections::Medium StartBleV2Scanning( - BleV2DiscoveredPeripheralCallback callback, ClientProxy* client, - const std::string& service_id, const DiscoveryOptions& discovery_options); + ClientProxy* client, const std::string& service_id, + const DiscoveryOptions& discovery_options); BasePcpHandler::ConnectImplResult BleV2ConnectImpl(ClientProxy* client, BleV2Endpoint* endpoint); @@ -263,8 +261,7 @@ class P2pClusterPcpHandler : public BasePcpHandler { const std::string& local_endpoint_id, const ByteArray& local_endpoint_info, WebRtcState web_rtc_state); location::nearby::proto::connections::Medium StartWifiLanDiscovery( - WifiLanDiscoveredServiceCallback callback, ClientProxy* client, - const std::string& service_id); + ClientProxy* client, const std::string& service_id); BasePcpHandler::ConnectImplResult WifiLanConnectImpl( ClientProxy* client, WifiLanEndpoint* endpoint); diff --git a/connections/implementation/p2p_cluster_pcp_handler_test.cc b/connections/implementation/p2p_cluster_pcp_handler_test.cc index f2329153..6cc0e58f 100644 --- a/connections/implementation/p2p_cluster_pcp_handler_test.cc +++ b/connections/implementation/p2p_cluster_pcp_handler_test.cc @@ -21,6 +21,8 @@ #include "protobuf-matchers/protocol-buffer-matchers.h" #include "gtest/gtest.h" #include "absl/time/time.h" +#include "connections/advertising_options.h" +#include "connections/implementation/bluetooth_device_name.h" #include "connections/implementation/bwu_manager.h" #include "connections/implementation/flags/nearby_connections_feature_flags.h" #include "connections/implementation/injected_bluetooth_device_store.h" @@ -333,6 +335,88 @@ TEST_P(P2pClusterPcpHandlerTest, CanDiscover) { env_.Stop(); } +TEST_P(P2pClusterPcpHandlerTest, CanBluetoothDiscoverChangeName) { + env_.Start(); + std::string endpoint_name{"endpoint_name"}; + Mediums mediums_a; + Mediums mediums_b; + EndpointChannelManager ecm_a; + EndpointChannelManager ecm_b; + EndpointManager em_a(&ecm_a); + EndpointManager em_b(&ecm_b); + BwuManager bwu_a(mediums_a, em_a, ecm_a, {}, {}); + BwuManager bwu_b(mediums_b, em_b, ecm_b, {}, {}); + InjectedBluetoothDeviceStore ibds_a; + InjectedBluetoothDeviceStore ibds_b; + P2pClusterPcpHandler handler_a(&mediums_a, &em_a, &ecm_a, &bwu_a, ibds_a); + P2pClusterPcpHandler handler_b(&mediums_b, &em_b, &ecm_b, &bwu_b, ibds_b); + // For the first time we find the device with the old name. + CountDownLatch first_found_latch(1); + // For the second time we "find" the device with the new name. + CountDownLatch second_found_latch(1); + // For when the name is changed. + CountDownLatch lost_latch(1); + AdvertisingOptions advertising_options = { + { + Strategy::kP2pCluster, + BooleanMediumSelector{ + .bluetooth = true, + }, + }, + }; + + DiscoveryOptions discovery_options = { + { + Strategy::kP2pCluster, + BooleanMediumSelector{ + .bluetooth = true, + }, + }, + }; + bool first = false; + EXPECT_EQ( + handler_a.StartAdvertising(&client_a_, service_id_, advertising_options, + {.endpoint_info = ByteArray{endpoint_name}}), + Status{Status::kSuccess}); + EXPECT_EQ(handler_b.StartDiscovery( + &client_b_, service_id_, discovery_options, + { + .endpoint_found_cb = + [&](const std::string& endpoint_id, + const ByteArray& endpoint_info, + const std::string& service_id) { + NEARBY_LOG(INFO, "Device discovered: id=%s", + endpoint_id.c_str()); + if (!first) { + first_found_latch.CountDown(); + first = true; + } else { + second_found_latch.CountDown(); + } + }, + .endpoint_lost_cb = + [&](const std::string& id) { + NEARBY_LOG(INFO, "Device lost: id=%s", id.c_str()); + lost_latch.CountDown(); + }, + }), + Status{Status::kSuccess}); + ASSERT_TRUE(mediums_a.GetBluetoothRadio().IsAdapterValid()); + BluetoothDeviceName name( + mediums_a.GetBluetoothRadio().GetBluetoothAdapter().GetName()); + BluetoothDeviceName new_name(name.GetVersion(), name.GetPcp(), + name.GetEndpointId(), name.GetServiceIdHash(), + ByteArray("BT Device A"), name.GetUwbAddress(), + name.GetWebRtcState()); + EXPECT_TRUE(first_found_latch.Await().Ok()); + mediums_a.GetBluetoothRadio().GetBluetoothAdapter().SetName( + std::string(new_name)); + EXPECT_TRUE(second_found_latch.Await().Ok()); + EXPECT_TRUE(lost_latch.Await().Ok()); + handler_b.StopDiscovery(&client_b_); + env_.Stop(); +} + TEST_P(P2pClusterPcpHandlerTest, CanConnect) { env_.Start(); std::string endpoint_name_a{"endpoint_name"}; diff --git a/internal/platform/bluetooth_classic.cc b/internal/platform/bluetooth_classic.cc index 1ab855b0..df8b1157 100644 --- a/internal/platform/bluetooth_classic.cc +++ b/internal/platform/bluetooth_classic.cc @@ -63,6 +63,9 @@ bool BluetoothClassicMedium::StartDiscovery(DiscoveryCallback callback) { .device_name_changed_cb = [this](api::BluetoothDevice& device) { MutexLock lock(&mutex_); + // If the device is not already in devices_, we should not be able + // to change its name. + if (devices_.find(&device) == devices_.end()) return; auto& context = *devices_[&device]; NEARBY_LOG(INFO, "Renaming device=%p, impl=%p", &context.device, &device);