[PATCH 1/2] android/gatt: Handle Register gatt client command

[PATCH 1/2] android/gatt: Handle Register gatt client command

[Grzegorz Kolodziejczyk]

This adds register gatt client app command handling.
---
 android/gatt.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 62 insertions(+), 1 deletion(-)

diff --git a/android/gatt.c b/android/gatt.c
index ce85af4..f8f7208 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -31,6 +31,7 @@
 #include <glib.h>
 
 #include "ipc.h"
+#include "ipc-common.h"
 #include "lib/bluetooth.h"
 #include "gatt.h"
 #include "src/log.h"
@@ -38,13 +39,73 @@
 
 static struct ipc *hal_ipc = NULL;
 static bdaddr_t adapter_addr;
+static int32_t client_if_counter;
+
+struct gatt_client {
+	int32_t client_if;
+	uint8_t uuid[16];
+};
+
+GList *gatt_client_list = NULL;
+
+static int find_client_uuid(gconstpointer data, gconstpointer user_data)
+{
+	const uint8_t *exp_uuid = user_data;
+	const uint8_t *cur_uuid = ((struct gatt_client *)data)->uuid;
+	struct gatt_client *client;
+
+	if (memcmp(exp_uuid, cur_uuid, sizeof(client->uuid)))
+		return 1;
+
+	return 0;
+}
+
 
 static void handle_client_register(const void *buf, uint16_t len)
 {
+	const struct hal_cmd_gatt_client_register *cmd = buf;
+	struct hal_ev_gatt_client_register_client ev;
+	struct gatt_client *client;
+	GList *found_client_uuid;
+
 	DBG("");
 
+	if (!cmd->uuid) {
+		error("no uuid received");
+		goto failed;
+	}
+
+	found_client_uuid = g_list_find_custom(gatt_client_list,
+					&cmd->uuid, find_client_uuid);
+
+	if (found_client_uuid) {
+		error("client uuid is already on list");
+		goto failed;
+	}
+
+	client = g_new0(struct gatt_client, 1);
+	g_memmove(client->uuid, cmd->uuid, sizeof(client->uuid));
+
+	client->client_if = ++client_if_counter;
+
+	gatt_client_list = g_list_append(gatt_client_list, client);
+
 	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT, HAL_OP_GATT_CLIENT_REGISTER,
-							HAL_STATUS_FAILED);
+							HAL_STATUS_SUCCESS);
+
+	ev.status = HAL_STATUS_SUCCESS;
+	ev.client_if = client->client_if;
+	g_memmove(ev.app_uuid, client->uuid, sizeof(client->uuid));
+
+	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
+			HAL_EV_GATT_CLIENT_REGISTER_CLIENT, sizeof(ev), &ev);
+
+	return;
+
+failed:
+	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT,
+				HAL_OP_GATT_CLIENT_REGISTER, HAL_STATUS_FAILED);
+
 }
 
 static void handle_client_unregister(const void *buf, uint16_t len)
-- 
1.8.5.2

[PATCH 2/2] android/gatt: Handle Unregister gatt client command

[Grzegorz Kolodziejczyk]

This adds unregister gatt client app command handling.
---
 android/gatt.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/android/gatt.c b/android/gatt.c
index f8f7208..1261c1f 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -60,6 +60,13 @@ static int find_client_uuid(gconstpointer data, gconstpointer user_data)
 	return 0;
 }
 
+static int find_client_if(gconstpointer data, gconstpointer user_data)
+{
+	const int32_t exp_cif = *((int32_t *)user_data);
+	const int32_t cur_cif = ((struct gatt_client *)data)->client_if;
+
+	return cur_cif != exp_cif;
+}
 
 static void handle_client_register(const void *buf, uint16_t len)
 {
@@ -110,10 +117,26 @@ failed:
 
 static void handle_client_unregister(const void *buf, uint16_t len)
 {
+	const struct hal_cmd_gatt_client_unregister *cmd = buf;
+	int32_t client_if = cmd->client_if;
+	GList *found_client_if;
+
 	DBG("");
 
-	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT,
+	found_client_if = g_list_find_custom(gatt_client_list, &client_if,
+								find_client_if);
+	if (!found_client_if) {
+		error("client_if: %d not found", client_if);
+		ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT,
 			HAL_OP_GATT_CLIENT_UNREGISTER, HAL_STATUS_FAILED);
+		return;
+	}
+
+	gatt_client_list = g_list_remove(gatt_client_list,
+							found_client_if->data);
+
+	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT,
+			HAL_OP_GATT_CLIENT_UNREGISTER, HAL_STATUS_SUCCESS);
 }
 
 static void handle_client_scan(const void *buf, uint16_t len)
-- 
1.8.5.2

Re: [PATCH 1/2] android/gatt: Handle Register gatt client command

[Szymon Janc]

Hi Grzegorz,

On Friday 07 of March 2014 14:04:58 Grzegorz Kolodziejczyk wrote:
> This adds register gatt client app command handling.
> ---
>  android/gatt.c | 63
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed,
> 62 insertions(+), 1 deletion(-)
> 
> diff --git a/android/gatt.c b/android/gatt.c
> index ce85af4..f8f7208 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -31,6 +31,7 @@
>  #include <glib.h>
> 
>  #include "ipc.h"
> +#include "ipc-common.h"
>  #include "lib/bluetooth.h"
>  #include "gatt.h"
>  #include "src/log.h"
> @@ -38,13 +39,73 @@
> 
>  static struct ipc *hal_ipc = NULL;
>  static bdaddr_t adapter_addr;
> +static int32_t client_if_counter;

Just use local static variable in function that use this counter.

> +
> +struct gatt_client {
> +	int32_t client_if;
> +	uint8_t uuid[16];
> +};
> +
> +GList *gatt_client_list = NULL;

This should be static. Also don't mix statics and type definitions.
And name it gatt_clients or just clients.

> +
> +static int find_client_uuid(gconstpointer data, gconstpointer user_data)
> +{
> +	const uint8_t *exp_uuid = user_data;
> +	const uint8_t *cur_uuid = ((struct gatt_client *)data)->uuid;
> +	struct gatt_client *client;
> +
> +	if (memcmp(exp_uuid, cur_uuid, sizeof(client->uuid)))
> +		return 1;
> +
> +	return 0;
> +}

Just return memcmp(); should be enough.

> +

This extra empty line is not needed.

> 
>  static void handle_client_register(const void *buf, uint16_t len)
>  {
> +	const struct hal_cmd_gatt_client_register *cmd = buf;
> +	struct hal_ev_gatt_client_register_client ev;
> +	struct gatt_client *client;
> +	GList *found_client_uuid;
> +
>  	DBG("");
> 
> +	if (!cmd->uuid) {
> +		error("no uuid received");

Lets prefix all info and error messages with "gatt: "

> +		goto failed;
> +	}
> +
> +	found_client_uuid = g_list_find_custom(gatt_client_list,
> +					&cmd->uuid, find_client_uuid);
> +
> +	if (found_client_uuid) {

if (g_list_find_custom(..)) {
}

> +		error("client uuid is already on list");
> +		goto failed;
> +	}
> +
> +	client = g_new0(struct gatt_client, 1);
> +	g_memmove(client->uuid, cmd->uuid, sizeof(client->uuid));

Why g_memmove? Just use memcpy() here.

> +
> +	client->client_if = ++client_if_counter;
> +
> +	gatt_client_list = g_list_append(gatt_client_list, client);
> +
>  	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT, HAL_OP_GATT_CLIENT_REGISTER,
> -							HAL_STATUS_FAILED);
> +							HAL_STATUS_SUCCESS);

I would add status variable and pass it to ipc_send_rsp() at the end of 
function.

> +
> +	ev.status = HAL_STATUS_SUCCESS;
> +	ev.client_if = client->client_if;
> +	g_memmove(ev.app_uuid, client->uuid, sizeof(client->uuid));

memcpy();

> +
> +	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
> +			HAL_EV_GATT_CLIENT_REGISTER_CLIENT, sizeof(ev), &ev);
> +
> +	return;
> +
> +failed:
> +	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT,
> +				HAL_OP_GATT_CLIENT_REGISTER, HAL_STATUS_FAILED);
> +
>  }
> 
>  static void handle_client_unregister(const void *buf, uint16_t len)

-- 
BR
Szymon Janc

Re: [PATCH 2/2] android/gatt: Handle Unregister gatt client command

[Szymon Janc]

Hi Grzegorz,

On Friday 07 of March 2014 14:04:59 Grzegorz Kolodziejczyk wrote:
> This adds unregister gatt client app command handling.
> ---
>  android/gatt.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/android/gatt.c b/android/gatt.c
> index f8f7208..1261c1f 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -60,6 +60,13 @@ static int find_client_uuid(gconstpointer data,
> gconstpointer user_data) return 0;
>  }
> 
> +static int find_client_if(gconstpointer data, gconstpointer user_data)
> +{
> +	const int32_t exp_cif = *((int32_t *)user_data);

You can use int to pointer macro to pass int here.

> +	const int32_t cur_cif = ((struct gatt_client *)data)->client_if;

const struct gatt_client *client = data;

> +
> +	return cur_cif != exp_cif;
> +}
> 
>  static void handle_client_register(const void *buf, uint16_t len)
>  {
> @@ -110,10 +117,26 @@ failed:
> 
>  static void handle_client_unregister(const void *buf, uint16_t len)
>  {
> +	const struct hal_cmd_gatt_client_unregister *cmd = buf;
> +	int32_t client_if = cmd->client_if;

Why is this copy needed?

> +	GList *found_client_if;
> +
>  	DBG("");
> 
> -	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT,
> +	found_client_if = g_list_find_custom(gatt_client_list, &client_if,
> +								find_client_if);
> +	if (!found_client_if) {
> +		error("client_if: %d not found", client_if);
> +		ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT,
>  			HAL_OP_GATT_CLIENT_UNREGISTER, HAL_STATUS_FAILED);

Lets follow this convention for error handling:

	status = HAL_STATUS_FAILED;
	goto failed;

> +		return;
> +	}
> +
> +	gatt_client_list = g_list_remove(gatt_client_list,
> +							found_client_if->data);
> +
> +	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT,
> +			HAL_OP_GATT_CLIENT_UNREGISTER, HAL_STATUS_SUCCESS);
>  }
> 
>  static void handle_client_scan(const void *buf, uint16_t len)

-- 
BR
Szymon Janc