2014-06-30 12:53:44

by Andrei Emeltchenko

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

From: Andrei Emeltchenko <[email protected]>

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



2014-06-30 13:16:57

by Szymon Janc

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

Hi Andrei,

On Monday 30 of June 2014 15:53:44 Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <[email protected]>
>
> 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

2014-06-30 12:53:46

by Andrei Emeltchenko

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

From: Andrei Emeltchenko <[email protected]>

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


2014-06-30 12:53:49

by Andrei Emeltchenko

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

From: Andrei Emeltchenko <[email protected]>

---
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


2014-06-30 12:53:47

by Andrei Emeltchenko

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

From: Andrei Emeltchenko <[email protected]>

---
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


2014-06-30 12:53:48

by Andrei Emeltchenko

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

From: Andrei Emeltchenko <[email protected]>

---
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


2014-06-30 12:53:45

by Andrei Emeltchenko

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

From: Andrei Emeltchenko <[email protected]>

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