[RFC BlueZ 0/3] Fix GATT server issues with BlueZ as LE peripheral

[RFC BlueZ 0/3] Fix GATT server issues with BlueZ as LE peripheral

[Anderson Lizardo]

Hi,

Currently, we do not officially support running BlueZ as LE peripheral. But
there is already enough support from the kernel to support this. For instance,
there is the "Set Advertising" mgmt command to keep LE advertising enabled
after each connection.

The current tracking of connections in BlueZ does not fully work for peripheral
role. These patches are a first attempt to improve the situation. Other fixes
are necessary to have full LE Peripheral support.

In particular, with these patches, notifications for PASP and ANP profiles
(implemented in profiles/alert/*) should work when BlueZ is on the peripheral
role. Other GATT server profiles will need fixing as well.

I'm sending as RFC because I only did minimal testing. I hope that people
having issues with GATT server profiles running on peripheral role can tests
these patches. Otherwise, I'll do more tests later today and send a final
version by tomorrow.

Notes: 
* The first patch is just cleanup, so can be applied as is.
* For testing, make sure to enable advertising on the machine running as
  peripheral (I use btmgmt tool for this).

Best Regards,
Anderson Lizardo

Anderson Lizardo (3):
  attrib-server: Remove unnecessary fields from struct gatt_channel
  core: Fix associating a GAttrib to a device on incoming connections
  alert: Only remove attio callback after ATT request was sent

 profiles/alert/server.c | 18 +++++++++++++++++-
 src/attrib-server.c     | 45 +++++++++++++++++++++++++++++----------------
 src/attrib-server.h     |  1 +
 src/device.c            |  5 +++++
 4 files changed, 52 insertions(+), 17 deletions(-)

-- 
1.8.3.2

[RFC BlueZ 1/3] attrib-server: Remove unnecessary fields from struct gatt_channel

[Anderson Lizardo]

These fields are only used internally by attrib_channel_attach(). The
"No GATT server found..." error message was also removed as it is also
displayed by find_gatt_server().
---
 src/attrib-server.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index 1c088df..13b86e1 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -71,8 +71,6 @@ struct gatt_server {
 };
 
 struct gatt_channel {
-	bdaddr_t src;
-	bdaddr_t dst;
 	GAttrib *attrib;
 	guint mtu;
 	gboolean le;
@@ -1107,6 +1105,7 @@ guint attrib_channel_attach(GAttrib *attrib)
 	struct gatt_server *server;
 	struct btd_device *device;
 	struct gatt_channel *channel;
+	bdaddr_t src, dst;
 	GIOChannel *io;
 	GError *gerr = NULL;
 	uint16_t cid;
@@ -1114,34 +1113,26 @@ guint attrib_channel_attach(GAttrib *attrib)
 
 	io = g_attrib_get_channel(attrib);
 
-	channel = g_new0(struct gatt_channel, 1);
-
 	bt_io_get(io, &gerr,
-			BT_IO_OPT_SOURCE_BDADDR, &channel->src,
-			BT_IO_OPT_DEST_BDADDR, &channel->dst,
+			BT_IO_OPT_SOURCE_BDADDR, &src,
+			BT_IO_OPT_DEST_BDADDR, &dst,
 			BT_IO_OPT_CID, &cid,
 			BT_IO_OPT_IMTU, &mtu,
 			BT_IO_OPT_INVALID);
 	if (gerr) {
 		error("bt_io_get: %s", gerr->message);
 		g_error_free(gerr);
-		g_free(channel);
 		return 0;
 	}
 
-	server = find_gatt_server(&channel->src);
-	if (server == NULL) {
-		char src[18];
-
-		ba2str(&channel->src, src);
-		error("No GATT server found in %s", src);
-		g_free(channel);
+	server = find_gatt_server(&src);
+	if (server == NULL)
 		return 0;
-	}
 
+	channel = g_new0(struct gatt_channel, 1);
 	channel->server = server;
 
-	device = btd_adapter_find_device(server->adapter, &channel->dst);
+	device = btd_adapter_find_device(server->adapter, &dst);
 	if (device == NULL) {
 		error("Device object not found for attrib server");
 		g_free(channel);
-- 
1.8.3.2

[RFC BlueZ 2/3] core: Fix associating a GAttrib to a device on incoming connections

[Anderson Lizardo]

If BlueZ is running as slave, incoming connections using ATT CID will
have a GAttrib created for them. But this GAttrib will not be assigned
to the underlying device (whose struct btd_device instance was created
in response to the Device Connected mgmt event).

This patch fixes this by assigning the GAttrib to the btd_device as soon
as a connection is first requested, which is done using attio callbacks.

Note that a new GAttrib reference is taken because attio callbacks "own"
device->attrib and thus need their own reference.
---
 src/attrib-server.c | 22 ++++++++++++++++++++++
 src/attrib-server.h |  1 +
 src/device.c        |  5 +++++
 3 files changed, 28 insertions(+)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index 13b86e1..7a50022 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -1100,6 +1100,28 @@ done:
 	g_attrib_send(channel->attrib, 0, opdu, length, NULL, NULL, NULL);
 }
 
+GAttrib *attrib_from_device(struct btd_device *device)
+{
+	struct btd_adapter *adapter = device_get_adapter(device);
+	struct gatt_server *server;
+	GSList *l;
+
+	l = g_slist_find_custom(servers, adapter, adapter_cmp);
+	if (!l)
+		return NULL;
+
+	server = l->data;
+
+	for (l = server->clients; l; l = l->next) {
+		struct gatt_channel *channel = l->data;
+
+		if (channel->device == device)
+			return g_attrib_ref(channel->attrib);
+	}
+
+	return NULL;
+}
+
 guint attrib_channel_attach(GAttrib *attrib)
 {
 	struct gatt_server *server;
diff --git a/src/attrib-server.h b/src/attrib-server.h
index 90ba17c..063cb66 100644
--- a/src/attrib-server.h
+++ b/src/attrib-server.h
@@ -37,5 +37,6 @@ int attrib_gap_set(struct btd_adapter *adapter, uint16_t uuid,
 uint32_t attrib_create_sdp(struct btd_adapter *adapter, uint16_t handle,
 							const char *name);
 void attrib_free_sdp(struct btd_adapter *adapter, uint32_t sdp_handle);
+GAttrib *attrib_from_device(struct btd_device *device);
 guint attrib_channel_attach(GAttrib *attrib);
 gboolean attrib_channel_detach(GAttrib *attrib, guint id);
diff --git a/src/device.c b/src/device.c
index 41a6fc8..f771b63 100644
--- a/src/device.c
+++ b/src/device.c
@@ -4470,6 +4470,11 @@ guint btd_device_add_attio_callback(struct btd_device *device,
 
 	device_set_auto_connect(device, TRUE);
 
+	/* Check if there is no GAttrib associated to the device created by a
+	 * incoming connection */
+	if (!device->attrib)
+		device->attrib = attrib_from_device(device);
+
 	if (device->attrib && cfunc) {
 		device->attios_offline = g_slist_append(device->attios_offline,
 									attio);
-- 
1.8.3.2

[RFC BlueZ 3/3] alert: Only remove attio callback after ATT request was sent

[Anderson Lizardo]

If there is a single registered attio callback, any pending ATT requests
will be dropped as soon as it is removed. This commit makes sure that
the request is sent before removing the callback.

Other places using attio callbacks may need fixing as well.
---
 profiles/alert/server.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/profiles/alert/server.c b/profiles/alert/server.c
index 94e9865..92d4fb6 100644
--- a/profiles/alert/server.c
+++ b/profiles/alert/server.c
@@ -363,6 +363,20 @@ end:
 	return result;
 }
 
+static void destroy_notify_callback(guint8 status, const guint8 *pdu, guint16 len,
+							gpointer user_data)
+{
+	struct notify_callback *cb = user_data;
+
+	DBG("status=%#x", status);
+
+	btd_device_remove_attio_callback(cb->device, cb->id);
+	btd_device_unref(cb->device);
+	g_free(cb->notify_data->value);
+	g_free(cb->notify_data);
+	g_free(cb);
+}
+
 static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
 {
 	struct notify_callback *cb = user_data;
@@ -398,7 +412,9 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
 					al_adapter->hnd_value[type],
 					al_adapter->hnd_ccc[type]);
 
-	g_attrib_send(attrib, 0, pdu, len, NULL, NULL, NULL);
+	g_attrib_send(attrib, 0, pdu, len, destroy_notify_callback, cb, NULL);
+
+	return;
 
 end:
 	btd_device_remove_attio_callback(cb->device, cb->id);
-- 
1.8.3.2

Re: [RFC BlueZ 0/3] Fix GATT server issues with BlueZ as LE peripheral

[Johan Hedberg]

Hi Lizardo,

On Tue, Jan 28, 2014, Anderson Lizardo wrote:
> Currently, we do not officially support running BlueZ as LE peripheral. But
> there is already enough support from the kernel to support this. For instance,
> there is the "Set Advertising" mgmt command to keep LE advertising enabled
> after each connection.
> 
> The current tracking of connections in BlueZ does not fully work for peripheral
> role. These patches are a first attempt to improve the situation. Other fixes
> are necessary to have full LE Peripheral support.
> 
> In particular, with these patches, notifications for PASP and ANP profiles
> (implemented in profiles/alert/*) should work when BlueZ is on the peripheral
> role. Other GATT server profiles will need fixing as well.
> 
> I'm sending as RFC because I only did minimal testing. I hope that people
> having issues with GATT server profiles running on peripheral role can tests
> these patches. Otherwise, I'll do more tests later today and send a final
> version by tomorrow.
> 
> Notes: 
> * The first patch is just cleanup, so can be applied as is.
> * For testing, make sure to enable advertising on the machine running as
>   peripheral (I use btmgmt tool for this).
> 
> Best Regards,
> Anderson Lizardo
> 
> Anderson Lizardo (3):
>   attrib-server: Remove unnecessary fields from struct gatt_channel
>   core: Fix associating a GAttrib to a device on incoming connections
>   alert: Only remove attio callback after ATT request was sent
> 
>  profiles/alert/server.c | 18 +++++++++++++++++-
>  src/attrib-server.c     | 45 +++++++++++++++++++++++++++++----------------
>  src/attrib-server.h     |  1 +
>  src/device.c            |  5 +++++
>  4 files changed, 52 insertions(+), 17 deletions(-)

These patches seem to have been forgotten about. They looked fairly good
to me so I rebased and applied them finally.

Johan

Re: [RFC BlueZ 0/3] Fix GATT server issues with BlueZ as LE peripheral

[Isa Ansharullah]

Hi Lizardo,

The patches fixed the issue.

Thank you very much.
Isa