[PATCH_v4 0/5] Fixed and implemented set report and send data ifaces

[PATCH_v4 0/5] Fixed and implemented set report and send data ifaces

[Ravi kumar Veeramally]

v4: Fixed changes as per Johan's comments. Conversion done inside
    functions now. Removed unnecessary allocation in hal-hidhost

v3: Fixed issues as per Johan's and Jerzy' comments and suggestions
    Implemented hex2bin as a static funtion and split the patches.

v2: Fixed issues as per Johan's comments and suggestions.
    This patch adds missing hid ipc documentation, hal headers,
    renaming and virtual unplug notification in hid HAL.
    Set report and send data interfaces are receiving data in ascii format
    but it should be in hex format. Provided utility for this and fixed
    set report data preparation. Implemented virtual unplug and send data
    methods in daemon.

v1: This patch adds missing hid ipc documentation, hal headers,
    renaming and virtual unplug notification in hid HAL.
    send RFC for ascii2hex utility.
    Unsupported methods are virtual unplug and send data in daemon.

Ravi kumar Veeramally (5):
  android/hid: Fix set seport ipc cmd preparation
  android/hid: Fix set report data format in daemon
  android/hid: Fill send data command struct in hal-hidhost
  android/hid: Add send data implemention in daemon
  android/hid: Add virtual unplug implemention in daemon

 android/hal-hidhost.c |  29 ++++++++------
 android/hal-msg.h     |   2 +-
 android/hidhost.c     | 104 ++++++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 116 insertions(+), 19 deletions(-)

-- 
1.8.3.2

[PATCH_v4 1/5] android/hid: Fix set seport ipc cmd preparation

[Ravi kumar Veeramally]

---
 android/hal-hidhost.c | 19 +++++++++++--------
 android/hal-msg.h     |  2 +-
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/android/hal-hidhost.c b/android/hal-hidhost.c
index 94f7e01..a0df7ce 100644
--- a/android/hal-hidhost.c
+++ b/android/hal-hidhost.c
@@ -18,6 +18,7 @@
 #include <stdbool.h>
 #include <stddef.h>
 #include <string.h>
+#include <stdlib.h>
 
 #include "hal-log.h"
 #include "hal.h"
@@ -298,7 +299,8 @@ static bt_status_t set_report(bt_bdaddr_t *bd_addr,
 						bthh_report_type_t report_type,
 						char *report)
 {
-	struct hal_cmd_hidhost_set_report cmd;
+	uint8_t buf[BLUEZ_HAL_MTU];
+	struct hal_cmd_hidhost_set_report *cmd = (void *) buf;
 
 	DBG("");
 
@@ -308,26 +310,27 @@ static bt_status_t set_report(bt_bdaddr_t *bd_addr,
 	if (!bd_addr || !report)
 		return BT_STATUS_PARM_INVALID;
 
-	memcpy(cmd.bdaddr, bd_addr, sizeof(cmd.bdaddr));
-	cmd.len = strlen(report);
-	memcpy(cmd.data, report, cmd.len);
+	memset(cmd, 0, sizeof(buf));
+	memcpy(cmd->bdaddr, bd_addr, sizeof(cmd->bdaddr));
+	cmd->len = strlen(report);
+	memcpy(cmd->data, report, cmd->len);
 
 	switch (report_type) {
 	case BTHH_INPUT_REPORT:
-		cmd.type = HAL_HIDHOST_INPUT_REPORT;
+		cmd->type = HAL_HIDHOST_INPUT_REPORT;
 		break;
 	case BTHH_OUTPUT_REPORT:
-		cmd.type = HAL_HIDHOST_OUTPUT_REPORT;
+		cmd->type = HAL_HIDHOST_OUTPUT_REPORT;
 		break;
 	case BTHH_FEATURE_REPORT:
-		cmd.type = HAL_HIDHOST_FEATURE_REPORT;
+		cmd->type = HAL_HIDHOST_FEATURE_REPORT;
 		break;
 	default:
 		return BT_STATUS_PARM_INVALID;
 	}
 
 	return hal_ipc_cmd(HAL_SERVICE_ID_HIDHOST, HAL_OP_HIDHOST_SET_REPORT,
-				sizeof(cmd), &cmd, 0, NULL, NULL);
+					sizeof(buf), buf, 0, NULL, NULL);
 }
 
 static bt_status_t send_data(bt_bdaddr_t *bd_addr, char *data)
diff --git a/android/hal-msg.h b/android/hal-msg.h
index 4af86de..4c7d344 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -293,7 +293,7 @@ struct hal_cmd_hidhost_set_report {
 	uint8_t  bdaddr[6];
 	uint8_t  type;
 	uint16_t len;
-	uint8_t  data[670];
+	uint8_t  data[0];
 } __attribute__((packed));
 
 #define HAL_OP_HIDHOST_SEND_DATA		0x09
-- 
1.8.3.2

[PATCH_v4 2/5] android/hid: Fix set report data format in daemon

[Ravi kumar Veeramally]

Report data coming to HAL is in ascii format, HAL sends
data in hex to daemon, so convert to binary.
---
 android/hidhost.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/android/hidhost.c b/android/hidhost.c
index 242fcbc..0e6bae0 100644
--- a/android/hidhost.c
+++ b/android/hidhost.c
@@ -884,7 +884,7 @@ static uint8_t bt_hid_set_report(struct hal_cmd_hidhost_set_report *cmd,
 	struct hid_device *dev;
 	GSList *l;
 	bdaddr_t dst;
-	int fd;
+	int i, fd;
 	uint8_t *req;
 	uint8_t req_size;
 
@@ -900,13 +900,20 @@ static uint8_t bt_hid_set_report(struct hal_cmd_hidhost_set_report *cmd,
 		return HAL_STATUS_FAILED;
 
 	dev = l->data;
-	req_size = 1 + cmd->len;
+
+	if (!(dev->ctrl_io))
+		return HAL_STATUS_FAILED;
+
+	req_size = 1 + (cmd->len / 2);
 	req = g_try_malloc0(req_size);
 	if (!req)
 		return HAL_STATUS_NOMEM;
 
 	req[0] = HID_MSG_SET_REPORT | cmd->type;
-	memcpy(req + 1, cmd->data, req_size - 1);
+	/* Report data coming to HAL is in ascii format, HAL sends
+	 * data in hex to daemon, so convert to binary. */
+	for (i = 0; i < (req_size - 1); i++)
+		sscanf((char *) &(cmd->data)[i * 2], "%hhx", &(req + 1)[i]);
 
 	fd = g_io_channel_unix_get_fd(dev->ctrl_io);
 
-- 
1.8.3.2

[PATCH_v4 3/5] android/hid: Fill send data command struct in hal-hidhost

[Ravi kumar Veeramally]

---
 android/hal-hidhost.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/android/hal-hidhost.c b/android/hal-hidhost.c
index a0df7ce..e4a15c0 100644
--- a/android/hal-hidhost.c
+++ b/android/hal-hidhost.c
@@ -335,7 +335,8 @@ static bt_status_t set_report(bt_bdaddr_t *bd_addr,
 
 static bt_status_t send_data(bt_bdaddr_t *bd_addr, char *data)
 {
-	struct hal_cmd_hidhost_send_data cmd;
+	uint8_t buf[BLUEZ_HAL_MTU];
+	struct hal_cmd_hidhost_send_data *cmd = (void *) buf;
 
 	DBG("");
 
@@ -345,10 +346,13 @@ static bt_status_t send_data(bt_bdaddr_t *bd_addr, char *data)
 	if (!bd_addr || !data)
 		return BT_STATUS_PARM_INVALID;
 
-	memcpy(cmd.bdaddr, bd_addr, sizeof(cmd.bdaddr));
+	memset(cmd, 0, sizeof(buf));
+	memcpy(cmd->bdaddr, bd_addr, sizeof(cmd->bdaddr));
+	cmd->len = strlen(data);
+	memcpy(cmd->data, data, cmd->len);
 
 	return hal_ipc_cmd(HAL_SERVICE_ID_HIDHOST, HAL_OP_HIDHOST_SEND_DATA,
-					sizeof(cmd), &cmd, 0, NULL, NULL);
+					sizeof(buf), buf, 0, NULL, NULL);
 }
 
 static bt_status_t init(bthh_callbacks_t *callbacks)
-- 
1.8.3.2

[PATCH_v4 4/5] android/hid: Add send data implemention in daemon

[Ravi kumar Veeramally]

Send data on interrupt channel on request from hid host.
---
 android/hidhost.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 43 insertions(+), 2 deletions(-)

diff --git a/android/hidhost.c b/android/hidhost.c
index 0e6bae0..250288c 100644
--- a/android/hidhost.c
+++ b/android/hidhost.c
@@ -931,9 +931,50 @@ static uint8_t bt_hid_set_report(struct hal_cmd_hidhost_set_report *cmd,
 static uint8_t bt_hid_send_data(struct hal_cmd_hidhost_send_data *cmd,
 								uint16_t len)
 {
-	DBG("Not Implemented");
+	struct hid_device *dev;
+	GSList *l;
+	bdaddr_t dst;
+	int i, fd;
+	uint8_t *req;
+	uint8_t req_size;
 
-	return HAL_STATUS_FAILED;
+	DBG("");
+
+	if (len < sizeof(*cmd))
+		return HAL_STATUS_INVALID;
+
+	android2bdaddr(&cmd->bdaddr, &dst);
+
+	l = g_slist_find_custom(devices, &dst, device_cmp);
+	if (!l)
+		return HAL_STATUS_FAILED;
+
+	dev = l->data;
+
+	if (!(dev->intr_io))
+		return HAL_STATUS_FAILED;
+
+	req_size = 1 + (cmd->len / 2);
+	req = g_try_malloc0(req_size);
+	if (!req)
+		return HAL_STATUS_NOMEM;
+
+	req[0] = HID_MSG_DATA | HID_DATA_TYPE_OUTPUT;
+	/* Report data coming to HAL is in ascii format, HAL sends
+	 * data in hex to daemon, so convert to binary. */
+	for (i = 0; i < (req_size - 1); i++)
+		sscanf((char *) &(cmd->data)[i * 2], "%hhx", &(req + 1)[i]);
+
+	fd = g_io_channel_unix_get_fd(dev->intr_io);
+
+	if (write(fd, req, req_size) < 0) {
+		error("error while sending data to device");
+		g_free(req);
+		return HAL_STATUS_FAILED;
+	}
+
+	g_free(req);
+	return HAL_STATUS_SUCCESS;
 }
 
 void bt_hid_handle_cmd(int sk, uint8_t opcode, void *buf, uint16_t len)
-- 
1.8.3.2

[PATCH_v4 5/5] android/hid: Add virtual unplug implemention in daemon

[Ravi kumar Veeramally]

Send virtual unplug command to hid device and disconnect and remove
hid device details.
---
 android/hidhost.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 44 insertions(+), 2 deletions(-)

diff --git a/android/hidhost.c b/android/hidhost.c
index 250288c..46f992e 100644
--- a/android/hidhost.c
+++ b/android/hidhost.c
@@ -55,6 +55,7 @@
 #define UHID_DEVICE_FILE	"/dev/uhid"
 
 /* HID message types */
+#define HID_MSG_CONTROL		0x10
 #define HID_MSG_GET_REPORT	0x40
 #define HID_MSG_SET_REPORT	0x50
 #define HID_MSG_GET_PROTOCOL	0x60
@@ -73,6 +74,9 @@
 /* HID GET REPORT Size Field */
 #define HID_GET_REPORT_SIZE_FIELD	0x08
 
+/* HID Virtual Cable Unplug */
+#define HID_VIRTUAL_CABLE_UNPLUG	0x05
+
 static int notification_sk = -1;
 static GIOChannel *ctrl_io = NULL;
 static GIOChannel *intr_io = NULL;
@@ -744,9 +748,47 @@ static uint8_t bt_hid_disconnect(struct hal_cmd_hidhost_disconnect *cmd,
 static uint8_t bt_hid_virtual_unplug(struct hal_cmd_hidhost_virtual_unplug *cmd,
 								uint16_t len)
 {
-	DBG("Not Implemented");
+	struct hid_device *dev;
+	GSList *l;
+	bdaddr_t dst;
+	uint8_t hdr;
+	int fd;
 
-	return HAL_STATUS_FAILED;
+	DBG("");
+
+	if (len < sizeof(*cmd))
+		return HAL_STATUS_INVALID;
+
+	android2bdaddr(&cmd->bdaddr, &dst);
+
+	l = g_slist_find_custom(devices, &dst, device_cmp);
+	if (!l)
+		return HAL_STATUS_FAILED;
+
+	dev = l->data;
+
+	if (!(dev->ctrl_io))
+		return HAL_STATUS_FAILED;
+
+	hdr = HID_MSG_CONTROL | HID_VIRTUAL_CABLE_UNPLUG;
+
+	fd = g_io_channel_unix_get_fd(dev->ctrl_io);
+
+	if (write(fd, &hdr, sizeof(hdr)) < 0) {
+		error("error while virtual unplug command");
+		return HAL_STATUS_FAILED;
+	}
+
+	/* Wait either channels to HUP */
+	if (dev->intr_io)
+		g_io_channel_shutdown(dev->intr_io, TRUE, NULL);
+
+	if (dev->ctrl_io)
+		g_io_channel_shutdown(dev->ctrl_io, TRUE, NULL);
+
+	bt_hid_notify_state(dev, HAL_HIDHOST_STATE_DISCONNECTING);
+
+	return HAL_STATUS_SUCCESS;
 }
 
 static uint8_t bt_hid_info(struct hal_cmd_hidhost_set_info *cmd, uint16_t len)
-- 
1.8.3.2

Re: [PATCH_v4 1/5] android/hid: Fix set seport ipc cmd preparation

[Johan Hedberg]

Hi Ravi,

On Fri, Nov 08, 2013, Ravi kumar Veeramally wrote:
> -	memcpy(cmd.data, report, cmd.len);
> +	memset(cmd, 0, sizeof(buf));

How about memset(buf, 0, sizeof(buf)) to make this perfectly clear. Btw,
why is the memset needed? I don't think the original code had it. Seems
like this is an independent bug fix?

> +	cmd->len = strlen(report);
> +	memcpy(cmd->data, report, cmd->len);
>  
>  	switch (report_type) {
>  	case BTHH_INPUT_REPORT:
> -		cmd.type = HAL_HIDHOST_INPUT_REPORT;
> +		cmd->type = HAL_HIDHOST_INPUT_REPORT;
>  		break;
>  	case BTHH_OUTPUT_REPORT:
> -		cmd.type = HAL_HIDHOST_OUTPUT_REPORT;
> +		cmd->type = HAL_HIDHOST_OUTPUT_REPORT;
>  		break;
>  	case BTHH_FEATURE_REPORT:
> -		cmd.type = HAL_HIDHOST_FEATURE_REPORT;
> +		cmd->type = HAL_HIDHOST_FEATURE_REPORT;
>  		break;
>  	default:
>  		return BT_STATUS_PARM_INVALID;
>  	}
>  
>  	return hal_ipc_cmd(HAL_SERVICE_ID_HIDHOST, HAL_OP_HIDHOST_SET_REPORT,
> -				sizeof(cmd), &cmd, 0, NULL, NULL);
> +					sizeof(buf), buf, 0, NULL, NULL);

This last call looks broken to me. Shouldn't you instead of sizeof(buf)
be sending sizeof(*cmd) + cmd->len?

Johan

Re: [PATCH_v4 1/5] android/hid: Fix set seport ipc cmd preparation

[Ravi Kumar Veeramally]

Hi Johan,

On 11/08/2013 02:15 PM, Johan Hedberg wrote:
> Hi Ravi,
>
> On Fri, Nov 08, 2013, Ravi kumar Veeramally wrote:
>> -	memcpy(cmd.data, report, cmd.len);
>> +	memset(cmd, 0, sizeof(buf));
> How about memset(buf, 0, sizeof(buf)) to make this perfectly clear.
   Ok.
>   Btw,
> why is the memset needed? I don't think the original code had it. Seems
> like this is an independent bug fix?
   Not in original, but in between versions (v1~v4) :(.
>
>> +	cmd->len = strlen(report);
>> +	memcpy(cmd->data, report, cmd->len);
>>   
>>   	switch (report_type) {
>>   	case BTHH_INPUT_REPORT:
>> -		cmd.type = HAL_HIDHOST_INPUT_REPORT;
>> +		cmd->type = HAL_HIDHOST_INPUT_REPORT;
>>   		break;
>>   	case BTHH_OUTPUT_REPORT:
>> -		cmd.type = HAL_HIDHOST_OUTPUT_REPORT;
>> +		cmd->type = HAL_HIDHOST_OUTPUT_REPORT;
>>   		break;
>>   	case BTHH_FEATURE_REPORT:
>> -		cmd.type = HAL_HIDHOST_FEATURE_REPORT;
>> +		cmd->type = HAL_HIDHOST_FEATURE_REPORT;
>>   		break;
>>   	default:
>>   		return BT_STATUS_PARM_INVALID;
>>   	}
>>   
>>   	return hal_ipc_cmd(HAL_SERVICE_ID_HIDHOST, HAL_OP_HIDHOST_SET_REPORT,
>> -				sizeof(cmd), &cmd, 0, NULL, NULL);
>> +					sizeof(buf), buf, 0, NULL, NULL);
> This last call looks broken to me. Shouldn't you instead of sizeof(buf)
> be sending sizeof(*cmd) + cmd->len?
   yes, should be "sizeof(*cmd) + cmd->len".
>
> Johan
>
Thanks,
Ravi.