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

Commit 90b25176 authored by Jakub Tyszkowski's avatar Jakub Tyszkowski
Browse files

vc: Send control point request only if device is ready 

When trying to do control point operations, being connected
is not enough as we also need to know the volume change counter.

If a group API is used to set volume bofore the individual
devices are reported as connected, we should not set it
as we dont know the volume change counter yet. The group volume
which could not be applied because of this, will be applied from
the cache once we report the device as connected.

Test: atest --host bluetooth_vc_test --no-bazel-mode
Bug: 248475583
Tag: #feature
Change-Id: I27b462f7f55122cc7c38fb18908d52e86ea4b5d8
parent 18038abf

system/bta/vc/devices.h

+1 −0
Original line number Diff line number Diff line
@@ -137,6 +137,7 @@ class VolumeControlDevice { 
  void EnqueueRemainingRequests(tGATT_IF gatt_if, GATT_READ_OP_CB chrc_read_cb,

                                GATT_WRITE_OP_CB cccd_write_cb);

  bool VerifyReady(uint16_t handle);

  bool IsReady() { return device_ready; }



 private:

  /*

system/bta/vc/vc.cc

+42 −20
Original line number Diff line number Diff line
@@ -389,7 +389,11 @@ class VolumeControlImpl : public VolumeControl { 
              << loghex(device->mute) << " change_counter "

              << loghex(device->change_counter);



    if (!device->device_ready) return;

    if (!device->IsReady()) {

      LOG_INFO("Device: %s is not ready yet.",

               device->address.ToString().c_str());

      return;

    }



    /* This is just a read, send single notification */

    if (!is_notification) {
@@ -462,7 +466,11 @@ class VolumeControlImpl : public VolumeControl { 
              << " offset: " << loghex(offset->offset)

              << " counter: " << loghex(offset->change_counter);



    if (!device->device_ready) return;

    if (!device->IsReady()) {

      LOG_INFO("Device: %s is not ready yet.",

               device->address.ToString().c_str());

      return;

    }



    callbacks_->OnExtAudioOutVolumeOffsetChanged(device->address, offset->id,

                                                 offset->offset);
@@ -483,7 +491,11 @@ class VolumeControlImpl : public VolumeControl { 
    LOG(INFO) << __func__ << "id " << loghex(offset->id) << "location "

              << loghex(offset->location);



    if (!device->device_ready) return;

    if (!device->IsReady()) {

      LOG_INFO("Device: %s is not ready yet.",

               device->address.ToString().c_str());

      return;

    }



    callbacks_->OnExtAudioOutLocationChanged(device->address, offset->id,

                                             offset->location);
@@ -514,7 +526,11 @@ class VolumeControlImpl : public VolumeControl { 


    LOG(INFO) << __func__ << " " << description;



    if (!device->device_ready) return;

    if (!device->IsReady()) {

      LOG_INFO("Device: %s is not ready yet.",

               device->address.ToString().c_str());

      return;

    }



    callbacks_->OnExtAudioOutDescriptionChanged(device->address, offset->id,

                                                std::move(description));
@@ -584,7 +600,7 @@ class VolumeControlImpl : public VolumeControl { 
    }



    // If we get here, it means, device has not been exlicitly disconnected.

    bool device_ready = device->device_ready;

    bool device_ready = device->IsReady();



    device_cleanup_helper(device, device->connecting_actively);
@@ -768,14 +784,16 @@ class VolumeControlImpl : public VolumeControl { 
    uint8_t opcode = mute ? kControlPointOpcodeMute : kControlPointOpcodeUnmute;



    if (std::holds_alternative<RawAddress>(addr_or_group_id)) {

      LOG_DEBUG("Address: %s: ",

                (std::get<RawAddress>(addr_or_group_id)).ToString().c_str());

      VolumeControlDevice* dev = volume_control_devices_.FindByAddress(

          std::get<RawAddress>(addr_or_group_id));

      if (dev && dev->IsConnected()) {

      if (dev != nullptr) {

        LOG_DEBUG("Address: %s: isReady: %s", dev->address.ToString().c_str(),

                  dev->IsReady() ? "true" : "false");

        if (dev->IsReady()) {

          std::vector<RawAddress> devices = {dev->address};

        PrepareVolumeControlOperation(devices, bluetooth::groups::kGroupUnknown,

                                      false, opcode, arg);

          PrepareVolumeControlOperation(

              devices, bluetooth::groups::kGroupUnknown, false, opcode, arg);

        }

      }

    } else {

      /* Handle group change */
@@ -790,7 +808,7 @@ class VolumeControlImpl : public VolumeControl { 
      auto devices = csis_api->GetDeviceList(group_id);

      for (auto it = devices.begin(); it != devices.end();) {

        auto dev = volume_control_devices_.FindByAddress(*it);

        if (!dev || !dev->IsConnected()) {

        if (!dev || !dev->IsReady()) {

          it = devices.erase(it);

        } else {

          it++;
@@ -831,12 +849,16 @@ class VolumeControlImpl : public VolumeControl { 
                std::get<RawAddress>(addr_or_group_id).ToString().c_str());

      VolumeControlDevice* dev = volume_control_devices_.FindByAddress(

          std::get<RawAddress>(addr_or_group_id));

      if (dev && dev->IsConnected() && (dev->volume != volume)) {

      if (dev != nullptr) {

        LOG_DEBUG("Address: %s: isReady: %s", dev->address.ToString().c_str(),

                  dev->IsReady() ? "true" : "false");

        if (dev->IsReady() && (dev->volume != volume)) {

          std::vector<RawAddress> devices = {dev->address};

        RemovePendingVolumeControlOperations(devices,

                                             bluetooth::groups::kGroupUnknown);

        PrepareVolumeControlOperation(devices, bluetooth::groups::kGroupUnknown,

                                      false, opcode, arg);

          RemovePendingVolumeControlOperations(

              devices, bluetooth::groups::kGroupUnknown);

          PrepareVolumeControlOperation(

              devices, bluetooth::groups::kGroupUnknown, false, opcode, arg);

        }

      }

    } else {

      /* Handle group change */
@@ -851,7 +873,7 @@ class VolumeControlImpl : public VolumeControl { 
      auto devices = csis_api->GetDeviceList(group_id);

      for (auto it = devices.begin(); it != devices.end();) {

        auto dev = volume_control_devices_.FindByAddress(*it);

        if (!dev || !dev->IsConnected()) {

        if (!dev || !dev->IsReady()) {

          it = devices.erase(it);

        } else {

          it++;
@@ -964,7 +986,7 @@ class VolumeControlImpl : public VolumeControl { 
  int latest_operation_id_;



  void verify_device_ready(VolumeControlDevice* device, uint16_t handle) {

    if (device->device_ready) return;

    if (device->IsReady()) return;



    // VerifyReady sets the device_ready flag if all remaining GATT operations

    // are completed

system/bta/vc/vc_test.cc

+48 −7
Original line number Diff line number Diff line
@@ -229,12 +229,15 @@ class VolumeControlTest : public ::testing::Test { 
              return;

          }



          if (do_not_respond_to_reads) return;

          cb(conn_id, GATT_SUCCESS, handle, value.size(), value.data(),

             cb_data);

        }));

  }



 protected:

  bool do_not_respond_to_reads = false;



  void SetUp(void) override {

    bluetooth::manager::SetMockBtmInterface(&btm_interface);

    MockCsisClient::SetMockInstanceForTesting(&mock_csis_client_module_);
@@ -1027,13 +1030,6 @@ class VolumeControlCsis : public VolumeControlTest { 
    SetSampleDatabase(conn_id_2);



    TestAppRegister();



    TestConnect(test_address_1);

    GetConnectedEvent(test_address_1, conn_id_1);

    GetSearchCompleteEvent(conn_id_1);

    TestConnect(test_address_2);

    GetConnectedEvent(test_address_2, conn_id_2);

    GetSearchCompleteEvent(conn_id_2);

  }



  void TearDown(void) override {
@@ -1057,6 +1053,13 @@ class VolumeControlCsis : public VolumeControlTest { 
};



TEST_F(VolumeControlCsis, test_set_volume) {

  TestConnect(test_address_1);

  GetConnectedEvent(test_address_1, conn_id_1);

  GetSearchCompleteEvent(conn_id_1);

  TestConnect(test_address_2);

  GetConnectedEvent(test_address_2, conn_id_2);

  GetSearchCompleteEvent(conn_id_2);



  /* Set value for the group */

  EXPECT_CALL(gatt_queue,

              WriteCharacteristic(conn_id_1, 0x0024, _, GATT_WRITE, _, _));
@@ -1073,7 +1076,38 @@ TEST_F(VolumeControlCsis, test_set_volume) { 
  GetNotificationEvent(conn_id_2, test_address_2, 0x0021, value);

}



TEST_F(VolumeControlCsis, test_set_volume_device_not_ready) {

  /* Make sure we did not get responds to the initial reads,

   * so that the device was not marked as ready yet.

   */

  do_not_respond_to_reads = true;



  TestConnect(test_address_1);

  GetConnectedEvent(test_address_1, conn_id_1);

  GetSearchCompleteEvent(conn_id_1);

  TestConnect(test_address_2);

  GetConnectedEvent(test_address_2, conn_id_2);

  GetSearchCompleteEvent(conn_id_2);



  /* Set value for the group */

  EXPECT_CALL(gatt_queue,

              WriteCharacteristic(conn_id_1, 0x0024, _, GATT_WRITE, _, _))

      .Times(0);

  EXPECT_CALL(gatt_queue,

              WriteCharacteristic(conn_id_2, 0x0024, _, GATT_WRITE, _, _))

      .Times(0);



  VolumeControl::Get()->SetVolume(group_id, 10);

}



TEST_F(VolumeControlCsis, autonomus_test_set_volume) {

  TestConnect(test_address_1);

  GetConnectedEvent(test_address_1, conn_id_1);

  GetSearchCompleteEvent(conn_id_1);

  TestConnect(test_address_2);

  GetConnectedEvent(test_address_2, conn_id_2);

  GetSearchCompleteEvent(conn_id_2);



  /* Now inject notification and make sure callback is sent up to Java layer */

  EXPECT_CALL(*callbacks, OnGroupVolumeStateChanged(group_id, 0x03, false, true));
@@ -1083,6 +1117,13 @@ TEST_F(VolumeControlCsis, autonomus_test_set_volume) { 
}



TEST_F(VolumeControlCsis, autonomus_single_device_test_set_volume) {

  TestConnect(test_address_1);

  GetConnectedEvent(test_address_1, conn_id_1);

  GetSearchCompleteEvent(conn_id_1);

  TestConnect(test_address_2);

  GetConnectedEvent(test_address_2, conn_id_2);

  GetSearchCompleteEvent(conn_id_2);



  /* Disconnect one device. */

  EXPECT_CALL(*callbacks,

              OnConnectionState(ConnectionState::DISCONNECTED, test_address_1));