Merge "VolumeControl: Fix multi-read request" into main (bb39f4b9) · Commits · e / os / android_packages_modules_Bluetooth

system/bta/Android.bp

+1 −0

Original line number	Diff line number	Diff line
		@@ -635,6 +635,7 @@ cc_test {
		srcs: [
		":TestCommonMockFunctions",
		":TestMockStackBtmInterface",
		":TestMockStackGatt",
		"gatt/database.cc",
		"gatt/database_builder.cc",
		"test/common/bta_gatt_api_mock.cc",

system/bta/vc/device.cc

+52 −32

Original line number	Diff line number	Diff line
		@@ -36,6 +36,7 @@
		#include "gatt/database.h"
		#include "gattdefs.h"
		#include "stack/btm/btm_sec.h"
		#include "stack/gatt/gatt_int.h"
		#include "stack/include/bt_types.h"
		#include "stack/include/gatt_api.h"
		#include "types/bluetooth/uuid.h"
		@@ -421,49 +422,68 @@ void VolumeControlDevice::EnqueueRemainingRequests(tGATT_IF /gatt_if/,
		GATT_READ_OP_CB chrc_read_cb,
		GATT_READ_MULTI_OP_CB chrc_multi_read_cb,
		GATT_WRITE_OP_CB /cccd_write_cb/) {
		std::vector<uint16_t> handles_to_read;
		const auto is_eatt_supported = gatt_profile_get_eatt_support_by_conn_id(connection_id);

		for (auto const& input : audio_inputs.volume_audio_inputs) {
		handles_to_read.push_back(input.state_handle);
		handles_to_read.push_back(input.gain_setting_handle);
		handles_to_read.push_back(input.type_handle);
		handles_to_read.push_back(input.status_handle);
		handles_to_read.push_back(input.description_handle);
		}
		/* List of handles to the attributes having known and fixed-size values to read using the
		* ATT_READ_MULTIPLE_REQ. The `.second` component contains 1 octet for the length + the actual
		* attribute value length, exactly as in the received HCI packet for ATT_READ_MULTIPLE_RSP.
		* We use this to make sure the request response will fit the current MTU size.
		*/
		std::list<std::pair<uint16_t, size_t>> handles_to_read;

		/* Variable-length attributes - always read using the regular read requests to automatically
		* handle truncation in the GATT layer if MTU is to small to fit even a single complete value.
		*/
		std::vector<uint16_t> handles_to_read_variable_length;

		for (auto const& offset : audio_offsets.volume_offsets) {
		handles_to_read.push_back(offset.state_handle);
		handles_to_read.push_back(offset.audio_location_handle);
		handles_to_read.push_back(offset.audio_descr_handle);
		handles_to_read.push_back(std::make_pair(offset.state_handle, 4));
		handles_to_read.push_back(std::make_pair(offset.audio_location_handle, 5));
		handles_to_read_variable_length.push_back(offset.audio_descr_handle);
		}

		log::debug("{}, number of handles={}", address, handles_to_read.size());

		if (!com::android::bluetooth::flags::le_ase_read_multiple_variable()) {
		for (auto const& handle : handles_to_read) {
		BtaGattQueue::ReadCharacteristic(connection_id, handle, chrc_read_cb, nullptr);
		}
		return;
		for (auto const& input : audio_inputs.volume_audio_inputs) {
		handles_to_read.push_back(std::make_pair(input.state_handle, 5));
		handles_to_read.push_back(std::make_pair(input.gain_setting_handle, 4));
		handles_to_read.push_back(std::make_pair(input.type_handle, 2));
		handles_to_read.push_back(std::make_pair(input.status_handle, 2));
		handles_to_read_variable_length.push_back(input.description_handle);
		}

		size_t sent_cnt = 0;
		log::debug("{}, number of fixed-size attribute handles={}", address, handles_to_read.size());
		log::debug("{}, number of variable-size attribute handles={}", address,
		handles_to_read_variable_length.size());

		while (sent_cnt < handles_to_read.size()) {
		tBTA_GATTC_MULTI multi_read{};
		size_t remain_cnt = (handles_to_read.size() - sent_cnt);
		if (com::android::bluetooth::flags::le_ase_read_multiple_variable() && is_eatt_supported) {
		const size_t payload_limit = this->mtu_ - 1;

		multi_read.num_attr =
		remain_cnt > GATT_MAX_READ_MULTI_HANDLES ? GATT_MAX_READ_MULTI_HANDLES : remain_cnt;
		auto pair_it = handles_to_read.begin();
		while (pair_it != handles_to_read.end()) {
		tBTA_GATTC_MULTI multi_read{.num_attr = 0};
		size_t size_limit = 0;

		auto handles_begin = handles_to_read.begin() + sent_cnt;
		std::copy(handles_begin, handles_begin + multi_read.num_attr, multi_read.handles);

		sent_cnt += multi_read.num_attr;
		log::debug{"{}, calling multi with {} attributes, sent_cnt {} ", address, multi_read.num_attr,
		sent_cnt};
		// Send at once just enough attributes to stay below the MTU size limit for the response
		while ((pair_it != handles_to_read.end()) && (size_limit + pair_it->second < payload_limit) &&
		(multi_read.num_attr < GATT_MAX_READ_MULTI_HANDLES)) {
		multi_read.handles[multi_read.num_attr] = pair_it->first;
		size_limit += pair_it->second;
		++multi_read.num_attr;
		++pair_it;
		}

		log::debug{"{}, calling multi-read with {} attributes, {} left", address, multi_read.num_attr,
		std::distance(pair_it, handles_to_read.end())};
		BtaGattQueue::ReadMultiCharacteristic(connection_id, multi_read, chrc_multi_read_cb, nullptr);
		}
		} else {
		for (auto const& [handle, _] : handles_to_read) {
		BtaGattQueue::ReadCharacteristic(connection_id, handle, chrc_read_cb, nullptr);
		}
		}

		for (auto const& handle : handles_to_read_variable_length) {
		BtaGattQueue::ReadCharacteristic(connection_id, handle, chrc_read_cb, nullptr);
		}
		}

		bool VolumeControlDevice::VerifyReady(uint16_t handle) {

system/bta/vc/devices.h

+1 −0

Original line number	Diff line number	Diff line
		@@ -48,6 +48,7 @@ public:
		uint8_t flags;

		tCONN_ID connection_id;
		uint16_t mtu_ = GATT_DEF_BLE_MTU_SIZE;

		/* Volume Control Service */
		uint16_t volume_state_handle;

system/bta/vc/devices_test.cc

+40 −10

Original line number	Diff line number	Diff line
		@@ -692,25 +692,39 @@ TEST_F(VolumeControlDeviceTest, test_enqueue_remaining_requests_multiread) {
		tGATT_IF gatt_if = 0x0001;
		std::vector<uint8_t> register_for_notification_data({0x01, 0x00});

		// The amount of attributes read at once is limited by the MTU size - 1 (here 22)
		tBTA_GATTC_MULTI expected_to_read_part_1 = {
		.num_attr = 10,
		.num_attr = 4,
		.handles = {0x0022 /* audio input state 1 /, 0x0025 / gain setting properties 1 */,
		0x0027 /* audio input type 1 /, 0x0029 / audio input status 1 */,
		0x002e /* audio input description 1 /, 0x0042 / audio input state 2 */,
		0x0045 /* gain setting properties 2 /, 0x0047 / audio input type 2 */,
		0x0049 /* audio input status 2 /, 0x004e / audio input description 2 */},
		0x0027 /* audio input type 1 /, 0x0029 / audio input status 1 */},
		};

		tBTA_GATTC_MULTI expected_to_read_part_2 = {
		.num_attr = 6,
		.handles = {0x0062 /* audio output state 1 /, 0x0065 / audio output location 1 */,
		0x0069 /* audio output description 1 /, 0x0082 / audio output state 1 */,
		0x0085 /* audio output location 1 */,
		0x008a /* audio output description 1 */},
		.num_attr = 5,
		.handles = {0x0042 /* audio input state 2 /, 0x0045 / gain setting properties 2 */,
		0x0047 /* audio input type 2 /, 0x0049 / audio input status 2 */,
		0x0062 /* audio output state 1 */},
		};

		tBTA_GATTC_MULTI expected_to_read_part_3 = {
		.num_attr = 3,
		.handles = {0x0065 /* audio output location 1 /, 0x0082 / audio output state 1 */,
		0x0085 /* audio output location 1 */},
		};

		uint16_t expected_audio_input_description_1 = 0x002e;
		uint16_t expected_audio_input_description_2 = 0x004e;
		uint16_t expected_audio_output_description_1 = 0x0069;
		uint16_t expected_audio_output_description_2 = 0x008a;

		tBTA_GATTC_MULTI received_to_read_part_1{};
		tBTA_GATTC_MULTI received_to_read_part_2{};
		tBTA_GATTC_MULTI received_to_read_part_3{};

		uint16_t audio_input_description_1 = 0;
		uint16_t audio_input_description_2 = 0;
		uint16_t audio_output_description_1 = 0;
		uint16_t audio_output_description_2 = 0;

		{
		testing::InSequence s;
		@@ -719,6 +733,16 @@ TEST_F(VolumeControlDeviceTest, test_enqueue_remaining_requests_multiread) {
		.WillOnce(SaveArg<1>(&received_to_read_part_1));
		EXPECT_CALL(gatt_queue, ReadMultiCharacteristic(_, _, _, _))
		.WillOnce(SaveArg<1>(&received_to_read_part_2));
		EXPECT_CALL(gatt_queue, ReadMultiCharacteristic(_, _, _, _))
		.WillOnce(SaveArg<1>(&received_to_read_part_3));
		EXPECT_CALL(gatt_queue, ReadCharacteristic(_, _, _, _))
		.WillOnce(SaveArg<1>(&audio_output_description_1));
		EXPECT_CALL(gatt_queue, ReadCharacteristic(_, _, _, _))
		.WillOnce(SaveArg<1>(&audio_output_description_2));
		EXPECT_CALL(gatt_queue, ReadCharacteristic(_, _, _, _))
		.WillOnce(SaveArg<1>(&audio_input_description_1));
		EXPECT_CALL(gatt_queue, ReadCharacteristic(_, _, _, _))
		.WillOnce(SaveArg<1>(&audio_input_description_2));
		}
		EXPECT_CALL(gatt_queue, WriteDescriptor(_, _, _, GATT_WRITE, _, _)).Times(0);
		EXPECT_CALL(gatt_interface, RegisterForNotifications(_, _, _)).Times(0);
		@@ -738,6 +762,12 @@ TEST_F(VolumeControlDeviceTest, test_enqueue_remaining_requests_multiread) {

		ASSERT_EQ(expected_to_read_part_1.num_attr, received_to_read_part_1.num_attr);
		ASSERT_EQ(expected_to_read_part_2.num_attr, received_to_read_part_2.num_attr);
		ASSERT_EQ(expected_to_read_part_3.num_attr, received_to_read_part_3.num_attr);

		EXPECT_EQ(expected_audio_input_description_1, audio_input_description_1);
		EXPECT_EQ(expected_audio_input_description_2, audio_input_description_2);
		EXPECT_EQ(expected_audio_output_description_1, audio_output_description_1);
		EXPECT_EQ(expected_audio_output_description_2, audio_output_description_2);
		}

		TEST_F(VolumeControlDeviceTest, test_check_link_encrypted) {

system/bta/vc/vc.cc

+18 −4

Original line number	Diff line number	Diff line
		@@ -176,7 +176,7 @@ public:
		}

		void OnGattConnected(tGATT_STATUS status, tCONN_ID connection_id, tGATT_IF /client_if/,
		RawAddress address, tBT_TRANSPORT transport, uint16_t /mtu/) {
		RawAddress address, tBT_TRANSPORT transport, uint16_t mtu) {
		bluetooth::log::info("{}, conn_id=0x{:04x}, transport={}, status={}(0x{:02x})", address,
		connection_id, bt_transport_text(transport), gatt_status_text(status),
		status);
		@@ -202,6 +202,7 @@ public:
		}

		device->connection_id = connection_id;
		device->mtu_ = mtu;

		/* Make sure to remove device from background connect.
		* It will be added back if needed, when device got disconnected
		@@ -275,6 +276,15 @@ public:
		ClearDeviceInformationAndStartSearch(device);
		}

		void OnMtuChanged(tCONN_ID conn_id, uint16_t mtu) {
		VolumeControlDevice* device = volume_control_devices_.FindByConnId(conn_id);
		if (!device) {
		bluetooth::log::error("Skipping unknown device conn_id: {}", conn_id);
		return;
		}
		device->mtu_ = mtu;
		}

		void OnServiceDiscDoneEvent(const RawAddress& address) {
		VolumeControlDevice* device = volume_control_devices_.FindByAddress(address);
		if (!device) {
		@@ -1461,6 +1471,7 @@ private:
		device->Disconnect(gatt_if_);

		RemoveDeviceFromOperationList(device->address);
		device->mtu_ = GATT_DEF_BLE_MTU_SIZE;

		if (notify) {
		callbacks_->OnConnectionState(ConnectionState::DISCONNECTED, device->address);
		@@ -1545,6 +1556,10 @@ private:
		OnServiceChangeEvent(p_data->service_changed.remote_bda);
		break;

		case BTA_GATTC_CFG_MTU_EVT:
		OnMtuChanged(p_data->cfg_mtu.conn_id, p_data->cfg_mtu.mtu);
		break;

		case BTA_GATTC_SRVC_DISC_DONE_EVT:
		OnServiceDiscDoneEvent(p_data->service_discovery_done.remote_bda);
		break;
		@@ -1599,14 +1614,13 @@ private:
		instance->OnCharacteristicValueChanged(conn_id, status, hdl, len, ptr,
		((index == (handles.num_attr - 1)) ? data : nullptr),
		false);

		position += len + 2; /* skip the length of data */
		index++;
		}

		if (handles.num_attr - 1 != index) {
		if (handles.num_attr != index) {
		bluetooth::log::warn("Attempted to read {} handles, but received just {} values",
		+handles.num_attr, index + 1);
		+handles.num_attr, index);
		}
		}
		};