diff --git a/pw_bluetooth_sapphire/host/gap/adapter_test.cc b/pw_bluetooth_sapphire/host/gap/adapter_test.cc index cdd008f5a..ec9427827 100644 --- a/pw_bluetooth_sapphire/host/gap/adapter_test.cc +++ b/pw_bluetooth_sapphire/host/gap/adapter_test.cc @@ -866,9 +866,11 @@ TEST_F(AdapterTest, LocalAddressForConnections) { EXPECT_EQ(pw::bluetooth::emboss::LEOwnAddressType::PUBLIC, test_device()->le_connect_params()->own_address_type); - // Create a new connection. The second attempt should use a random address. - // re-enabled. + // Disconnect conn_ref = nullptr; + + // Create a new connection. The LowEnergyConnectionManager should try to + // update the local address before creating the connection. adapter()->le()->Connect( peer->identifier(), connect_cb, LowEnergyConnectionOptions()); RunUntilIdle(); diff --git a/pw_bluetooth_sapphire/host/gap/low_energy_connection_manager.cc b/pw_bluetooth_sapphire/host/gap/low_energy_connection_manager.cc index 80d8458c6..a74926f2f 100644 --- a/pw_bluetooth_sapphire/host/gap/low_energy_connection_manager.cc +++ b/pw_bluetooth_sapphire/host/gap/low_energy_connection_manager.cc @@ -436,7 +436,8 @@ void LowEnergyConnectionManager::RegisterRemoteInitiatedLink( l2cap_, gatt_, adapter_state_, - dispatcher_); + dispatcher_, + local_address_delegate_); auto [conn_iter, _] = remote_connectors_.emplace( peer_id, RequestAndConnector{std::move(request), std::move(connector)}); // Wait until the connector is in the map to start in case the result callback @@ -575,7 +576,8 @@ void LowEnergyConnectionManager::TryCreateNextConnection() { l2cap_, gatt_, adapter_state_, - dispatcher_); + dispatcher_, + local_address_delegate_); connector->AttachInspect(inspect_node_, kInspectOutboundConnectorNodeName); diff --git a/pw_bluetooth_sapphire/host/gap/low_energy_connection_manager_test.cc b/pw_bluetooth_sapphire/host/gap/low_energy_connection_manager_test.cc index 12733b5a1..1bf310d11 100644 --- a/pw_bluetooth_sapphire/host/gap/low_energy_connection_manager_test.cc +++ b/pw_bluetooth_sapphire/host/gap/low_energy_connection_manager_test.cc @@ -3349,70 +3349,6 @@ TEST_F(LowEnergyConnectionManagerTest, AutoConnectSkipsScanning) { EXPECT_EQ(scan_cb_count, 0u); } -TEST_F(LowEnergyConnectionManagerTest, ConnectSinglePeerStartDiscoveryFailed) { - auto* peer = peer_cache()->NewPeer(kAddress0, /*connectable=*/true); - EXPECT_TRUE(peer->temporary()); - - auto fake_peer = std::make_unique(kAddress0, dispatcher()); - test_device()->AddPeer(std::move(fake_peer)); - - size_t connect_cb_count = 0; - auto callback = [&connect_cb_count](auto result) { - EXPECT_TRUE(result.is_error()); - connect_cb_count++; - }; - - // Cause discovery to fail. - test_device()->SetDefaultCommandStatus( - hci_spec::kLESetScanEnable, - pw::bluetooth::emboss::StatusCode::COMMAND_DISALLOWED); - - EXPECT_TRUE(connected_peers().empty()); - conn_mgr()->Connect(peer->identifier(), callback, kConnectionOptions); - ASSERT_TRUE(peer->le()); - EXPECT_EQ(Peer::ConnectionState::kInitializing, - peer->le()->connection_state()); - - RunUntilIdle(); - EXPECT_EQ(connect_cb_count, 1u); - EXPECT_FALSE(peer->temporary()); - EXPECT_EQ(Peer::ConnectionState::kNotConnected, - peer->le()->connection_state()); -} - -TEST_F(LowEnergyConnectionManagerTest, - ConnectSinglePeerDiscoveryFailedDuringScan) { - auto* peer = peer_cache()->NewPeer(kAddress0, /*connectable=*/true); - EXPECT_TRUE(peer->temporary()); - - // Don't add peer to FakeController to prevent scan from completing. - - size_t connect_cb_count = 0; - auto callback = [&connect_cb_count](auto result) { - EXPECT_TRUE(result.is_error()); - connect_cb_count++; - }; - - EXPECT_TRUE(connected_peers().empty()); - conn_mgr()->Connect(peer->identifier(), callback, kConnectionOptions); - ASSERT_TRUE(peer->le()); - EXPECT_EQ(Peer::ConnectionState::kInitializing, - peer->le()->connection_state()); - RunUntilIdle(); - EXPECT_EQ(connect_cb_count, 0u); - - // Cause discovery to fail when attempting to restart scan after scan period - // ends. - test_device()->SetDefaultCommandStatus( - hci_spec::kLESetScanEnable, - pw::bluetooth::emboss::StatusCode::COMMAND_DISALLOWED); - RunFor(kLEGeneralDiscoveryScanMin); - EXPECT_EQ(connect_cb_count, 1u); - EXPECT_FALSE(peer->temporary()); - EXPECT_EQ(Peer::ConnectionState::kNotConnected, - peer->le()->connection_state()); -} - TEST_F(LowEnergyConnectionManagerTest, PeerDisconnectBeforeInterrogationCompletes) { auto* peer = peer_cache()->NewPeer(kAddress0, /*connectable=*/true); @@ -3924,7 +3860,7 @@ TEST_F(LowEnergyConnectionManagerTest, Inspect) { StringIs("peer_id", peer->identifier().ToString()), IntIs("connection_attempt", 0), BoolIs("is_outbound", true), - StringIs("state", "StartingScanning")))))); + StringIs("state", "Connecting")))))); auto empty_connections_matcher = AllOf(NodeMatches(NameMatches("connections")), diff --git a/pw_bluetooth_sapphire/host/gap/low_energy_connector.cc b/pw_bluetooth_sapphire/host/gap/low_energy_connector.cc index 7d9eb3736..6a62b18eb 100644 --- a/pw_bluetooth_sapphire/host/gap/low_energy_connector.cc +++ b/pw_bluetooth_sapphire/host/gap/low_energy_connector.cc @@ -52,7 +52,8 @@ LowEnergyConnector::LowEnergyConnector( l2cap::ChannelManager* l2cap, gatt::GATT::WeakPtr gatt, const AdapterState& adapter_state, - pw::async::Dispatcher& dispatcher) + pw::async::Dispatcher& dispatcher, + hci::LocalAddressDelegate* local_address_delegate) : dispatcher_(dispatcher), peer_id_(peer_id), peer_cache_(peer_cache), @@ -61,7 +62,8 @@ LowEnergyConnector::LowEnergyConnector( adapter_state_(adapter_state), options_(options), hci_(std::move(hci)), - le_connection_manager_(std::move(conn_mgr)) { + le_connection_manager_(std::move(conn_mgr)), + local_address_delegate_(local_address_delegate) { PW_CHECK(peer_cache_); PW_CHECK(l2cap_); PW_CHECK(gatt_.is_alive()); @@ -116,11 +118,7 @@ void LowEnergyConnector::StartOutbound( result_cb_ = std::move(cb); set_is_outbound(true); - if (options_.auto_connect) { - RequestCreateConnection(); - } else { - StartScanningForPeer(); - } + EnsureLocalAddress(); } void LowEnergyConnector::StartInbound( @@ -156,13 +154,7 @@ void LowEnergyConnector::Cancel() { // There is nothing to do if cancel is called before the procedure has // started. There is no result callback to call yet. break; - case State::kStartingScanning: - discovery_session_.reset(); - NotifyFailure(ToResult(HostError::kCanceled)); - break; - case State::kScanning: - discovery_session_.reset(); - scan_timeout_task_.reset(); + case State::kEnsuringLocalAddress: NotifyFailure(ToResult(HostError::kCanceled)); break; case State::kConnecting: @@ -204,10 +196,8 @@ const char* LowEnergyConnector::StateToString(State state) { switch (state) { case State::kDefault: return "Default"; - case State::kStartingScanning: - return "StartingScanning"; - case State::kScanning: - return "Scanning"; + case State::kEnsuringLocalAddress: + return "EnsuringLocalAddress"; case State::kConnecting: return "Connecting"; case State::kInterrogating: @@ -223,94 +213,23 @@ const char* LowEnergyConnector::StateToString(State state) { } } -void LowEnergyConnector::StartScanningForPeer() { - if (!discovery_manager_.is_alive()) { - return; - } - auto self = weak_self_.GetWeakPtr(); - - state_.Set(State::kStartingScanning); - - discovery_manager_->StartDiscovery(/*active=*/false, [self](auto session) { - if (self.is_alive()) { - self->OnScanStart(std::move(session)); - } - }); -} - -void LowEnergyConnector::OnScanStart(LowEnergyDiscoverySessionPtr session) { - if (*state_ == State::kFailed) { - return; - } - PW_CHECK(*state_ == State::kStartingScanning); - - // Failed to start scan, abort connection procedure. - if (!session) { - bt_log(INFO, "gap-le", "failed to start scan (peer: %s)", bt_str(peer_id_)); - NotifyFailure(ToResult(HostError::kFailed)); - return; - } - - bt_log(INFO, - "gap-le", - "started scanning for pending connection (peer: %s)", - bt_str(peer_id_)); - state_.Set(State::kScanning); - - auto self = weak_self_.GetWeakPtr(); - scan_timeout_task_.emplace( - dispatcher_, [this](pw::async::Context& /*ctx*/, pw::Status status) { - if (!status.ok()) { +void LowEnergyConnector::EnsureLocalAddress() { + PW_CHECK(*state_ == State::kDefault); + state_.Set(State::kEnsuringLocalAddress); + local_address_delegate_->EnsureLocalAddress( + /*address_type=*/std::nullopt, [self = weak_self_.GetWeakPtr()](auto) { + if (!self.is_alive() || *self->state_ == State::kFailed) { return; } - PW_CHECK(*state_ == State::kScanning); - bt_log(INFO, - "gap-le", - "scan for pending connection timed out (peer: %s)", - bt_str(peer_id_)); - NotifyFailure(ToResult(HostError::kTimedOut)); + self->RequestCreateConnection(); }); - // The scan timeout may include time during which scanning is paused. - scan_timeout_task_->PostAfter(kLEGeneralCepScanTimeout); - - discovery_session_ = std::move(session); - discovery_session_->filter()->set_connectable(true); - - // The error callback must be set before the result callback in case the - // result callback is called synchronously. - discovery_session_->set_error_callback([self] { - PW_CHECK(self->state_.value() == State::kScanning); - bt_log(INFO, - "gap-le", - "discovery error while scanning for peer (peer: %s)", - bt_str(self->peer_id_)); - self->scan_timeout_task_.reset(); - self->NotifyFailure(ToResult(HostError::kFailed)); - }); - - discovery_session_->SetResultCallback([self](auto& peer) { - PW_CHECK(self->state_.value() == State::kScanning); - - if (peer.identifier() != self->peer_id_) { - return; - } - - bt_log(INFO, - "gap-le", - "discovered peer for pending connection (peer: %s)", - bt_str(self->peer_id_)); - - self->scan_timeout_task_.reset(); - self->discovery_session_->Stop(); - - self->RequestCreateConnection(); - }); } void LowEnergyConnector::RequestCreateConnection() { - // Scanning may be skipped. When the peer disconnects during/after - // interrogation, a retry may be initiated by calling this method. - PW_CHECK(*state_ == State::kDefault || *state_ == State::kScanning || + // When the peer disconnects during/after interrogation, a retry may be + // initiated by calling this method. + PW_CHECK(*state_ == State::kDefault || + *state_ == State::kEnsuringLocalAddress || *state_ == State::kPauseBeforeConnectionRetry); // Pause discovery until connection complete. @@ -458,8 +377,8 @@ void LowEnergyConnector::OnInterrogationComplete(hci::Result<> status) { void LowEnergyConnector::OnPeerDisconnect( pw::bluetooth::emboss::StatusCode status_code) { - // The peer can't disconnect while scanning or connecting, and we unregister - // from disconnects after kFailed & kComplete. + // The peer can't disconnect while connecting, and we unregister from + // disconnects after kFailed & kComplete. PW_CHECK( *state_ == State::kInterrogating || *state_ == State::kAwaitingConnectionFailedToBeEstablishedDisconnect, diff --git a/pw_bluetooth_sapphire/host/gap/public/pw_bluetooth_sapphire/internal/host/gap/low_energy_connector.h b/pw_bluetooth_sapphire/host/gap/public/pw_bluetooth_sapphire/internal/host/gap/low_energy_connector.h index 1eb7cb513..e5096f68e 100644 --- a/pw_bluetooth_sapphire/host/gap/public/pw_bluetooth_sapphire/internal/host/gap/low_energy_connector.h +++ b/pw_bluetooth_sapphire/host/gap/public/pw_bluetooth_sapphire/internal/host/gap/low_energy_connector.h @@ -47,7 +47,8 @@ class LowEnergyConnector final { l2cap::ChannelManager* l2cap, gatt::GATT::WeakPtr gatt, const AdapterState& adapter_state, - pw::async::Dispatcher& dispatcher); + pw::async::Dispatcher& dispatcher, + hci::LocalAddressDelegate* local_address_delegate); // Instances should only be destroyed after the result callback is called // (except for stack tear down). Due to the asynchronous nature of cancelling @@ -82,8 +83,7 @@ class LowEnergyConnector final { private: enum class State { kDefault, - kStartingScanning, // Outbound only - kScanning, // Outbound only + kEnsuringLocalAddress, // Outbound only kConnecting, // Outbound only kInterrogating, // Outbound & inbound kAwaitingConnectionFailedToBeEstablishedDisconnect, // Outbound & inbound @@ -94,9 +94,9 @@ class LowEnergyConnector final { static const char* StateToString(State); - // Initiate scanning for peer before connecting to ensure it is advertising. - void StartScanningForPeer(); - void OnScanStart(LowEnergyDiscoverySessionPtr session); + // Ensure the local address is updated (e.g. if privacy mode was just + // changed). + void EnsureLocalAddress(); // Initiate HCI connection procedure. void RequestCreateConnection(); @@ -183,8 +183,6 @@ class LowEnergyConnector final { // Task called after the scan attempt times out. std::optional scan_timeout_task_; - std::unique_ptr discovery_session_; - // Sends HCI commands that request version and feature support information // from peer controllers. Initialized only during interrogation. std::optional interrogator_; @@ -198,6 +196,8 @@ class LowEnergyConnector final { // Only used to construct a LowEnergyConnection. WeakSelf::WeakPtr le_connection_manager_; + hci::LocalAddressDelegate* local_address_delegate_; + struct InspectProperties { inspect::StringProperty peer_id; inspect::BoolProperty is_outbound;