[PATCHv2 1/6] android/health: Set L2CAP mode for MCAP channels

[PATCHv2 1/6] android/health: Set L2CAP mode for MCAP channels

[Andrei Emeltchenko]

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

Check configuration and set L2CAP mode. Enables several PTS test cases.
---
 android/health.c | 73 ++++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 58 insertions(+), 15 deletions(-)

diff --git a/android/health.c b/android/health.c
index 83ae107..fcb40f2 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1301,24 +1301,18 @@ static struct health_channel *create_channel(struct health_app *app,
 	return channel;
 }
 
-static struct health_channel *connect_channel(struct mcap_mcl *mcl,
-								uint8_t mdepid)
+static struct health_channel *connect_channel(struct health_app *app,
+							struct mcap_mcl *mcl,
+							uint8_t mdepid)
 {
-	struct health_app *app;
 	struct health_device *device;
 	struct health_channel *channel = NULL;
 	bdaddr_t addr;
 
-	DBG("mcl %p mdepid %u", mcl, mdepid);
+	DBG("app %p mdepid %u", app, mdepid);
 
 	mcap_mcl_get_addr(mcl, &addr);
 
-	if (mdepid == MDEP_ECHO)
-		/* For echo service take last app */
-		app = queue_peek_tail(apps);
-	else
-		app = search_app_by_mdepid(mdepid);
-
 	if (!app) {
 		DBG("No app found for mdepid %u", mdepid);
 		return NULL;
@@ -1330,25 +1324,41 @@ static struct health_channel *connect_channel(struct mcap_mcl *mcl,
 
 	channel = create_channel(app, mdepid, device);
 
-	/* Channel is assigned here after creation */
-	mcl->cb->user_data = channel;
-
 	return channel;
 }
 
+static uint8_t conf_to_l2cap(uint8_t conf)
+{
+	return conf == CHANNEL_TYPE_STREAM ? L2CAP_MODE_STREAMING :
+								L2CAP_MODE_ERTM;
+}
+
 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;
+	struct health_app *app;
+	struct mdep_cfg *mdep;
 
 	DBG("Data channel request: mdepid %u mdlid %u", mdepid, mdlid);
 
-	/* TODO: find / create device */
-	channel = connect_channel(mcl, mdepid);
+	if (mdepid == MDEP_ECHO)
+		/* For echo service take last app */
+		app = queue_peek_tail(apps);
+	else
+		app = search_app_by_mdepid(mdepid);
+
+	if (!app)
+		return MCAP_MDL_BUSY;
+
+	channel = connect_channel(app, mcl, mdepid);
 	if (!channel)
 		return MCAP_MDL_BUSY;
 
+	/* Channel is assigned here after creation */
+	mcl->cb->user_data = channel;
+
 	if (mdepid == MDEP_ECHO) {
 		switch (*conf) {
 		case CHANNEL_TYPE_ANY:
@@ -1379,6 +1389,39 @@ static uint8_t mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
 		return MCAP_SUCCESS;
 	}
 
+	mdep = queue_find(app->mdeps, match_mdep_by_id, INT_TO_PTR(mdepid));
+	if (!mdep)
+		return MCAP_MDL_BUSY;
+
+	switch (*conf) {
+	case CHANNEL_TYPE_ANY:
+		if (mdep->role == HAL_HEALTH_MDEP_ROLE_SINK)
+			return MCAP_CONFIGURATION_REJECTED;
+		else
+			*conf = CHANNEL_TYPE_RELIABLE;
+		break;
+	case CHANNEL_TYPE_STREAM:
+		if (mdep->role == HAL_HEALTH_MDEP_ROLE_SOURCE)
+			return MCAP_CONFIGURATION_REJECTED;
+		break;
+	case CHANNEL_TYPE_RELIABLE:
+		if (mdep->role == HAL_HEALTH_MDEP_ROLE_SOURCE)
+			return MCAP_CONFIGURATION_REJECTED;
+		break;
+	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, conf_to_l2cap(*conf), &gerr)) {
+		error("health: error setting L2CAP mode: %s", gerr->message);
+		g_error_free(gerr);
+		return MCAP_MDL_BUSY;
+	}
+
 	return MCAP_SUCCESS;
 }
 
-- 
1.8.3.2

[PATCHv2 2/6] android/health: Add channel_notify for incoming connections

[Andrei Emeltchenko]

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

For other then ECHO mdep send send_channel_state_notify with fd.
---
 android/health.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/android/health.c b/android/health.c
index fcb40f2..94d3804 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1136,25 +1136,36 @@ static gboolean serve_echo(GIOChannel *io, GIOCondition cond, gpointer data)
 static void mcap_mdl_connected_cb(struct mcap_mdl *mdl, void *data)
 {
 	struct health_channel *channel = data;
+	int fd;
 
 	if (!channel->mdl)
 		channel->mdl = mcap_mdl_ref(mdl);
 
 	DBG("Data channel connected: mdl %p channel %p", mdl, channel);
 
+	fd = mcap_mdl_get_fd(channel->mdl);
+	if (fd < 0) {
+		error("health: error retrieving fd");
+		goto fail;
+	}
+
 	if (channel->mdep_id == 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);
+
+		return;
 	}
+
+	send_channel_state_notify(channel, HAL_HEALTH_CHANNEL_CONNECTED, fd);
+
+	return;
+fail:
+	/* TODO: mcap_mdl_abort */
+	destroy_channel(channel);
 }
 
 static void mcap_mdl_closed_cb(struct mcap_mdl *mdl, void *data)
-- 
1.8.3.2

[PATCHv2 3/6] android/client: Fix using unvalidated index to access array

[Andrei Emeltchenko]

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

Fixes static analyzers warnings.
---
 android/client/if-hl.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/android/client/if-hl.c b/android/client/if-hl.c
index 949c574..7b75575 100644
--- a/android/client/if-hl.c
+++ b/android/client/if-hl.c
@@ -295,7 +295,16 @@ static void close_channel_p(int argc, const char **argv)
 	}
 
 	app_id = (uint32_t) atoi(argv[2]);
+	if (app_id >= APP_ID_SIZE) {
+		haltest_error("Wrong app_id specidied: %u\n", app_id);
+		return;
+	}
+
 	mdep_cfg_index = (uint8_t) atoi(argv[3]);
+	if (mdep_cfg_index >= MDEP_CFG_SIZE) {
+		haltest_error("Wrong mdep cgf index: %u\n", mdep_cfg_index);
+		return;
+	}
 
 	if (app_info[app_id][mdep_cfg_index][2] >= 0) {
 		shutdown(app_info[app_id][mdep_cfg_index][2], SHUT_RDWR);
-- 
1.8.3.2

[PATCHv2 4/6] android/health: Fix adding missing break

[Andrei Emeltchenko]

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

---
 android/health.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/android/health.c b/android/health.c
index 94d3804..9d360be 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1374,6 +1374,7 @@ static uint8_t mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
 		switch (*conf) {
 		case CHANNEL_TYPE_ANY:
 			*conf = CHANNEL_TYPE_RELIABLE;
+			break;
 		case CHANNEL_TYPE_RELIABLE:
 			break;
 		case CHANNEL_TYPE_STREAM:
-- 
1.8.3.2

[PATCHv2 5/6] HDP: Fix adding missing breaks

[Andrei Emeltchenko]

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

---
 profiles/health/hdp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/profiles/health/hdp.c b/profiles/health/hdp.c
index 54aca9d..d09bdd4 100644
--- a/profiles/health/hdp.c
+++ b/profiles/health/hdp.c
@@ -1066,6 +1066,7 @@ static uint8_t hdp_mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
 		switch (*conf) {
 		case HDP_NO_PREFERENCE_DC:
 			*conf = HDP_RELIABLE_DC;
+			break;
 		case HDP_RELIABLE_DC:
 			break;
 		case HDP_STREAMING_DC:
@@ -1112,6 +1113,7 @@ static uint8_t hdp_mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
 	case HDP_STREAMING_DC:
 		if (!dev->fr || app->role == HDP_SOURCE)
 			return MCAP_CONFIGURATION_REJECTED;
+		break;
 	case HDP_RELIABLE_DC:
 		if (app->role == HDP_SOURCE)
 			return MCAP_CONFIGURATION_REJECTED;
-- 
1.8.3.2

[PATCHv2 6/6] android/pts: Update PTS test results for HDP

[Andrei Emeltchenko]

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

---
 android/pts-hdp.txt | 54 ++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 45 insertions(+), 9 deletions(-)

diff --git a/android/pts-hdp.txt b/android/pts-hdp.txt
index f2b1528..a629937 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: 27-June-2014
+Tested: 30-June-2014
 Android version: 4.4.2
 
 Results:
@@ -69,7 +69,11 @@ TC_SRC_CC_BV_03_C	PASS    haltest:
 
 				when prompted: bluetooth ssp_reply <args>
 
-TC_SRC_CC_BV_05_C	INC
+TC_SRC_CC_BV_05_C	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_CC_BV_07_C	PASS    haltest:
 				hl register_application bluez-android Bluez
 				bluez-hdp health-device-profile 2
@@ -98,7 +102,11 @@ TC_SRC_HCT_BV_01_I	PASS    haltest:
 				<mdep_cfg_index>
 
 				when prompted: bluetooth ssp_reply <args>
-TC_SRC_HCT_BV_02_I	INC
+TC_SRC_HCT_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_HCT_BV_03_I	N/A
 TC_SRC_HCT_BV_04_I	INC
 TC_SRC_HCT_BV_05_C	N/A
@@ -162,7 +170,13 @@ TC_SNK_CC_BV_04_C	PASS    haltest:
 
 				when prompted: bluetooth ssp_reply <args>
 
-TC_SNK_CC_BV_06_C	INC
+TC_SNK_CC_BV_06_C	PASS	haltest:
+				hl register_application bluez-android Bluez
+				bluez-hdp health-device-profile 2
+				BTHL_MDEP_ROLE_SINK 4100 BTHL_CHANNEL_TYPE_RELIABLE
+				pulse-oximeter
+				BTHL_MDEP_ROLE_SINK 4100 BTHL_CHANNEL_TYPE_STREAMING
+				pulse-oximeter
 TC_SNK_CC_BV_08_C	PASS    haltest:
 				hl register_application bluez-android Bluez
 				bluez-hdp health-device-profile 2
@@ -180,8 +194,18 @@ TC_SNK_CC_BV_08_C	PASS    haltest:
 				hl connect_channel <app_id> <bd_addr>
 				<mdep_cfg_index>
 
-TC_SNK_CC_BV_10_C	INC
-TC_SNK_CC_BI_11_C	INC
+TC_SNK_CC_BV_10_C	PASS	haltest:
+				hl register_application bluez-android Bluez
+				bluez-hdp health-device-profile 2
+				BTHL_MDEP_ROLE_SINK 4100 BTHL_CHANNEL_TYPE_RELIABLE
+				pulse-oximeter
+				BTHL_MDEP_ROLE_SINK 4100 BTHL_CHANNEL_TYPE_STREAMING
+				pulse-oximeter
+TC_SNK_CC_BI_11_C	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_HCT_BV_01_I	PASS    haltest:
 				hl register_application bluez-android Bluez
 				bluez-hdp health-device-profile 1
@@ -192,12 +216,24 @@ TC_SNK_HCT_BV_01_I	PASS    haltest:
 				<mdep_cfg_index>
 
 				when prompted: bluetooth ssp_reply <args>
-TC_SNK_HCT_BV_02_I	INC
+TC_SNK_HCT_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_HCT_BV_03_I	N/A
 TC_SNK_HCT_BV_04_I	INC
 TC_SNK_HCT_BV_05_C	N/A
-TC_SNK_HCT_BV_06_C	INC
-TC_SNK_HCT_BV_07_C	INC
+TC_SNK_HCT_BV_06_C	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_HCT_BV_07_C	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_DE_BV_01_I	N/A
 TC_SNK_DE_BV_02_I	PASS	haltest:
 				hl register_application bluez-android Bluez
-- 
1.8.3.2

Re: [PATCHv2 1/6] android/health: Set L2CAP mode for MCAP channels

[Szymon Janc]

Hi Andrei,

On Monday 30 of June 2014 15:53:44 Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> Check configuration and set L2CAP mode. Enables several PTS test cases.
> ---
>  android/health.c | 73 ++++++++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 58 insertions(+), 15 deletions(-)
> 
> diff --git a/android/health.c b/android/health.c
> index 83ae107..fcb40f2 100644
> --- a/android/health.c
> +++ b/android/health.c
> @@ -1301,24 +1301,18 @@ static struct health_channel *create_channel(struct health_app *app,
>  	return channel;
>  }
>  
> -static struct health_channel *connect_channel(struct mcap_mcl *mcl,
> -								uint8_t mdepid)
> +static struct health_channel *connect_channel(struct health_app *app,
> +							struct mcap_mcl *mcl,
> +							uint8_t mdepid)
>  {
> -	struct health_app *app;
>  	struct health_device *device;
>  	struct health_channel *channel = NULL;
>  	bdaddr_t addr;
>  
> -	DBG("mcl %p mdepid %u", mcl, mdepid);
> +	DBG("app %p mdepid %u", app, mdepid);
>  
>  	mcap_mcl_get_addr(mcl, &addr);
>  
> -	if (mdepid == MDEP_ECHO)
> -		/* For echo service take last app */
> -		app = queue_peek_tail(apps);
> -	else
> -		app = search_app_by_mdepid(mdepid);
> -
>  	if (!app) {
>  		DBG("No app found for mdepid %u", mdepid);
>  		return NULL;
> @@ -1330,25 +1324,41 @@ static struct health_channel *connect_channel(struct mcap_mcl *mcl,
>  
>  	channel = create_channel(app, mdepid, device);
>  
> -	/* Channel is assigned here after creation */
> -	mcl->cb->user_data = channel;
> -
>  	return channel;
>  }
>  
> +static uint8_t conf_to_l2cap(uint8_t conf)
> +{
> +	return conf == CHANNEL_TYPE_STREAM ? L2CAP_MODE_STREAMING :
> +								L2CAP_MODE_ERTM;
> +}
> +
>  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;
> +	struct health_app *app;
> +	struct mdep_cfg *mdep;
>  
>  	DBG("Data channel request: mdepid %u mdlid %u", mdepid, mdlid);
>  
> -	/* TODO: find / create device */
> -	channel = connect_channel(mcl, mdepid);
> +	if (mdepid == MDEP_ECHO)
> +		/* For echo service take last app */
> +		app = queue_peek_tail(apps);
> +	else
> +		app = search_app_by_mdepid(mdepid);
> +
> +	if (!app)
> +		return MCAP_MDL_BUSY;
> +
> +	channel = connect_channel(app, mcl, mdepid);
>  	if (!channel)
>  		return MCAP_MDL_BUSY;
>  
> +	/* Channel is assigned here after creation */
> +	mcl->cb->user_data = channel;
> +
>  	if (mdepid == MDEP_ECHO) {
>  		switch (*conf) {
>  		case CHANNEL_TYPE_ANY:
> @@ -1379,6 +1389,39 @@ static uint8_t mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
>  		return MCAP_SUCCESS;
>  	}
>  
> +	mdep = queue_find(app->mdeps, match_mdep_by_id, INT_TO_PTR(mdepid));
> +	if (!mdep)
> +		return MCAP_MDL_BUSY;
> +
> +	switch (*conf) {
> +	case CHANNEL_TYPE_ANY:
> +		if (mdep->role == HAL_HEALTH_MDEP_ROLE_SINK)
> +			return MCAP_CONFIGURATION_REJECTED;
> +		else
> +			*conf = CHANNEL_TYPE_RELIABLE;
> +		break;
> +	case CHANNEL_TYPE_STREAM:
> +		if (mdep->role == HAL_HEALTH_MDEP_ROLE_SOURCE)
> +			return MCAP_CONFIGURATION_REJECTED;
> +		break;
> +	case CHANNEL_TYPE_RELIABLE:
> +		if (mdep->role == HAL_HEALTH_MDEP_ROLE_SOURCE)
> +			return MCAP_CONFIGURATION_REJECTED;
> +		break;
> +	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, conf_to_l2cap(*conf), &gerr)) {
> +		error("health: error setting L2CAP mode: %s", gerr->message);
> +		g_error_free(gerr);
> +		return MCAP_MDL_BUSY;
> +	}
> +
>  	return MCAP_SUCCESS;
>  }
 
All patches applied, thanks. 

-- 
Best regards, 
Szymon Janc