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

Commit bbc15b23 authored by Łukasz Rymanowski's avatar Łukasz Rymanowski Committed by Gerrit Code Review
Browse files

Merge changes Icf835d91,I27b462f7 

* changes:
  VolumeControl: Correct device volume only if connected
  vc: Send control point request only if device is ready
parents 1a25b385 1e1e2f42

android/app/src/com/android/bluetooth/vc/VolumeControlService.java

+26 −3
Original line number Diff line number Diff line
@@ -654,9 +654,21 @@ public class VolumeControlService extends ProfileService { 
        Integer groupVolume = mGroupVolumeCache.getOrDefault(groupId,

                IBluetoothVolumeControl.VOLUME_CONTROL_UNKNOWN_VOLUME);

        if (groupVolume != IBluetoothVolumeControl.VOLUME_CONTROL_UNKNOWN_VOLUME) {

            // Correct the volume level only if device was already reported as connected.

            boolean can_change_volume = false;

            synchronized (mStateMachines) {

                VolumeControlStateMachine sm = mStateMachines.get(device);

                if (sm != null) {

                    can_change_volume =

                            (sm.getConnectionState() == BluetoothProfile.STATE_CONNECTED);

                }

            }

            if (can_change_volume) {

                Log.i(TAG, "Setting value:" + groupVolume + " to " + device);

                mVolumeControlNativeInterface.setVolume(device, groupVolume);

            }

        }

    }



    void handleVolumeControlChanged(BluetoothDevice device, int groupId,

                                    int volume, boolean mute, boolean isAutonomous) {
@@ -696,8 +708,19 @@ public class VolumeControlService extends ProfileService { 


            if (device != null && groupVolume

                            != IBluetoothVolumeControl.VOLUME_CONTROL_UNKNOWN_VOLUME) {

                // Correct the volume level only if device was already reported as connected.

                boolean can_change_volume = false;

                synchronized (mStateMachines) {

                    VolumeControlStateMachine sm = mStateMachines.get(device);

                    if (sm != null) {

                        can_change_volume =

                                (sm.getConnectionState() == BluetoothProfile.STATE_CONNECTED);

                    }

                }

                if (can_change_volume) {

                    Log.i(TAG, "Setting value:" + groupVolume + " to " + device);

                    mVolumeControlNativeInterface.setVolume(device, groupVolume);

                }

            } else {

                Log.e(TAG, "Volume changed did not succeed. Volume: " + volume

                                + " expected volume: " + groupVolume);

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));