Re: Suspected bug in hid-microsoft.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Feb 6, 2024 at 6:25 AM Taco Jerkface <tacodog311@xxxxxxxxx> wrote:
>
> Thanks!
>
> > 1. (easiest) tune your udev rule to also give user access to the
> > hidraw subsystem on this device. SDL should be able to read it
> > directly, and handle it properly, but this won't solve for future
> > users
>
> This udev rule does in fact "fix" the problem
>
> KERNELS=="*:045E:0B22.*", MODE="0666"

Right :)

Well you could restrict it further by using the recommendations from SDL:
https://github.com/libsdl-org/SDL/blob/main/src/hidapi/udev/69-hid.rules

# HIDAPI/hidraw
KERNEL=="hidraw*", ATTRS{idVendor}=="045E", ATTRS{idProduct}=="0B22",
TAG+="uaccess"

(not entirely sure about the capitals in vendor/product)


>
>
> > 2. (no kernel compilation required) we can try to fix the report
> > descriptor of the device through HID-BPF. Assuming you have
> > CONFIG_HID_BPF enabled in your kernel, we can relatively easily change
> > the way the device is exported/handled by the kernel, to make it
> > useful hopefully
> > 3. (hardest IMO as you'll have to recompile your kernel for the tests)
> > we can try to tune hid-microsoft.c to properly export these buttons
> >
> > For 2 and 3, I'll need some events from your device with hid-recorder.
> > You only gave me the report descriptor, but no events which are hard
> > to deduce based on the long report descriptor.
>
> With the udev rule, I attached the hid-recorder output when pressing
> the paddles.  I see the 01 02 04 08 showing up at the end of the line.

Thanks for that. And as you can see, they are using the "Assign
Selection" usage, which is... new to me :)

>
>
> > Also for 2 and 3 we need to have a BTN_* button code to use, and I
> > don't know which ones should be used from the top of my head. HID-BPF
> > would be easiest to use as we can let the user decide of it, while
> > we'll need to have a more formal usage in case we fix hid-microsoft.
>
> When connected with USB, evemu-record shows button names
> "BTN_TRIGGER_HAPPY5" "BTN_TRIGGER_HAPPY6" "BTN_TRIGGER_HAPPY7"
> "BTN_TRIGGER_HAPPY8".  I assume these are the button codes we can use?
>
> ################################
> #      Waiting for events      #
> ################################
> E: 0.000001 0001 02c4 0001      # EV_KEY / BTN_TRIGGER_HAPPY5   1
> E: 0.000001 0000 0000 0000      # ------------ SYN_REPORT (0) ---------- +0ms
> E: 0.143950 0001 02c4 0000      # EV_KEY / BTN_TRIGGER_HAPPY5   0
> E: 0.143950 0000 0000 0000      # ------------ SYN_REPORT (0) ---------- +143ms
> E: 6.543984 0001 02c5 0001      # EV_KEY / BTN_TRIGGER_HAPPY6   1
> E: 6.543984 0000 0000 0000      # ------------ SYN_REPORT (0) ---------- +6400ms
> E: 6.615981 0001 02c5 0000      # EV_KEY / BTN_TRIGGER_HAPPY6   0
> E: 6.615981 0000 0000 0000      # ------------ SYN_REPORT (0) ---------- +72ms
> E: 7.520034 0001 02c6 0001      # EV_KEY / BTN_TRIGGER_HAPPY7   1
> E: 7.520034 0000 0000 0000      # ------------ SYN_REPORT (0) ---------- +905ms
> E: 7.648127 0001 02c6 0000      # EV_KEY / BTN_TRIGGER_HAPPY7   0
> E: 7.648127 0000 0000 0000      # ------------ SYN_REPORT (0) ---------- +128ms
> E: 8.344035 0001 02c7 0001      # EV_KEY / BTN_TRIGGER_HAPPY8   1
> E: 8.344035 0000 0000 0000      # ------------ SYN_REPORT (0) ---------- +696ms
> E: 8.480049 0001 02c7 0000      # EV_KEY / BTN_TRIGGER_HAPPY8   0
> E: 8.480049 0000 0000 0000      # ------------ SYN_REPORT (0) ---------- +136ms
>

Good point. Then we can easily tweak the device to use those BTN_* instead

>
> >
> > For 1, maybe SDL (or Steam) already ships some udev rules, and
> > submitting a fix there would make things working for everybody.
> >
> > Anyway, depending on how much you want this to be fixed and what you
> > can do (is CONFIG_HID_BPF enabled in your distro? and can you
> > recompile a kernel module?) we can figure out the next step.
>
> Yes HID-BPF is configured in my kernel:
>
> cat /proc/config.gz | gunzip
> #
> # HID-BPF support
> #
> CONFIG_HID_BPF=y
> # end of HID-BPF support

Even better, it'll be way easier for the tests :)

I've opened a new MR with your device fix:
https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/merge_requests/32

You should be able to head to the last job in latest pipeline (in the
pipeline tab) and download the artifact:
https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/jobs/54687597/artifacts/browse

Then unpack and run `sudo ./install --verbose`.

Then reconnect the device over Bluetooth, and see in evemu-record that
the paddles are now reporting BTN_TRIGGER_HAPPY[5-8].

I'd then be curious about 2 things:
- are the paddle now reporting the same events over USB and Bluetooth?
(is there no inversions between the BTN_TRIGGER*?, and are all the
buttons correctly handled?)
- does this mess up with SDL over hidraw (so keeping your udev rule in place)

>
> I've never recompiled a module before, but I've compiled lots of other
> stuff before.  How hard can it be?
>

Technically it's not hard, I'm sure there is a proper Arch howto
available. But it's painful because you'll have to maintain &
recompile your kernel until your fix gets in the standard kernel of
your distribution (archlinux if I'm not wrong).

But the good point is that with HID-BPF that pain is relieved:  I can
provide you with the binary, you can check it if you like (the loader
is in rust and the bpf itself is C), and then once installed you can
forget about it.

Cheers,
Benjamin






[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux