Re: Diff to support so-called "gaming" USB keyboards
On 2012/07/03 15:23, Mike Larkin wrote: > Many low-cost USB keyboards have a limit of either 3 or 6 simultaneous > keypresses before they wedge and stop supplying any more keypress events > (at least until you release one of the pressed keys). No regressions with my HHKB lite 2. uhub4 at uhub2 port 2 "Chicony Generic USB Hub" rev 1.10/1.00 addr 3 uhidev0 at uhub4 port 1 configuration 1 interface 0 "Chicony PFU-65 USB Keyboard" rev 1.10/1.00 addr 4 uhidev0: iclass 3/1 ukbd0 at uhidev0: 8 variable keys, 6 key codes wskbd1 at ukbd0 mux 1 wskbd1: connecting to wsdisplay0 Also works fine with an adderlink ipeps 1-port KVM/IP (nice little box, you can just use normal vnc to connect to it) uhub3: port 2, set config at addr 3 failed uhub3: device problem, disabling port 2 uhidev0 at uhub3 port 2 configuration 1 interface 0 "RealVNC Mouse, Keyboard & Mass Storage" rev 2.00/1.01 addr 3 uhidev0: iclass 3/1 ukbd0 at uhidev0: 8 variable keys, 6 key codes wskbd1 at ukbd0 mux 1 wskbd1: connecting to wsdisplay0 uhidev1 at uhub3 port 2 configuration 1 interface 1 "RealVNC Mouse, Keyboard & Mass Storage" rev 2.00/1.01 addr 3 uhidev1: iclass 3/0 ums0 at uhidev1: 3 buttons, Z dir wsmouse1 at ums0 mux 0 uhidev2 at uhub3 port 2 configuration 1 interface 2 "RealVNC Mouse, Keyboard & Mass Storage" rev 2.00/1.01 addr 3 uhidev2: iclass 3/0 ums1 at uhidev2: 3 buttons, Z dir wsmouse2 at ums1 mux 0
Re: Diff to support so-called "gaming" USB keyboards
On Tue, Jul 03, 2012 at 03:23:16PM -0700, Mike Larkin wrote:
> Many low-cost USB keyboards have a limit of either 3 or 6 simultaneous
> keypresses before they wedge and stop supplying any more keypress events
> (at least until you release one of the pressed keys).
>
> Some newer (usually called "gaming") keyboards use a different way of
> reporting keypress events in order to support an arbitrary number of
> simultaneous keypresses (likely under the assumption that in certain
> games, you might need more than 3 or 6 keys pressed at the same time).
>
> Our USB HID keyboard code had some bad assumptions in it - namely, that
> every key reported in this fashion was a 'modifier' key like shift,
> alt, or control, and that there would only be 8 such keys maximum.
>
> The diff below reworks the HID keyboard code a bit to remove these
> assumptions and support these newer style USB keyboards. It also
> changes the term 'modifier' to 'variable' which is a more proper
> description of these keys (as per the HID spec), in various
> comments and printfs.
>
> If you have a USB keyboard (whether or not it is a "gaming" variety),
> please test this diff to ensure that it does not break you. I've tested
> on i386 and amd64 on a variety of keyboards and haven't seen any
> problems, so it's time to turn it loose to a wider audience.
>
> -ml
This keyboard is still working fine:
uhidev2 at uhub2 port 1 configuration 1 interface 0 "Microsoft Natural\M-.
Ergonomic Keyboard 4000" rev 2.00/1.73 addr 2
uhidev2: iclass 3/1
ukbd1 at uhidev2: 8 variable keys, 6 key codes
wskbd2 at ukbd1 mux 1
wskbd2: connecting to wsdisplay0
uhidev3 at uhub2 port 1 configuration 1 interface 1 "Microsoft Natural\M-.
Ergonomic Keyboard 4000" rev 2.00/1.73 addr 2
uhidev3: iclass 3/0, 1 report id
uhid0 at uhidev3 reportid 1: input=7, output=0, feature=0
Although it's still suffering from the fact I need to detach/attach the cables
upon every boot/suspend otherwise it just sits there playing dead. But that's
unrelated to this diff.
> Index: hidkbd.c
> ===
> RCS file: /cvs/src/sys/dev/usb/hidkbd.c,v
> retrieving revision 1.5
> diff -a -u -r1.5 hidkbd.c
> --- hidkbd.c 9 Nov 2011 14:22:38 - 1.5
> +++ hidkbd.c 29 Jun 2012 06:45:59 -
> @@ -41,6 +41,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -164,6 +165,8 @@
> {
> const char *parserr;
>
> + kbd->sc_var = NULL;
> +
> parserr = hidkbd_parse_desc(kbd, id, desc, dlen);
> if (parserr != NULL) {
> printf(": %s\n", parserr);
> @@ -171,8 +174,8 @@
> }
>
> #ifdef DIAGNOSTIC
> - printf(": %d modifier keys, %d key codes",
> - kbd->sc_nmod, kbd->sc_nkeycode);
> + printf(": %d variable keys, %d key codes",
> + kbd->sc_nvar, kbd->sc_nkeycode);
> #endif
>
> kbd->sc_device = self;
> @@ -245,6 +248,9 @@
> if (kbd->sc_wskbddev != NULL)
> rv = config_detach(kbd->sc_wskbddev, flags);
>
> + if (kbd->sc_var != NULL)
> + free(kbd->sc_var, M_DEVBUF);
> +
> return (rv);
> }
>
> @@ -263,11 +269,9 @@
> }
> #endif
>
> - /* extract key modifiers */
> - ud->modifiers = 0;
> - for (i = 0; i < kbd->sc_nmod; i++)
> - if (hid_get_data(data, &kbd->sc_modloc[i]))
> - ud->modifiers |= kbd->sc_mods[i].mask;
> + /* extract variable keys */
> + for (i = 0; i < kbd->sc_nvar; i++)
> + ud->var[i] = (u_int8_t)hid_get_data(data, &kbd->sc_var[i].loc);
>
> /* extract keycodes */
> memcpy(ud->keycode, data + kbd->sc_keycodeloc.pos / 8,
> @@ -311,7 +315,6 @@
> void
> hidkbd_decode(struct hidkbd *kbd, struct hidkbd_data *ud)
> {
> - uint32_t mod, omod;
> u_int16_t ibuf[MAXKEYS];/* chars events */
> int s;
> int nkeys, i, j;
> @@ -347,15 +350,15 @@
> return; /* ignore */
> }
> nkeys = 0;
> - mod = ud->modifiers;
> - omod = kbd->sc_odata.modifiers;
> - if (mod != omod)
> - for (i = 0; i < kbd->sc_nmod; i++)
> - if (( mod & kbd->sc_mods[i].mask) !=
> - (omod & kbd->sc_mods[i].mask))
> - ADDKEY(kbd->sc_mods[i].key |
> -(mod & kbd->sc_mods[i].mask
> - ? PRESS : RELEASE));
> +
> + for (i = 0; i < kbd->sc_nvar; i++)
> + if ((kbd->sc_odata.var[i] & kbd->sc_var[i].mask) !=
> + (ud->var[i] & kbd->sc_var[i].mask)) {
> + ADDKEY(kbd->sc_var[i].key |
> + ((ud->var[i] & kbd->sc_var[i].mask) ?
> + PRESS : RELEASE));
> + }
> +
> if (memcmp(ud->keycode, kbd->sc_odata.keycode, kbd->sc_nkeycode) != 0) {
> /* Check for released keys. */
> for (i = 0; i <
Re: Diff to support so-called "gaming" USB keyboards
Yep. I had a similar keyboard. I'll test the diff when I'll get home.
I submitted a diff to get it to work & miod committed it along with other
stuff.
http://archives.neohapsis.com/archives/openbsd/cvs/2010-08/0017.html
On Wed, Jul 4, 2012 at 2:23 AM, Mike Larkin wrote:
> Many low-cost USB keyboards have a limit of either 3 or 6 simultaneous
> keypresses before they wedge and stop supplying any more keypress events
> (at least until you release one of the pressed keys).
>
> Some newer (usually called "gaming") keyboards use a different way of
> reporting keypress events in order to support an arbitrary number of
> simultaneous keypresses (likely under the assumption that in certain
> games, you might need more than 3 or 6 keys pressed at the same time).
>
> Our USB HID keyboard code had some bad assumptions in it - namely, that
> every key reported in this fashion was a 'modifier' key like shift,
> alt, or control, and that there would only be 8 such keys maximum.
>
> The diff below reworks the HID keyboard code a bit to remove these
> assumptions and support these newer style USB keyboards. It also
> changes the term 'modifier' to 'variable' which is a more proper
> description of these keys (as per the HID spec), in various
> comments and printfs.
>
> If you have a USB keyboard (whether or not it is a "gaming" variety),
> please test this diff to ensure that it does not break you. I've tested
> on i386 and amd64 on a variety of keyboards and haven't seen any
> problems, so it's time to turn it loose to a wider audience.
>
> -ml
>
>
> Index: hidkbd.c
> ===
> RCS file: /cvs/src/sys/dev/usb/hidkbd.c,v
> retrieving revision 1.5
> diff -a -u -r1.5 hidkbd.c
> --- hidkbd.c9 Nov 2011 14:22:38 - 1.5
> +++ hidkbd.c29 Jun 2012 06:45:59 -
> @@ -41,6 +41,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -164,6 +165,8 @@
> {
> const char *parserr;
>
> + kbd->sc_var = NULL;
> +
> parserr = hidkbd_parse_desc(kbd, id, desc, dlen);
> if (parserr != NULL) {
> printf(": %s\n", parserr);
> @@ -171,8 +174,8 @@
> }
>
> #ifdef DIAGNOSTIC
> - printf(": %d modifier keys, %d key codes",
> - kbd->sc_nmod, kbd->sc_nkeycode);
> + printf(": %d variable keys, %d key codes",
> + kbd->sc_nvar, kbd->sc_nkeycode);
> #endif
>
> kbd->sc_device = self;
> @@ -245,6 +248,9 @@
> if (kbd->sc_wskbddev != NULL)
> rv = config_detach(kbd->sc_wskbddev, flags);
>
> + if (kbd->sc_var != NULL)
> + free(kbd->sc_var, M_DEVBUF);
> +
> return (rv);
> }
>
> @@ -263,11 +269,9 @@
> }
> #endif
>
> - /* extract key modifiers */
> - ud->modifiers = 0;
> - for (i = 0; i < kbd->sc_nmod; i++)
> - if (hid_get_data(data, &kbd->sc_modloc[i]))
> - ud->modifiers |= kbd->sc_mods[i].mask;
> + /* extract variable keys */
> + for (i = 0; i < kbd->sc_nvar; i++)
> + ud->var[i] = (u_int8_t)hid_get_data(data,
> &kbd->sc_var[i].loc);
>
> /* extract keycodes */
> memcpy(ud->keycode, data + kbd->sc_keycodeloc.pos / 8,
> @@ -311,7 +315,6 @@
> void
> hidkbd_decode(struct hidkbd *kbd, struct hidkbd_data *ud)
> {
> - uint32_t mod, omod;
> u_int16_t ibuf[MAXKEYS];/* chars events */
> int s;
> int nkeys, i, j;
> @@ -347,15 +350,15 @@
> return; /* ignore */
> }
> nkeys = 0;
> - mod = ud->modifiers;
> - omod = kbd->sc_odata.modifiers;
> - if (mod != omod)
> - for (i = 0; i < kbd->sc_nmod; i++)
> - if (( mod & kbd->sc_mods[i].mask) !=
> - (omod & kbd->sc_mods[i].mask))
> - ADDKEY(kbd->sc_mods[i].key |
> - (mod & kbd->sc_mods[i].mask
> - ? PRESS : RELEASE));
> +
> + for (i = 0; i < kbd->sc_nvar; i++)
> + if ((kbd->sc_odata.var[i] & kbd->sc_var[i].mask) !=
> + (ud->var[i] & kbd->sc_var[i].mask)) {
> + ADDKEY(kbd->sc_var[i].key |
> + ((ud->var[i] & kbd->sc_var[i].mask) ?
> + PRESS : RELEASE));
> + }
> +
> if (memcmp(ud->keycode, kbd->sc_odata.keycode, kbd->sc_nkeycode) !=
> 0) {
> /* Check for released keys. */
> for (i = 0; i < kbd->sc_nkeycode; i++) {
> @@ -547,10 +550,36 @@
> {
> struct hid_data *d;
> struct hid_item h;
> - int imod;
> + int i, ivar = 0;
>
> - imod = 0;
> kbd->sc_nkeycode = 0;
> +
> + d = hid_start_parse(desc, dlen, hid_input);
> + while (hid_get_item(d, &h)) {
> + if (h.kind != hid_input || (h.flags
Diff to support so-called "gaming" USB keyboards
Many low-cost USB keyboards have a limit of either 3 or 6 simultaneous
keypresses before they wedge and stop supplying any more keypress events
(at least until you release one of the pressed keys).
Some newer (usually called "gaming") keyboards use a different way of
reporting keypress events in order to support an arbitrary number of
simultaneous keypresses (likely under the assumption that in certain
games, you might need more than 3 or 6 keys pressed at the same time).
Our USB HID keyboard code had some bad assumptions in it - namely, that
every key reported in this fashion was a 'modifier' key like shift,
alt, or control, and that there would only be 8 such keys maximum.
The diff below reworks the HID keyboard code a bit to remove these
assumptions and support these newer style USB keyboards. It also
changes the term 'modifier' to 'variable' which is a more proper
description of these keys (as per the HID spec), in various
comments and printfs.
If you have a USB keyboard (whether or not it is a "gaming" variety),
please test this diff to ensure that it does not break you. I've tested
on i386 and amd64 on a variety of keyboards and haven't seen any
problems, so it's time to turn it loose to a wider audience.
-ml
Index: hidkbd.c
===
RCS file: /cvs/src/sys/dev/usb/hidkbd.c,v
retrieving revision 1.5
diff -a -u -r1.5 hidkbd.c
--- hidkbd.c9 Nov 2011 14:22:38 - 1.5
+++ hidkbd.c29 Jun 2012 06:45:59 -
@@ -41,6 +41,7 @@
#include
#include
#include
+#include
#include
#include
@@ -164,6 +165,8 @@
{
const char *parserr;
+ kbd->sc_var = NULL;
+
parserr = hidkbd_parse_desc(kbd, id, desc, dlen);
if (parserr != NULL) {
printf(": %s\n", parserr);
@@ -171,8 +174,8 @@
}
#ifdef DIAGNOSTIC
- printf(": %d modifier keys, %d key codes",
- kbd->sc_nmod, kbd->sc_nkeycode);
+ printf(": %d variable keys, %d key codes",
+ kbd->sc_nvar, kbd->sc_nkeycode);
#endif
kbd->sc_device = self;
@@ -245,6 +248,9 @@
if (kbd->sc_wskbddev != NULL)
rv = config_detach(kbd->sc_wskbddev, flags);
+ if (kbd->sc_var != NULL)
+ free(kbd->sc_var, M_DEVBUF);
+
return (rv);
}
@@ -263,11 +269,9 @@
}
#endif
- /* extract key modifiers */
- ud->modifiers = 0;
- for (i = 0; i < kbd->sc_nmod; i++)
- if (hid_get_data(data, &kbd->sc_modloc[i]))
- ud->modifiers |= kbd->sc_mods[i].mask;
+ /* extract variable keys */
+ for (i = 0; i < kbd->sc_nvar; i++)
+ ud->var[i] = (u_int8_t)hid_get_data(data, &kbd->sc_var[i].loc);
/* extract keycodes */
memcpy(ud->keycode, data + kbd->sc_keycodeloc.pos / 8,
@@ -311,7 +315,6 @@
void
hidkbd_decode(struct hidkbd *kbd, struct hidkbd_data *ud)
{
- uint32_t mod, omod;
u_int16_t ibuf[MAXKEYS];/* chars events */
int s;
int nkeys, i, j;
@@ -347,15 +350,15 @@
return; /* ignore */
}
nkeys = 0;
- mod = ud->modifiers;
- omod = kbd->sc_odata.modifiers;
- if (mod != omod)
- for (i = 0; i < kbd->sc_nmod; i++)
- if (( mod & kbd->sc_mods[i].mask) !=
- (omod & kbd->sc_mods[i].mask))
- ADDKEY(kbd->sc_mods[i].key |
- (mod & kbd->sc_mods[i].mask
- ? PRESS : RELEASE));
+
+ for (i = 0; i < kbd->sc_nvar; i++)
+ if ((kbd->sc_odata.var[i] & kbd->sc_var[i].mask) !=
+ (ud->var[i] & kbd->sc_var[i].mask)) {
+ ADDKEY(kbd->sc_var[i].key |
+ ((ud->var[i] & kbd->sc_var[i].mask) ?
+ PRESS : RELEASE));
+ }
+
if (memcmp(ud->keycode, kbd->sc_odata.keycode, kbd->sc_nkeycode) != 0) {
/* Check for released keys. */
for (i = 0; i < kbd->sc_nkeycode; i++) {
@@ -547,10 +550,36 @@
{
struct hid_data *d;
struct hid_item h;
- int imod;
+ int i, ivar = 0;
- imod = 0;
kbd->sc_nkeycode = 0;
+
+ d = hid_start_parse(desc, dlen, hid_input);
+ while (hid_get_item(d, &h)) {
+ if (h.kind != hid_input || (h.flags & HIO_CONST) ||
+ HID_GET_USAGE_PAGE(h.usage) != HUP_KEYBOARD ||
+ h.report_ID != id)
+ continue;
+ if (h.flags & HIO_VARIABLE)
+ ivar++;
+ }
+ hid_end_parse(d);
+
+ if (ivar > MAXVARS) {
+ DPRINTF((": too many variable keys\n"));
+ ivar = MAXVARS;
+ }
+
+ kbd->sc_nvar = ivar;
+ kbd->sc_var = (struct hidkbd_variable *)malloc(
+ sizeof(str
