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

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

VolumeControl: Do not set the same volume level twice 

Devices may connect or be discovered as a new group
memeber after the streaming was already started, or
services can be connected in a different order each
time. The newly connected device may need additional
volume level corrections. Some of these volume changes
may already be pending on the native layer due to
previous group API calls, while Java has not yet been
notified about all the native stack events.

This change eliminates any additional volume state
changes if the volume is already set to the desired
level but the Java layer did not know that, or
when the volume changing GATT operation with same
parameters is already on the operation queue.

Bug: 248969553
Bug: 248915809
Tag: #feature
Test: atest --host bluetooth_vc_test --no-bazel-mode
Change-Id: Ic7ad61a8f8be6a48bb09aa3b07059e8f1abbd528
parent e8e7f2e0

system/bta/vc/vc.cc

+35 −20
Original line number Diff line number Diff line
@@ -743,13 +743,23 @@ class VolumeControlImpl : public VolumeControl { 
                                     int group_id, bool is_autonomous,

                                     uint8_t opcode,

                                     std::vector<uint8_t>& arguments) {

    DLOG(INFO) << __func__ << " num of devices: " << devices.size()

               << " group_id: " << group_id

               << " is_autonomous: " << is_autonomous << " opcode: " << +opcode

               << " arg size: " << arguments.size();



    LOG_DEBUG(

        "num of devices: %zu, group_id: %d, is_autonomous: %s  opcode: %d, arg "

        "size: %zu",

        devices.size(), group_id, is_autonomous ? "true" : "false", +opcode,

        arguments.size());



    if (std::find_if(ongoing_operations_.begin(), ongoing_operations_.end(),

                     [opcode, &arguments](const VolumeOperation& op) {

                       return (op.opcode_ == opcode) &&

                              std::equal(op.arguments_.begin(),

                                         op.arguments_.end(),

                                         arguments.begin());

                     }) == ongoing_operations_.end()) {

      ongoing_operations_.emplace_back(latest_operation_id_++, group_id,

                                     is_autonomous, opcode, arguments, devices);

                                       is_autonomous, opcode, arguments,

                                       devices);

    }

  }



  void MuteUnmute(std::variant<RawAddress, int> addr_or_group_id, bool mute) {
@@ -760,11 +770,13 @@ class VolumeControlImpl : public VolumeControl { 
    if (std::holds_alternative<RawAddress>(addr_or_group_id)) {

      LOG_DEBUG("Address: %s: ",

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

      std::vector<RawAddress> devices = {

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



      VolumeControlDevice* dev = volume_control_devices_.FindByAddress(

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

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

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

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

                                      false, opcode, arg);

      }

    } else {

      /* Handle group change */

      auto group_id = std::get<int>(addr_or_group_id);
@@ -815,14 +827,17 @@ class VolumeControlImpl : public VolumeControl { 
    uint8_t opcode = kControlPointOpcodeSetAbsoluteVolume;



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

      DLOG(INFO) << __func__ << " " << std::get<RawAddress>(addr_or_group_id);

      std::vector<RawAddress> devices = {

          std::get<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() && (dev->volume != volume)) {

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

        RemovePendingVolumeControlOperations(devices,

                                             bluetooth::groups::kGroupUnknown);

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

                                      false, opcode, arg);

      }

    } else {

      /* Handle group change */

      auto group_id = std::get<int>(addr_or_group_id);

system/bta/vc/vc_test.cc

+30 −3
Original line number Diff line number Diff line
@@ -915,10 +915,37 @@ class VolumeControlValueSetTest : public VolumeControlTest { 
};



TEST_F(VolumeControlValueSetTest, test_set_volume) {

  std::vector<uint8_t> expected_data({0x04, 0x00, 0x10});

  EXPECT_CALL(gatt_queue, WriteCharacteristic(conn_id, 0x0024, expected_data,

                                              GATT_WRITE, _, _));

  ON_CALL(gatt_queue, WriteCharacteristic(conn_id, 0x0024, _, GATT_WRITE, _, _))

      .WillByDefault([this](uint16_t conn_id, uint16_t handle,

                            std::vector<uint8_t> value,

                            tGATT_WRITE_TYPE write_type, GATT_WRITE_OP_CB cb,

                            void* cb_data) {

        std::vector<uint8_t> ntf_value({

            value[2],                            // volume level

            0,                                   // muted

            static_cast<uint8_t>(value[1] + 1),  // change counter

        });

        GetNotificationEvent(0x0021, ntf_value);

      });



  const std::vector<uint8_t> vol_x10({0x04, 0x00, 0x10});

  EXPECT_CALL(gatt_queue,

              WriteCharacteristic(conn_id, 0x0024, vol_x10, GATT_WRITE, _, _))

      .Times(1);

  VolumeControl::Get()->SetVolume(test_address, 0x10);



  // Same volume level should not be applied twice

  const std::vector<uint8_t> vol_x10_2({0x04, 0x01, 0x10});

  EXPECT_CALL(gatt_queue,

              WriteCharacteristic(conn_id, 0x0024, vol_x10_2, GATT_WRITE, _, _))

      .Times(0);

  VolumeControl::Get()->SetVolume(test_address, 0x10);



  const std::vector<uint8_t> vol_x20({0x04, 0x01, 0x20});

  EXPECT_CALL(gatt_queue,

              WriteCharacteristic(conn_id, 0x0024, vol_x20, GATT_WRITE, _, _))

      .Times(1);

  VolumeControl::Get()->SetVolume(test_address, 0x20);

}



TEST_F(VolumeControlValueSetTest, test_mute) {