atar-axis / xpadneo

Advanced Linux Driver for Xbox One Wireless Controller (shipped with Xbox One S)
https://atar-axis.github.io/xpadneo/
GNU General Public License v3.0
1.87k stars 110 forks source link

Implement proper Impulse Triggers for Xbox controllers. #374

Open Giovani1906 opened 1 year ago

Giovani1906 commented 1 year ago

Version of xpadneo

v0.9.4

Controller Model

Connection mode

Describe your feature request

Is your feature request related to a problem? Please describe. I'd like Xbox controllers to have proper Impulse Triggers rather than what currently is done which just a synthesis of the effect. The current synthesized effect is inaccurate and the vibrations seem too strong at times.

Describe the solution you'd like I would like that Impulse Triggers would be implemented using the following kernel patches:

Describe alternatives you've considered Currently there are no alternatives that I have heard of.

Additional context The maintainer kakra has asked me to make an issue in other for them to keep track of it.

kakra commented 1 year ago

The current synthesized effect is inaccurate and the vibrations seem too strong at times.

You can reduce the amount of vibration effect using rumble_attenuation=0,50 as a module parameter to reduce the rumble effect for the main rumble by 0% and for the trigger rumble by 50%. This is not a solution to the problem but it may improve your experience until proper support is in place.

The Arch wiki has information on how to use module parameters and set them on boot: https://wiki.archlinux.org/title/Kernel_module

adamdmoss commented 1 month ago

Is the trigger rumble strength supposed to be proportional to the body rumble strength? I have an issue(?) where I can scale the rumble strength in a game's settings (Saints Row in this case) and that appears to work for the body rumble, but the trigger rumble appears to remain full strength.

kakra commented 1 month ago

The trigger rumble adjusts to the strength of the main rumble. But the trigger rumble motors are pretty intense, you can scale them down with the example from https://github.com/atar-axis/xpadneo/issues/374#issuecomment-1200894085.

So yes, the trigger rumble strength is proportional to the main rumble.

The formula goes like this:

The game sends rumble strength for the weak (W) and strong (S) motor - if you made it weaker within the game, these values will be lower. We take the maximum of both values for our main (M) rumble strength: M = max(W,S).

Now we attenuate the W value by the parameter given to the module (a0), and we do the same for the S value. Both new values will be send to the rumble motors: (Wa,Sa) = (W,S) * (100-a0)%.

Now we use the M value to calculate the trigger motor strength by multiplying the trigger pressure with value M, and additionally attenuate by either a0, or attenuate by a0%*a1%, if the second parameter value is given.

So attenuation 0,50 lowers the main motors by 0%, and the trigger motors by 50%. If you'd use 50,50, it would effectively lower the main motors by 50%, and the trigger motors by an additional 50% (thus 75% effectively).

I hope this explains it well enough. I've double-checked the code, it should work correctly. But feel free to check yourself, the code starts here:

adamdmoss commented 1 month ago

Thanks for the explanation! So it sounds like it's just a lot stronger than I'd expect by default even when the app thinks it's scaled way down, but this is expected. :)

kakra commented 1 month ago

Thanks for the explanation! So it sounds like it's just a lot stronger than I'd expect by default even when the app thinks it's scaled way down, but this is expected. :)

The problem is: The app doesn't know anything about the trigger rumble - we synthesize the effect from existing main rumble effects. Maybe we should attenuate it down a whole lot by default. The trigger rumble motors can be really really pronounced, especially at some different pressures you apply to them while they rumble: There's kind of some resonance effect then.

Giovani1906 commented 2 weeks ago

I've managed to make trigger rumble/impulse triggers work by using the patch below and having SDL patched for support as well.

diff --git a/hid-xpadneo/src/hid-xpadneo.c b/hid-xpadneo/src/hid-xpadneo.c
index 1a25631..8660864 100644
--- a/hid-xpadneo/src/hid-xpadneo.c
+++ b/hid-xpadneo/src/hid-xpadneo.c
@@ -290,7 +290,7 @@ static int xpadneo_ff_play(struct input_dev *dev, void *data, struct ff_effect *
    unsigned long flags, ff_run_at, ff_throttle_until;
    long delay_work;
    int fraction_TL, fraction_TR, fraction_MAIN, percent_TRIGGERS, percent_MAIN;
-   s32 weak, strong, direction, max_main;
+   s32 weak, strong, trigger_left, trigger_right, direction;

    struct hid_device *hdev = input_get_drvdata(dev);
    struct xpadneo_devdata *xdata = hid_get_drvdata(hdev);
@@ -301,6 +301,8 @@ static int xpadneo_ff_play(struct input_dev *dev, void *data, struct ff_effect *
    /* copy data from effect structure at the very beginning */
    weak = effect->u.rumble.weak_magnitude;
    strong = effect->u.rumble.strong_magnitude;
+   trigger_left = effect->u.rumble.trigger_left;
+   trigger_right = effect->u.rumble.trigger_right;

    /* calculate the rumble attenuation */
    percent_MAIN = 100 - param_rumble_attenuation[0];
@@ -308,7 +310,6 @@ static int xpadneo_ff_play(struct input_dev *dev, void *data, struct ff_effect *

    percent_TRIGGERS = 100 - param_rumble_attenuation[1];
    percent_TRIGGERS = clamp(percent_TRIGGERS, 0, 100);
-   percent_TRIGGERS = percent_TRIGGERS * percent_MAIN / 100;

    switch (param_trigger_rumble_mode) {
    case PARAM_TRIGGER_RUMBLE_DIRECTIONAL:
@@ -378,12 +379,6 @@ static int xpadneo_ff_play(struct input_dev *dev, void *data, struct ff_effect *
        break;
    }

-   /*
-    * we want to keep the rumbling at the triggers at the maximum
-    * of the weak and strong main rumble
-    */
-   max_main = max(weak, strong);
-
    spin_lock_irqsave(&xdata->ff_lock, flags);

    /* calculate the physical magnitudes, scale from 16 bit to 0..100 */
@@ -394,9 +389,9 @@ static int xpadneo_ff_play(struct input_dev *dev, void *data, struct ff_effect *

    /* calculate the physical magnitudes, scale from 16 bit to 0..100 */
    update_magnitude(xdata->ff.magnitude_left,
-            (u8)((max_main * fraction_TL + S16_MAX) / U16_MAX));
+            (u8)((trigger_left * fraction_TL + S16_MAX) / U16_MAX));
    update_magnitude(xdata->ff.magnitude_right,
-            (u8)((max_main * fraction_TR + S16_MAX) / U16_MAX));
+            (u8)((trigger_right * fraction_TR + S16_MAX) / U16_MAX));

    /* synchronize: is our worker still scheduled? */
    if (xdata->ff_scheduled) {

The SDL2 patch can be found at CachyOS/CachyOS-PKGBUILDS#247. It contains a Linux headers patch as well, otherwise SDL2 won't compile properly. I've also made a usable kernel patch file at CachyOS/kernel-patches#64.

kakra commented 2 weeks ago

Please create a PR for this patch: It will have some issues we need to address.