2020-04-01 22:15:12

by Sonny Sasaka

[permalink] [raw]
Subject: [PATCH 1/5] D-Bus API changes for managing SDP records

From: Rahul Chaturvedi <[email protected]>

This defines the DBus API that we'll use with BlueZ to create, remove
and get service records.
---
doc/adapter-api.txt | 46 +++++++++++++++++++++++++++++++++++++++++++++
doc/device-api.txt | 37 ++++++++++++++++++++++++++++++++++++
2 files changed, 83 insertions(+)

diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
index acae032d9..6e4c37fc9 100644
--- a/doc/adapter-api.txt
+++ b/doc/adapter-api.txt
@@ -204,6 +204,52 @@ Methods void StartDiscovery()
org.bluez.Error.NotReady
org.bluez.Error.Failed

+ uint32 CreateServiceRecord(dict record)
+
+ This method creates an entry with the local SDP server
+ for this adapter for the specified record. This method
+ will only create the SDP record and not start listening
+ on any ports. It is up to the caller of the method to
+ ensure the validity of the service record. This record
+ will not be parsed for any validation but will instead
+ directly be inserted into the local SDP server’s
+ records.
+
+ The return value from this method will be the 32 bit
+ handle for the created service record.
+
+ The record dictionary will have dictionary entries of
+ the format: {id : (type, size, value)}, where,
+
+ uint16 id: The 16 bit attribute ID for an
+ attribute.
+ uint8 type: This will contain the type of the
+ attribute value. Attribute type values
+ are defined in the Bluetooth spec in
+ Volume 3, Part B, 3.2.
+ uint32 size: This is the size of the attribute
+ value.
+ variant value: This will contain the attribute value
+ for a given attribute_id. This variant
+ can either contain a primitive type, or
+ if type is SEQUENCE, an array of struct
+ of the signature (type, size, value).
+
+ Possible errors: org.bluez.Error.NotReady
+ org.bluez.Error.AlreadyExists
+ org.bluez.Error.Failed
+ org.bluez.Error.InvalidArguments
+
+ void RemoveServiceRecord(uint32 handle)
+
+ This method removes the SDP record with the given
+ handle from the local SDP server.
+
+ Possible errors: org.bluez.Error.NotReady
+ org.bluez.Error.DoesNotExist
+ org.bluez.Error.Failed
+ org.bluez.Error.InvalidArguments
+
Properties string Address [readonly]

The Bluetooth device address.
diff --git a/doc/device-api.txt b/doc/device-api.txt
index ceb68d2f6..e8f2c670d 100644
--- a/doc/device-api.txt
+++ b/doc/device-api.txt
@@ -120,6 +120,43 @@ Methods void Connect()
Possible errors: org.bluez.Error.DoesNotExist
org.bluez.Error.Failed

+ array{array{dict}} GetServiceRecords()
+
+ This method returns the complete service records of all
+ discovered BR/EDR services of the connected device till
+ now. The return value will be an array of an array of
+ dictionary entries. Each nested array of dictionary
+ entries will contain one service record. Each pair in
+ the returned dictionary entries will represent an
+ attribute in the service record.
+
+ The dictionary entries in the returned array of entries
+ will be of the format: {id : (type, size, value)} where,
+
+ uint16 id: The 16 bit attribute ID for an
+ attribute.
+ uint8 type: This will contain the type of the
+ attribute value. Attribute type values
+ are defined in the Bluetooth spec in
+ Volume 3, Part B, 3.2.
+ uint32 size: This is the size of the attribute
+ value.
+ variant value: This will contain the attribute value
+ for a given attribute_id. This variant
+ can either contain a primitive type, or
+ if type is SEQUENCE, an array of struct
+ of the signature (type, size, value).
+
+ Since the service record contains the service UUID and
+ name, these fields do not need to be given separately.
+ This method should be called after the ServiceResolved
+ property for this object has been set to true otherwise
+ it may return a partial or stale cached list.
+
+ Possible errors: org.bluez.Error.NotReady
+ org.bluez.Error.NotConnected
+ org.bluez.Error.Failed
+
Properties string Address [readonly]

The Bluetooth device address of the remote device.
--
2.24.1


2020-04-01 22:15:18

by Sonny Sasaka

[permalink] [raw]
Subject: [PATCH 4/5] Implement RemoveServiceRecord method

From: Miao Chou <[email protected]>

This implements the RemoveServiceRecord method of org.bluez.Adapter1 interface.
---
src/adapter.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)

diff --git a/src/adapter.c b/src/adapter.c
index c02aaf32b..1be3e7984 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -3834,6 +3834,29 @@ failed_to_alloc:
return btd_error_failed(msg, "Failed to allocate SDP record");
}

+static DBusMessage *remove_service_record(DBusConnection *conn,
+ DBusMessage *msg, void *user_data)
+{
+ struct btd_adapter *adapter = user_data;
+ uint32_t rec_handle = 0xffffffff;
+ sdp_record_t *rec = NULL;
+
+ if (!(adapter->current_settings & MGMT_SETTING_POWERED))
+ return btd_error_not_ready(msg);
+
+ if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_UINT32, &rec_handle,
+ DBUS_TYPE_INVALID))
+ return btd_error_invalid_args(msg);
+
+ rec = sdp_record_find(rec_handle);
+ if (!rec)
+ return btd_error_does_not_exist(msg);
+
+ adapter_service_remove(adapter, rec_handle);
+
+ return dbus_message_new_method_return(msg);
+}
+
static const GDBusMethodTable adapter_methods[] = {
{ GDBUS_ASYNC_METHOD("StartDiscovery", NULL, NULL, start_discovery) },
{ GDBUS_METHOD("SetDiscoveryFilter",
@@ -3852,6 +3875,8 @@ static const GDBusMethodTable adapter_methods[] = {
GDBUS_ARGS({"record", "a{q(yuv)}"}),
GDBUS_ARGS({"handle", "u"}),
create_service_record)},
+ { GDBUS_METHOD("RemoveServiceRecord", GDBUS_ARGS({"handle", "u"}), NULL,
+ remove_service_record)},
{ }
};

--
2.24.1

2020-04-01 22:15:28

by Sonny Sasaka

[permalink] [raw]
Subject: [PATCH 5/5] Add SDP records retrieval for a unpaired/unconnected device

From: Miao Chou <[email protected]>

This add the support of SDP record inquiry to a device which is not paired or
connected previously.
---
src/device.c | 130 ++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 109 insertions(+), 21 deletions(-)

diff --git a/src/device.c b/src/device.c
index 1b5afc226..dd7c32625 100644
--- a/src/device.c
+++ b/src/device.c
@@ -270,9 +270,35 @@ static const uint16_t uuid_list[] = {
0
};

+struct pending_sdp_query {
+ bdaddr_t dev_bdaddr;
+ DBusConnection *conn;
+ DBusMessage *msg;
+};
+
static int device_browse_gatt(struct btd_device *device, DBusMessage *msg);
static int device_browse_sdp(struct btd_device *device, DBusMessage *msg);

+static struct pending_sdp_query *pending_sdp_query_new(DBusConnection *conn,
+ DBusMessage *msg, const struct btd_device *dev)
+{
+ struct pending_sdp_query *query;
+
+ query = g_new0(struct pending_sdp_query, 1);
+ query->dev_bdaddr = dev->bdaddr;
+ query->conn = dbus_connection_ref(conn);
+ query->msg = dbus_message_ref(msg);
+
+ return query;
+}
+
+static void pending_sdp_query_free(struct pending_sdp_query *query)
+{
+ dbus_connection_unref(query->conn);
+ dbus_message_unref(query->msg);
+ g_free(query);
+}
+
static struct bearer_state *get_state(struct btd_device *dev,
uint8_t bdaddr_type)
{
@@ -3008,28 +3034,13 @@ static dbus_bool_t append_record(const sdp_record_t *rec,
return TRUE;
}

-static DBusMessage *get_service_records(DBusConnection *conn,
- DBusMessage *msg, void *data)
+static dbus_bool_t append_records(DBusMessage *reply, sdp_list_t *seq)
{
- struct btd_device *dev = data;
- sdp_list_t *seq;
- DBusMessage *reply;
DBusMessageIter rec_array;
DBusMessageIter attr_array;

- if (!btd_adapter_get_powered(dev->adapter))
- return btd_error_not_ready(msg);
-
- if (!btd_device_is_connected(dev))
- return btd_error_not_connected(msg);
-
- reply = dbus_message_new_method_return(msg);
- if (!reply)
- return btd_error_failed(msg, "Failed to create method reply");
-
- /* Load records from storage if there is nothing in cache */
- if (!dev->tmp_records)
- return btd_error_failed(msg, "SDP record not found");
+ if (!seq)
+ return FALSE;

dbus_message_iter_init_append(reply, &rec_array);

@@ -3037,18 +3048,95 @@ static DBusMessage *get_service_records(DBusConnection *conn,
"a{q(yuv)}", &attr_array))
return FALSE;

- for (seq = dev->tmp_records; seq; seq = seq->next) {
+ for (; seq; seq = seq->next) {
sdp_record_t *rec = (sdp_record_t *) seq->data;
if (!rec)
continue;
if (!append_record(rec, &attr_array))
- return btd_error_failed(msg,
- "SDP record attachment failed");
+ return FALSE;
}

if (!dbus_message_iter_close_container(&rec_array, &attr_array))
return FALSE;

+ return TRUE;
+}
+
+static void get_service_records_cb(struct btd_device *dev, int err,
+ void *user_data)
+{
+ struct pending_sdp_query *query = user_data;
+ DBusMessage *reply;
+ sdp_list_t *seq = dev->tmp_records;
+
+ if (memcmp(&query->dev_bdaddr, &dev->bdaddr, sizeof(bdaddr_t))) {
+ reply = btd_error_failed(query->msg, "Device mismatched");
+ goto send_reply;
+ }
+
+ if (!dev->bredr_state.svc_resolved) {
+ reply = btd_error_not_ready(query->msg);
+ goto send_reply;
+ }
+
+ /* Load records from storage if there is nothing in cache */
+ if (!seq) {
+ reply = btd_error_failed(query->msg, "SDP record not found");
+ goto send_reply;
+ }
+
+ reply = dbus_message_new_method_return(query->msg);
+ if (!reply) {
+ reply = btd_error_failed(query->msg, "Failed to create method reply");
+ goto send_reply;
+ }
+
+ if (!append_records(reply, dev->tmp_records))
+ reply = btd_error_failed(query->msg, "SDP record attachment failed");
+
+send_reply:
+ g_dbus_send_message(query->conn, reply);
+ pending_sdp_query_free(query);
+}
+
+static DBusMessage *get_service_records(DBusConnection *conn,
+ DBusMessage *msg, void *data)
+{
+ struct btd_device *dev = data;
+ DBusMessage *reply;
+ struct pending_sdp_query *query = NULL;
+ sdp_list_t *seq = dev->tmp_records;
+
+ if (!btd_adapter_get_powered(dev->adapter))
+ return btd_error_not_ready(msg);
+
+ if (!btd_device_is_connected(dev)) {
+ /*
+ * Do a SDP service discovery on the device if the device is not
+ * previously connected. SDP records will be returned later in
+ * get_service_records_cb.
+ */
+ query = pending_sdp_query_new(conn, msg, dev);
+ device_wait_for_svc_complete(dev, get_service_records_cb,
+ query);
+ device_browse_sdp(dev, NULL);
+ return NULL;
+ }
+
+ if (!dev->bredr_state.svc_resolved)
+ return btd_error_not_ready(msg);
+
+ /* Load records from storage if there is nothing in cache */
+ if (!seq)
+ return btd_error_failed(msg, "SDP record not found");
+
+ reply = dbus_message_new_method_return(msg);
+ if (!reply)
+ return btd_error_failed(msg, "Failed to create method reply");
+
+ if (!append_records(reply, seq))
+ return btd_error_failed(msg, "SDP record attachment failed");
+
return reply;
}

--
2.24.1

2020-04-01 22:16:06

by Sonny Sasaka

[permalink] [raw]
Subject: [PATCH 3/5] Implement CreateServiceRecord method

From: Miao Chou <[email protected]>

This implements the CreateServiceRecord method of org.bluez.Adapter1 interface.
---
src/adapter.c | 414 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 414 insertions(+)

diff --git a/src/adapter.c b/src/adapter.c
index 972d88772..c02aaf32b 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -48,6 +48,7 @@
#include "bluetooth/sdp_lib.h"
#include "lib/uuid.h"
#include "lib/mgmt.h"
+#include "lib/sdp.h"

#include "gdbus/gdbus.h"

@@ -3424,6 +3425,415 @@ static DBusMessage *connect_device(DBusConnection *conn,
return NULL;
}

+static int parse_int_uint_variant(DBusMessageIter *iter, bool unsign,
+ uint32_t size, void **val, uint8_t *dtd)
+{
+ int dbus_type = DBUS_TYPE_INVALID;
+ char arg[4];
+
+ switch(size) {
+ case 1:
+ if (unsign) {
+ *val = malloc(sizeof(uint8_t));
+ *dtd = SDP_UINT8;
+ } else {
+ *val = malloc(sizeof(int8_t));
+ *dtd = SDP_INT8;
+ }
+ dbus_type = DBUS_TYPE_BYTE;
+ break;
+ case 2:
+ if (unsign) {
+ *val = malloc(sizeof(uint16_t));
+ *dtd = SDP_UINT16;
+ dbus_type = DBUS_TYPE_UINT16;
+ } else {
+ *val = malloc(sizeof(int16_t));
+ *dtd = SDP_INT16;
+ dbus_type = DBUS_TYPE_INT16;
+ }
+ break;
+ case 4:
+ if (unsign) {
+ *val = malloc(sizeof(uint32_t));
+ *dtd = SDP_UINT32;
+ dbus_type = DBUS_TYPE_UINT32;
+ } else {
+ *val = malloc(sizeof(int32_t));
+ *dtd = SDP_INT32;
+ dbus_type = DBUS_TYPE_INT32;
+ }
+ break;
+ case 8:
+ if (unsign) {
+ *val = malloc(sizeof(uint64_t));
+ *dtd = SDP_UINT64;
+ dbus_type = DBUS_TYPE_UINT64;
+ } else {
+ *val = malloc(sizeof(int64_t));
+ *dtd = SDP_INT64;
+ dbus_type = DBUS_TYPE_INT64;
+ }
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (!*val)
+ return -1;
+
+ if (dbus_message_iter_get_arg_type(iter) != dbus_type) {
+ free(*val);
+ return -EINVAL;
+ }
+ dbus_message_iter_get_basic(iter, arg);
+
+ memcpy(*val, arg, size);
+
+ return 0;
+}
+
+static int parse_str_variant(DBusMessageIter *iter, void **val, int *str_len)
+{
+ const char *str;
+
+ if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_STRING)
+ return -EINVAL;
+ dbus_message_iter_get_basic(iter, &str);
+
+ if (!str)
+ return -1;
+
+ *str_len = strlen(str) + 1;
+
+ *val = calloc(*str_len, sizeof(char));
+ if (!val)
+ return -1;
+
+ strncpy(*val, str, *str_len - 1);
+
+ return 0;
+}
+
+static int parse_attr_value(DBusMessageIter *val_struct,
+ sdp_data_t **attr, sdp_record_t* rec)
+{
+ DBusMessageIter iter, val_variant;
+ uint8_t val_type = SDP_VAL_TYPE_NIL;
+ uint32_t val_size = 0;
+
+ uint8_t dtd;
+ void *val = NULL;
+ int str_len = 0;
+ int ret = 0;
+
+ *attr = NULL;
+
+ if (dbus_message_iter_get_arg_type(val_struct) != DBUS_TYPE_STRUCT)
+ return -EINVAL;
+ dbus_message_iter_recurse(val_struct, &iter);
+
+ // Extract attribute value type.
+ if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_BYTE)
+ return -EINVAL;
+ dbus_message_iter_get_basic(&iter, &val_type);
+ if (!dbus_message_iter_next(&iter))
+ return -ENODATA;
+
+ // Extract attribute value size.
+ if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_UINT32)
+ return -EINVAL;
+ dbus_message_iter_get_basic(&iter, &val_size);
+ if (!dbus_message_iter_next(&iter))
+ return -ENODATA;
+
+ // Extract attribute value based on its type.
+ if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
+ return -EINVAL;
+ dbus_message_iter_recurse(&iter, &val_variant);
+
+ switch(val_type) {
+ case SDP_VAL_TYPE_NIL:
+ dtd = SDP_DATA_NIL;
+ break;
+ case SDP_VAL_TYPE_UINT: {
+ ret = parse_int_uint_variant(&val_variant, true, val_size, &val,
+ &dtd);
+ if (ret < 0)
+ return ret;
+ break;
+ }
+ case SDP_VAL_TYPE_INT: {
+ ret = parse_int_uint_variant(&val_variant, false, val_size, &val,
+ &dtd);
+ if (ret < 0)
+ return ret;
+ break;
+ }
+ case SDP_VAL_TYPE_UUID: {
+ // The UUID string is in "%08x-%04x-%04x-%04x-%08x%04x" format.
+ // Take Bluetooth base UUID for instance, the base UUID string
+ // is "00000000-0000-1000-8000-00805F9B34FB". Extra action may
+ // be needed to change the UUID type of a newly-created |uuid|
+ // based on the |val_size| field.
+ uuid_t *uuid = NULL;
+ const char *uuid_str;
+ void *arg = NULL;
+
+ if (dbus_message_iter_get_arg_type(&val_variant) !=
+ DBUS_TYPE_STRING)
+ return -EINVAL;
+ dbus_message_iter_get_basic(&val_variant, &uuid_str);
+
+ if (!uuid_str)
+ return -1;
+
+ uuid = (uuid_t *)malloc(sizeof(uuid_t));
+ if (!uuid)
+ return -1;
+
+ if (bt_string2uuid(uuid, uuid_str) < 0) {
+ free(uuid);
+ return -1;
+ }
+
+ switch(val_size) {
+ case 2:
+ dtd = SDP_UUID16;
+ if (!sdp_uuid128_to_uuid(uuid)) {
+ free(uuid);
+ return -1;
+ }
+ arg = &uuid->value.uuid16;
+ break;
+ case 4:
+ dtd = SDP_UUID32;
+ if (!sdp_uuid128_to_uuid(uuid)) {
+ free(uuid);
+ return -1;
+ }
+ arg = &uuid->value.uuid32;
+ break;
+ case 16:
+ dtd = SDP_UUID128;
+ arg = &uuid->value.uuid128;
+ break;
+ default:
+ free(uuid);
+ return -EINVAL;
+ }
+
+ val = malloc(val_size);
+ memcpy(val, arg, val_size);
+
+ // Insert the UUID into |rec|'s search pattern. The ownership
+ // of |uuid| will NOT be transferred, so |uuid| should be freed
+ // after use.
+ sdp_pattern_add_uuid(rec, uuid);
+
+ free(uuid);
+ break;
+ }
+ case SDP_VAL_TYPE_STRING: {
+ dtd = SDP_TEXT_STR8;
+ ret = parse_str_variant(&val_variant, &val, &str_len);
+ if (ret < 0)
+ return ret;
+ break;
+ }
+ case SDP_VAL_TYPE_BOOL: {
+ dbus_bool_t *arg = NULL;
+
+ dtd = SDP_BOOL;
+
+ if (dbus_message_iter_get_arg_type(&val_variant) !=
+ DBUS_TYPE_BOOLEAN)
+ return -EINVAL;
+ dbus_message_iter_get_basic(&val_variant, arg);
+
+ if (!arg)
+ return -1;
+
+ val = malloc(sizeof(bool));
+ if (!val)
+ return -1;
+
+ memcpy(val, arg, sizeof(bool));
+ break;
+ }
+ case SDP_VAL_TYPE_URL: {
+ dtd = SDP_URL_STR8;
+ ret = parse_str_variant(&val_variant, &val, &str_len);
+ if (ret < 0)
+ return ret;
+ break;
+ }
+ case SDP_VAL_TYPE_SEQUENCE: {
+ sdp_data_t *seq = NULL;
+ DBusMessageIter seq_iter;
+
+ dtd = SDP_SEQ32;
+
+ if (dbus_message_iter_get_arg_type(&val_variant) !=
+ DBUS_TYPE_ARRAY)
+ return -EINVAL;
+
+ dbus_message_iter_recurse(&val_variant, &seq_iter);
+
+ // Recursively extract the value from each element of the
+ // sequence.
+ while (dbus_message_iter_get_arg_type(&seq_iter) ==
+ DBUS_TYPE_STRUCT) {
+ sdp_data_t *data;
+
+ ret = parse_attr_value(&seq_iter, &data, rec);
+ if (!data || ret < 0) {
+ if (data)
+ sdp_data_free(data);
+ if (seq)
+ sdp_data_free(seq);
+
+ return ret;
+ }
+ seq = sdp_seq_append(seq, data);
+
+ dbus_message_iter_next(&seq_iter);
+ }
+
+ val = seq;
+ break;
+ }
+ default:
+ return -EINVAL;
+ }
+
+ *attr = sdp_data_alloc_with_length(dtd, val, str_len);
+ if (!*attr) {
+ ret = -1;
+ goto error_alloc_attr;
+ }
+
+ return 0;
+
+error_alloc_attr:
+ if (!val)
+ return ret;
+
+ if (val_type == SDP_VAL_TYPE_SEQUENCE)
+ sdp_data_free((sdp_data_t *)val);
+ else
+ free(val);
+
+ return ret;
+}
+
+static int parse_record(DBusMessageIter *rec_array,
+ sdp_record_t *rec)
+{
+ DBusMessageIter attr_dict;
+ sdp_data_t *attr = NULL;
+ int ret = 0;
+
+ if (dbus_message_iter_get_arg_type(rec_array) != DBUS_TYPE_ARRAY)
+ return -EINVAL;
+
+ dbus_message_iter_recurse(rec_array, &attr_dict);
+
+ while(dbus_message_iter_get_arg_type(&attr_dict) ==
+ DBUS_TYPE_DICT_ENTRY) {
+ DBusMessageIter entry;
+ uint16_t attr_id;
+
+ dbus_message_iter_recurse(&attr_dict, &entry);
+
+ // Extract attribute ID.
+ if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_UINT16)
+ return -EINVAL;
+ dbus_message_iter_get_basic(&entry, &attr_id);
+ if (!dbus_message_iter_next(&entry))
+ return -ENODATA;
+
+ // Extract attribute value structure.
+ ret = parse_attr_value(&entry, &attr, rec);
+ if (ret < 0)
+ goto error_parse_attr;
+
+ // Add the new attribute to the record.
+ if (sdp_attr_add(rec, attr_id, attr) < 0) {
+ ret = -1;
+ goto error_parse_attr;
+ }
+
+ dbus_message_iter_next(&attr_dict);
+ }
+
+ return 0;
+
+error_parse_attr:
+ if (attr)
+ sdp_data_free(attr);
+
+ // Release the UUID pattern list.
+ sdp_list_free(rec->pattern, free);
+
+ return ret;
+}
+
+static DBusMessage *create_service_record(DBusConnection *conn,
+ DBusMessage *msg, void *user_data)
+{
+ struct btd_adapter *adapter = user_data;
+ DBusMessage *reply;
+ sdp_record_t *rec;
+ DBusMessageIter rec_array, iter;
+ int ret = 0;
+
+ if (!(adapter->current_settings & MGMT_SETTING_POWERED))
+ return btd_error_not_ready(msg);
+
+ if(!dbus_message_iter_init(msg, &rec_array))
+ return btd_error_failed(msg, "Failed to read record argument");
+
+ reply = dbus_message_new_method_return(msg);
+ if (!reply)
+ return btd_error_failed(msg, "Failed to create method reply");
+
+ rec = sdp_record_alloc();
+ if (!rec)
+ goto failed_to_alloc;
+
+ ret = parse_record(&rec_array, rec);
+ if (ret < 0) {
+ sdp_record_free(rec);
+ if (ret == -EINVAL || ret == -ENODATA)
+ return btd_error_invalid_args(msg);
+ else
+ return btd_error_failed(msg,
+ "Failed to parse record argument");
+ }
+
+ ret = adapter_service_add(adapter, rec);
+ if (ret < 0) {
+ sdp_record_free(rec);
+ if (ret == -EEXIST)
+ return btd_error_already_exists(msg);
+ else
+ return btd_error_failed(msg, "Failed to add record");
+ }
+
+ dbus_message_iter_init_append(reply, &iter);
+ if (!dbus_message_iter_append_basic(
+ &iter, DBUS_TYPE_UINT32, &rec->handle)) {
+ sdp_record_free(rec);
+ return btd_error_failed(msg, "Failed to append record handle");
+ }
+
+ return reply;
+
+failed_to_alloc:
+ return btd_error_failed(msg, "Failed to allocate SDP record");
+}
+
static const GDBusMethodTable adapter_methods[] = {
{ GDBUS_ASYNC_METHOD("StartDiscovery", NULL, NULL, start_discovery) },
{ GDBUS_METHOD("SetDiscoveryFilter",
@@ -3438,6 +3848,10 @@ static const GDBusMethodTable adapter_methods[] = {
{ GDBUS_EXPERIMENTAL_ASYNC_METHOD("ConnectDevice",
GDBUS_ARGS({ "properties", "a{sv}" }), NULL,
connect_device) },
+ { GDBUS_METHOD("CreateServiceRecord",
+ GDBUS_ARGS({"record", "a{q(yuv)}"}),
+ GDBUS_ARGS({"handle", "u"}),
+ create_service_record)},
{ }
};

--
2.24.1

2020-04-01 22:16:11

by Sonny Sasaka

[permalink] [raw]
Subject: [PATCH 2/5] Implement GetServiceRecords method

From: Miao Chou <[email protected]>

This implements the GetServiceRecords method of org.bluez.Device1 interface.
---
lib/sdp.h | 13 +++
src/device.c | 293 +++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 306 insertions(+)

diff --git a/lib/sdp.h b/lib/sdp.h
index f586eb5eb..c6fdf3d3a 100644
--- a/lib/sdp.h
+++ b/lib/sdp.h
@@ -396,6 +396,19 @@ extern "C" {
#define SDP_URL_STR16 0x46
#define SDP_URL_STR32 0x47

+/*
+ * These are Chromium specific SDP data types which map the above SDP datatype
+ * but without specifying the sizes of types.
+ */
+#define SDP_VAL_TYPE_NIL 0x00
+#define SDP_VAL_TYPE_UINT 0x01
+#define SDP_VAL_TYPE_INT 0x02
+#define SDP_VAL_TYPE_UUID 0x03
+#define SDP_VAL_TYPE_STRING 0x04
+#define SDP_VAL_TYPE_BOOL 0x05
+#define SDP_VAL_TYPE_SEQUENCE 0x06
+#define SDP_VAL_TYPE_URL 0x07
+
/*
* The PDU identifiers of SDP packets between client and server
*/
diff --git a/src/device.c b/src/device.c
index ace9c348c..1b5afc226 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2762,6 +2762,296 @@ static DBusMessage *cancel_pairing(DBusConnection *conn, DBusMessage *msg,
return dbus_message_new_method_return(msg);
}

+static dbus_bool_t append_attr_value(sdp_data_t* val,
+ DBusMessageIter *val_entry)
+{
+ DBusMessageIter val_struct;
+ DBusMessageIter val_variant;
+ DBusMessageIter val_seq_array;
+
+ uint8_t value_type;
+ void* value = NULL;
+ uint32_t value_size = (uint32_t) val->unitSize;
+ sdp_data_t *data;
+ char *str = NULL;
+
+ int type;
+ const char *type_sig;
+
+ if (!dbus_message_iter_open_container(val_entry, DBUS_TYPE_STRUCT,
+ NULL, &val_struct))
+ return FALSE;
+
+ switch (val->dtd) {
+ case SDP_DATA_NIL:
+ goto val_nil;
+ case SDP_BOOL:
+ value_type = SDP_VAL_TYPE_BOOL;
+ value = &val->val.uint8;
+ value_size = sizeof(uint8_t);
+ type_sig = DBUS_TYPE_BOOLEAN_AS_STRING;
+ type = DBUS_TYPE_BOOLEAN;
+ break;
+ case SDP_UINT8:
+ value_type = SDP_VAL_TYPE_UINT;
+ value = &val->val.uint8;
+ value_size = sizeof(uint8_t);
+ type_sig = DBUS_TYPE_BYTE_AS_STRING;
+ type = DBUS_TYPE_BYTE;
+ break;
+ case SDP_UINT16:
+ value_type = SDP_VAL_TYPE_UINT;
+ value = &val->val.uint16;
+ value_size = sizeof(uint16_t);
+ type_sig = DBUS_TYPE_UINT16_AS_STRING;
+ type = DBUS_TYPE_UINT16;
+ break;
+ case SDP_UINT32:
+ value_type = SDP_VAL_TYPE_UINT;
+ value = &val->val.uint32;
+ value_size = sizeof(uint32_t);
+ type_sig = DBUS_TYPE_UINT32_AS_STRING;
+ type = DBUS_TYPE_UINT32;
+ break;
+ case SDP_UINT64:
+ value_type = SDP_VAL_TYPE_UINT;
+ value = &val->val.uint64;
+ value_size = sizeof(uint64_t);
+ type_sig = DBUS_TYPE_UINT64_AS_STRING;
+ type = DBUS_TYPE_UINT64;
+ break;
+ case SDP_INT8:
+ value_type = SDP_VAL_TYPE_INT;
+ value = &val->val.int8;
+ value_size = sizeof(int8_t);
+ type_sig = DBUS_TYPE_BYTE_AS_STRING;
+ type = DBUS_TYPE_BYTE;
+ break;
+ case SDP_INT16:
+ value_type = SDP_VAL_TYPE_INT;
+ value = &val->val.int16;
+ value_size = sizeof(int16_t);
+ type_sig = DBUS_TYPE_INT16_AS_STRING;
+ type = DBUS_TYPE_INT16;
+ break;
+ case SDP_INT32:
+ value_type = SDP_VAL_TYPE_INT;
+ value = &val->val.int32;
+ value_size = sizeof(int32_t);
+ type_sig = DBUS_TYPE_INT32_AS_STRING;
+ type = DBUS_TYPE_INT32;
+ break;
+ case SDP_INT64:
+ value_type = SDP_VAL_TYPE_INT;
+ value = &val->val.int64;
+ value_size = sizeof(int64_t);
+ type_sig = DBUS_TYPE_INT64_AS_STRING;
+ type = DBUS_TYPE_INT64;
+ break;
+ case SDP_UUID16:
+ case SDP_UUID32:
+ case SDP_UUID128:
+ // Although a UUID is passed as a string in format
+ // "0000XXXX-0000-1000-8000-00805F9B34FB", |value_size|
+ // should hold the original length of a UUID. The length unit
+ // is byte, so a length can be 2, 4, 16.
+ if (val->dtd == SDP_UUID16)
+ value_size = sizeof(uint16_t);
+ else if (val->dtd == SDP_UUID32)
+ value_size = sizeof(uint32_t);
+ else
+ value_size = sizeof(uint128_t);
+ value_type = SDP_VAL_TYPE_UUID;
+ str = bt_uuid2string(&val->val.uuid);
+ value = &str;
+ type_sig = DBUS_TYPE_STRING_AS_STRING;
+ type = DBUS_TYPE_STRING;
+ break;
+ case SDP_TEXT_STR8:
+ case SDP_TEXT_STR16:
+ case SDP_TEXT_STR32:
+ // TODO(chromium:633929): Distinguish string and data array by
+ // checking if the given string is in UTF-8 format or not. For
+ // now the binary case is skipped by setting |str| as a empty
+ // string.
+ str = val->val.str;
+ if (!dbus_validate_utf8(str, NULL))
+ str = "";
+ value_size = strlen(str);
+ value_type = SDP_VAL_TYPE_STRING;
+ value = &str;
+ type_sig = DBUS_TYPE_STRING_AS_STRING;
+ type = DBUS_TYPE_STRING;
+ break;
+ case SDP_URL_STR8:
+ case SDP_URL_STR16:
+ case SDP_URL_STR32:
+ value_type = SDP_VAL_TYPE_URL;
+ str = val->val.str;
+ value_size = strlen(str);
+ value = &str;
+ type_sig = DBUS_TYPE_STRING_AS_STRING;
+ type = DBUS_TYPE_STRING;
+ break;
+ case SDP_ALT8:
+ case SDP_ALT16:
+ case SDP_ALT32:
+ case SDP_SEQ8:
+ case SDP_SEQ16:
+ case SDP_SEQ32: {
+ uint32_t size = 0;
+
+ value_type = SDP_VAL_TYPE_SEQUENCE;
+
+ // Calculate the number of elements in the sequence.
+ for (data = val->val.dataseq; data; data = data->next)
+ size++;
+
+ if (!dbus_message_iter_append_basic(&val_struct,
+ DBUS_TYPE_BYTE, &value_type))
+ return FALSE;
+ if (!dbus_message_iter_append_basic(&val_struct,
+ DBUS_TYPE_UINT32, &size))
+ return FALSE;
+ if (!dbus_message_iter_open_container(&val_struct,
+ DBUS_TYPE_VARIANT, "a(yuv)", &val_variant))
+ return FALSE;
+ if (!dbus_message_iter_open_container(&val_variant,
+ DBUS_TYPE_ARRAY, "(yuv)", &val_seq_array))
+ return FALSE;
+
+ for (data = val->val.dataseq; data; data = data->next)
+ append_attr_value(data, &val_seq_array);
+
+ if (!dbus_message_iter_close_container(&val_variant,
+ &val_seq_array))
+ return FALSE;
+ if (!dbus_message_iter_close_container(&val_struct,
+ &val_variant))
+ return FALSE;
+ goto done;
+ }
+ default:
+ goto val_nil;
+ }
+
+ if (!dbus_message_iter_append_basic(&val_struct,
+ DBUS_TYPE_BYTE, &value_type))
+ goto failed_to_append;
+ if (!dbus_message_iter_append_basic(&val_struct,
+ DBUS_TYPE_UINT32, &value_size))
+ goto failed_to_append;
+ if (!dbus_message_iter_open_container(&val_struct, DBUS_TYPE_VARIANT,
+ type_sig, &val_variant))
+ goto failed_to_append;
+ if (!dbus_message_iter_append_basic(&val_variant, type, value))
+ goto failed_to_append;
+
+ if (value_type == SDP_VAL_TYPE_UUID)
+ free(str);
+
+ if (!dbus_message_iter_close_container(&val_struct, &val_variant))
+ return FALSE;
+
+val_nil:
+done:
+ if (!dbus_message_iter_close_container(val_entry, &val_struct))
+ return FALSE;
+ return TRUE;
+
+failed_to_append:
+ if (value_type == SDP_VAL_TYPE_UUID)
+ free(str);
+ return FALSE;
+}
+
+static dbus_bool_t append_attr(sdp_data_t *attr, DBusMessageIter *attr_dict)
+{
+ DBusMessageIter attr_entry;
+
+ if (!dbus_message_iter_open_container(attr_dict, DBUS_TYPE_DICT_ENTRY,
+ NULL, &attr_entry))
+ return FALSE;
+
+ if (!dbus_message_iter_append_basic(&attr_entry, DBUS_TYPE_UINT16,
+ &attr->attrId))
+ return FALSE;
+
+ if (!append_attr_value(attr, &attr_entry))
+ return FALSE;
+
+ if (!dbus_message_iter_close_container(attr_dict, &attr_entry))
+ return FALSE;
+
+ return TRUE;
+}
+
+static dbus_bool_t append_record(const sdp_record_t *rec,
+ DBusMessageIter *attr_array)
+{
+ sdp_list_t *seq;
+ DBusMessageIter attr_dict;
+
+ if (!dbus_message_iter_open_container(attr_array, DBUS_TYPE_ARRAY,
+ "{q(yuv)}", &attr_dict))
+ return FALSE;
+
+ for (seq = rec->attrlist; seq; seq = seq->next) {
+ sdp_data_t *attr = (sdp_data_t *) seq->data;
+ if (!append_attr(attr, &attr_dict))
+ return FALSE;
+ }
+
+ if (!dbus_message_iter_close_container(attr_array, &attr_dict))
+ return FALSE;
+
+ return TRUE;
+}
+
+static DBusMessage *get_service_records(DBusConnection *conn,
+ DBusMessage *msg, void *data)
+{
+ struct btd_device *dev = data;
+ sdp_list_t *seq;
+ DBusMessage *reply;
+ DBusMessageIter rec_array;
+ DBusMessageIter attr_array;
+
+ if (!btd_adapter_get_powered(dev->adapter))
+ return btd_error_not_ready(msg);
+
+ if (!btd_device_is_connected(dev))
+ return btd_error_not_connected(msg);
+
+ reply = dbus_message_new_method_return(msg);
+ if (!reply)
+ return btd_error_failed(msg, "Failed to create method reply");
+
+ /* Load records from storage if there is nothing in cache */
+ if (!dev->tmp_records)
+ return btd_error_failed(msg, "SDP record not found");
+
+ dbus_message_iter_init_append(reply, &rec_array);
+
+ if (!dbus_message_iter_open_container(&rec_array, DBUS_TYPE_ARRAY,
+ "a{q(yuv)}", &attr_array))
+ return FALSE;
+
+ for (seq = dev->tmp_records; seq; seq = seq->next) {
+ sdp_record_t *rec = (sdp_record_t *) seq->data;
+ if (!rec)
+ continue;
+ if (!append_record(rec, &attr_array))
+ return btd_error_failed(msg,
+ "SDP record attachment failed");
+ }
+
+ if (!dbus_message_iter_close_container(&rec_array, &attr_array))
+ return FALSE;
+
+ return reply;
+}
+
static const GDBusMethodTable device_methods[] = {
{ GDBUS_ASYNC_METHOD("Disconnect", NULL, NULL, dev_disconnect) },
{ GDBUS_ASYNC_METHOD("Connect", NULL, NULL, dev_connect) },
@@ -2771,6 +3061,9 @@ static const GDBusMethodTable device_methods[] = {
NULL, disconnect_profile) },
{ GDBUS_ASYNC_METHOD("Pair", NULL, NULL, pair_device) },
{ GDBUS_METHOD("CancelPairing", NULL, NULL, cancel_pairing) },
+ { GDBUS_ASYNC_METHOD("GetServiceRecords", NULL,
+ GDBUS_ARGS({"records", "aa{q(yuv)}"}),
+ get_service_records) },
{ }
};

--
2.24.1

2020-04-09 18:06:23

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 1/5] D-Bus API changes for managing SDP records

Hi Sonny,

> This defines the DBus API that we'll use with BlueZ to create, remove
> and get service records.
> ---
> doc/adapter-api.txt | 46 +++++++++++++++++++++++++++++++++++++++++++++
> doc/device-api.txt | 37 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 83 insertions(+)
>
> diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
> index acae032d9..6e4c37fc9 100644
> --- a/doc/adapter-api.txt
> +++ b/doc/adapter-api.txt
> @@ -204,6 +204,52 @@ Methods void StartDiscovery()
> org.bluez.Error.NotReady
> org.bluez.Error.Failed
>
> + uint32 CreateServiceRecord(dict record)
> +
> + This method creates an entry with the local SDP server
> + for this adapter for the specified record. This method
> + will only create the SDP record and not start listening
> + on any ports. It is up to the caller of the method to
> + ensure the validity of the service record. This record
> + will not be parsed for any validation but will instead
> + directly be inserted into the local SDP server’s
> + records.
> +
> + The return value from this method will be the 32 bit
> + handle for the created service record.
> +
> + The record dictionary will have dictionary entries of
> + the format: {id : (type, size, value)}, where,
> +
> + uint16 id: The 16 bit attribute ID for an
> + attribute.
> + uint8 type: This will contain the type of the
> + attribute value. Attribute type values
> + are defined in the Bluetooth spec in
> + Volume 3, Part B, 3.2.
> + uint32 size: This is the size of the attribute
> + value.
> + variant value: This will contain the attribute value
> + for a given attribute_id. This variant
> + can either contain a primitive type, or
> + if type is SEQUENCE, an array of struct
> + of the signature (type, size, value).
> +
> + Possible errors: org.bluez.Error.NotReady
> + org.bluez.Error.AlreadyExists
> + org.bluez.Error.Failed
> + org.bluez.Error.InvalidArguments
> +
> + void RemoveServiceRecord(uint32 handle)
> +
> + This method removes the SDP record with the given
> + handle from the local SDP server.
> +
> + Possible errors: org.bluez.Error.NotReady
> + org.bluez.Error.DoesNotExist
> + org.bluez.Error.Failed
> + org.bluez.Error.InvalidArguments
> +

so when design the BlueZ 5.x APIs, we on purpose didn’t do this. You are suppose to use doc/profile-api.txt for these kind of things.

I am not in favor of dangling SDP records where we have no lifetime guarantee of the service behind it.

If you look at profiles/iap/main.c then you see how you could write a vendor profile just as easily. That one is the skeleton for iOS accessory protocol over Bluetooth.

> Properties string Address [readonly]
>
> The Bluetooth device address.
> diff --git a/doc/device-api.txt b/doc/device-api.txt
> index ceb68d2f6..e8f2c670d 100644
> --- a/doc/device-api.txt
> +++ b/doc/device-api.txt
> @@ -120,6 +120,43 @@ Methods void Connect()
> Possible errors: org.bluez.Error.DoesNotExist
> org.bluez.Error.Failed
>
> + array{array{dict}} GetServiceRecords()
> +
> + This method returns the complete service records of all
> + discovered BR/EDR services of the connected device till
> + now. The return value will be an array of an array of
> + dictionary entries. Each nested array of dictionary
> + entries will contain one service record. Each pair in
> + the returned dictionary entries will represent an
> + attribute in the service record.
> +
> + The dictionary entries in the returned array of entries
> + will be of the format: {id : (type, size, value)} where,
> +
> + uint16 id: The 16 bit attribute ID for an
> + attribute.
> + uint8 type: This will contain the type of the
> + attribute value. Attribute type values
> + are defined in the Bluetooth spec in
> + Volume 3, Part B, 3.2.
> + uint32 size: This is the size of the attribute
> + value.
> + variant value: This will contain the attribute value
> + for a given attribute_id. This variant
> + can either contain a primitive type, or
> + if type is SEQUENCE, an array of struct
> + of the signature (type, size, value).
> +
> + Since the service record contains the service UUID and
> + name, these fields do not need to be given separately.
> + This method should be called after the ServiceResolved
> + property for this object has been set to true otherwise
> + it may return a partial or stale cached list.
> +
> + Possible errors: org.bluez.Error.NotReady
> + org.bluez.Error.NotConnected
> + org.bluez.Error.Failed
> +

These two things are not related and do not belong in the same patch. However what is your goal behind this API since even here we tried not to give raw SDP to the application. It is all baked into the Profile API.

Regards

Marcel

2020-04-09 18:27:27

by Sonny Sasaka

[permalink] [raw]
Subject: Re: [PATCH 1/5] D-Bus API changes for managing SDP records

Hi Marcel,

This was needed to support emulation of Android apps's Bluetooth API
on top of BlueZ. Android expects to be able to access the SDP database
of a remote device via its HAL interface
(https://android.googlesource.com/platform/system/bt/+/master/include/hardware/bt_sdp.h#38).

Thanks for the feedback, we will revisit this and see if a redesign
based on your suggestion is possible.

On Thu, Apr 9, 2020 at 11:05 AM Marcel Holtmann <[email protected]> wrote:
>
> Hi Sonny,
>
> > This defines the DBus API that we'll use with BlueZ to create, remove
> > and get service records.
> > ---
> > doc/adapter-api.txt | 46 +++++++++++++++++++++++++++++++++++++++++++++
> > doc/device-api.txt | 37 ++++++++++++++++++++++++++++++++++++
> > 2 files changed, 83 insertions(+)
> >
> > diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
> > index acae032d9..6e4c37fc9 100644
> > --- a/doc/adapter-api.txt
> > +++ b/doc/adapter-api.txt
> > @@ -204,6 +204,52 @@ Methods void StartDiscovery()
> > org.bluez.Error.NotReady
> > org.bluez.Error.Failed
> >
> > + uint32 CreateServiceRecord(dict record)
> > +
> > + This method creates an entry with the local SDP server
> > + for this adapter for the specified record. This method
> > + will only create the SDP record and not start listening
> > + on any ports. It is up to the caller of the method to
> > + ensure the validity of the service record. This record
> > + will not be parsed for any validation but will instead
> > + directly be inserted into the local SDP server’s
> > + records.
> > +
> > + The return value from this method will be the 32 bit
> > + handle for the created service record.
> > +
> > + The record dictionary will have dictionary entries of
> > + the format: {id : (type, size, value)}, where,
> > +
> > + uint16 id: The 16 bit attribute ID for an
> > + attribute.
> > + uint8 type: This will contain the type of the
> > + attribute value. Attribute type values
> > + are defined in the Bluetooth spec in
> > + Volume 3, Part B, 3.2.
> > + uint32 size: This is the size of the attribute
> > + value.
> > + variant value: This will contain the attribute value
> > + for a given attribute_id. This variant
> > + can either contain a primitive type, or
> > + if type is SEQUENCE, an array of struct
> > + of the signature (type, size, value).
> > +
> > + Possible errors: org.bluez.Error.NotReady
> > + org.bluez.Error.AlreadyExists
> > + org.bluez.Error.Failed
> > + org.bluez.Error.InvalidArguments
> > +
> > + void RemoveServiceRecord(uint32 handle)
> > +
> > + This method removes the SDP record with the given
> > + handle from the local SDP server.
> > +
> > + Possible errors: org.bluez.Error.NotReady
> > + org.bluez.Error.DoesNotExist
> > + org.bluez.Error.Failed
> > + org.bluez.Error.InvalidArguments
> > +
>
> so when design the BlueZ 5.x APIs, we on purpose didn’t do this. You are suppose to use doc/profile-api.txt for these kind of things.
>
> I am not in favor of dangling SDP records where we have no lifetime guarantee of the service behind it.
>
> If you look at profiles/iap/main.c then you see how you could write a vendor profile just as easily. That one is the skeleton for iOS accessory protocol over Bluetooth.
>
> > Properties string Address [readonly]
> >
> > The Bluetooth device address.
> > diff --git a/doc/device-api.txt b/doc/device-api.txt
> > index ceb68d2f6..e8f2c670d 100644
> > --- a/doc/device-api.txt
> > +++ b/doc/device-api.txt
> > @@ -120,6 +120,43 @@ Methods void Connect()
> > Possible errors: org.bluez.Error.DoesNotExist
> > org.bluez.Error.Failed
> >
> > + array{array{dict}} GetServiceRecords()
> > +
> > + This method returns the complete service records of all
> > + discovered BR/EDR services of the connected device till
> > + now. The return value will be an array of an array of
> > + dictionary entries. Each nested array of dictionary
> > + entries will contain one service record. Each pair in
> > + the returned dictionary entries will represent an
> > + attribute in the service record.
> > +
> > + The dictionary entries in the returned array of entries
> > + will be of the format: {id : (type, size, value)} where,
> > +
> > + uint16 id: The 16 bit attribute ID for an
> > + attribute.
> > + uint8 type: This will contain the type of the
> > + attribute value. Attribute type values
> > + are defined in the Bluetooth spec in
> > + Volume 3, Part B, 3.2.
> > + uint32 size: This is the size of the attribute
> > + value.
> > + variant value: This will contain the attribute value
> > + for a given attribute_id. This variant
> > + can either contain a primitive type, or
> > + if type is SEQUENCE, an array of struct
> > + of the signature (type, size, value).
> > +
> > + Since the service record contains the service UUID and
> > + name, these fields do not need to be given separately.
> > + This method should be called after the ServiceResolved
> > + property for this object has been set to true otherwise
> > + it may return a partial or stale cached list.
> > +
> > + Possible errors: org.bluez.Error.NotReady
> > + org.bluez.Error.NotConnected
> > + org.bluez.Error.Failed
> > +
>
> These two things are not related and do not belong in the same patch. However what is your goal behind this API since even here we tried not to give raw SDP to the application. It is all baked into the Profile API.
>
> Regards
>
> Marcel
>

2020-04-09 18:37:18

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 1/5] D-Bus API changes for managing SDP records

Hi Sonny,

On Thu, Apr 9, 2020 at 11:28 AM Sonny Sasaka <[email protected]> wrote:
>
> Hi Marcel,
>
> This was needed to support emulation of Android apps's Bluetooth API
> on top of BlueZ. Android expects to be able to access the SDP database
> of a remote device via its HAL interface
> (https://android.googlesource.com/platform/system/bt/+/master/include/hardware/bt_sdp.h#38).

I suppose you could read the cache directly, though there is no
guarantee that we won't change the format of that, btw I suppose
services handled by the daemon should not be exposed to the Android
side otherwise you may have conflicts so going with Profile interface
is probably the way to go, but you will need to know the UUIDs that
would be handled by Android.

> Thanks for the feedback, we will revisit this and see if a redesign
> based on your suggestion is possible.
>
> On Thu, Apr 9, 2020 at 11:05 AM Marcel Holtmann <[email protected]> wrote:
> >
> > Hi Sonny,
> >
> > > This defines the DBus API that we'll use with BlueZ to create, remove
> > > and get service records.
> > > ---
> > > doc/adapter-api.txt | 46 +++++++++++++++++++++++++++++++++++++++++++++
> > > doc/device-api.txt | 37 ++++++++++++++++++++++++++++++++++++
> > > 2 files changed, 83 insertions(+)
> > >
> > > diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
> > > index acae032d9..6e4c37fc9 100644
> > > --- a/doc/adapter-api.txt
> > > +++ b/doc/adapter-api.txt
> > > @@ -204,6 +204,52 @@ Methods void StartDiscovery()
> > > org.bluez.Error.NotReady
> > > org.bluez.Error.Failed
> > >
> > > + uint32 CreateServiceRecord(dict record)
> > > +
> > > + This method creates an entry with the local SDP server
> > > + for this adapter for the specified record. This method
> > > + will only create the SDP record and not start listening
> > > + on any ports. It is up to the caller of the method to
> > > + ensure the validity of the service record. This record
> > > + will not be parsed for any validation but will instead
> > > + directly be inserted into the local SDP server’s
> > > + records.
> > > +
> > > + The return value from this method will be the 32 bit
> > > + handle for the created service record.
> > > +
> > > + The record dictionary will have dictionary entries of
> > > + the format: {id : (type, size, value)}, where,
> > > +
> > > + uint16 id: The 16 bit attribute ID for an
> > > + attribute.
> > > + uint8 type: This will contain the type of the
> > > + attribute value. Attribute type values
> > > + are defined in the Bluetooth spec in
> > > + Volume 3, Part B, 3.2.
> > > + uint32 size: This is the size of the attribute
> > > + value.
> > > + variant value: This will contain the attribute value
> > > + for a given attribute_id. This variant
> > > + can either contain a primitive type, or
> > > + if type is SEQUENCE, an array of struct
> > > + of the signature (type, size, value).
> > > +
> > > + Possible errors: org.bluez.Error.NotReady
> > > + org.bluez.Error.AlreadyExists
> > > + org.bluez.Error.Failed
> > > + org.bluez.Error.InvalidArguments
> > > +
> > > + void RemoveServiceRecord(uint32 handle)
> > > +
> > > + This method removes the SDP record with the given
> > > + handle from the local SDP server.
> > > +
> > > + Possible errors: org.bluez.Error.NotReady
> > > + org.bluez.Error.DoesNotExist
> > > + org.bluez.Error.Failed
> > > + org.bluez.Error.InvalidArguments
> > > +
> >
> > so when design the BlueZ 5.x APIs, we on purpose didn’t do this. You are suppose to use doc/profile-api.txt for these kind of things.
> >
> > I am not in favor of dangling SDP records where we have no lifetime guarantee of the service behind it.
> >
> > If you look at profiles/iap/main.c then you see how you could write a vendor profile just as easily. That one is the skeleton for iOS accessory protocol over Bluetooth.
> >
> > > Properties string Address [readonly]
> > >
> > > The Bluetooth device address.
> > > diff --git a/doc/device-api.txt b/doc/device-api.txt
> > > index ceb68d2f6..e8f2c670d 100644
> > > --- a/doc/device-api.txt
> > > +++ b/doc/device-api.txt
> > > @@ -120,6 +120,43 @@ Methods void Connect()
> > > Possible errors: org.bluez.Error.DoesNotExist
> > > org.bluez.Error.Failed
> > >
> > > + array{array{dict}} GetServiceRecords()
> > > +
> > > + This method returns the complete service records of all
> > > + discovered BR/EDR services of the connected device till
> > > + now. The return value will be an array of an array of
> > > + dictionary entries. Each nested array of dictionary
> > > + entries will contain one service record. Each pair in
> > > + the returned dictionary entries will represent an
> > > + attribute in the service record.
> > > +
> > > + The dictionary entries in the returned array of entries
> > > + will be of the format: {id : (type, size, value)} where,
> > > +
> > > + uint16 id: The 16 bit attribute ID for an
> > > + attribute.
> > > + uint8 type: This will contain the type of the
> > > + attribute value. Attribute type values
> > > + are defined in the Bluetooth spec in
> > > + Volume 3, Part B, 3.2.
> > > + uint32 size: This is the size of the attribute
> > > + value.
> > > + variant value: This will contain the attribute value
> > > + for a given attribute_id. This variant
> > > + can either contain a primitive type, or
> > > + if type is SEQUENCE, an array of struct
> > > + of the signature (type, size, value).
> > > +
> > > + Since the service record contains the service UUID and
> > > + name, these fields do not need to be given separately.
> > > + This method should be called after the ServiceResolved
> > > + property for this object has been set to true otherwise
> > > + it may return a partial or stale cached list.
> > > +
> > > + Possible errors: org.bluez.Error.NotReady
> > > + org.bluez.Error.NotConnected
> > > + org.bluez.Error.Failed
> > > +
> >
> > These two things are not related and do not belong in the same patch. However what is your goal behind this API since even here we tried not to give raw SDP to the application. It is all baked into the Profile API.
> >
> > Regards
> >
> > Marcel
> >



--
Luiz Augusto von Dentz

2020-04-09 21:09:09

by Sonny Sasaka

[permalink] [raw]
Subject: Re: [PATCH 1/5] D-Bus API changes for managing SDP records

Thanks for the feedback, Luiz. We will explore the ways you all suggested.

On Thu, Apr 9, 2020 at 11:35 AM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi Sonny,
>
> On Thu, Apr 9, 2020 at 11:28 AM Sonny Sasaka <[email protected]> wrote:
> >
> > Hi Marcel,
> >
> > This was needed to support emulation of Android apps's Bluetooth API
> > on top of BlueZ. Android expects to be able to access the SDP database
> > of a remote device via its HAL interface
> > (https://android.googlesource.com/platform/system/bt/+/master/include/hardware/bt_sdp.h#38).
>
> I suppose you could read the cache directly, though there is no
> guarantee that we won't change the format of that, btw I suppose
> services handled by the daemon should not be exposed to the Android
> side otherwise you may have conflicts so going with Profile interface
> is probably the way to go, but you will need to know the UUIDs that
> would be handled by Android.
>
> > Thanks for the feedback, we will revisit this and see if a redesign
> > based on your suggestion is possible.
> >
> > On Thu, Apr 9, 2020 at 11:05 AM Marcel Holtmann <[email protected]> wrote:
> > >
> > > Hi Sonny,
> > >
> > > > This defines the DBus API that we'll use with BlueZ to create, remove
> > > > and get service records.
> > > > ---
> > > > doc/adapter-api.txt | 46 +++++++++++++++++++++++++++++++++++++++++++++
> > > > doc/device-api.txt | 37 ++++++++++++++++++++++++++++++++++++
> > > > 2 files changed, 83 insertions(+)
> > > >
> > > > diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
> > > > index acae032d9..6e4c37fc9 100644
> > > > --- a/doc/adapter-api.txt
> > > > +++ b/doc/adapter-api.txt
> > > > @@ -204,6 +204,52 @@ Methods void StartDiscovery()
> > > > org.bluez.Error.NotReady
> > > > org.bluez.Error.Failed
> > > >
> > > > + uint32 CreateServiceRecord(dict record)
> > > > +
> > > > + This method creates an entry with the local SDP server
> > > > + for this adapter for the specified record. This method
> > > > + will only create the SDP record and not start listening
> > > > + on any ports. It is up to the caller of the method to
> > > > + ensure the validity of the service record. This record
> > > > + will not be parsed for any validation but will instead
> > > > + directly be inserted into the local SDP server’s
> > > > + records.
> > > > +
> > > > + The return value from this method will be the 32 bit
> > > > + handle for the created service record.
> > > > +
> > > > + The record dictionary will have dictionary entries of
> > > > + the format: {id : (type, size, value)}, where,
> > > > +
> > > > + uint16 id: The 16 bit attribute ID for an
> > > > + attribute.
> > > > + uint8 type: This will contain the type of the
> > > > + attribute value. Attribute type values
> > > > + are defined in the Bluetooth spec in
> > > > + Volume 3, Part B, 3.2.
> > > > + uint32 size: This is the size of the attribute
> > > > + value.
> > > > + variant value: This will contain the attribute value
> > > > + for a given attribute_id. This variant
> > > > + can either contain a primitive type, or
> > > > + if type is SEQUENCE, an array of struct
> > > > + of the signature (type, size, value).
> > > > +
> > > > + Possible errors: org.bluez.Error.NotReady
> > > > + org.bluez.Error.AlreadyExists
> > > > + org.bluez.Error.Failed
> > > > + org.bluez.Error.InvalidArguments
> > > > +
> > > > + void RemoveServiceRecord(uint32 handle)
> > > > +
> > > > + This method removes the SDP record with the given
> > > > + handle from the local SDP server.
> > > > +
> > > > + Possible errors: org.bluez.Error.NotReady
> > > > + org.bluez.Error.DoesNotExist
> > > > + org.bluez.Error.Failed
> > > > + org.bluez.Error.InvalidArguments
> > > > +
> > >
> > > so when design the BlueZ 5.x APIs, we on purpose didn’t do this. You are suppose to use doc/profile-api.txt for these kind of things.
> > >
> > > I am not in favor of dangling SDP records where we have no lifetime guarantee of the service behind it.
> > >
> > > If you look at profiles/iap/main.c then you see how you could write a vendor profile just as easily. That one is the skeleton for iOS accessory protocol over Bluetooth.
> > >
> > > > Properties string Address [readonly]
> > > >
> > > > The Bluetooth device address.
> > > > diff --git a/doc/device-api.txt b/doc/device-api.txt
> > > > index ceb68d2f6..e8f2c670d 100644
> > > > --- a/doc/device-api.txt
> > > > +++ b/doc/device-api.txt
> > > > @@ -120,6 +120,43 @@ Methods void Connect()
> > > > Possible errors: org.bluez.Error.DoesNotExist
> > > > org.bluez.Error.Failed
> > > >
> > > > + array{array{dict}} GetServiceRecords()
> > > > +
> > > > + This method returns the complete service records of all
> > > > + discovered BR/EDR services of the connected device till
> > > > + now. The return value will be an array of an array of
> > > > + dictionary entries. Each nested array of dictionary
> > > > + entries will contain one service record. Each pair in
> > > > + the returned dictionary entries will represent an
> > > > + attribute in the service record.
> > > > +
> > > > + The dictionary entries in the returned array of entries
> > > > + will be of the format: {id : (type, size, value)} where,
> > > > +
> > > > + uint16 id: The 16 bit attribute ID for an
> > > > + attribute.
> > > > + uint8 type: This will contain the type of the
> > > > + attribute value. Attribute type values
> > > > + are defined in the Bluetooth spec in
> > > > + Volume 3, Part B, 3.2.
> > > > + uint32 size: This is the size of the attribute
> > > > + value.
> > > > + variant value: This will contain the attribute value
> > > > + for a given attribute_id. This variant
> > > > + can either contain a primitive type, or
> > > > + if type is SEQUENCE, an array of struct
> > > > + of the signature (type, size, value).
> > > > +
> > > > + Since the service record contains the service UUID and
> > > > + name, these fields do not need to be given separately.
> > > > + This method should be called after the ServiceResolved
> > > > + property for this object has been set to true otherwise
> > > > + it may return a partial or stale cached list.
> > > > +
> > > > + Possible errors: org.bluez.Error.NotReady
> > > > + org.bluez.Error.NotConnected
> > > > + org.bluez.Error.Failed
> > > > +
> > >
> > > These two things are not related and do not belong in the same patch. However what is your goal behind this API since even here we tried not to give raw SDP to the application. It is all baked into the Profile API.
> > >
> > > Regards
> > >
> > > Marcel
> > >
>
>
>
> --
> Luiz Augusto von Dentz

2020-04-10 06:58:04

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 1/5] D-Bus API changes for managing SDP records

Hi Sonny,

> This was needed to support emulation of Android apps's Bluetooth API
> on top of BlueZ. Android expects to be able to access the SDP database
> of a remote device via its HAL interface
> (https://android.googlesource.com/platform/system/bt/+/master/include/hardware/bt_sdp.h#38).
>
> Thanks for the feedback, we will revisit this and see if a redesign
> based on your suggestion is possible.

if you can get this done by using existing APIs and the Profile API, then that would be best. I would have to get myself a bit more familiar with the Android APIs. However in the worst case, we can introduce an Android specific D-Bus API as part of a plugin. It would be then disabled in standard Linux systems.

From BlueZ for Android project, we fundamentally took the Android HAL parts 1:1 and mapped them to a Unix based protocol. Then the Android bluetoothd had to do the rest of the work. A similar approach (and using D-Bus) might be most efficient here actually.

However as Luiz noted, we need to be careful that bluetoothd system services / profiles are not messed with. Meaning bluetoothd has to play guardian on what Android apps can do.

On a side note, I am going to remove the BlueZ for Android code from the tree. Let me know if you find parts of it still useful. It is git, so we can always bring it back, but if you already have some idea on how to utilize it, feel free to start with that right away.

Regards

Marcel