From c48b62cd9c2713828e9d594f5e94646bafe7b9d3 Mon Sep 17 00:00:00 2001
From: Andrew Cheng <chengandrew@google.com>
Date: Fri, 29 Oct 2021 15:25:40 -0700
Subject: [PATCH] Report back pbap and map uuids to AdapterProperties in Java
It seems services were supposed to be enabled by {@link
btif_enable_service}, which would also automatically update the {@link
btif_enabled_services} mask. This "enabled services mask" is the basis
of the local UUIDs that is reported back up to Java (e.g., {@link
AdapterProperties.getUuids}).
However, it seems like only the HFP (server/client) and A2DP
(source/sink) profiles go through this codepath, but not PBAP or MAP
(server/client), which is why their UUIDs are not reported back to Java.
This CL adds calls to {@link btif_enable_service} when the SDP records
for PBAP/MAP (server/client) are created in order to update the "enabled
services mask". This is analogous to how their UUIDs are added to the
EIR when their SDP records are created.
Because these services were enabled elsewhere, the codepath following
the call to {@link btif_enable_service} needs to be modified to catch
the respective service IDs and no-op (instead of enabling a service).
Similar, the "enabled services mask" is updated when the respective SDP
record is removed.
Tag: #stability
Bug: 204597459
Test: atest BluetoothInstrumentationTests
Change-Id: I4c4f3db01c18b2db674ec639c2b714a3c4fec81b
---
system/bta/include/bta_api.h | 4 +++
system/btif/src/btif_dm.cc | 16 ++++++++++
system/btif/src/btif_sdp_server.cc | 49 ++++++++++++++++++++++++++++++
system/btif/src/btif_storage.cc | 16 ++++++++++
4 files changed, 85 insertions(+)
diff --git a/system/bta/include/bta_api.h b/system/bta/include/bta_api.h
index 7335609dae7..8ac26359c64 100644
--- a/system/bta/include/bta_api.h
+++ b/system/bta/include/bta_api.h
@@ -65,7 +65,11 @@ typedef enum : uint8_t {
#define BTA_BIP_SERVICE_ID 13 /* Basic Imaging profile */
#define BTA_A2DP_SINK_SERVICE_ID 18 /* A2DP Sink */
#define BTA_HID_SERVICE_ID 20 /* HID */
+#define BTA_PBAP_SERVICE_ID 22 /* PhoneBook Access Server*/
#define BTA_HFP_HS_SERVICE_ID 24 /* HSP HS role */
+#define BTA_MAP_SERVICE_ID 25 /* Message Access Profile */
+#define BTA_MN_SERVICE_ID 26 /* Message Notification Service */
+#define BTA_PCE_SERVICE_ID 28 /* PhoneBook Access Client */
#define BTA_SDP_SERVICE_ID 29 /* SDP Search */
#define BTA_HIDD_SERVICE_ID 30 /* HID Device */
diff --git a/system/btif/src/btif_dm.cc b/system/btif/src/btif_dm.cc
index 4f0097fe193..3fd506c3d79 100644
--- a/system/btif/src/btif_dm.cc
+++ b/system/btif/src/btif_dm.cc
@@ -307,6 +307,22 @@ bt_status_t btif_in_execute_service_request(tBTA_SERVICE_ID service_id,
case BTA_HIDD_SERVICE_ID: {
btif_hd_execute_service(b_enable);
} break;
+ case BTA_PBAP_SERVICE_ID:
+ FALLTHROUGH_INTENDED; /* FALLTHROUGH */
+ case BTA_PCE_SERVICE_ID:
+ FALLTHROUGH_INTENDED; /* FALLTHROUGH */
+ case BTA_MAP_SERVICE_ID:
+ FALLTHROUGH_INTENDED; /* FALLTHROUGH */
+ case BTA_MN_SERVICE_ID: {
+ /**
+ * Do nothing; these services were started elsewhere. However, we need to flow through this
+ * codepath in order to properly report back the local UUIDs back to adapter properties in
+ * Java. To achieve this, we need to catch these service IDs in order for {@link
+ * btif_in_execute_service_request} to return {@code BT_STATUS_SUCCESS}, so that in {@link
+ * btif_dm_enable_service} the check passes and the UUIDs are allowed to be passed up into
+ * the Java layer.
+ */
+ } break;
default:
BTIF_TRACE_ERROR("%s: Unknown service %d being %s", __func__, service_id,
(b_enable) ? "enabled" : "disabled");
diff --git a/system/btif/src/btif_sdp_server.cc b/system/btif/src/btif_sdp_server.cc
index cab45200da6..ba166b60675 100644
--- a/system/btif/src/btif_sdp_server.cc
+++ b/system/btif/src/btif_sdp_server.cc
@@ -288,6 +288,38 @@ bt_status_t create_sdp_record(bluetooth_sdp_record* record,
bt_status_t remove_sdp_record(int record_id) {
int handle;
+ bluetooth_sdp_record* record;
+ bluetooth_sdp_types sdp_type = SDP_TYPE_RAW;
+ {
+ std::unique_lock<std::recursive_mutex> lock(sdp_lock);
+ record = sdp_slots[record_id].record_data;
+ if (record != NULL) {
+ sdp_type = record->hdr.type;
+ }
+ }
+ tBTA_SERVICE_ID service_id = -1;
+ switch (sdp_type) {
+ case SDP_TYPE_MAP_MAS:
+ service_id = BTA_MAP_SERVICE_ID;
+ break;
+ case SDP_TYPE_MAP_MNS:
+ service_id = BTA_MN_SERVICE_ID;
+ break;
+ case SDP_TYPE_PBAP_PSE:
+ service_id = BTA_PBAP_SERVICE_ID;
+ break;
+ case SDP_TYPE_PBAP_PCE:
+ service_id = BTA_PCE_SERVICE_ID;
+ break;
+ default:
+ /* other enumeration values were not enabled in {@link on_create_record_event} */
+ break;
+ }
+ if (service_id > 0) {
+ // {@link btif_disable_service} sets the mask {@link btif_enabled_services}.
+ btif_disable_service(service_id);
+ }
+
/* Get the Record handle, and free the slot */
handle = free_sdp_slot(record_id);
BTIF_TRACE_DEBUG("Sdp Server %s id=%d to handle=0x%08x", __func__, record_id,
@@ -317,6 +349,7 @@ void on_create_record_event(int id) {
* */
BTIF_TRACE_DEBUG("Sdp Server %s", __func__);
const sdp_slot_t* sdp_slot = start_create_sdp(id);
+ tBTA_SERVICE_ID service_id = -1;
/* In the case we are shutting down, sdp_slot is NULL */
if (sdp_slot != NULL) {
bluetooth_sdp_record* record = sdp_slot->record_data;
@@ -324,12 +357,15 @@ void on_create_record_event(int id) {
switch (record->hdr.type) {
case SDP_TYPE_MAP_MAS:
handle = add_maps_sdp(&record->mas);
+ service_id = BTA_MAP_SERVICE_ID;
break;
case SDP_TYPE_MAP_MNS:
handle = add_mapc_sdp(&record->mns);
+ service_id = BTA_MN_SERVICE_ID;
break;
case SDP_TYPE_PBAP_PSE:
handle = add_pbaps_sdp(&record->pse);
+ service_id = BTA_PBAP_SERVICE_ID;
break;
case SDP_TYPE_OPP_SERVER:
handle = add_opps_sdp(&record->ops);
@@ -339,6 +375,7 @@ void on_create_record_event(int id) {
break;
case SDP_TYPE_PBAP_PCE:
handle = add_pbapc_sdp(&record->pce);
+ service_id = BTA_PCE_SERVICE_ID;
break;
default:
BTIF_TRACE_DEBUG("Record type %d is not supported", record->hdr.type);
@@ -346,6 +383,18 @@ void on_create_record_event(int id) {
}
if (handle != -1) {
set_sdp_handle(id, handle);
+ if (service_id > 0) {
+ /**
+ * {@link btif_enable_service} calls {@link btif_dm_enable_service}, which calls {@link
+ * btif_in_execute_service_request}.
+ * - {@link btif_enable_service} sets the mask {@link btif_enabled_services}.
+ * - {@link btif_dm_enable_service} invokes the java callback to return uuids based
+ * on the enabled services mask.
+ * - {@link btif_in_execute_service_request} gates the java callback in {@link
+ * btif_dm_enable_service}.
+ */
+ btif_enable_service(service_id);
+ }
}
}
}
diff --git a/system/btif/src/btif_storage.cc b/system/btif/src/btif_storage.cc
index 38640e235d9..af84a225b69 100644
--- a/system/btif/src/btif_storage.cc
+++ b/system/btif/src/btif_storage.cc
@@ -698,11 +698,27 @@ bt_status_t btif_storage_get_adapter_property(bt_property_t* property) {
*(p_uuid + num_uuids) = Uuid::From16Bit(UUID_SERVCLASS_AUDIO_SINK);
num_uuids++;
} break;
+ case BTA_PBAP_SERVICE_ID: {
+ *(p_uuid + num_uuids) = Uuid::From16Bit(UUID_SERVCLASS_PBAP_PSE);
+ num_uuids++;
+ } break;
case BTA_HFP_HS_SERVICE_ID: {
*(p_uuid + num_uuids) =
Uuid::From16Bit(UUID_SERVCLASS_HF_HANDSFREE);
num_uuids++;
} break;
+ case BTA_MAP_SERVICE_ID: {
+ *(p_uuid + num_uuids) = Uuid::From16Bit(UUID_SERVCLASS_MESSAGE_ACCESS);
+ num_uuids++;
+ } break;
+ case BTA_MN_SERVICE_ID: {
+ *(p_uuid + num_uuids) = Uuid::From16Bit(UUID_SERVCLASS_MESSAGE_NOTIFICATION);
+ num_uuids++;
+ } break;
+ case BTA_PCE_SERVICE_ID: {
+ *(p_uuid + num_uuids) = Uuid::From16Bit(UUID_SERVCLASS_PBAP_PCE);
+ num_uuids++;
+ } break;
}
}
}
--
GitLab