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

Commit ea9abca0 authored by Łukasz Rymanowski's avatar Łukasz Rymanowski Committed by Jakub Tyszkowski
Browse files

vc: Use oportunistic connect mode 

With this patch, VC uses opportinistic connect
This is needed to proper handle reconnection and switch between BR/EDR
and LE

Bug: 282778469
Bug: 279990836
Test: atest bluetooth_vc_test
Test: atest BluetoothInstrumentationTests
Tag: #feature

Change-Id: I9e881d6c26cada4c4b49bbe8558a659a1af559df
parent e759dd8d

system/bta/include/bta_vc_api.h

+4 −2
Original line number Diff line number Diff line
@@ -25,18 +25,20 @@ class VolumeControl { 
 public:

  virtual ~VolumeControl() = default;



  static void Initialize(bluetooth::vc::VolumeControlCallbacks* callbacks);

  static void Initialize(bluetooth::vc::VolumeControlCallbacks* callbacks,

                         const base::Closure& initCb);

  static void CleanUp();

  static VolumeControl* Get();

  static void DebugDump(int fd);



  static void AddFromStorage(const RawAddress& address, bool auto_connect);

  static void AddFromStorage(const RawAddress& address);



  static bool IsVolumeControlRunning();



  /* Volume Control Server (VCS) */

  virtual void Connect(const RawAddress& address) = 0;

  virtual void Disconnect(const RawAddress& address) = 0;

  virtual void Remove(const RawAddress& address) = 0;

  virtual void SetVolume(std::variant<RawAddress, int> addr_or_group_id,

                         uint8_t volume) = 0;

  virtual void Mute(std::variant<RawAddress, int> addr_or_group_id) = 0;

system/bta/vc/device.cc

+0 −2
Original line number Diff line number Diff line
@@ -53,8 +53,6 @@ void VolumeControlDevice::Disconnect(tGATT_IF gatt_if) { 
    BtaGattQueue::Clean(connection_id);

    BTA_GATTC_Close(connection_id);

    connection_id = GATT_INVALID_CONN_ID;

  } else {

    BTA_GATTC_CancelOpen(gatt_if, address, false);

  }



  device_ready = false;

system/bta/vc/devices_test.cc

+0 −1
Original line number Diff line number Diff line
@@ -130,7 +130,6 @@ TEST_F(VolumeControlDevicesTest, test_disconnect) { 
  test_device_0->connection_id = 0x0005;

  tGATT_IF gatt_if = 8;

  EXPECT_CALL(gatt_interface, Close(test_device_0->connection_id));

  EXPECT_CALL(gatt_interface, CancelOpen(gatt_if, test_address_1, _));

  devices_->Disconnect(gatt_if);

}

system/bta/vc/vc.cc

+49 −30
Original line number Diff line number Diff line
@@ -81,23 +81,35 @@ class VolumeControlImpl : public VolumeControl { 
 public:

  ~VolumeControlImpl() override = default;



  VolumeControlImpl(bluetooth::vc::VolumeControlCallbacks* callbacks)

  VolumeControlImpl(bluetooth::vc::VolumeControlCallbacks* callbacks,

                    const base::Closure& initCb)

      : gatt_if_(0), callbacks_(callbacks), latest_operation_id_(0) {

    BTA_GATTC_AppRegister(

        gattc_callback_static,

        base::Bind([](uint8_t client_id, uint8_t status) {

        base::Bind(

            [](const base::Closure& initCb, uint8_t client_id, uint8_t status) {

              if (status != GATT_SUCCESS) {

                LOG(ERROR) << "Can't start Volume Control profile - no gatt "

                              "clients left!";

                return;

              }

              instance->gatt_if_ = client_id;

        }),

              initCb.Run();

            },

            initCb),

        true);

  }



  void StartOpportunisticConnect(const RawAddress& address) {

    /* Oportunistic works only for direct connect,

     * but in fact this is background connect

     */

    LOG_INFO(": %s ", ADDRESS_TO_LOGGABLE_CSTR(address));

    BTA_GATTC_Open(gatt_if_, address, BTM_BLE_DIRECT_CONNECTION, true);

  }



  void Connect(const RawAddress& address) override {

    LOG(INFO) << __func__ << " " << ADDRESS_TO_LOGGABLE_STR(address);

    LOG_INFO(": %s ", ADDRESS_TO_LOGGABLE_CSTR(address));



    auto device = volume_control_devices_.FindByAddress(address);

    if (!device) {
@@ -121,19 +133,13 @@ class VolumeControlImpl : public VolumeControl { 
      }

    }



    BTA_GATTC_Open(gatt_if_, address, BTM_BLE_DIRECT_CONNECTION, false);

    StartOpportunisticConnect(address);

  }



  void AddFromStorage(const RawAddress& address, bool auto_connect) {

    LOG(INFO) << __func__ << " " << ADDRESS_TO_LOGGABLE_STR(address)

              << ", auto_connect=" << auto_connect;



    if (auto_connect) {

  void AddFromStorage(const RawAddress& address) {

    LOG_INFO("%s ", ADDRESS_TO_LOGGABLE_CSTR(address));

    volume_control_devices_.Add(address, false);



      /* Add device into BG connection to accept remote initiated connection */

      BTA_GATTC_Open(gatt_if_, address, BTM_BLE_BKG_CONNECT_ALLOW_LIST, false);

    }

    StartOpportunisticConnect(address);

  }



  void OnGattConnected(tGATT_STATUS status, uint16_t connection_id,
@@ -158,6 +164,11 @@ class VolumeControlImpl : public VolumeControl { 


    device->connection_id = connection_id;



    /* Make sure to remove device from background connect.

     * It will be added back if needed, when device got disconnected

     */

    BTA_GATTC_CancelOpen(gatt_if_, address, false);



    if (device->IsEncryptionEnabled()) {

      OnEncryptionComplete(address, BTM_SUCCESS);

      return;
@@ -627,6 +638,8 @@ class VolumeControlImpl : public VolumeControl { 
  }



  void Disconnect(const RawAddress& address) override {

    LOG_INFO(": %s ", ADDRESS_TO_LOGGABLE_CSTR(address));



    VolumeControlDevice* device =

        volume_control_devices_.FindByAddress(address);

    if (!device) {
@@ -642,6 +655,15 @@ class VolumeControlImpl : public VolumeControl { 
    device_cleanup_helper(device, true);

  }



  void Remove(const RawAddress& address) override {

    LOG_INFO(": %s", ADDRESS_TO_LOGGABLE_CSTR(address));



    /* Removes all registrations for connection. */

    BTA_GATTC_CancelOpen(gatt_if_, address, false);



    Disconnect(address);

  }



  void OnGattDisconnected(uint16_t connection_id, tGATT_IF /*client_if*/,

                          RawAddress remote_bda, tGATT_DISCONN_REASON reason) {

    VolumeControlDevice* device =
@@ -665,9 +687,7 @@ class VolumeControlImpl : public VolumeControl { 


    if (reason != GATT_CONN_TERMINATE_LOCAL_HOST &&

        device->connecting_actively) {

      /* Add device into BG connection to accept remote initiated connection */

      BTA_GATTC_Open(gatt_if_, remote_bda, BTM_BLE_BKG_CONNECT_ALLOW_LIST,

                     false);

      StartOpportunisticConnect(remote_bda);

    }

  }
@@ -1181,15 +1201,15 @@ class VolumeControlImpl : public VolumeControl { 
};

}  // namespace



void VolumeControl::Initialize(

    bluetooth::vc::VolumeControlCallbacks* callbacks) {

void VolumeControl::Initialize(bluetooth::vc::VolumeControlCallbacks* callbacks,

                               const base::Closure& initCb) {

  std::scoped_lock<std::mutex> lock(instance_mutex);

  if (instance) {

    LOG(ERROR) << "Already initialized!";

    return;

  }



  instance = new VolumeControlImpl(callbacks);

  instance = new VolumeControlImpl(callbacks, initCb);

}



bool VolumeControl::IsVolumeControlRunning() { return instance; }
@@ -1199,14 +1219,13 @@ VolumeControl* VolumeControl::Get(void) { 
  return instance;

};



void VolumeControl::AddFromStorage(const RawAddress& address,

                                   bool auto_connect) {

void VolumeControl::AddFromStorage(const RawAddress& address) {

  if (!instance) {

    LOG(ERROR) << "Not initialized yet";

    return;

  }



  instance->AddFromStorage(address, auto_connect);

  instance->AddFromStorage(address);

};



void VolumeControl::CleanUp() {

system/bta/vc/vc_test.cc

+59 −19
Original line number Diff line number Diff line
@@ -317,7 +317,7 @@ class VolumeControlTest : public ::testing::Test { 
    EXPECT_CALL(gatt_interface, AppRegister(_, _, _))

        .WillOnce(DoAll(SaveArg<0>(&gatt_callback),

                        SaveArg<1>(&app_register_callback)));

    VolumeControl::Initialize(callbacks.get());

    VolumeControl::Initialize(callbacks.get(), base::DoNothing());

    ASSERT_TRUE(gatt_callback);

    ASSERT_TRUE(app_register_callback);

    app_register_callback.Run(gatt_if, GATT_SUCCESS);
@@ -337,33 +337,40 @@ class VolumeControlTest : public ::testing::Test { 
        .WillByDefault(DoAll(Return(true)));



    EXPECT_CALL(gatt_interface,

                Open(gatt_if, address, BTM_BLE_DIRECT_CONNECTION, _));

                Open(gatt_if, address, BTM_BLE_DIRECT_CONNECTION, true));

    VolumeControl::Get()->Connect(address);

    Mock::VerifyAndClearExpectations(&gatt_interface);

  }



  void TestRemove(const RawAddress& address, uint16_t conn_id) {

    EXPECT_CALL(gatt_interface, CancelOpen(gatt_if, address, false));

    if (conn_id) {

      EXPECT_CALL(gatt_interface, Close(conn_id));

    } else {

      EXPECT_CALL(gatt_interface, Close(conn_id)).Times(0);

    }

    VolumeControl::Get()->Remove(address);

    Mock::VerifyAndClearExpectations(&gatt_interface);

  }



  void TestDisconnect(const RawAddress& address, uint16_t conn_id) {

    if (conn_id) {

      EXPECT_CALL(gatt_interface, Close(conn_id));

    } else {

      EXPECT_CALL(gatt_interface, CancelOpen(gatt_if, address, _));

      EXPECT_CALL(gatt_interface, Close(conn_id)).Times(0);

    }

    VolumeControl::Get()->Disconnect(address);

    Mock::VerifyAndClearExpectations(&gatt_interface);

  }



  void TestAddFromStorage(const RawAddress& address, bool auto_connect) {

  void TestAddFromStorage(const RawAddress& address) {

    // by default indicate link as encrypted

    ON_CALL(btm_interface, BTM_IsEncrypted(address, _))

        .WillByDefault(DoAll(Return(true)));



    if (auto_connect) {

    EXPECT_CALL(gatt_interface,

                  Open(gatt_if, address, BTM_BLE_BKG_CONNECT_ALLOW_LIST, _));

    } else {

      EXPECT_CALL(gatt_interface, Open(gatt_if, address, _, _)).Times(0);

    }

    VolumeControl::Get()->AddFromStorage(address, auto_connect);

                Open(gatt_if, address, BTM_BLE_DIRECT_CONNECTION, true));

    VolumeControl::Get()->AddFromStorage(address);

  }



  void TestSubscribeNotifications(const RawAddress& address, uint16_t conn_id,
@@ -511,21 +518,34 @@ TEST_F(VolumeControlTest, test_get_uninitialized) { 
}



TEST_F(VolumeControlTest, test_initialize) {

  VolumeControl::Initialize(callbacks.get());

  bool init_cb_called = false;

  BtaAppRegisterCallback app_register_callback;

  EXPECT_CALL(gatt_interface, AppRegister(_, _, _))

      .WillOnce(DoAll(SaveArg<0>(&gatt_callback),

                      SaveArg<1>(&app_register_callback)));

  VolumeControl::Initialize(

      callbacks.get(),

      base::Bind([](bool* init_cb_called) { *init_cb_called = true; },

                 &init_cb_called));

  ASSERT_TRUE(gatt_callback);

  ASSERT_TRUE(app_register_callback);

  app_register_callback.Run(gatt_if, GATT_SUCCESS);

  ASSERT_TRUE(init_cb_called);



  ASSERT_TRUE(VolumeControl::IsVolumeControlRunning());

  VolumeControl::CleanUp();

}



TEST_F(VolumeControlTest, test_initialize_twice) {

  VolumeControl::Initialize(callbacks.get());

  VolumeControl::Initialize(callbacks.get(), base::DoNothing());

  VolumeControl* volume_control_p = VolumeControl::Get();

  VolumeControl::Initialize(callbacks.get());

  VolumeControl::Initialize(callbacks.get(), base::DoNothing());

  ASSERT_EQ(volume_control_p, VolumeControl::Get());

  VolumeControl::CleanUp();

}



TEST_F(VolumeControlTest, test_cleanup_initialized) {

  VolumeControl::Initialize(callbacks.get());

  VolumeControl::Initialize(callbacks.get(), base::DoNothing());

  VolumeControl::CleanUp();

  ASSERT_FALSE(VolumeControl::IsVolumeControlRunning());

}
@@ -597,8 +617,28 @@ TEST_F(VolumeControlTest, test_reconnect_after_interrupted_discovery) { 


TEST_F(VolumeControlTest, test_add_from_storage) {

  TestAppRegister();

  TestAddFromStorage(GetTestAddress(0), true);

  TestAddFromStorage(GetTestAddress(1), false);

  TestAddFromStorage(GetTestAddress(0));

  TestAppUnregister();

}



TEST_F(VolumeControlTest, test_remove_non_connected) {

  const RawAddress test_address = GetTestAddress(0);

  TestAppRegister();

  TestConnect(test_address);

  EXPECT_CALL(*callbacks,

              OnConnectionState(ConnectionState::DISCONNECTED, test_address));

  TestRemove(test_address, 0);

  TestAppUnregister();

}



TEST_F(VolumeControlTest, test_remove_connected) {

  const RawAddress test_address = GetTestAddress(0);

  TestAppRegister();

  TestConnect(test_address);

  GetConnectedEvent(test_address, 1);

  EXPECT_CALL(*callbacks,

              OnConnectionState(ConnectionState::DISCONNECTED, test_address));

  TestDisconnect(test_address, 1);

  TestAppUnregister();

}
@@ -637,7 +677,7 @@ TEST_F(VolumeControlTest, test_disconnected) { 
TEST_F(VolumeControlTest, test_disconnected_while_autoconnect) {

  const RawAddress test_address = GetTestAddress(0);

  TestAppRegister();

  TestAddFromStorage(test_address, true);

  TestAddFromStorage(test_address);

  GetConnectedEvent(test_address, 1);

  // autoconnect - don't indicate disconnection

  EXPECT_CALL(*callbacks,
@@ -650,7 +690,7 @@ TEST_F(VolumeControlTest, test_disconnected_while_autoconnect) { 
TEST_F(VolumeControlTest, test_reconnect_after_encryption_failed) {

  const RawAddress test_address = GetTestAddress(0);

  TestAppRegister();

  TestAddFromStorage(test_address, true);

  TestAddFromStorage(test_address);

  SetEncryptionResult(test_address, false);

  // autoconnect - don't indicate disconnection

  EXPECT_CALL(*callbacks,