Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invert ABS values when min greater than max #21

Closed

Conversation

puffnfresh
Copy link

uinput runs validation on event definitions to ensure min is less than max. This means uinput can't clone devices like the "ADC attached joystick" driver which says:

If min > max, it is left to userspace to treat the axis as inverted.

https://www.kernel.org/doc/Documentation/devicetree/bindings/input/adc-joystick.yaml

So we need something which inverts everything in user-space. That's what this commit does.

uinput runs validation on event definitions to ensure min is less than
max. This means uinput can't clone devices like the "ADC attached
joystick" driver which says:

> If min > max, it is left to userspace to treat the axis as inverted.

https://www.kernel.org/doc/Documentation/devicetree/bindings/input/adc-joystick.yaml

So we need something which inverts everything in user-space. That's
what this commit does.
@KarsMulder
Copy link
Owner

Thank you for the report. I did not expect it to be possible that some device's axis would have a smaller minimum than maximum value, but here we are.

Currently quite some capability-propagation logic has been assuming that the invariant max >= min would be upheld. For example, if some device dev1 emits abs:x events in the range 1~5 and some device dev2 emits abs:x event in the range 3~8, then the script --input /dev/input/by-id/dev1 --input /dev/input/by-id/dev2 --output should generate a device that has capabilities in the range 1~8. The code responsible for this does not function correctly if one of those ranges would have min > max.

Consequently, the pull request you submitted does not fix the whole issue yet: it happens to work in simple situations where events are passed verbatim like --input PATH --output, but it could arbitrarily break when transformations are applied, because the capability propagation logic assumes that max >= min.

This means that either the capability propagation needs to be revisited to consider how to deal with inverted ranges, or that this issue should be tackled at the InputDevice level instead of the OutputDevice level.

Since I am not sure if it is even possible to deal with inverted ranges in the general case (e.g. what should happen if the events of devices with ranges 5~1 and 3~8 get merged? Does it become 1~8 with the inversion forgotten about?), I would prefer the latter approach (handling this at the InputDevice level.)

I added commit 13ad710 to simply interpret inverted ranges like 5~1 as if they were 1~5. This only solves half of the issue so far.

It does not yet invert the events that were generated by said device. What is not clear so far is if it would be correct to do so: we have documentation of one linux kernel device driver which says that userspace is expected to interpret malformed ranges as if the event codes were inverted, but does that hold for all device drivers that can output malformed ranges?

If all device drivers that output malformed ranges expect their events to be interpreted as if they were inverted, then it would make sense for InputDevice to automatically invert such events. However, if it is only a single driver which expects their events to be inverted, and some other drivers also output malformed ranges without expecting their events to be inverted, then it would be incorrect for InputDevice to automatically invert those events; it could be pretty confusing for the user if evsieve were to claim to read other events from input devices than evtest for no apparent reason. In that case it should be left to the user to provide a map that inverts those events.

(Relatedly: I am aware that there is currently no map that inverts the events in a range. I am now working on that now.)

Do you know whether it is kernel convention that an axis with a bigger minimum than maximum means that the userspace is intended to treat the received events as inverted?

@puffnfresh
Copy link
Author

@KarsMulder I've had a quick look through the kernel source and I can't easily see another case where the range is documented that it's allowed to be backwards. I do see a few places where the values are passed in straight from the device tree (e.g. touchscreen-min-x) but these have things like the touchscreen-invert-x property which does the inverting in the kernel.

I also had a look at the ADC joystick driver's uses and there's nothing in-tree today which has the range inverted. Only thing I can find is my my out-of-tree device,

Maybe this is just a weird case and we should just live with an inverting event mapper.

@KarsMulder
Copy link
Owner

I suppose that magically inverting some events without being certain that doing so is the right thing, might end up causing more trouble than it solves. If at this point the only known source of min>max situations is a single out-of-tree driver, then I suppose I may hold off on adding automatic inversion of the axes.

In the meanwhile, it is now possible to manually revert axes in userspace using affine maps that are have been added to the main branch (issue #17), e.g.

--map abs:x abs:x:127-x \
--map abs:y abs:y:127-x \

If the range of abs:x and abs:y were to be 127~0. The x represents whatever the original value of the event that gets mapped was.

@puffnfresh
Copy link
Author

The ADC joystick driver is in the mainline Linux kernel but the weird uses of it (e.g. DTS files) are out of tree today. Even if the device configurations were merged (hopefully one day soon) I agree that it's not enough evidence that this should be handled specially within evsieve.

The range for me is -127..127 so abs:x:0-x should work great, I think.

@puffnfresh puffnfresh closed this Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants