Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ffd7e220 authored by Jakub Tyszkowski's avatar Jakub Tyszkowski
Browse files

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
Merged-In: I46fa8e1da1b5c630fd633f51d29c0084367adb33
(cherry picked from commit d916000d)
parent 59997cae

system/bta/vc/vc.cc

+25 −2
Original line number Diff line number Diff line
@@ -102,6 +102,21 @@ class VolumeControlImpl : public VolumeControl { 
      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);
@@ -628,13 +643,22 @@ class VolumeControlImpl : public VolumeControl { 
      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,
@@ -1068,7 +1092,6 @@ class VolumeControlImpl : public VolumeControl { 
    if (notify)

      callbacks_->OnConnectionState(ConnectionState::DISCONNECTED,

                                    device->address);

    volume_control_devices_.Remove(device->address);

  }



  void devices_control_point_helper(std::vector<RawAddress>& devices,

system/bta/vc/vc_test.cc

+49 −0
Original line number Diff line number Diff line
@@ -547,6 +547,55 @@ TEST_F(VolumeControlTest, test_connect) { 
  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);