vc: Dont ignore VC devices connected by another gatt client
Like in other services we should not remove disconnected
devices from the list of known and valid VC devices. Once
the device gets connected by another gatt client, we still
get the GATT connection event in VC, and if we ignore that
(which is the case when we remove the device from the list of known VC
devices), we will not be able to reconnect, since calling
BTA_GATTC_Open() will not trigger the connection event for
the already connected device.
Bug: 254810994
Tag: #feature
Test: atest --host bluetooth_vc_test --no-bazel-mode
Test: atest BluetoothInstrumentationTests
Change-Id: I46fa8e1da1b5c630fd633f51d29c0084367adb33
diff --git a/system/bta/vc/vc.cc b/system/bta/vc/vc.cc
index 75750a3..8eaa339 100644
--- a/system/bta/vc/vc.cc
+++ b/system/bta/vc/vc.cc
@@ -102,6 +102,21 @@
volume_control_devices_.Add(address, true);
} else {
device->connecting_actively = true;
+
+ if (device->IsConnected()) {
+ LOG(WARNING) << __func__ << ": address=" << address
+ << ", connection_id=" << device->connection_id
+ << " already connected.";
+
+ if (device->IsReady()) {
+ callbacks_->OnConnectionState(ConnectionState::CONNECTED,
+ device->address);
+ } else {
+ OnGattConnected(GATT_SUCCESS, device->connection_id, gatt_if_,
+ device->address, BT_TRANSPORT_LE, GATT_MAX_MTU_SIZE);
+ }
+ return;
+ }
}
BTA_GATTC_Open(gatt_if_, address, BTM_BLE_DIRECT_CONNECTION, false);
@@ -637,13 +652,22 @@
return;
}
+ if (!device->IsConnected()) {
+ LOG(ERROR) << __func__
+ << " Skipping disconnect of the already disconnected device, "
+ "connection_id="
+ << loghex(connection_id);
+ return;
+ }
+
// If we get here, it means, device has not been exlicitly disconnected.
bool device_ready = device->IsReady();
device_cleanup_helper(device, device->connecting_actively);
if (device_ready) {
- volume_control_devices_.Add(remote_bda, true);
+ device->first_connection = true;
+ device->connecting_actively = true;
/* Add device into BG connection to accept remote initiated connection */
BTA_GATTC_Open(gatt_if_, remote_bda, BTM_BLE_BKG_CONNECT_ALLOW_LIST,
@@ -1079,7 +1103,6 @@
if (notify)
callbacks_->OnConnectionState(ConnectionState::DISCONNECTED,
device->address);
- volume_control_devices_.Remove(device->address);
}
void devices_control_point_helper(std::vector<RawAddress>& devices,
diff --git a/system/bta/vc/vc_test.cc b/system/bta/vc/vc_test.cc
index 2d75393..f2c4cbb 100644
--- a/system/bta/vc/vc_test.cc
+++ b/system/bta/vc/vc_test.cc
@@ -546,6 +546,55 @@
TestAppUnregister();
}
+TEST_F(VolumeControlTest, test_reconnect_after_interrupted_discovery) {
+ const RawAddress test_address = GetTestAddress(0);
+
+ // Initial connection - no callback calls yet as we want to disconnect in the
+ // middle
+ SetSampleDatabaseVOCS(1);
+ TestAppRegister();
+ TestConnect(test_address);
+ EXPECT_CALL(*callbacks,
+ OnConnectionState(ConnectionState::CONNECTED, test_address))
+ .Times(0);
+ EXPECT_CALL(*callbacks, OnDeviceAvailable(test_address, 2)).Times(0);
+ GetConnectedEvent(test_address, 1);
+ Mock::VerifyAndClearExpectations(callbacks.get());
+
+ // Remote disconnects in the middle of the service discovery
+ EXPECT_CALL(*callbacks,
+ OnConnectionState(ConnectionState::DISCONNECTED, test_address));
+ GetDisconnectedEvent(test_address, 1);
+ Mock::VerifyAndClearExpectations(callbacks.get());
+
+ // This time let the service discovery pass
+ ON_CALL(gatt_interface, ServiceSearchRequest(_, _))
+ .WillByDefault(Invoke(
+ [&](uint16_t conn_id, const bluetooth::Uuid* p_srvc_uuid) -> void {
+ if (*p_srvc_uuid == kVolumeControlUuid)
+ GetSearchCompleteEvent(conn_id);
+ }));
+
+ // Remote is being connected by another GATT client
+ EXPECT_CALL(*callbacks,
+ OnConnectionState(ConnectionState::CONNECTED, test_address));
+ EXPECT_CALL(*callbacks, OnDeviceAvailable(test_address, 2));
+ GetConnectedEvent(test_address, 1);
+ Mock::VerifyAndClearExpectations(callbacks.get());
+
+ // Request connect when the remote was already connected by another service
+ EXPECT_CALL(*callbacks, OnDeviceAvailable(test_address, 2)).Times(0);
+ EXPECT_CALL(*callbacks,
+ OnConnectionState(ConnectionState::CONNECTED, test_address));
+ VolumeControl::Get()->Connect(test_address);
+ // The GetConnectedEvent(test_address, 1); should not be triggered here, since
+ // GATT implementation will not send this event for the already connected
+ // device
+ Mock::VerifyAndClearExpectations(callbacks.get());
+
+ TestAppUnregister();
+}
+
TEST_F(VolumeControlTest, test_add_from_storage) {
TestAppRegister();
TestAddFromStorage(GetTestAddress(0), true);
