[PATCH 1/9] android/health: Add error check when creating app

[PATCH 1/9] android/health: Add error check when creating app

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 android/health.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/android/health.c b/android/health.c
index 42c9a6e..de67475 100644
--- a/android/health.c
+++ b/android/health.c
@@ -820,6 +820,8 @@ static void bt_health_register_app(const void *buf, uint16_t len)
 
 	app = create_health_app(app_name, provider, srv_name, srv_descr,
 							cmd->num_of_mdep);
+	if (!app)
+		goto fail;
 
 	if (!queue_push_tail(apps, app))
 		goto fail;
@@ -830,7 +832,9 @@ static void bt_health_register_app(const void *buf, uint16_t len)
 	return;
 
 fail:
-	free_health_app(app);
+	if (app)
+		free_health_app(app);
+
 	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_HEALTH, HAL_OP_HEALTH_MDEP,
 							HAL_STATUS_FAILED);
 }
-- 
1.8.3.2

[PATCH 2/9] android/health: Add handling MDL connection request

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 android/health.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/android/health.c b/android/health.c
index de67475..dd2e5af 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1101,7 +1101,38 @@ static void mcap_mdl_aborted_cb(struct mcap_mdl *mdl, void *data)
 static void mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
 				uint16_t mdlid, uint8_t *conf, void *data)
 {
-	DBG("Not Implemeneted");
+	GError *gerr = NULL;
+
+	DBG("Data channel request: mdepid %u mdlid %u", mdepid, mdlid);
+
+	/* TODO: find / create device */
+
+	if (mdepid == HDP_MDEP_ECHO) {
+		switch (*conf) {
+		case HDP_NO_PREFERENCE_DC:
+			*conf = HDP_RELIABLE_DC;
+		case HDP_RELIABLE_DC:
+			break;
+		case HDP_STREAMING_DC:
+			return MCAP_CONFIGURATION_REJECTED;
+		default:
+			/* Special case defined in HDP spec 3.4. When an invalid
+			* configuration is received we shall close the MCL when
+			* we are still processing the callback. */
+			/* TODO close device */
+			return MCAP_CONFIGURATION_REJECTED; /* not processed */
+		}
+
+		if (!mcap_set_data_chan_mode(mcap, L2CAP_MODE_ERTM, &gerr)) {
+			error("Error: %s", gerr->message);
+			g_error_free(gerr);
+			return MCAP_MDL_BUSY;
+		}
+
+		/* TODO: Create channel */
+
+		return MCAP_SUCCESS;
+	}
 }
 
 static void mcap_mdl_reconn_req_cb(struct mcap_mdl *mdl, void *data)
-- 
1.8.3.2

[PATCH 3/9] android/health: Fix wrong callback return type

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 android/health.c | 8 ++++++--
 android/health.h | 8 ++++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/android/health.c b/android/health.c
index dd2e5af..3cb2016 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1098,7 +1098,7 @@ static void mcap_mdl_aborted_cb(struct mcap_mdl *mdl, void *data)
 	DBG("Not Implemeneted");
 }
 
-static void mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
+static uint8_t mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
 				uint16_t mdlid, uint8_t *conf, void *data)
 {
 	GError *gerr = NULL;
@@ -1133,11 +1133,15 @@ static void mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
 
 		return MCAP_SUCCESS;
 	}
+
+	return MCAP_SUCCESS;
 }
 
-static void mcap_mdl_reconn_req_cb(struct mcap_mdl *mdl, void *data)
+static uint8_t mcap_mdl_reconn_req_cb(struct mcap_mdl *mdl, void *data)
 {
 	DBG("Not Implemeneted");
+
+	return MCAP_SUCCESS;
 }
 
 static void connect_mdl_cb(struct mcap_mdl *mdl, GError *gerr, gpointer data)
diff --git a/android/health.h b/android/health.h
index 0b32fd3..ab7d478 100644
--- a/android/health.h
+++ b/android/health.h
@@ -21,5 +21,13 @@
  *
  */
 
+#define HDP_MDEP_ECHO		0x00
+#define HDP_MDEP_INITIAL	0x01
+#define HDP_MDEP_FINAL		0x7F
+
+#define HDP_NO_PREFERENCE_DC	0x00
+#define HDP_RELIABLE_DC		0x01
+#define HDP_STREAMING_DC	0x02
+
 bool bt_health_register(struct ipc *ipc, const bdaddr_t *addr, uint8_t mode);
 void bt_health_unregister(void);
-- 
1.8.3.2

[PATCH 4/9] android/health: Refactor channel creation

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Avoid using app_id when we shall use app structure itself. Otherwise we
end up with unnecessary searches for app and problems for incoming
connections.
---
 android/health.c | 43 ++++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/android/health.c b/android/health.c
index 3cb2016..8f91cf1 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1477,43 +1477,37 @@ static struct health_device *create_device(struct health_app *app,
 	return dev;
 }
 
-static struct health_device *get_device(uint16_t app_id, const uint8_t *addr)
+static struct health_app *get_app(uint16_t app_id)
+{
+	return queue_find(apps, match_app_by_id, INT_TO_PTR(app_id));
+}
+
+static struct health_device *get_device(struct health_app *app,
+							const uint8_t *addr)
 {
-	struct health_app *app;
 	struct health_device *dev;
 	bdaddr_t bdaddr;
 
-	app = queue_find(apps, match_app_by_id, INT_TO_PTR(app_id));
-	if (!app)
-		return NULL;
-
 	android2bdaddr(addr, &bdaddr);
 	dev = queue_find(app->devices, match_dev_by_addr, &bdaddr);
 	if (dev)
 		return dev;
 
-	dev = create_device(app, addr);
-	if (dev)
-		dev->app_id = app_id;
-
-	return dev;
+	return create_device(app, addr);
 }
 
-static struct health_channel *create_channel(uint16_t app_id,
+static struct health_channel *create_channel(struct health_app *app,
 						uint8_t mdep_index,
 						struct health_device *dev)
 {
-	struct health_app *app;
 	struct mdep_cfg *mdep;
 	struct health_channel *channel;
 	uint8_t index;
 	static unsigned int channel_id = 1;
 
-	if (!dev)
-		return NULL;
+	DBG("mdep %u", mdep_index);
 
-	app = queue_find(apps, match_app_by_id, INT_TO_PTR(app_id));
-	if (!app)
+	if (!dev || !app)
 		return NULL;
 
 	index = mdep_index + 1;
@@ -1539,7 +1533,7 @@ static struct health_channel *create_channel(uint16_t app_id,
 	return channel;
 }
 
-static struct health_channel *get_channel(uint16_t app_id,
+static struct health_channel *get_channel(struct health_app *app,
 						uint8_t mdep_index,
 						struct health_device *dev)
 {
@@ -1555,7 +1549,7 @@ static struct health_channel *get_channel(uint16_t app_id,
 	if (channel)
 		return channel;
 
-	return create_channel(app_id, mdep_index, dev);
+	return create_channel(app, mdep_index, dev);
 }
 
 static void bt_health_connect_channel(const void *buf, uint16_t len)
@@ -1564,14 +1558,21 @@ static void bt_health_connect_channel(const void *buf, uint16_t len)
 	struct hal_rsp_health_connect_channel rsp;
 	struct health_device *dev = NULL;
 	struct health_channel *channel = NULL;
+	struct health_app *app;
 
 	DBG("");
 
-	dev = get_device(cmd->app_id, cmd->bdaddr);
+	app = get_app(cmd->app_id);
+	if (!app)
+		goto fail;
+
+	dev = get_device(app, cmd->bdaddr);
 	if (!dev)
 		goto fail;
 
-	channel = get_channel(cmd->app_id, cmd->mdep_index, dev);
+	dev->app_id = cmd->app_id;
+
+	channel = get_channel(app, cmd->mdep_index, dev);
 	if (!channel)
 		goto fail;
 
-- 
1.8.3.2

[PATCH 5/9] android/health: Add channel connect

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Connect channel on incoming connection callback
---
 android/health.c | 190 ++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 111 insertions(+), 79 deletions(-)

diff --git a/android/health.c b/android/health.c
index 8f91cf1..7624790 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1098,14 +1098,125 @@ static void mcap_mdl_aborted_cb(struct mcap_mdl *mdl, void *data)
 	DBG("Not Implemeneted");
 }
 
+static struct health_device *create_device(struct health_app *app,
+							const uint8_t *addr)
+{
+	struct health_device *dev;
+
+	if (!app)
+		return NULL;
+
+	/* create device and push it to devices queue */
+	dev = new0(struct health_device, 1);
+	if (!dev)
+		return NULL;
+
+	android2bdaddr(addr, &dev->dst);
+	dev->channels = queue_new();
+	if (!dev->channels) {
+		free_health_device(dev);
+		return NULL;
+	}
+
+	if (!queue_push_tail(app->devices, dev)) {
+		free_health_device(dev);
+		return NULL;
+	}
+
+	return dev;
+}
+
+static struct health_device *get_device(struct health_app *app,
+							const uint8_t *addr)
+{
+	struct health_device *dev;
+	bdaddr_t bdaddr;
+
+	android2bdaddr(addr, &bdaddr);
+	dev = queue_find(app->devices, match_dev_by_addr, &bdaddr);
+	if (dev)
+		return dev;
+
+	return create_device(app, addr);
+}
+
+static struct health_channel *create_channel(struct health_app *app,
+						uint8_t mdep_index,
+						struct health_device *dev)
+{
+	struct mdep_cfg *mdep;
+	struct health_channel *channel;
+	uint8_t index;
+	static unsigned int channel_id = 1;
+
+	DBG("mdep %u", mdep_index);
+
+	if (!dev || !app)
+		return NULL;
+
+	index = mdep_index + 1;
+	mdep = queue_find(app->mdeps, match_mdep_by_id, INT_TO_PTR(index));
+	if (!mdep)
+		return NULL;
+
+	/* create channel and push it to device */
+	channel = new0(struct health_channel, 1);
+	if (!channel)
+		return NULL;
+
+	channel->mdep_id = mdep->id;
+	channel->type = mdep->channel_type;
+	channel->id = channel_id++;
+	channel->dev = dev;
+
+	if (!queue_push_tail(dev->channels, channel)) {
+		free_health_channel(channel);
+		return NULL;
+	}
+
+	return channel;
+}
+
+static struct health_channel *connect_channel(struct mcap_mcl *mcl,
+								uint8_t mdepid)
+{
+	struct health_app *app;
+	struct health_device *device;
+	struct health_channel *channel = NULL;
+	bdaddr_t addr;
+
+	mcap_mcl_get_addr(mcl, &addr);
+
+	/* TODO: Search app for mdepid */
+
+	if (mdepid == HDP_MDEP_ECHO) {
+		/* For echo service take last app */
+		app = queue_peek_tail(apps);
+		if (!app)
+			return NULL;
+
+		device = get_device(app, (uint8_t *) &addr);
+		if (!device)
+			return NULL;
+
+		channel = create_channel(app, mdepid, device);
+	}
+
+	return channel;
+}
+
 static uint8_t mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
 				uint16_t mdlid, uint8_t *conf, void *data)
 {
 	GError *gerr = NULL;
+	struct health_channel *channel;
 
 	DBG("Data channel request: mdepid %u mdlid %u", mdepid, mdlid);
 
 	/* TODO: find / create device */
+	channel = connect_channel(mcl, mdepid);
+	if (!channel)
+		return MCAP_MDL_BUSY;
 
 	if (mdepid == HDP_MDEP_ECHO) {
 		switch (*conf) {
@@ -1449,90 +1560,11 @@ static int connect_mcl(struct health_channel *channel)
 						search_cb, channel, NULL, 0);
 }
 
-static struct health_device *create_device(struct health_app *app,
-							const uint8_t *addr)
-{
-	struct health_device *dev;
-
-	if (!app)
-		return NULL;
-
-	/* create device and push it to devices queue */
-	dev = new0(struct health_device, 1);
-	if (!dev)
-		return NULL;
-
-	android2bdaddr(addr, &dev->dst);
-	dev->channels = queue_new();
-	if (!dev->channels) {
-		free_health_device(dev);
-		return NULL;
-	}
-
-	if (!queue_push_tail(app->devices, dev)) {
-		free_health_device(dev);
-		return NULL;
-	}
-
-	return dev;
-}
-
 static struct health_app *get_app(uint16_t app_id)
 {
 	return queue_find(apps, match_app_by_id, INT_TO_PTR(app_id));
 }
 
-static struct health_device *get_device(struct health_app *app,
-							const uint8_t *addr)
-{
-	struct health_device *dev;
-	bdaddr_t bdaddr;
-
-	android2bdaddr(addr, &bdaddr);
-	dev = queue_find(app->devices, match_dev_by_addr, &bdaddr);
-	if (dev)
-		return dev;
-
-	return create_device(app, addr);
-}
-
-static struct health_channel *create_channel(struct health_app *app,
-						uint8_t mdep_index,
-						struct health_device *dev)
-{
-	struct mdep_cfg *mdep;
-	struct health_channel *channel;
-	uint8_t index;
-	static unsigned int channel_id = 1;
-
-	DBG("mdep %u", mdep_index);
-
-	if (!dev || !app)
-		return NULL;
-
-	index = mdep_index + 1;
-	mdep = queue_find(app->mdeps, match_mdep_by_id, INT_TO_PTR(index));
-	if (!mdep)
-		return NULL;
-
-	/* create channel and push it to device */
-	channel = new0(struct health_channel, 1);
-	if (!channel)
-		return NULL;
-
-	channel->mdep_id = mdep->id;
-	channel->type = mdep->channel_type;
-	channel->id = channel_id++;
-	channel->dev = dev;
-
-	if (!queue_push_tail(dev->channels, channel)) {
-		free_health_channel(channel);
-		return NULL;
-	}
-
-	return channel;
-}
-
 static struct health_channel *get_channel(struct health_app *app,
 						uint8_t mdep_index,
 						struct health_device *dev)
-- 
1.8.3.2

[PATCH 6/9] android/health: Assign channel to user_data

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Assign channel for incoming connections when it is created.
---
 android/health.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/android/health.c b/android/health.c
index 7624790..6998913 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1202,6 +1202,9 @@ static struct health_channel *connect_channel(struct mcap_mcl *mcl,
 		channel = create_channel(app, mdepid, device);
 	}
 
+	/* Device is created here */
+	mcl->cb->user_data = channel;
+
 	return channel;
 }
 
-- 
1.8.3.2

[PATCH 7/9] android/health: Create mdep for ECHO channel

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 android/health.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/android/health.c b/android/health.c
index 6998913..a12933b 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1146,7 +1146,6 @@ static struct health_channel *create_channel(struct health_app *app,
 {
 	struct mdep_cfg *mdep;
 	struct health_channel *channel;
-	uint8_t index;
 	static unsigned int channel_id = 1;
 
 	DBG("mdep %u", mdep_index);
@@ -1154,10 +1153,23 @@ static struct health_channel *create_channel(struct health_app *app,
 	if (!dev || !app)
 		return NULL;
 
-	index = mdep_index + 1;
-	mdep = queue_find(app->mdeps, match_mdep_by_id, INT_TO_PTR(index));
-	if (!mdep)
-		return NULL;
+	mdep = queue_find(app->mdeps, match_mdep_by_id, INT_TO_PTR(mdep_index));
+	if (!mdep) {
+		if (mdep_index == HDP_MDEP_ECHO) {
+			mdep = new0(struct mdep_cfg, 1);
+			if (!mdep)
+				return NULL;
+
+			/* Leave other configuration zeroes */
+			mdep->id = HDP_MDEP_ECHO;
+
+			if (!queue_push_tail(app->mdeps, mdep)) {
+				free_mdep_cfg(mdep);
+				return NULL;
+			}
+		} else
+			return NULL;
+	}
 
 	/* create channel and push it to device */
 	channel = new0(struct health_channel, 1);
-- 
1.8.3.2

[PATCH 8/9] android/health: Add handling for ECHO service

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Reply received buffer back for ECHO service.
---
 android/health.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 62 insertions(+), 1 deletion(-)

diff --git a/android/health.c b/android/health.c
index a12933b..2d52197 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1054,9 +1054,70 @@ static int get_dcpsm(sdp_list_t *recs, uint16_t *dcpsm)
 	return -1;
 }
 
+static int send_echo_data(int sock, const void *buf, uint32_t size)
+{
+	const uint8_t *buf_b = buf;
+	uint32_t sent = 0;
+
+	while (sent < size) {
+		int n = write(sock, buf_b + sent, size - sent);
+		if (n < 0)
+			return -1;
+		sent += n;
+	}
+
+	return 0;
+}
+
+static gboolean serve_echo(GIOChannel *io, GIOCondition cond, gpointer data)
+{
+	struct health_channel *channel = data;
+	uint8_t buf[MCAP_DC_MTU];
+	int fd, len;
+
+	DBG("channel %p", channel);
+
+	if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL)) {
+		DBG("Error condition on channel");
+		return FALSE;
+	}
+
+	fd = g_io_channel_unix_get_fd(io);
+
+	len = read(fd, buf, sizeof(buf));
+	if (len < 0)
+		goto fail;
+
+	if (send_echo_data(fd, buf, len)  >= 0)
+		return TRUE;
+
+fail:
+	free_health_device(channel->dev);
+	return FALSE;
+}
+
 static void mcap_mdl_connected_cb(struct mcap_mdl *mdl, void *data)
 {
-	DBG("Not Implemeneted");
+	struct health_channel *channel = data;
+
+	if (!channel->mdl)
+		channel->mdl = mcap_mdl_ref(mdl);
+
+	DBG("Data channel connected: mdl %p channel %p", mdl, channel);
+
+	if (channel->mdep_id == HDP_MDEP_ECHO) {
+		GIOChannel *io;
+		int fd;
+
+		fd = mcap_mdl_get_fd(channel->mdl);
+		if (fd < 0)
+			return;
+
+		io = g_io_channel_unix_new(fd);
+		g_io_add_watch(io, G_IO_ERR | G_IO_HUP | G_IO_NVAL | G_IO_IN,
+							serve_echo, channel);
+		g_io_channel_unref(io);
+	}
 }
 
 static void mcap_mdl_closed_cb(struct mcap_mdl *mdl, void *data)
-- 
1.8.3.2

[PATCH 9/9] android/health: Improve debug

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 android/health.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/android/health.c b/android/health.c
index 2d52197..a12970d 100644
--- a/android/health.c
+++ b/android/health.c
@@ -160,6 +160,8 @@ static void free_health_channel(void *data)
 {
 	struct health_channel *channel = data;
 
+	DBG("channel %p", channel);
+
 	if (!channel)
 		return;
 
@@ -1137,12 +1139,13 @@ static void mcap_mdl_deleted_cb(struct mcap_mdl *mdl, void *data)
 	struct health_channel *channel = data;
 	struct health_device *dev;
 
-	DBG("");
-
 	if (!channel)
 		return;
 
 	dev = channel->dev;
+
+	DBG("device %p channel %p mdl %p", dev, channel, mdl);
+
 	/* mdl == NULL means, delete all mdls */
 	if (!mdl) {
 		queue_foreach(dev->channels, notify_channel, NULL);
@@ -1258,6 +1261,8 @@ static struct health_channel *connect_channel(struct mcap_mcl *mcl,
 	struct health_channel *channel = NULL;
 	bdaddr_t addr;
 
+	DBG("mcl %p mdepid %u", mcl, mdepid);
+
 	mcap_mcl_get_addr(mcl, &addr);
 
 	/* TODO: Search app for mdepid */
-- 
1.8.3.2

Re: [PATCH 2/9] android/health: Add handling MDL connection request

[Szymon Janc]

Hi Andrei,

On Thursday 26 of June 2014 15:04:18 Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> ---
>  android/health.c | 33 ++++++++++++++++++++++++++++++++-
>  1 file changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/android/health.c b/android/health.c
> index de67475..dd2e5af 100644
> --- a/android/health.c
> +++ b/android/health.c
> @@ -1101,7 +1101,38 @@ static void mcap_mdl_aborted_cb(struct mcap_mdl *mdl, void *data)
>  static void mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
>  				uint16_t mdlid, uint8_t *conf, void *data)
>  {
> -	DBG("Not Implemeneted");
> +	GError *gerr = NULL;
> +
> +	DBG("Data channel request: mdepid %u mdlid %u", mdepid, mdlid);
> +
> +	/* TODO: find / create device */
> +
> +	if (mdepid == HDP_MDEP_ECHO) {
> +		switch (*conf) {
> +		case HDP_NO_PREFERENCE_DC:
> +			*conf = HDP_RELIABLE_DC;
> +		case HDP_RELIABLE_DC:
> +			break;
> +		case HDP_STREAMING_DC:
> +			return MCAP_CONFIGURATION_REJECTED;

Does this compile? mcap_mdl_conn_req_cb() is returning void.

> +		default:
> +			/* Special case defined in HDP spec 3.4. When an invalid
> +			* configuration is received we shall close the MCL when
> +			* we are still processing the callback. */

Comment style is not correct.

> +			/* TODO close device */
> +			return MCAP_CONFIGURATION_REJECTED; /* not processed */
> +		}
> +
> +		if (!mcap_set_data_chan_mode(mcap, L2CAP_MODE_ERTM, &gerr)) {
> +			error("Error: %s", gerr->message);
> +			g_error_free(gerr);
> +			return MCAP_MDL_BUSY;
> +		}
> +
> +		/* TODO: Create channel */
> +
> +		return MCAP_SUCCESS;
> +	}
>  }
>  
>  static void mcap_mdl_reconn_req_cb(struct mcap_mdl *mdl, void *data)
> 

-- 
Best regards, 
Szymon Janc

Re: [PATCH 3/9] android/health: Fix wrong callback return type

[Szymon Janc]

Hi Andrei,

On Thursday 26 of June 2014 15:04:19 Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> ---
>  android/health.c | 8 ++++++--
>  android/health.h | 8 ++++++++
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/android/health.c b/android/health.c
> index dd2e5af..3cb2016 100644
> --- a/android/health.c
> +++ b/android/health.c
> @@ -1098,7 +1098,7 @@ static void mcap_mdl_aborted_cb(struct mcap_mdl *mdl, void *data)
>  	DBG("Not Implemeneted");
>  }
>  
> -static void mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
> +static uint8_t mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
>  				uint16_t mdlid, uint8_t *conf, void *data)
>  {
>  	GError *gerr = NULL;
> @@ -1133,11 +1133,15 @@ static void mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
>  
>  		return MCAP_SUCCESS;
>  	}
> +
> +	return MCAP_SUCCESS;
>  }
>  
> -static void mcap_mdl_reconn_req_cb(struct mcap_mdl *mdl, void *data)
> +static uint8_t mcap_mdl_reconn_req_cb(struct mcap_mdl *mdl, void *data)
>  {
>  	DBG("Not Implemeneted");
> +
> +	return MCAP_SUCCESS;
>  }
>  
>  static void connect_mdl_cb(struct mcap_mdl *mdl, GError *gerr, gpointer data)
> diff --git a/android/health.h b/android/health.h
> index 0b32fd3..ab7d478 100644
> --- a/android/health.h
> +++ b/android/health.h
> @@ -21,5 +21,13 @@
>   *
>   */
>  
> +#define HDP_MDEP_ECHO		0x00
> +#define HDP_MDEP_INITIAL	0x01
> +#define HDP_MDEP_FINAL		0x7F
> +
> +#define HDP_NO_PREFERENCE_DC	0x00
> +#define HDP_RELIABLE_DC		0x01
> +#define HDP_STREAMING_DC	0x02
> +
>  bool bt_health_register(struct ipc *ipc, const bdaddr_t *addr, uint8_t mode);
>  void bt_health_unregister(void);
> 

This should go before Patch 2, to avoid build errors.

-- 
Best regards, 
Szymon Janc

[PATCHv2 1/9] android/health: Add error check when creating app

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 android/health.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/android/health.c b/android/health.c
index 42c9a6e..de67475 100644
--- a/android/health.c
+++ b/android/health.c
@@ -820,6 +820,8 @@ static void bt_health_register_app(const void *buf, uint16_t len)
 
 	app = create_health_app(app_name, provider, srv_name, srv_descr,
 							cmd->num_of_mdep);
+	if (!app)
+		goto fail;
 
 	if (!queue_push_tail(apps, app))
 		goto fail;
@@ -830,7 +832,9 @@ static void bt_health_register_app(const void *buf, uint16_t len)
 	return;
 
 fail:
-	free_health_app(app);
+	if (app)
+		free_health_app(app);
+
 	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_HEALTH, HAL_OP_HEALTH_MDEP,
 							HAL_STATUS_FAILED);
 }
-- 
1.8.3.2

[PATCHv2 2/9] android/health: Fix wrong callback return type

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 android/health.c | 8 ++++++--
 android/health.h | 8 ++++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/android/health.c b/android/health.c
index de67475..140afee 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1098,15 +1098,19 @@ static void mcap_mdl_aborted_cb(struct mcap_mdl *mdl, void *data)
 	DBG("Not Implemeneted");
 }
 
-static void mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
+static uint8_t mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
 				uint16_t mdlid, uint8_t *conf, void *data)
 {
 	DBG("Not Implemeneted");
+
+	return MCAP_SUCCESS;
 }
 
-static void mcap_mdl_reconn_req_cb(struct mcap_mdl *mdl, void *data)
+static uint8_t mcap_mdl_reconn_req_cb(struct mcap_mdl *mdl, void *data)
 {
 	DBG("Not Implemeneted");
+
+	return MCAP_SUCCESS;
 }
 
 static void connect_mdl_cb(struct mcap_mdl *mdl, GError *gerr, gpointer data)
diff --git a/android/health.h b/android/health.h
index 0b32fd3..ab7d478 100644
--- a/android/health.h
+++ b/android/health.h
@@ -21,5 +21,13 @@
  *
  */
 
+#define HDP_MDEP_ECHO		0x00
+#define HDP_MDEP_INITIAL	0x01
+#define HDP_MDEP_FINAL		0x7F
+
+#define HDP_NO_PREFERENCE_DC	0x00
+#define HDP_RELIABLE_DC		0x01
+#define HDP_STREAMING_DC	0x02
+
 bool bt_health_register(struct ipc *ipc, const bdaddr_t *addr, uint8_t mode);
 void bt_health_unregister(void);
-- 
1.8.3.2

[PATCHv2 3/9] android/health: Add handling MDL connection request

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 android/health.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/android/health.c b/android/health.c
index 140afee..3cb2016 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1101,7 +1101,38 @@ static void mcap_mdl_aborted_cb(struct mcap_mdl *mdl, void *data)
 static uint8_t mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
 				uint16_t mdlid, uint8_t *conf, void *data)
 {
-	DBG("Not Implemeneted");
+	GError *gerr = NULL;
+
+	DBG("Data channel request: mdepid %u mdlid %u", mdepid, mdlid);
+
+	/* TODO: find / create device */
+
+	if (mdepid == HDP_MDEP_ECHO) {
+		switch (*conf) {
+		case HDP_NO_PREFERENCE_DC:
+			*conf = HDP_RELIABLE_DC;
+		case HDP_RELIABLE_DC:
+			break;
+		case HDP_STREAMING_DC:
+			return MCAP_CONFIGURATION_REJECTED;
+		default:
+			/* Special case defined in HDP spec 3.4. When an invalid
+			* configuration is received we shall close the MCL when
+			* we are still processing the callback. */
+			/* TODO close device */
+			return MCAP_CONFIGURATION_REJECTED; /* not processed */
+		}
+
+		if (!mcap_set_data_chan_mode(mcap, L2CAP_MODE_ERTM, &gerr)) {
+			error("Error: %s", gerr->message);
+			g_error_free(gerr);
+			return MCAP_MDL_BUSY;
+		}
+
+		/* TODO: Create channel */
+
+		return MCAP_SUCCESS;
+	}
 
 	return MCAP_SUCCESS;
 }
-- 
1.8.3.2

[PATCHv2 4/9] android/health: Refactor channel creation

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Avoid using app_id when we shall use app structure itself. Otherwise we
end up with unnecessary searches for app and problems for incoming
connections.
---
 android/health.c | 43 ++++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/android/health.c b/android/health.c
index 3cb2016..d664324 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1477,43 +1477,37 @@ static struct health_device *create_device(struct health_app *app,
 	return dev;
 }
 
-static struct health_device *get_device(uint16_t app_id, const uint8_t *addr)
+static struct health_app *get_app(uint16_t app_id)
+{
+	return queue_find(apps, match_app_by_id, INT_TO_PTR(app_id));
+}
+
+static struct health_device *get_device(struct health_app *app,
+							const uint8_t *addr)
 {
-	struct health_app *app;
 	struct health_device *dev;
 	bdaddr_t bdaddr;
 
-	app = queue_find(apps, match_app_by_id, INT_TO_PTR(app_id));
-	if (!app)
-		return NULL;
-
 	android2bdaddr(addr, &bdaddr);
 	dev = queue_find(app->devices, match_dev_by_addr, &bdaddr);
 	if (dev)
 		return dev;
 
-	dev = create_device(app, addr);
-	if (dev)
-		dev->app_id = app_id;
-
-	return dev;
+	return create_device(app, addr);
 }
 
-static struct health_channel *create_channel(uint16_t app_id,
+static struct health_channel *create_channel(struct health_app *app,
 						uint8_t mdep_index,
 						struct health_device *dev)
 {
-	struct health_app *app;
 	struct mdep_cfg *mdep;
 	struct health_channel *channel;
 	uint8_t index;
 	static unsigned int channel_id = 1;
 
-	if (!dev)
-		return NULL;
+	DBG("mdep %u", mdep_index);
 
-	app = queue_find(apps, match_app_by_id, INT_TO_PTR(app_id));
-	if (!app)
+	if (!dev || !app)
 		return NULL;
 
 	index = mdep_index + 1;
@@ -1539,7 +1533,7 @@ static struct health_channel *create_channel(uint16_t app_id,
 	return channel;
 }
 
-static struct health_channel *get_channel(uint16_t app_id,
+static struct health_channel *get_channel(struct health_app *app,
 						uint8_t mdep_index,
 						struct health_device *dev)
 {
@@ -1555,7 +1549,7 @@ static struct health_channel *get_channel(uint16_t app_id,
 	if (channel)
 		return channel;
 
-	return create_channel(app_id, mdep_index, dev);
+	return create_channel(app, index, dev);
 }
 
 static void bt_health_connect_channel(const void *buf, uint16_t len)
@@ -1564,14 +1558,21 @@ static void bt_health_connect_channel(const void *buf, uint16_t len)
 	struct hal_rsp_health_connect_channel rsp;
 	struct health_device *dev = NULL;
 	struct health_channel *channel = NULL;
+	struct health_app *app;
 
 	DBG("");
 
-	dev = get_device(cmd->app_id, cmd->bdaddr);
+	app = get_app(cmd->app_id);
+	if (!app)
+		goto fail;
+
+	dev = get_device(app, cmd->bdaddr);
 	if (!dev)
 		goto fail;
 
-	channel = get_channel(cmd->app_id, cmd->mdep_index, dev);
+	dev->app_id = cmd->app_id;
+
+	channel = get_channel(app, cmd->mdep_index, dev);
 	if (!channel)
 		goto fail;
 
-- 
1.8.3.2

[PATCHv2 5/9] android/health: Add channel connect

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Connect channel on incoming connection callback
---
 android/health.c | 190 ++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 111 insertions(+), 79 deletions(-)

diff --git a/android/health.c b/android/health.c
index d664324..ff2c663 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1098,14 +1098,125 @@ static void mcap_mdl_aborted_cb(struct mcap_mdl *mdl, void *data)
 	DBG("Not Implemeneted");
 }
 
+static struct health_device *create_device(struct health_app *app,
+							const uint8_t *addr)
+{
+	struct health_device *dev;
+
+	if (!app)
+		return NULL;
+
+	/* create device and push it to devices queue */
+	dev = new0(struct health_device, 1);
+	if (!dev)
+		return NULL;
+
+	android2bdaddr(addr, &dev->dst);
+	dev->channels = queue_new();
+	if (!dev->channels) {
+		free_health_device(dev);
+		return NULL;
+	}
+
+	if (!queue_push_tail(app->devices, dev)) {
+		free_health_device(dev);
+		return NULL;
+	}
+
+	return dev;
+}
+
+static struct health_device *get_device(struct health_app *app,
+							const uint8_t *addr)
+{
+	struct health_device *dev;
+	bdaddr_t bdaddr;
+
+	android2bdaddr(addr, &bdaddr);
+	dev = queue_find(app->devices, match_dev_by_addr, &bdaddr);
+	if (dev)
+		return dev;
+
+	return create_device(app, addr);
+}
+
+static struct health_channel *create_channel(struct health_app *app,
+						uint8_t mdep_index,
+						struct health_device *dev)
+{
+	struct mdep_cfg *mdep;
+	struct health_channel *channel;
+	uint8_t index;
+	static unsigned int channel_id = 1;
+
+	DBG("mdep %u", mdep_index);
+
+	if (!dev || !app)
+		return NULL;
+
+	index = mdep_index + 1;
+	mdep = queue_find(app->mdeps, match_mdep_by_id, INT_TO_PTR(index));
+	if (!mdep)
+		return NULL;
+
+	/* create channel and push it to device */
+	channel = new0(struct health_channel, 1);
+	if (!channel)
+		return NULL;
+
+	channel->mdep_id = mdep->id;
+	channel->type = mdep->channel_type;
+	channel->id = channel_id++;
+	channel->dev = dev;
+
+	if (!queue_push_tail(dev->channels, channel)) {
+		free_health_channel(channel);
+		return NULL;
+	}
+
+	return channel;
+}
+
+static struct health_channel *connect_channel(struct mcap_mcl *mcl,
+								uint8_t mdepid)
+{
+	struct health_app *app;
+	struct health_device *device;
+	struct health_channel *channel = NULL;
+	bdaddr_t addr;
+
+	mcap_mcl_get_addr(mcl, &addr);
+
+	/* TODO: Search app for mdepid */
+
+	if (mdepid == HDP_MDEP_ECHO) {
+		/* For echo service take last app */
+		app = queue_peek_tail(apps);
+		if (!app)
+			return NULL;
+
+		device = get_device(app, (uint8_t *) &addr);
+		if (!device)
+			return NULL;
+
+		channel = create_channel(app, mdepid, device);
+	}
+
+	return channel;
+}
+
 static uint8_t mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
 				uint16_t mdlid, uint8_t *conf, void *data)
 {
 	GError *gerr = NULL;
+	struct health_channel *channel;
 
 	DBG("Data channel request: mdepid %u mdlid %u", mdepid, mdlid);
 
 	/* TODO: find / create device */
+	channel = connect_channel(mcl, mdepid);
+	if (!channel)
+		return MCAP_MDL_BUSY;
 
 	if (mdepid == HDP_MDEP_ECHO) {
 		switch (*conf) {
@@ -1449,90 +1560,11 @@ static int connect_mcl(struct health_channel *channel)
 						search_cb, channel, NULL, 0);
 }
 
-static struct health_device *create_device(struct health_app *app,
-							const uint8_t *addr)
-{
-	struct health_device *dev;
-
-	if (!app)
-		return NULL;
-
-	/* create device and push it to devices queue */
-	dev = new0(struct health_device, 1);
-	if (!dev)
-		return NULL;
-
-	android2bdaddr(addr, &dev->dst);
-	dev->channels = queue_new();
-	if (!dev->channels) {
-		free_health_device(dev);
-		return NULL;
-	}
-
-	if (!queue_push_tail(app->devices, dev)) {
-		free_health_device(dev);
-		return NULL;
-	}
-
-	return dev;
-}
-
 static struct health_app *get_app(uint16_t app_id)
 {
 	return queue_find(apps, match_app_by_id, INT_TO_PTR(app_id));
 }
 
-static struct health_device *get_device(struct health_app *app,
-							const uint8_t *addr)
-{
-	struct health_device *dev;
-	bdaddr_t bdaddr;
-
-	android2bdaddr(addr, &bdaddr);
-	dev = queue_find(app->devices, match_dev_by_addr, &bdaddr);
-	if (dev)
-		return dev;
-
-	return create_device(app, addr);
-}
-
-static struct health_channel *create_channel(struct health_app *app,
-						uint8_t mdep_index,
-						struct health_device *dev)
-{
-	struct mdep_cfg *mdep;
-	struct health_channel *channel;
-	uint8_t index;
-	static unsigned int channel_id = 1;
-
-	DBG("mdep %u", mdep_index);
-
-	if (!dev || !app)
-		return NULL;
-
-	index = mdep_index + 1;
-	mdep = queue_find(app->mdeps, match_mdep_by_id, INT_TO_PTR(index));
-	if (!mdep)
-		return NULL;
-
-	/* create channel and push it to device */
-	channel = new0(struct health_channel, 1);
-	if (!channel)
-		return NULL;
-
-	channel->mdep_id = mdep->id;
-	channel->type = mdep->channel_type;
-	channel->id = channel_id++;
-	channel->dev = dev;
-
-	if (!queue_push_tail(dev->channels, channel)) {
-		free_health_channel(channel);
-		return NULL;
-	}
-
-	return channel;
-}
-
 static struct health_channel *get_channel(struct health_app *app,
 						uint8_t mdep_index,
 						struct health_device *dev)
-- 
1.8.3.2

[PATCHv2 6/9] android/health: Assign channel to user_data

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Assign channel for incoming connections when it is created.
---
 android/health.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/android/health.c b/android/health.c
index ff2c663..d9731fd 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1202,6 +1202,9 @@ static struct health_channel *connect_channel(struct mcap_mcl *mcl,
 		channel = create_channel(app, mdepid, device);
 	}
 
+	/* Device is created here */
+	mcl->cb->user_data = channel;
+
 	return channel;
 }
 
-- 
1.8.3.2

[PATCHv2 7/9] android/health: Create mdep for ECHO channel

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 android/health.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/android/health.c b/android/health.c
index d9731fd..4f15f93 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1146,7 +1146,6 @@ static struct health_channel *create_channel(struct health_app *app,
 {
 	struct mdep_cfg *mdep;
 	struct health_channel *channel;
-	uint8_t index;
 	static unsigned int channel_id = 1;
 
 	DBG("mdep %u", mdep_index);
@@ -1154,10 +1153,23 @@ static struct health_channel *create_channel(struct health_app *app,
 	if (!dev || !app)
 		return NULL;
 
-	index = mdep_index + 1;
-	mdep = queue_find(app->mdeps, match_mdep_by_id, INT_TO_PTR(index));
-	if (!mdep)
-		return NULL;
+	mdep = queue_find(app->mdeps, match_mdep_by_id, INT_TO_PTR(mdep_index));
+	if (!mdep) {
+		if (mdep_index == HDP_MDEP_ECHO) {
+			mdep = new0(struct mdep_cfg, 1);
+			if (!mdep)
+				return NULL;
+
+			/* Leave other configuration zeroes */
+			mdep->id = HDP_MDEP_ECHO;
+
+			if (!queue_push_tail(app->mdeps, mdep)) {
+				free_mdep_cfg(mdep);
+				return NULL;
+			}
+		} else
+			return NULL;
+	}
 
 	/* create channel and push it to device */
 	channel = new0(struct health_channel, 1);
-- 
1.8.3.2

[PATCHv2 8/9] android/health: Add handling for ECHO service

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Reply received buffer back for ECHO service.
---
 android/health.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 62 insertions(+), 1 deletion(-)

diff --git a/android/health.c b/android/health.c
index 4f15f93..9631d9e 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1054,9 +1054,70 @@ static int get_dcpsm(sdp_list_t *recs, uint16_t *dcpsm)
 	return -1;
 }
 
+static int send_echo_data(int sock, const void *buf, uint32_t size)
+{
+	const uint8_t *buf_b = buf;
+	uint32_t sent = 0;
+
+	while (sent < size) {
+		int n = write(sock, buf_b + sent, size - sent);
+		if (n < 0)
+			return -1;
+		sent += n;
+	}
+
+	return 0;
+}
+
+static gboolean serve_echo(GIOChannel *io, GIOCondition cond, gpointer data)
+{
+	struct health_channel *channel = data;
+	uint8_t buf[MCAP_DC_MTU];
+	int fd, len;
+
+	DBG("channel %p", channel);
+
+	if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL)) {
+		DBG("Error condition on channel");
+		return FALSE;
+	}
+
+	fd = g_io_channel_unix_get_fd(io);
+
+	len = read(fd, buf, sizeof(buf));
+	if (len < 0)
+		goto fail;
+
+	if (send_echo_data(fd, buf, len)  >= 0)
+		return TRUE;
+
+fail:
+	free_health_device(channel->dev);
+	return FALSE;
+}
+
 static void mcap_mdl_connected_cb(struct mcap_mdl *mdl, void *data)
 {
-	DBG("Not Implemeneted");
+	struct health_channel *channel = data;
+
+	if (!channel->mdl)
+		channel->mdl = mcap_mdl_ref(mdl);
+
+	DBG("Data channel connected: mdl %p channel %p", mdl, channel);
+
+	if (channel->mdep_id == HDP_MDEP_ECHO) {
+		GIOChannel *io;
+		int fd;
+
+		fd = mcap_mdl_get_fd(channel->mdl);
+		if (fd < 0)
+			return;
+
+		io = g_io_channel_unix_new(fd);
+		g_io_add_watch(io, G_IO_ERR | G_IO_HUP | G_IO_NVAL | G_IO_IN,
+							serve_echo, channel);
+		g_io_channel_unref(io);
+	}
 }
 
 static void mcap_mdl_closed_cb(struct mcap_mdl *mdl, void *data)
-- 
1.8.3.2

[PATCHv2 9/9] android/health: Improve debug

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 android/health.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/android/health.c b/android/health.c
index 9631d9e..093bdea 100644
--- a/android/health.c
+++ b/android/health.c
@@ -160,6 +160,8 @@ static void free_health_channel(void *data)
 {
 	struct health_channel *channel = data;
 
+	DBG("channel %p", channel);
+
 	if (!channel)
 		return;
 
@@ -1137,12 +1139,13 @@ static void mcap_mdl_deleted_cb(struct mcap_mdl *mdl, void *data)
 	struct health_channel *channel = data;
 	struct health_device *dev;
 
-	DBG("");
-
 	if (!channel)
 		return;
 
 	dev = channel->dev;
+
+	DBG("device %p channel %p mdl %p", dev, channel, mdl);
+
 	/* mdl == NULL means, delete all mdls */
 	if (!mdl) {
 		queue_foreach(dev->channels, notify_channel, NULL);
@@ -1258,6 +1261,8 @@ static struct health_channel *connect_channel(struct mcap_mcl *mcl,
 	struct health_channel *channel = NULL;
 	bdaddr_t addr;
 
+	DBG("mcl %p mdepid %u", mcl, mdepid);
+
 	mcap_mcl_get_addr(mcl, &addr);
 
 	/* TODO: Search app for mdepid */
@@ -1500,7 +1505,7 @@ static void get_mdep_cb(sdp_list_t *recs, int err, gpointer user_data)
 
 	if (!get_mdep_from_rec(recs->data, mdep->role, mdep->data_type,
 								&mdep_id)) {
-		error("health: no matching MDEP found");
+		error("health: no matching MDEP: %u", channel->mdep_id);
 		goto fail;
 	}
 
-- 
1.8.3.2

[PATCH] android/pts: Update HDP test results

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Mark ECHO server tests passed.
---
 android/pts-hdp.txt | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/android/pts-hdp.txt b/android/pts-hdp.txt
index 99aa2d6..459e2f1 100644
--- a/android/pts-hdp.txt
+++ b/android/pts-hdp.txt
@@ -105,7 +105,11 @@ TC_SRC_HCT_BV_05_C	N/A
 TC_SRC_HCT_BV_06_C	INC
 TC_SRC_HCT_BV_07_C	INC
 TC_SRC_DE_BV_01_I	INC
-TC_SRC_DE_BV_02_I	INC
+TC_SRC_DE_BV_02_I	PASS	haltest:
+				hl register_application bluez-android Bluez
+				bluez-hdp health-device-profile 1
+				BTHL_MDEP_ROLE_SOURCE 4100
+				BTHL_CHANNEL_TYPE_RELIABLE pulse-oximeter
 TC_SRC_DEP_BV_01_I	INC
 TC_SRC_DEP_BV_02_I	N/A
 TC_SNK_CON_BV_01_I	PASS	haltest:
@@ -195,7 +199,11 @@ TC_SNK_HCT_BV_05_C	N/A
 TC_SNK_HCT_BV_06_C	INC
 TC_SNK_HCT_BV_07_C	INC
 TC_SNK_DE_BV_01_I	N/A
-TC_SNK_DE_BV_02_I	INC
+TC_SNK_DE_BV_02_I	PASS	haltest:
+				hl register_application bluez-android Bluez
+				bluez-hdp health-device-profile 1
+				BTHL_MDEP_ROLE_SINK 4100
+				BTHL_CHANNEL_TYPE_RELIABLE pulse-oximeter
 TC_SNK_DEP_BV_03_I	INC
 TC_SNK_DEP_BV_04_I	INC
 -------------------------------------------------------------------------------
-- 
1.8.3.2

[PATCHv3 01/12] android/health: Add error check when creating app

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 android/health.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/android/health.c b/android/health.c
index 42c9a6e..de67475 100644
--- a/android/health.c
+++ b/android/health.c
@@ -820,6 +820,8 @@ static void bt_health_register_app(const void *buf, uint16_t len)
 
 	app = create_health_app(app_name, provider, srv_name, srv_descr,
 							cmd->num_of_mdep);
+	if (!app)
+		goto fail;
 
 	if (!queue_push_tail(apps, app))
 		goto fail;
@@ -830,7 +832,9 @@ static void bt_health_register_app(const void *buf, uint16_t len)
 	return;
 
 fail:
-	free_health_app(app);
+	if (app)
+		free_health_app(app);
+
 	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_HEALTH, HAL_OP_HEALTH_MDEP,
 							HAL_STATUS_FAILED);
 }
-- 
1.8.3.2

[PATCHv3 02/12] android/health: Fix wrong callback return type

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 android/health.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/android/health.c b/android/health.c
index de67475..140afee 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1098,15 +1098,19 @@ static void mcap_mdl_aborted_cb(struct mcap_mdl *mdl, void *data)
 	DBG("Not Implemeneted");
 }
 
-static void mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
+static uint8_t mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
 				uint16_t mdlid, uint8_t *conf, void *data)
 {
 	DBG("Not Implemeneted");
+
+	return MCAP_SUCCESS;
 }
 
-static void mcap_mdl_reconn_req_cb(struct mcap_mdl *mdl, void *data)
+static uint8_t mcap_mdl_reconn_req_cb(struct mcap_mdl *mdl, void *data)
 {
 	DBG("Not Implemeneted");
+
+	return MCAP_SUCCESS;
 }
 
 static void connect_mdl_cb(struct mcap_mdl *mdl, GError *gerr, gpointer data)
-- 
1.8.3.2

[PATCHv3 03/12] android/health: Add HDP definitions

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 android/health.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/android/health.h b/android/health.h
index 0b32fd3..ab7d478 100644
--- a/android/health.h
+++ b/android/health.h
@@ -21,5 +21,13 @@
  *
  */
 
+#define HDP_MDEP_ECHO		0x00
+#define HDP_MDEP_INITIAL	0x01
+#define HDP_MDEP_FINAL		0x7F
+
+#define HDP_NO_PREFERENCE_DC	0x00
+#define HDP_RELIABLE_DC		0x01
+#define HDP_STREAMING_DC	0x02
+
 bool bt_health_register(struct ipc *ipc, const bdaddr_t *addr, uint8_t mode);
 void bt_health_unregister(void);
-- 
1.8.3.2

[PATCHv3 04/12] android/health: Add handling MDL connection request

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 android/health.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/android/health.c b/android/health.c
index 140afee..3cb2016 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1101,7 +1101,38 @@ static void mcap_mdl_aborted_cb(struct mcap_mdl *mdl, void *data)
 static uint8_t mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
 				uint16_t mdlid, uint8_t *conf, void *data)
 {
-	DBG("Not Implemeneted");
+	GError *gerr = NULL;
+
+	DBG("Data channel request: mdepid %u mdlid %u", mdepid, mdlid);
+
+	/* TODO: find / create device */
+
+	if (mdepid == HDP_MDEP_ECHO) {
+		switch (*conf) {
+		case HDP_NO_PREFERENCE_DC:
+			*conf = HDP_RELIABLE_DC;
+		case HDP_RELIABLE_DC:
+			break;
+		case HDP_STREAMING_DC:
+			return MCAP_CONFIGURATION_REJECTED;
+		default:
+			/* Special case defined in HDP spec 3.4. When an invalid
+			* configuration is received we shall close the MCL when
+			* we are still processing the callback. */
+			/* TODO close device */
+			return MCAP_CONFIGURATION_REJECTED; /* not processed */
+		}
+
+		if (!mcap_set_data_chan_mode(mcap, L2CAP_MODE_ERTM, &gerr)) {
+			error("Error: %s", gerr->message);
+			g_error_free(gerr);
+			return MCAP_MDL_BUSY;
+		}
+
+		/* TODO: Create channel */
+
+		return MCAP_SUCCESS;
+	}
 
 	return MCAP_SUCCESS;
 }
-- 
1.8.3.2

[PATCHv3 05/12] android/health: Refactor channel creation

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Avoid using app_id when we shall use app structure itself. Otherwise we
end up with unnecessary searches for app and problems for incoming
connections.
---
 android/health.c | 43 ++++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/android/health.c b/android/health.c
index 3cb2016..d664324 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1477,43 +1477,37 @@ static struct health_device *create_device(struct health_app *app,
 	return dev;
 }
 
-static struct health_device *get_device(uint16_t app_id, const uint8_t *addr)
+static struct health_app *get_app(uint16_t app_id)
+{
+	return queue_find(apps, match_app_by_id, INT_TO_PTR(app_id));
+}
+
+static struct health_device *get_device(struct health_app *app,
+							const uint8_t *addr)
 {
-	struct health_app *app;
 	struct health_device *dev;
 	bdaddr_t bdaddr;
 
-	app = queue_find(apps, match_app_by_id, INT_TO_PTR(app_id));
-	if (!app)
-		return NULL;
-
 	android2bdaddr(addr, &bdaddr);
 	dev = queue_find(app->devices, match_dev_by_addr, &bdaddr);
 	if (dev)
 		return dev;
 
-	dev = create_device(app, addr);
-	if (dev)
-		dev->app_id = app_id;
-
-	return dev;
+	return create_device(app, addr);
 }
 
-static struct health_channel *create_channel(uint16_t app_id,
+static struct health_channel *create_channel(struct health_app *app,
 						uint8_t mdep_index,
 						struct health_device *dev)
 {
-	struct health_app *app;
 	struct mdep_cfg *mdep;
 	struct health_channel *channel;
 	uint8_t index;
 	static unsigned int channel_id = 1;
 
-	if (!dev)
-		return NULL;
+	DBG("mdep %u", mdep_index);
 
-	app = queue_find(apps, match_app_by_id, INT_TO_PTR(app_id));
-	if (!app)
+	if (!dev || !app)
 		return NULL;
 
 	index = mdep_index + 1;
@@ -1539,7 +1533,7 @@ static struct health_channel *create_channel(uint16_t app_id,
 	return channel;
 }
 
-static struct health_channel *get_channel(uint16_t app_id,
+static struct health_channel *get_channel(struct health_app *app,
 						uint8_t mdep_index,
 						struct health_device *dev)
 {
@@ -1555,7 +1549,7 @@ static struct health_channel *get_channel(uint16_t app_id,
 	if (channel)
 		return channel;
 
-	return create_channel(app_id, mdep_index, dev);
+	return create_channel(app, index, dev);
 }
 
 static void bt_health_connect_channel(const void *buf, uint16_t len)
@@ -1564,14 +1558,21 @@ static void bt_health_connect_channel(const void *buf, uint16_t len)
 	struct hal_rsp_health_connect_channel rsp;
 	struct health_device *dev = NULL;
 	struct health_channel *channel = NULL;
+	struct health_app *app;
 
 	DBG("");
 
-	dev = get_device(cmd->app_id, cmd->bdaddr);
+	app = get_app(cmd->app_id);
+	if (!app)
+		goto fail;
+
+	dev = get_device(app, cmd->bdaddr);
 	if (!dev)
 		goto fail;
 
-	channel = get_channel(cmd->app_id, cmd->mdep_index, dev);
+	dev->app_id = cmd->app_id;
+
+	channel = get_channel(app, cmd->mdep_index, dev);
 	if (!channel)
 		goto fail;
 
-- 
1.8.3.2

[PATCHv3 06/12] android/health: Add channel connect

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Connect channel on incoming connection callback
---
 android/health.c | 190 ++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 111 insertions(+), 79 deletions(-)

diff --git a/android/health.c b/android/health.c
index d664324..ff2c663 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1098,14 +1098,125 @@ static void mcap_mdl_aborted_cb(struct mcap_mdl *mdl, void *data)
 	DBG("Not Implemeneted");
 }
 
+static struct health_device *create_device(struct health_app *app,
+							const uint8_t *addr)
+{
+	struct health_device *dev;
+
+	if (!app)
+		return NULL;
+
+	/* create device and push it to devices queue */
+	dev = new0(struct health_device, 1);
+	if (!dev)
+		return NULL;
+
+	android2bdaddr(addr, &dev->dst);
+	dev->channels = queue_new();
+	if (!dev->channels) {
+		free_health_device(dev);
+		return NULL;
+	}
+
+	if (!queue_push_tail(app->devices, dev)) {
+		free_health_device(dev);
+		return NULL;
+	}
+
+	return dev;
+}
+
+static struct health_device *get_device(struct health_app *app,
+							const uint8_t *addr)
+{
+	struct health_device *dev;
+	bdaddr_t bdaddr;
+
+	android2bdaddr(addr, &bdaddr);
+	dev = queue_find(app->devices, match_dev_by_addr, &bdaddr);
+	if (dev)
+		return dev;
+
+	return create_device(app, addr);
+}
+
+static struct health_channel *create_channel(struct health_app *app,
+						uint8_t mdep_index,
+						struct health_device *dev)
+{
+	struct mdep_cfg *mdep;
+	struct health_channel *channel;
+	uint8_t index;
+	static unsigned int channel_id = 1;
+
+	DBG("mdep %u", mdep_index);
+
+	if (!dev || !app)
+		return NULL;
+
+	index = mdep_index + 1;
+	mdep = queue_find(app->mdeps, match_mdep_by_id, INT_TO_PTR(index));
+	if (!mdep)
+		return NULL;
+
+	/* create channel and push it to device */
+	channel = new0(struct health_channel, 1);
+	if (!channel)
+		return NULL;
+
+	channel->mdep_id = mdep->id;
+	channel->type = mdep->channel_type;
+	channel->id = channel_id++;
+	channel->dev = dev;
+
+	if (!queue_push_tail(dev->channels, channel)) {
+		free_health_channel(channel);
+		return NULL;
+	}
+
+	return channel;
+}
+
+static struct health_channel *connect_channel(struct mcap_mcl *mcl,
+								uint8_t mdepid)
+{
+	struct health_app *app;
+	struct health_device *device;
+	struct health_channel *channel = NULL;
+	bdaddr_t addr;
+
+	mcap_mcl_get_addr(mcl, &addr);
+
+	/* TODO: Search app for mdepid */
+
+	if (mdepid == HDP_MDEP_ECHO) {
+		/* For echo service take last app */
+		app = queue_peek_tail(apps);
+		if (!app)
+			return NULL;
+
+		device = get_device(app, (uint8_t *) &addr);
+		if (!device)
+			return NULL;
+
+		channel = create_channel(app, mdepid, device);
+	}
+
+	return channel;
+}
+
 static uint8_t mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
 				uint16_t mdlid, uint8_t *conf, void *data)
 {
 	GError *gerr = NULL;
+	struct health_channel *channel;
 
 	DBG("Data channel request: mdepid %u mdlid %u", mdepid, mdlid);
 
 	/* TODO: find / create device */
+	channel = connect_channel(mcl, mdepid);
+	if (!channel)
+		return MCAP_MDL_BUSY;
 
 	if (mdepid == HDP_MDEP_ECHO) {
 		switch (*conf) {
@@ -1449,90 +1560,11 @@ static int connect_mcl(struct health_channel *channel)
 						search_cb, channel, NULL, 0);
 }
 
-static struct health_device *create_device(struct health_app *app,
-							const uint8_t *addr)
-{
-	struct health_device *dev;
-
-	if (!app)
-		return NULL;
-
-	/* create device and push it to devices queue */
-	dev = new0(struct health_device, 1);
-	if (!dev)
-		return NULL;
-
-	android2bdaddr(addr, &dev->dst);
-	dev->channels = queue_new();
-	if (!dev->channels) {
-		free_health_device(dev);
-		return NULL;
-	}
-
-	if (!queue_push_tail(app->devices, dev)) {
-		free_health_device(dev);
-		return NULL;
-	}
-
-	return dev;
-}
-
 static struct health_app *get_app(uint16_t app_id)
 {
 	return queue_find(apps, match_app_by_id, INT_TO_PTR(app_id));
 }
 
-static struct health_device *get_device(struct health_app *app,
-							const uint8_t *addr)
-{
-	struct health_device *dev;
-	bdaddr_t bdaddr;
-
-	android2bdaddr(addr, &bdaddr);
-	dev = queue_find(app->devices, match_dev_by_addr, &bdaddr);
-	if (dev)
-		return dev;
-
-	return create_device(app, addr);
-}
-
-static struct health_channel *create_channel(struct health_app *app,
-						uint8_t mdep_index,
-						struct health_device *dev)
-{
-	struct mdep_cfg *mdep;
-	struct health_channel *channel;
-	uint8_t index;
-	static unsigned int channel_id = 1;
-
-	DBG("mdep %u", mdep_index);
-
-	if (!dev || !app)
-		return NULL;
-
-	index = mdep_index + 1;
-	mdep = queue_find(app->mdeps, match_mdep_by_id, INT_TO_PTR(index));
-	if (!mdep)
-		return NULL;
-
-	/* create channel and push it to device */
-	channel = new0(struct health_channel, 1);
-	if (!channel)
-		return NULL;
-
-	channel->mdep_id = mdep->id;
-	channel->type = mdep->channel_type;
-	channel->id = channel_id++;
-	channel->dev = dev;
-
-	if (!queue_push_tail(dev->channels, channel)) {
-		free_health_channel(channel);
-		return NULL;
-	}
-
-	return channel;
-}
-
 static struct health_channel *get_channel(struct health_app *app,
 						uint8_t mdep_index,
 						struct health_device *dev)
-- 
1.8.3.2

[PATCHv3 07/12] android/health: Assign channel to user_data

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Assign channel for incoming connections when it is created.
---
 android/health.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/android/health.c b/android/health.c
index ff2c663..d9731fd 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1202,6 +1202,9 @@ static struct health_channel *connect_channel(struct mcap_mcl *mcl,
 		channel = create_channel(app, mdepid, device);
 	}
 
+	/* Device is created here */
+	mcl->cb->user_data = channel;
+
 	return channel;
 }
 
-- 
1.8.3.2

[PATCHv3 08/12] android/health: Create mdep for ECHO channel

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 android/health.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/android/health.c b/android/health.c
index d9731fd..4f15f93 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1146,7 +1146,6 @@ static struct health_channel *create_channel(struct health_app *app,
 {
 	struct mdep_cfg *mdep;
 	struct health_channel *channel;
-	uint8_t index;
 	static unsigned int channel_id = 1;
 
 	DBG("mdep %u", mdep_index);
@@ -1154,10 +1153,23 @@ static struct health_channel *create_channel(struct health_app *app,
 	if (!dev || !app)
 		return NULL;
 
-	index = mdep_index + 1;
-	mdep = queue_find(app->mdeps, match_mdep_by_id, INT_TO_PTR(index));
-	if (!mdep)
-		return NULL;
+	mdep = queue_find(app->mdeps, match_mdep_by_id, INT_TO_PTR(mdep_index));
+	if (!mdep) {
+		if (mdep_index == HDP_MDEP_ECHO) {
+			mdep = new0(struct mdep_cfg, 1);
+			if (!mdep)
+				return NULL;
+
+			/* Leave other configuration zeroes */
+			mdep->id = HDP_MDEP_ECHO;
+
+			if (!queue_push_tail(app->mdeps, mdep)) {
+				free_mdep_cfg(mdep);
+				return NULL;
+			}
+		} else
+			return NULL;
+	}
 
 	/* create channel and push it to device */
 	channel = new0(struct health_channel, 1);
-- 
1.8.3.2

[PATCHv3 09/12] android/health: Add handling for ECHO service

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Reply received buffer back for ECHO service.
---
 android/health.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 62 insertions(+), 1 deletion(-)

diff --git a/android/health.c b/android/health.c
index 4f15f93..9631d9e 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1054,9 +1054,70 @@ static int get_dcpsm(sdp_list_t *recs, uint16_t *dcpsm)
 	return -1;
 }
 
+static int send_echo_data(int sock, const void *buf, uint32_t size)
+{
+	const uint8_t *buf_b = buf;
+	uint32_t sent = 0;
+
+	while (sent < size) {
+		int n = write(sock, buf_b + sent, size - sent);
+		if (n < 0)
+			return -1;
+		sent += n;
+	}
+
+	return 0;
+}
+
+static gboolean serve_echo(GIOChannel *io, GIOCondition cond, gpointer data)
+{
+	struct health_channel *channel = data;
+	uint8_t buf[MCAP_DC_MTU];
+	int fd, len;
+
+	DBG("channel %p", channel);
+
+	if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL)) {
+		DBG("Error condition on channel");
+		return FALSE;
+	}
+
+	fd = g_io_channel_unix_get_fd(io);
+
+	len = read(fd, buf, sizeof(buf));
+	if (len < 0)
+		goto fail;
+
+	if (send_echo_data(fd, buf, len)  >= 0)
+		return TRUE;
+
+fail:
+	free_health_device(channel->dev);
+	return FALSE;
+}
+
 static void mcap_mdl_connected_cb(struct mcap_mdl *mdl, void *data)
 {
-	DBG("Not Implemeneted");
+	struct health_channel *channel = data;
+
+	if (!channel->mdl)
+		channel->mdl = mcap_mdl_ref(mdl);
+
+	DBG("Data channel connected: mdl %p channel %p", mdl, channel);
+
+	if (channel->mdep_id == HDP_MDEP_ECHO) {
+		GIOChannel *io;
+		int fd;
+
+		fd = mcap_mdl_get_fd(channel->mdl);
+		if (fd < 0)
+			return;
+
+		io = g_io_channel_unix_new(fd);
+		g_io_add_watch(io, G_IO_ERR | G_IO_HUP | G_IO_NVAL | G_IO_IN,
+							serve_echo, channel);
+		g_io_channel_unref(io);
+	}
 }
 
 static void mcap_mdl_closed_cb(struct mcap_mdl *mdl, void *data)
-- 
1.8.3.2

[PATCHv3 10/12] android/health: Improve debug

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 android/health.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/android/health.c b/android/health.c
index 9631d9e..093bdea 100644
--- a/android/health.c
+++ b/android/health.c
@@ -160,6 +160,8 @@ static void free_health_channel(void *data)
 {
 	struct health_channel *channel = data;
 
+	DBG("channel %p", channel);
+
 	if (!channel)
 		return;
 
@@ -1137,12 +1139,13 @@ static void mcap_mdl_deleted_cb(struct mcap_mdl *mdl, void *data)
 	struct health_channel *channel = data;
 	struct health_device *dev;
 
-	DBG("");
-
 	if (!channel)
 		return;
 
 	dev = channel->dev;
+
+	DBG("device %p channel %p mdl %p", dev, channel, mdl);
+
 	/* mdl == NULL means, delete all mdls */
 	if (!mdl) {
 		queue_foreach(dev->channels, notify_channel, NULL);
@@ -1258,6 +1261,8 @@ static struct health_channel *connect_channel(struct mcap_mcl *mcl,
 	struct health_channel *channel = NULL;
 	bdaddr_t addr;
 
+	DBG("mcl %p mdepid %u", mcl, mdepid);
+
 	mcap_mcl_get_addr(mcl, &addr);
 
 	/* TODO: Search app for mdepid */
@@ -1500,7 +1505,7 @@ static void get_mdep_cb(sdp_list_t *recs, int err, gpointer user_data)
 
 	if (!get_mdep_from_rec(recs->data, mdep->role, mdep->data_type,
 								&mdep_id)) {
-		error("health: no matching MDEP found");
+		error("health: no matching MDEP: %u", channel->mdep_id);
 		goto fail;
 	}
 
-- 
1.8.3.2

[PATCHv3 11/12] android/mcap: Fix using uninitialised value

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Fixes clang warning:
...
android/mcap-lib.c:2366:20: warning: The left operand of '*' is a
garbage value
        return tv->tv_sec * 1000000ll + tv->tv_nsec / 1000ll;
               ~~~~~~~~~~ ^
1 warning generated.
...
---
 android/mcap-lib.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/android/mcap-lib.c b/android/mcap-lib.c
index 256abe1..79f6ce2 100644
--- a/android/mcap-lib.c
+++ b/android/mcap-lib.c
@@ -2454,7 +2454,8 @@ uint64_t mcap_get_timestamp(struct mcap_mcl *mcl,
 	if (given_time)
 		now = *given_time;
 	else
-		clock_gettime(CLK, &now);
+		if (clock_gettime(CLK, &now) < 0)
+			return MCAP_TMSTAMP_DONTSET;
 
 	tmstamp = time_us(&now) - time_us(&mcl->csp->base_time)
 		+ mcl->csp->base_tmstamp;
-- 
1.8.3.2

[PATCHv3 12/12] android/pts: Update HDP test results

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Mark ECHO server tests passed.
---
 android/pts-hdp.txt | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/android/pts-hdp.txt b/android/pts-hdp.txt
index 99aa2d6..f2b1528 100644
--- a/android/pts-hdp.txt
+++ b/android/pts-hdp.txt
@@ -1,7 +1,7 @@
 PTS test results for HDP
 
 PTS version: 5.1
-Tested: 17-June-2014
+Tested: 27-June-2014
 Android version: 4.4.2
 
 Results:
@@ -105,7 +105,11 @@ TC_SRC_HCT_BV_05_C	N/A
 TC_SRC_HCT_BV_06_C	INC
 TC_SRC_HCT_BV_07_C	INC
 TC_SRC_DE_BV_01_I	INC
-TC_SRC_DE_BV_02_I	INC
+TC_SRC_DE_BV_02_I	PASS	haltest:
+				hl register_application bluez-android Bluez
+				bluez-hdp health-device-profile 1
+				BTHL_MDEP_ROLE_SOURCE 4100
+				BTHL_CHANNEL_TYPE_RELIABLE pulse-oximeter
 TC_SRC_DEP_BV_01_I	INC
 TC_SRC_DEP_BV_02_I	N/A
 TC_SNK_CON_BV_01_I	PASS	haltest:
@@ -195,7 +199,11 @@ TC_SNK_HCT_BV_05_C	N/A
 TC_SNK_HCT_BV_06_C	INC
 TC_SNK_HCT_BV_07_C	INC
 TC_SNK_DE_BV_01_I	N/A
-TC_SNK_DE_BV_02_I	INC
+TC_SNK_DE_BV_02_I	PASS	haltest:
+				hl register_application bluez-android Bluez
+				bluez-hdp health-device-profile 1
+				BTHL_MDEP_ROLE_SINK 4100
+				BTHL_CHANNEL_TYPE_RELIABLE pulse-oximeter
 TC_SNK_DEP_BV_03_I	INC
 TC_SNK_DEP_BV_04_I	INC
 -------------------------------------------------------------------------------
-- 
1.8.3.2

Re: [PATCHv3 01/12] android/health: Add error check when creating app

[Szymon Janc]

Hi Andrei,

On Friday 27 of June 2014 14:24:52 Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> ---
>  android/health.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/android/health.c b/android/health.c
> index 42c9a6e..de67475 100644
> --- a/android/health.c
> +++ b/android/health.c
> @@ -820,6 +820,8 @@ static void bt_health_register_app(const void *buf, uint16_t len)
>  
>  	app = create_health_app(app_name, provider, srv_name, srv_descr,
>  							cmd->num_of_mdep);
> +	if (!app)
> +		goto fail;
>  
>  	if (!queue_push_tail(apps, app))
>  		goto fail;
> @@ -830,7 +832,9 @@ static void bt_health_register_app(const void *buf, uint16_t len)
>  	return;
>  
>  fail:
> -	free_health_app(app);
> +	if (app)
> +		free_health_app(app);
> +

This is not needed. free_health_app() already checks for NULL.

>  	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_HEALTH, HAL_OP_HEALTH_MDEP,
>  							HAL_STATUS_FAILED);
>  }
> 

-- 
Best regards, 
Szymon Janc

Re: [PATCHv3 03/12] android/health: Add HDP definitions

[Szymon Janc]

Hi Andrei,

On Friday 27 of June 2014 14:24:54 Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> ---
>  android/health.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/android/health.h b/android/health.h
> index 0b32fd3..ab7d478 100644
> --- a/android/health.h
> +++ b/android/health.h
> @@ -21,5 +21,13 @@
>   *
>   */
>  
> +#define HDP_MDEP_ECHO		0x00
> +#define HDP_MDEP_INITIAL	0x01
> +#define HDP_MDEP_FINAL		0x7F
> +
> +#define HDP_NO_PREFERENCE_DC	0x00
> +#define HDP_RELIABLE_DC		0x01
> +#define HDP_STREAMING_DC	0x02

Are those going to be used outside of health.c ?
Also last three are already defined in health.c.

> +
>  bool bt_health_register(struct ipc *ipc, const bdaddr_t *addr, uint8_t mode);
>  void bt_health_unregister(void);
> 

-- 
Best regards, 
Szymon Janc

Re: [PATCHv3 05/12] android/health: Refactor channel creation

[Szymon Janc]

Hi Andrei,

On Friday 27 of June 2014 14:24:56 Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> Avoid using app_id when we shall use app structure itself. Otherwise we
> end up with unnecessary searches for app and problems for incoming
> connections.
> ---
>  android/health.c | 43 ++++++++++++++++++++++---------------------
>  1 file changed, 22 insertions(+), 21 deletions(-)
> 
> diff --git a/android/health.c b/android/health.c
> index 3cb2016..d664324 100644
> --- a/android/health.c
> +++ b/android/health.c
> @@ -1477,43 +1477,37 @@ static struct health_device *create_device(struct health_app *app,
>  	return dev;
>  }
>  
> -static struct health_device *get_device(uint16_t app_id, const uint8_t *addr)
> +static struct health_app *get_app(uint16_t app_id)
> +{
> +	return queue_find(apps, match_app_by_id, INT_TO_PTR(app_id));
> +}
> +
> +static struct health_device *get_device(struct health_app *app,
> +							const uint8_t *addr)
>  {
> -	struct health_app *app;
>  	struct health_device *dev;
>  	bdaddr_t bdaddr;
>  
> -	app = queue_find(apps, match_app_by_id, INT_TO_PTR(app_id));
> -	if (!app)
> -		return NULL;
> -
>  	android2bdaddr(addr, &bdaddr);
>  	dev = queue_find(app->devices, match_dev_by_addr, &bdaddr);
>  	if (dev)
>  		return dev;
>  
> -	dev = create_device(app, addr);
> -	if (dev)
> -		dev->app_id = app_id;
> -
> -	return dev;
> +	return create_device(app, addr);
>  }
>  
> -static struct health_channel *create_channel(uint16_t app_id,
> +static struct health_channel *create_channel(struct health_app *app,
>  						uint8_t mdep_index,
>  						struct health_device *dev)
>  {
> -	struct health_app *app;
>  	struct mdep_cfg *mdep;
>  	struct health_channel *channel;
>  	uint8_t index;
>  	static unsigned int channel_id = 1;
>  
> -	if (!dev)
> -		return NULL;
> +	DBG("mdep %u", mdep_index);
>  
> -	app = queue_find(apps, match_app_by_id, INT_TO_PTR(app_id));
> -	if (!app)
> +	if (!dev || !app)
>  		return NULL;
>  
>  	index = mdep_index + 1;
> @@ -1539,7 +1533,7 @@ static struct health_channel *create_channel(uint16_t app_id,
>  	return channel;
>  }
>  
> -static struct health_channel *get_channel(uint16_t app_id,
> +static struct health_channel *get_channel(struct health_app *app,
>  						uint8_t mdep_index,
>  						struct health_device *dev)
>  {
> @@ -1555,7 +1549,7 @@ static struct health_channel *get_channel(uint16_t app_id,
>  	if (channel)
>  		return channel;
>  
> -	return create_channel(app_id, mdep_index, dev);
> +	return create_channel(app, index, dev);
>  }
>  
>  static void bt_health_connect_channel(const void *buf, uint16_t len)
> @@ -1564,14 +1558,21 @@ static void bt_health_connect_channel(const void *buf, uint16_t len)
>  	struct hal_rsp_health_connect_channel rsp;
>  	struct health_device *dev = NULL;
>  	struct health_channel *channel = NULL;
> +	struct health_app *app;
>  
>  	DBG("");
>  
> -	dev = get_device(cmd->app_id, cmd->bdaddr);
> +	app = get_app(cmd->app_id);
> +	if (!app)
> +		goto fail;
> +
> +	dev = get_device(app, cmd->bdaddr);
>  	if (!dev)
>  		goto fail;
>  
> -	channel = get_channel(cmd->app_id, cmd->mdep_index, dev);
> +	dev->app_id = cmd->app_id;

Isn't device already having this set?

> +
> +	channel = get_channel(app, cmd->mdep_index, dev);
>  	if (!channel)
>  		goto fail;
>  
> 

-- 
Best regards, 
Szymon Janc

Re: [PATCHv3 09/12] android/health: Add handling for ECHO service

[Szymon Janc]

Hi Andrei,

On Friday 27 of June 2014 14:25:00 Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> Reply received buffer back for ECHO service.
> ---
>  android/health.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 62 insertions(+), 1 deletion(-)
> 
> diff --git a/android/health.c b/android/health.c
> index 4f15f93..9631d9e 100644
> --- a/android/health.c
> +++ b/android/health.c
> @@ -1054,9 +1054,70 @@ static int get_dcpsm(sdp_list_t *recs, uint16_t *dcpsm)
>  	return -1;
>  }
>  
> +static int send_echo_data(int sock, const void *buf, uint32_t size)
> +{
> +	const uint8_t *buf_b = buf;
> +	uint32_t sent = 0;
> +
> +	while (sent < size) {
> +		int n = write(sock, buf_b + sent, size - sent);
> +		if (n < 0)
> +			return -1;
> +		sent += n;
> +	}
> +
> +	return 0;
> +}
> +
> +static gboolean serve_echo(GIOChannel *io, GIOCondition cond, gpointer data)
> +{
> +	struct health_channel *channel = data;
> +	uint8_t buf[MCAP_DC_MTU];
> +	int fd, len;
> +
> +	DBG("channel %p", channel);
> +
> +	if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL)) {
> +		DBG("Error condition on channel");
> +		return FALSE;
> +	}
> +
> +	fd = g_io_channel_unix_get_fd(io);
> +
> +	len = read(fd, buf, sizeof(buf));
> +	if (len < 0)
> +		goto fail;
> +
> +	if (send_echo_data(fd, buf, len)  >= 0)
> +		return TRUE;

Shouldn't initiator send only one single data packet in echo test function?

> +
> +fail:
> +	free_health_device(channel->dev);

Wouldn't that leave dangling pointer in channel? Also, why is this free here?

> +	return FALSE;
> +}
> +
>  static void mcap_mdl_connected_cb(struct mcap_mdl *mdl, void *data)
>  {
> -	DBG("Not Implemeneted");
> +	struct health_channel *channel = data;
> +
> +	if (!channel->mdl)
> +		channel->mdl = mcap_mdl_ref(mdl);
> +
> +	DBG("Data channel connected: mdl %p channel %p", mdl, channel);
> +
> +	if (channel->mdep_id == HDP_MDEP_ECHO) {
> +		GIOChannel *io;
> +		int fd;
> +
> +		fd = mcap_mdl_get_fd(channel->mdl);
> +		if (fd < 0)
> +			return;
> +
> +		io = g_io_channel_unix_new(fd);
> +		g_io_add_watch(io, G_IO_ERR | G_IO_HUP | G_IO_NVAL | G_IO_IN,
> +							serve_echo, channel);
> +		g_io_channel_unref(io);
> +	}
>  }
>  
>  static void mcap_mdl_closed_cb(struct mcap_mdl *mdl, void *data)
> 

-- 
Best regards, 
Szymon Janc

Re: [PATCHv3 05/12] android/health: Refactor channel creation

[Andrei Emeltchenko]

Hi Szymon,

On Fri, Jun 27, 2014 at 03:54:20PM +0200, Szymon Janc wrote:
> Hi Andrei,
> 
> On Friday 27 of June 2014 14:24:56 Andrei Emeltchenko wrote:
> > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> > 
> > Avoid using app_id when we shall use app structure itself. Otherwise we
> > end up with unnecessary searches for app and problems for incoming
> > connections.
> > ---
> >  android/health.c | 43 ++++++++++++++++++++++---------------------
> >  1 file changed, 22 insertions(+), 21 deletions(-)
> > 
> > diff --git a/android/health.c b/android/health.c
> > index 3cb2016..d664324 100644
> > --- a/android/health.c
> > +++ b/android/health.c
> > @@ -1477,43 +1477,37 @@ static struct health_device *create_device(struct health_app *app,
> >  	return dev;
> >  }
> >  
> > -static struct health_device *get_device(uint16_t app_id, const uint8_t *addr)
> > +static struct health_app *get_app(uint16_t app_id)
> > +{
> > +	return queue_find(apps, match_app_by_id, INT_TO_PTR(app_id));
> > +}
> > +
> > +static struct health_device *get_device(struct health_app *app,
> > +							const uint8_t *addr)
> >  {
> > -	struct health_app *app;
> >  	struct health_device *dev;
> >  	bdaddr_t bdaddr;
> >  
> > -	app = queue_find(apps, match_app_by_id, INT_TO_PTR(app_id));
> > -	if (!app)
> > -		return NULL;
> > -
> >  	android2bdaddr(addr, &bdaddr);
> >  	dev = queue_find(app->devices, match_dev_by_addr, &bdaddr);
> >  	if (dev)
> >  		return dev;
> >  
> > -	dev = create_device(app, addr);
> > -	if (dev)
> > -		dev->app_id = app_id;
> > -
> > -	return dev;
> > +	return create_device(app, addr);
> >  }
> >  
> > -static struct health_channel *create_channel(uint16_t app_id,
> > +static struct health_channel *create_channel(struct health_app *app,
> >  						uint8_t mdep_index,
> >  						struct health_device *dev)
> >  {
> > -	struct health_app *app;
> >  	struct mdep_cfg *mdep;
> >  	struct health_channel *channel;
> >  	uint8_t index;
> >  	static unsigned int channel_id = 1;
> >  
> > -	if (!dev)
> > -		return NULL;
> > +	DBG("mdep %u", mdep_index);
> >  
> > -	app = queue_find(apps, match_app_by_id, INT_TO_PTR(app_id));
> > -	if (!app)
> > +	if (!dev || !app)
> >  		return NULL;
> >  
> >  	index = mdep_index + 1;
> > @@ -1539,7 +1533,7 @@ static struct health_channel *create_channel(uint16_t app_id,
> >  	return channel;
> >  }
> >  
> > -static struct health_channel *get_channel(uint16_t app_id,
> > +static struct health_channel *get_channel(struct health_app *app,
> >  						uint8_t mdep_index,
> >  						struct health_device *dev)
> >  {
> > @@ -1555,7 +1549,7 @@ static struct health_channel *get_channel(uint16_t app_id,
> >  	if (channel)
> >  		return channel;
> >  
> > -	return create_channel(app_id, mdep_index, dev);
> > +	return create_channel(app, index, dev);
> >  }
> >  
> >  static void bt_health_connect_channel(const void *buf, uint16_t len)
> > @@ -1564,14 +1558,21 @@ static void bt_health_connect_channel(const void *buf, uint16_t len)
> >  	struct hal_rsp_health_connect_channel rsp;
> >  	struct health_device *dev = NULL;
> >  	struct health_channel *channel = NULL;
> > +	struct health_app *app;
> >  
> >  	DBG("");
> >  
> > -	dev = get_device(cmd->app_id, cmd->bdaddr);
> > +	app = get_app(cmd->app_id);
> > +	if (!app)
> > +		goto fail;
> > +
> > +	dev = get_device(app, cmd->bdaddr);
> >  	if (!dev)
> >  		goto fail;
> >  
> > -	channel = get_channel(cmd->app_id, cmd->mdep_index, dev);
> > +	dev->app_id = cmd->app_id;
> 
> Isn't device already having this set?

This one is just refactoring, so I leave all logic.

Best regards 
Andrei Emeltchenko 

Re: [PATCHv3 02/12] android/health: Fix wrong callback return type

[Szymon Janc]

Hi Andrei,

On Friday 27 of June 2014 14:24:53 Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> ---
>  android/health.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/android/health.c b/android/health.c
> index de67475..140afee 100644
> --- a/android/health.c
> +++ b/android/health.c
> @@ -1098,15 +1098,19 @@ static void mcap_mdl_aborted_cb(struct mcap_mdl *mdl, void *data)
>  	DBG("Not Implemeneted");
>  }
>  
> -static void mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
> +static uint8_t mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
>  				uint16_t mdlid, uint8_t *conf, void *data)
>  {
>  	DBG("Not Implemeneted");
> +
> +	return MCAP_SUCCESS;
>  }
>  
> -static void mcap_mdl_reconn_req_cb(struct mcap_mdl *mdl, void *data)
> +static uint8_t mcap_mdl_reconn_req_cb(struct mcap_mdl *mdl, void *data)
>  {
>  	DBG("Not Implemeneted");
> +
> +	return MCAP_SUCCESS;
>  }
>  
>  static void connect_mdl_cb(struct mcap_mdl *mdl, GError *gerr, gpointer data)

This patch is now applied. Thanks. 

-- 
Best regards, 
Szymon Janc

Re: [PATCHv3 11/12] android/mcap: Fix using uninitialised value

[Szymon Janc]

Hi Andrei,

On Friday 27 of June 2014 14:25:02 Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> Fixes clang warning:
> ...
> android/mcap-lib.c:2366:20: warning: The left operand of '*' is a
> garbage value
>         return tv->tv_sec * 1000000ll + tv->tv_nsec / 1000ll;
>                ~~~~~~~~~~ ^
> 1 warning generated.
> ...
> ---
>  android/mcap-lib.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/android/mcap-lib.c b/android/mcap-lib.c
> index 256abe1..79f6ce2 100644
> --- a/android/mcap-lib.c
> +++ b/android/mcap-lib.c
> @@ -2454,7 +2454,8 @@ uint64_t mcap_get_timestamp(struct mcap_mcl *mcl,
>  	if (given_time)
>  		now = *given_time;
>  	else
> -		clock_gettime(CLK, &now);
> +		if (clock_gettime(CLK, &now) < 0)
> +			return MCAP_TMSTAMP_DONTSET;
>  
>  	tmstamp = time_us(&now) - time_us(&mcl->csp->base_time)
>  		+ mcl->csp->base_tmstamp;
> 

Patch applied. Thanks.

-- 
Best regards, 
Szymon Janc

Re: [PATCHv3 09/12] android/health: Add handling for ECHO service

[Andrei Emeltchenko]

Hi Szymon,

On Fri, Jun 27, 2014 at 04:09:18PM +0200, Szymon Janc wrote:
> Hi Andrei,
> 
> On Friday 27 of June 2014 14:25:00 Andrei Emeltchenko wrote:
> > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> > 
> > Reply received buffer back for ECHO service.
> > ---
> >  android/health.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 62 insertions(+), 1 deletion(-)
> > 
> > diff --git a/android/health.c b/android/health.c
> > index 4f15f93..9631d9e 100644
> > --- a/android/health.c
> > +++ b/android/health.c
> > @@ -1054,9 +1054,70 @@ static int get_dcpsm(sdp_list_t *recs, uint16_t *dcpsm)
> >  	return -1;
> >  }
> >  
> > +static int send_echo_data(int sock, const void *buf, uint32_t size)
> > +{
> > +	const uint8_t *buf_b = buf;
> > +	uint32_t sent = 0;
> > +
> > +	while (sent < size) {
> > +		int n = write(sock, buf_b + sent, size - sent);
> > +		if (n < 0)
> > +			return -1;
> > +		sent += n;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static gboolean serve_echo(GIOChannel *io, GIOCondition cond, gpointer data)
> > +{
> > +	struct health_channel *channel = data;
> > +	uint8_t buf[MCAP_DC_MTU];
> > +	int fd, len;
> > +
> > +	DBG("channel %p", channel);
> > +
> > +	if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL)) {
> > +		DBG("Error condition on channel");
> > +		return FALSE;
> > +	}
> > +
> > +	fd = g_io_channel_unix_get_fd(io);
> > +
> > +	len = read(fd, buf, sizeof(buf));
> > +	if (len < 0)
> > +		goto fail;
> > +
> > +	if (send_echo_data(fd, buf, len)  >= 0)
> > +		return TRUE;
> 
> Shouldn't initiator send only one single data packet in echo test function?
> 

yes you are right here.

> > +
> > +fail:
> > +	free_health_device(channel->dev);
> 
> Wouldn't that leave dangling pointer in channel? Also, why is this free here?
> 

I will rework this patch.

Best regards 
Andrei Emeltchenko 

Re: [PATCHv3 05/12] android/health: Refactor channel creation

[Szymon Janc]

Hi Andrei,

On Friday 27 of June 2014 17:10:05 Andrei Emeltchenko wrote:
> Hi Szymon,
> 
> On Fri, Jun 27, 2014 at 03:54:20PM +0200, Szymon Janc wrote:
> > Hi Andrei,
> > 
> > On Friday 27 of June 2014 14:24:56 Andrei Emeltchenko wrote:
> > > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> > > 
> > > Avoid using app_id when we shall use app structure itself. Otherwise we
> > > end up with unnecessary searches for app and problems for incoming
> > > connections.
> > > ---
> > >  android/health.c | 43 ++++++++++++++++++++++---------------------
> > >  1 file changed, 22 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/android/health.c b/android/health.c
> > > index 3cb2016..d664324 100644
> > > --- a/android/health.c
> > > +++ b/android/health.c
> > > @@ -1477,43 +1477,37 @@ static struct health_device *create_device(struct health_app *app,
> > >  	return dev;
> > >  }
> > >  
> > > -static struct health_device *get_device(uint16_t app_id, const uint8_t *addr)
> > > +static struct health_app *get_app(uint16_t app_id)
> > > +{
> > > +	return queue_find(apps, match_app_by_id, INT_TO_PTR(app_id));
> > > +}
> > > +
> > > +static struct health_device *get_device(struct health_app *app,
> > > +							const uint8_t *addr)
> > >  {
> > > -	struct health_app *app;
> > >  	struct health_device *dev;
> > >  	bdaddr_t bdaddr;
> > >  
> > > -	app = queue_find(apps, match_app_by_id, INT_TO_PTR(app_id));
> > > -	if (!app)
> > > -		return NULL;
> > > -
> > >  	android2bdaddr(addr, &bdaddr);
> > >  	dev = queue_find(app->devices, match_dev_by_addr, &bdaddr);
> > >  	if (dev)
> > >  		return dev;
> > >  
> > > -	dev = create_device(app, addr);
> > > -	if (dev)
> > > -		dev->app_id = app_id;
> > > -
> > > -	return dev;
> > > +	return create_device(app, addr);
> > >  }
> > >  
> > > -static struct health_channel *create_channel(uint16_t app_id,
> > > +static struct health_channel *create_channel(struct health_app *app,
> > >  						uint8_t mdep_index,
> > >  						struct health_device *dev)
> > >  {
> > > -	struct health_app *app;
> > >  	struct mdep_cfg *mdep;
> > >  	struct health_channel *channel;
> > >  	uint8_t index;
> > >  	static unsigned int channel_id = 1;
> > >  
> > > -	if (!dev)
> > > -		return NULL;
> > > +	DBG("mdep %u", mdep_index);
> > >  
> > > -	app = queue_find(apps, match_app_by_id, INT_TO_PTR(app_id));
> > > -	if (!app)
> > > +	if (!dev || !app)
> > >  		return NULL;
> > >  
> > >  	index = mdep_index + 1;
> > > @@ -1539,7 +1533,7 @@ static struct health_channel *create_channel(uint16_t app_id,
> > >  	return channel;
> > >  }
> > >  
> > > -static struct health_channel *get_channel(uint16_t app_id,
> > > +static struct health_channel *get_channel(struct health_app *app,
> > >  						uint8_t mdep_index,
> > >  						struct health_device *dev)
> > >  {
> > > @@ -1555,7 +1549,7 @@ static struct health_channel *get_channel(uint16_t app_id,
> > >  	if (channel)
> > >  		return channel;
> > >  
> > > -	return create_channel(app_id, mdep_index, dev);
> > > +	return create_channel(app, index, dev);
> > >  }
> > >  
> > >  static void bt_health_connect_channel(const void *buf, uint16_t len)
> > > @@ -1564,14 +1558,21 @@ static void bt_health_connect_channel(const void *buf, uint16_t len)
> > >  	struct hal_rsp_health_connect_channel rsp;
> > >  	struct health_device *dev = NULL;
> > >  	struct health_channel *channel = NULL;
> > > +	struct health_app *app;
> > >  
> > >  	DBG("");
> > >  
> > > -	dev = get_device(cmd->app_id, cmd->bdaddr);
> > > +	app = get_app(cmd->app_id);
> > > +	if (!app)
> > > +		goto fail;
> > > +
> > > +	dev = get_device(app, cmd->bdaddr);
> > >  	if (!dev)
> > >  		goto fail;
> > >  
> > > -	channel = get_channel(cmd->app_id, cmd->mdep_index, dev);
> > > +	dev->app_id = cmd->app_id;
> > 
> > Isn't device already having this set?
> 
> This one is just refactoring, so I leave all logic.

OK, but then while you do refactor why not clean this up?
I find it a bit strange that moving dev->app_id assignment outside
of get_device() (which already takes app as parameter) is suppose to
make code better.

-- 
Best regards, 
Szymon Janc