| Message ID | 20201101193504.679934-1-lzye@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | [v2] Input: Fix the HID usage of DPAD input event generation. | expand |
Hi Chris, On Sun, Nov 1, 2020 at 8:35 PM Chris Ye <lzye@google.com> wrote: > > Generic Desktop DPAD usage is mapped by hid-input, that only the first > DPAD usage maps to usage type EV_ABS and code of an axis. If HID > descriptor has DPAD UP/DOWN/LEFT/RIGHT HID usages and each of usage size > is 1 bit, then only the first one will generate input event, the rest of > the HID usages will be assigned to hat direction only. > The hid input event should check the HID report value and generate > HID event for its hat direction. > > Test: Connect HID device with Generic Desktop DPAD usage and press the > DPAD to generate input events. Thanks for the patch, but I would rather have a proper tests added to https://gitlab.freedesktop.org/libevdev/hid-tools We already have gamepads tests, and it would be very nice to have this patch reflected as a test as well. This would also allow me to better understand the problem. I am not sure I follow the whole logic of this patch without seeing the 2 variants of report descriptors. Cheers, Benjamin > > Signed-off-by: Chris Ye <lzye@google.com> > --- > drivers/hid/hid-input.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c > index 9770db624bfa..6c1007de3409 100644 > --- a/drivers/hid/hid-input.c > +++ b/drivers/hid/hid-input.c > @@ -1269,7 +1269,7 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct > struct input_dev *input; > unsigned *quirks = &hid->quirks; > > - if (!usage->type) > + if (!usage->type && !field->dpad) > return; > > if (usage->type == EV_PWR) { > @@ -1286,9 +1286,17 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct > int hat_dir = usage->hat_dir; > if (!hat_dir) > hat_dir = (value - usage->hat_min) * 8 / (usage->hat_max - usage->hat_min + 1) + 1; > - if (hat_dir < 0 || hat_dir > 8) hat_dir = 0; > - input_event(input, usage->type, usage->code , hid_hat_to_axis[hat_dir].x); > - input_event(input, usage->type, usage->code + 1, hid_hat_to_axis[hat_dir].y); > + if (hat_dir < 0 || hat_dir > 8 || value == 0) > + hat_dir = 0; > + if (field->dpad) { > + input_event(input, EV_ABS, field->dpad, hid_hat_to_axis[hat_dir].x); > + input_event(input, EV_ABS, field->dpad + 1, hid_hat_to_axis[hat_dir].y); > + } else { > + input_event(input, usage->type, usage->code, > + hid_hat_to_axis[hat_dir].x); > + input_event(input, usage->type, usage->code + 1, > + hid_hat_to_axis[hat_dir].y); > + } > return; > } > > -- > 2.29.1.341.ge80a0c044ae-goog >
Hi Benjamin, I've tried the hid-tool for testing on my linux machine and it works. However the issue comes from a game controller I don't posses in hand right now so I can't physically connect it and provide the log from hid-tool recording. I do have the raw HID descriptor and in Linux kernel the debug info is like below: # cat /d/hid/0005\:27F8\:0BBE.0001/rdesc 05 01 09 05 a1 01 a1 02 15 81 25 7f 05 01 09 01 a1 00 75 08 95 04 35 00 46 ff 00 09 30 09 31 09 32 09 35 81 02 75 08 95 02 15 01 26 ff 00 09 39 09 39 81 02 c0 05 07 19 4f 29 52 15 00 25 01 75 01 95 04 81 02 05 01 09 90 09 91 09 92 09 93 75 01 95 04 81 02 75 01 95 10 05 09 19 01 29 10 81 02 06 02 ff 09 01 a1 01 15 00 25 01 09 04 75 01 95 01 81 02 c0 05 0c 09 01 a1 01 15 00 25 01 0a 24 02 75 01 95 01 81 06 c0 75 01 95 06 81 03 15 00 25 ff 05 02 09 01 a1 00 75 08 95 02 35 00 45 ff 09 c4 09 c5 81 02 c0 06 00 ff 09 80 75 08 95 08 15 00 26 ff 00 b1 02 c0 c0 INPUT[INPUT] Field(0) Physical(GenericDesktop.Pointer) Application(GenericDesktop.GamePad) Usage(4) GenericDesktop.X GenericDesktop.Y GenericDesktop.Z GenericDesktop.Rz Logical Minimum(-127) Logical Maximum(127) Physical Minimum(0) Physical Maximum(255) Report Size(8) Report Count(4) Report Offset(0) Flags( Variable Absolute ) Field(1) Physical(GenericDesktop.Pointer) Application(GenericDesktop.GamePad) Usage(2) GenericDesktop.HatSwitch GenericDesktop.HatSwitch Logical Minimum(1) Logical Maximum(255) Physical Minimum(0) Physical Maximum(255) Report Size(8) Report Count(2) Report Offset(32) Flags( Variable Absolute ) Field(2) Application(GenericDesktop.GamePad) Usage(4) Keyboard.004f Keyboard.0050 Keyboard.0051 Keyboard.0052 Logical Minimum(0) Logical Maximum(1) Physical Minimum(0) Physical Maximum(255) Report Size(1) Report Count(4) Report Offset(48) Flags( Variable Absolute ) Field(3) Application(GenericDesktop.GamePad) Usage(4) GenericDesktop.D-PadUp GenericDesktop.D-PadDown GenericDesktop.D-PadRight GenericDesktop.D-PadLeft Logical Minimum(0) Logical Maximum(1) Physical Minimum(0) Physical Maximum(255) Report Size(1) Report Count(4) Report Offset(52) Flags( Variable Absolute ) Field(4) Application(GenericDesktop.GamePad) Usage(16) Button.0001 Button.0002 Button.0003 Button.0004 Button.0005 Button.0006 Button.0007 Button.0008 Button.0009 Button.000a Button.000b Button.000c Button.000d Button.000e Button.000f Button.0010 Logical Minimum(0) Logical Maximum(1) Physical Minimum(0) Physical Maximum(255) Report Size(1) Report Count(16) Report Offset(56) Flags( Variable Absolute ) Field(5) Application(ff02.0001) Usage(1) ff02.0004 Logical Minimum(0) Logical Maximum(1) Physical Minimum(0) Physical Maximum(255) Report Size(1) Report Count(1) Report Offset(72) Flags( Variable Absolute ) Field(6) Application(Consumer.0001) Usage(1) Consumer.0224 Logical Minimum(0) Logical Maximum(1) Physical Minimum(0) Physical Maximum(255) Report Size(1) Report Count(1) Report Offset(73) Flags( Variable Relative ) Field(7) Physical(Simulation.0001) Application(GenericDesktop.GamePad) Usage(2) Simulation.00c4 Simulation.00c5 Logical Minimum(0) Logical Maximum(255) Physical Minimum(0) Physical Maximum(255) Report Size(8) Report Count(2) Report Offset(80) Flags( Variable Absolute ) FEATURE[FEATURE] Field(0) Application(GenericDesktop.GamePad) Usage(8) ff00.0080 ff00.0080 ff00.0080 ff00.0080 ff00.0080 ff00.0080 ff00.0080 ff00.0080 Logical Minimum(0) Logical Maximum(255) Physical Minimum(0) Physical Maximum(255) Report Size(8) Report Count(8) Report Offset(0) Flags( Variable Absolute ) GenericDesktop.X ---> Absolute.X GenericDesktop.Y ---> Absolute.Y GenericDesktop.Z ---> Absolute.Z GenericDesktop.Rz ---> Absolute.Rz GenericDesktop.HatSwitch ---> Absolute.Hat0X GenericDesktop.HatSwitch ---> Absolute.Hat0Y Keyboard.004f ---> Key.Right Keyboard.0050 ---> Key.Left Keyboard.0051 ---> Key.Down Keyboard.0052 ---> Key.Up GenericDesktop.D-PadUp ---> Absolute.Hat1X GenericDesktop.D-PadDown ---> Sync.Report GenericDesktop.D-PadRight ---> Sync.Report GenericDesktop.D-PadLeft ---> Sync.Report Button.0001 ---> Key.BtnA Button.0002 ---> Key.BtnB Button.0003 ---> Key.BtnC Button.0004 ---> Key.BtnX Button.0005 ---> Key.BtnY Button.0006 ---> Key.BtnZ Button.0007 ---> Key.BtnTL Button.0008 ---> Key.BtnTR Button.0009 ---> Key.BtnTL2 Button.000a ---> Key.BtnTR2 Button.000b ---> Key.BtnSelect Button.000c ---> Key.BtnStart Button.000d ---> Key.BtnMode Button.000e ---> Key.BtnThumbL Button.000f ---> Key.BtnThumbR Button.0010 ---> Key.? ff02.0004 ---> Key.Btn0 Consumer.0224 ---> Key.Back Simulation.00c4 ---> Absolute.Gas Simulation.00c5 ---> Absolute.Brake So you can see the device has D-Up, D-Down,D-Right,D-Left usages, and D-up is mapped to Hat1X. Also if you can send HID report from hid-tool, you will see there are always intail events on Hat1X/Hat1Y as the HatDir is 0, even no DPAD buttons pressed. When you send HID report with D-DPAD buttons with different state, it doesn't generate any axis events because the HatDir internally is still 0 regardless the report value of the 4 DPAD usages. Thanks! Chris On 11/2/20 12:16 AM, Benjamin Tissoires wrote: > Hi Chris, > > > On Sun, Nov 1, 2020 at 8:35 PM Chris Ye <lzye@google.com> wrote: >> Generic Desktop DPAD usage is mapped by hid-input, that only the first >> DPAD usage maps to usage type EV_ABS and code of an axis. If HID >> descriptor has DPAD UP/DOWN/LEFT/RIGHT HID usages and each of usage size >> is 1 bit, then only the first one will generate input event, the rest of >> the HID usages will be assigned to hat direction only. >> The hid input event should check the HID report value and generate >> HID event for its hat direction. >> >> Test: Connect HID device with Generic Desktop DPAD usage and press the >> DPAD to generate input events. > Thanks for the patch, but I would rather have a proper tests added to > https://gitlab.freedesktop.org/libevdev/hid-tools > > We already have gamepads tests, and it would be very nice to have this > patch reflected as a test as well. This would also allow me to better > understand the problem. I am not sure I follow the whole logic of this > patch without seeing the 2 variants of report descriptors. > > Cheers, > Benjamin > >> Signed-off-by: Chris Ye <lzye@google.com> >> --- >> drivers/hid/hid-input.c | 16 ++++++++++++---- >> 1 file changed, 12 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c >> index 9770db624bfa..6c1007de3409 100644 >> --- a/drivers/hid/hid-input.c >> +++ b/drivers/hid/hid-input.c >> @@ -1269,7 +1269,7 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct >> struct input_dev *input; >> unsigned *quirks = &hid->quirks; >> >> - if (!usage->type) >> + if (!usage->type && !field->dpad) >> return; >> >> if (usage->type == EV_PWR) { >> @@ -1286,9 +1286,17 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct >> int hat_dir = usage->hat_dir; >> if (!hat_dir) >> hat_dir = (value - usage->hat_min) * 8 / (usage->hat_max - usage->hat_min + 1) + 1; >> - if (hat_dir < 0 || hat_dir > 8) hat_dir = 0; >> - input_event(input, usage->type, usage->code , hid_hat_to_axis[hat_dir].x); >> - input_event(input, usage->type, usage->code + 1, hid_hat_to_axis[hat_dir].y); >> + if (hat_dir < 0 || hat_dir > 8 || value == 0) >> + hat_dir = 0; >> + if (field->dpad) { >> + input_event(input, EV_ABS, field->dpad, hid_hat_to_axis[hat_dir].x); >> + input_event(input, EV_ABS, field->dpad + 1, hid_hat_to_axis[hat_dir].y); >> + } else { >> + input_event(input, usage->type, usage->code, >> + hid_hat_to_axis[hat_dir].x); >> + input_event(input, usage->type, usage->code + 1, >> + hid_hat_to_axis[hat_dir].y); >> + } >> return; >> } >> >> -- >> 2.29.1.341.ge80a0c044ae-goog >>
Hi Chris, On Mon, Nov 2, 2020 at 6:24 PM Chris Ye <linzhao.ye@gmail.com> wrote: > > Hi Benjamin, > > I've tried the hid-tool for testing on my linux machine and it > works. However the issue comes from a game controller I don't posses in > hand right now so I can't physically connect it and provide the log from > hid-tool recording. I do have the raw HID descriptor and in Linux > kernel the debug info is like below: > > # cat /d/hid/0005\:27F8\:0BBE.0001/rdesc > 05 01 09 05 a1 01 a1 02 15 81 25 7f 05 01 09 01 a1 00 75 08 95 04 35 00 > 46 ff 00 09 30 09 31 09 32 09 35 81 02 75 08 95 02 15 01 26 ff 00 09 39 > 09 39 81 02 c0 05 07 19 4f 29 52 15 00 25 01 75 01 95 04 81 02 05 01 09 > 90 09 91 09 92 09 93 75 01 95 04 81 02 75 01 95 10 05 09 19 01 29 10 81 > 02 06 02 ff 09 01 a1 01 15 00 25 01 09 04 75 01 95 01 81 02 c0 05 0c 09 > 01 a1 01 15 00 25 01 0a 24 02 75 01 95 01 81 06 c0 75 01 95 06 81 03 15 > 00 25 ff 05 02 09 01 a1 00 75 08 95 02 35 00 45 ff 09 c4 09 c5 81 02 c0 > 06 00 ff 09 80 75 08 95 08 15 00 26 ff 00 b1 02 c0 c0 Thanks for the report descriptor. That allowed me to add the device to the tests at https://gitlab.freedesktop.org/bentiss/hid-tools/-/commits/wip/gamevice And also to realize that... "how is that thing supposed to be working????" > > INPUT[INPUT] > Field(0) > Physical(GenericDesktop.Pointer) > Application(GenericDesktop.GamePad) > Usage(4) > GenericDesktop.X > GenericDesktop.Y > GenericDesktop.Z > GenericDesktop.Rz > Logical Minimum(-127) > Logical Maximum(127) > Physical Minimum(0) > Physical Maximum(255) > Report Size(8) > Report Count(4) > Report Offset(0) > Flags( Variable Absolute ) > Field(1) > Physical(GenericDesktop.Pointer) > Application(GenericDesktop.GamePad) > Usage(2) > GenericDesktop.HatSwitch > GenericDesktop.HatSwitch > Logical Minimum(1) > Logical Maximum(255) > Physical Minimum(0) > Physical Maximum(255) > Report Size(8) > Report Count(2) > Report Offset(32) > Flags( Variable Absolute ) > Field(2) > Application(GenericDesktop.GamePad) > Usage(4) > Keyboard.004f > Keyboard.0050 > Keyboard.0051 > Keyboard.0052 > Logical Minimum(0) > Logical Maximum(1) > Physical Minimum(0) > Physical Maximum(255) > Report Size(1) > Report Count(4) > Report Offset(48) > Flags( Variable Absolute ) > Field(3) > Application(GenericDesktop.GamePad) > Usage(4) > GenericDesktop.D-PadUp > GenericDesktop.D-PadDown > GenericDesktop.D-PadRight > GenericDesktop.D-PadLeft > Logical Minimum(0) > Logical Maximum(1) > Physical Minimum(0) > Physical Maximum(255) > Report Size(1) > Report Count(4) > Report Offset(52) > Flags( Variable Absolute ) > Field(4) > Application(GenericDesktop.GamePad) > Usage(16) > Button.0001 > Button.0002 > Button.0003 > Button.0004 > Button.0005 > Button.0006 > Button.0007 > Button.0008 > Button.0009 > Button.000a > Button.000b > Button.000c > Button.000d > Button.000e > Button.000f > Button.0010 > Logical Minimum(0) > Logical Maximum(1) > Physical Minimum(0) > Physical Maximum(255) > Report Size(1) > Report Count(16) > Report Offset(56) > Flags( Variable Absolute ) > Field(5) > Application(ff02.0001) > Usage(1) > ff02.0004 > Logical Minimum(0) > Logical Maximum(1) > Physical Minimum(0) > Physical Maximum(255) > Report Size(1) > Report Count(1) > Report Offset(72) > Flags( Variable Absolute ) > Field(6) > Application(Consumer.0001) > Usage(1) > Consumer.0224 > Logical Minimum(0) > Logical Maximum(1) > Physical Minimum(0) > Physical Maximum(255) > Report Size(1) > Report Count(1) > Report Offset(73) > Flags( Variable Relative ) > Field(7) > Physical(Simulation.0001) > Application(GenericDesktop.GamePad) > Usage(2) > Simulation.00c4 > Simulation.00c5 > Logical Minimum(0) > Logical Maximum(255) > Physical Minimum(0) > Physical Maximum(255) > Report Size(8) > Report Count(2) > Report Offset(80) > Flags( Variable Absolute ) > FEATURE[FEATURE] > Field(0) > Application(GenericDesktop.GamePad) > Usage(8) > ff00.0080 > ff00.0080 > ff00.0080 > ff00.0080 > ff00.0080 > ff00.0080 > ff00.0080 > ff00.0080 > Logical Minimum(0) > Logical Maximum(255) > Physical Minimum(0) > Physical Maximum(255) > Report Size(8) > Report Count(8) > Report Offset(0) > Flags( Variable Absolute ) > > GenericDesktop.X ---> Absolute.X > GenericDesktop.Y ---> Absolute.Y > GenericDesktop.Z ---> Absolute.Z > GenericDesktop.Rz ---> Absolute.Rz > GenericDesktop.HatSwitch ---> Absolute.Hat0X > GenericDesktop.HatSwitch ---> Absolute.Hat0Y It took me a while to realize that you were needing https://patchwork.kernel.org/project/linux-input/patch/20201101193452.678628-1-lzye@google.com/ for that. But the weird part is that Hat switch are usually used as a single variable, with values being mapped to Hat0X and Hat0Y. So I still haven't manage to understand how the hid-input driver would map the axis to a regular one between 1 and 255... > Keyboard.004f ---> Key.Right > Keyboard.0050 ---> Key.Left > Keyboard.0051 ---> Key.Down > Keyboard.0052 ---> Key.Up > GenericDesktop.D-PadUp ---> Absolute.Hat1X > GenericDesktop.D-PadDown ---> Sync.Report > GenericDesktop.D-PadRight ---> Sync.Report > GenericDesktop.D-PadLeft ---> Sync.Report > Button.0001 ---> Key.BtnA > Button.0002 ---> Key.BtnB > Button.0003 ---> Key.BtnC > Button.0004 ---> Key.BtnX > Button.0005 ---> Key.BtnY > Button.0006 ---> Key.BtnZ > Button.0007 ---> Key.BtnTL > Button.0008 ---> Key.BtnTR > Button.0009 ---> Key.BtnTL2 > Button.000a ---> Key.BtnTR2 > Button.000b ---> Key.BtnSelect > Button.000c ---> Key.BtnStart > Button.000d ---> Key.BtnMode > Button.000e ---> Key.BtnThumbL > Button.000f ---> Key.BtnThumbR > Button.0010 ---> Key.? > ff02.0004 ---> Key.Btn0 > Consumer.0224 ---> Key.Back > Simulation.00c4 ---> Absolute.Gas > Simulation.00c5 ---> Absolute.Brake > > So you can see the device has D-Up, D-Down,D-Right,D-Left usages, and > D-up is mapped to Hat1X. OK, now I am starting to understand the problem better. > > Also if you can send HID report from hid-tool, you will see there are > always intail events on Hat1X/Hat1Y as the HatDir is 0, even no DPAD > buttons pressed. When you send HID report with D-DPAD buttons with > different state, it doesn't generate any axis events because the HatDir > internally is still 0 regardless the report value of the 4 DPAD usages. That's the part I am a little bit stuck. I can emulate events for X,Y, buttons,... but I am not sure how the gamepad sends the events for the Hat switch and the D-Pad together. Again, before we merge anything, I'd like to have the proper tests written for it, on top of https://gitlab.freedesktop.org/bentiss/hid-tools/-/commits/wip/gamevice so we can ensure there is no regression for it, and that we will not regress on it later on. Cheers, Benjamin > > > Thanks! > > Chris > > > On 11/2/20 12:16 AM, Benjamin Tissoires wrote: > > Hi Chris, > > > > > > On Sun, Nov 1, 2020 at 8:35 PM Chris Ye <lzye@google.com> wrote: > >> Generic Desktop DPAD usage is mapped by hid-input, that only the first > >> DPAD usage maps to usage type EV_ABS and code of an axis. If HID > >> descriptor has DPAD UP/DOWN/LEFT/RIGHT HID usages and each of usage size > >> is 1 bit, then only the first one will generate input event, the rest of > >> the HID usages will be assigned to hat direction only. > >> The hid input event should check the HID report value and generate > >> HID event for its hat direction. > >> > >> Test: Connect HID device with Generic Desktop DPAD usage and press the > >> DPAD to generate input events. > > Thanks for the patch, but I would rather have a proper tests added to > > https://gitlab.freedesktop.org/libevdev/hid-tools > > > > We already have gamepads tests, and it would be very nice to have this > > patch reflected as a test as well. This would also allow me to better > > understand the problem. I am not sure I follow the whole logic of this > > patch without seeing the 2 variants of report descriptors. > > > > Cheers, > > Benjamin > > > >> Signed-off-by: Chris Ye <lzye@google.com> > >> --- > >> drivers/hid/hid-input.c | 16 ++++++++++++---- > >> 1 file changed, 12 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c > >> index 9770db624bfa..6c1007de3409 100644 > >> --- a/drivers/hid/hid-input.c > >> +++ b/drivers/hid/hid-input.c > >> @@ -1269,7 +1269,7 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct > >> struct input_dev *input; > >> unsigned *quirks = &hid->quirks; > >> > >> - if (!usage->type) > >> + if (!usage->type && !field->dpad) > >> return; > >> > >> if (usage->type == EV_PWR) { > >> @@ -1286,9 +1286,17 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct > >> int hat_dir = usage->hat_dir; > >> if (!hat_dir) > >> hat_dir = (value - usage->hat_min) * 8 / (usage->hat_max - usage->hat_min + 1) + 1; > >> - if (hat_dir < 0 || hat_dir > 8) hat_dir = 0; > >> - input_event(input, usage->type, usage->code , hid_hat_to_axis[hat_dir].x); > >> - input_event(input, usage->type, usage->code + 1, hid_hat_to_axis[hat_dir].y); > >> + if (hat_dir < 0 || hat_dir > 8 || value == 0) > >> + hat_dir = 0; > >> + if (field->dpad) { > >> + input_event(input, EV_ABS, field->dpad, hid_hat_to_axis[hat_dir].x); > >> + input_event(input, EV_ABS, field->dpad + 1, hid_hat_to_axis[hat_dir].y); > >> + } else { > >> + input_event(input, usage->type, usage->code, > >> + hid_hat_to_axis[hat_dir].x); > >> + input_event(input, usage->type, usage->code + 1, > >> + hid_hat_to_axis[hat_dir].y); > >> + } > >> return; > >> } > >> > >> -- > >> 2.29.1.341.ge80a0c044ae-goog > >> >
Hi Benjamin, We are using hid driver to inject hid report for D-Pad and Hat switch events, like: [0x00, 0x00, 0x00, 0x00, 0x01, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00], Check the HID format dump from kernel, this will send a 0x1 on HID usage GenericDesktop.HatSwitch. Do you need me to help write the gamevice tests? I've not observed any regression from my side, as this patch is very specific to usage D-PadUp/Down/Left/Right. Thanks! Chris On 11/3/20 9:36 AM, Benjamin Tissoires wrote: > I can emulate events for X,Y, > buttons,... but I am not sure how the gamepad sends the events for the > Hat switch and the D-Pad together.
+stable@vger.kernel.org On 11/3/20 9:36 AM, Benjamin Tissoires wrote: > Hi Chris, > > On Mon, Nov 2, 2020 at 6:24 PM Chris Ye <linzhao.ye@gmail.com> wrote: >> Hi Benjamin, >> >> I've tried the hid-tool for testing on my linux machine and it >> works. However the issue comes from a game controller I don't posses in >> hand right now so I can't physically connect it and provide the log from >> hid-tool recording. I do have the raw HID descriptor and in Linux >> kernel the debug info is like below: >> >> # cat /d/hid/0005\:27F8\:0BBE.0001/rdesc >> 05 01 09 05 a1 01 a1 02 15 81 25 7f 05 01 09 01 a1 00 75 08 95 04 35 00 >> 46 ff 00 09 30 09 31 09 32 09 35 81 02 75 08 95 02 15 01 26 ff 00 09 39 >> 09 39 81 02 c0 05 07 19 4f 29 52 15 00 25 01 75 01 95 04 81 02 05 01 09 >> 90 09 91 09 92 09 93 75 01 95 04 81 02 75 01 95 10 05 09 19 01 29 10 81 >> 02 06 02 ff 09 01 a1 01 15 00 25 01 09 04 75 01 95 01 81 02 c0 05 0c 09 >> 01 a1 01 15 00 25 01 0a 24 02 75 01 95 01 81 06 c0 75 01 95 06 81 03 15 >> 00 25 ff 05 02 09 01 a1 00 75 08 95 02 35 00 45 ff 09 c4 09 c5 81 02 c0 >> 06 00 ff 09 80 75 08 95 08 15 00 26 ff 00 b1 02 c0 c0 > Thanks for the report descriptor. That allowed me to add the device to > the tests at https://gitlab.freedesktop.org/bentiss/hid-tools/-/commits/wip/gamevice > > And also to realize that... "how is that thing supposed to be working????" > >> INPUT[INPUT] >> Field(0) >> Physical(GenericDesktop.Pointer) >> Application(GenericDesktop.GamePad) >> Usage(4) >> GenericDesktop.X >> GenericDesktop.Y >> GenericDesktop.Z >> GenericDesktop.Rz >> Logical Minimum(-127) >> Logical Maximum(127) >> Physical Minimum(0) >> Physical Maximum(255) >> Report Size(8) >> Report Count(4) >> Report Offset(0) >> Flags( Variable Absolute ) >> Field(1) >> Physical(GenericDesktop.Pointer) >> Application(GenericDesktop.GamePad) >> Usage(2) >> GenericDesktop.HatSwitch >> GenericDesktop.HatSwitch >> Logical Minimum(1) >> Logical Maximum(255) >> Physical Minimum(0) >> Physical Maximum(255) >> Report Size(8) >> Report Count(2) >> Report Offset(32) >> Flags( Variable Absolute ) >> Field(2) >> Application(GenericDesktop.GamePad) >> Usage(4) >> Keyboard.004f >> Keyboard.0050 >> Keyboard.0051 >> Keyboard.0052 >> Logical Minimum(0) >> Logical Maximum(1) >> Physical Minimum(0) >> Physical Maximum(255) >> Report Size(1) >> Report Count(4) >> Report Offset(48) >> Flags( Variable Absolute ) >> Field(3) >> Application(GenericDesktop.GamePad) >> Usage(4) >> GenericDesktop.D-PadUp >> GenericDesktop.D-PadDown >> GenericDesktop.D-PadRight >> GenericDesktop.D-PadLeft >> Logical Minimum(0) >> Logical Maximum(1) >> Physical Minimum(0) >> Physical Maximum(255) >> Report Size(1) >> Report Count(4) >> Report Offset(52) >> Flags( Variable Absolute ) >> Field(4) >> Application(GenericDesktop.GamePad) >> Usage(16) >> Button.0001 >> Button.0002 >> Button.0003 >> Button.0004 >> Button.0005 >> Button.0006 >> Button.0007 >> Button.0008 >> Button.0009 >> Button.000a >> Button.000b >> Button.000c >> Button.000d >> Button.000e >> Button.000f >> Button.0010 >> Logical Minimum(0) >> Logical Maximum(1) >> Physical Minimum(0) >> Physical Maximum(255) >> Report Size(1) >> Report Count(16) >> Report Offset(56) >> Flags( Variable Absolute ) >> Field(5) >> Application(ff02.0001) >> Usage(1) >> ff02.0004 >> Logical Minimum(0) >> Logical Maximum(1) >> Physical Minimum(0) >> Physical Maximum(255) >> Report Size(1) >> Report Count(1) >> Report Offset(72) >> Flags( Variable Absolute ) >> Field(6) >> Application(Consumer.0001) >> Usage(1) >> Consumer.0224 >> Logical Minimum(0) >> Logical Maximum(1) >> Physical Minimum(0) >> Physical Maximum(255) >> Report Size(1) >> Report Count(1) >> Report Offset(73) >> Flags( Variable Relative ) >> Field(7) >> Physical(Simulation.0001) >> Application(GenericDesktop.GamePad) >> Usage(2) >> Simulation.00c4 >> Simulation.00c5 >> Logical Minimum(0) >> Logical Maximum(255) >> Physical Minimum(0) >> Physical Maximum(255) >> Report Size(8) >> Report Count(2) >> Report Offset(80) >> Flags( Variable Absolute ) >> FEATURE[FEATURE] >> Field(0) >> Application(GenericDesktop.GamePad) >> Usage(8) >> ff00.0080 >> ff00.0080 >> ff00.0080 >> ff00.0080 >> ff00.0080 >> ff00.0080 >> ff00.0080 >> ff00.0080 >> Logical Minimum(0) >> Logical Maximum(255) >> Physical Minimum(0) >> Physical Maximum(255) >> Report Size(8) >> Report Count(8) >> Report Offset(0) >> Flags( Variable Absolute ) >> >> GenericDesktop.X ---> Absolute.X >> GenericDesktop.Y ---> Absolute.Y >> GenericDesktop.Z ---> Absolute.Z >> GenericDesktop.Rz ---> Absolute.Rz >> GenericDesktop.HatSwitch ---> Absolute.Hat0X >> GenericDesktop.HatSwitch ---> Absolute.Hat0Y > It took me a while to realize that you were needing > https://patchwork.kernel.org/project/linux-input/patch/20201101193452.678628-1-lzye@google.com/ > for that. > > But the weird part is that Hat switch are usually used as a single > variable, with values being mapped to Hat0X and Hat0Y. So I still > haven't manage to understand how the hid-input driver would map the > axis to a regular one between 1 and 255... > >> Keyboard.004f ---> Key.Right >> Keyboard.0050 ---> Key.Left >> Keyboard.0051 ---> Key.Down >> Keyboard.0052 ---> Key.Up >> GenericDesktop.D-PadUp ---> Absolute.Hat1X >> GenericDesktop.D-PadDown ---> Sync.Report >> GenericDesktop.D-PadRight ---> Sync.Report >> GenericDesktop.D-PadLeft ---> Sync.Report >> Button.0001 ---> Key.BtnA >> Button.0002 ---> Key.BtnB >> Button.0003 ---> Key.BtnC >> Button.0004 ---> Key.BtnX >> Button.0005 ---> Key.BtnY >> Button.0006 ---> Key.BtnZ >> Button.0007 ---> Key.BtnTL >> Button.0008 ---> Key.BtnTR >> Button.0009 ---> Key.BtnTL2 >> Button.000a ---> Key.BtnTR2 >> Button.000b ---> Key.BtnSelect >> Button.000c ---> Key.BtnStart >> Button.000d ---> Key.BtnMode >> Button.000e ---> Key.BtnThumbL >> Button.000f ---> Key.BtnThumbR >> Button.0010 ---> Key.? >> ff02.0004 ---> Key.Btn0 >> Consumer.0224 ---> Key.Back >> Simulation.00c4 ---> Absolute.Gas >> Simulation.00c5 ---> Absolute.Brake >> >> So you can see the device has D-Up, D-Down,D-Right,D-Left usages, and >> D-up is mapped to Hat1X. > OK, now I am starting to understand the problem better. > >> Also if you can send HID report from hid-tool, you will see there are >> always intail events on Hat1X/Hat1Y as the HatDir is 0, even no DPAD >> buttons pressed. When you send HID report with D-DPAD buttons with >> different state, it doesn't generate any axis events because the HatDir >> internally is still 0 regardless the report value of the 4 DPAD usages. > That's the part I am a little bit stuck. I can emulate events for X,Y, > buttons,... but I am not sure how the gamepad sends the events for the > Hat switch and the D-Pad together. > > Again, before we merge anything, I'd like to have the proper tests > written for it, on top of > https://gitlab.freedesktop.org/bentiss/hid-tools/-/commits/wip/gamevice > so we can ensure there is no regression for it, and that we will not > regress on it later on. > > Cheers, > Benjamin > >> >> Thanks! >> >> Chris >> >> >> On 11/2/20 12:16 AM, Benjamin Tissoires wrote: >>> Hi Chris, >>> >>> >>> On Sun, Nov 1, 2020 at 8:35 PM Chris Ye <lzye@google.com> wrote: >>>> Generic Desktop DPAD usage is mapped by hid-input, that only the first >>>> DPAD usage maps to usage type EV_ABS and code of an axis. If HID >>>> descriptor has DPAD UP/DOWN/LEFT/RIGHT HID usages and each of usage size >>>> is 1 bit, then only the first one will generate input event, the rest of >>>> the HID usages will be assigned to hat direction only. >>>> The hid input event should check the HID report value and generate >>>> HID event for its hat direction. >>>> >>>> Test: Connect HID device with Generic Desktop DPAD usage and press the >>>> DPAD to generate input events. >>> Thanks for the patch, but I would rather have a proper tests added to >>> https://gitlab.freedesktop.org/libevdev/hid-tools >>> >>> We already have gamepads tests, and it would be very nice to have this >>> patch reflected as a test as well. This would also allow me to better >>> understand the problem. I am not sure I follow the whole logic of this >>> patch without seeing the 2 variants of report descriptors. >>> >>> Cheers, >>> Benjamin >>> >>>> Signed-off-by: Chris Ye <lzye@google.com> >>>> --- >>>> drivers/hid/hid-input.c | 16 ++++++++++++---- >>>> 1 file changed, 12 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c >>>> index 9770db624bfa..6c1007de3409 100644 >>>> --- a/drivers/hid/hid-input.c >>>> +++ b/drivers/hid/hid-input.c >>>> @@ -1269,7 +1269,7 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct >>>> struct input_dev *input; >>>> unsigned *quirks = &hid->quirks; >>>> >>>> - if (!usage->type) >>>> + if (!usage->type && !field->dpad) >>>> return; >>>> >>>> if (usage->type == EV_PWR) { >>>> @@ -1286,9 +1286,17 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct >>>> int hat_dir = usage->hat_dir; >>>> if (!hat_dir) >>>> hat_dir = (value - usage->hat_min) * 8 / (usage->hat_max - usage->hat_min + 1) + 1; >>>> - if (hat_dir < 0 || hat_dir > 8) hat_dir = 0; >>>> - input_event(input, usage->type, usage->code , hid_hat_to_axis[hat_dir].x); >>>> - input_event(input, usage->type, usage->code + 1, hid_hat_to_axis[hat_dir].y); >>>> + if (hat_dir < 0 || hat_dir > 8 || value == 0) >>>> + hat_dir = 0; >>>> + if (field->dpad) { >>>> + input_event(input, EV_ABS, field->dpad, hid_hat_to_axis[hat_dir].x); >>>> + input_event(input, EV_ABS, field->dpad + 1, hid_hat_to_axis[hat_dir].y); >>>> + } else { >>>> + input_event(input, usage->type, usage->code, >>>> + hid_hat_to_axis[hat_dir].x); >>>> + input_event(input, usage->type, usage->code + 1, >>>> + hid_hat_to_axis[hat_dir].y); >>>> + } >>>> return; >>>> } >>>> >>>> -- >>>> 2.29.1.341.ge80a0c044ae-goog >>>>
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index 9770db624bfa..6c1007de3409 100644 --- a/drivers/hid/hid-input.c +++ b/drivers/hid/hid-input.c @@ -1269,7 +1269,7 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct struct input_dev *input; unsigned *quirks = &hid->quirks; - if (!usage->type) + if (!usage->type && !field->dpad) return; if (usage->type == EV_PWR) { @@ -1286,9 +1286,17 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct int hat_dir = usage->hat_dir; if (!hat_dir) hat_dir = (value - usage->hat_min) * 8 / (usage->hat_max - usage->hat_min + 1) + 1; - if (hat_dir < 0 || hat_dir > 8) hat_dir = 0; - input_event(input, usage->type, usage->code , hid_hat_to_axis[hat_dir].x); - input_event(input, usage->type, usage->code + 1, hid_hat_to_axis[hat_dir].y); + if (hat_dir < 0 || hat_dir > 8 || value == 0) + hat_dir = 0; + if (field->dpad) { + input_event(input, EV_ABS, field->dpad, hid_hat_to_axis[hat_dir].x); + input_event(input, EV_ABS, field->dpad + 1, hid_hat_to_axis[hat_dir].y); + } else { + input_event(input, usage->type, usage->code, + hid_hat_to_axis[hat_dir].x); + input_event(input, usage->type, usage->code + 1, + hid_hat_to_axis[hat_dir].y); + } return; }
Generic Desktop DPAD usage is mapped by hid-input, that only the first DPAD usage maps to usage type EV_ABS and code of an axis. If HID descriptor has DPAD UP/DOWN/LEFT/RIGHT HID usages and each of usage size is 1 bit, then only the first one will generate input event, the rest of the HID usages will be assigned to hat direction only. The hid input event should check the HID report value and generate HID event for its hat direction. Test: Connect HID device with Generic Desktop DPAD usage and press the DPAD to generate input events. Signed-off-by: Chris Ye <lzye@google.com> --- drivers/hid/hid-input.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)