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

Commit 37bd31d6 authored by TreeHugger Robot's avatar TreeHugger Robot Committed by Android (Google) Code Review
Browse files

Merge changes Ia4f40a6b,Icf835d91,I27b462f7 into tm-qpr-dev 

* changes:
  vc: Fix doing initial reads and writes twice
  VolumeControl: Correct device volume only if connected
  vc: Send control point request only if device is ready
parents cc1b6dbd 84de8db1

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/device.cc

+2 −4
Original line number Diff line number Diff line
@@ -415,10 +415,8 @@ bool VolumeControlDevice::IsEncryptionEnabled() { 
  return BTM_IsEncrypted(address, BT_TRANSPORT_LE);

}



bool VolumeControlDevice::EnableEncryption(tBTM_SEC_CALLBACK* callback) {

  int result = BTM_SetEncryption(address, BT_TRANSPORT_LE, callback, nullptr,

void VolumeControlDevice::EnableEncryption() {

  int result = BTM_SetEncryption(address, BT_TRANSPORT_LE, nullptr, nullptr,

                                 BTM_BLE_SEC_ENCRYPT);

  LOG(INFO) << __func__ << ": result=" << +result;

  // TODO: should we care about the result??

  return true;

}

system/bta/vc/devices.h

+2 −1
Original line number Diff line number Diff line
@@ -130,13 +130,14 @@ class VolumeControlDevice { 
                                        GATT_WRITE_OP_CB cb, void* cb_data);

  bool IsEncryptionEnabled();



  bool EnableEncryption(tBTM_SEC_CALLBACK* callback);

  void EnableEncryption();



  bool EnqueueInitialRequests(tGATT_IF gatt_if, GATT_READ_OP_CB chrc_read_cb,

                              GATT_WRITE_OP_CB cccd_write_cb);

  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

+43 −28
Original line number Diff line number Diff line
@@ -145,9 +145,7 @@ class VolumeControlImpl : public VolumeControl { 
      return;

    }



    if (!device->EnableEncryption(enc_callback_static)) {

      device_cleanup_helper(device, device->connecting_actively);

    }

    device->EnableEncryption();

  }



  void OnEncryptionComplete(const RawAddress& address, uint8_t success) {
@@ -389,7 +387,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 +464,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 +489,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 +524,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 +598,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 +782,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 +806,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 +847,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 +871,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 +984,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
@@ -1098,11 +1118,6 @@ class VolumeControlImpl : public VolumeControl { 
    if (instance) instance->gattc_callback(event, p_data);

  }



  static void enc_callback_static(const RawAddress* address, tBT_TRANSPORT,

                                  void*, tBTM_STATUS status) {

    if (instance) instance->OnEncryptionComplete(*address, status);

  }



  static void chrc_read_callback_static(uint16_t conn_id, tGATT_STATUS status,

                                        uint16_t handle, uint16_t len,

                                        uint8_t* value, void* data) {

system/bta/vc/vc_test.cc

+69 −15
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_);
@@ -438,19 +441,32 @@ class VolumeControlTest : public ::testing::Test { 
    gatt_callback(BTA_GATTC_SEARCH_CMPL_EVT, (tBTA_GATTC*)&event_data);

  }



  void GetEncryptionCompleteEvt(const RawAddress& bda) {

    tBTA_GATTC cb_data{};



    cb_data.enc_cmpl.client_if = gatt_if;

    cb_data.enc_cmpl.remote_bda = bda;

    gatt_callback(BTA_GATTC_ENC_CMPL_CB_EVT, &cb_data);

  }



  void SetEncryptionResult(const RawAddress& address, bool success) {

    ON_CALL(btm_interface, BTM_IsEncrypted(address, _))

        .WillByDefault(DoAll(Return(false)));

    EXPECT_CALL(btm_interface,

                SetEncryption(address, _, NotNull(), _, BTM_BLE_SEC_ENCRYPT))

        .WillOnce(Invoke(

            [&success](const RawAddress& bd_addr, tBT_TRANSPORT transport,

    ON_CALL(btm_interface, SetEncryption(address, _, _, _, BTM_BLE_SEC_ENCRYPT))

        .WillByDefault(Invoke(

            [&success, this](const RawAddress& bd_addr, tBT_TRANSPORT transport,

                             tBTM_SEC_CALLBACK* p_callback, void* p_ref_data,

                             tBTM_BLE_SEC_ACT sec_act) -> tBTM_STATUS {

              if (p_callback) {

                p_callback(&bd_addr, transport, p_ref_data,

                           success ? BTM_SUCCESS : BTM_FAILED_ON_SECURITY);

              }

              GetEncryptionCompleteEvt(bd_addr);

              return BTM_SUCCESS;

            }));

    EXPECT_CALL(btm_interface,

                SetEncryption(address, _, _, _, BTM_BLE_SEC_ENCRYPT))

        .Times(1);

  }



  void SetSampleDatabaseVCS(uint16_t conn_id) {
@@ -1027,13 +1043,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 +1066,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 +1089,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 +1130,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));