[PATCH_v3 0/4] Add HDP profile support at HAL side

[PATCH_v3 0/4] Add HDP profile support at HAL side

[Ravi kumar Veeramally]

v3: Patch set contains 10-13 from v2. Solved Szymon comments (strings
 copying to byte array with null terminated character).

v2: Solved Luiz, Szymon, Andrzej and Grzegorz comments.

v1: Patch set contains HDP profile support at HAL side and initial
 profile registration support at daemon side.

Ravi kumar Veeramally (4):
  android/hal-health: Add HDP .register_application method
  android/hal-health: Add HDP .unregister_application method
  android/hal-health: Add HDP .connect_channel method
  android/hal-health: Add HDP .destroy_channel method

 android/hal-health.c | 175 +++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 171 insertions(+), 4 deletions(-)

-- 
1.8.3.2

[PATCH_v3 1/4] android/hal-health: Add HDP .register_application method

[Ravi kumar Veeramally]

---
 android/hal-health.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 108 insertions(+), 1 deletion(-)

diff --git a/android/hal-health.c b/android/hal-health.c
index 918fb69..6e0f731 100644
--- a/android/hal-health.c
+++ b/android/hal-health.c
@@ -38,6 +38,113 @@ static bool interface_ready(void)
 static const struct hal_ipc_handler ev_handlers[] = {
 };
 
+static bt_status_t register_application(bthl_reg_param_t *reg, int *app_id)
+{
+	char buf[IPC_MTU];
+	struct hal_cmd_health_reg_app *cmd = (void *) buf;
+	struct hal_rsp_health_reg_app rsp;
+	size_t len = sizeof(rsp);
+	bt_status_t status;
+	ssize_t cmd_len;
+	uint8_t i;
+
+	DBG("");
+
+	if (!interface_ready())
+		return BT_STATUS_NOT_READY;
+
+	if (!reg || !app_id)
+		return BT_STATUS_PARM_INVALID;
+
+
+	cmd_len = sizeof(*cmd);
+
+	if (reg->application_name)
+		cmd_len += strlen(reg->application_name) + 1;
+
+	if (reg->provider_name)
+		cmd_len += strlen(reg->provider_name) + 1;
+
+	if (reg->srv_name)
+		cmd_len += strlen(reg->srv_name) + 1;
+
+	if (reg->srv_desp)
+		cmd_len += strlen(reg->srv_desp) + 1;
+
+	if (reg->number_of_mdeps > 0 && reg->mdep_cfg[0].mdep_description)
+		cmd_len += strlen(reg->mdep_cfg[0].mdep_description) + 1;
+
+	for (i = 1; i < reg->number_of_mdeps; i++)
+		cmd_len += sizeof(cmd->mdep_cfg) +
+				strlen(reg->mdep_cfg[i].mdep_description) + 1;
+
+	if (cmd_len > IPC_MTU) {
+		error("HDP app registration data length is more than IPC_MTU");
+		return BT_STATUS_PARM_INVALID;
+	}
+
+	if (reg->application_name) {
+		cmd->app_name.len = strlen(reg->application_name);
+		memcpy(cmd->app_name.data, reg->application_name,
+						cmd->app_name.len);
+	} else {
+		cmd->app_name.len = 0;
+	}
+
+	if (reg->provider_name) {
+		cmd->provider_name.len = strlen(reg->provider_name);
+		memcpy(cmd->provider_name.data, reg->provider_name,
+						cmd->provider_name.len);
+	} else {
+		cmd->provider_name.len = 0;
+	}
+
+	if (reg->srv_name) {
+		cmd->service_name.len = strlen(reg->srv_name);
+		memcpy(cmd->service_name.data, reg->srv_name,
+						cmd->service_name.len);
+	} else {
+		cmd->service_name.len = 0;
+	}
+
+	if (reg->srv_desp) {
+		cmd->service_descr.len = strlen(reg->srv_desp);
+		memcpy(cmd->service_descr.data, reg->srv_desp,
+						cmd->service_descr.len);
+	} else {
+		cmd->service_descr.len = 0;
+	}
+
+	cmd->num_of_mdep = reg->number_of_mdeps;
+
+	if (reg->mdep_cfg && reg->number_of_mdeps > 0) {
+		for (i = 0; i < reg->number_of_mdeps; i++) {
+			cmd->mdep_cfg[i].role = reg->mdep_cfg[i].mdep_role;
+			cmd->mdep_cfg[i].data_type = reg->mdep_cfg[i].data_type;
+			cmd->mdep_cfg[i].channel_type =
+						reg->mdep_cfg[i].channel_type;
+
+			if (cmd->mdep_cfg[i].descr.data) {
+				cmd->mdep_cfg[i].descr.len =
+				strlen(reg->mdep_cfg[i].mdep_description);
+				memcpy(cmd->mdep_cfg[i].descr.data,
+					reg->mdep_cfg[i].mdep_description,
+						cmd->mdep_cfg[i].descr.len);
+			} else {
+				cmd->mdep_cfg[i].descr.len = 0;
+			}
+		}
+	}
+
+	status = hal_ipc_cmd(HAL_SERVICE_ID_HEALTH, HAL_OP_HEALTH_REG_APP,
+						cmd_len, cmd, &len, &rsp, NULL);
+
+	if (status == HAL_STATUS_SUCCESS)
+		*app_id = rsp.app_id;
+
+	return status;
+}
+
 static bt_status_t init(bthl_callbacks_t *callbacks)
 {
 	struct hal_cmd_register_module cmd;
@@ -89,7 +196,7 @@ static void cleanup(void)
 static bthl_interface_t health_if = {
 	.size = sizeof(health_if),
 	.init = init,
-	.register_application = NULL,
+	.register_application = register_application,
 	.unregister_application = NULL,
 	.connect_channel = NULL,
 	.destroy_channel = NULL,
-- 
1.8.3.2

Re: [PATCH_v3 1/4] android/hal-health: Add HDP .register_application method

[Szymon Janc]

Hi Ravi,

On Friday 14 of March 2014 15:30:55 Ravi kumar Veeramally wrote:
> ---
>  android/hal-health.c | 109
> ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 108
> insertions(+), 1 deletion(-)
> 
> diff --git a/android/hal-health.c b/android/hal-health.c
> index 918fb69..6e0f731 100644
> --- a/android/hal-health.c
> +++ b/android/hal-health.c
> @@ -38,6 +38,113 @@ static bool interface_ready(void)
>  static const struct hal_ipc_handler ev_handlers[] = {
>  };
> 
> +static bt_status_t register_application(bthl_reg_param_t *reg, int *app_id)
> +{
> +	char buf[IPC_MTU];
> +	struct hal_cmd_health_reg_app *cmd = (void *) buf;
> +	struct hal_rsp_health_reg_app rsp;
> +	size_t len = sizeof(rsp);
> +	bt_status_t status;
> +	ssize_t cmd_len;
> +	uint8_t i;
> +
> +	DBG("");
> +
> +	if (!interface_ready())
> +		return BT_STATUS_NOT_READY;
> +
> +	if (!reg || !app_id)
> +		return BT_STATUS_PARM_INVALID;
> +
> +

Double empty line here.

> +	cmd_len = sizeof(*cmd);
> +
> +	if (reg->application_name)
> +		cmd_len += strlen(reg->application_name) + 1;
> +
> +	if (reg->provider_name)
> +		cmd_len += strlen(reg->provider_name) + 1;
> +
> +	if (reg->srv_name)
> +		cmd_len += strlen(reg->srv_name) + 1;
> +
> +	if (reg->srv_desp)
> +		cmd_len += strlen(reg->srv_desp) + 1;
> +
> +	if (reg->number_of_mdeps > 0 && reg->mdep_cfg[0].mdep_description)
> +		cmd_len += strlen(reg->mdep_cfg[0].mdep_description) + 1;
> +
> +	for (i = 1; i < reg->number_of_mdeps; i++)
> +		cmd_len += sizeof(cmd->mdep_cfg) +
> +				strlen(reg->mdep_cfg[i].mdep_description) + 1;
> +
> +	if (cmd_len > IPC_MTU) {
> +		error("HDP app registration data length is more than IPC_MTU");
> +		return BT_STATUS_PARM_INVALID;
> +	}
> +
> +	if (reg->application_name) {
> +		cmd->app_name.len = strlen(reg->application_name);
> +		memcpy(cmd->app_name.data, reg->application_name,
> +						cmd->app_name.len);

You are missing to copy NULL byte here and since buf is not zeroed those 
strings likely wont be NULL terminated.

So I think to avoid such problems in future we should have some simple helpers 
for creating hal_string. Also there is no need to calculate those strings 
lengths twice.

So that we could have something like (feel free to propose better API if this 
doesn't suites you).

bool string_to_hal(const char *str, struct hal_str *hstr, uint16_t *len);

if (!string_to_hal(reg->application_name, &hal_str, *cmd_len)
   goto failed;

> +	} else {
> +		cmd->app_name.len = 0;
> +	}
> +
> +	if (reg->provider_name) {
> +		cmd->provider_name.len = strlen(reg->provider_name);
> +		memcpy(cmd->provider_name.data, reg->provider_name,
> +						cmd->provider_name.len);
> +	} else {
> +		cmd->provider_name.len = 0;
> +	}
> +
> +	if (reg->srv_name) {
> +		cmd->service_name.len = strlen(reg->srv_name);
> +		memcpy(cmd->service_name.data, reg->srv_name,
> +						cmd->service_name.len);
> +	} else {
> +		cmd->service_name.len = 0;
> +	}
> +
> +	if (reg->srv_desp) {
> +		cmd->service_descr.len = strlen(reg->srv_desp);
> +		memcpy(cmd->service_descr.data, reg->srv_desp,
> +						cmd->service_descr.len);
> +	} else {
> +		cmd->service_descr.len = 0;
> +	}
> +
> +	cmd->num_of_mdep = reg->number_of_mdeps;
> +
> +	if (reg->mdep_cfg && reg->number_of_mdeps > 0) {
> +		for (i = 0; i < reg->number_of_mdeps; i++) {
> +			cmd->mdep_cfg[i].role = reg->mdep_cfg[i].mdep_role;
> +			cmd->mdep_cfg[i].data_type = reg->mdep_cfg[i].data_type;
> +			cmd->mdep_cfg[i].channel_type =
> +						reg->mdep_cfg[i].channel_type;
> +
> +			if (cmd->mdep_cfg[i].descr.data) {
> +				cmd->mdep_cfg[i].descr.len =
> +				strlen(reg->mdep_cfg[i].mdep_description);
> +				memcpy(cmd->mdep_cfg[i].descr.data,
> +					reg->mdep_cfg[i].mdep_description,
> +						cmd->mdep_cfg[i].descr.len);
> +			} else {
> +				cmd->mdep_cfg[i].descr.len = 0;
> +			}
> +		}
> +	}
> +
> +	status = hal_ipc_cmd(HAL_SERVICE_ID_HEALTH, HAL_OP_HEALTH_REG_APP,
> +						cmd_len, cmd, &len, &rsp, NULL);
> +
> +	if (status == HAL_STATUS_SUCCESS)
> +		*app_id = rsp.app_id;
> +
> +	return status;
> +}
> +
>  static bt_status_t init(bthl_callbacks_t *callbacks)
>  {
>  	struct hal_cmd_register_module cmd;
> @@ -89,7 +196,7 @@ static void cleanup(void)
>  static bthl_interface_t health_if = {
>  	.size = sizeof(health_if),
>  	.init = init,
> -	.register_application = NULL,
> +	.register_application = register_application,
>  	.unregister_application = NULL,
>  	.connect_channel = NULL,
>  	.destroy_channel = NULL,

-- 
BR
Szymon Janc

[PATCH_v3 2/4] android/hal-health: Add HDP .unregister_application method

[Ravi kumar Veeramally]

---
 android/hal-health.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/android/hal-health.c b/android/hal-health.c
index 6e0f731..6467b20 100644
--- a/android/hal-health.c
+++ b/android/hal-health.c
@@ -145,6 +145,21 @@ static bt_status_t register_application(bthl_reg_param_t *reg, int *app_id)
 	return status;
 }
 
+static bt_status_t unregister_application(int app_id)
+{
+	struct hal_cmd_health_unreg_app cmd;
+
+	DBG("");
+
+	if (!interface_ready())
+		return BT_STATUS_NOT_READY;
+
+	cmd.app_id = app_id;
+
+	return hal_ipc_cmd(HAL_SERVICE_ID_HEALTH, HAL_OP_HEALTH_UNREG_APP,
+					sizeof(cmd), &cmd, 0, NULL, NULL);
+}
+
 static bt_status_t init(bthl_callbacks_t *callbacks)
 {
 	struct hal_cmd_register_module cmd;
@@ -197,7 +212,7 @@ static bthl_interface_t health_if = {
 	.size = sizeof(health_if),
 	.init = init,
 	.register_application = register_application,
-	.unregister_application = NULL,
+	.unregister_application = unregister_application,
 	.connect_channel = NULL,
 	.destroy_channel = NULL,
 	.cleanup = cleanup
-- 
1.8.3.2

[PATCH_v3 3/4] android/hal-health: Add HDP .connect_channel method

[Ravi kumar Veeramally]

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

diff --git a/android/hal-health.c b/android/hal-health.c
index 6467b20..9153d5b 100644
--- a/android/hal-health.c
+++ b/android/hal-health.c
@@ -160,6 +160,36 @@ static bt_status_t unregister_application(int app_id)
 					sizeof(cmd), &cmd, 0, NULL, NULL);
 }
 
+static bt_status_t connect_channel(int app_id, bt_bdaddr_t *bd_addr,
+					int mdep_cfg_index, int *channel_id)
+{
+	struct hal_cmd_health_connect_channel cmd;
+	struct hal_rsp_health_connect_channel rsp;
+	size_t len = sizeof(rsp);
+	bt_status_t status;
+
+	DBG("");
+
+	if (!interface_ready())
+		return BT_STATUS_NOT_READY;
+
+	if (!bd_addr || !channel_id)
+		return BT_STATUS_PARM_INVALID;
+
+	cmd.app_id = app_id;
+	cmd.mdep_index = mdep_cfg_index;
+	memcpy(cmd.bdaddr, bd_addr, sizeof(cmd.bdaddr));
+
+	status = hal_ipc_cmd(HAL_SERVICE_ID_HEALTH,
+					HAL_OP_HEALTH_CONNECT_CHANNEL,
+					sizeof(cmd), &cmd, &len, &rsp, NULL);
+
+	if (status == HAL_STATUS_SUCCESS)
+		*channel_id = rsp.channel_id;
+
+	return status;
+}
+
 static bt_status_t init(bthl_callbacks_t *callbacks)
 {
 	struct hal_cmd_register_module cmd;
@@ -213,7 +243,7 @@ static bthl_interface_t health_if = {
 	.init = init,
 	.register_application = register_application,
 	.unregister_application = unregister_application,
-	.connect_channel = NULL,
+	.connect_channel = connect_channel,
 	.destroy_channel = NULL,
 	.cleanup = cleanup
 };
-- 
1.8.3.2

[PATCH_v3 4/4] android/hal-health: Add HDP .destroy_channel method

[Ravi kumar Veeramally]

---
 android/hal-health.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/android/hal-health.c b/android/hal-health.c
index 9153d5b..407f958 100644
--- a/android/hal-health.c
+++ b/android/hal-health.c
@@ -190,6 +190,21 @@ static bt_status_t connect_channel(int app_id, bt_bdaddr_t *bd_addr,
 	return status;
 }
 
+static bt_status_t destroy_channel(int channel_id)
+{
+	struct hal_cmd_health_destroy_channel cmd;
+
+	DBG("");
+
+	if (!interface_ready())
+		return BT_STATUS_NOT_READY;
+
+	cmd.channel_id = channel_id;
+
+	return hal_ipc_cmd(HAL_SERVICE_ID_HEALTH, HAL_OP_HEALTH_DESTROY_CHANNEL,
+					sizeof(cmd), &cmd, 0, NULL, NULL);
+}
+
 static bt_status_t init(bthl_callbacks_t *callbacks)
 {
 	struct hal_cmd_register_module cmd;
@@ -244,7 +259,7 @@ static bthl_interface_t health_if = {
 	.register_application = register_application,
 	.unregister_application = unregister_application,
 	.connect_channel = connect_channel,
-	.destroy_channel = NULL,
+	.destroy_channel = destroy_channel,
 	.cleanup = cleanup
 };
 
-- 
1.8.3.2

Re: [PATCH_v3 0/4] Add HDP profile support at HAL side

[Szymon Janc]

Hi Ravi,

On Friday 14 of March 2014 15:30:54 Ravi kumar Veeramally wrote:
> v3: Patch set contains 10-13 from v2. Solved Szymon comments (strings
>  copying to byte array with null terminated character).
> 
> v2: Solved Luiz, Szymon, Andrzej and Grzegorz comments.
> 
> v1: Patch set contains HDP profile support at HAL side and initial
>  profile registration support at daemon side.
> 
> Ravi kumar Veeramally (4):
>   android/hal-health: Add HDP .register_application method
>   android/hal-health: Add HDP .unregister_application method
>   android/hal-health: Add HDP .connect_channel method
>   android/hal-health: Add HDP .destroy_channel method
> 
>  android/hal-health.c | 175
> +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 171
> insertions(+), 4 deletions(-)

I've applied patches 2-4 so no need to resend them.

-- 
BR
Szymon Janc