From e8de32ea5a0b7735f4f5fbf774e7c7dfe3c4951b Mon Sep 17 00:00:00 2001 From: mescon <5875228+mescon@users.noreply.github.com> Date: Mon, 14 Sep 2026 23:22:27 +0200 Subject: [PATCH] ffb(engine): band-limit the inertia estimate, let the levels pass 100 FF_INERTIA was fed this tick's velocity minus the last one, which on a quantised encoder is an impulse train: a rim turning smoothly reads as a burst of full-count accelerations for one tick and nothing the next, and that came through as grain (#89, where the author of another engine had met the same thing and suggested the cure). The estimate is now the gap between the velocity and a 50 ms one-pole chasing it. The gap of a first-order lag behind a ramp settles at slope times tau, so a steady acceleration reads exactly as before and the effect's scale is untouched, while a one-tick blip becomes a bump of a fiftieth the height that decays over tau. It runs every tick on the held velocity, so a wheel reporting every 2 ms is filtered at the same rate as one reporting every tick, and a stop after the hold decays instead of arriving as one tick of minus the velocity. Fixed point in 1/256ths so sub-count accelerations survive the division, with a snap to the velocity within one step so a parked wheel reads exactly zero. Five effect-math tests cover the steady state, the quantisation case, a step, rest and symmetry. spring_level, damper_level and friction_level accept up to 400 instead of 100, and inertia_level joins them. The engine's gains sit below what the firmware renders by amounts owners have measured (damper at 0.61 of the firmware on a G923 Xbox edition, #87; 2.25x damper and 4x spring on a G PRO, #89), and the cap left no way to try those numbers without a rebuild. The fields widen to u16; the summed force is clamped to the wire range after every effect, so a large level saturates. Defaults are unchanged. Builds clean against 7.2.4 with clang. --- CHANGELOG.md | 16 +++++ docs/SYSFS_API.md | 31 ++++++-- mainline/hid-logitech-hidpp.c | 96 +++++++++++++++++-------- mainline/hidpp_dd_effect_math.h | 40 +++++++++++ tests/effect-math/test_effect_math.c | 102 +++++++++++++++++++++++++++ 5 files changed, 250 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e3a2e3..b31ab7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,22 @@ the contract is "it works on RS50 and G Pro as listed here". ## Unreleased +**Inertia no longer renders the encoder's grain, and the per-class levels +reach past 100.** `FF_INERTIA` was fed the per-tick velocity difference, +which on a quantised encoder is an impulse train: a rim turning smoothly +read as a burst of full-count accelerations and came through as grain +(issue [#89](../../issues/89), where the author of TF4ALL had met the same +thing and suggested the cure). The estimate is now the gap between the +velocity and a 50 ms one-pole chasing it, which returns exactly the +acceleration under a steady one, so the effect's scale is unchanged, and +spreads a blip into a decaying bump instead of a spike. Covered by the +effect-math tests. The `spring_level`, `damper_level` and `friction_level` +scales accept up to 400 instead of 100, and `inertia_level` joins them, +because the engine's gains sit below the firmware's by amounts owners have +measured (0.61 for damper on a G923 Xbox edition, #87; 2.25x damper and 4x +spring on a G PRO, #89) and a cap of 100 left no way to try those numbers +without a rebuild. Defaults are unchanged. + **Every release asset is signed, and the release carries provenance.** The signing job only ever covered the Arch packages and their repository database, although the documentation said every asset; the Debian diff --git a/docs/SYSFS_API.md b/docs/SYSFS_API.md index 8e16bb2..426fa03 100644 --- a/docs/SYSFS_API.md +++ b/docs/SYSFS_API.md @@ -1274,15 +1274,34 @@ means games that write autocenter 0 before taking over force feedback correctly disable it for their session. Useful for desk-driving without a game, or as idle centring. -### spring_level / damper_level / friction_level +### spring_level / damper_level / friction_level / inertia_level **Access**: Read/Write -**Values**: `0` to `100` (percent), default `100` +**Values**: `0` to `400` (percent), default `100` Global output scales for the emulated `FF_SPRING` / `FF_DAMPER` / -`FF_FRICTION` effect classes, matching the new-lg4ff semantics: 100 = -effects play as the game commanded, lower values tame that effect -class across all games, 0 mutes it. `damper_level` scales DAMPER -effects from games; the wheel's own firmware damping is `wheel_damping`. +`FF_FRICTION` / `FF_INERTIA` effect classes, matching the new-lg4ff +semantics: 100 = effects play as the game commanded, lower values tame +that effect class across all games, 0 mutes it. `damper_level` scales +DAMPER effects from games; the wheel's own firmware damping is +`wheel_damping`. + +new-lg4ff stops at 100. This engine accepts up to 400 because its gains sit +below what the wheels' firmware renders for the same effect, by amounts that +differ per wheel: a G923 Xbox edition owner measured the engine's damper at +0.61 of the firmware's ([#87](../../issues/87)), and a G PRO owner runs +2.25x damper and 4x spring against theirs ([#89](../../issues/89)). Until +those measurements settle the defaults, the levels are how to try them: + +```bash +echo 225 > damper_level # the G PRO numbers from #89 +echo 400 > spring_level +``` + +The summed force is clamped to the motor's range after every effect is +added, so a large level saturates instead of wrapping. `inertia_level` has +no new-lg4ff counterpart; the engine's inertia estimate is band-limited +(a 50 ms one-pole on the velocity), so a steady acceleration reads as +itself and the encoder's quantisation no longer arrives as grain. --- diff --git a/mainline/hid-logitech-hidpp.c b/mainline/hid-logitech-hidpp.c index f43f19b..fdd7bc7 100644 --- a/mainline/hid-logitech-hidpp.c +++ b/mainline/hid-logitech-hidpp.c @@ -6034,14 +6034,20 @@ struct hidpp_dd_ff_data { */ u16 autocenter; /* - * Per-effect-class output scales, 0-100 percent, default 100 - * (the new-lg4ff / Oversteer convention: spring_level, - * damper_level, friction_level files). Applied to the emulated - * SPRING/DAMPER/FRICTION outputs in the effect tick. - */ - u8 spring_level; - u8 damper_level; - u8 friction_level; + * Per-effect-class output scales in percent, default 100 (the + * new-lg4ff / Oversteer convention: spring_level, damper_level, + * friction_level files, plus inertia_level here). Applied to the + * emulated SPRING/DAMPER/FRICTION/INERTIA outputs in the effect + * tick. Up to HIDPP_DD_LEVEL_MAX rather than 100: the engine's + * gains sit below what the firmware renders on the wheels measured + * so far (0.61 of it for damper on a G923 Xbox edition, issue #87; + * a G PRO owner runs 2.25x damper and 4x spring against theirs, + * issue #89), and a cap of 100 left no way to try those numbers. + */ + u16 spring_level; + u16 damper_level; + u16 friction_level; + u16 inertia_level; /* * True once interface 0 has delivered at least one input report. * Until then ff->wheel_pos is its kzalloc 0 ("hard left"), and @@ -6063,9 +6069,11 @@ struct hidpp_dd_ff_data { * input report handler at the wheel's native poll rate (roughly * 1 kHz for these wheels). The timer callback reads these lock-free * via READ_ONCE; writers use WRITE_ONCE. wheel_pos is raw encoder - * 0..65535 (0x8000 == centre). wheel_vel and wheel_accel are - * signed derivatives in encoder-counts per input sample, computed - * inside the FFB timer tick from successive wheel_pos samples. + * 0..65535 (0x8000 == centre). wheel_vel is the signed velocity + * in encoder counts per tick, from successive wheel_pos samples + * and the real time between them; wheel_accel is its band-limited + * derivative in 1/256ths (see hidpp_dd_accel_filter). Both are + * computed inside the FFB timer tick. */ u16 wheel_pos; /* latest raw encoder position, 0..65535 */ ktime_t wheel_pos_ts; /* when wheel_pos arrived; paired with it under wheel_pos_seq */ @@ -6084,7 +6092,8 @@ struct hidpp_dd_ff_data { u16 wheel_hold_ticks; /* ticks since wheel_pos last changed (see the timer) */ s32 wheel_vel; /* encoder counts per timer tick, held between position reports */ s32 wheel_vel_prev; - s32 wheel_accel; + s32 wheel_vel_slow_q8; /* the one-pole tracker behind wheel_accel, in 1/256 counts per tick */ + s32 wheel_accel; /* counts per tick per tick in 1/256ths, band-limited (hidpp_dd_accel_filter) */ bool wheel_state_primed; /* false until the timer has seen two samples */ /* * "any effect is currently playing" short-circuit. When false the @@ -6325,7 +6334,9 @@ static void hidpp_dd_ff_recompute_constant_force_locked(struct hidpp_dd_ff_data * FRICTION: condition formula fed by a saturated unit velocity * (±S16_MAX for any non-zero velocity, 0 otherwise). Produces * constant friction opposing motion direction. - * INERTIA: condition formula fed by wheel_accel. Opposes acceleration. + * INERTIA: condition formula fed by wheel_accel, the band-limited + * acceleration estimate (hidpp_dd_accel_filter). Opposes + * acceleration. */ /* * `sub_qms` is a time offset in quarter-milliseconds, added to elapsed_ms, @@ -6484,13 +6495,18 @@ static s32 hidpp_dd_ff_effect_tick(const struct hidpp_dd_ff_data *ff_state, case FF_INERTIA: /* * Acceleration is even smaller than velocity. Scale by - * 4096 so a quick hand-shake reaches saturation. INERTIA - * is rare in games; this is a reasonable default. Same - * multiplication-not-shift rule as DAMPER above. + * 4096 per count/tick^2 so a quick hand-shake reaches + * saturation; wheel_accel arrives in 1/256ths, so that is + * x16 here. INERTIA is rare in games; this is a reasonable + * default, and the band-limited estimate keeps it (a steady + * acceleration reads the same as before, only the spikes + * went). Same multiplication-not-shift rule as DAMPER above. */ c = &eff->u.condition[0]; + /* Global per-class scale (inertia_level). */ return hidpp_dd_condition_force(c, - clamp(wheel_accel * 4096, (s32)S16_MIN, (s32)S16_MAX)); + clamp(wheel_accel * 16, (s32)S16_MIN, (s32)S16_MAX)) * + READ_ONCE(ff_state->inertia_level) / 100; case FF_RAMP: { /* * Linear interpolation from start_level to end_level over @@ -6684,6 +6700,13 @@ static bool hidpp_dd_foreign_stream_active(struct hidpp_dd_ff_data *ff) * only once it is longer than any gap a moving wheel produces. */ #define HIDPP_DD_VEL_HOLD_TICKS 8 +/* + * Time constant of the acceleration estimate behind FF_INERTIA, in + * milliseconds (see hidpp_dd_accel_filter). 50 ms band-limits it to a + * few hertz, which is what a rim's inertia is about; the per-tick + * difference it replaces was an impulse train. + */ +#define HIDPP_DD_ACCEL_TAU_MS 50 static enum hrtimer_restart hidpp_dd_ff_effect_timer_callback(struct hrtimer *t) { @@ -6730,7 +6753,7 @@ static enum hrtimer_restart hidpp_dd_ff_effect_timer_callback(struct hrtimer *t) ff->wheel_hold_ticks = 0; ff->wheel_vel = 0; ff->wheel_vel_prev = 0; - ff->wheel_accel = 0; + ff->wheel_vel_slow_q8 = 0; ff->wheel_state_primed = true; } else if (cur_pos == ff->wheel_pos_prev) { /* @@ -6749,11 +6772,8 @@ static enum hrtimer_restart hidpp_dd_ff_effect_timer_callback(struct hrtimer *t) if (ff->wheel_hold_ticks < U16_MAX) ff->wheel_hold_ticks++; if (ff->wheel_hold_ticks >= HIDPP_DD_VEL_HOLD_TICKS) { - ff->wheel_accel = -ff->wheel_vel; ff->wheel_vel_prev = ff->wheel_vel; ff->wheel_vel = 0; - } else { - ff->wheel_accel = 0; } } else { s32 delta = (s32)(s16)(cur_pos - ff->wheel_pos_prev); @@ -6779,13 +6799,23 @@ static enum hrtimer_restart hidpp_dd_ff_effect_timer_callback(struct hrtimer *t) new_vel = delta / (s32)(ff->wheel_hold_ticks + 1); if (!new_vel) new_vel = delta > 0 ? 1 : -1; /* keep the direction */ - ff->wheel_accel = new_vel - ff->wheel_vel; ff->wheel_vel_prev = ff->wheel_vel; ff->wheel_vel = new_vel; ff->wheel_pos_prev = cur_pos; ff->wheel_pos_prev_ts = cur_ts; ff->wheel_hold_ticks = 0; } + /* + * Every tick, whichever branch ran: on a tick without a report the + * held velocity is the input, so a wheel that reports every 2 ms + * (the G923) is filtered at the same rate as one that reports every + * tick, and a stop after the hold decays over tau instead of + * arriving as one tick of -wheel_vel. + */ + ff->wheel_accel = hidpp_dd_accel_filter(&ff->wheel_vel_slow_q8, + ff->wheel_vel, + HIDPP_DD_FF_TICK_MS, + HIDPP_DD_ACCEL_TAU_MS); wheel_pos_signed = (s32)cur_pos - 0x8000; wheel_vel = ff->wheel_vel; wheel_accel = ff->wheel_accel; @@ -10473,13 +10503,18 @@ static struct device_attribute dev_attr_wheel_compat_autocenter = __ATTR(autocenter, 0664, wheel_autocenter_show, wheel_autocenter_store); /* - * Oversteer-compatible per-effect-class output scales, 0-100 percent, - * default 100 (the new-lg4ff convention): spring_level, damper_level, - * friction_level scale the emulated FF_SPRING / FF_DAMPER / - * FF_FRICTION outputs respectively. Note damper_level scales DAMPER - * EFFECTS from games; the wheel's own firmware damping stays on - * wheel_damping. + * Oversteer-compatible per-effect-class output scales in percent, default + * 100 (the new-lg4ff convention): spring_level, damper_level, + * friction_level and inertia_level scale the emulated FF_SPRING / + * FF_DAMPER / FF_FRICTION / FF_INERTIA outputs respectively. Note + * damper_level scales DAMPER EFFECTS from games; the wheel's own firmware + * damping stays on wheel_damping. new-lg4ff stops at 100; this engine + * accepts up to HIDPP_DD_LEVEL_MAX so that the gains owners have measured + * against the firmware's rendering (issues #87 and #89) can be tried + * without a rebuild. The summed force is clamped to the wire range after + * every effect is added, so a large level saturates rather than wraps. */ +#define HIDPP_DD_LEVEL_MAX 400 #define HIDPP_DD_LEVEL_ATTR(_name) \ static ssize_t wheel_##_name##_show(struct device *dev, \ struct device_attribute *attr, \ @@ -10517,7 +10552,7 @@ static ssize_t wheel_##_name##_store(struct device *dev, \ ret = kstrtoint(buf, 10, &val); \ if (ret) \ return ret; \ - WRITE_ONCE(ff->_name, (u8)clamp(val, 0, 100)); \ + WRITE_ONCE(ff->_name, (u16)clamp(val, 0, HIDPP_DD_LEVEL_MAX)); \ return count; \ } \ static struct device_attribute dev_attr_wheel_compat_##_name = \ @@ -10526,6 +10561,7 @@ static struct device_attribute dev_attr_wheel_compat_##_name = \ HIDPP_DD_LEVEL_ATTR(spring_level); HIDPP_DD_LEVEL_ATTR(friction_level); HIDPP_DD_LEVEL_ATTR(damper_level); +HIDPP_DD_LEVEL_ATTR(inertia_level); static ssize_t wheel_damping_show(struct device *dev, struct device_attribute *attr, char *buf) @@ -15540,6 +15576,7 @@ static struct attribute *hidpp_dd_wheel_group_attrs[] = { &dev_attr_wheel_compat_spring_level.attr, &dev_attr_wheel_compat_damper_level.attr, &dev_attr_wheel_compat_friction_level.attr, + &dev_attr_wheel_compat_inertia_level.attr, NULL, }; @@ -15749,6 +15786,7 @@ static int hidpp_dd_ff_init(struct hidpp_device *hidpp) ff->spring_level = 100; /* per-class scales: neutral */ ff->damper_level = 100; ff->friction_level = 100; + ff->inertia_level = 100; ff->texture_route = HIDPP_DD_TEXTURE_ROUTE_TF; ff->range_restore = true; ff->range_restore_attempts = 0; diff --git a/mainline/hidpp_dd_effect_math.h b/mainline/hidpp_dd_effect_math.h index aab20f8..47c8cee 100644 --- a/mainline/hidpp_dd_effect_math.h +++ b/mainline/hidpp_dd_effect_math.h @@ -174,6 +174,46 @@ static s32 hidpp_dd_condition_force(const struct ff_condition_effect *c, return force; } +/* + * Acceleration for FF_INERTIA, band-limited. + * + * The obvious estimate, this tick's velocity minus the last one, is an + * impulse train on a quantised encoder: the velocity moves in whole counts + * per millisecond, so a rim turning smoothly reads as a burst of +1/-1 + * steps, each a full count of "acceleration" for one tick and zero the + * next. Fed to INERTIA that came through as grain on the rim (issue #89, + * where the author of another engine had met exactly the same thing). + * + * Instead, chase the velocity with a slow one-pole and take the gap: + * + * v_slow += (v - v_slow) * dt / (tau + dt); a = (v - v_slow) / tau + * + * The gap of a first-order lag behind a ramp settles at slope * tau, so + * under a steady acceleration a0 this returns exactly a0 (the INERTIA + * scale needs no retuning), while a one-tick blip of dv is spread into a + * peak of dv / tau that decays over tau instead of a spike of dv. Band + * limited to a few hertz, which is all inertia wants. + * + * Fixed point: velocities in counts per millisecond, the tracker and the + * result in 1/256ths (q8) so that sub-count accelerations survive the + * division by tau. Once the tracker is within one step of the velocity it + * snaps to it, so a wheel at rest reads an acceleration of exactly zero + * instead of the residue an integer division leaves behind. + */ +static s32 hidpp_dd_accel_filter(s32 *vel_slow_q8, s32 vel, u32 dt_ms, + u32 tau_ms) +{ + s32 v_q8 = vel * 256; + s32 diff = v_q8 - *vel_slow_q8; + s32 span = (s32)(tau_ms + dt_ms); + + if (diff > -span && diff < span) + *vel_slow_q8 = v_q8; + else + *vel_slow_q8 += diff * (s32)dt_ms / span; + return (v_q8 - *vel_slow_q8) / (s32)tau_ms; +} + /* * The wire form of a signed force: offset binary around 0x8000, which is * what the stream's cur field and the KF packet both carry. Clamped to the diff --git a/tests/effect-math/test_effect_math.c b/tests/effect-math/test_effect_math.c index db5ac00..5da5212 100644 --- a/tests/effect-math/test_effect_math.c +++ b/tests/effect-math/test_effect_math.c @@ -129,6 +129,103 @@ static void test_anti_spring_is_not_dropped(void) CHECK(right == 100, "anti-spring pushes further right, clipped at +saturation (got %d)", right); } +/* ---- acceleration filter (FF_INERTIA) ---------------------------------- */ + +#define TAU 50 +#define TICK 1 + +static void test_steady_acceleration_reads_as_itself(void) +{ + /* + * The whole point of taking the gap behind a one-pole: under a + * constant acceleration a0 the lag settles at a0 * tau, so the + * estimate settles at a0 and the INERTIA scale needs no retuning. + * Ramp the velocity by 3 counts per tick and wait ten time + * constants; the result is in 1/256ths. + */ + s32 slow = 0, a = 0, vel = 0; + int t; + + for (t = 0; t < 10 * TAU; t++) { + vel += 3; + a = hidpp_dd_accel_filter(&slow, vel, TICK, TAU); + } + CHECK(a >= 3 * 256 - 8 && a <= 3 * 256, "steady 3 counts/tick^2 reads as ~768 q8 (got %d)", a); +} + +static void test_quantisation_blips_are_spread_not_spiked(void) +{ + /* + * A rim turning at half a count per tick reads on a quantised + * encoder as velocity 1, 0, 1, 0, ... The old per-tick difference + * made that +1, -1, +1, -1: a full count of "acceleration" every + * tick, 256 in these units, and grain on the rim. Band-limited, + * the estimate never leaves a small band around zero. + */ + s32 slow = 0, a, worst = 0; + int t; + + for (t = 0; t < 20 * TAU; t++) { + a = hidpp_dd_accel_filter(&slow, t & 1, TICK, TAU); + if (a > worst) + worst = a; + if (-a > worst) + worst = -a; + } + CHECK(worst <= 8, "alternating 1/0 velocity stays within 8 q8 of zero (worst %d, raw would be 256)", worst); +} + +static void test_a_velocity_step_peaks_at_step_over_tau_and_decays(void) +{ + /* + * A single step of dv shows up as a peak of dv / tau that decays + * over tau, instead of a one-tick spike of dv. Step from rest to + * 100 counts per tick and hold. + */ + s32 slow = 0, first, later, late; + int t; + + first = hidpp_dd_accel_filter(&slow, 100, TICK, TAU); + CHECK(first > 0 && first <= 100 * 256 / TAU, "peak is at most dv/tau = 512 q8 (got %d)", first); + for (t = 0; t < TAU; t++) + later = hidpp_dd_accel_filter(&slow, 100, TICK, TAU); + CHECK(later < first / 2, "one tau later it has decayed below half (got %d after %d)", later, first); + for (t = 0; t < 10 * TAU; t++) + late = hidpp_dd_accel_filter(&slow, 100, TICK, TAU); + CHECK(late == 0, "at a steady velocity the estimate returns to exactly zero (got %d)", late); +} + +static void test_rest_reads_exactly_zero(void) +{ + /* + * After a stop the tracker snaps onto the velocity once it is + * within a step of it, so a still wheel reads 0 and not the + * residue of an integer division: an INERTIA effect on a parked + * car must not hold a standing force. + */ + s32 slow = 0, a = 1; + int t; + + for (t = 0; t < 2 * TAU; t++) + hidpp_dd_accel_filter(&slow, 40, TICK, TAU); + for (t = 0; t < 10 * TAU; t++) + a = hidpp_dd_accel_filter(&slow, 0, TICK, TAU); + CHECK(a == 0 && slow == 0, "rest reads zero acceleration with the tracker parked (a %d, tracker %d)", a, slow); +} + +static void test_filter_is_sign_symmetric(void) +{ + s32 sp = 0, sn = 0, ap = 0, an = 0, vel = 0; + int t; + + for (t = 0; t < 5 * TAU; t++) { + vel += 2; + ap = hidpp_dd_accel_filter(&sp, vel, TICK, TAU); + an = hidpp_dd_accel_filter(&sn, -vel, TICK, TAU); + } + CHECK(ap == -an, "turning left mirrors turning right (%d vs %d)", ap, an); +} + /* ---- wire mapping ------------------------------------------------------ */ static void test_wire_mapping_is_offset_binary_and_saturates(void) @@ -150,6 +247,11 @@ int main(void) test_deadband_is_a_dead_zone(); test_saturation_clips_both_signs(); test_anti_spring_is_not_dropped(); + test_steady_acceleration_reads_as_itself(); + test_quantisation_blips_are_spread_not_spiked(); + test_a_velocity_step_peaks_at_step_over_tau_and_decays(); + test_rest_reads_exactly_zero(); + test_filter_is_sign_symmetric(); test_wire_mapping_is_offset_binary_and_saturates(); if (failures) { fprintf(stderr, "%d failure(s)\n", failures);