Merge "Skip service discovery if robust caching is unavailable" am: 5ab1764930... (98b2c5ad) · Commits · e / os / android_packages_modules_Bluetooth

android/pandora/test/gatt_test.py

+79 −17

Original line number	Diff line number	Diff line
		@@ -83,14 +83,7 @@ class GattTest(base_test.BaseTestClass): # type: ignore[misc]
		services = gatt.DiscoverServices(ref_dut)
		self.ref.log.info(f'REF services: {services}')

		@avatar.asynchronous
		async def test_read_characteristic_while_pairing(self) -> None:
		if isinstance(self.dut, BumblePandoraDevice):
		raise signals.TestSkip('TODO: b/273941061')
		if not isinstance(self.ref, BumblePandoraDevice):
		raise signals.TestSkip('Test require Bumble as reference device(s)')

		async def connect_dut_to_ref() -> Tuple[Connection, Connection]:
		async def connect_dut_to_ref(self) -> Tuple[Connection, Connection]:
		ref_advertisement = self.ref.aio.host.Advertise(
		legacy=True,
		connectable=True,
		@@ -106,6 +99,13 @@ class GattTest(base_test.BaseTestClass): # type: ignore[misc]

		return dut_connection_to_ref, ref_connection_to_dut

		@avatar.asynchronous
		async def test_read_characteristic_while_pairing(self) -> None:
		if isinstance(self.dut, BumblePandoraDevice):
		raise signals.TestSkip('TODO: b/273941061')
		if not isinstance(self.ref, BumblePandoraDevice):
		raise signals.TestSkip('Test require Bumble as reference device(s)')

		# arrange: set up GATT service on REF side with a characteristic
		# that can only be read after pairing
		SERVICE_UUID = "00005a00-0000-1000-8000-00805f9b34fb"
		@@ -129,7 +129,7 @@ class GattTest(base_test.BaseTestClass): # type: ignore[misc]
		# manually handle pairing on the DUT side
		dut_pairing_events = self.dut.aio.security.OnPairing()
		# set up connection
		dut_connection_to_ref, ref_connection_to_dut = await connect_dut_to_ref()
		dut_connection_to_ref, ref_connection_to_dut = await self.connect_dut_to_ref()

		# act: initiate pairing from REF side (send a security request)
		async def ref_secure() -> SecureResponse:
		@@ -167,6 +167,68 @@ class GattTest(base_test.BaseTestClass): # type: ignore[misc]
		ref_secure_res = await ref_secure_task
		assert ref_secure_res.result_variant() == 'success'

		@avatar.asynchronous
		async def test_rediscover_whenever_unbonded(self) -> None:
		if not isinstance(self.ref, BumblePandoraDevice):
		raise signals.TestSkip('Test require Bumble as reference device(s)')

		# arrange: set up one GATT service on REF side
		dut_gatt = AioGATT(self.dut.aio.channel)
		SERVICE_UUID_1 = "00005a00-0000-1000-8000-00805f9b34fb"
		SERVICE_UUID_2 = "00005a01-0000-1000-8000-00805f9b34fb"
		self.ref.device.add_service(Service(SERVICE_UUID_1, [])) # type:ignore
		# connect both devices
		dut_connection_to_ref, ref_connection_to_dut = await self.connect_dut_to_ref()

		# act: perform service discovery, disconnect, add the second service, reconnect, and try discovery again
		first_discovery = await dut_gatt.DiscoverServices(dut_connection_to_ref)
		await self.ref.aio.host.Disconnect(ref_connection_to_dut)
		self.ref.device.add_service(Service(SERVICE_UUID_2, [])) # type:ignore
		dut_connection_to_ref, _ = await self.connect_dut_to_ref()
		second_discovery = await dut_gatt.DiscoverServices(dut_connection_to_ref)

		# assert: that we found only one service in the first discovery
		assert any(service.uuid == SERVICE_UUID_1 for service in first_discovery.services)
		assert not any(service.uuid == SERVICE_UUID_2 for service in first_discovery.services)
		# assert: but found both in the second discovery
		assert any(service.uuid == SERVICE_UUID_1 for service in second_discovery.services)
		assert any(service.uuid == SERVICE_UUID_2 for service in second_discovery.services)

		@avatar.asynchronous
		async def test_do_not_discover_when_bonded(self) -> None:
		# NOTE: if service change indication is ever enabled in Bumble, both this test + the previous test must DISABLE IT
		# otherwise this test will fail, and the previous test will pass even on a broken implementation

		if not isinstance(self.ref, BumblePandoraDevice):
		raise signals.TestSkip('Test require Bumble as reference device(s)')

		# arrange: set up one GATT service on REF side
		dut_gatt = AioGATT(self.dut.aio.channel)
		SERVICE_UUID_1 = "00005a00-0000-1000-8000-00805f9b34fb"
		SERVICE_UUID_2 = "00005a01-0000-1000-8000-00805f9b34fb"
		self.ref.device.add_service(Service(SERVICE_UUID_1, [])) # type:ignore
		# connect both devices
		dut_connection_to_ref, ref_connection_to_dut = await self.connect_dut_to_ref()
		# bond devices and disconnect
		await self.dut.aio.security.Secure(connection=dut_connection_to_ref, le=LE_LEVEL3)
		await self.ref.aio.host.Disconnect(ref_connection_to_dut)

		# act: connect, perform service discovery, disconnect, add the second service, reconnect, and try discovery again
		dut_connection_to_ref, ref_connection_to_dut = await self.connect_dut_to_ref()
		first_discovery = await dut_gatt.DiscoverServices(dut_connection_to_ref)
		await self.ref.aio.host.Disconnect(ref_connection_to_dut)

		self.ref.device.add_service(Service(SERVICE_UUID_2, [])) # type:ignore
		dut_connection_to_ref, _ = await self.connect_dut_to_ref()
		second_discovery = await dut_gatt.DiscoverServices(dut_connection_to_ref)

		# assert: that we found only one service in the first discovery
		assert any(service.uuid == SERVICE_UUID_1 for service in first_discovery.services)
		assert not any(service.uuid == SERVICE_UUID_2 for service in first_discovery.services)
		# assert: but found both in the second discovery
		assert any(service.uuid == SERVICE_UUID_1 for service in second_discovery.services)
		assert any(service.uuid == SERVICE_UUID_2 for service in second_discovery.services)


		if __name__ == '__main__':
		logging.basicConfig(level=logging.DEBUG)

system/bta/gatt/bta_gattc_act.cc

+26 −37

Original line number	Diff line number	Diff line
		@@ -548,14 +548,24 @@ void bta_gattc_conn(tBTA_GATTC_CLCB* p_clcb, const tBTA_GATTC_DATA* p_data) {
		p_clcb->p_srcb->state = BTA_GATTC_SERV_LOAD;
		// Consider the case that if GATT Server is changed, but no service
		// changed indication is received, the database might be out of date. So
		// if robust caching is enabled, any time when connection is established,
		// always check the db hash first, not just load the stored database.
		gatt::Database db = bta_gattc_cache_load(p_clcb->p_srcb->server_bda);
		if (!bta_gattc_is_robust_caching_enabled() && !db.IsEmpty()) {
		p_clcb->p_srcb->gatt_database = db;
		p_clcb->p_srcb->state = BTA_GATTC_SERV_IDLE;
		bta_gattc_reset_discover_st(p_clcb->p_srcb, GATT_SUCCESS);
		} else {
		// if robust caching is known to be supported, always check the db hash
		// first, before loading the stored database.

		// Only load the database if we are bonded, since the device cache is
		// meaningless otherwise (as we need to do rediscovery regardless)
		gatt::Database db = btm_sec_is_a_bonded_dev(p_clcb->bda)
		? bta_gattc_cache_load(p_clcb->p_srcb->server_bda)
		: gatt::Database();
		auto robust_caching_support = GetRobustCachingSupport(p_clcb, db);
		LOG_INFO("Connected to %s, robust caching support is %d",
		p_clcb->bda.ToRedactedStringForLogging().c_str(),
		robust_caching_support);
		if (db.IsEmpty() \|\|
		robust_caching_support == RobustCachingSupport::SUPPORTED) {
		// If the peer device is expected to support robust caching, or if we
		// don't know its services yet, then we should do discovery (which may
		// short-circuit through a hash match, but might also do the full
		// discovery).
		p_clcb->p_srcb->state = BTA_GATTC_SERV_DISC;

		/* set true to read database hash before service discovery */
		@@ -565,6 +575,10 @@ void bta_gattc_conn(tBTA_GATTC_CLCB* p_clcb, const tBTA_GATTC_DATA* p_data) {

		/* cache load failure, start discovery */
		bta_gattc_start_discover(p_clcb, NULL);
		} else {
		p_clcb->p_srcb->gatt_database = db;
		p_clcb->p_srcb->state = BTA_GATTC_SERV_IDLE;
		bta_gattc_reset_discover_st(p_clcb->p_srcb, GATT_SUCCESS);
		}
		} else /* cache is building */
		p_clcb->state = BTA_GATTC_DISCOVER_ST;
		@@ -832,35 +846,10 @@ void bta_gattc_start_discover(tBTA_GATTC_CLCB* p_clcb,
		p_clcb->p_srcb->update_count = 0;
		p_clcb->p_srcb->state = BTA_GATTC_SERV_DISC_ACT;

		/* This is workaround for the embedded devices being already on the market
		* and having a serious problem with handle Read By Type with
		* GATT_UUID_DATABASE_HASH. With this workaround, Android will assume that
		* embedded device having LMP version lower than 5.1 (0x0a), it does not
		* support GATT Caching.
		*/
		uint8_t lmp_version = 0;
		if (!BTM_ReadRemoteVersion(p_clcb->bda, &lmp_version, nullptr, nullptr)) {
		LOG_WARN("Could not read remote version for %s",
		ADDRESS_TO_LOGGABLE_CSTR(p_clcb->bda));
		}

		if (lmp_version < 0x0a) {
		LOG_WARN(
		" Device LMP version 0x%02x < Bluetooth 5.1. Ignore database cache "
		"read.",
		lmp_version);
		p_clcb->p_srcb->srvc_hdl_db_hash = false;
		}

		// Some LMP 5.2 devices also don't support robust caching. This workaround
		// conditionally disables the feature based on a combination of LMP
		// version and OUI prefix.
		if (lmp_version < 0x0c &&
		interop_match_addr(INTEROP_DISABLE_ROBUST_CACHING, &p_clcb->bda)) {
		LOG_WARN(
		"Device LMP version 0x%02x <= Bluetooth 5.2 and MAC addr on "
		"interop list, skipping robust caching",
		lmp_version);
		if (GetRobustCachingSupport(p_clcb, p_clcb->p_srcb->gatt_database) ==
		RobustCachingSupport::UNSUPPORTED) {
		// Skip initial DB hash read if we have strong reason (due to interop,
		// or a prior discovery) to believe that it is unsupported.
		p_clcb->p_srcb->srvc_hdl_db_hash = false;
		}

system/bta/gatt/bta_gattc_cache.cc

+78 −0

Original line number	Diff line number	Diff line
		@@ -36,6 +36,7 @@
		#include "bt_target.h" // Must be first to define build configuration
		#include "bta/gatt/bta_gattc_int.h"
		#include "bta/gatt/database.h"
		#include "device/include/interop.h"
		#include "osi/include/allocator.h"
		#include "osi/include/log.h"
		#include "stack/btm/btm_sec.h"
		@@ -122,6 +123,80 @@ const Service* bta_gattc_find_matching_service(
		return nullptr;
		}

		/// Whether the peer device uses robust caching
		RobustCachingSupport GetRobustCachingSupport(const tBTA_GATTC_CLCB* p_clcb,
		const gatt::Database& db) {
		LOG_DEBUG("GetRobustCachingSupport %s",
		p_clcb->bda.ToRedactedStringForLogging().c_str());

		// If the feature is disabled, then we never support it
		if (!bta_gattc_is_robust_caching_enabled()) {
		LOG_DEBUG("robust caching is disabled, so UNSUPPORTED");
		return RobustCachingSupport::UNSUPPORTED;
		}

		// An empty database means that discovery hasn't taken place yet, so
		// we can't infer anything from that
		if (!db.IsEmpty()) {
		// Here, we can simply check whether the database hash is present
		for (const auto& service : db.Services()) {
		if (service.uuid.As16Bit() != UUID_SERVCLASS_GATT_SERVER) {
		continue;
		}
		for (const auto& characteristic : service.characteristics) {
		if (characteristic.uuid.As16Bit() == GATT_UUID_DATABASE_HASH) {
		// the hash was found, so we should read it
		LOG_DEBUG("database hash characteristic found, so SUPPORTED");
		return RobustCachingSupport::SUPPORTED;
		}
		}
		}

		// The database hash characteristic was not found, so there's no point
		// searching for it. Even if the hash was previously not present but is now,
		// we will still get the service changed indication, so there's no need to
		// speculatively check for the hash every time.
		LOG_DEBUG("database hash characteristic not found, so UNSUPPORTED");
		return RobustCachingSupport::UNSUPPORTED;
		}

		// This is workaround for the embedded devices being already on the market
		// and having a serious problem with handle Read By Type with
		// GATT_UUID_DATABASE_HASH. With this workaround, Android will assume that
		// embedded device having LMP version lower than 5.1 (0x0a), it does not
		// support GATT Caching.
		uint8_t lmp_version = 0;
		if (!BTM_ReadRemoteVersion(p_clcb->bda, &lmp_version, nullptr, nullptr)) {
		LOG_WARN("Could not read remote version for %s",
		ADDRESS_TO_LOGGABLE_CSTR(p_clcb->bda));
		}

		if (lmp_version < 0x0a) {
		LOG_WARN(
		" Device LMP version 0x%02x < Bluetooth 5.1. Ignore database cache "
		"read.",
		lmp_version);
		return RobustCachingSupport::UNSUPPORTED;
		}

		// Some LMP 5.2 devices also don't support robust caching. This workaround
		// conditionally disables the feature based on a combination of LMP
		// version and OUI prefix.
		if (lmp_version < 0x0c &&
		interop_match_addr(INTEROP_DISABLE_ROBUST_CACHING, &p_clcb->bda)) {
		LOG_WARN(
		"Device LMP version 0x%02x <= Bluetooth 5.2 and MAC addr on "
		"interop list, skipping robust caching",
		lmp_version);
		return RobustCachingSupport::UNSUPPORTED;
		}

		// If we have no cached database and no interop considerations,
		// it is unknown whether or not robust caching is supported
		LOG_DEBUG("database hash support is UNKNOWN");
		return RobustCachingSupport::UNKNOWN;
		}

		/** Start primary service discovery */
		tGATT_STATUS bta_gattc_discover_pri_service(uint16_t conn_id,
		tBTA_GATTC_SERV* p_server_cb,
		@@ -231,6 +306,9 @@ static void bta_gattc_explore_srvc_finished(uint16_t conn_id,

		// If the device is trusted, link the addr file to hash file
		if (success && btm_sec_is_a_bonded_dev(p_srvc_cb->server_bda)) {
		LOG_DEBUG(
		"Linking db hash to address %s",
		p_clcb->p_srcb->server_bda.ToRedactedStringForLogging().c_str());
		bta_gattc_cache_link(p_clcb->p_srcb->server_bda, hash);
		}

system/bta/gatt/bta_gattc_int.h

+9 −0

Original line number	Diff line number	Diff line
		@@ -496,6 +496,15 @@ extern void bta_gattc_get_gatt_db(uint16_t conn_id, uint16_t start_handle,
		uint16_t end_handle, btgatt_db_element_t** db,
		int* count);
		extern void bta_gattc_init_cache(tBTA_GATTC_SERV* p_srvc_cb);

		enum class RobustCachingSupport {
		UNSUPPORTED,
		SUPPORTED,
		UNKNOWN,
		};
		RobustCachingSupport GetRobustCachingSupport(const tBTA_GATTC_CLCB* p_clcb,
		const gatt::Database& db);

		extern void bta_gattc_reset_discover_st(tBTA_GATTC_SERV* p_srcb,
		tGATT_STATUS status);