[PATCH 00/11] Implemented hid interfaces in daemon and hal

[PATCH 00/11] Implemented hid interfaces in daemon and hal

[Ravi kumar Veeramally]

This patch set implements get/set protocol and get/set report
interfaces and supported functionality in daemon. Sending notifications
and notification handling in hal. Few naming fixes and error handling.
And last handling of few uHID events.

Ravi kumar Veeramally (11):
  android/hid: Rename function name set_state to notify_state
  android/hid: Retrieve BOOT_DEVICE attribute from SDP and cache it
  android/hid: Implement hid get protocol in daemon
  android/hid: Implement hid set protocol in daemon
  android/hid: Handle protocol mode notification in hal
  android/hid: Implement hid get report in daemon
  android/hid: Implement hid set report in daemon
  android/hid: Handle get report notification in hal
  android/hid: Replace header checking magic number with defines
  android/hid: Handle invalid parameters in hal
  android/hid: Handle uhid events

 android/hal-hidhost.c |  40 +++++-
 android/hal-msg.h     |  18 +++
 android/hid.c         | 353 +++++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 384 insertions(+), 27 deletions(-)

-- 
1.8.1.2

[PATCH 01/11] android/hid: Rename function name set_state to notify_state

[Ravi kumar Veeramally]

Renaming notification preparation function name from bt_hid_set_state
to bt_hid_notify_state. Rest of the funtions name will have proper
notify* names.
---
 android/hid.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/android/hid.c b/android/hid.c
index 1ee8ed4..2498d2d 100644
--- a/android/hid.c
+++ b/android/hid.c
@@ -194,7 +194,7 @@ static gboolean intr_io_watch_cb(GIOChannel *chan, gpointer data)
 	return TRUE;
 }
 
-static void bt_hid_set_state(struct hid_device *dev, uint8_t state)
+static void bt_hid_notify_state(struct hid_device *dev, uint8_t state)
 {
 	struct hal_ev_hid_conn_state ev;
 	char address[18];
@@ -249,7 +249,7 @@ static gboolean ctrl_watch_cb(GIOChannel *chan, GIOCondition cond,
 	char address[18];
 
 	ba2str(&dev->dst, address);
-	bt_hid_set_state(dev, HAL_HID_STATE_DISCONNECTED);
+	bt_hid_notify_state(dev, HAL_HID_STATE_DISCONNECTED);
 
 	/* Checking for intr_watch avoids a double g_io_channel_shutdown since
 	 * it's likely that intr_watch_cb has been queued for dispatching in
@@ -296,6 +296,7 @@ static int uhid_create(struct hid_device *dev)
 	dev->uhid_fd = open(UHID_DEVICE_FILE, O_RDWR | O_CLOEXEC);
 	if (dev->uhid_fd < 0) {
 		error("Failed to open uHID device: %s", strerror(errno));
+		bt_hid_notify_state(dev, HAL_HID_STATE_NO_HID);
 		return -errno;
 	}
 
@@ -344,7 +345,7 @@ static void interrupt_connect_cb(GIOChannel *chan, GError *conn_err,
 				G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
 				intr_watch_cb, dev);
 
-	bt_hid_set_state(dev, HAL_HID_STATE_CONNECTED);
+	bt_hid_notify_state(dev, HAL_HID_STATE_CONNECTED);
 
 	return;
 
@@ -374,7 +375,7 @@ static void control_connect_cb(GIOChannel *chan, GError *conn_err,
 	DBG("");
 
 	if (conn_err) {
-		bt_hid_set_state(dev, HAL_HID_STATE_DISCONNECTED);
+		bt_hid_notify_state(dev, HAL_HID_STATE_DISCONNECTED);
 		error("%s", conn_err->message);
 		goto failed;
 	}
@@ -491,7 +492,7 @@ static void hid_sdp_search_cb(sdp_list_t *recs, int err, gpointer data)
 	return;
 
 fail:
-	bt_hid_set_state(dev, HAL_HID_STATE_DISCONNECTED);
+	bt_hid_notify_state(dev, HAL_HID_STATE_DISCONNECTED);
 	hid_device_free(dev);
 }
 
@@ -531,7 +532,7 @@ static uint8_t bt_hid_connect(struct hal_cmd_hid_connect *cmd, uint16_t len)
 	}
 
 	devices = g_slist_append(devices, dev);
-	bt_hid_set_state(dev, HAL_HID_STATE_CONNECTING);
+	bt_hid_notify_state(dev, HAL_HID_STATE_CONNECTING);
 
 	return HAL_STATUS_SUCCESS;
 }
@@ -563,7 +564,7 @@ static uint8_t bt_hid_disconnect(struct hal_cmd_hid_disconnect *cmd,
 	if (dev->ctrl_io)
 		g_io_channel_shutdown(dev->ctrl_io, TRUE, NULL);
 
-	bt_hid_set_state(dev, HAL_HID_STATE_DISCONNECTING);
+	bt_hid_notify_state(dev, HAL_HID_STATE_DISCONNECTING);
 
 	return HAL_STATUS_SUCCESS;
 }
@@ -715,7 +716,7 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
 		dev->ctrl_watch = g_io_add_watch(dev->ctrl_io,
 					G_IO_HUP | G_IO_ERR | G_IO_NVAL,
 					ctrl_watch_cb, dev);
-		bt_hid_set_state(dev, HAL_HID_STATE_CONNECTING);
+		bt_hid_notify_state(dev, HAL_HID_STATE_CONNECTING);
 		break;
 
 	case L2CAP_PSM_HIDP_INTR:
@@ -728,7 +729,7 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
 		dev->intr_watch = g_io_add_watch(dev->intr_io,
 				G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
 				intr_watch_cb, dev);
-		bt_hid_set_state(dev, HAL_HID_STATE_CONNECTED);
+		bt_hid_notify_state(dev, HAL_HID_STATE_CONNECTED);
 		break;
 	}
 }
-- 
1.8.1.2

[PATCH 02/11] android/hid: Retrieve BOOT_DEVICE attribute from SDP and cache it

[Ravi kumar Veeramally]

It will be usefull to handle when application level requests
get or set protocol from hid device.
---
 android/hid.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/android/hid.c b/android/hid.c
index 2498d2d..c0c9aeb 100644
--- a/android/hid.c
+++ b/android/hid.c
@@ -69,6 +69,7 @@ struct hid_device {
 	uint8_t		country;
 	int		rd_size;
 	void		*rd_data;
+	uint8_t		boot_dev;
 	GIOChannel	*ctrl_io;
 	GIOChannel	*intr_io;
 	guint		ctrl_watch;
@@ -446,6 +447,10 @@ static void hid_sdp_search_cb(sdp_list_t *recs, int err, gpointer data)
 		if (data)
 			dev->subclass = data->val.uint8;
 
+		data = sdp_data_get(rec, SDP_ATTR_HID_BOOT_DEVICE);
+		if (data)
+			dev->boot_dev = data->val.uint8;
+
 		data = sdp_data_get(rec, SDP_ATTR_HID_DESCRIPTOR_LIST);
 		if (data) {
 			if (!SDP_IS_SEQ(data->dtd))
-- 
1.8.1.2

[PATCH 03/11] android/hid: Implement hid get protocol in daemon

[Ravi kumar Veeramally]

This patch requests hid device protocol mode and reads reply
message and sends notification to hal.
---
 android/hal-msg.h |   9 +++++
 android/hid.c     | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 113 insertions(+), 4 deletions(-)

diff --git a/android/hal-msg.h b/android/hal-msg.h
index f381862..214daa9 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -442,6 +442,8 @@ struct hal_ev_hid_conn_state {
 	uint8_t state;
 } __attribute__((packed));
 
+#define HAL_HID_STATUS_OK		0x00
+
 #define HAL_EV_HID_INFO			0x82
 struct hal_ev_hid_info {
 	uint8_t bdaddr[6];
@@ -456,6 +458,13 @@ struct hal_ev_hid_info {
 	uint8_t descr[884];
 } __attribute__((packed));
 
+#define HAL_EV_HID_PROTO_MODE		0x83
+struct hal_ev_hid_protocol {
+	uint8_t bdaddr[6];
+	uint8_t status;
+	uint8_t mode;
+} __attribute__((packed));
+
 #define HAL_EV_AV_CONNECTION_STATE	0x81
 struct hal_ev_av_connection_state {
 	uint8_t state;
diff --git a/android/hid.c b/android/hid.c
index c0c9aeb..9a5477d 100644
--- a/android/hid.c
+++ b/android/hid.c
@@ -54,6 +54,14 @@
 #define L2CAP_PSM_HIDP_INTR	0x13
 #define UHID_DEVICE_FILE	"/dev/uhid"
 
+/* HID message types */
+#define HID_MSG_GET_PROTOCOL	0x60
+#define HID_MSG_DATA		0xa0
+
+/* HID protocol header parameters */
+#define HID_PROTO_BOOT		0x00
+#define HID_PROTO_REPORT	0x01
+
 static GIOChannel *notification_io = NULL;
 static GIOChannel *ctrl_io = NULL;
 static GIOChannel *intr_io = NULL;
@@ -76,6 +84,7 @@ struct hid_device {
 	guint		intr_watch;
 	int		uhid_fd;
 	guint		uhid_watch_id;
+	int		hid_msg;
 };
 
 static int device_cmp(gconstpointer s, gconstpointer user_data)
@@ -243,12 +252,74 @@ static gboolean intr_watch_cb(GIOChannel *chan, GIOCondition cond,
 	return FALSE;
 }
 
+static void bt_hid_notify_protocol_mode(struct hid_device *dev, uint8_t *buf,
+									int len)
+{
+	struct hal_ev_hid_protocol ev;
+	char address[18];
+
+	ba2str(&dev->dst, address);
+	DBG("device %s", address);
+
+	memset(&ev, 0, sizeof(ev));
+	bdaddr2android(&dev->dst, ev.bdaddr);
+
+	if (buf[0] == HID_MSG_DATA) {
+		ev.status = HAL_HID_STATUS_OK;
+		if (buf[1] == HID_PROTO_REPORT)
+			ev.mode = HAL_HID_REPORT_PROTOCOL;
+		else if (buf[1] == HID_PROTO_BOOT)
+			ev.mode = HAL_HID_BOOT_PROTOCOL;
+		else
+			ev.mode = HAL_HID_UNSUPPORTED_PROTOCOL;
+
+	} else {
+		ev.status = buf[0];
+		ev.mode = HAL_HID_UNSUPPORTED_PROTOCOL;
+	}
+
+	ipc_send(notification_io, HAL_SERVICE_ID_HIDHOST,
+				HAL_EV_HID_PROTO_MODE, sizeof(ev), &ev, -1);
+}
+
+static gboolean ctrl_io_watch_cb(GIOChannel *chan, gpointer data)
+{
+	struct hid_device *dev = data;
+	int fd, bread;
+	uint8_t buf[UHID_DATA_MAX];
+
+	DBG("");
+
+	fd = g_io_channel_unix_get_fd(chan);
+	bread = read(fd, buf, sizeof(buf));
+	if (bread < 0) {
+		error("read: %s(%d)", strerror(errno), -errno);
+		return TRUE;
+	}
+
+	switch (dev->hid_msg) {
+	case HID_MSG_GET_PROTOCOL:
+		bt_hid_notify_protocol_mode(dev, buf, bread);
+		break;
+	default:
+		DBG("unhandled hid msg type 0x%02x", dev->hid_msg);
+	}
+
+	/* reset msg type request */
+	dev->hid_msg = -1;
+
+	return TRUE;
+}
+
 static gboolean ctrl_watch_cb(GIOChannel *chan, GIOCondition cond,
 								gpointer data)
 {
 	struct hid_device *dev = data;
 	char address[18];
 
+	if (cond & G_IO_IN)
+		return ctrl_io_watch_cb(chan, data);
+
 	ba2str(&dev->dst, address);
 	bt_hid_notify_state(dev, HAL_HID_STATE_DISCONNECTED);
 
@@ -395,8 +466,8 @@ static void control_connect_cb(GIOChannel *chan, GError *conn_err,
 	}
 
 	dev->ctrl_watch = g_io_add_watch(dev->ctrl_io,
-					G_IO_HUP | G_IO_ERR | G_IO_NVAL,
-					ctrl_watch_cb, dev);
+				G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+				ctrl_watch_cb, dev);
 
 	return;
 
@@ -591,9 +662,38 @@ static uint8_t bt_hid_info(struct hal_cmd_hid_set_info *cmd, uint16_t len)
 static uint8_t bt_hid_get_protocol(struct hal_cmd_hid_get_protocol *cmd,
 								uint16_t len)
 {
-	DBG("Not Implemented");
+	struct hid_device *dev;
+	GSList *l;
+	bdaddr_t dst;
+	int fd;
+	uint8_t hdr[1];
 
-	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->boot_dev)
+		return HAL_STATUS_UNSUPPORTED;
+
+	dev->hid_msg = HID_MSG_GET_PROTOCOL;
+	hdr[0] = HID_MSG_GET_PROTOCOL | cmd->mode;
+	fd = g_io_channel_unix_get_fd(dev->ctrl_io);
+
+	if (write(fd, hdr, sizeof(hdr)) < 0) {
+		error("error while querying device protocol");
+		return HAL_STATUS_FAILED;
+	}
+
+	return HAL_STATUS_SUCCESS;
 }
 
 static uint8_t bt_hid_set_protocol(struct hal_cmd_hid_set_protocol *cmd,
-- 
1.8.1.2

Re: [PATCH 03/11] android/hid: Implement hid get protocol in daemon

[Johan Hedberg]

Hi Ravi,

On Tue, Nov 05, 2013, Ravi kumar Veeramally wrote:
> This patch requests hid device protocol mode and reads reply
> message and sends notification to hal.
> ---
>  android/hal-msg.h |   9 +++++
>  android/hid.c     | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 113 insertions(+), 4 deletions(-)

I've applied patches 1 and 2, but from this one onwards there were
sevaral issues.

> diff --git a/android/hal-msg.h b/android/hal-msg.h
> index f381862..214daa9 100644
> --- a/android/hal-msg.h
> +++ b/android/hal-msg.h
> @@ -442,6 +442,8 @@ struct hal_ev_hid_conn_state {
>  	uint8_t state;
>  } __attribute__((packed));
>  
> +#define HAL_HID_STATUS_OK		0x00
> +
>  #define HAL_EV_HID_INFO			0x82
>  struct hal_ev_hid_info {
>  	uint8_t bdaddr[6];
> @@ -456,6 +458,13 @@ struct hal_ev_hid_info {
>  	uint8_t descr[884];
>  } __attribute__((packed));
>  
> +#define HAL_EV_HID_PROTO_MODE		0x83
> +struct hal_ev_hid_protocol {
> +	uint8_t bdaddr[6];
> +	uint8_t status;
> +	uint8_t mode;
> +} __attribute__((packed));

To be consistent, shouldn't the struct be called hal_ev_hid_proto_mode?

Also, I'd split this hal-msg.h update into a separate patch. Am I right
to assume hal-ipc-api.txt doesn't need updating for this one?

> @@ -76,6 +84,7 @@ struct hid_device {
>  	guint		intr_watch;
>  	int		uhid_fd;
>  	guint		uhid_watch_id;
> +	int		hid_msg;
>  };
>  
>  static int device_cmp(gconstpointer s, gconstpointer user_data)
> @@ -243,12 +252,74 @@ static gboolean intr_watch_cb(GIOChannel *chan, GIOCondition cond,
>  	return FALSE;
>  }
>  
> +static void bt_hid_notify_protocol_mode(struct hid_device *dev, uint8_t *buf,
> +									int len)

To be consistent with the struct names you might use "proto" instead of
"protocol" here.

> +static gboolean ctrl_io_watch_cb(GIOChannel *chan, gpointer data)
> +{
> +	struct hid_device *dev = data;
> +	int fd, bread;
> +	uint8_t buf[UHID_DATA_MAX];
> +
> +	DBG("");
> +
> +	fd = g_io_channel_unix_get_fd(chan);
> +	bread = read(fd, buf, sizeof(buf));
> +	if (bread < 0) {
> +		error("read: %s(%d)", strerror(errno), -errno);
> +		return TRUE;
> +	}
> +
> +	switch (dev->hid_msg) {
> +	case HID_MSG_GET_PROTOCOL:
> +		bt_hid_notify_protocol_mode(dev, buf, bread);
> +		break;
> +	default:
> +		DBG("unhandled hid msg type 0x%02x", dev->hid_msg);
> +	}
> +
> +	/* reset msg type request */
> +	dev->hid_msg = -1;

Do we really need to do this tracking of what we wrote to the socket.
Isn't it possible to infer the type of procedure from the content of the
data that we read from the socket?

Johan

[PATCH 04/11] android/hid: Implement hid set protocol in daemon

[Ravi kumar Veeramally]

This patch requests hid device to set protocol mode and reads
reply message and sends notification to hal.
---
 android/hid.c | 35 +++++++++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/android/hid.c b/android/hid.c
index 9a5477d..115a1c5 100644
--- a/android/hid.c
+++ b/android/hid.c
@@ -56,6 +56,7 @@
 
 /* HID message types */
 #define HID_MSG_GET_PROTOCOL	0x60
+#define HID_MSG_SET_PROTOCOL	0x70
 #define HID_MSG_DATA		0xa0
 
 /* HID protocol header parameters */
@@ -299,6 +300,7 @@ static gboolean ctrl_io_watch_cb(GIOChannel *chan, gpointer data)
 
 	switch (dev->hid_msg) {
 	case HID_MSG_GET_PROTOCOL:
+	case HID_MSG_SET_PROTOCOL:
 		bt_hid_notify_protocol_mode(dev, buf, bread);
 		break;
 	default:
@@ -699,9 +701,38 @@ static uint8_t bt_hid_get_protocol(struct hal_cmd_hid_get_protocol *cmd,
 static uint8_t bt_hid_set_protocol(struct hal_cmd_hid_set_protocol *cmd,
 								uint16_t len)
 {
-	DBG("Not Implemented");
+	struct hid_device *dev;
+	GSList *l;
+	bdaddr_t dst;
+	int fd;
+	uint8_t hdr[1];
 
-	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->boot_dev)
+		return HAL_STATUS_UNSUPPORTED;
+
+	dev->hid_msg = HID_MSG_SET_PROTOCOL;
+	hdr[0] = HID_MSG_SET_PROTOCOL | cmd->mode;
+	fd = g_io_channel_unix_get_fd(dev->ctrl_io);
+
+	if (write(fd, hdr, sizeof(hdr)) < 0) {
+		error("error while setting device protocol");
+		return HAL_STATUS_FAILED;
+	}
+
+	return HAL_STATUS_SUCCESS;
 }
 
 static uint8_t bt_hid_get_report(struct hal_cmd_hid_get_report *cmd,
-- 
1.8.1.2

[PATCH 05/11] android/hid: Handle protocol mode notification in 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 c20c785..a287e40 100644
--- a/android/hal-hidhost.c
+++ b/android/hal-hidhost.c
@@ -59,6 +59,15 @@ static void handle_info(void *buf)
 		bt_hh_cbacks->hid_info_cb((bt_bdaddr_t *) ev->bdaddr, info);
 }
 
+static void handle_protocol_mode(void *buf)
+{
+	struct hal_ev_hid_protocol *ev = buf;
+
+	if (bt_hh_cbacks->protocol_mode_cb)
+		bt_hh_cbacks->protocol_mode_cb((bt_bdaddr_t *) ev->bdaddr,
+							ev->status, ev->mode);
+}
+
 /* will be called from notification thread context */
 void bt_notify_hh(uint16_t opcode, void *buf, uint16_t len)
 {
@@ -72,6 +81,9 @@ void bt_notify_hh(uint16_t opcode, void *buf, uint16_t len)
 	case HAL_EV_HID_INFO:
 		handle_info(buf);
 		break;
+	case HAL_EV_HID_PROTO_MODE:
+		handle_protocol_mode(buf);
+		break;
 	default:
 		DBG("Unhandled callback opcode=0x%x", opcode);
 		break;
-- 
1.8.1.2

[PATCH 06/11] android/hid: Implement hid get report in daemon

[Ravi kumar Veeramally]

This patch requests hid device report and reads reply
message and sends notification to hal.
---
 android/hal-hidhost.c |   1 +
 android/hal-msg.h     |   8 ++++
 android/hid.c         | 104 +++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 111 insertions(+), 2 deletions(-)

diff --git a/android/hal-hidhost.c b/android/hal-hidhost.c
index a287e40..027e3b8 100644
--- a/android/hal-hidhost.c
+++ b/android/hal-hidhost.c
@@ -251,6 +251,7 @@ static bt_status_t hh_get_report(bt_bdaddr_t *bd_addr,
 
 	memcpy(cmd.bdaddr, bd_addr, sizeof(cmd.bdaddr));
 	cmd.id = reportId;
+	cmd.buf = bufferSize;
 
 	switch (reportType) {
 	case BTHH_INPUT_REPORT:
diff --git a/android/hal-msg.h b/android/hal-msg.h
index 214daa9..eaa5c9c 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -465,6 +465,14 @@ struct hal_ev_hid_protocol {
 	uint8_t mode;
 } __attribute__((packed));
 
+#define HAL_EV_HID_GET_REPORT		0x84
+struct hal_ev_hid_get_report {
+	uint8_t bdaddr[6];
+	uint8_t status;
+	uint16_t len;
+	uint8_t data[0];
+} __attribute__((packed));
+
 #define HAL_EV_AV_CONNECTION_STATE	0x81
 struct hal_ev_av_connection_state {
 	uint8_t state;
diff --git a/android/hid.c b/android/hid.c
index 115a1c5..6cb9604 100644
--- a/android/hid.c
+++ b/android/hid.c
@@ -55,14 +55,23 @@
 #define UHID_DEVICE_FILE	"/dev/uhid"
 
 /* HID message types */
+#define HID_MSG_GET_REPORT	0x40
 #define HID_MSG_GET_PROTOCOL	0x60
 #define HID_MSG_SET_PROTOCOL	0x70
 #define HID_MSG_DATA		0xa0
 
+/* HID data types */
+#define HID_DATA_TYPE_INPUT	0x01
+#define HID_DATA_TYPE_OUTPUT	0x02
+#define HID_DATA_TYPE_FEATURE	0x03
+
 /* HID protocol header parameters */
 #define HID_PROTO_BOOT		0x00
 #define HID_PROTO_REPORT	0x01
 
+/* HID GET REPORT Size Field */
+#define HID_GET_REPORT_SIZE_FIELD	0x08
+
 static GIOChannel *notification_io = NULL;
 static GIOChannel *ctrl_io = NULL;
 static GIOChannel *intr_io = NULL;
@@ -283,6 +292,52 @@ static void bt_hid_notify_protocol_mode(struct hid_device *dev, uint8_t *buf,
 				HAL_EV_HID_PROTO_MODE, sizeof(ev), &ev, -1);
 }
 
+static void bt_hid_notify_get_report(struct hid_device *dev, uint8_t *buf,
+									int len)
+{
+	struct hal_ev_hid_get_report *ev;
+	int ev_len;
+	char address[18];
+
+	ba2str(&dev->dst, address);
+	DBG("device %s", address);
+
+	ev_len = sizeof(*ev) + sizeof(struct hal_ev_hid_get_report) + 1;
+
+	if (!((buf[0] == (HID_MSG_DATA | HID_DATA_TYPE_INPUT)) ||
+			(buf[0] == (HID_MSG_DATA | HID_DATA_TYPE_OUTPUT)) ||
+			(buf[0]	== (HID_MSG_DATA | HID_DATA_TYPE_FEATURE)))) {
+		ev = g_malloc(len);
+		memset(ev, 0, ev_len);
+		ev->status = buf[0];
+		bdaddr2android(&dev->dst, ev->bdaddr);
+		goto send;
+	}
+
+	/* Report porotocol mode reply contains id after hdr, in boot
+	 * protocol mode id doesn't exist */
+	ev_len += (dev->boot_dev) ? (len - 1) : (len - 2);
+	ev = g_malloc(ev_len);
+	memset(ev, 0, ev_len);
+	ev->status = HAL_HID_STATUS_OK;
+	bdaddr2android(&dev->dst, ev->bdaddr);
+
+	/* Report porotocol mode reply contains id after hdr, in boot
+	 * protocol mode id doesn't exist */
+	if (dev->boot_dev) {
+		ev->len = len - 1;
+		memcpy(ev->data, buf + 1, ev->len);
+	} else {
+		ev->len = len - 2;
+		memcpy(ev->data, buf + 2, ev->len);
+	}
+
+send:
+	ipc_send(notification_io, HAL_SERVICE_ID_HIDHOST, HAL_EV_HID_GET_REPORT,
+						ev_len, ev, -1);
+	g_free(ev);
+}
+
 static gboolean ctrl_io_watch_cb(GIOChannel *chan, gpointer data)
 {
 	struct hid_device *dev = data;
@@ -303,6 +358,9 @@ static gboolean ctrl_io_watch_cb(GIOChannel *chan, gpointer data)
 	case HID_MSG_SET_PROTOCOL:
 		bt_hid_notify_protocol_mode(dev, buf, bread);
 		break;
+	case HID_MSG_GET_REPORT:
+		bt_hid_notify_get_report(dev, buf, bread);
+		break;
 	default:
 		DBG("unhandled hid msg type 0x%02x", dev->hid_msg);
 	}
@@ -738,9 +796,51 @@ static uint8_t bt_hid_set_protocol(struct hal_cmd_hid_set_protocol *cmd,
 static uint8_t bt_hid_get_report(struct hal_cmd_hid_get_report *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;
+	dev->hid_msg = HID_MSG_GET_REPORT;
+	req_size = (cmd->buf > 0) ? 4 : 2;
+	req = g_try_malloc0(req_size);
+	if (!req)
+		return HAL_STATUS_NOMEM;
+
+	req[0] = HID_MSG_GET_REPORT | cmd->type;
+
+	if (cmd->buf > 0)
+		req[0] = req[0] | HID_GET_REPORT_SIZE_FIELD;
+
+	req[1] = cmd->id;
+
+	if (cmd->buf > 0)
+		bt_put_le16(cmd->buf, (req + 2));
+
+	fd = g_io_channel_unix_get_fd(dev->ctrl_io);
+
+	if (write(fd, req, req_size) < 0) {
+		error("error while querying device protocol");
+		g_free(req);
+		return HAL_STATUS_FAILED;
+	}
+
+	g_free(req);
+	return HAL_STATUS_SUCCESS;
 }
 
 static uint8_t bt_hid_set_report(struct hal_cmd_hid_set_report *cmd,
-- 
1.8.1.2

Re: [PATCH 06/11] android/hid: Implement hid get report in daemon

[Johan Hedberg]

Hi Ravi,

On Tue, Nov 05, 2013, Ravi kumar Veeramally wrote:
> This patch requests hid device report and reads reply
> message and sends notification to hal.
> ---
>  android/hal-hidhost.c |   1 +
>  android/hal-msg.h     |   8 ++++
>  android/hid.c         | 104 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 111 insertions(+), 2 deletions(-)

Again, I'd split this into separate patches. One to update the
hal-msg.h, one for the daemon and another for the HAL.

> +#define HAL_EV_HID_GET_REPORT		0x84
> +struct hal_ev_hid_get_report {
> +	uint8_t bdaddr[6];
> +	uint8_t status;
> +	uint16_t len;
> +	uint8_t data[0];
> +} __attribute__((packed));

In our hal-ipc.api.txt this event is defined as 0x85. Not 0x84. Is the
code or documentation wrong?

Also, please follow the same alignment for the variables in the struct
as elsewhere in hal-msg.h.

Johan

[PATCH 07/11] android/hid: Implement hid set report in daemon

[Ravi kumar Veeramally]

This patch requests hid device to set report.
---
 android/hal-hidhost.c |  1 +
 android/hal-msg.h     |  1 +
 android/hid.c         | 40 ++++++++++++++++++++++++++++++++++++++--
 3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/android/hal-hidhost.c b/android/hal-hidhost.c
index 027e3b8..040d517 100644
--- a/android/hal-hidhost.c
+++ b/android/hal-hidhost.c
@@ -284,6 +284,7 @@ static bt_status_t hh_set_report(bt_bdaddr_t *bd_addr,
 		return BT_STATUS_PARM_INVALID;
 
 	memcpy(cmd.bdaddr, bd_addr, sizeof(cmd.bdaddr));
+	cmd.report = report;
 
 	switch (reportType) {
 	case BTHH_INPUT_REPORT:
diff --git a/android/hal-msg.h b/android/hal-msg.h
index eaa5c9c..e1fd027 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -292,6 +292,7 @@ struct hal_cmd_hid_get_report {
 struct hal_cmd_hid_set_report {
 	uint8_t bdaddr[6];
 	uint8_t type;
+	char *report;
 } __attribute__((packed));
 
 #define HAL_OP_HID_SEND_DATA		0x09
diff --git a/android/hid.c b/android/hid.c
index 6cb9604..2682b21e6 100644
--- a/android/hid.c
+++ b/android/hid.c
@@ -56,6 +56,7 @@
 
 /* HID message types */
 #define HID_MSG_GET_REPORT	0x40
+#define HID_MSG_SET_REPORT	0x50
 #define HID_MSG_GET_PROTOCOL	0x60
 #define HID_MSG_SET_PROTOCOL	0x70
 #define HID_MSG_DATA		0xa0
@@ -846,9 +847,44 @@ static uint8_t bt_hid_get_report(struct hal_cmd_hid_get_report *cmd,
 static uint8_t bt_hid_set_report(struct hal_cmd_hid_set_report *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;
+	dev->hid_msg = HID_MSG_SET_REPORT;
+	req_size = 1 + strlen(cmd->report);
+	req = g_try_malloc0(req_size);
+	if (!req)
+		return HAL_STATUS_NOMEM;
+
+	req[0] = HID_MSG_SET_REPORT | cmd->type;
+	memcpy(req + 1, cmd->report, req_size - 1);
+
+	fd = g_io_channel_unix_get_fd(dev->ctrl_io);
+
+	if (write(fd, req, req_size) < 0) {
+		error("error while querying device protocol");
+		g_free(req);
+		return HAL_STATUS_FAILED;
+	}
+
+	g_free(req);
+	return HAL_STATUS_SUCCESS;
 }
 
 static uint8_t bt_hid_send_data(struct hal_cmd_hid_send_data *cmd,
-- 
1.8.1.2

Re: [PATCH 07/11] android/hid: Implement hid set report in daemon

[Johan Hedberg]

Hi Ravi,

On Tue, Nov 05, 2013, Ravi kumar Veeramally wrote:
> This patch requests hid device to set report.
> ---
>  android/hal-hidhost.c |  1 +
>  android/hal-msg.h     |  1 +
>  android/hid.c         | 40 ++++++++++++++++++++++++++++++++++++++--
>  3 files changed, 40 insertions(+), 2 deletions(-)

Again, split this into separate patches.

> diff --git a/android/hal-hidhost.c b/android/hal-hidhost.c
> index 027e3b8..040d517 100644
> --- a/android/hal-hidhost.c
> +++ b/android/hal-hidhost.c
> @@ -284,6 +284,7 @@ static bt_status_t hh_set_report(bt_bdaddr_t *bd_addr,
>  		return BT_STATUS_PARM_INVALID;
>  
>  	memcpy(cmd.bdaddr, bd_addr, sizeof(cmd.bdaddr));
> +	cmd.report = report;
>  
>  	switch (reportType) {
>  	case BTHH_INPUT_REPORT:
> diff --git a/android/hal-msg.h b/android/hal-msg.h
> index eaa5c9c..e1fd027 100644
> --- a/android/hal-msg.h
> +++ b/android/hal-msg.h
> @@ -292,6 +292,7 @@ struct hal_cmd_hid_get_report {
>  struct hal_cmd_hid_set_report {
>  	uint8_t bdaddr[6];
>  	uint8_t type;
> +	char *report;
>  } __attribute__((packed));

Seriously? Trying to send pointers over IPC :)

Also verify that whatever you add matches what we have in
hal-ipc-api.txt (and if the API documentation is wrong or missing
something fix it).

Johan

[PATCH 08/11] android/hid: Handle get report notification in 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 040d517..45767b8 100644
--- a/android/hal-hidhost.c
+++ b/android/hal-hidhost.c
@@ -68,6 +68,15 @@ static void handle_protocol_mode(void *buf)
 							ev->status, ev->mode);
 }
 
+static void handle_get_report(void *buf)
+{
+	struct hal_ev_hid_get_report *ev = buf;
+
+	if (bt_hh_cbacks->get_report_cb)
+		bt_hh_cbacks->get_report_cb((bt_bdaddr_t *) ev->bdaddr,
+						ev->status, ev->data, ev->len);
+}
+
 /* will be called from notification thread context */
 void bt_notify_hh(uint16_t opcode, void *buf, uint16_t len)
 {
@@ -84,6 +93,9 @@ void bt_notify_hh(uint16_t opcode, void *buf, uint16_t len)
 	case HAL_EV_HID_PROTO_MODE:
 		handle_protocol_mode(buf);
 		break;
+	case HAL_EV_HID_GET_REPORT:
+		handle_get_report(buf);
+		break;
 	default:
 		DBG("Unhandled callback opcode=0x%x", opcode);
 		break;
-- 
1.8.1.2

[PATCH 09/11] android/hid: Replace header checking magic number with defines

[Ravi kumar Veeramally]

---
 android/hid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/android/hid.c b/android/hid.c
index 2682b21e6..74de02f 100644
--- a/android/hid.c
+++ b/android/hid.c
@@ -200,7 +200,7 @@ static gboolean intr_io_watch_cb(GIOChannel *chan, gpointer data)
 	}
 
 	/* Discard non-data packets */
-	if (bread == 0 || buf[0] != 0xA1)
+	if (bread == 0 || buf[0] != (HID_MSG_DATA | HID_DATA_TYPE_INPUT))
 		return TRUE;
 
 	/* send data to uHID device skipping HIDP header byte */
-- 
1.8.1.2

[PATCH 10/11] android/hid: Handle invalid parameters in hal

[Ravi kumar Veeramally]

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

diff --git a/android/hal-hidhost.c b/android/hal-hidhost.c
index 45767b8..412258c 100644
--- a/android/hal-hidhost.c
+++ b/android/hal-hidhost.c
@@ -204,9 +204,8 @@ static bt_status_t hh_get_protocol(bt_bdaddr_t *bd_addr,
 	case BTHH_BOOT_MODE:
 		cmd.mode = HAL_HID_BOOT_PROTOCOL;
 		break;
-	case BTHH_UNSUPPORTED_MODE:
-		cmd.mode = HAL_HID_UNSUPPORTED_PROTOCOL;
-		break;
+	default:
+		return BT_STATUS_PARM_INVALID;
 	}
 
 	return hal_ipc_cmd(HAL_SERVICE_ID_HIDHOST,
@@ -236,9 +235,8 @@ static bt_status_t hh_set_protocol(bt_bdaddr_t *bd_addr,
 	case BTHH_BOOT_MODE:
 		cmd.mode = HAL_HID_BOOT_PROTOCOL;
 		break;
-	case BTHH_UNSUPPORTED_MODE:
-		cmd.mode = HAL_HID_UNSUPPORTED_PROTOCOL;
-		break;
+	default:
+		return BT_STATUS_PARM_INVALID;
 	}
 
 	return hal_ipc_cmd(HAL_SERVICE_ID_HIDHOST,
@@ -275,6 +273,8 @@ static bt_status_t hh_get_report(bt_bdaddr_t *bd_addr,
 	case BTHH_FEATURE_REPORT:
 		cmd.type = HAL_HID_FEATURE_REPORT;
 		break;
+	default:
+		return BT_STATUS_PARM_INVALID;
 	}
 
 	return hal_ipc_cmd(HAL_SERVICE_ID_HIDHOST, HAL_OP_HID_GET_REPORT,
@@ -308,6 +308,8 @@ static bt_status_t hh_set_report(bt_bdaddr_t *bd_addr,
 	case BTHH_FEATURE_REPORT:
 		cmd.type = HAL_HID_FEATURE_REPORT;
 		break;
+	default:
+		return BT_STATUS_PARM_INVALID;
 	}
 
 	return hal_ipc_cmd(HAL_SERVICE_ID_HIDHOST, HAL_OP_HID_SET_REPORT,
-- 
1.8.1.2

[PATCH 11/11] android/hid: Handle uhid events

[Ravi kumar Veeramally]

Handling few uhid events and described scenarios. OUTPUT and
FEATURE events are not yet handled.
---
 android/hid.c | 40 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/android/hid.c b/android/hid.c
index 74de02f..fccaa87 100644
--- a/android/hid.c
+++ b/android/hid.c
@@ -149,6 +149,11 @@ static void hid_device_free(struct hid_device *dev)
 	g_free(dev);
 }
 
+static void handle_uhid_event(struct hid_device *dev, struct uhid_event *ev)
+{
+	DBG("unsupported event");
+}
+
 static gboolean uhid_event_cb(GIOChannel *io, GIOCondition cond,
 							gpointer user_data)
 {
@@ -172,7 +177,40 @@ static gboolean uhid_event_cb(GIOChannel *io, GIOCondition cond,
 	}
 
 	DBG("uHID event type %d received", ev.type);
-	/* TODO Handle events */
+
+	switch (ev.type) {
+	case UHID_START:
+	case UHID_STOP:
+		/* These are called to start and stop the underlying hardware.
+		 * We open the channels before creating the device so the
+		 * hardware is always ready. No need to handle these.
+		 * The kernel never destroys a device itself! Only an explicit
+		 * UHID_DESTROY request can remove a device. */
+
+		break;
+	case UHID_OPEN:
+	case UHID_CLOSE:
+		/* OPEN/CLOSE are sent whenever user-space opens any interface
+		 * provided by the kernel HID device. Whenever the open-count
+		 * is non-zero we must be ready for I/O. As long as it is zero,
+		 * we can decide to drop all I/O and put the device
+		 * asleep This is optional, though. */
+		break;
+	case UHID_OUTPUT:
+	case UHID_FEATURE:
+		handle_uhid_event(dev, &ev);
+		break;
+	case UHID_OUTPUT_EV:
+		/* This is only sent by kernels prior to linux-3.11. It
+		 * requires us to parse HID-descriptors in user-space to
+		 * properly handle it. This is redundant as the kernel
+		 * does it already. That's why newer kernels assemble
+		 * the output-reports and send it to us via UHID_OUTPUT. */
+		DBG("Unsupported uHID event: type %d", ev.u.output_ev.type);
+		break;
+	default:
+		warn("unexpected uHID event");
+	}
 
 	return TRUE;
 
-- 
1.8.1.2

Re: [PATCH 11/11] android/hid: Handle uhid events

[Andrei Emeltchenko]

Hi Ravi,

On Tue, Nov 05, 2013 at 12:20:15AM +0200, Ravi kumar Veeramally wrote:
> Handling few uhid events and described scenarios. OUTPUT and
> FEATURE events are not yet handled.
> ---
>  android/hid.c | 40 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/android/hid.c b/android/hid.c
> index 74de02f..fccaa87 100644
> --- a/android/hid.c
> +++ b/android/hid.c
> @@ -149,6 +149,11 @@ static void hid_device_free(struct hid_device *dev)
>  	g_free(dev);
>  }
>  
> +static void handle_uhid_event(struct hid_device *dev, struct uhid_event *ev)
> +{
> +	DBG("unsupported event");
> +}
> +
>  static gboolean uhid_event_cb(GIOChannel *io, GIOCondition cond,
>  							gpointer user_data)
>  {
> @@ -172,7 +177,40 @@ static gboolean uhid_event_cb(GIOChannel *io, GIOCondition cond,
>  	}
>  
>  	DBG("uHID event type %d received", ev.type);
> -	/* TODO Handle events */
> +
> +	switch (ev.type) {
> +	case UHID_START:
> +	case UHID_STOP:
> +		/* These are called to start and stop the underlying hardware.
> +		 * We open the channels before creating the device so the
> +		 * hardware is always ready. No need to handle these.
> +		 * The kernel never destroys a device itself! Only an explicit
> +		 * UHID_DESTROY request can remove a device. */
> +
> +		break;
> +	case UHID_OPEN:
> +	case UHID_CLOSE:
> +		/* OPEN/CLOSE are sent whenever user-space opens any interface
> +		 * provided by the kernel HID device. Whenever the open-count
> +		 * is non-zero we must be ready for I/O. As long as it is zero,
> +		 * we can decide to drop all I/O and put the device
> +		 * asleep This is optional, though. */
> +		break;
> +	case UHID_OUTPUT:
> +	case UHID_FEATURE:
> +		handle_uhid_event(dev, &ev);
> +		break;
> +	case UHID_OUTPUT_EV:
> +		/* This is only sent by kernels prior to linux-3.11. It
> +		 * requires us to parse HID-descriptors in user-space to
> +		 * properly handle it. This is redundant as the kernel
> +		 * does it already. That's why newer kernels assemble
> +		 * the output-reports and send it to us via UHID_OUTPUT. */
> +		DBG("Unsupported uHID event: type %d", ev.u.output_ev.type);

Use handle_uhid_event helper? You could add printing event type there.

Best regards 
Andrei Emeltchenko 

> +		break;
> +	default:
> +		warn("unexpected uHID event");
> +	}
>  
>  	return TRUE;
>  
> -- 
> 1.8.1.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