[PATCH 0/6] android/gatt: GATT client improvements

[PATCH 0/6] 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

Lukasz Rymanowski (6):
  android/gatt: Add client listen support
  android/gatt: Refactor send_client_connect_notify
  android/gatt: Move functions up in the file
  android/gatt: Fix handling client unregister
  android/gatt: Move put_device_on_disc_list up in the file
  android/gatt: Move not used devices to disconnected dev queue

 android/gatt.c | 347 ++++++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 282 insertions(+), 65 deletions(-)

-- 
1.8.4

[PATCH 1/6] android/gatt: Add client listen support

[Lukasz Rymanowski]

This patch adds handling client listen support
---
 android/gatt.c | 148 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 146 insertions(+), 2 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index e10324e..df9c675 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -121,6 +121,12 @@ struct gatt_device {
 	guint watch_id;
 };
 
+struct gatt_listen {
+	struct queue *clients;
+
+	GIOChannel *att_io;
+};
+
 static struct ipc *hal_ipc = NULL;
 static bdaddr_t adapter_addr;
 static bool scanning = false;
@@ -131,6 +137,10 @@ static struct queue *conn_list	= NULL;		/* Connected devices */
 static struct queue *conn_wait_queue = NULL;	/* Devs waiting to connect */
 static struct queue *disc_dev_list = NULL;	/* Disconnected devices */
 
+static struct gatt_listen listen_clients;
+
+static uint32_t conn_id = 0;
+
 static void bt_le_discovery_stop_cb(void);
 
 static void android2uuid(const uint8_t *uuid, bt_uuid_t *dst)
@@ -815,7 +825,6 @@ static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
 	struct gatt_device *dev;
 	struct hal_ev_gatt_client_connect ev;
 	GAttrib *attrib;
-	static uint32_t conn_id = 0;
 	int32_t status;
 
 	/* Take device from conn waiting queue */
@@ -1200,12 +1209,147 @@ reply:
 	put_device_on_disc_list(dev);
 }
 
+static void copy_client_to_dev(void *data, void *user_data)
+{
+	int32_t *client_id = data;
+	struct gatt_device *dev = user_data;
+
+	if (!queue_push_tail(dev->clients, client_id))
+		error("gatt: Could not copy client to dev->clients");
+}
+
+static void connect_event(GIOChannel *io, GError *gerr, void *user_data)
+{
+	struct gatt_device *dev = NULL;
+	struct hal_ev_gatt_client_connect ev;
+	GAttrib *attrib;
+	uint8_t dst_type;
+	bdaddr_t src, dst;
+
+	DBG("");
+
+	if (gerr) {
+		error("gatt: %s", gerr->message);
+		g_error_free(gerr);
+		ev.status = GATT_FAILURE;
+		goto done;
+	}
+
+	bt_io_get(io, &gerr,
+			BT_IO_OPT_SOURCE_BDADDR, &src,
+			BT_IO_OPT_DEST_BDADDR, &dst,
+			BT_IO_OPT_DEST_TYPE, &dst_type,
+			BT_IO_OPT_INVALID);
+	if (gerr) {
+		error("gatt: bt_io_get: %s", gerr->message);
+		g_error_free(gerr);
+		ev.status = GATT_FAILURE;
+		goto done;
+	}
+
+	dev = create_device(&dst);
+	if (!dev) {
+		ev.status = GATT_FAILURE;
+		dev = NULL;
+		goto done;
+	}
+
+	dev->bdaddr_type = dst_type;
+
+	/* Set address and client id in the event */
+	bdaddr2android(&dev->bdaddr, &ev.bda);
+
+	attrib = g_attrib_new(io);
+	if (!attrib) {
+		error("gatt: unable to create new GAttrib instance");
+		ev.status = GATT_FAILURE;
+		destroy_device(dev);
+		dev = NULL;
+		goto done;
+	}
+
+	dev->attrib = attrib;
+	dev->watch_id = g_io_add_watch(io, G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+							disconnected_cb, dev);
+	dev->conn_id = ++conn_id;
+
+	ev.status = GATT_SUCCESS;
+
+done:
+	/* Only in success case we have valid conn_id */
+	ev.conn_id = ev.status == GATT_SUCCESS ? dev->conn_id : 0;
+
+	queue_foreach(listen_clients.clients, send_client_connect_notify, &ev);
+
+	if (ev.status != GATT_SUCCESS)
+		return;
+
+	/*copy list of clients to device and clear listen */
+	queue_foreach(listen_clients.clients, copy_client_to_dev, dev);
+
+	queue_remove_all(listen_clients.clients, NULL, NULL, NULL);
+	listen_clients.att_io = NULL;
+
+	/*TODO: Attach to attrib db */
+}
+
 static void handle_client_listen(const void *buf, uint16_t len)
 {
+	const struct hal_cmd_gatt_client_listen *cmd = buf;
+	struct hal_ev_gatt_client_listen ev;
+	uint8_t status;
+	GError *gerr;
+
 	DBG("");
 
+	if (!queue_push_tail(listen_clients.clients,
+						INT_TO_PTR(cmd->client_if))) {
+		error("gatt: Could not put client on the listen list");
+		status = HAL_STATUS_FAILED;
+		goto reply;
+	}
+
+	/* Do not be confused with server_if here. This is some typo in android
+	 * headers.
+	 */
+	ev.server_if = cmd->client_if;
+
+	if (listen_clients.att_io) {
+		/* There is already listening socket on. just return success */
+		status = HAL_STATUS_SUCCESS;
+		ev.status = GATT_SUCCESS;
+		goto reply;
+	}
+
+	status = HAL_STATUS_SUCCESS;
+
+	listen_clients.att_io = bt_io_listen(connect_event, NULL,
+					&listen_clients.att_io, NULL, &gerr,
+					BT_IO_OPT_SOURCE_BDADDR, adapter_addr,
+					BT_IO_OPT_SOURCE_TYPE, BDADDR_LE_PUBLIC,
+					BT_IO_OPT_CID, ATT_CID,
+					BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
+					BT_IO_OPT_INVALID);
+
+	if (listen_clients.att_io == NULL) {
+		error("%s", gerr->message);
+		g_error_free(gerr);
+		ev.status = GATT_FAILURE;
+	}
+
+	/*TODO: Start advertising */
+reply:
 	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT, HAL_OP_GATT_CLIENT_LISTEN,
-							HAL_STATUS_FAILED);
+								status);
+
+	/* If listen command succeed, send listen notification as well.
+	 * Android expects that.
+	*/
+	if (!status)
+		ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
+						HAL_EV_GATT_CLIENT_LISTEN,
+						sizeof(ev), &ev);
+
 }
 
 static void handle_client_refresh(const void *buf, uint16_t len)
-- 
1.8.4

[PATCH 2/6] 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 | 80 +++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 45 insertions(+), 35 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index df9c675..1bb797d 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -491,6 +491,25 @@ failed:
 					HAL_OP_GATT_CLIENT_REGISTER, status);
 }
 
+static void send_client_connect_notify(int32_t client_id,
+						struct gatt_device *dev,
+						int32_t status)
+{
+	struct hal_ev_gatt_client_connect ev;
+
+	ev.client_if = client_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;
@@ -808,24 +827,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)
 {
 	bdaddr_t *addr = user_data;
 	struct gatt_device *dev;
-	struct hal_ev_gatt_client_connect ev;
+	struct connect_data data;
 	GAttrib *attrib;
-	int32_t status;
 
 	/* Take device from conn waiting queue */
 	dev = queue_remove_if(conn_wait_queue, match_dev_by_bdaddr, addr);
@@ -838,19 +858,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;
 	}
 
@@ -863,21 +880,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 */
@@ -1221,7 +1235,7 @@ static void copy_client_to_dev(void *data, void *user_data)
 static void connect_event(GIOChannel *io, GError *gerr, void *user_data)
 {
 	struct gatt_device *dev = NULL;
-	struct hal_ev_gatt_client_connect ev;
+	struct connect_data data;
 	GAttrib *attrib;
 	uint8_t dst_type;
 	bdaddr_t src, dst;
@@ -1231,7 +1245,7 @@ static void connect_event(GIOChannel *io, GError *gerr, void *user_data)
 	if (gerr) {
 		error("gatt: %s", gerr->message);
 		g_error_free(gerr);
-		ev.status = GATT_FAILURE;
+		data.status = GATT_FAILURE;
 		goto done;
 	}
 
@@ -1243,26 +1257,23 @@ static void connect_event(GIOChannel *io, GError *gerr, void *user_data)
 	if (gerr) {
 		error("gatt: bt_io_get: %s", gerr->message);
 		g_error_free(gerr);
-		ev.status = GATT_FAILURE;
+		data.status = GATT_FAILURE;
 		goto done;
 	}
 
 	dev = create_device(&dst);
 	if (!dev) {
-		ev.status = GATT_FAILURE;
+		data.status = GATT_FAILURE;
 		dev = NULL;
 		goto done;
 	}
 
 	dev->bdaddr_type = dst_type;
 
-	/* Set address and client id in the event */
-	bdaddr2android(&dev->bdaddr, &ev.bda);
-
 	attrib = g_attrib_new(io);
 	if (!attrib) {
 		error("gatt: unable to create new GAttrib instance");
-		ev.status = GATT_FAILURE;
+		data.status = GATT_FAILURE;
 		destroy_device(dev);
 		dev = NULL;
 		goto done;
@@ -1273,15 +1284,15 @@ static void connect_event(GIOChannel *io, GError *gerr, void *user_data)
 							disconnected_cb, dev);
 	dev->conn_id = ++conn_id;
 
-	ev.status = GATT_SUCCESS;
+	data.status = GATT_SUCCESS;
 
 done:
-	/* Only in success case we have valid conn_id */
-	ev.conn_id = ev.status == GATT_SUCCESS ? dev->conn_id : 0;
+	data.dev = dev;
 
-	queue_foreach(listen_clients.clients, send_client_connect_notify, &ev);
+	queue_foreach(listen_clients.clients,
+				send_client_connect_notifications, &data);
 
-	if (ev.status != GATT_SUCCESS)
+	if (data.status != GATT_SUCCESS)
 		return;
 
 	/*copy list of clients to device and clear listen */
@@ -1349,7 +1360,6 @@ reply:
 		ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
 						HAL_EV_GATT_CLIENT_LISTEN,
 						sizeof(ev), &ev);
-
 }
 
 static void handle_client_refresh(const void *buf, uint16_t len)
-- 
1.8.4

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

[Lukasz Rymanowski]

Move send_client_disconnect_notify and connection_cleanup up in the file
---
 android/gatt.c | 70 +++++++++++++++++++++++++++++-----------------------------
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 1bb797d..82299a1 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -491,6 +491,41 @@ failed:
 					HAL_OP_GATT_CLIENT_REGISTER, 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,
+								uint8_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 client_id,
 						struct gatt_device *dev,
 						int32_t status)
@@ -669,41 +704,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,
-								uint8_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;
-- 
1.8.4

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

[Lukasz Rymanowski]

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

diff --git a/android/gatt.c b/android/gatt.c
index 82299a1..769d392 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -545,6 +545,42 @@ static void send_client_connect_notify(int32_t client_id,
 				HAL_EV_GATT_CLIENT_CONNECT, sizeof(ev), &ev);
 }
 
+static void remove_cl_from_connected_dev_and_notify(void *data, void *user_data)
+{
+	struct gatt_device *dev = data;
+	int32_t id = PTR_TO_INT(user_data);
+
+	if (queue_remove_if(dev->clients, match_by_value, INT_TO_PTR(id))) {
+		if (queue_isempty(dev->clients))
+			connection_cleanup(dev);
+
+		send_client_disconnect_notify(id, dev, GATT_SUCCESS);
+	}
+}
+
+static void remove_cl_from_connecting_dev_and_notify(void *data,
+							void *user_data)
+{
+	struct gatt_device *dev = data;
+	int32_t id = PTR_TO_INT(user_data);
+
+	if (queue_remove_if(dev->clients, match_by_value, INT_TO_PTR(id)))
+		send_client_connect_notify(id, dev, GATT_FAILURE);
+}
+
+static void remove_client_from_devices(int32_t client_id)
+{
+	queue_foreach(conn_list, remove_cl_from_connected_dev_and_notify,
+							INT_TO_PTR(client_id));
+
+	/*TODO: Check if there is any zombie device (connected no client)*/
+
+	queue_foreach(conn_wait_queue, remove_cl_from_connecting_dev_and_notify,
+							INT_TO_PTR(client_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;
@@ -561,6 +597,11 @@ static void handle_client_unregister(const void *buf, uint16_t len)
 		goto failed;
 	}
 
+	/* 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 5/6] android/gatt: Move put_device_on_disc_list up in the file

[Lukasz Rymanowski]

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

diff --git a/android/gatt.c b/android/gatt.c
index 769d392..d9c5773 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -568,6 +568,13 @@ static void remove_cl_from_connecting_dev_and_notify(void *data,
 		send_client_connect_notify(id, dev, GATT_FAILURE);
 }
 
+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 client_id)
 {
 	queue_foreach(conn_list, remove_cl_from_connected_dev_and_notify,
@@ -820,13 +827,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 6/6] android/gatt: Move not used devices to disconnected dev queue

[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.
---
 android/gatt.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index d9c5773..4d7a3e4 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -295,6 +295,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;
@@ -575,17 +582,32 @@ static void put_device_on_disc_list(struct gatt_device *dev)
 	queue_push_tail(disc_dev_list, dev);
 }
 
+static void park_not_used_devices(struct queue *q)
+{
+	struct gatt_device *dev;
+
+	/* Find device without client */
+	dev = queue_remove_if(q, match_dev_without_client, NULL);
+	while (dev) {
+		/* Park device on disconnected device queue */
+		put_device_on_disc_list(dev);
+		dev = queue_remove_if(q, match_dev_without_client, NULL);
+	};
+}
+
 static void remove_client_from_devices(int32_t client_id)
 {
 	queue_foreach(conn_list, remove_cl_from_connected_dev_and_notify,
 							INT_TO_PTR(client_id));
 
-	/*TODO: Check if there is any zombie device (connected no client)*/
+	/*If there is any device without client, move it to disc list*/
+	park_not_used_devices(conn_list);
 
 	queue_foreach(conn_wait_queue, remove_cl_from_connecting_dev_and_notify,
 							INT_TO_PTR(client_id));
 
-	/*TODO: Check if there is not zombie device plus stop scan */
+	/*If there is any device without client, move it to disc list*/
+	park_not_used_devices(conn_wait_queue);
 }
 
 static void handle_client_unregister(const void *buf, uint16_t len)
-- 
1.8.4

Re: [PATCH 1/6] android/gatt: Add client listen support

[Lukasz Rymanowski]

Hi,

On 10 April 2014 10:42, Lukasz Rymanowski <lukasz.rymanowski@tieto.com> wrote:
> This patch adds handling client listen support
> ---
>  android/gatt.c | 148 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 146 insertions(+), 2 deletions(-)
>
> diff --git a/android/gatt.c b/android/gatt.c
> index e10324e..df9c675 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -121,6 +121,12 @@ struct gatt_device {
>         guint watch_id;
>  };
>
> +struct gatt_listen {
> +       struct queue *clients;
> +
> +       GIOChannel *att_io;
> +};
> +
>  static struct ipc *hal_ipc = NULL;
>  static bdaddr_t adapter_addr;
>  static bool scanning = false;
> @@ -131,6 +137,10 @@ static struct queue *conn_list     = NULL;         /* Connected devices */
>  static struct queue *conn_wait_queue = NULL;   /* Devs waiting to connect */
>  static struct queue *disc_dev_list = NULL;     /* Disconnected devices */
>
> +static struct gatt_listen listen_clients;
> +
> +static uint32_t conn_id = 0;
> +
>  static void bt_le_discovery_stop_cb(void);
>
>  static void android2uuid(const uint8_t *uuid, bt_uuid_t *dst)
> @@ -815,7 +825,6 @@ static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
>         struct gatt_device *dev;
>         struct hal_ev_gatt_client_connect ev;
>         GAttrib *attrib;
> -       static uint32_t conn_id = 0;
>         int32_t status;
>
>         /* Take device from conn waiting queue */
> @@ -1200,12 +1209,147 @@ reply:
>         put_device_on_disc_list(dev);
>  }
>
> +static void copy_client_to_dev(void *data, void *user_data)
> +{
> +       int32_t *client_id = data;
> +       struct gatt_device *dev = user_data;
> +
> +       if (!queue_push_tail(dev->clients, client_id))
> +               error("gatt: Could not copy client to dev->clients");
> +}
> +
> +static void connect_event(GIOChannel *io, GError *gerr, void *user_data)
> +{
> +       struct gatt_device *dev = NULL;
> +       struct hal_ev_gatt_client_connect ev;
> +       GAttrib *attrib;
> +       uint8_t dst_type;
> +       bdaddr_t src, dst;
> +
> +       DBG("");
> +
> +       if (gerr) {
> +               error("gatt: %s", gerr->message);
> +               g_error_free(gerr);
> +               ev.status = GATT_FAILURE;
> +               goto done;
> +       }
> +
> +       bt_io_get(io, &gerr,
> +                       BT_IO_OPT_SOURCE_BDADDR, &src,
> +                       BT_IO_OPT_DEST_BDADDR, &dst,
> +                       BT_IO_OPT_DEST_TYPE, &dst_type,
> +                       BT_IO_OPT_INVALID);
> +       if (gerr) {
> +               error("gatt: bt_io_get: %s", gerr->message);
> +               g_error_free(gerr);
> +               ev.status = GATT_FAILURE;
> +               goto done;
> +       }
> +
> +       dev = create_device(&dst);
> +       if (!dev) {
> +               ev.status = GATT_FAILURE;
> +               dev = NULL;
this  is not needed of course :)
> +               goto done;
> +       }
> +
> +       dev->bdaddr_type = dst_type;
> +
> +       /* Set address and client id in the event */
> +       bdaddr2android(&dev->bdaddr, &ev.bda);
> +
> +       attrib = g_attrib_new(io);
> +       if (!attrib) {
> +               error("gatt: unable to create new GAttrib instance");
> +               ev.status = GATT_FAILURE;
> +               destroy_device(dev);
> +               dev = NULL;
> +               goto done;
> +       }
> +
> +       dev->attrib = attrib;
> +       dev->watch_id = g_io_add_watch(io, G_IO_HUP | G_IO_ERR | G_IO_NVAL,
> +                                                       disconnected_cb, dev);
> +       dev->conn_id = ++conn_id;
> +
> +       ev.status = GATT_SUCCESS;
> +
> +done:
> +       /* Only in success case we have valid conn_id */
> +       ev.conn_id = ev.status == GATT_SUCCESS ? dev->conn_id : 0;
> +
> +       queue_foreach(listen_clients.clients, send_client_connect_notify, &ev);
> +
> +       if (ev.status != GATT_SUCCESS)
> +               return;
> +
> +       /*copy list of clients to device and clear listen */
> +       queue_foreach(listen_clients.clients, copy_client_to_dev, dev);
> +
> +       queue_remove_all(listen_clients.clients, NULL, NULL, NULL);
> +       listen_clients.att_io = NULL;
> +
> +       /*TODO: Attach to attrib db */
> +}
> +
>  static void handle_client_listen(const void *buf, uint16_t len)
>  {
> +       const struct hal_cmd_gatt_client_listen *cmd = buf;
> +       struct hal_ev_gatt_client_listen ev;
> +       uint8_t status;
> +       GError *gerr;
> +
>         DBG("");
>
> +       if (!queue_push_tail(listen_clients.clients,
> +                                               INT_TO_PTR(cmd->client_if))) {
> +               error("gatt: Could not put client on the listen list");
> +               status = HAL_STATUS_FAILED;
> +               goto reply;
> +       }
> +
> +       /* Do not be confused with server_if here. This is some typo in android
> +        * headers.
> +        */
> +       ev.server_if = cmd->client_if;
> +
> +       if (listen_clients.att_io) {
> +               /* There is already listening socket on. just return success */
> +               status = HAL_STATUS_SUCCESS;
> +               ev.status = GATT_SUCCESS;
> +               goto reply;
> +       }
> +
> +       status = HAL_STATUS_SUCCESS;
> +
> +       listen_clients.att_io = bt_io_listen(connect_event, NULL,
> +                                       &listen_clients.att_io, NULL, &gerr,
> +                                       BT_IO_OPT_SOURCE_BDADDR, adapter_addr,
> +                                       BT_IO_OPT_SOURCE_TYPE, BDADDR_LE_PUBLIC,
> +                                       BT_IO_OPT_CID, ATT_CID,
> +                                       BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
> +                                       BT_IO_OPT_INVALID);
> +
> +       if (listen_clients.att_io == NULL) {
> +               error("%s", gerr->message);
> +               g_error_free(gerr);
> +               ev.status = GATT_FAILURE;
> +       }
> +
> +       /*TODO: Start advertising */
> +reply:
>         ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT, HAL_OP_GATT_CLIENT_LISTEN,
> -                                                       HAL_STATUS_FAILED);
> +                                                               status);
> +
> +       /* If listen command succeed, send listen notification as well.
> +        * Android expects that.
> +       */
> +       if (!status)
> +               ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
> +                                               HAL_EV_GATT_CLIENT_LISTEN,
> +                                               sizeof(ev), &ev);
> +
>  }
>
>  static void handle_client_refresh(const void *buf, uint16_t len)
> --
> 1.8.4
>

\Lukasz

Re: [PATCH 1/6] android/gatt: Add client listen support

[Szymon Janc]

Hi Łukasz,

On Thursday 10 of April 2014 10:42:56 Lukasz Rymanowski wrote:
> This patch adds handling client listen support
> ---
>  android/gatt.c | 148 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 146 insertions(+), 2 deletions(-)
> 
> diff --git a/android/gatt.c b/android/gatt.c
> index e10324e..df9c675 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -121,6 +121,12 @@ struct gatt_device {
>  	guint watch_id;
>  };
>  
> +struct gatt_listen {
> +	struct queue *clients;
> +
> +	GIOChannel *att_io;
> +};

I would not put this in dedicated structure, especially since att_io will be
shared with server (and so it should be created on init and not on demand).

> +
>  static struct ipc *hal_ipc = NULL;
>  static bdaddr_t adapter_addr;
>  static bool scanning = false;
> @@ -131,6 +137,10 @@ static struct queue *conn_list	= NULL;		/* Connected devices */
>  static struct queue *conn_wait_queue = NULL;	/* Devs waiting to connect */
>  static struct queue *disc_dev_list = NULL;	/* Disconnected devices */
>  
> +static struct gatt_listen listen_clients;
> +
> +static uint32_t conn_id = 0;
> +
>  static void bt_le_discovery_stop_cb(void);
>  
>  static void android2uuid(const uint8_t *uuid, bt_uuid_t *dst)
> @@ -815,7 +825,6 @@ static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
>  	struct gatt_device *dev;
>  	struct hal_ev_gatt_client_connect ev;
>  	GAttrib *attrib;
> -	static uint32_t conn_id = 0;
>  	int32_t status;
>  
>  	/* Take device from conn waiting queue */
> @@ -1200,12 +1209,147 @@ reply:
>  	put_device_on_disc_list(dev);
>  }
>  
> +static void copy_client_to_dev(void *data, void *user_data)
> +{

I would name it add_client_to_dev.

> +	int32_t *client_id = data;
> +	struct gatt_device *dev = user_data;
> +
> +	if (!queue_push_tail(dev->clients, client_id))
> +		error("gatt: Could not copy client to dev->clients");
> +}
> +
> +static void connect_event(GIOChannel *io, GError *gerr, void *user_data)
> +{
> +	struct gatt_device *dev = NULL;
> +	struct hal_ev_gatt_client_connect ev;
> +	GAttrib *attrib;
> +	uint8_t dst_type;
> +	bdaddr_t src, dst;
> +
> +	DBG("");
> +
> +	if (gerr) {
> +		error("gatt: %s", gerr->message);
> +		g_error_free(gerr);
> +		ev.status = GATT_FAILURE;
> +		goto done;
> +	}
> +
> +	bt_io_get(io, &gerr,
> +			BT_IO_OPT_SOURCE_BDADDR, &src,

This is not used, and we only have single adapter anyway.

> +			BT_IO_OPT_DEST_BDADDR, &dst,
> +			BT_IO_OPT_DEST_TYPE, &dst_type,
> +			BT_IO_OPT_INVALID);
> +	if (gerr) {
> +		error("gatt: bt_io_get: %s", gerr->message);
> +		g_error_free(gerr);
> +		ev.status = GATT_FAILURE;
> +		goto done;
> +	}
> +
> +	dev = create_device(&dst);
> +	if (!dev) {
> +		ev.status = GATT_FAILURE;
> +		dev = NULL;
> +		goto done;
> +	}
> +
> +	dev->bdaddr_type = dst_type;
> +
> +	/* Set address and client id in the event */
> +	bdaddr2android(&dev->bdaddr, &ev.bda);
> +
> +	attrib = g_attrib_new(io);
> +	if (!attrib) {
> +		error("gatt: unable to create new GAttrib instance");
> +		ev.status = GATT_FAILURE;
> +		destroy_device(dev);
> +		dev = NULL;
> +		goto done;
> +	}
> +
> +	dev->attrib = attrib;
> +	dev->watch_id = g_io_add_watch(io, G_IO_HUP | G_IO_ERR | G_IO_NVAL,
> +							disconnected_cb, dev);
> +	dev->conn_id = ++conn_id;
> +
> +	ev.status = GATT_SUCCESS;
> +
> +done:
> +	/* Only in success case we have valid conn_id */
> +	ev.conn_id = ev.status == GATT_SUCCESS ? dev->conn_id : 0;
> +
> +	queue_foreach(listen_clients.clients, send_client_connect_notify, &ev);
> +
> +	if (ev.status != GATT_SUCCESS)
> +		return;
> +
> +	/*copy list of clients to device and clear listen */
> +	queue_foreach(listen_clients.clients, copy_client_to_dev, dev);
> +
> +	queue_remove_all(listen_clients.clients, NULL, NULL, NULL);
> +	listen_clients.att_io = NULL;

This NULL assignment looks strange.

> +
> +	/*TODO: Attach to attrib db */
> +}
> +
>  static void handle_client_listen(const void *buf, uint16_t len)
>  {
> +	const struct hal_cmd_gatt_client_listen *cmd = buf;
> +	struct hal_ev_gatt_client_listen ev;
> +	uint8_t status;
> +	GError *gerr;
> +
>  	DBG("");
>  
> +	if (!queue_push_tail(listen_clients.clients,
> +						INT_TO_PTR(cmd->client_if))) {
> +		error("gatt: Could not put client on the listen list");
> +		status = HAL_STATUS_FAILED;
> +		goto reply;
> +	}
> +
> +	/* Do not be confused with server_if here. This is some typo in android
> +	 * headers.
> +	 */
> +	ev.server_if = cmd->client_if;
> +
> +	if (listen_clients.att_io) {
> +		/* There is already listening socket on. just return success */
> +		status = HAL_STATUS_SUCCESS;
> +		ev.status = GATT_SUCCESS;
> +		goto reply;
> +	}
> +
> +	status = HAL_STATUS_SUCCESS;
> +
> +	listen_clients.att_io = bt_io_listen(connect_event, NULL,
> +					&listen_clients.att_io, NULL, &gerr,
> +					BT_IO_OPT_SOURCE_BDADDR, adapter_addr,
> +					BT_IO_OPT_SOURCE_TYPE, BDADDR_LE_PUBLIC,
> +					BT_IO_OPT_CID, ATT_CID,
> +					BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
> +					BT_IO_OPT_INVALID);
> +
> +	if (listen_clients.att_io == NULL) {

if (!io) ..

> +		error("%s", gerr->message);
> +		g_error_free(gerr);
> +		ev.status = GATT_FAILURE;
> +	}
> +
> +	/*TODO: Start advertising */
> +reply:
>  	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT, HAL_OP_GATT_CLIENT_LISTEN,
> -							HAL_STATUS_FAILED);
> +								status);
> +
> +	/* If listen command succeed, send listen notification as well.
> +	 * Android expects that.
> +	*/
> +	if (!status)
> +		ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
> +						HAL_EV_GATT_CLIENT_LISTEN,
> +						sizeof(ev), &ev);
> +
>  }
>  
>  static void handle_client_refresh(const void *buf, uint16_t len)
> 

-- 
Best regards, 
Szymon Janc

Re: [PATCH 2/6] android/gatt: Refactor send_client_connect_notify

[Szymon Janc]

Hi Łukasz,

On Thursday 10 of April 2014 10:42:57 Lukasz Rymanowski wrote:
> Create helper function to send connect notification, similar to the
> send_client_disconnect_notify
> ---
>  android/gatt.c | 80 +++++++++++++++++++++++++++++++++-------------------------
>  1 file changed, 45 insertions(+), 35 deletions(-)
> 
> diff --git a/android/gatt.c b/android/gatt.c
> index df9c675..1bb797d 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -491,6 +491,25 @@ failed:
>  					HAL_OP_GATT_CLIENT_REGISTER, status);
>  }
>  
> +static void send_client_connect_notify(int32_t client_id,
> +						struct gatt_device *dev,
> +						int32_t status)
> +{
> +	struct hal_ev_gatt_client_connect ev;
> +
> +	ev.client_if = client_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;
> @@ -808,24 +827,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)
>  {
>  	bdaddr_t *addr = user_data;
>  	struct gatt_device *dev;
> -	struct hal_ev_gatt_client_connect ev;
> +	struct connect_data data;
>  	GAttrib *attrib;
> -	int32_t status;
>  
>  	/* Take device from conn waiting queue */
>  	dev = queue_remove_if(conn_wait_queue, match_dev_by_bdaddr, addr);
> @@ -838,19 +858,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;
>  	}
>  
> @@ -863,21 +880,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 */
> @@ -1221,7 +1235,7 @@ static void copy_client_to_dev(void *data, void *user_data)
>  static void connect_event(GIOChannel *io, GError *gerr, void *user_data)
>  {
>  	struct gatt_device *dev = NULL;
> -	struct hal_ev_gatt_client_connect ev;
> +	struct connect_data data;
>  	GAttrib *attrib;
>  	uint8_t dst_type;
>  	bdaddr_t src, dst;
> @@ -1231,7 +1245,7 @@ static void connect_event(GIOChannel *io, GError *gerr, void *user_data)
>  	if (gerr) {
>  		error("gatt: %s", gerr->message);
>  		g_error_free(gerr);
> -		ev.status = GATT_FAILURE;
> +		data.status = GATT_FAILURE;
>  		goto done;
>  	}
>  
> @@ -1243,26 +1257,23 @@ static void connect_event(GIOChannel *io, GError *gerr, void *user_data)
>  	if (gerr) {
>  		error("gatt: bt_io_get: %s", gerr->message);
>  		g_error_free(gerr);
> -		ev.status = GATT_FAILURE;
> +		data.status = GATT_FAILURE;
>  		goto done;
>  	}
>  
>  	dev = create_device(&dst);
>  	if (!dev) {
> -		ev.status = GATT_FAILURE;
> +		data.status = GATT_FAILURE;
>  		dev = NULL;
>  		goto done;
>  	}
>  
>  	dev->bdaddr_type = dst_type;
>  
> -	/* Set address and client id in the event */
> -	bdaddr2android(&dev->bdaddr, &ev.bda);
> -
>  	attrib = g_attrib_new(io);
>  	if (!attrib) {
>  		error("gatt: unable to create new GAttrib instance");
> -		ev.status = GATT_FAILURE;
> +		data.status = GATT_FAILURE;
>  		destroy_device(dev);
>  		dev = NULL;
>  		goto done;
> @@ -1273,15 +1284,15 @@ static void connect_event(GIOChannel *io, GError *gerr, void *user_data)
>  							disconnected_cb, dev);
>  	dev->conn_id = ++conn_id;
>  
> -	ev.status = GATT_SUCCESS;
> +	data.status = GATT_SUCCESS;
>  
>  done:
> -	/* Only in success case we have valid conn_id */
> -	ev.conn_id = ev.status == GATT_SUCCESS ? dev->conn_id : 0;
> +	data.dev = dev;
>  
> -	queue_foreach(listen_clients.clients, send_client_connect_notify, &ev);
> +	queue_foreach(listen_clients.clients,
> +				send_client_connect_notifications, &data);

dev can be NULL here so this might crash.

>  
> -	if (ev.status != GATT_SUCCESS)
> +	if (data.status != GATT_SUCCESS)
>  		return;
>  
>  	/*copy list of clients to device and clear listen */
> @@ -1349,7 +1360,6 @@ reply:
>  		ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
>  						HAL_EV_GATT_CLIENT_LISTEN,
>  						sizeof(ev), &ev);
> -
>  }

Unrelated (if this is due to previous patch, please fix it there).

>  
>  static void handle_client_refresh(const void *buf, uint16_t len)
> 

-- 
Best regards, 
Szymon Janc

Re: [PATCH 6/6] android/gatt: Move not used devices to disconnected dev queue

[Szymon Janc]

Hi Łukasz,

On Thursday 10 of April 2014 10:43:01 Lukasz Rymanowski wrote:
> 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.
> ---
>  android/gatt.c | 26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/android/gatt.c b/android/gatt.c
> index d9c5773..4d7a3e4 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -295,6 +295,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;
> @@ -575,17 +582,32 @@ static void put_device_on_disc_list(struct gatt_device *dev)
>  	queue_push_tail(disc_dev_list, dev);
>  }
>  
> +static void park_not_used_devices(struct queue *q)
> +{
> +	struct gatt_device *dev;
> +
> +	/* Find device without client */
> +	dev = queue_remove_if(q, match_dev_without_client, NULL);
> +	while (dev) {
> +		/* Park device on disconnected device queue */
> +		put_device_on_disc_list(dev);
> +		dev = queue_remove_if(q, match_dev_without_client, NULL);
> +	};
> +}
> +
>  static void remove_client_from_devices(int32_t client_id)
>  {
>  	queue_foreach(conn_list, remove_cl_from_connected_dev_and_notify,
>  							INT_TO_PTR(client_id));
>  
> -	/*TODO: Check if there is any zombie device (connected no client)*/
> +	/*If there is any device without client, move it to disc list*/
> +	park_not_used_devices(conn_list);

Shouldn't those devices be disconnected as well? Also 'park' term is a bit
unusual, maybe just call it cleanup?

>  
>  	queue_foreach(conn_wait_queue, remove_cl_from_connecting_dev_and_notify,
>  							INT_TO_PTR(client_id));
>  
> -	/*TODO: Check if there is not zombie device plus stop scan */
> +	/*If there is any device without client, move it to disc list*/
> +	park_not_used_devices(conn_wait_queue);
>  }
>  
>  static void handle_client_unregister(const void *buf, uint16_t len)
> 

-- 
Best regards, 
Szymon Janc

Re: [PATCH 1/6] android/gatt: Add client listen support

[Lukasz Rymanowski]

Hi Szymon,


On 10 April 2014 14:15, Szymon Janc <szymon.janc@tieto.com> wrote:
> Hi Łukasz,
>
> On Thursday 10 of April 2014 10:42:56 Lukasz Rymanowski wrote:
>> This patch adds handling client listen support
>> ---
>>  android/gatt.c | 148 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 146 insertions(+), 2 deletions(-)
>>
>> diff --git a/android/gatt.c b/android/gatt.c
>> index e10324e..df9c675 100644
>> --- a/android/gatt.c
>> +++ b/android/gatt.c
>> @@ -121,6 +121,12 @@ struct gatt_device {
>>       guint watch_id;
>>  };
>>
>> +struct gatt_listen {
>> +     struct queue *clients;
>> +
>> +     GIOChannel *att_io;
>> +};
>
> I would not put this in dedicated structure, especially since att_io will be
> shared with server (and so it should be created on init and not on demand).
>
>> +
>>  static struct ipc *hal_ipc = NULL;
>>  static bdaddr_t adapter_addr;
>>  static bool scanning = false;
>> @@ -131,6 +137,10 @@ static struct queue *conn_list   = NULL;         /* Connected devices */
>>  static struct queue *conn_wait_queue = NULL; /* Devs waiting to connect */
>>  static struct queue *disc_dev_list = NULL;   /* Disconnected devices */
>>
>> +static struct gatt_listen listen_clients;
>> +
>> +static uint32_t conn_id = 0;
>> +
>>  static void bt_le_discovery_stop_cb(void);
>>
>>  static void android2uuid(const uint8_t *uuid, bt_uuid_t *dst)
>> @@ -815,7 +825,6 @@ static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
>>       struct gatt_device *dev;
>>       struct hal_ev_gatt_client_connect ev;
>>       GAttrib *attrib;
>> -     static uint32_t conn_id = 0;
>>       int32_t status;
>>
>>       /* Take device from conn waiting queue */
>> @@ -1200,12 +1209,147 @@ reply:
>>       put_device_on_disc_list(dev);
>>  }
>>
>> +static void copy_client_to_dev(void *data, void *user_data)
>> +{
>
> I would name it add_client_to_dev.

Ok

>
>> +     int32_t *client_id = data;
>> +     struct gatt_device *dev = user_data;
>> +
>> +     if (!queue_push_tail(dev->clients, client_id))
>> +             error("gatt: Could not copy client to dev->clients");
>> +}
>> +
>> +static void connect_event(GIOChannel *io, GError *gerr, void *user_data)
>> +{
>> +     struct gatt_device *dev = NULL;
>> +     struct hal_ev_gatt_client_connect ev;
>> +     GAttrib *attrib;
>> +     uint8_t dst_type;
>> +     bdaddr_t src, dst;
>> +
>> +     DBG("");
>> +
>> +     if (gerr) {
>> +             error("gatt: %s", gerr->message);
>> +             g_error_free(gerr);
>> +             ev.status = GATT_FAILURE;
>> +             goto done;
>> +     }
>> +
>> +     bt_io_get(io, &gerr,
>> +                     BT_IO_OPT_SOURCE_BDADDR, &src,
>
> This is not used, and we only have single adapter anyway.
yup
>
>> +                     BT_IO_OPT_DEST_BDADDR, &dst,
>> +                     BT_IO_OPT_DEST_TYPE, &dst_type,
>> +                     BT_IO_OPT_INVALID);
>> +     if (gerr) {
>> +             error("gatt: bt_io_get: %s", gerr->message);
>> +             g_error_free(gerr);
>> +             ev.status = GATT_FAILURE;
>> +             goto done;
>> +     }
>> +
>> +     dev = create_device(&dst);
>> +     if (!dev) {
>> +             ev.status = GATT_FAILURE;
>> +             dev = NULL;
>> +             goto done;
>> +     }
>> +
>> +     dev->bdaddr_type = dst_type;
>> +
>> +     /* Set address and client id in the event */
>> +     bdaddr2android(&dev->bdaddr, &ev.bda);
>> +
>> +     attrib = g_attrib_new(io);
>> +     if (!attrib) {
>> +             error("gatt: unable to create new GAttrib instance");
>> +             ev.status = GATT_FAILURE;
>> +             destroy_device(dev);
>> +             dev = NULL;
>> +             goto done;
>> +     }
>> +
>> +     dev->attrib = attrib;
>> +     dev->watch_id = g_io_add_watch(io, G_IO_HUP | G_IO_ERR | G_IO_NVAL,
>> +                                                     disconnected_cb, dev);
>> +     dev->conn_id = ++conn_id;
>> +
>> +     ev.status = GATT_SUCCESS;
>> +
>> +done:
>> +     /* Only in success case we have valid conn_id */
>> +     ev.conn_id = ev.status == GATT_SUCCESS ? dev->conn_id : 0;
>> +
>> +     queue_foreach(listen_clients.clients, send_client_connect_notify, &ev);
>> +
>> +     if (ev.status != GATT_SUCCESS)
>> +             return;
>> +
>> +     /*copy list of clients to device and clear listen */
>> +     queue_foreach(listen_clients.clients, copy_client_to_dev, dev);
>> +
>> +     queue_remove_all(listen_clients.clients, NULL, NULL, NULL);
>> +     listen_clients.att_io = NULL;
>
> This NULL assignment looks strange.

Yes, this is stupid. .
>
>> +
>> +     /*TODO: Attach to attrib db */
>> +}
>> +
>>  static void handle_client_listen(const void *buf, uint16_t len)
>>  {
>> +     const struct hal_cmd_gatt_client_listen *cmd = buf;
>> +     struct hal_ev_gatt_client_listen ev;
>> +     uint8_t status;
>> +     GError *gerr;
>> +
>>       DBG("");
>>
>> +     if (!queue_push_tail(listen_clients.clients,
>> +                                             INT_TO_PTR(cmd->client_if))) {
>> +             error("gatt: Could not put client on the listen list");
>> +             status = HAL_STATUS_FAILED;
>> +             goto reply;
>> +     }
>> +
>> +     /* Do not be confused with server_if here. This is some typo in android
>> +      * headers.
>> +      */
>> +     ev.server_if = cmd->client_if;
>> +
>> +     if (listen_clients.att_io) {
>> +             /* There is already listening socket on. just return success */
>> +             status = HAL_STATUS_SUCCESS;
>> +             ev.status = GATT_SUCCESS;
>> +             goto reply;
>> +     }
>> +
>> +     status = HAL_STATUS_SUCCESS;
>> +
>> +     listen_clients.att_io = bt_io_listen(connect_event, NULL,
>> +                                     &listen_clients.att_io, NULL, &gerr,
>> +                                     BT_IO_OPT_SOURCE_BDADDR, adapter_addr,
>> +                                     BT_IO_OPT_SOURCE_TYPE, BDADDR_LE_PUBLIC,
>> +                                     BT_IO_OPT_CID, ATT_CID,
>> +                                     BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
>> +                                     BT_IO_OPT_INVALID);
>> +
>> +     if (listen_clients.att_io == NULL) {
>
> if (!io) ..
>
>> +             error("%s", gerr->message);
>> +             g_error_free(gerr);
>> +             ev.status = GATT_FAILURE;
>> +     }
>> +
>> +     /*TODO: Start advertising */
>> +reply:
>>       ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT, HAL_OP_GATT_CLIENT_LISTEN,
>> -                                                     HAL_STATUS_FAILED);
>> +                                                             status);
>> +
>> +     /* If listen command succeed, send listen notification as well.
>> +      * Android expects that.
>> +     */
>> +     if (!status)
>> +             ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
>> +                                             HAL_EV_GATT_CLIENT_LISTEN,
>> +                                             sizeof(ev), &ev);
>> +
>>  }
>>
>>  static void handle_client_refresh(const void *buf, uint16_t len)
>>
>
> --
> Best regards,
> Szymon Janc

Anyway, will move creating this socket to some better place as
discussed offline.

\Lukasz