fix force feedback arithmetic and wheel input filtering - #211
Conversation
|
Thanks for the PR, appreciate it! I commented on a couple minor things, although really only #211 (comment) is a functional change, the other ones are kind of personal preference I guess. |
|
Thanks for the feedback. I addressed your commets with fixup commits for now. Before merge, I'll rebase and reorder the commits. |
kakra
changed the title
|
@Kimplul BTW: I have a few Proton commits pending (not related to your code, just informational): Games in Proton do not detect the wheel when cold plugged while they use the raw Gaming Input interfaces. There's a missing index reset in the interface. Also, for games using that interface, the force values seem to be sent inverted. I'm still in the process of testing those commits here before submitting them upstream to wine, so nothing to see yet. I just wanted to let you know because it looks like some issues mention exactly these bugs. |
|
I updated the initial PR text to reflect the current state of the code. |
+ Gain is documented and sent to the hardware as a value in the range 0 to 65535, but the module parameter and sysfs attribute accept wider integer values. Store gain as a u16 and parse sysfs writes with kstrtou16 so out-of-range values are rejected instead of being silently changed. + This constrains only the configured value. Calculations combining gain values still require a wider intermediate to cover the product of two full-range 16-bit values.
+ FF_GAIN and the driver gain both use the full 16-bit range. Their product can exceed INT_MAX. Signed overflow is undefined and happens for common settings such as 75 percent times 75 percent. Use a u32 intermediate for the scaling operation. + Pass the configured gain directly when initializing or updating the hardware. This also removes two redundant multiply-divide expressions which could overflow before cancelling out.
+ Direction scaling can produce 32768 for the valid input pair -32768 and a negative full-scale sine value. Storing the intermediate result in s16 wraps it before constant, ramp or periodic effects are converted. + Keep direction-scaled values in s32 until their destination range is known. Periodic effects cannot represent a magnitude above S16_MAX, so clamp the asymmetric endpoint after making the magnitude positive. This also prevents the following headroom calculation from using a negative wrapped magnitude.
+ HID derives fuzz and flat from the logical axis range. On the tested T300 this gives steering fuzz=255 and flat=4095. It filters small corrections and adds a large centered deadzone before applications apply their own steering model. + Flat is also centered for pedal axes, while their rest position is an endpoint. Fuzz without an endpoint deadzone can retain a small nonzero value after the hardware has returned to rest. Both behaviours are inappropriate for wheel and pedal input, and applications already provide the required steering and endpoint deadzones. + Override fuzz and flat for every available wheel and pedal axis in input_configured on devices handled by hid-tmff2, following the existing hid-universal-pidff precedent for high-resolution racing devices. Skip absent axes through the input device's absbit mask.
4a02f58 to
4c142e1
Compare
Cool! Please do ping me when you're sending the patches, I can relay the info to the relevant threads and see what happens. Thanks a lot for the helpful pointers as well. |
Noted, I've set a reminder to do that when the Valve PRs are created. |
|
I checked that my T300RS and T248 behaved properly and my nitpicks were taken care of, so I went ahead and merged. Thanks! |
2 participants
Summary
This series contains four independent correctness changes in force feedback and wheel input handling:
The commits are intentionally kept separate so each change remains bisectable and can be reviewed or reverted independently.
No gain or rotation-range defaults are changed.
Gain configuration and scaling
The configured driver gain is documented as an unsigned 16-bit value, but was stored and parsed as
int. Values outside0..65535were accepted and silently clamped later.The configured gain is now stored as
u16. Module-parameter and sysfs values outside the documented range are rejected as invalid user input.Both
FF_GAINand the driver gain use the full unsigned 16-bit range. Their product can exceedINT_MAX, so the existing signed multiplication invokes undefined behaviour for common configurations such as 75 percent application gain combined with 75 percent driver gain.Gain multiplication now uses a
u32intermediate, which can hold the product of two full-range 16-bit values. The initial and sysfs paths pass the already validated gain directly instead of using redundant multiply-divide expressions which could overflow before cancelling out.Signed effect scaling
The driver uses a symmetric fixed-point sine range of
-32767..32767. Scaling the valid input value-32768by-32767therefore produces32768.The old code stored that result in
s16before completing the effect conversion, wrapping it to-32768. At full scale this could reverse the force direction instead of producing the expected positive force.Direction-scaled values are now kept in
s32until the destination range is known. Periodic magnitudes are made positive in the wider type and clamped toS16_MAX, preventing both the endpoint wrap and the subsequent invalid headroom calculation.Constant and ramp effects retain their wider intermediates until their final conversion as well.
Input filtering
The generic HID input setup derives fuzz and flat values from the logical axis range. On the tested T300 this produces:
The steering values suppress and smooth small corrections before an application receives them. This is particularly noticeable when changing steering direction.
flat represents a centered deadzone, which is also unsuitable for pedals whose rest position is an endpoint. Pedal fuzz can suppress small changes near an endpoint, causing the reported value to remain slightly short of the hardware endpoint.
The input setup now sets fuzz=0 and flat=0 for every available wheel and pedal axis on devices handled by hid-tmff2. Missing axes are skipped through the input device's absbit mask.
This follows the existing hid-universal-pidff precedent of overriding generic HID filtering for high-resolution racing devices.
Related issues
Testing
Completed on a T300RS GT:
Damper and Friction effects were accepted without an observed error, although their individual contribution was subjectively subtle. Damper appeared to reduce wheel run-on, consistent with velocity-dependent damping.
Still pending before marking this PR ready:
Deferred hardware validation: