[bt][gap-le] Make Disconnect on unconnected peers return true
Change the contract of LowEnergyConnectionManager::Disconnect from
"perform disconnection on peer" to "ensure that peer is disconnected."
BT-824
BT-652
Test: In bt-host-unittests
GAP_LowEnergyConnectionManagerTest.Disconnect*
Change-Id: I3cbee118dd306c10df76815c1e9ab0b92a6870f8
diff --git a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection_manager.cc b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection_manager.cc
index 6c99295..99bd726 100644
--- a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection_manager.cc
+++ b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection_manager.cc
@@ -536,15 +536,14 @@
auto iter = connections_.find(peer_id);
if (iter == connections_.end()) {
bt_log(WARN, "gap-le", "peer not connected (id: %s)", bt_str(peer_id));
- return false;
+ return true;
}
// Remove the connection state from the internal map right away.
auto conn = std::move(iter->second);
connections_.erase(iter);
- bt_log(INFO, "gap-le", "disconnecting link: %s",
- conn->link()->ToString().c_str());
+ bt_log(INFO, "gap-le", "disconnecting link: %s", bt_str(*conn->link()));
CleanUpConnection(std::move(conn));
return true;
}
diff --git a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection_manager.h b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection_manager.h
index 00f960a..11350f7 100644
--- a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection_manager.h
+++ b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection_manager.h
@@ -138,8 +138,7 @@
}
// Disconnects any existing LE connection to |peer_id|, invalidating all
- // active LowEnergyConnectionRefs. Returns false if |peer_id| is not
- // recognized, the corresponding peer is not connected, or the peer can not be
+ // active LowEnergyConnectionRefs. Returns false if the peer can not be
// disconnected.
bool Disconnect(PeerId peer_id);
diff --git a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection_manager_unittest.cc b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection_manager_unittest.cc
index 64364c8..aecae7c 100644
--- a/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection_manager_unittest.cc
+++ b/src/connectivity/bluetooth/core/bt-host/gap/low_energy_connection_manager_unittest.cc
@@ -731,12 +731,17 @@
EXPECT_FALSE(conn_mgr()->Disconnect(dev1->identifier()));
}
-TEST_F(GAP_LowEnergyConnectionManagerTest, DisconnectError) {
+TEST_F(GAP_LowEnergyConnectionManagerTest, DisconnectUnknownPeer) {
+ // Unknown peers are inherently "not connected."
+ EXPECT_TRUE(conn_mgr()->Disconnect(PeerId(999)));
+}
+
+TEST_F(GAP_LowEnergyConnectionManagerTest, DisconnectUnconnectedPeer) {
auto* peer = peer_cache()->NewPeer(kAddress0, true);
test_device()->AddPeer(std::make_unique<FakePeer>(kAddress0));
- // This should fail as |peer0| is not connected.
- EXPECT_FALSE(conn_mgr()->Disconnect(peer->identifier()));
+ // This returns true so long the peer is not connected.
+ EXPECT_TRUE(conn_mgr()->Disconnect(peer->identifier()));
}
TEST_F(GAP_LowEnergyConnectionManagerTest, Disconnect) {
@@ -771,6 +776,41 @@
EXPECT_TRUE(canceled_peers().empty());
}
+TEST_F(GAP_LowEnergyConnectionManagerTest, DisconnectThrice) {
+ auto* peer = peer_cache()->NewPeer(kAddress0, true);
+ test_device()->AddPeer(std::make_unique<FakePeer>(kAddress0));
+
+ int closed_count = 0;
+ auto closed_cb = [&closed_count] { closed_count++; };
+
+ LowEnergyConnectionRefPtr conn_ref;
+ auto success_cb = [&closed_cb, &conn_ref](auto status, auto cb_conn_ref) {
+ EXPECT_TRUE(status);
+ conn_ref = std::move(cb_conn_ref);
+ ASSERT_TRUE(conn_ref);
+ conn_ref->set_closed_callback(closed_cb);
+ };
+
+ EXPECT_TRUE(conn_mgr()->Connect(peer->identifier(), success_cb));
+
+ RunLoopUntilIdle();
+
+ EXPECT_TRUE(conn_mgr()->Disconnect(peer->identifier()));
+
+ // Try to disconnect again while the first disconnection is in progress.
+ EXPECT_TRUE(conn_mgr()->Disconnect(peer->identifier()));
+
+ RunLoopUntilIdle();
+
+ // The single ref should get only one "closed" call.
+ EXPECT_EQ(1, closed_count);
+ EXPECT_TRUE(connected_peers().empty());
+ EXPECT_TRUE(canceled_peers().empty());
+
+ // Try to disconnect once more, now that the link is gone.
+ EXPECT_TRUE(conn_mgr()->Disconnect(peer->identifier()));
+}
+
// Tests when a link is lost without explicitly disconnecting
TEST_F(GAP_LowEnergyConnectionManagerTest, DisconnectEvent) {
auto* peer = peer_cache()->NewPeer(kAddress0, true);
@@ -804,6 +844,32 @@
EXPECT_EQ(2, closed_count);
}
+TEST_F(GAP_LowEnergyConnectionManagerTest, DisconnectAfterRefsReleased) {
+ auto* peer = peer_cache()->NewPeer(kAddress0, true);
+ test_device()->AddPeer(std::make_unique<FakePeer>(kAddress0));
+
+ LowEnergyConnectionRefPtr conn_ref;
+ auto success_cb = [&conn_ref](auto status, auto cb_conn_ref) {
+ EXPECT_TRUE(status);
+ conn_ref = std::move(cb_conn_ref);
+ };
+
+ EXPECT_TRUE(conn_mgr()->Connect(peer->identifier(), success_cb));
+
+ RunLoopUntilIdle();
+
+ ASSERT_TRUE(conn_ref);
+ conn_ref.reset();
+
+ // Try to disconnect while the zero-refs connection is being disconnected.
+ EXPECT_TRUE(conn_mgr()->Disconnect(peer->identifier()));
+
+ RunLoopUntilIdle();
+
+ EXPECT_TRUE(connected_peers().empty());
+ EXPECT_TRUE(canceled_peers().empty());
+}
+
TEST_F(GAP_LowEnergyConnectionManagerTest, DisconnectWhileRefPending) {
auto* peer = peer_cache()->NewPeer(kAddress0, true);
test_device()->AddPeer(std::make_unique<FakePeer>(kAddress0));
