[PATCH_v2 00/11] Fix issues and implement virtual unplug and send data

[PATCH_v2 00/11] Fix issues and implement virtual unplug and send data

[Ravi kumar Veeramally]

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 (11):
  android/hid: Remove fixed number of hid set report data length in ipc
    doc
  android/hid: Add a ascii2hex utility
  android/hid: Fix set report data format
  android/hid: Add missing hid send data parameters in ipc document
  android/hid: Add missing parameters in send data struct in HAL headers
  android/hid: Fill send data structure variables in hal-hidhost
  android/hid: Rename virtual unplug define and struct
  android/hid: Add hid event virtual unplug structure to HAL msg headers
  android/hid: Handle virtual unplug notification in hid hal
  android/hid: Add virtual unplug implemention in daemon
  android/hid: Add send data implemention in daemon

 android/Android.mk      |  1 +
 android/Makefile.am     |  2 +-
 android/hal-hidhost.c   | 64 ++++++++++++++++++++++++++--------
 android/hal-ipc-api.txt |  6 ++--
 android/hal-msg.h       | 16 ++++++---
 android/hidhost.c       | 91 ++++++++++++++++++++++++++++++++++++++++++++-----
 android/utils.c         | 41 ++++++++++++++++++++++
 android/utils.h         |  3 ++
 8 files changed, 193 insertions(+), 31 deletions(-)
 create mode 100644 android/utils.c

-- 
1.8.3.2

[PATCH_v2 01/11] android/hid: Remove fixed number of hid set report data length in ipc doc

[Ravi kumar Veeramally]

HAL receives data in ascii format but it should be in hex format. So remove
fixed size of report data length and depend on report length parameter.
---
 android/hal-ipc-api.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/android/hal-ipc-api.txt b/android/hal-ipc-api.txt
index c39cb0d..297f565 100644
--- a/android/hal-ipc-api.txt
+++ b/android/hal-ipc-api.txt
@@ -553,7 +553,7 @@ Commands and responses:
 		Command parameters: Remote address (6 octets)
 		                    Report type (1 octet)
 		                    Report length (2 octets)
-		                    Report data (670 octets)
+		                    Report data (Report length)
 
 		Response parameters: <none>
 
-- 
1.8.3.2

[PATCH_v2 02/11] android/hid: Add a ascii2hex utility

[Ravi kumar Veeramally]

Data from few interfaces on HAL is in ascii format but it should be
in hex format. This conversion utility does that job.
---
 android/Android.mk  |  1 +
 android/Makefile.am |  2 +-
 android/utils.c     | 41 +++++++++++++++++++++++++++++++++++++++++
 android/utils.h     |  3 +++
 4 files changed, 46 insertions(+), 1 deletion(-)
 create mode 100644 android/utils.c

diff --git a/android/Android.mk b/android/Android.mk
index 51037a7..628b2f9 100644
--- a/android/Android.mk
+++ b/android/Android.mk
@@ -27,6 +27,7 @@ LOCAL_SRC_FILES := \
 	ipc.c ipc.h \
 	a2dp.c \
 	pan.c \
+	utils.c \
 	../src/log.c \
 	../src/shared/mgmt.c \
 	../src/shared/util.c \
diff --git a/android/Makefile.am b/android/Makefile.am
index 073edc8..6063a3a 100644
--- a/android/Makefile.am
+++ b/android/Makefile.am
@@ -9,7 +9,7 @@ noinst_PROGRAMS += android/bluetoothd
 android_bluetoothd_SOURCES = android/main.c \
 				src/log.c \
 				android/hal-msg.h \
-				android/utils.h \
+				android/utils.h android/utils.c \
 				src/sdpd-database.c src/sdpd-server.c \
 				src/sdpd-service.c src/sdpd-request.c \
 				src/glib-helper.h src/glib-helper.c \
diff --git a/android/utils.c b/android/utils.c
new file mode 100644
index 0000000..48d6c38
--- /dev/null
+++ b/android/utils.c
@@ -0,0 +1,41 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2013  Intel Corporation. All rights reserved.
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include "utils.h"
+
+void ascii2hex(const uint8_t *ascii, int ascii_len, uint8_t *hex)
+{
+	int i;
+
+	if (!ascii || !hex)
+		return;
+
+	for (i = 0; i < ascii_len / 2; i++)
+		sscanf((char *) &ascii[i * 2], "%02x",
+						(unsigned int *) &hex[i]);
+
+}
diff --git a/android/utils.h b/android/utils.h
index 5b009bc..d424968 100644
--- a/android/utils.h
+++ b/android/utils.h
@@ -21,6 +21,7 @@
  *
  */
 
+#include "lib/bluetooth.h"
 
 static inline void android2bdaddr(const void *buf, bdaddr_t *dst)
 {
@@ -31,3 +32,5 @@ static inline void bdaddr2android(const bdaddr_t *src, void *buf)
 {
 	baswap(buf, src);
 }
+
+void ascii2hex(const uint8_t *ascii, int ascii_len, uint8_t *hex);
-- 
1.8.3.2

Re: [PATCH_v2 02/11] android/hid: Add a ascii2hex utility

[Jerzy Kasenberg]

Hi Ravi

On 7 November 2013 23:10, Ravi kumar Veeramally
<ravikumar.veeramally@linux.intel.com> wrote:
> Data from few interfaces on HAL is in ascii format but it should be
> in hex format. This conversion utility does that job.
> ---
>  android/Android.mk  |  1 +
>  android/Makefile.am |  2 +-
>  android/utils.c     | 41 +++++++++++++++++++++++++++++++++++++++++
>  android/utils.h     |  3 +++
>  4 files changed, 46 insertions(+), 1 deletion(-)
>  create mode 100644 android/utils.c
>
> diff --git a/android/Android.mk b/android/Android.mk
> index 51037a7..628b2f9 100644
> --- a/android/Android.mk
> +++ b/android/Android.mk
> @@ -27,6 +27,7 @@ LOCAL_SRC_FILES := \
>         ipc.c ipc.h \
>         a2dp.c \
>         pan.c \
> +       utils.c \
>         ../src/log.c \
>         ../src/shared/mgmt.c \
>         ../src/shared/util.c \
> diff --git a/android/Makefile.am b/android/Makefile.am
> index 073edc8..6063a3a 100644
> --- a/android/Makefile.am
> +++ b/android/Makefile.am
> @@ -9,7 +9,7 @@ noinst_PROGRAMS += android/bluetoothd
>  android_bluetoothd_SOURCES = android/main.c \
>                                 src/log.c \
>                                 android/hal-msg.h \
> -                               android/utils.h \
> +                               android/utils.h android/utils.c \
>                                 src/sdpd-database.c src/sdpd-server.c \
>                                 src/sdpd-service.c src/sdpd-request.c \
>                                 src/glib-helper.h src/glib-helper.c \
> diff --git a/android/utils.c b/android/utils.c
> new file mode 100644
> index 0000000..48d6c38
> --- /dev/null
> +++ b/android/utils.c
> @@ -0,0 +1,41 @@
> +/*
> + *
> + *  BlueZ - Bluetooth protocol stack for Linux
> + *
> + *  Copyright (C) 2013  Intel Corporation. All rights reserved.
> + *
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include "utils.h"
> +
> +void ascii2hex(const uint8_t *ascii, int ascii_len, uint8_t *hex)
> +{
> +       int i;
> +
> +       if (!ascii || !hex)
> +               return;
> +
> +       for (i = 0; i < ascii_len / 2; i++)
> +               sscanf((char *) &ascii[i * 2], "%02x",
> +                                               (unsigned int *) &hex[i]);

Use hh in format instead of cast.

> +
> +}
> diff --git a/android/utils.h b/android/utils.h
> index 5b009bc..d424968 100644
> --- a/android/utils.h
> +++ b/android/utils.h
> @@ -21,6 +21,7 @@
>   *
>   */
>
> +#include "lib/bluetooth.h"
>
>  static inline void android2bdaddr(const void *buf, bdaddr_t *dst)
>  {
> @@ -31,3 +32,5 @@ static inline void bdaddr2android(const bdaddr_t *src, void *buf)
>  {
>         baswap(buf, src);
>  }
> +
> +void ascii2hex(const uint8_t *ascii, int ascii_len, uint8_t *hex);
> --
> 1.8.3.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Jerzy

Re: [PATCH_v2 02/11] android/hid: Add a ascii2hex utility

[Johan Hedberg]

Hi Ravi,

On Fri, Nov 08, 2013, Ravi kumar Veeramally wrote:
> +++ b/android/utils.c
> @@ -0,0 +1,41 @@
> +/*
> + *
> + *  BlueZ - Bluetooth protocol stack for Linux
> + *
> + *  Copyright (C) 2013  Intel Corporation. All rights reserved.
> + *
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include "utils.h"
> +
> +void ascii2hex(const uint8_t *ascii, int ascii_len, uint8_t *hex)
> +{
> +	int i;
> +
> +	if (!ascii || !hex)
> +		return;
> +
> +	for (i = 0; i < ascii_len / 2; i++)
> +		sscanf((char *) &ascii[i * 2], "%02x",
> +						(unsigned int *) &hex[i]);
> +
> +}

I'd just keep this static inside the HID HAL since that seems to be the
only user for it. Actually I'm not even convinced that it's worth to
have this as a separate function since you only have two users and
essentially the function is just two lines (the if-statement is
redundant). Also, the function name isn't quite right. You're converting
from hex to binary. Not from ascii to hex (hex notation is already using
the ascii character set). Also, do consider the point from Jerzy about
the format specifier.

Johan

Re: [PATCH_v2 02/11] android/hid: Add a ascii2hex utility

[Johan Hedberg]

Hi Ravi,

On Fri, Nov 08, 2013, Ravi Kumar Veeramally wrote:
>>> +void ascii2hex(const uint8_t *ascii, int ascii_len, uint8_t *hex)
>>> +{
>>> +	int i;
>>> +
>>> +	if (!ascii || !hex)
>>> +		return;
>>> +
>>> +	for (i = 0; i < ascii_len / 2; i++)
>>> +		sscanf((char *) &ascii[i * 2], "%02x",
>>> +						(unsigned int *) &hex[i]);
>>> +
>>> +}
>
>> I'd just keep this static inside the HID HAL since that seems to be
>> the only user for it. Actually I'm not even convinced that it's worth
>> to have this as a separate function since you only have two users and
>> essentially the function is just two lines (the if-statement is
>> redundant).
>
> Ok, I will keep it as static. Do you want me to keep in HID HAL or HID
> daemon?

The daemon is where you were doing the conversion now as well, so no
need to change that (sorry about being unclear when I said "HID HAL").

>> Also, the function name isn't quite right. You're converting from hex
>> to binary
> 
>  We are converting to hex right? correct me If I am wrong.

No, we're converting from hex to binary.

>>. Not from ascii to hex (hex notation is already using
>>the ascii character set). Also, do consider the point from Jerzy about
>>the format specifier.
>>
>  Ok, I will fix Jerzy's suggestion also.
>
> Apart from this patch, do you have any comments on rest of the
> patches?

Only the ones I already sent.

Johan

Re: [PATCH_v2 02/11] android/hid: Add a ascii2hex utility

[Ravi Kumar Veeramally]

On 11/08/2013 10:48 AM, Johan Hedberg wrote:
> Hi Ravi,
>
> On Fri, Nov 08, 2013, Ravi Kumar Veeramally wrote:
>>>> +void ascii2hex(const uint8_t *ascii, int ascii_len, uint8_t *hex)
>>>> +{
>>>> +	int i;
>>>> +
>>>> +	if (!ascii || !hex)
>>>> +		return;
>>>> +
>>>> +	for (i = 0; i < ascii_len / 2; i++)
>>>> +		sscanf((char *) &ascii[i * 2], "%02x",
>>>> +						(unsigned int *) &hex[i]);
>>>> +
>>>> +}
>>> I'd just keep this static inside the HID HAL since that seems to be
>>> the only user for it. Actually I'm not even convinced that it's worth
>>> to have this as a separate function since you only have two users and
>>> essentially the function is just two lines (the if-statement is
>>> redundant).
>> Ok, I will keep it as static. Do you want me to keep in HID HAL or HID
>> daemon?
> The daemon is where you were doing the conversion now as well, so no
> need to change that (sorry about being unclear when I said "HID HAL").
>
>>> Also, the function name isn't quite right. You're converting from hex
>>> to binary
>>   We are converting to hex right? correct me If I am wrong.
> No, we're converting from hex to binary.
>
>>> . Not from ascii to hex (hex notation is already using
>>> the ascii character set). Also, do consider the point from Jerzy about
>>> the format specifier.
>>>
>>   Ok, I will fix Jerzy's suggestion also.
>>
>> Apart from this patch, do you have any comments on rest of the
>> patches?
> Only the ones I already sent.
>
> Johan
>
  Ok, Patch_v2 6 & 7 are independent of ascii stuff, can be it possible to
  apply if there are no comments? (just a request!!).

  Thanks a lot,
  Ravi.

[PATCH_v2 03/11] android/hid: Fix set report data format

[Ravi kumar Veeramally]

Data is in ascii format from HAL. Convert it to hex and send it to
hid device.
---
 android/hal-hidhost.c | 29 ++++++++++++++++++++---------
 android/hal-msg.h     |  2 +-
 android/hidhost.c     |  8 +++++---
 3 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/android/hal-hidhost.c b/android/hal-hidhost.c
index 3c147e8..6fe4c52 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"
@@ -285,7 +286,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;
+	struct hal_cmd_hidhost_set_report *cmd;
+	int cmd_len, status;
 
 	DBG("");
 
@@ -295,26 +297,35 @@ 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);
+	cmd_len = sizeof(*cmd) + sizeof(struct hal_cmd_hidhost_set_report)
+							+ 1 + strlen(report);
+
+	cmd = malloc(cmd_len);
+	memset(cmd, 0, cmd_len);
+	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:
+		free(cmd);
 		return BT_STATUS_PARM_INVALID;
 	}
 
-	return hal_ipc_cmd(HAL_SERVICE_ID_HIDHOST, HAL_OP_HIDHOST_SET_REPORT,
-				sizeof(cmd), &cmd, 0, NULL, NULL);
+	status = hal_ipc_cmd(HAL_SERVICE_ID_HIDHOST, HAL_OP_HIDHOST_SET_REPORT,
+						cmd_len, cmd, 0, NULL, NULL);
+	free(cmd);
+
+	return status;
 }
 
 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 7045c8c..08a159e 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
diff --git a/android/hidhost.c b/android/hidhost.c
index c7b4114..4b12318 100644
--- a/android/hidhost.c
+++ b/android/hidhost.c
@@ -900,18 +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;
+	/* Report data coming from HAL is in ascii format, so convert
+	 * it to hex and calculate length according to it. */
+	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);
+	ascii2hex(cmd->data, cmd->len, (req + 1));
 
 	fd = g_io_channel_unix_get_fd(dev->ctrl_io);
 
 	if (write(fd, req, req_size) < 0) {
-		error("error while querying device protocol");
+		error("error while sending report");
 		g_free(req);
 		return HAL_STATUS_FAILED;
 	}
-- 
1.8.3.2

[PATCH_v2 04/11] android/hid: Add missing hid send data parameters in ipc document

[Ravi kumar Veeramally]

---
 android/hal-ipc-api.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/android/hal-ipc-api.txt b/android/hal-ipc-api.txt
index 297f565..91ea280 100644
--- a/android/hal-ipc-api.txt
+++ b/android/hal-ipc-api.txt
@@ -566,7 +566,9 @@ Commands and responses:
 	Opcode 0x09 - Send Data command/response
 
 		Command parameters: Remote address (6 octets)
-		                    ...
+		                    Data length (2 octets)
+		                    Data (Data length)
+
 		Response parameters: <none>
 
 		In case of an error, the error response will be returned.
-- 
1.8.3.2

[PATCH_v2 05/11] android/hid: Add missing parameters in send data struct in HAL headers

[Ravi kumar Veeramally]

---
 android/hal-msg.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/android/hal-msg.h b/android/hal-msg.h
index 08a159e..05c9de0 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -298,7 +298,9 @@ struct hal_cmd_hidhost_set_report {
 
 #define HAL_OP_HIDHOST_SEND_DATA		0x09
 struct hal_cmd_hidhost_send_data {
-	uint8_t bdaddr[6];
+	uint8_t  bdaddr[6];
+	uint16_t len;
+	uint8_t  data[0];
 } __attribute__((packed));
 
 /* a2dp HAL API */
-- 
1.8.3.2

[PATCH_v2 06/11] android/hid: Fill send data structure variables in hal-hidhost

[Ravi kumar Veeramally]

---
 android/hal-hidhost.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/android/hal-hidhost.c b/android/hal-hidhost.c
index 6fe4c52..1617a16 100644
--- a/android/hal-hidhost.c
+++ b/android/hal-hidhost.c
@@ -330,7 +330,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;
+	struct hal_cmd_hidhost_send_data *cmd;
+	int cmd_len, status;
 
 	DBG("");
 
@@ -340,10 +341,19 @@ 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));
+	cmd_len = sizeof(*cmd) + sizeof(struct hal_cmd_hidhost_send_data)
+							+ 1 + strlen(data);
 
-	return hal_ipc_cmd(HAL_SERVICE_ID_HIDHOST, HAL_OP_HIDHOST_SEND_DATA,
-					sizeof(cmd), &cmd, 0, NULL, NULL);
+	cmd = malloc(cmd_len);
+	memset(cmd, 0, cmd_len);
+	memcpy(cmd->bdaddr, bd_addr, sizeof(cmd->bdaddr));
+	cmd->len = strlen(data);
+	memcpy(cmd->data, data, cmd->len);
+
+	status = hal_ipc_cmd(HAL_SERVICE_ID_HIDHOST, HAL_OP_HIDHOST_SEND_DATA,
+						cmd_len, cmd, 0, NULL, NULL);
+	free(cmd);
+	return status;
 }
 
 static bt_status_t init(bthh_callbacks_t *callbacks)
-- 
1.8.3.2

Re: [PATCH_v2 06/11] android/hid: Fill send data structure variables in hal-hidhost

[Johan Hedberg]

Hi Ravi,

On Fri, Nov 08, 2013, Ravi kumar Veeramally wrote:
> ---
>  android/hal-hidhost.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)

This doesn't compile:

  CC       android/android_libhal_internal_la-hal-hidhost.lo
android/hal-hidhost.c: In function 'send_data':
android/hal-hidhost.c:356:2: error: implicit declaration of function 'free' [-Werror=implicit-function-declaration]
  free(cmd);
  ^
android/hal-hidhost.c:356:2: error: incompatible implicit declaration of built-in function 'free' [-Werror]
cc1: all warnings being treated as errors
make[1]: *** [android/android_libhal_internal_la-hal-hidhost.lo] Error 1

Johan

Re: [PATCH_v2 06/11] android/hid: Fill send data structure variables in hal-hidhost

[Ravi Kumar Veeramally]

Hi Johan,

On 11/08/2013 11:17 AM, Johan Hedberg wrote:
> Hi Ravi,
>
> On Fri, Nov 08, 2013, Ravi kumar Veeramally wrote:
>> ---
>>   android/hal-hidhost.c | 18 ++++++++++++++----
>>   1 file changed, 14 insertions(+), 4 deletions(-)
> This doesn't compile:
>
>    CC       android/android_libhal_internal_la-hal-hidhost.lo
> android/hal-hidhost.c: In function 'send_data':
> android/hal-hidhost.c:356:2: error: implicit declaration of function 'free' [-Werror=implicit-function-declaration]
>    free(cmd);
>    ^
> android/hal-hidhost.c:356:2: error: incompatible implicit declaration of built-in function 'free' [-Werror]
> cc1: all warnings being treated as errors
> make[1]: *** [android/android_libhal_internal_la-hal-hidhost.lo] Error 1
>
  Ok, I will send _v3 of this and rest.

Thanks,
Ravi.

[PATCH_v2 07/11] android/hid: Rename virtual unplug define and struct

[Ravi kumar Veeramally]

Renaming virtual unplug define and strcut name from VP to more
meaning full.
---
 android/hal-hidhost.c | 5 +++--
 android/hal-msg.h     | 4 ++--
 android/hidhost.c     | 4 ++--
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/android/hal-hidhost.c b/android/hal-hidhost.c
index 1617a16..dbdf176 100644
--- a/android/hal-hidhost.c
+++ b/android/hal-hidhost.c
@@ -141,7 +141,7 @@ static bt_status_t disconnect(bt_bdaddr_t *bd_addr)
 
 static bt_status_t virtual_unplug(bt_bdaddr_t *bd_addr)
 {
-	struct hal_cmd_hidhost_vp cmd;
+	struct hal_cmd_hidhost_virtual_unplug cmd;
 
 	DBG("");
 
@@ -153,7 +153,8 @@ static bt_status_t virtual_unplug(bt_bdaddr_t *bd_addr)
 
 	memcpy(cmd.bdaddr, bd_addr, sizeof(cmd.bdaddr));
 
-	return hal_ipc_cmd(HAL_SERVICE_ID_HIDHOST, HAL_OP_HIDHOST_VP,
+	return hal_ipc_cmd(HAL_SERVICE_ID_HIDHOST,
+					HAL_OP_HIDHOST_VIRTUAL_UNPLUG,
 					sizeof(cmd), &cmd, 0, NULL, NULL);
 }
 
diff --git a/android/hal-msg.h b/android/hal-msg.h
index 05c9de0..3ad9af6 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -242,8 +242,8 @@ struct hal_cmd_hidhost_disconnect {
 	uint8_t bdaddr[6];
 } __attribute__((packed));
 
-#define HAL_OP_HIDHOST_VP			0x03
-struct hal_cmd_hidhost_vp {
+#define HAL_OP_HIDHOST_VIRTUAL_UNPLUG		0x03
+struct hal_cmd_hidhost_virtual_unplug {
 	uint8_t bdaddr[6];
 } __attribute__((packed));
 
diff --git a/android/hidhost.c b/android/hidhost.c
index 4b12318..b7ec6ff 100644
--- a/android/hidhost.c
+++ b/android/hidhost.c
@@ -741,7 +741,7 @@ static uint8_t bt_hid_disconnect(struct hal_cmd_hidhost_disconnect *cmd,
 	return HAL_STATUS_SUCCESS;
 }
 
-static uint8_t bt_hid_virtual_unplug(struct hal_cmd_hidhost_vp *cmd,
+static uint8_t bt_hid_virtual_unplug(struct hal_cmd_hidhost_virtual_unplug *cmd,
 								uint16_t len)
 {
 	DBG("Not Implemented");
@@ -942,7 +942,7 @@ void bt_hid_handle_cmd(int sk, uint8_t opcode, void *buf, uint16_t len)
 	case HAL_OP_HIDHOST_DISCONNECT:
 		status = bt_hid_disconnect(buf, len);
 		break;
-	case HAL_OP_HIDHOST_VP:
+	case HAL_OP_HIDHOST_VIRTUAL_UNPLUG:
 		status = bt_hid_virtual_unplug(buf, len);
 		break;
 	case HAL_OP_HIDHOST_SET_INFO:
-- 
1.8.3.2

Re: [PATCH_v2 07/11] android/hid: Rename virtual unplug define and struct

[Johan Hedberg]

Hi Ravi,

On Fri, Nov 08, 2013, Ravi kumar Veeramally wrote:
> Renaming virtual unplug define and strcut name from VP to more
> meaning full.
> ---
>  android/hal-hidhost.c | 5 +++--
>  android/hal-msg.h     | 4 ++--
>  android/hidhost.c     | 4 ++--
>  3 files changed, 7 insertions(+), 6 deletions(-)

This patch has also been applied. Thanks.

Johan

[PATCH_v2 08/11] android/hid: Add hid event virtual unplug structure to HAL msg headers

[Ravi kumar Veeramally]

---
 android/hal-msg.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/android/hal-msg.h b/android/hal-msg.h
index 3ad9af6..4c7d344 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -479,6 +479,12 @@ struct hal_ev_hidhost_get_report {
 	uint8_t  data[0];
 } __attribute__((packed));
 
+#define HAL_EV_HIDHOST_VIRTUAL_UNPLUG		0x86
+struct hal_ev_hidhost_virtual_unplug {
+	uint8_t  bdaddr[6];
+	uint8_t  status;
+} __attribute__((packed));
+
 #define HAL_EV_A2DP_CONNECTION_STATE	0x81
 struct hal_ev_a2dp_connection_state {
 	uint8_t state;
-- 
1.8.3.2

[PATCH_v2 09/11] android/hid: Handle virtual unplug notification in hid hal

[Ravi kumar Veeramally]

---
 android/hal-hidhost.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/android/hal-hidhost.c b/android/hal-hidhost.c
index dbdf176..db95e3e 100644
--- a/android/hal-hidhost.c
+++ b/android/hal-hidhost.c
@@ -78,6 +78,15 @@ static void handle_get_report(void *buf)
 							ev->data, ev->len);
 }
 
+static void handle_virtual_unplug(void *buf)
+{
+	struct hal_ev_hidhost_virtual_unplug *ev = buf;
+
+	if (cbacks->virtual_unplug_cb)
+		cbacks->virtual_unplug_cb((bt_bdaddr_t *) ev->bdaddr,
+								ev->status);
+}
+
 /* will be called from notification thread context */
 void bt_notify_hidhost(uint16_t opcode, void *buf, uint16_t len)
 {
@@ -97,6 +106,9 @@ void bt_notify_hidhost(uint16_t opcode, void *buf, uint16_t len)
 	case HAL_EV_HIDHOST_GET_REPORT:
 		handle_get_report(buf);
 		break;
+	case HAL_EV_HIDHOST_VIRTUAL_UNPLUG:
+		handle_virtual_unplug(buf);
+		break;
 	default:
 		DBG("Unhandled callback opcode=0x%x", opcode);
 		break;
-- 
1.8.3.2

Re: [PATCH_v2 09/11] android/hid: Handle virtual unplug notification in hid hal

[Johan Hedberg]

Hi Ravi,

On Fri, Nov 08, 2013, Ravi kumar Veeramally wrote:
> ---
>  android/hal-hidhost.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

Patches 1, 4, 5, 8 and 9 have been applied, so no need to resend those
in your next revision.

Johan

[PATCH_v2 10/11] 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 | 41 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/android/hidhost.c b/android/hidhost.c
index b7ec6ff..4fc8963 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,42 @@ 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;
+	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_v2 10/11] android/hid: Add virtual unplug implemention in daemon

[Johan Hedberg]

Hi Ravi,

On Fri, Nov 08, 2013, Ravi kumar Veeramally wrote:
> +	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);

This looks a bit suspicious. First you try to get the fd from ctrl_io
without checking if it's NULL and then later you have an explicit check
for whether it is NULL or not. Is there some check missing earlier in
the function?

Johan

[PATCH_v2 11/11] android/hid: Add send data implemention in daemon

[Ravi kumar Veeramally]

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

diff --git a/android/hidhost.c b/android/hidhost.c
index 4fc8963..945a2dd 100644
--- a/android/hidhost.c
+++ b/android/hidhost.c
@@ -963,9 +963,43 @@ 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 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;
+	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;
+	ascii2hex(cmd->data, cmd->len, (req + 1));
+
+	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