[PATCH v4 0/4] android/gatt: GATT client improvements

[PATCH v4 0/4] android/gatt: GATT client improvements

[Lukasz Rymanowski]

This patch sets adds:
* preparation for client listen handling
* refactor send_client_connect_notify function
* fix scenario when application unregister client without proper cleaning

v2:
* removed listen from this patch set. Will send it later.
* cleaning of other patches

v3:
* added memset in patch 1/5

v4:
* rebase


Lukasz Rymanowski (4):
  android/gatt: Refactor send_client_connect_notify
  android/gatt: Fix handling client unregister
  android/gatt: Move functions up in the file
  android/gatt: Cleanup device lists

 android/gatt.c | 210 ++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 147 insertions(+), 63 deletions(-)

-- 
1.8.4

[PATCH v4 1/4] android/gatt: Refactor send_client_connect_notify

[Lukasz Rymanowski]

Create helper function to send connect notification, similar to the
send_client_disconnect_notify
---
 android/gatt.c | 57 ++++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 36 insertions(+), 21 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index e74a60c..0b1acf8 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -493,6 +493,26 @@ failed:
 									status);
 }
 
+static void send_client_connect_notify(int32_t id, struct gatt_device *dev,
+								int32_t status)
+{
+	struct hal_ev_gatt_client_connect ev;
+
+	memset(&ev, 0, sizeof(ev));
+
+	ev.client_if = id;
+	ev.status = status;
+
+	if (status == GATT_SUCCESS) {
+		/* Set address and client id in the event */
+		bdaddr2android(&dev->bdaddr, &ev.bda);
+		ev.conn_id = dev->conn_id;
+	}
+
+	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
+				HAL_EV_GATT_CLIENT_CONNECT, sizeof(ev), &ev);
+}
+
 static void handle_client_unregister(const void *buf, uint16_t len)
 {
 	const struct hal_cmd_gatt_client_unregister *cmd = buf;
@@ -801,24 +821,25 @@ done:
 	return FALSE;
 }
 
-static void send_client_connect_notify(void *data, void *user_data)
+struct connect_data {
+	struct gatt_device *dev;
+	int32_t status;
+};
+
+static void send_client_connect_notifications(void *data, void *user_data)
 {
-	struct hal_ev_gatt_client_connect *ev = user_data;
 	int32_t id = PTR_TO_INT(data);
+	struct connect_data *c = user_data;
 
-	ev->client_if = id;
-
-	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
-				HAL_EV_GATT_CLIENT_CONNECT, sizeof(*ev), ev);
+	send_client_connect_notify(id, c->dev, c->status);
 }
 
 static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
 {
 	struct gatt_device *dev = user_data;
-	struct hal_ev_gatt_client_connect ev;
+	struct connect_data data;
 	GAttrib *attrib;
 	static uint32_t conn_id = 0;
-	int32_t status;
 
 	/* Take device from conn waiting queue */
 	if (!queue_remove(conn_wait_queue, dev)) {
@@ -830,19 +851,16 @@ static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
 	g_io_channel_unref(dev->att_io);
 	dev->att_io = NULL;
 
-	/* Set address and client id in the event */
-	bdaddr2android(&dev->bdaddr, &ev.bda);
-
 	if (gerr) {
 		error("gatt: connection failed %s", gerr->message);
-		status = GATT_FAILURE;
+		data.status = GATT_FAILURE;
 		goto reply;
 	}
 
 	attrib = g_attrib_new(io);
 	if (!attrib) {
 		error("gatt: unable to create new GAttrib instance");
-		status = GATT_FAILURE;
+		data.status = GATT_FAILURE;
 		goto reply;
 	}
 
@@ -855,21 +873,18 @@ static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
 	if (!queue_push_tail(conn_list, dev)) {
 		error("gatt: Cannot push dev on conn_list");
 		connection_cleanup(dev);
-		status = GATT_FAILURE;
+		data.status = GATT_FAILURE;
 		goto reply;
 	}
 
-	status = GATT_SUCCESS;
-	goto reply;
+	data.status = GATT_SUCCESS;
 
 reply:
-	ev.conn_id = dev ? dev->conn_id : 0;
-	ev.status = status;
-
-	queue_foreach(dev->clients, send_client_connect_notify, &ev);
+	data.dev = dev;
+	queue_foreach(dev->clients, send_client_connect_notifications, &data);
 
 	/* If connection did not succeed, destroy device */
-	if (status)
+	if (data.status)
 		destroy_device(dev);
 
 	/* Check if we should restart scan */
-- 
1.8.4

[PATCH v4 2/4] android/gatt: Fix handling client unregister

[Lukasz Rymanowski]

When client do unregister we need to make sure that there is no
connected device or outstanding connection request for this client.
---
 android/gatt.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/android/gatt.c b/android/gatt.c
index 0b1acf8..2b1ed6f 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -513,6 +513,28 @@ static void send_client_connect_notify(int32_t id, struct gatt_device *dev,
 				HAL_EV_GATT_CLIENT_CONNECT, sizeof(ev), &ev);
 }
 
+static void remove_client_from_device_list(void *data, void *user_data)
+{
+	struct gatt_device *dev = data;
+
+	queue_remove_if(dev->clients, match_by_value, user_data);
+}
+
+static void remove_client_from_devices(int32_t id)
+{
+	DBG("");
+
+	queue_foreach(conn_list, remove_client_from_device_list,
+							INT_TO_PTR(id));
+
+	/*TODO: Check if there is any zombie device (connected no client)*/
+
+	queue_foreach(conn_wait_queue, remove_client_from_device_list,
+							INT_TO_PTR(id));
+
+	/*TODO: Check if there is not zombie device plus stop scan */
+}
+
 static void handle_client_unregister(const void *buf, uint16_t len)
 {
 	const struct hal_cmd_gatt_client_unregister *cmd = buf;
@@ -527,6 +549,10 @@ static void handle_client_unregister(const void *buf, uint16_t len)
 		error("gatt: client_if=%d not found", cmd->client_if);
 		status = HAL_STATUS_FAILED;
 	} else {
+		/* Check if there is any connect request or connected device for this
+		 * client. If so, remove this client from those lists.
+		 */
+		remove_client_from_devices(cl->id);
 		destroy_gatt_client(cl);
 		status = HAL_STATUS_SUCCESS;
 	}
-- 
1.8.4

[PATCH v4 3/4] android/gatt: Move functions up in the file

[Lukasz Rymanowski]

Move send_client_disconnect_notify connection_cleanup and
put_device_on_disc_list up in the file

This is needed by following patch
---
 android/gatt.c | 84 +++++++++++++++++++++++++++++-----------------------------
 1 file changed, 42 insertions(+), 42 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 2b1ed6f..83a02ec 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -493,6 +493,41 @@ failed:
 									status);
 }
 
+static void connection_cleanup(struct gatt_device *device)
+{
+	if (device->watch_id) {
+		g_source_remove(device->watch_id);
+		device->watch_id = 0;
+	}
+
+	if (device->att_io) {
+		g_io_channel_shutdown(device->att_io, FALSE, NULL);
+		g_io_channel_unref(device->att_io);
+		device->att_io = NULL;
+	}
+
+	if (device->attrib) {
+		GAttrib *attrib = device->attrib;
+		device->attrib = NULL;
+		g_attrib_cancel_all(attrib);
+		g_attrib_unref(attrib);
+	}
+}
+
+static void send_client_disconnect_notify(int32_t id, struct gatt_device *dev,
+								int32_t status)
+{
+	struct hal_ev_gatt_client_disconnect ev;
+
+	ev.client_if = id;
+	ev.conn_id = dev->conn_id;
+	ev.status = status;
+	bdaddr2android(&dev->bdaddr, &ev.bda);
+
+	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
+			HAL_EV_GATT_CLIENT_DISCONNECT, sizeof(ev), &ev);
+}
+
 static void send_client_connect_notify(int32_t id, struct gatt_device *dev,
 								int32_t status)
 {
@@ -520,6 +555,13 @@ static void remove_client_from_device_list(void *data, void *user_data)
 	queue_remove_if(dev->clients, match_by_value, user_data);
 }
 
+static void put_device_on_disc_list(struct gatt_device *dev)
+{
+	dev->conn_id = 0;
+	queue_remove_all(dev->clients, NULL, NULL, NULL);
+	queue_push_tail(disc_dev_list, dev);
+}
+
 static void remove_client_from_devices(int32_t id)
 {
 	DBG("");
@@ -701,41 +743,6 @@ done:
 	send_client_all_primary(gatt_status, dev->services, dev->conn_id);
 }
 
-static void connection_cleanup(struct gatt_device *device)
-{
-	if (device->watch_id) {
-		g_source_remove(device->watch_id);
-		device->watch_id = 0;
-	}
-
-	if (device->att_io) {
-		g_io_channel_shutdown(device->att_io, FALSE, NULL);
-		g_io_channel_unref(device->att_io);
-		device->att_io = NULL;
-	}
-
-	if (device->attrib) {
-		GAttrib *attrib = device->attrib;
-		device->attrib = NULL;
-		g_attrib_cancel_all(attrib);
-		g_attrib_unref(attrib);
-	}
-}
-
-static void send_client_disconnect_notify(int32_t id, struct gatt_device *dev,
-								int32_t status)
-{
-	struct hal_ev_gatt_client_disconnect ev;
-
-	ev.client_if = id;
-	ev.conn_id = dev->conn_id;
-	ev.status = status;
-	bdaddr2android(&dev->bdaddr, &ev.bda);
-
-	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
-			HAL_EV_GATT_CLIENT_DISCONNECT, sizeof(ev), &ev);
-}
-
 static void client_disconnect_notify(void *data, void *user_data)
 {
 	struct gatt_device *dev = user_data;
@@ -800,13 +807,6 @@ connect:
 	bt_le_discovery_stop(bt_le_discovery_stop_cb);
 }
 
-static void put_device_on_disc_list(struct gatt_device *dev)
-{
-	dev->conn_id = 0;
-	queue_remove_all(dev->clients, NULL, NULL, NULL);
-	queue_push_tail(disc_dev_list, dev);
-}
-
 static gboolean disconnected_cb(GIOChannel *io, GIOCondition cond,
 							gpointer user_data)
 {
-- 
1.8.4

[PATCH v4 4/4] android/gatt: Cleanup device lists

[Lukasz Rymanowski]

With this patch, devices from conn_list and conn_wait_queue which are
without a client e.g. because client has unregister without any
cleaning, are move to the disconnected device queue.

Also connected device without clients are disconnected.
And if there is no dev waiting for connect we do stop scan.
---
 android/gatt.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 47 insertions(+), 4 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 83a02ec..cb094db 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -284,6 +284,13 @@ static bool match_dev_by_conn_id(const void *data, const void *user_data)
 	return dev->conn_id == conn_id;
 }
 
+static bool match_dev_without_client(const void *data, const void *user_data)
+{
+	const struct gatt_device *dev = data;
+
+	return queue_isempty(dev->clients);
+}
+
 static bool match_srvc_by_element_id(const void *data, const void *user_data)
 {
 	const struct element_id *exp_id = user_data;
@@ -562,19 +569,55 @@ static void put_device_on_disc_list(struct gatt_device *dev)
 	queue_push_tail(disc_dev_list, dev);
 }
 
+static void cleanup_conn_list(int32_t id)
+{
+	struct gatt_device *dev;
+
+	/* Find device without client */
+	dev = queue_remove_if(conn_list, match_dev_without_client, NULL);
+	while (dev) {
+		/* Device is connected, lets disconnect and notify client */
+		connection_cleanup(dev);
+		send_client_disconnect_notify(id, dev, GATT_SUCCESS);
+
+		/* Put device on disconnected device list */
+		put_device_on_disc_list(dev);
+		dev = queue_remove_if(conn_list, match_dev_without_client,
+									NULL);
+	};
+}
+
+static void cleanup_conn_wait_queue(int32_t id)
+{
+	struct gatt_device *dev;
+
+	/* Find device without client */
+	dev = queue_remove_if(conn_wait_queue, match_dev_without_client, NULL);
+	while (dev) {
+		send_client_connect_notify(id, dev, GATT_FAILURE);
+
+		/* Put device on disconnected device list */
+		put_device_on_disc_list(dev);
+		dev = queue_remove_if(conn_wait_queue,
+						match_dev_without_client, NULL);
+	};
+
+	/* Stop scan we had for connecting devices */
+	if (queue_isempty(conn_wait_queue) && !scanning)
+		bt_le_discovery_stop(NULL);
+}
+
 static void remove_client_from_devices(int32_t id)
 {
 	DBG("");
 
 	queue_foreach(conn_list, remove_client_from_device_list,
 							INT_TO_PTR(id));
-
-	/*TODO: Check if there is any zombie device (connected no client)*/
+	cleanup_conn_list(id);
 
 	queue_foreach(conn_wait_queue, remove_client_from_device_list,
 							INT_TO_PTR(id));
-
-	/*TODO: Check if there is not zombie device plus stop scan */
+	cleanup_conn_wait_queue(id);
 }
 
 static void handle_client_unregister(const void *buf, uint16_t len)
-- 
1.8.4

Re: [PATCH v4 0/4] android/gatt: GATT client improvements

[Szymon Janc]

Hi Łukasz,

On Friday 11 of April 2014 16:37:05 Lukasz Rymanowski wrote:
> This patch sets adds:
> * preparation for client listen handling
> * refactor send_client_connect_notify function
> * fix scenario when application unregister client without proper cleaning
> 
> v2:
> * removed listen from this patch set. Will send it later.
> * cleaning of other patches
> 
> v3:
> * added memset in patch 1/5
> 
> v4:
> * rebase
> 
> 
> Lukasz Rymanowski (4):
>   android/gatt: Refactor send_client_connect_notify
>   android/gatt: Fix handling client unregister
>   android/gatt: Move functions up in the file
>   android/gatt: Cleanup device lists
> 
>  android/gatt.c | 210 ++++++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 147 insertions(+), 63 deletions(-)
> 

All patches applied (with change we discussed offline), thanks.

-- 
Best regards, 
Szymon Janc