From 2791bcf9cf83fe0c6b98b5838e9b4e21197aa7ad Mon Sep 17 00:00:00 2001
From: Jakub Pawlowski <jpawlowski@google.com>
Date: Mon, 27 Nov 2023 20:48:04 +0100
Subject: [PATCH] Separate service discovery and read remote name callback
Long time ago someone decided that using one callback for service
search and device search is good idea.
Turns out it's a bad idea that's causing service discovery callback when
name read callback comes back. This, in turn, causes profiles to not
connect properly.
To fix this, separate event is introduced for name read.
Test: atest net_test_btif_stack
Bug: 311217960
Change-Id: I343a585ffcb7b6112b493af1593beca871d70c91
---
system/bta/dm/bta_dm_disc.cc | 2 +-
system/bta/include/bta_api.h | 4 +++-
system/bta/test/bta_dm_test.cc | 1 +
system/btif/src/btif_dm.cc | 13 +++++++++----
system/btif/src/btif_util.cc | 1 +
system/btif/test/btif_core_test.cc | 1 +
6 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/system/bta/dm/bta_dm_disc.cc b/system/bta/dm/bta_dm_disc.cc
index 65314bc6d25..d403cd43759 100644
--- a/system/bta/dm/bta_dm_disc.cc
+++ b/system/bta/dm/bta_dm_disc.cc
@@ -494,7 +494,7 @@ static void bta_dm_remote_name_cmpl(const tBTA_DM_MSG* p_data) {
if (remote_name_msg.hci_status == HCI_SUCCESS) {
bd_name_copy(search_data.disc_res.bd_name, remote_name_msg.bd_name);
}
- bta_dm_search_cb.p_search_cback(BTA_DM_DISC_RES_EVT, &search_data);
+ bta_dm_search_cb.p_search_cback(BTA_DM_NAME_READ_EVT, &search_data);
} else {
LOG_WARN("Received remote name complete without callback");
}
diff --git a/system/bta/include/bta_api.h b/system/bta/include/bta_api.h
index 2f559f746b0..3613a95966c 100644
--- a/system/bta/include/bta_api.h
+++ b/system/bta/include/bta_api.h
@@ -217,13 +217,14 @@ typedef void(tBTA_DM_ACL_CBACK)(tBTA_DM_ACL_EVT event, tBTA_DM_ACL* p_data);
typedef enum : uint8_t {
BTA_DM_INQ_RES_EVT = 0, /* Inquiry result for a peer device. */
BTA_DM_INQ_CMPL_EVT = 1, /* Inquiry complete. */
- BTA_DM_DISC_RES_EVT = 2, /* Discovery result for a peer device. */
+ BTA_DM_DISC_RES_EVT = 2, /* Service Discovery result for a peer device. */
BTA_DM_GATT_OVER_LE_RES_EVT =
3, /* GATT services over LE transport discovered */
BTA_DM_DISC_CMPL_EVT = 4, /* Discovery complete. */
BTA_DM_SEARCH_CANCEL_CMPL_EVT = 5, /* Search cancelled */
BTA_DM_DID_RES_EVT = 6, /* Vendor/Product ID search result */
BTA_DM_GATT_OVER_SDP_RES_EVT = 7, /* GATT services over SDP discovered */
+ BTA_DM_NAME_READ_EVT = 8, /* Name read complete. */
} tBTA_DM_SEARCH_EVT;
#ifndef CASE_RETURN_TEXT
@@ -242,6 +243,7 @@ inline std::string bta_dm_search_evt_text(const tBTA_DM_SEARCH_EVT& event) {
CASE_RETURN_TEXT(BTA_DM_SEARCH_CANCEL_CMPL_EVT);
CASE_RETURN_TEXT(BTA_DM_DID_RES_EVT);
CASE_RETURN_TEXT(BTA_DM_GATT_OVER_SDP_RES_EVT);
+ CASE_RETURN_TEXT(BTA_DM_NAME_READ_EVT);
default:
return base::StringPrintf("UNKNOWN[%hhu]", event);
}
diff --git a/system/bta/test/bta_dm_test.cc b/system/bta/test/bta_dm_test.cc
index 7c943752348..64453338ee2 100644
--- a/system/bta/test/bta_dm_test.cc
+++ b/system/bta/test/bta_dm_test.cc
@@ -546,6 +546,7 @@ TEST_F(BtaDmTest, bta_dm_search_evt_text) {
std::make_pair(BTA_DM_DID_RES_EVT, "BTA_DM_DID_RES_EVT"),
std::make_pair(BTA_DM_GATT_OVER_SDP_RES_EVT,
"BTA_DM_GATT_OVER_SDP_RES_EVT"),
+ std::make_pair(BTA_DM_NAME_READ_EVT, "BTA_DM_NAME_READ_EVT"),
};
for (const auto& event : events) {
ASSERT_STREQ(event.second.c_str(),
diff --git a/system/btif/src/btif_dm.cc b/system/btif/src/btif_dm.cc
index 6ade654d291..7ef6740f006 100644
--- a/system/btif/src/btif_dm.cc
+++ b/system/btif/src/btif_dm.cc
@@ -1374,7 +1374,7 @@ static void btif_dm_search_devices_evt(tBTA_DM_SEARCH_EVT event,
LOG_VERBOSE("%s event=%s", __func__, dump_dm_search_event(event));
switch (event) {
- case BTA_DM_DISC_RES_EVT: {
+ case BTA_DM_NAME_READ_EVT: {
/* Remote name update */
if (strlen((const char*)p_search_data->disc_res.bd_name)) {
/** Fix inquiry time too long @{ */
@@ -1399,10 +1399,10 @@ static void btif_dm_search_devices_evt(tBTA_DM_SEARCH_EVT event,
BTIF_STORAGE_FILL_PROPERTY(&properties[2], BT_PROPERTY_CLASS_OF_DEVICE, sizeof(uint32_t), &cod);
if (btif_storage_get_remote_device_property(
&bdaddr, &properties[2]) == BT_STATUS_SUCCESS) {
- LOG_VERBOSE("%s, BTA_DM_DISC_RES_EVT, cod in storage = 0x%08x",
+ LOG_VERBOSE("%s, BTA_DM_NAME_READ_EVT, cod in storage = 0x%08x",
__func__, cod);
} else {
- LOG_VERBOSE("%s, BTA_DM_DISC_RES_EVT, no cod in storage", __func__);
+ LOG_VERBOSE("%s, BTA_DM_NAME_READ_EVT, no cod in storage", __func__);
cod = 0;
}
if (cod != 0) {
@@ -2025,6 +2025,10 @@ static void btif_dm_search_services_evt(tBTA_DM_SEARCH_EVT event,
BT_STATUS_SUCCESS, bd_addr, 1, &prop_did);
} break;
+ case BTA_DM_NAME_READ_EVT: {
+ LOG_INFO("Skipping name read event - called on bad callback.");
+ } break;
+
default: {
ASSERTC(0, "unhandled search services event", event);
} break;
@@ -3018,7 +3022,8 @@ bt_status_t btif_dm_get_adapter_property(bt_property_t* prop) {
*
******************************************************************************/
void btif_dm_get_remote_services(RawAddress remote_addr, const int transport) {
- LOG_VERBOSE("%s: transport=%d, remote_addr=%s", __func__, transport,
+ LOG_VERBOSE("%s: transport=%s, remote_addr=%s", __func__,
+ bt_transport_text(transport).c_str(),
ADDRESS_TO_LOGGABLE_CSTR(remote_addr));
BTM_LogHistory(
diff --git a/system/btif/src/btif_util.cc b/system/btif/src/btif_util.cc
index 66711d550ca..874e55561bb 100644
--- a/system/btif/src/btif_util.cc
+++ b/system/btif/src/btif_util.cc
@@ -114,6 +114,7 @@ const char* dump_dm_search_event(uint16_t event) {
CASE_RETURN_STR(BTA_DM_DISC_CMPL_EVT)
CASE_RETURN_STR(BTA_DM_SEARCH_CANCEL_CMPL_EVT)
CASE_RETURN_STR(BTA_DM_GATT_OVER_SDP_RES_EVT)
+ CASE_RETURN_STR(BTA_DM_NAME_READ_EVT)
default:
return "UNKNOWN MSG ID";
diff --git a/system/btif/test/btif_core_test.cc b/system/btif/test/btif_core_test.cc
index ea13ac04774..e08e43f4d9c 100644
--- a/system/btif/test/btif_core_test.cc
+++ b/system/btif/test/btif_core_test.cc
@@ -267,6 +267,7 @@ TEST_F(BtifCoreTest, dump_dm_search_event) {
"BTA_DM_SEARCH_CANCEL_CMPL_EVT"),
std::make_pair(BTA_DM_GATT_OVER_SDP_RES_EVT,
"BTA_DM_GATT_OVER_SDP_RES_EVT"),
+ std::make_pair(BTA_DM_NAME_READ_EVT, "BTA_DM_NAME_READ_EVT"),
};
for (const auto& event : events) {
ASSERT_STREQ(event.second.c_str(), dump_dm_search_event(event.first));
--
GitLab