[PATCH 1/6] android: Initial implementation of get_remote_services

[PATCH 1/6] android: Initial implementation of get_remote_services

[Marcin Kraglak]

This patch implements method to retrieve remote device sdp records.
Caching struct is implemented for adding fetched uuids to list.
sdp_req_list will contain list of devices which are asked for records.
Function get_remote_services will check if device is on list, and will
start sdp procedure.
---
 android/adapter.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 102 insertions(+), 9 deletions(-)

diff --git a/android/adapter.c b/android/adapter.c
index 929e8cb..027af25 100644
--- a/android/adapter.c
+++ b/android/adapter.c
@@ -35,6 +35,10 @@
 #include "src/shared/mgmt.h"
 #include "src/glib-helper.h"
 #include "src/eir.h"
+#include "lib/sdp.h"
+#include "lib/sdp_lib.h"
+#include "lib/uuid.h"
+#include "src/sdp-client.h"
 #include "log.h"
 #include "hal-msg.h"
 #include "ipc.h"
@@ -45,6 +49,8 @@
 #define DEFAULT_IO_CAPABILITY 0x01
 
 static GIOChannel *notification_io = NULL;
+/* This list contains addresses which are asked for records */
+static GSList *sdp_req_list;
 
 struct bt_adapter {
 	uint16_t index;
@@ -63,6 +69,20 @@ struct bt_adapter {
 	bool discovering;
 };
 
+struct browse_req {
+	bdaddr_t bdaddr;
+	GSList *uuids;
+	int search_uuid;
+	int reconnect_attempt;
+};
+
+static const uint16_t uuid_list[] = {
+	L2CAP_UUID,
+	PNP_INFO_SVCLASS_ID,
+	PUBLIC_BROWSE_GROUP,
+	0
+};
+
 static struct bt_adapter *adapter;
 static GSList *found_devices = NULL;
 
@@ -257,6 +277,79 @@ static void send_bond_state_change(const bdaddr_t *addr, uint8_t status,
 			HAL_EV_BOND_STATE_CHANGED, sizeof(ev), &ev, -1);
 }
 
+static void browse_req_free(struct browse_req *req)
+{
+	g_slist_free_full(req->uuids, g_free);
+	g_free(req);
+}
+
+static void update_records(struct browse_req *req, sdp_list_t *recs)
+{
+	/* TODO cache found uuids */
+}
+
+static void browse_cb(sdp_list_t *recs, int err, gpointer user_data)
+{
+	struct browse_req *req = user_data;
+	uuid_t uuid;
+
+	/* If we have a valid response and req->search_uuid == 2, then L2CAP
+	 * UUID & PNP searching was successful -- we are done */
+	if (err < 0 || req->search_uuid == 2) {
+		if (err == -ECONNRESET && req->reconnect_attempt < 1) {
+			req->search_uuid--;
+			req->reconnect_attempt++;
+		} else {
+			goto done;
+		}
+	}
+
+	update_records(req, recs);
+
+	/* Search for mandatory uuids */
+	if (uuid_list[req->search_uuid]) {
+		sdp_uuid16_create(&uuid, uuid_list[req->search_uuid++]);
+		bt_search_service(&adapter->bdaddr, &req->bdaddr, &uuid,
+						browse_cb, user_data, NULL);
+		return;
+	}
+
+done:
+	sdp_req_list = g_slist_remove(sdp_req_list, &req->bdaddr);
+	browse_req_free(req);
+}
+
+static int bdaddr_cmp(gconstpointer a, gconstpointer b)
+{
+	const bdaddr_t *bda = a;
+	const bdaddr_t *bdb = b;
+
+	return bacmp(bdb, bda);
+}
+
+static bool fetch_remote_uuids(const bdaddr_t *addr)
+{
+	struct browse_req *req;
+	uuid_t uuid;
+
+	if (g_slist_find_custom(sdp_req_list, addr, bdaddr_cmp))
+		return false;
+
+	req = g_new0(struct browse_req, 1);
+	bacpy(&req->bdaddr, addr);
+	sdp_req_list = g_slist_append(sdp_req_list, &req->bdaddr);
+	sdp_uuid16_create(&uuid, uuid_list[req->search_uuid++]);
+
+	if (bt_search_service(&adapter->bdaddr,
+			&req->bdaddr, &uuid, browse_cb, req, NULL) < 0) {
+		sdp_req_list = g_slist_remove(sdp_req_list, &req->bdaddr);
+		browse_req_free(req);
+		return false;
+	}
+
+	return true;
+}
+
 static void new_link_key_callback(uint16_t index, uint16_t length,
 					const void *param, void *user_data)
 {
@@ -445,14 +538,6 @@ static void confirm_device_name(const bdaddr_t *addr, uint8_t addr_type)
 		error("Failed to send confirm name request");
 }
 
-static int bdaddr_cmp(gconstpointer a, gconstpointer b)
-{
-	const bdaddr_t *bda = a;
-	const bdaddr_t *bdb = b;
-
-	return bacmp(bdb, bda);
-}
-
 static int fill_device_props(struct hal_property *prop, bdaddr_t *addr,
 					uint32_t cod, int8_t rssi, char *name)
 {
@@ -1477,7 +1562,15 @@ static uint8_t ssp_reply(void *buf, uint16_t len)
 
 static uint8_t get_remote_services(void *buf, uint16_t len)
 {
-	return HAL_STATUS_UNSUPPORTED;
+	struct hal_cmd_get_remote_services *cmd = buf;
+	bool ret;
+	bdaddr_t addr;
+
+	android2bdaddr(&cmd->bdaddr, &addr);
+
+	ret = fetch_remote_uuids(&addr);
+
+	return ret ? HAL_STATUS_SUCCESS : HAL_STATUS_FAILED;
 }
 
 void bt_adapter_handle_cmd(GIOChannel *io, uint8_t opcode, void *buf,
-- 
1.8.4.1

[PATCH 2/6] android: Fetch remote device uuids after pairing

[Marcin Kraglak]

Android framework expects list of bonded device's uuids.
Start sdp query after setting bond state to BOND_STATE_BONBED.
---
 android/adapter.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/android/adapter.c b/android/adapter.c
index 027af25..692ffe4 100644
--- a/android/adapter.c
+++ b/android/adapter.c
@@ -382,6 +382,8 @@ static void new_link_key_callback(uint16_t index, uint16_t length,
 
 	send_bond_state_change(&addr->bdaddr, HAL_STATUS_SUCCESS,
 							HAL_BOND_STATE_BONDED);
+
+	fetch_remote_uuids(&addr->bdaddr);
 }
 
 static void pin_code_request_callback(uint16_t index, uint16_t length,
-- 
1.8.4.1

[PATCH 3/6] android: Pass found uuids to remote_device_properties_cb

[Marcin Kraglak]

Send remote device's uuids in remote_device_properties_cb.
This patch will pack found uuids to buffer containing property
with list of uuids and send it to notification socket.
---
 android/adapter.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 81 insertions(+), 1 deletion(-)

diff --git a/android/adapter.c b/android/adapter.c
index 692ffe4..64f0192 100644
--- a/android/adapter.c
+++ b/android/adapter.c
@@ -283,9 +283,87 @@ static void browse_req_free(struct browse_req *req)
 	g_free(req);
 }
 
+static void fill_uuids(GSList *list, void *buf)
+{
+	for (; list; list = g_slist_next(list)) {
+		memcpy(buf, list->data, sizeof(uint128_t));
+		buf += sizeof(uint128_t);
+	}
+}
+
+static void remote_uuids_callback(struct browse_req *req)
+{
+	struct hal_ev_remote_device_props *ev;
+	int len;
+
+	len = sizeof(*ev) + sizeof(struct hal_property) + (sizeof(uint128_t) *
+						g_slist_length(req->uuids));
+	ev = g_malloc(len);
+
+	ev->status = HAL_STATUS_SUCCESS;
+	bdaddr2android(&req->bdaddr, &ev->bdaddr);
+	ev->num_props = 1;
+	ev->props[0].type = HAL_PROP_DEVICE_UUIDS;
+	ev->props[0].len = sizeof(uint128_t) * g_slist_length(req->uuids);
+	fill_uuids(req->uuids, ev->props[0].val);
+
+	ipc_send(notification_io, HAL_SERVICE_ID_BLUETOOTH,
+				HAL_EV_REMOTE_DEVICE_PROPS, len, ev, -1);
+
+	g_free(ev);
+}
+
+static int uuid_128_cmp(gconstpointer a, gconstpointer b)
+{
+	return memcmp(a, b, sizeof(uint128_t));
+}
+
 static void update_records(struct browse_req *req, sdp_list_t *recs)
 {
-	/* TODO cache found uuids */
+	for (; recs; recs = recs->next) {
+		sdp_record_t *rec = (sdp_record_t *) recs->data;
+		sdp_list_t *svcclass = NULL;
+		uuid_t uuid128;
+		uuid_t *tmp;
+		uint8_t *new_uuid;
+
+		if (!rec)
+			break;
+
+		if (sdp_get_service_classes(rec, &svcclass) < 0)
+			continue;
+
+		if (!svcclass)
+			continue;
+
+		tmp = svcclass->data;
+
+		switch (tmp->type) {
+		case SDP_UUID16:
+			sdp_uuid16_to_uuid128(&uuid128, tmp);
+			break;
+		case SDP_UUID32:
+			sdp_uuid32_to_uuid128(&uuid128, tmp);
+			break;
+		case SDP_UUID128:
+			memcpy(&uuid128, tmp, sizeof(uuid_t));
+			break;
+		default:
+			continue;
+		}
+
+		new_uuid = g_malloc(16);/* size of 128 bit uuid */
+		memcpy(new_uuid, &uuid128.value.uuid128,
+				sizeof(uuid128.value.uuid128));
+
+		/* Check if uuid is already added */
+		if (g_slist_find_custom(req->uuids, new_uuid, uuid_128_cmp))
+			g_free(new_uuid);
+		else
+			req->uuids = g_slist_append(req->uuids, new_uuid);
+
+		sdp_list_free(svcclass, free);
+	}
 }
 
 static void browse_cb(sdp_list_t *recs, int err, gpointer user_data)
@@ -315,6 +393,8 @@ static void browse_cb(sdp_list_t *recs, int err, gpointer user_data)
 	}
 
 done:
+	remote_uuids_callback(req);
+
 	sdp_req_list = g_slist_remove(sdp_req_list, &req->bdaddr);
 	browse_req_free(req);
 }
-- 
1.8.4.1

[PATCH 4/6] android: Add supported uuids when adapter is initialized

[Marcin Kraglak]

It will set class of device with proper service hints.
We set it statically because we want to keep code simple.
---
 android/adapter.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/android/adapter.c b/android/adapter.c
index 64f0192..d791450 100644
--- a/android/adapter.c
+++ b/android/adapter.c
@@ -52,6 +52,29 @@ static GIOChannel *notification_io = NULL;
 /* This list contains addresses which are asked for records */
 static GSList *sdp_req_list;
 
+/*
+ * This is an array of supported uuids and service hints. We add them via mgmt
+ * interface when adapter is initialized. Uuids are in reverse orded.
+ */
+static const struct mgmt_cp_add_uuid supported_services[] = {
+	/* OBEX_OPP_UUID */
+	{ .uuid = { 0xfb, 0x34, 0x9b, 0x5f, 0x80, 0x00, 0x00, 0x80,
+			0x00, 0x10, 0x00, 0x00, 0x05, 0x11, 0x00, 0x00 },
+	.svc_hint = 0x10 },
+	/* HFP_AG_UUID */
+	{ .uuid = { 0xfb, 0x34, 0x9b, 0x5f, 0x80, 0x00, 0x00, 0x80,
+			0x00, 0x10, 0x00, 0x00, 0x1f, 0x11, 0x00, 0x00 },
+	.svc_hint = 0x40 },
+	/* ADVANCED_AUDIO_UUID */
+	{ .uuid = { 0xfb, 0x34, 0x9b, 0x5f, 0x80, 0x00, 0x00, 0x80,
+			0x00, 0x10, 0x00, 0x00, 0x0d, 0x11, 0x00, 0x00 },
+	.svc_hint = 0x08 },
+	/* PANU_UUID */
+	{ .uuid = { 0xfb, 0x34, 0x9b, 0x5f, 0x80, 0x00, 0x00, 0x80,
+			0x00, 0x10, 0x00, 0x00, 0x15, 0x11, 0x00, 0x00 },
+	.svc_hint = 0x02 }
+};
+
 struct bt_adapter {
 	uint16_t index;
 	struct mgmt *mgmt;
@@ -967,6 +990,39 @@ static void load_link_keys(GSList *keys)
 	}
 }
 
+static void add_uuid_complete(uint8_t status, uint16_t length,
+				const void *param, void *user_data)
+{
+	if (status != MGMT_STATUS_SUCCESS) {
+		error("Failed to add UUID: %s (0x%02x)",
+				mgmt_errstr(status), status);
+		return;
+	}
+
+	mgmt_dev_class_changed_event(adapter->index, length, param, NULL);
+}
+
+static void add_uuids(void)
+{
+	unsigned int i;
+
+	for (i = 0; i < NELEM(supported_services); i++)
+		mgmt_send(adapter->mgmt, MGMT_OP_ADD_UUID, adapter->index,
+				sizeof(supported_services[i]),
+				&supported_services[i], add_uuid_complete,
+				NULL, NULL);
+}
+
+static void clear_uuids(void)
+{
+	struct mgmt_cp_remove_uuid cp;
+
+	memset(&cp, 0, sizeof(cp));
+
+	mgmt_send(adapter->mgmt, MGMT_OP_REMOVE_UUID, adapter->index,
+					sizeof(cp), &cp, NULL, NULL, NULL);
+}
+
 static void set_mode_complete(uint8_t status, uint16_t length,
 					const void *param, void *user_data)
 {
@@ -982,6 +1038,9 @@ static void set_mode_complete(uint8_t status, uint16_t length,
 	 * event handling functions here.
 	 */
 	new_settings_callback(adapter->index, length, param, NULL);
+
+	clear_uuids();
+	add_uuids();
 }
 
 static bool set_mode(uint16_t opcode, uint8_t mode)
-- 
1.8.4.1

[PATCH 5/6] android: Implement class of device property callback

[Marcin Kraglak]

This will send adapter property with class of device
to notification socket.
---
 android/adapter.c | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/android/adapter.c b/android/adapter.c
index d791450..523209e 100644
--- a/android/adapter.c
+++ b/android/adapter.c
@@ -185,6 +185,30 @@ static void scan_mode_changed(void)
 	g_free(ev);
 }
 
+static void send_adapter_class(void)
+{
+	struct hal_ev_adapter_props_changed *ev;
+	int len;
+
+	len = sizeof(*ev) + sizeof(struct hal_property) + sizeof(uint32_t);
+
+	ev = g_malloc(len);
+
+	ev->num_props = 1;
+	ev->status = HAL_STATUS_SUCCESS;
+
+	ev->props[0].type = HAL_PROP_ADAPTER_CLASS;
+	ev->props[0].len = sizeof(uint32_t);
+	memcpy(ev->props->val, &adapter->dev_class, sizeof(uint32_t));
+
+	DBG("Adapter class %u", adapter->dev_class);
+
+	ipc_send(notification_io, HAL_SERVICE_ID_BLUETOOTH,
+				HAL_EV_ADAPTER_PROPS_CHANGED, len, ev, -1);
+
+	g_free(ev);
+}
+
 static void adapter_name_changed(const uint8_t *name)
 {
 	struct hal_ev_adapter_props_changed *ev;
@@ -275,7 +299,7 @@ static void mgmt_dev_class_changed_event(uint16_t index, uint16_t length,
 
 	adapter->dev_class = dev_class;
 
-	/* TODO: Inform prop change: Class */
+	send_adapter_class();
 
 	/* TODO: Gatt attrib set*/
 }
@@ -1265,11 +1289,11 @@ static bool get_uuids(void)
 
 static bool get_class(void)
 {
-	DBG("Not implemented");
+	DBG("");
 
-	/* TODO: Add implementation */
+	send_adapter_class();
 
-	return false;
+	return true;
 }
 
 static bool get_type(void)
-- 
1.8.4.1

[PATCH 6/6] android: Add support for getting adapter uuids

[Marcin Kraglak]

Implement get adapter uuids command. It will pack supported
uuids to structure and send it via notification socket
---
 android/adapter.c | 40 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 37 insertions(+), 3 deletions(-)

diff --git a/android/adapter.c b/android/adapter.c
index 523209e..483bb1c 100644
--- a/android/adapter.c
+++ b/android/adapter.c
@@ -1278,13 +1278,47 @@ static bool get_name(void)
 	return true;
 }
 
+static void swap_uuid(const uint8_t *src, uint8_t *dst)
+{
+	int i;
+
+	for (i = 0; i < 16; i++)
+		dst[15 - i] = src[i];
+}
+
 static bool get_uuids(void)
 {
-	DBG("Not implemented");
+	struct hal_ev_adapter_props_changed *ev;
+	unsigned int i;
+	int len;
+	uint8_t *p;
 
-	/* TODO: Add implementation */
+	len = sizeof(*ev) + sizeof(struct hal_property) +
+			NELEM(supported_services) * sizeof(uint128_t);
 
-	return false;
+	ev = g_malloc(len);
+
+	ev->num_props = 1;
+	ev->status = HAL_STATUS_SUCCESS;
+
+	ev->props[0].type = HAL_PROP_ADAPTER_UUIDS;
+	ev->props[0].len = sizeof(uint128_t) * NELEM(supported_services);
+	p = ev->props->val;
+
+	for (i = 0; i < NELEM(supported_services); i++) {
+		/* we have to swap supported uuids
+		 * because hal expects them in reversed order
+		 * than mgmt intrface */
+		swap_uuid(supported_services[i].uuid, p);
+		p += sizeof(uint128_t);
+	}
+
+	ipc_send(notification_io, HAL_SERVICE_ID_BLUETOOTH,
+				HAL_EV_ADAPTER_PROPS_CHANGED, len, ev, -1);
+
+	g_free(ev);
+
+	return true;
 }
 
 static bool get_class(void)
-- 
1.8.4.1

Re: [PATCH 1/6] android: Initial implementation of get_remote_services

[Johan Hedberg]

Hi Marcin,

On Tue, Nov 05, 2013, Marcin Kraglak wrote:
> +static int bdaddr_cmp(gconstpointer a, gconstpointer b)
> +{
> +	const bdaddr_t *bda = a;
> +	const bdaddr_t *bdb = b;
> +
> +	return bacmp(bdb, bda);
> +}
> +
> +static bool fetch_remote_uuids(const bdaddr_t *addr)
> +{
> +	struct browse_req *req;
> +	uuid_t uuid;
> +
> +	if (g_slist_find_custom(sdp_req_list, addr, bdaddr_cmp))
> +		return false;
> +
> +	req = g_new0(struct browse_req, 1);
> +	bacpy(&req->bdaddr, addr);
> +	sdp_req_list = g_slist_append(sdp_req_list, &req->bdaddr);

This is quite wild coding, having a list with pointers into the middle
of a struct instead of the struct itself. To avoid having to twist ones
brain around to figure out correctness, could you just store the request
pointers in this list and then modify your bdaddr_cmp function to
compare a struct browse_req against a bdaddr_t pointer instead of
comparing two bdaddr pointers. You'll probably want to rename it to
req_cmp or something similar then though.

> +	sdp_uuid16_create(&uuid, uuid_list[req->search_uuid++]);
> +
> +	if (bt_search_service(&adapter->bdaddr,
> +			&req->bdaddr, &uuid, browse_cb, req, NULL) < 0) {
> +		sdp_req_list = g_slist_remove(sdp_req_list, &req->bdaddr);
> +		browse_req_free(req);

Instead of having to do an extra g_slist_remove here why not just add
the entry to the sdp_req_list only after bt_search_service has been
successful?

Also, how about renaming sdp_req_list to browse_req_list to match the
name you've chosen for the context struct? Or maybe even simpler as
"browse_reqs"

>  static uint8_t get_remote_services(void *buf, uint16_t len)
>  {
> -	return HAL_STATUS_UNSUPPORTED;
> +	struct hal_cmd_get_remote_services *cmd = buf;
> +	bool ret;
> +	bdaddr_t addr;
> +
> +	android2bdaddr(&cmd->bdaddr, &addr);
> +
> +	ret = fetch_remote_uuids(&addr);
> +
> +	return ret ? HAL_STATUS_SUCCESS : HAL_STATUS_FAILED;
>  }

How about just making fetch_remote_uuids() return a proper HAL status
directly instead of this mapping of bool to HAL status? You could also
just consider merging the entire fetch_remote_uuids function (whose name
I don't really like btw) into this get_remote_services function,
especially if you don't foresee it having any other callers in the
future.

Johan

Re: [PATCH 1/6] android: Initial implementation of get_remote_services

[Marcin Kraglak]

Hi Johan,

On 5 November 2013 11:57, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Marcin,
>
> On Tue, Nov 05, 2013, Marcin Kraglak wrote:
>> +static int bdaddr_cmp(gconstpointer a, gconstpointer b)
>> +{
>> +     const bdaddr_t *bda = a;
>> +     const bdaddr_t *bdb = b;
>> +
>> +     return bacmp(bdb, bda);
>> +}
>> +
>> +static bool fetch_remote_uuids(const bdaddr_t *addr)
>> +{
>> +     struct browse_req *req;
>> +     uuid_t uuid;
>> +
>> +     if (g_slist_find_custom(sdp_req_list, addr, bdaddr_cmp))
>> +             return false;
>> +
>> +     req = g_new0(struct browse_req, 1);
>> +     bacpy(&req->bdaddr, addr);
>> +     sdp_req_list = g_slist_append(sdp_req_list, &req->bdaddr);
>
> This is quite wild coding, having a list with pointers into the middle
> of a struct instead of the struct itself. To avoid having to twist ones
> brain around to figure out correctness, could you just store the request
> pointers in this list and then modify your bdaddr_cmp function to
> compare a struct browse_req against a bdaddr_t pointer instead of
> comparing two bdaddr pointers. You'll probably want to rename it to
> req_cmp or something similar then though.

Ok, I'll change it, it will be more readable.

>
>> +     sdp_uuid16_create(&uuid, uuid_list[req->search_uuid++]);
>> +
>> +     if (bt_search_service(&adapter->bdaddr,
>> +                     &req->bdaddr, &uuid, browse_cb, req, NULL) < 0) {
>> +             sdp_req_list = g_slist_remove(sdp_req_list, &req->bdaddr);
>> +             browse_req_free(req);
>
> Instead of having to do an extra g_slist_remove here why not just add
> the entry to the sdp_req_list only after bt_search_service has been
> successful?
>
> Also, how about renaming sdp_req_list to browse_req_list to match the
> name you've chosen for the context struct? Or maybe even simpler as
> "browse_reqs"

I agree.

>
>>  static uint8_t get_remote_services(void *buf, uint16_t len)
>>  {
>> -     return HAL_STATUS_UNSUPPORTED;
>> +     struct hal_cmd_get_remote_services *cmd = buf;
>> +     bool ret;
>> +     bdaddr_t addr;
>> +
>> +     android2bdaddr(&cmd->bdaddr, &addr);
>> +
>> +     ret = fetch_remote_uuids(&addr);
>> +
>> +     return ret ? HAL_STATUS_SUCCESS : HAL_STATUS_FAILED;
>>  }
>
> How about just making fetch_remote_uuids() return a proper HAL status
> directly instead of this mapping of bool to HAL status? You could also
> just consider merging the entire fetch_remote_uuids function (whose name
> I don't really like btw) into this get_remote_services function,
> especially if you don't foresee it having any other callers in the
> future.

I realized it in previous version, but fetch_remote_uuids will be called in
other places too (see next patches).

>
> Johan

BR
Marcin