[PATCH 0/4] Add GET/SET_REPORT support to HoG

[PATCH 0/4] Add GET/SET_REPORT support to HoG

[David Herrmann]

Hi

With linux-3.18 uhid will gain GET/SET_REPORT support. 3.18 isn't released, yet,
but the uhid features are part of 3.18-rc5 so I think it's fine to rely on it.
We can always revert it if it will not be released (for whatever reason..).

Benjamin wrote these 4 patches about 2 months ago and I promised to fix a few
bugs and send them upstream. I already feel bad for letting them rot for 2
months.. sorry!

Anyway, I fixed the ID tracking, some typos, and fixed handling of devices that
dont send report IDs in GET_REPORT. Patches should be straightforward. The
GET/SET_REPORT feature is important for nearly all non-standard HID device
drivers. It's used to query and modify device behavior. With this in place, we
should be able to use all the kernel hid drivers via uhid/hog.

Benjamin has an example program to modify MS Arc Surface Touch mouse behavior,
which needs these patches to query/modify the device. See:
    https://github.com/bentiss/mstouchmouse/tree/WIP-USB
(If you check it out, make sure to use the WIP-USB branch! You should also apply
 the diff below, to make sure it works even if the report-ID is not prepended.)

Works fine with my devices here!
David


diff --git a/set_touchmode.c b/set_touchmode.c
index 7e5d59a..0e6f412 100644
--- a/set_touchmode.c
+++ b/set_touchmode.c
@@ -48,6 +48,11 @@ static int get_report(int fd, unsigned char id, ...
     } else {
         printf("Report data (%d):\n\t", id);
         pr_buffer(buf, res);
+        /* prepend report-ID if not sent by device */
+        if (res > 0 && buf[0] != id) {
+            memmove(buf + 1, buf, (res >= len) ? (res - 1) : res);
+            buf[0] = id;
+        }
     }
     return res;
 }

Benjamin Tissoires (4):
  hog: import latest uhid.h definition
  hog: break out the report retrieval in its own function
  hog: implement get_report functionality
  hog: implement set_report functionality

 profiles/input/hog.c       | 204 +++++++++++++++++++++++++++++++++++++++++----
 profiles/input/uhid_copy.h | 117 +++++++++++++++++++++++---
 2 files changed, 292 insertions(+), 29 deletions(-)

-- 
2.1.3

[PATCH 1/4] hog: import latest uhid.h definition

[David Herrmann]

From: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Kernel 3.18 ships a new API for uhid devices. This API is retro-compatible
so we can keep the current symbols.
---
 profiles/input/uhid_copy.h | 117 ++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 105 insertions(+), 12 deletions(-)

diff --git a/profiles/input/uhid_copy.h b/profiles/input/uhid_copy.h
index 23a6287..0ef73d4 100644
--- a/profiles/input/uhid_copy.h
+++ b/profiles/input/uhid_copy.h
@@ -21,35 +21,49 @@
 
 #include <linux/input.h>
 #include <linux/types.h>
+#include <linux/hid.h>
 
 enum uhid_event_type {
-	UHID_CREATE,
+	__UHID_LEGACY_CREATE,
 	UHID_DESTROY,
 	UHID_START,
 	UHID_STOP,
 	UHID_OPEN,
 	UHID_CLOSE,
 	UHID_OUTPUT,
-	UHID_OUTPUT_EV,			/* obsolete! */
-	UHID_INPUT,
-	UHID_FEATURE,
-	UHID_FEATURE_ANSWER,
+	__UHID_LEGACY_OUTPUT_EV,
+	__UHID_LEGACY_INPUT,
+	UHID_GET_REPORT,
+	UHID_GET_REPORT_REPLY,
+	UHID_CREATE2,
+	UHID_INPUT2,
+	UHID_SET_REPORT,
+	UHID_SET_REPORT_REPLY,
 };
 
-struct uhid_create_req {
+struct uhid_create2_req {
 	__u8 name[128];
 	__u8 phys[64];
 	__u8 uniq[64];
-	__u8 *rd_data;
 	__u16 rd_size;
-
 	__u16 bus;
 	__u32 vendor;
 	__u32 product;
 	__u32 version;
 	__u32 country;
+	__u8 rd_data[HID_MAX_DESCRIPTOR_SIZE];
 } __attribute__((__packed__));
 
+enum uhid_dev_flag {
+	UHID_DEV_NUMBERED_FEATURE_REPORTS			= (1ULL << 0),
+	UHID_DEV_NUMBERED_OUTPUT_REPORTS			= (1ULL << 1),
+	UHID_DEV_NUMBERED_INPUT_REPORTS				= (1ULL << 2),
+};
+
+struct uhid_start_req {
+	__u64 dev_flags;
+};
+
 #define UHID_DATA_MAX 4096
 
 enum uhid_report_type {
@@ -58,9 +72,9 @@ enum uhid_report_type {
 	UHID_INPUT_REPORT,
 };
 
-struct uhid_input_req {
-	__u8 data[UHID_DATA_MAX];
+struct uhid_input2_req {
 	__u16 size;
+	__u8 data[UHID_DATA_MAX];
 } __attribute__((__packed__));
 
 struct uhid_output_req {
@@ -69,20 +83,83 @@ struct uhid_output_req {
 	__u8 rtype;
 } __attribute__((__packed__));
 
-/* Obsolete! Newer kernels will no longer send these events but instead convert
- * it into raw output reports via UHID_OUTPUT. */
+struct uhid_get_report_req {
+	__u32 id;
+	__u8 rnum;
+	__u8 rtype;
+} __attribute__((__packed__));
+
+struct uhid_get_report_reply_req {
+	__u32 id;
+	__u16 err;
+	__u16 size;
+	__u8 data[UHID_DATA_MAX];
+} __attribute__((__packed__));
+
+struct uhid_set_report_req {
+	__u32 id;
+	__u8 rnum;
+	__u8 rtype;
+	__u16 size;
+	__u8 data[UHID_DATA_MAX];
+} __attribute__((__packed__));
+
+struct uhid_set_report_reply_req {
+	__u32 id;
+	__u16 err;
+} __attribute__((__packed__));
+
+/*
+ * Compat Layer
+ * All these commands and requests are obsolete. You should avoid using them in
+ * new code. We support them for backwards-compatibility, but you might not get
+ * access to new feature in case you use them.
+ */
+
+enum uhid_legacy_event_type {
+	UHID_CREATE			= __UHID_LEGACY_CREATE,
+	UHID_OUTPUT_EV			= __UHID_LEGACY_OUTPUT_EV,
+	UHID_INPUT			= __UHID_LEGACY_INPUT,
+	UHID_FEATURE			= UHID_GET_REPORT,
+	UHID_FEATURE_ANSWER		= UHID_GET_REPORT_REPLY,
+};
+
+/* Obsolete! Use UHID_CREATE2. */
+struct uhid_create_req {
+	__u8 name[128];
+	__u8 phys[64];
+	__u8 uniq[64];
+	__u8 *rd_data;
+	__u16 rd_size;
+
+	__u16 bus;
+	__u32 vendor;
+	__u32 product;
+	__u32 version;
+	__u32 country;
+} __attribute__((__packed__));
+
+/* Obsolete! Use UHID_INPUT2. */
+struct uhid_input_req {
+	__u8 data[UHID_DATA_MAX];
+	__u16 size;
+} __attribute__((__packed__));
+
+/* Obsolete! Kernel uses UHID_OUTPUT exclusively now. */
 struct uhid_output_ev_req {
 	__u16 type;
 	__u16 code;
 	__s32 value;
 } __attribute__((__packed__));
 
+/* Obsolete! Kernel uses ABI compatible UHID_GET_REPORT. */
 struct uhid_feature_req {
 	__u32 id;
 	__u8 rnum;
 	__u8 rtype;
 } __attribute__((__packed__));
 
+/* Obsolete! Use ABI compatible UHID_GET_REPORT_REPLY. */
 struct uhid_feature_answer_req {
 	__u32 id;
 	__u16 err;
@@ -90,6 +167,15 @@ struct uhid_feature_answer_req {
 	__u8 data[UHID_DATA_MAX];
 } __attribute__((__packed__));
 
+/*
+ * UHID Events
+ * All UHID events from and to the kernel are encoded as "struct uhid_event".
+ * The "type" field contains a UHID_* type identifier. All payload depends on
+ * that type and can be accessed via ev->u.XYZ accordingly.
+ * If user-space writes short events, they're extended with 0s by the kernel. If
+ * the kernel writes short events, user-space shall extend them with 0s.
+ */
+
 struct uhid_event {
 	__u32 type;
 
@@ -99,7 +185,14 @@ struct uhid_event {
 		struct uhid_output_req output;
 		struct uhid_output_ev_req output_ev;
 		struct uhid_feature_req feature;
+		struct uhid_get_report_req get_report;
 		struct uhid_feature_answer_req feature_answer;
+		struct uhid_get_report_reply_req get_report_reply;
+		struct uhid_create2_req create2;
+		struct uhid_input2_req input2;
+		struct uhid_set_report_req set_report;
+		struct uhid_set_report_reply_req set_report_reply;
+		struct uhid_start_req start;
 	} u;
 } __attribute__((__packed__));
 
-- 
2.1.3

[PATCH 2/4] hog: break out the report retrieval in its own function

[David Herrmann]

From: Benjamin Tissoires <benjamin.tissoires@redhat.com>

HoG currently only implements UHID_OUTPUT. We need to also implement
UHID_GET_REPORT and UHID_SET_REPORT. Break out the report retrieval
to use this function in the two missing implementation.
---
 profiles/input/hog.c | 42 +++++++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/profiles/input/hog.c b/profiles/input/hog.c
index b9aba65..de7e583 100644
--- a/profiles/input/hog.c
+++ b/profiles/input/hog.c
@@ -327,15 +327,12 @@ static void output_written_cb(guint8 status, const guint8 *pdu,
 	}
 }
 
-static void forward_report(struct uhid_event *ev, void *user_data)
+static struct report *find_report(struct hog_device *hogdev, uint8_t type, uint8_t id)
 {
-	struct hog_device *hogdev = user_data;
-	struct report *report, cmp;
+	struct report cmp;
 	GSList *l;
-	uint8_t *data;
-	int size;
 
-	switch (ev->u.output.rtype) {
+	switch (type) {
 	case UHID_FEATURE_REPORT:
 		cmp.type = HOG_REPORT_TYPE_FEATURE;
 		break;
@@ -346,25 +343,36 @@ static void forward_report(struct uhid_event *ev, void *user_data)
 		cmp.type = HOG_REPORT_TYPE_INPUT;
 		break;
 	default:
-		return;
+		return NULL;
 	}
 
-	cmp.id = 0;
+	cmp.id = hogdev->has_report_id ? id : 0;
+
+	l = g_slist_find_custom(hogdev->reports, &cmp, report_cmp);
+
+	return l ? l->data : NULL;
+}
+
+static void forward_report(struct uhid_event *ev, void *user_data)
+{
+	struct hog_device *hogdev = user_data;
+	struct report *report;
+	void *data;
+	int size;
+
+	report = find_report(hogdev, ev->u.output.rtype, ev->u.output.data[0]);
+	if (!report)
+		return;
+
 	data = ev->u.output.data;
 	size = ev->u.output.size;
 	if (hogdev->has_report_id && size > 0) {
-		cmp.id = *data++;
+		data++;
 		--size;
 	}
 
-	l = g_slist_find_custom(hogdev->reports, &cmp, report_cmp);
-	if (!l)
-		return;
-
-	report = l->data;
-
-	DBG("Sending report type %d ID %d to device 0x%04X handle 0x%X",
-		cmp.type, cmp.id, hogdev->id, report->decl->value_handle);
+	DBG("Sending report type %d ID %d to handle 0x%X", report->type,
+				report->id, report->decl->value_handle);
 
 	if (hogdev->attrib == NULL)
 		return;
-- 
2.1.3

[PATCH 3/4] hog: implement get_report functionality

[David Herrmann]

From: Benjamin Tissoires <benjamin.tissoires@redhat.com>

When UHID_GET_REPORT is received, hog has to update the current value
of the report and send it back to the caller.

This function should be synchronous, so the answer is sent in
get_report_cb().

(David: Track reports and cancel them on uhid timeouts. Otherwise, we'd
        incorrectly match reponses. Also fix "feature"->"get" typos and
        handle reports without IDs)
---
 profiles/input/hog.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/profiles/input/hog.c b/profiles/input/hog.c
index de7e583..25672a0 100644
--- a/profiles/input/hog.c
+++ b/profiles/input/hog.c
@@ -90,6 +90,8 @@ struct hog_device {
 	uint16_t		proto_mode_handle;
 	uint16_t		ctrlpt_handle;
 	uint8_t			flags;
+	guint			getrep_att;
+	uint16_t		getrep_id;
 };
 
 struct report {
@@ -385,6 +387,89 @@ static void forward_report(struct uhid_event *ev, void *user_data)
 						data, size, NULL, NULL);
 }
 
+static void get_report_cb(guint8 status, const guint8 *pdu, guint16 len,
+							gpointer user_data)
+{
+	struct hog_device *hogdev = user_data;
+	struct uhid_event rsp;
+	int err;
+
+	hogdev->getrep_att = 0;
+
+	memset(&rsp, 0, sizeof(rsp));
+	rsp.type = UHID_GET_REPORT_REPLY;
+	rsp.u.get_report_reply.id = hogdev->getrep_id;
+
+	if (status != 0) {
+		error("Error reading Report value: %s", att_ecode2str(status));
+		goto exit;
+	}
+
+	if (len <= 0) {
+		error("Error reading Report, length %d", len);
+		status = EIO;
+		goto exit;
+	}
+
+	if (pdu[0] != 0x0b) {
+		error("Error reading Report, invalid response: %02x", pdu[0]);
+		status = EPROTO;
+		goto exit;
+	}
+
+	--len;
+	++pdu;
+	if (hogdev->has_report_id && len > 0) {
+		--len;
+		++pdu;
+	}
+
+	rsp.u.get_report_reply.size = len;
+	memcpy(rsp.u.get_report_reply.data, pdu, len);
+
+exit:
+	rsp.u.get_report_reply.err = status;
+	err = bt_uhid_send(hogdev->uhid, &rsp);
+	if (err < 0)
+		error("bt_uhid_send: %s", strerror(-err));
+}
+
+static void get_report(struct uhid_event *ev, void *user_data)
+{
+	struct hog_device *hogdev = user_data;
+	struct report *report;
+	guint8 err;
+
+	/* uhid never sends reqs in parallel; if there's a req, it timed out */
+	if (hogdev->getrep_att) {
+		g_attrib_cancel(hogdev->attrib, hogdev->getrep_att);
+		hogdev->getrep_att = 0;
+	}
+
+	hogdev->getrep_id = ev->u.get_report.id;
+
+	report = find_report(hogdev, ev->u.get_report.rtype,
+							ev->u.get_report.rnum);
+	if (!report) {
+		err = ENOTSUP;
+		goto fail;
+	}
+
+	hogdev->getrep_att = gatt_read_char(hogdev->attrib,
+						report->decl->value_handle,
+						get_report_cb, hogdev);
+	if (!hogdev->getrep_att) {
+		err = ENOMEM;
+		goto fail;
+	}
+
+	return;
+
+fail:
+	/* cancel the request on failure */
+	get_report_cb(err, NULL, 0, hogdev);
+}
+
 static bool get_descriptor_item_info(uint8_t *buf, ssize_t blen, ssize_t *len,
 								bool *is_long)
 {
@@ -525,6 +610,7 @@ static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
 	}
 
 	bt_uhid_register(hogdev->uhid, UHID_OUTPUT, forward_report, hogdev);
+	bt_uhid_register(hogdev->uhid, UHID_GET_REPORT, get_report, hogdev);
 }
 
 static void info_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
-- 
2.1.3

[PATCH 4/4] hog: implement set_report functionality

[David Herrmann]

From: Benjamin Tissoires <benjamin.tissoires@redhat.com>

When UHID_SET_REPORT is received, hog has to send the given value
of the report to the device and send back the error code to the caller.

This function has to be synchronous, so the answer is sent in
set_report_cb().

(David: Track reports and cancel them on timeouts. Also fix some
        "get"->"set" typos.)
---
 profiles/input/hog.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/profiles/input/hog.c b/profiles/input/hog.c
index 25672a0..41da9f9 100644
--- a/profiles/input/hog.c
+++ b/profiles/input/hog.c
@@ -92,6 +92,8 @@ struct hog_device {
 	uint8_t			flags;
 	guint			getrep_att;
 	uint16_t		getrep_id;
+	guint			setrep_att;
+	uint16_t		setrep_id;
 };
 
 struct report {
@@ -387,6 +389,79 @@ static void forward_report(struct uhid_event *ev, void *user_data)
 						data, size, NULL, NULL);
 }
 
+static void set_report_cb(guint8 status, const guint8 *pdu,
+					guint16 plen, gpointer user_data)
+{
+	struct hog_device *hogdev = user_data;
+	struct uhid_event rsp;
+	int err;
+
+	hogdev->setrep_att = 0;
+
+	memset(&rsp, 0, sizeof(rsp));
+	rsp.type = UHID_SET_REPORT_REPLY;
+	rsp.u.set_report_reply.id = hogdev->setrep_id;
+	rsp.u.set_report_reply.err = status;
+
+	if (status != 0)
+		error("Error setting Report value: %s", att_ecode2str(status));
+
+	err = bt_uhid_send(hogdev->uhid, &rsp);
+	if (err < 0)
+		error("bt_uhid_send: %s", strerror(-err));
+}
+
+static void set_report(struct uhid_event *ev, void *user_data)
+{
+	struct hog_device *hogdev = user_data;
+	struct report *report;
+	void *data;
+	int size;
+	int err;
+
+	/* uhid never sends reqs in parallel; if there's a req, it timed out */
+	if (hogdev->setrep_att) {
+		g_attrib_cancel(hogdev->attrib, hogdev->setrep_att);
+		hogdev->setrep_att = 0;
+	}
+
+	hogdev->setrep_id = ev->u.set_report.id;
+
+	report = find_report(hogdev, ev->u.set_report.rtype,
+							ev->u.set_report.rnum);
+	if (!report) {
+		err = ENOTSUP;
+		goto fail;
+	}
+
+	data = ev->u.set_report.data;
+	size = ev->u.set_report.size;
+	if (hogdev->has_report_id && size > 0) {
+		data++;
+		--size;
+	}
+
+	DBG("Sending report type %d ID %d to handle 0x%X", report->type,
+				report->id, report->decl->value_handle);
+
+	if (hogdev->attrib == NULL)
+		return;
+
+	hogdev->setrep_att = gatt_write_char(hogdev->attrib,
+						report->decl->value_handle,
+						data, size, set_report_cb,
+						hogdev);
+	if (!hogdev->setrep_att) {
+		err = ENOMEM;
+		goto fail;
+	}
+
+	return;
+fail:
+	/* cancel the request on failure */
+	set_report_cb(err, NULL, 0, hogdev);
+}
+
 static void get_report_cb(guint8 status, const guint8 *pdu, guint16 len,
 							gpointer user_data)
 {
@@ -610,6 +685,7 @@ static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
 	}
 
 	bt_uhid_register(hogdev->uhid, UHID_OUTPUT, forward_report, hogdev);
+	bt_uhid_register(hogdev->uhid, UHID_SET_REPORT, set_report, hogdev);
 	bt_uhid_register(hogdev->uhid, UHID_GET_REPORT, get_report, hogdev);
 }
 
-- 
2.1.3

Re: [PATCH 0/4] Add GET/SET_REPORT support to HoG

[Benjamin Tissoires]

Hi David,

On Nov 21 2014 or thereabouts, David Herrmann wrote:
> Hi
> 
> With linux-3.18 uhid will gain GET/SET_REPORT support. 3.18 isn't released, yet,
> but the uhid features are part of 3.18-rc5 so I think it's fine to rely on it.
> We can always revert it if it will not be released (for whatever reason..).
> 
> Benjamin wrote these 4 patches about 2 months ago and I promised to fix a few
> bugs and send them upstream. I already feel bad for letting them rot for 2
> months.. sorry!

No problems from my side. As long as it gets applied in the end, that's
fine (or at least that the features are implemented).

> 
> Anyway, I fixed the ID tracking, some typos, and fixed handling of devices that
> dont send report IDs in GET_REPORT. Patches should be straightforward. The
> GET/SET_REPORT feature is important for nearly all non-standard HID device
> drivers. It's used to query and modify device behavior. With this in place, we
> should be able to use all the kernel hid drivers via uhid/hog.

Yay, thanks for that!

> 
> Benjamin has an example program to modify MS Arc Surface Touch mouse behavior,
> which needs these patches to query/modify the device. See:
>     https://github.com/bentiss/mstouchmouse/tree/WIP-USB
> (If you check it out, make sure to use the WIP-USB branch! You should also apply
>  the diff below, to make sure it works even if the report-ID is not prepended.)
> 
> Works fine with my devices here!
> David
> 
> 
> diff --git a/set_touchmode.c b/set_touchmode.c
> index 7e5d59a..0e6f412 100644
> --- a/set_touchmode.c
> +++ b/set_touchmode.c
> @@ -48,6 +48,11 @@ static int get_report(int fd, unsigned char id, ...
>      } else {
>          printf("Report data (%d):\n\t", id);
>          pr_buffer(buf, res);
> +        /* prepend report-ID if not sent by device */
> +        if (res > 0 && buf[0] != id) {
> +            memmove(buf + 1, buf, (res >= len) ? (res - 1) : res);
> +            buf[0] = id;
> +        }
>      }
>      return res;
>  }

Committed and pushed to the repo, so there will be no need to apply it
now :)

Cheers,
Benjamin

> 
> Benjamin Tissoires (4):
>   hog: import latest uhid.h definition
>   hog: break out the report retrieval in its own function
>   hog: implement get_report functionality
>   hog: implement set_report functionality
> 
>  profiles/input/hog.c       | 204 +++++++++++++++++++++++++++++++++++++++++----
>  profiles/input/uhid_copy.h | 117 +++++++++++++++++++++++---
>  2 files changed, 292 insertions(+), 29 deletions(-)
> 
> -- 
> 2.1.3
> 

Re: [PATCH 0/4] Add GET/SET_REPORT support to HoG

[Johan Hedberg]

Hi David,

On Fri, Nov 21, 2014, David Herrmann wrote:
> Benjamin Tissoires (4):
>   hog: import latest uhid.h definition
>   hog: break out the report retrieval in its own function
>   hog: implement get_report functionality
>   hog: implement set_report functionality
> 
>  profiles/input/hog.c       | 204 +++++++++++++++++++++++++++++++++++++++++----
>  profiles/input/uhid_copy.h | 117 +++++++++++++++++++++++---
>  2 files changed, 292 insertions(+), 29 deletions(-)

All these patches have been applied upstream. There was just a minor
thing I fixed myself with a follow-up patch where the code was doing an
unnecessary < 0 comparison of an unsigned value.

Johan

Re: [PATCH 0/4] Add GET/SET_REPORT support to HoG

[David Herrmann]

Hi

On Wed, Nov 26, 2014 at 9:04 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi David,
>
> On Fri, Nov 21, 2014, David Herrmann wrote:
>> Benjamin Tissoires (4):
>>   hog: import latest uhid.h definition
>>   hog: break out the report retrieval in its own function
>>   hog: implement get_report functionality
>>   hog: implement set_report functionality
>>
>>  profiles/input/hog.c       | 204 +++++++++++++++++++++++++++++++++++++++++----
>>  profiles/input/uhid_copy.h | 117 +++++++++++++++++++++++---
>>  2 files changed, 292 insertions(+), 29 deletions(-)
>
> All these patches have been applied upstream. There was just a minor
> thing I fixed myself with a follow-up patch where the code was doing an
> unnecessary < 0 comparison of an unsigned value.

Thanks, Johan!

David