diff --git a/src/connectivity/bluetooth/core/bt-host/gap/adapter.cc b/src/connectivity/bluetooth/core/bt-host/gap/adapter.cc index 3c9450311ae0cc5465c44e3797ce1e4fade4f8ba..5bfb04af0acb6b9c85a2955416e65d062fb2d122 100644 --- a/src/connectivity/bluetooth/core/bt-host/gap/adapter.cc +++ b/src/connectivity/bluetooth/core/bt-host/gap/adapter.cc @@ -548,7 +548,7 @@ void Adapter::InitializeStep4(InitializeCallback callback) { // Initialize the LE manager objects le_discovery_manager_ = std::make_unique<LowEnergyDiscoveryManager>( hci_, hci_le_scanner_.get(), &peer_cache_); - le_discovery_manager_->set_directed_connectable_callback( + le_discovery_manager_->set_bonded_peer_connectable_callback( fit::bind_member(this, &Adapter::OnLeAutoConnectRequest)); le_connection_manager_ = std::make_unique<LowEnergyConnectionManager>( hci_, le_address_manager_.get(), hci_le_connector_.get(), &peer_cache_, @@ -689,9 +689,12 @@ void Adapter::OnTransportClosed() { void Adapter::OnLeAutoConnectRequest(PeerId peer_id) { ZX_DEBUG_ASSERT(le_connection_manager_); + + // TODO(BT-888): We shouldn't always accept connection requests from all + // bonded peripherals (e.g. if one is explicitly disconnected). Maybe add an + // "auto_connect()" property to Peer? auto self = weak_ptr_factory_.GetWeakPtr(); le_connection_manager_->Connect(peer_id, [self](auto status, auto conn) { - PeerId id = conn->peer_identifier(); if (!self) { bt_log(INFO, "gap", "ignoring auto-connection (adapter destroyed)"); return; @@ -701,6 +704,8 @@ void Adapter::OnLeAutoConnectRequest(PeerId peer_id) { return; } + ZX_DEBUG_ASSERT(conn); + PeerId id = conn->peer_identifier(); bt_log(INFO, "gap", "peer auto-connected (id: %s)", bt_str(id)); if (self->auto_conn_cb_) { self->auto_conn_cb_(std::move(conn)); diff --git a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_discovery_manager.cc b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_discovery_manager.cc index d27b955683f1322ea4400fd1637ef02c6a318fc8..ddbcd79286fa9aef1d5a2492f382e16745ce29a5 100644 --- a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_discovery_manager.cc +++ b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_discovery_manager.cc @@ -180,12 +180,19 @@ void LowEnergyDiscoveryManager::OnPeerFound( const hci::LowEnergyScanResult& result, const ByteBuffer& data) { ZX_DEBUG_ASSERT(thread_checker_.IsCreationThreadCurrent()); - // Ignore regular scan results during a passive scan. + auto peer = peer_cache_->FindByAddress(result.address); + if (peer && peer->bonded() && bonded_conn_cb_) { + bt_log(SPEW, "gap-le", "found connectable bonded peer (id: %s)", + bt_str(peer->identifier())); + bonded_conn_cb_(peer->identifier()); + } + + // Do not process further during a passive scan. if (scanner_->IsPassiveScanning()) { return; } - auto peer = peer_cache_->FindByAddress(result.address); + // Create a new entry if we found the device during general discovery. if (!peer) { peer = peer_cache_->NewPeer(result.address, result.connectable); } @@ -204,30 +211,27 @@ void LowEnergyDiscoveryManager::OnDirectedAdvertisement( // TODO(NET-1572): Resolve the address in the host if it is random and // |result.resolved| is false. - bt_log(SPEW, "gap", "Received directed advertisement (address: %s, %s)", + bt_log(SPEW, "gap-le", "Received directed advertisement (address: %s, %s)", result.address.ToString().c_str(), (result.resolved ? "resolved" : "not resolved")); auto peer = peer_cache_->FindByAddress(result.address); if (!peer) { - bt_log(TRACE, "gap", + bt_log(TRACE, "gap-le", "ignoring connection request from unknown peripheral: %s", result.address.ToString().c_str()); return; } if (!peer->le() || !peer->le()->bonded()) { - bt_log(TRACE, "gap", + bt_log(TRACE, "gap-le", "rejecting connection request from unbonded peripheral: %s", result.address.ToString().c_str()); return; } - // TODO(armansito): We shouldn't always accept connection requests from all - // bonded peripherals (e.g. if one is explicitly disconnected). Maybe add an - // "auto_connect()" property to Peer? - if (directed_conn_cb_) { - directed_conn_cb_(peer->identifier()); + if (bonded_conn_cb_) { + bonded_conn_cb_(peer->identifier()); } } diff --git a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_discovery_manager.h b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_discovery_manager.h index 56424dc990d9b183186df0eacac5f855569d5f24..2df23f00f6830b73b640685ab3f06234297c156d 100644 --- a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_discovery_manager.h +++ b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_discovery_manager.h @@ -199,11 +199,15 @@ class LowEnergyDiscoveryManager final : public hci::LowEnergyScanner::Delegate { // Returns whether there is an active discovery session. bool discovering() const { return !sessions_.empty(); } - // Registers a callback which runs after a directed connectable advertisement - // is received from a bonded peer with the given |id|. - using DirectedConnectableCallback = fit::function<void(PeerId id)>; - void set_directed_connectable_callback(DirectedConnectableCallback callback) { - directed_conn_cb_ = std::move(callback); + // Registers a callback which runs when a connectable advertisement is + // received from a bonded peer. + // + // Note: this callback can be triggered during a background scan as well as + // general discovery. + using BondedPeerConnectableCallback = fit::function<void(PeerId id)>; + void set_bonded_peer_connectable_callback( + BondedPeerConnectableCallback callback) { + bonded_conn_cb_ = std::move(callback); } private: @@ -251,7 +255,7 @@ class LowEnergyDiscoveryManager final : public hci::LowEnergyScanner::Delegate { // Called when a directed connectable advertisement is received during an // active or passive scan. - DirectedConnectableCallback directed_conn_cb_; + BondedPeerConnectableCallback bonded_conn_cb_; // The list of currently pending calls to start discovery. std::queue<SessionCallback> pending_; diff --git a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_discovery_manager_unittest.cc b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_discovery_manager_unittest.cc index a2cb13284821712a97c1f3229137205202888761..58a4930dc43068395d4522ccf5891796ffd22566 100644 --- a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_discovery_manager_unittest.cc +++ b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_discovery_manager_unittest.cc @@ -28,14 +28,18 @@ using bt::testing::FakePeer; using TestingBase = bt::testing::FakeControllerTest<FakeController>; const DeviceAddress kAddress0(DeviceAddress::Type::kLEPublic, - "00:00:00:00:00:01"); + "00:00:00:00:00:00"); const DeviceAddress kAddrAlias0(DeviceAddress::Type::kBREDR, kAddress0.value()); const DeviceAddress kAddress1(DeviceAddress::Type::kLERandom, - "00:00:00:00:00:02"); + "00:00:00:00:00:01"); const DeviceAddress kAddress2(DeviceAddress::Type::kLEPublic, - "00:00:00:00:00:03"); + "00:00:00:00:00:02"); const DeviceAddress kAddress3(DeviceAddress::Type::kLEPublic, + "00:00:00:00:00:03"); +const DeviceAddress kAddress4(DeviceAddress::Type::kLEPublic, "00:00:00:00:00:04"); +const DeviceAddress kAddress5(DeviceAddress::Type::kLEPublic, + "00:00:00:00:00:05"); constexpr zx::duration kTestScanPeriod = zx::sec(10); @@ -811,7 +815,7 @@ TEST_F(GAP_LowEnergyDiscoveryManagerTest, DirectedConnectableEvent) { test_device()->AddPeer(std::move(fake_peer)); int count = 0; - discovery_manager()->set_directed_connectable_callback( + discovery_manager()->set_bonded_peer_connectable_callback( [&](const auto&) { count++; }); discovery_manager()->set_scan_period(kTestScanPeriod); @@ -1018,37 +1022,43 @@ TEST_F(GAP_LowEnergyDiscoveryManagerTest, } TEST_F(GAP_LowEnergyDiscoveryManagerTest, - BackgroundScanOnlyHandlesDirectedEventsFromBondedPeers) { - PeerId kBondedPeerId(1); + BackgroundScanOnlyHandlesEventsFromBondedDevices) { + PeerId kBondedPeerId1(1); + PeerId kBondedPeerId2(2); + + // kAddress0 and kAddress1 are in undirected connectable mode. AddFakePeers(); - // Add a bonded peer. - auto fake_peer = std::make_unique<FakePeer>(kAddress0, true, false); + // Add two devices that are in directed connectable mode. + auto fake_peer = std::make_unique<FakePeer>(kAddress4, true, false); fake_peer->enable_directed_advertising(true); test_device()->AddPeer(std::move(fake_peer)); - sm::PairingData pdata; - pdata.ltk = sm::LTK(); - peer_cache()->AddBondedPeer(kBondedPeerId, kAddress0, pdata, {}); - EXPECT_EQ(1u, peer_cache()->count()); - - // Add a second peer the sends directed advertisements but do not mark it as - // bonded. Advertisements from this peer should be ignored. - fake_peer = std::make_unique<FakePeer>(kAddress1, true, false); + fake_peer = std::make_unique<FakePeer>(kAddress5, true, false); fake_peer->enable_directed_advertising(true); test_device()->AddPeer(std::move(fake_peer)); + // Mark one directed and one undirected connectable device as bonded. We + // expect advertisements from all other devices to get ignored. + sm::PairingData pdata; + pdata.ltk = sm::LTK(); + peer_cache()->AddBondedPeer(kBondedPeerId1, kAddress0, pdata, {}); + peer_cache()->AddBondedPeer(kBondedPeerId2, kAddress4, pdata, {}); + EXPECT_EQ(2u, peer_cache()->count()); + int count = 0; - discovery_manager()->set_directed_connectable_callback([&](const auto& id) { - count++; - EXPECT_EQ(kBondedPeerId, id); - }); + discovery_manager()->set_bonded_peer_connectable_callback( + [&](const auto& id) { + count++; + EXPECT_TRUE(id == kBondedPeerId1 || id == kBondedPeerId2) + << id.ToString(); + }); discovery_manager()->EnableBackgroundScan(true); RunLoopUntilIdle(); - EXPECT_EQ(1, count); + EXPECT_EQ(2, count); // No new remote peer cache entries should have been created. - EXPECT_EQ(1u, peer_cache()->count()); + EXPECT_EQ(2u, peer_cache()->count()); } TEST_F(GAP_LowEnergyDiscoveryManagerTest, BackgroundScanPeriodRestart) { diff --git a/src/connectivity/bluetooth/core/bt-host/hci/legacy_low_energy_scanner.cc b/src/connectivity/bluetooth/core/bt-host/hci/legacy_low_energy_scanner.cc index 728418b5bd63bd289dcd8a09489ea7ae56e25620..1496c61b86f81fe1600882b72effd3e2caf1ab4b 100644 --- a/src/connectivity/bluetooth/core/bt-host/hci/legacy_low_energy_scanner.cc +++ b/src/connectivity/bluetooth/core/bt-host/hci/legacy_low_energy_scanner.cc @@ -263,6 +263,8 @@ void LegacyLowEnergyScanner::StopScanInternal(bool stopped) { void LegacyLowEnergyScanner::OnAdvertisingReportEvent( const EventPacket& event) { + bt_log(DEBUG, "hci-le", "received advertising report"); + // Drop the event if not requested to scan. if (!IsScanning()) return; diff --git a/src/connectivity/bluetooth/core/bt-host/testing/fake_controller.cc b/src/connectivity/bluetooth/core/bt-host/testing/fake_controller.cc index 77568c8eedad1fdcdec7be722db73b552c90a430..8747a8435ea1da38180d4b42c900cf69804d5e01 100644 --- a/src/connectivity/bluetooth/core/bt-host/testing/fake_controller.cc +++ b/src/connectivity/bluetooth/core/bt-host/testing/fake_controller.cc @@ -195,6 +195,12 @@ void FakeController::ClearDefaultResponseStatus(hci::OpCode opcode) { } void FakeController::AddPeer(std::unique_ptr<FakePeer> peer) { + // Prevent multiple entries with the same address. + ZX_DEBUG_ASSERT( + std::find_if(peers_.begin(), peers_.end(), [&](const auto& p) { + return p->address() == peer->address(); + }) == peers_.end()); + peer->set_ctrl(this); peers_.push_back(std::move(peer)); }