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
diff --git a/system/bta/vc/devices.h b/system/bta/vc/devices.h
index dcf22b0..02b3543 100644
--- a/system/bta/vc/devices.h
+++ b/system/bta/vc/devices.h
@@ -137,6 +137,7 @@
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:
/*
diff --git a/system/bta/vc/vc.cc b/system/bta/vc/vc.cc
index 1d406a9..9862196 100644
--- a/system/bta/vc/vc.cc
+++ b/system/bta/vc/vc.cc
@@ -389,7 +389,11 @@
<< 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 @@
<< " 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 @@
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 @@
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 @@
}
// 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 @@
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()) {
- std::vector<RawAddress> devices = {dev->address};
- PrepareVolumeControlOperation(devices, bluetooth::groups::kGroupUnknown,
- false, opcode, arg);
+ 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);
+ }
}
} else {
/* Handle group change */
@@ -790,7 +808,7 @@
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 @@
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);
+ 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);
+ }
}
} else {
/* Handle group change */
@@ -851,7 +873,7 @@
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 @@
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
diff --git a/system/bta/vc/vc_test.cc b/system/bta/vc/vc_test.cc
index 71fca36..3c266ee 100644
--- a/system/bta/vc/vc_test.cc
+++ b/system/bta/vc/vc_test.cc
@@ -229,12 +229,15 @@
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 @@
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 @@
};
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 @@
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_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));
