Set ff_effects_max correctly by subtracting FF_EFFECT_MIN.

[human9]

Hi, I believe there is a bug as far as setting ff_effects_max when creating ff-capable uinput devices.

Opening a controller device with fftest, I am told.. Number of simultaneous effects: 16. With python-evdev I create a uinput device that copies the exact capabilities of this device. For this device, fftest reports.. Number of simultaneous effects: 96. Off by 80.

In the kernel input.h, I can see FF_EFFECT_MIN and FF_RUMBLE is defined as 0x50 (80), and represents the offset to calculate the actual number of effects. So FF_EFFECT_MIN should be subtracted from FF_MAX_EFFECTS to set the value expected in ff_effects_max.

This pull request makes the described change in python-evdev's uinput.c. With the change, a copied device will report the correct number of simultaneous effects.

[sezanzeb]

Are "effects" something like rumbling controllers and the likes? Never heard of it to be honest, never worked with that

[human9]

Yes exactly, effects are specific types of force-feedback, take a look here for more info. To get a controller to rumble you have to "upload" effects to it, which python-evdev has the code for. You can only have so many effects uploaded simultaneously to a device. But there's a problem that means the virtual devices created by python-evdev report that they support a larger number of effects than the actual device they're based on, which this should fix.

[sezanzeb]

Thanks

For future reference, here is a docstring on ff_effects_max: https://github.com/torvalds/linux/blob/33cc938e65a98f1d29d0a18403dbbee050dcad9a/include/uapi/linux/uinput.h#L86

I can confirm that the "Number of simultaneous effects" is correct with this change.

original gamepad:

➜  ~ fftest /dev/input/event20
Force feedback test program.
HOLD FIRMLY YOUR WHEEL OR JOYSTICK TO PREVENT DAMAGES

Device /dev/input/event20 opened
Features:
  * Absolute axes: X, Y, Z, RX, RY, RZ, Hat 0 X, Hat 0 Y, 
    [3F 00 03 00 00 00 00 00 ]
  * Relative axes: 
    [00 00 ]
  * Force feedback effects types: Periodic, Rumble, Gain, 
    Force feedback periodic effects: Square, Triangle, Sine, 
    [00 00 00 00 00 00 00 00 00 00 03 07 01 00 00 00 ]
  * Number of simultaneous effects: 16

Setting master gain to 75% ... OK
Uploading effect #0 (Periodic sinusoidal) ... OK (id 0)
Uploading effect #1 (Constant) ... Error: Invalid argument
Uploading effect #2 (Spring) ... Error: Invalid argument
Uploading effect #3 (Damper) ... Error: Invalid argument
Uploading effect #4 (Strong rumble, with heavy motor) ... OK (id 1)
Uploading effect #5 (Weak rumble, with light motor) ... OK (id 2)
Enter effect number, -1 to exit

uinput:

➜  ~ fftest /dev/input/event21
Force feedback test program.
HOLD FIRMLY YOUR WHEEL OR JOYSTICK TO PREVENT DAMAGES

Device /dev/input/event21 opened
Features:
  * Absolute axes: X, Y, Z, RX, RY, RZ, Hat 0 X, Hat 0 Y, 
    [3F 00 03 00 00 00 00 00 ]
  * Relative axes: 
    [00 00 ]
  * Force feedback effects types: 
    Force feedback periodic effects: 
    [00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ]
  * Number of simultaneous effects: 16

Uploading effect #0 (Periodic sinusoidal) ... Error:: Invalid argument
Uploading effect #1 (Constant) ... Error: Invalid argument
Uploading effect #2 (Spring) ... Error: Invalid argument
Uploading effect #3 (Damper) ... Error: Invalid argument
Uploading effect #4 (Strong rumble, with heavy motor) ... Error: Invalid argument
Uploading effect #5 (Weak rumble, with light motor) ... Error: Invalid argument
Enter effect number, -1 to exit

Why does it say "Invalid argument" for 0, 4 and 5, is this a separate problem?

This is what I used to generate the uinput:

import evdev
import time

input_device = evdev.InputDevice("/dev/input/event20")
uinput = evdev.UInput.from_device(input_device)
time.sleep(9999)

[human9]

Glad you could reproduce the fix!

As far as the "Invalid argument" errors go, this is (mainly)? part of a separate issue. Few things regarding that:

1. Uinput.from_device specifically filters out EV_FF capabilities here, so you have to pass filtered_types=[e.EV_SYN] explicitly. This fixes the "invalid argument" problem.
2. If you actually try and test the rumble with the above modification, it will most likely freeze the program. You have to manually handle EV_UINPUT events, as discussed in the tutorial docs here.
3. Even if you use that exact code from the tutorial, you still won't have a functional rumble. The reason for this is documented in https://github.com/gvalkov/python-evdev/issues/199. I've just posted a code snippet over there you can use to test that should work. It may be worth discussing over there though, as I'd consider that a separate issue.

[sezanzeb]

Thanks, that worked, looks good to me

I hope you don't mind further questions. Someone has to review this, and I seriously never looked into that part of evdev despite being a maintainer here:

Why is there both FF_EFFECT_MAX and FF_MAX_EFFECTS in https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/input.h?

I'll merge it afterwards

[human9]

I certainly don't mind. It's a great idea in fact because that question has made me notice my reasoning is wrong!

FF_EFFECT_MIN and FF_EFFECT_MAX are about the available effect types, rather than the number of effects that can be uploaded. So actually the FF_MAX_EFFECTS - FF_EFFECT_MIN thing just works out of luck, because they aren't even talking about the same thing... They're also hard coded definitions so we'll always get 16. I think it just happens all the devices we've tested support 16 effects so it keeps working.

FF_MAX_EFFECTS is an upper bound on the number of effects, rather than for a particular device. We can see it how it's used here in the kernel. When creating a uinput device, python-evdev currently sets the maximum number of effects to FF_MAX_EFFECTS (here and here), which is probably fine as a default, but we need to be able to modify it.

There's a special way to query the number of effects a device really supports, and python-evdev actually did it all along! It's right in device.py, ff_effects_count. This is the actual maximum number of effects we want to have for our uinput device (so long as we're basing it on a real device), not FF_MAX_EFFECTS.

So, I would propose I change this pull request (or make a new one) to do the following:

1. Add a parameter to the UInput class in uinput.py, max_effects, that will default to FF_MAX_EFFECTS.
2. Set max_effects based on the target device's ff_effects_count when from_device is used.

Then we need to pass max_effects through to the C code. That'll involve:

3. Passing max_effects to the _uinput.setup function here
4. That comes out as uinput_setup in uinput.c in which ff_effects_max will need be set based on the new max_effects parameter.

Sorry, that's a lot of text. Let me know if that all sounds reasonable!

[human9]

During actually doing what I said I noticed that from_device can take multiple devices, therefore I can't implement point 2 above (unless I add a special case for len(devices) == 1). But the rest is implemented.

[sezanzeb]

So when I upload effects into the uinput, they end up in the controller even without using from_device.

What happens if I have multiple gamepads connected? I only have one here to try it out

[sezanzeb]

If the upload just goes into an arbitrary device, or all of them, why is the maximum number of simultaneous effects of the actual gamepad related to what is happening with the uinput? If evdev is making an arbitrary wild guess, then copying ff_effects_max of the gamepad to the uinput seems like an arbitrary wild guess as well.

[human9]

What happens if I have multiple gamepads connected?

Where the uploaded effects go is determined by what the user does during handling of the EV_UINPUT event types. If there are two gamepads a and b connected, a user could decide they only want to upload ff effects to a. In this case they should setup the uinput device with max_effects=a.ff_effects_count. If for some reason they wanted to upload effects to both at the same time they could take the minimum ff_effects_count of both to make sure both are supported.

If the upload just goes into an arbitrary device, or all of them, why is the maximum number of simultaneous effects of the actual gamepad related to what is happening with the uinput?

You're right that it's not necessarily related, but if you intend to forward effects uploaded to the uinput device to a real gamepad, the uinput device cannot report supporting more effects than the target gamepad. You do have to set it to something, which is currently FF_MAX_EFFECTS. I think if you use the from_device function, the ff_effects_count of that device would be a better guess, but still a guess. I haven't implemented that.

The best approach is to have the maximum simultaneous effects be a manually configurable parameter. That's what this pull request does implement now.

Now you can do this:

uinput = evdev.UInput.from_device(input_device, max_effects=number)

Or this:

uinput = evdev.UInput(capabilities, max_effects=number)

If you omit max_effects the default is FF_MAX_EFFECTS, same as it was before.

[sezanzeb]

a user could decide they only want to upload ff effects to a. In this case they should setup the uinput device with max_effects=a.ff_effects_count

This reads like ff_effects_count is treated like an id. Meaning that the uinput only uploads effects to the device with a matching ff_effects_count.

However, when I set a different max_effects number for my uinput, then effects are still uploaded to the controller. It's just that only a limited number of effects are available.

uinput = UInput({
    ecodes.EV_FF: [80, 81, 88, 89, 90, 96],
    ecodes.EV_KEY: [ecodes.KEY_A, ecodes.KEY_B]
}, max_effects=1)

Makes fftest go

Setting master gain to 75% ... OK
Uploading effect #0 (Periodic sinusoidal) ... OK (id 0)
Uploading effect #1 (Constant) ... Error: Invalid argument
Uploading effect #2 (Spring) ... Error: Invalid argument
Uploading effect #3 (Damper) ... Error: Invalid argument
Uploading effect #4 (Strong rumble, with heavy motor) ... Error: No space left on device
Uploading effect #5 (Weak rumble, with light motor) ... Error: No space left on device
Enter effect number, -1 to exit
0
Now Playing: Sine vibration
Enter effect number, -1 to exit

and effect #0 ends up in the gamepad, and it rumbles.

I wish I had a second gamepad to experiment with them...

[human9]

This reads like ff_effects_count is treated like an id. Meaning that the uinput only uploads effects to the device with a matching ff_effects_count.

Sorry, I didn't mean to make it read that way. It's definitely not an id. Handling of EV_UINPUT event types determines where effects are uploaded, and handling of EV_FF events determines what actually rumbles.

I have two controllers here, both with 16 maximum simultaneous effects, and only the one I actually forward these events to will rumble. Let me know if there's something specific you want me to test.

I say the same ff_effects_count should be used for practical reasons. As you found it'll still work if you setup with a lower max_effects, but you won't be able to upload all the effects you otherwise could. If you setup with a higher max_effects than the real device, programs might try to allocate more effects than is possible, which will probably cause a crash unless handled properly.

[sezanzeb]

So when I upload effects into the uinput, they end up in the controller even without using from_device.

Whoops I made a really stupid mistake there in my code. They of course end up in the device of input_device.upload_effect + input_device.write

All right, thanks.

If you setup with a higher max_effects than the real device, programs might try to allocate more effects than is possible, which will probably cause a crash unless handled properly.

It would crash with OSError: [Errno 28] No space left on device, and programs using the uinput need to read the ff_effects_count from it to limit themselves.

During actually doing what I said I noticed that from_device can take multiple devices, therefore I can't implement point 2 above (unless I add a special case for len(devices) == 1). But the rest is implemented.

I would be in favor of using the smallest number of ff_effects_count of all input_devices passed into from_device

[human9]

I would be in favor of using the smallest number of ff_effects_count of all input_devices passed into from_device

I also think that would make sense, have added a commit.

[sezanzeb]

Works, thank you :)

For the records, here is the script I used to test it:

import time
import asyncio
from evdev import ff, InputDevice, UInput, categorize, ecodes

def test_takes_the_min():
    uinput_1 = UInput(max_effects=5)
    uinput_2 = UInput(max_effects=10)
    uinput_3 = UInput()
    uinput_4 = UInput.from_device(
        uinput_1.device,
        uinput_2.device,
        uinput_3.device,
    )

    print(uinput_1.device.ff_effects_count)
    print(uinput_2.device.ff_effects_count)
    print(uinput_3.device.ff_effects_count)
    print(uinput_4.device.ff_effects_count)

    print(uinput_4.device.path)

test_takes_the_min()
time.sleep(50)

And then I went into fftest to check if the "Number of simultaneous effects" matches.

[sezanzeb]

Ready to merge?

[human9]

Yep, looks good to me!

[human9]

Actually maybe hold off a moment, I'll just add some documentation.

[human9]

Okay, all documented and ready for merge!

[sezanzeb]

Thank you