Open kitten opened 1 week ago
Another follow-up, for my mice this doesn't seem to matter (as far as I can observe at least), but I've got some additional changes here in case we want to be more strict about SYN_REPORT
handling: https://github.com/kitten/YeetMouse/compare/feat/input-handler-overrides...kitten:YeetMouse:experiment/syn-handling?expand=0
This splits the batch of events into at most two parts. While it still applies the accelerated REL_X
and REL_Y
events to all values it receives, the calculation is based on all prior x/y values (before SYN_REPORT
) and all the ones after are stored for the next events
run.
So, this is technically more correct, but might not be necessary in a lot of cases.
Edit: I've tested this out and since it's not causing issues and is more correct, I'm picking these changes over to this branch
Hey, I tried to run this PR, and I had some issues with the udev rules (or the lack of them). I know they should not be necessary, but I added all the udev stuff back in and it worked. The most important change I think is that there is actually no Leetmouse driver, at least not in the /sys/bus/usb/drivers/
directory. This kind of makes binding devices through the GUI unusable, as there is nothing to bind to.
Also, I had one small issue with the code. This is the declaration of usb_mouse_events
:
https://github.com/kitten/YeetMouse/blob/d3f8f081e0b65e61b38f569758a0821fbb0cecd0/driver/usbmouse.c#L58.diff
static unsigned int usb_mouse_events(struct input_handle *handle,
struct input_value *vals, unsigned int count)
while for me, the correct signature of the input_handler
's event
function is:
static void usb_mouse_events(struct input_handle *handle,
const struct input_value *vals, unsigned int count)
the return type is void, and input_value
's qualifier is constant. This is most likely a caused by different kernel version or something like this.
_(looking at the older kernel versions, it seems the same as mine though)_
After this change the code compiled smoothly.
Other than that, it looks great, I'll look a bit more into the code, and try to remove the udev
dependencies (this might be causes, by the fact that I'm building everything using the install.sh
and makefile scripts, instead of the NixOS files).
This kind of makes binding devices through the GUI unusable, as there is nothing to bind to.
Yup, that's by design for now and I've basically remove the driver entirely, as you've noticed here :v: If this works well though and we're happy to go ahead with this, what I'd suggest for implementation next is:
udev
rules,This should allow us to seamlessly bind devices to the driver with the GUI, while still having acceleration applied to all other devices (or if no devices are bound of course)
while for me, the correct signature of the input_handler's event function is:
That's pretty odd! I've checked this against the latest kernel code, and this is a public API that shouldn't have changed. Odd! Basically, the return value is the modified length of events, so it definitly shouldn't be missing. I'll look into that.
Other than that, it looks great, I'll look a bit more into the code, and try to remove the udev dependencies (this might be causes, by the fact that I'm building everything using the install.sh and makefile scripts
Nice! I suppose if we want to have both a driver and this input handler approach in parallel, then if I build off of this, we'd be fine?
Ah, alright, https://github.com/torvalds/linux/commit/14498e993fb77adce75f0106162902b2f8b1d480 I'll have to see what the alternative was before. The easiest way to fix this is to probably replace removed events with noops in older kernel versions
I think something like this will work too:
#if LINUX_VERSION_CODE <= KERNEL_VERSION(6,12,3)
static void usb_mouse_events(struct input_handle *handle, const struct input_value *vals, unsigned int count)
#else
static unsigned int usb_mouse_events(struct input_handle *handle, struct input_value *vals, unsigned int count)
#endif
For me it uses the top (void with const version, so it's correct). I'm using version 6.8
Also, I think that udev_trigger
should be removed from this line:
https://github.com/AndyFilter/YeetMouse/blob/ba430f6da3f53b318505590e732cbf0c6031d315/install_files/dkms/dkms.conf#L4
It caused errors for me, because there is no "udev_trigger" in the makefile. I tried suggesting this change for this PR, but it seems as though I can only edit the files that were already altered in some way.
@AndyFilter: Sorry π
Was having some issues getting the last version to compile on both kernel versions, but I just made some adjustments and it's working fine for me now. I'm choosing not to handle the const
warnings. They should be fine and not affect anything though (?). As long as it's non-const on the newer kernel, the event dropping should still work.
It think the const's are fine either way. Current version builds for me just fine (using the install.sh
script), but it doesn't install the driver. I have to manually do make driver
, cd driver/
, sudo insmod leetmouse.ko
. Which was not the case in the original version. This obviously can be automated, but it just makes me wonder why it didn't load the driver on it's own.
Supersedes #2 (A prior draft of this PR)
Problem
Currently, the kernel module is a full driver, based on the boot-mode USB mouse driver in Linux. It then uses a slim USB HID descriptor parser (
util.c
) and creates a new driver to bind input devices to.However, Linux contains a lot of modifications and custom drivers for input devices as well, and there's quite a long list of problematic devices that misreport events or HID descriptors, which are patched upstream in the kernel.
These patches are only available to the built-in
usbhid
driver though.Likely, a combination of the USB HID descriptor parser being very slim and sacrificing accuracy for brevity, and missing device patches and workarounds, means that a lot of mice that'd usually work just don't π
While it's possible to improve the USB HID descriptor parser, it's also possible to find a new approach, with the goal of intercepting events and modifying them while still letting the
usbhid
driver handle the device. (This also has the benefit that it leaves outputs intact, i.e. on-device configuration would still work)Halfway Solution: Input handlers and virtual devices
The
maccel
project, which essentially started as yet another fork ofleetmouse
(but has diverged quite a lot), has a solution to this that I've seen replicated in a few other places. This approach uses regularinput_handler
s in the Linux Input subsystem implementation to filter outEV_REL
events and to replace them by sending them to a virtual input device.This is close to what we want but then comes with some drawbacks:
yeetmouse
projectFull(?) Solution: Intercepting and modifying events
input_handlers
are very powerful and theirfilter
callback concept is essentially just a template that uses the underlyingevents
API to omit events dynamically. However, this can also be used to modify events.The new driver in this PR intercepts events, collects them, passes them to the
accelerate()
function, then modifies the original events. If we make sure that ourinput_handler
is registered at the front of the list of all handlers (which the kernel'sinput_register_handle()
function usually only does for filters, i.e. handlers with->filter
set rather than->events
), then we're able to intercept and mutate the events as we please.Further, we can also reach into the device and update the original events in a similar way, which makes sure that there's no residual data of the unaccelerated input (fwiw, I don't know if that's important, but who knows)
In theory this now means:
usbhid
's handling of devices, sub-drivers, and quirks which should blanket-support every device that works out of the box with LinuxSet of changes
module_usb_driver
module implementation with amodule_init
/module_exit
moduleinput_handler
receivingevents
and updating themutil.c
USB HID descriptor parser codeudev
installation/code/etctemp
because this PR still requires testing, and I'm assuming we might want to work on a hybrid driver, that can function using bothinput_handler
s and as a driver to bind a device to.