Add GET/SET_REPORT support to HoG
cover Add GET/SET_REPORT support to HoG
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
set_touchmode.c +5 -0
@@ -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(-)
Signature · 1 line
2.1.3
PATCH 1/4 hog: import latest uhid.h definition
From: Benjamin Tissoires <[email protected]>
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(-)
profiles/input/uhid_copy.h +105 -12
@@ -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 4096enum 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__));
Signature · 1 line
2.1.3
PATCH 2/4 hog: break out the report retrieval in its own function
In reply to David HerrmannFrom: Benjamin Tissoires <[email protected]>
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(-)
profiles/input/hog.c +25 -17
@@ -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 ;
Signature · 1 line
2.1.3
PATCH 3/4 hog: implement get_report functionality
In reply to David HerrmannFrom: Benjamin Tissoires <[email protected]>
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(+)
profiles/input/hog.c +86 -0
@@ -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,
Signature · 1 line
2.1.3
PATCH 4/4 hog: implement set_report functionality
In reply to David HerrmannFrom: Benjamin Tissoires <[email protected]>
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(+)
profiles/input/hog.c +76 -0
@@ -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); }
Signature · 1 line
2.1.3
Hi David,
Quoted 9 lines · 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!
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).
Quoted 6 lines
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!
Quoted 17 lines
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!
Davidset_touchmode.c +0
-0@@ -48,6 +48,11 @@ static int get_report(int fd, unsigned char id, ... }else {
set_touchmode.c static int get_report(int fd, unsigned char id, ... · line 58
Quoted 10 lines
set_touchmode.c +5
-0printf ("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
Quoted 14 lines
Benjamin Tissoires (4):
hog: import latestuhid.hdefinition
hog: break out the report retrieval in its own function
hog: implementget_reportfunctionality
hog: implementset_reportfunctionalityprofiles/input/hog.c | 204 +++++++++++++++++++++++++++++++++++++++++---- profiles/input/uhid_copy.h | 117 +++++++++++++++++++++++--- 2 files changed, 292 insertions(+), 29 deletions(-)Signature · 1 line
2.1.3
Hi David,
Quoted 9 lines · David Herrmann
Benjamin Tissoires (4):
hog: import latestuhid.hdefinition
hog: break out the report retrieval in its own function
hog: implementget_reportfunctionality
hog: implementset_reportfunctionalityprofiles/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
Hi
Quoted 9 lines · Johan Hedberg
Hi David,
Quoted 6 lines · David Herrmann
Benjamin Tissoires (4):
hog: import latestuhid.hdefinition
hog: break out the report retrieval in its own function
hog: implementget_reportfunctionality
hog: implementset_reportfunctionality
Quoted 7 lines
Quoted 3 lines
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