-
Notifications
You must be signed in to change notification settings - Fork 108
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
Set ff_effects_max correctly by subtracting FF_EFFECT_MIN. #198
Conversation
Are "effects" something like rumbling controllers and the likes? Never heard of it to be honest, never worked with that |
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. |
Thanks For future reference, here is a docstring on I can confirm that the "Number of simultaneous effects" is correct with this change. original gamepad:
uinput:
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) |
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:
|
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 I'll merge it afterwards |
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, So, I would propose I change this pull request (or make a new one) to do the following:
Then we need to pass max_effects through to the C code. That'll involve:
Sorry, that's a lot of text. Let me know if that all sounds reasonable! |
The default value will be set to FF_MAX_EFFECTS, so if forwarding events to a real device, pass max_effects=device.ff_effects_count
Adjusts PyArg_ParseTuple as required. See definition of ff_effects_max as u32 in include/uapi/linux/uinput.h
During actually doing what I said I noticed that |
So when I upload effects into the uinput, they end up in the controller even without using What happens if I have multiple gamepads connected? I only have one here to try it out |
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 |
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
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 |
This reads like However, when I set a different 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
and effect #0 ends up in the gamepad, and it rumbles. I wish I had a second gamepad to experiment with them... |
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 |
Whoops I made a really stupid mistake there in my code. They of course end up in the device of All right, thanks.
It would crash with
I would be in favor of using the smallest number of |
I also think that would make sense, have added a commit. |
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. |
Ready to merge? |
Yep, looks good to me! |
Actually maybe hold off a moment, I'll just add some documentation. |
Okay, all documented and ready for merge! |
Thank you |
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
andFF_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.