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