Compare commits

...
Author SHA1 Message Date
enricobuehler ec4bf75a6e fix(core/rumble): the Deck's keepalive stops being swallowed by its own renewals
ci / docs-site (pull_request) Successful in 1m14s
apple / swift (pull_request) Successful in 1m28s
apple / screenshots (pull_request) Skipped
ci / web (pull_request) Successful in 2m54s
ci / rust-arm64 (pull_request) Successful in 4m8s
windows / build (x86_64-pc-windows-msvc) (pull_request) Successful in 4m46s
android / android (pull_request) Successful in 5m31s
windows / build (aarch64-pc-windows-msvc) (pull_request) Successful in 4m11s
ci / rust (pull_request) Successful in 9m39s
Three faults in the shared rumble policy engine, all answered by one change of
shape: the free-running jitter phase becomes `last_emit` — the exact value last
handed to an embedder — and every emit routes through one helper. That single
field answers all three live questions: would re-sending this be a no-op device
write, is this stop redundant, and would the nudge invent a stop.

The Steam Deck declares a 40 ms keepalive with a 1-LSB nudge, because an
SDL-class layer discards a write identical to the last one. But the nudge lived
only in the keepalive branch, so every host renewal re-emitted the raw level,
collided with the last jittered write, was discarded, AND re-anchored the
keepalive timer. The gap between distinct device writes stretched to 80 ms at
the 400 ms default TTL and 100 ms at the hatch floor — two to two and a half
times the cadence the quirk exists to guarantee. Nudging on any repeat closes
it: 40 ms throughout.

Level (1, 0) turned that nudge into (0, 0) — the value the engine reserves for
"stop now" — and handed it out with a non-zero backstop, under a live lease.
It is the only such level: high must already be zero, and low ^ 1 == 0 implies
low == 1. The nudge now steps the LSB up instead, so the phase still alternates
and no stop is ever invented.

A zero for a pad the engine already believes silent is now dropped. Under the
legacy hatch the host re-sends zeros for every latched pad every 500 ms for the
rest of the session, which cost Android an unconditional log line and a binder
cancel() at 2 Hz per pad. The deliberate stop-burst heal is untouched, because
a stop that was LOST leaves the pad buzzing, and that is exactly the guard's
pass condition.

The client also now bounds the lease it will honour. RUMBLE_TTL_CEIL_MS is
sender-side only, so a modified or third-party host could stamp a long TTL and
wedge its pump, leaving Apple — whose renderer deliberately keeps no staleness
policy of its own — and a Deck slot buzzing for all of it.

Every new test was proven to fail with its own fix reverted, including the two
that guard against over-reach: a default-quirks pad must still get the level
verbatim, or an off-by-one amplitude would land in Apple's identical-target
comparison and Android's one-shots.

One suspicion from the audit did NOT survive: a v2 envelope carrying ttl_ms 0
cannot take the legacy backstop, because the expiry check preempts the relay
branch. No fix; pinned with a test so that ordering stays load-bearing.

Verified: 17/17 rumble tests, clippy --all-targets --features quic -D warnings
= 0, fmt clean, generated C header unchanged. (`c_abi_harness_round_trips`
fails on this Mac with a linker error, identically on an unmodified tree.)

From the 2026-08-03 force-feedback sweep (B12, B22, R9, T1).
2026-08-04 07:40:19 +02:00
+219 -28
View File
@@ -36,6 +36,22 @@ pub const LEGACY_STALE_MS: u64 = 1000;
/// engine's staleness zero lands at 1 s; this is the hardware-level net under an engine stall). /// engine's staleness zero lands at 1 s; this is the hardware-level net under an engine stall).
const BACKSTOP_LEGACY_MS: u32 = 2000; const BACKSTOP_LEGACY_MS: u32 = 2000;
/// The longest lease the engine honours, whatever the envelope claims — the receiver-side mirror of
/// the host's own `RUMBLE_TTL_CEIL_MS`.
///
/// No host built from this tree can exceed it (the `PUNKTFUNK_RUMBLE_TTL_MS` hatch is clamped to
/// `[150, 5000]` before it reaches the wire), so this is defence in depth against a third-party or
/// modified sender that stamps a long TTL and then wedges its renewal pump while the connection
/// stays up. It matters on exactly the platforms that sustain a level for the whole lease: Apple,
/// whose renderer deliberately keeps no staleness policy of its own, and a Deck slot, whose
/// keepalive re-kicks the actuator until the lease ends. Duration-parameterized embedders (SDL,
/// Android) already self-terminate at the clamped backstop.
///
/// Deliberately NOT `pub`: an embedder has no use for it, and every `pub` const in this crate is
/// emitted into `include/punktfunk_core.h` as an UNPREFIXED `#define` — a collision hazard the
/// header already has ~170 instances of, and one this has no reason to add to.
const MAX_LEASE_MS: u16 = 5_000;
/// One effective actuator command. `(0, 0)` means stop now. `backstop_ms` is a safety-net /// One effective actuator command. `(0, 0)` means stop now. `backstop_ms` is a safety-net
/// duration for platform APIs that take one (SDL rumble, Android one-shots): the engine emits /// duration for platform APIs that take one (SDL rumble, Android one-shots): the engine emits
/// explicit zeros at every policy stop, so the backstop only matters if the embedder thread itself /// explicit zeros at every policy stop, so the backstop only matters if the embedder thread itself
@@ -75,8 +91,11 @@ struct PadState {
/// A wire update landed since the last emit (level change OR renewal — renewals re-emit). /// A wire update landed since the last emit (level change OR renewal — renewals re-emit).
dirty: bool, dirty: bool,
next_keepalive: Option<Instant>, next_keepalive: Option<Instant>,
/// Current jitter phase (see [`ActuatorQuirks::dedup_jitter`]). /// The exact value last handed to an embedder. `(0, 0)` ⇔ the engine believes this actuator is
jitter: bool, /// silent. It replaces a free-running jitter phase because one field answers all three live
/// questions: would re-sending this be a no-op device write (the dedupe nudge), is a stop
/// redundant, and would the nudge synthesize the reserved stop.
last_emit: (u16, u16),
quirks: ActuatorQuirks, quirks: ActuatorQuirks,
} }
@@ -88,7 +107,7 @@ impl PadState {
legacy_wire: None, legacy_wire: None,
dirty: false, dirty: false,
next_keepalive: None, next_keepalive: None,
jitter: false, last_emit: (0, 0),
quirks: ActuatorQuirks { quirks: ActuatorQuirks {
keepalive_ms: 0, keepalive_ms: 0,
min_pulse_ms: 0, min_pulse_ms: 0,
@@ -112,6 +131,7 @@ impl PadState {
self.legacy_wire = None; self.legacy_wire = None;
self.next_keepalive = None; self.next_keepalive = None;
self.dirty = false; self.dirty = false;
self.last_emit = (0, 0);
RumbleCommand { RumbleCommand {
pad, pad,
low: 0, low: 0,
@@ -119,6 +139,40 @@ impl PadState {
backstop_ms: 0, backstop_ms: 0,
} }
} }
/// Build the command for the pad's current level, and record what we handed out.
///
/// On a `dedup_jitter` actuator, re-emitting the value the device last took is a no-op write on
/// an SDL-class layer, so the low motor's LSB is nudged. Keying that on `last_emit` rather than
/// on a free-running phase is what makes it work on EVERY emit path. Previously the nudge lived
/// only in the keepalive branch, so a host renewal — which arrives every `ttl*3/10` ms, 120 ms
/// at the 400 ms default and 60 ms at the hatch floor — re-emitted the raw level, collided with
/// the last jittered write, was swallowed, AND re-anchored the keepalive. That stretched the
/// gap between *distinct* device writes to 80 ms at the default cadence and 100 ms at the
/// floor, on an actuator whose quirk declares 40.
///
/// The nudge is refused when it would synthesize the reserved `(0, 0)` stop. That is level
/// `(1, 0)` and only that: `high` must already be 0, and `low ^ 1 == 0` implies `low == 1`.
/// There the LSB steps up instead, so the phase still alternates (1 ↔ 3, two parts in 65535)
/// and the pad never receives a stop the policy did not order.
fn emit(&mut self, pad: u16) -> RumbleCommand {
let (mut low, high) = self.level;
if self.quirks.dedup_jitter && (low, high) == self.last_emit {
let alt = low ^ 1;
low = if (alt, high) == (0, 0) {
low | 0b10
} else {
alt
};
}
self.last_emit = (low, high);
RumbleCommand {
pad,
low,
high,
backstop_ms: self.backstop(),
}
}
} }
/// The pure per-connection policy state machine. Time is always passed in (`now`) so the policy /// The pure per-connection policy state machine. Time is always passed in (`now`) so the policy
@@ -156,6 +210,8 @@ impl RumbleEngine {
p.dirty = true; p.dirty = true;
match ttl_ms { match ttl_ms {
Some(t) => { Some(t) => {
// Never honour a lease longer than [`MAX_LEASE_MS`], whatever the sender claims.
let t = t.min(MAX_LEASE_MS);
p.ttl_ms = t; p.ttl_ms = t;
p.legacy_wire = None; p.legacy_wire = None;
p.deadline = if (low, high) != (0, 0) { p.deadline = if (low, high) != (0, 0) {
@@ -214,22 +270,25 @@ impl RumbleEngine {
if p.dirty { if p.dirty {
p.dirty = false; p.dirty = false;
if p.level == (0, 0) { if p.level == (0, 0) {
return (Some(p.silence(pad)), None); // Relay a stop only if the actuator is, as far as the engine knows, still
// buzzing. A zero on an already-silent pad heals nothing and costs every
// embedder a command — Android an unconditional log line plus a binder
// `cancel()`. Two senders produce them: the host's deliberate
// `RUMBLE_STOP_BURST` re-sends after the first stop already landed, and (behind
// `PUNKTFUNK_RUMBLE_ENVELOPE=0`) the legacy flat 500 ms refresh, which re-sends
// zeros for every latched pad for the rest of the session. The burst still
// heals the case it exists for: a LOST first stop leaves the pad buzzing, so
// `last_emit != (0, 0)` and the re-send does emit.
if p.last_emit != (0, 0) {
return (Some(p.silence(pad)), None);
}
continue;
} }
if p.quirks.keepalive_ms > 0 { if p.quirks.keepalive_ms > 0 {
p.next_keepalive = p.next_keepalive =
Some(now + Duration::from_millis(p.quirks.keepalive_ms as u64)); Some(now + Duration::from_millis(p.quirks.keepalive_ms as u64));
} }
let (low, high) = p.level; return (Some(p.emit(pad)), None);
return (
Some(RumbleCommand {
pad,
low,
high,
backstop_ms: p.backstop(),
}),
None,
);
} }
// 4) actuator-decay keepalive, bounded by (1)/(2) above by construction: an expired // 4) actuator-decay keepalive, bounded by (1)/(2) above by construction: an expired
// or stale pad was silenced before reaching here, so a keepalive can never sustain a // or stale pad was silenced before reaching here, so a keepalive can never sustain a
@@ -239,20 +298,7 @@ impl RumbleEngine {
let due = *p.next_keepalive.get_or_insert(now + ka); let due = *p.next_keepalive.get_or_insert(now + ka);
if now >= due { if now >= due {
p.next_keepalive = Some(now + ka); p.next_keepalive = Some(now + ka);
let (mut low, high) = p.level; return (Some(p.emit(pad)), None);
if p.quirks.dedup_jitter {
p.jitter = !p.jitter;
low ^= p.jitter as u16;
}
return (
Some(RumbleCommand {
pad,
low,
high,
backstop_ms: p.backstop(),
}),
None,
);
} }
merge_wake(&mut wake, due); merge_wake(&mut wake, due);
} }
@@ -357,6 +403,22 @@ pub(crate) struct Closed;
mod tests { mod tests {
use super::*; use super::*;
/// The Steam Deck's declared quirks — the only shipping actuator with `dedup_jitter`.
const DECK: ActuatorQuirks = ActuatorQuirks {
keepalive_ms: 40,
min_pulse_ms: 0,
dedup_jitter: true,
};
/// Drain the engine the way an embedder does: poll until nothing is due.
fn drain(e: &mut RumbleEngine, t: Instant) -> Vec<(u16, u16)> {
let mut out = Vec::new();
while let (Some(c), _) = e.poll(t) {
out.push((c.low, c.high));
}
out
}
fn ms(v: u64) -> Duration { fn ms(v: u64) -> Duration {
Duration::from_millis(v) Duration::from_millis(v)
} }
@@ -527,4 +589,133 @@ mod tests {
); );
assert_eq!(shared.next_command(ms(10)), Err(Closed)); assert_eq!(shared.next_command(ms(10)), Err(Closed));
} }
/// A host renewal must not repeat the value the device last took, or an SDL-class layer
/// swallows the write. Before the jitter moved onto every emit path it lived only in the
/// keepalive branch, so each renewal collided with the last jittered write and was deduped.
#[test]
fn renewal_keeps_the_dedupe_jitter_alternating() {
let mut e = RumbleEngine::new();
e.set_quirks(0, DECK);
let t0 = Instant::now();
e.wire_update(t0, 0, 100, 200, Some(400));
assert_eq!(drain(&mut e, t0), vec![(100, 200)]);
assert_eq!(drain(&mut e, t0 + ms(40)), vec![(101, 200)]);
assert_eq!(drain(&mut e, t0 + ms(80)), vec![(100, 200)]);
// The renewal at the 120 ms default cadence: same level, must still be a distinct write.
e.wire_update(t0 + ms(120), 0, 100, 200, Some(400));
assert_eq!(drain(&mut e, t0 + ms(120)), vec![(101, 200)]);
assert_eq!(drain(&mut e, t0 + ms(160)), vec![(100, 200)]);
}
/// Phase-robust version of the same property, at the TTL hatch's 60 ms renewal floor: no two
/// consecutive DISTINCT device writes may be further apart than the declared 40 ms cadence.
#[test]
fn renewal_never_gaps_distinct_writes_at_the_60ms_floor() {
let mut e = RumbleEngine::new();
e.set_quirks(0, DECK);
let t0 = Instant::now();
let (mut last, mut last_write, mut worst) = ((0u16, 0u16), 0u64, 0u64);
for tick in 0..=360u64 {
let t = t0 + ms(tick);
if tick % 60 == 0 {
e.wire_update(t, 0, 100, 200, Some(400));
}
for v in drain(&mut e, t) {
assert_ne!(v, (0, 0), "a live lease must never emit the stop sentinel");
if v != last {
worst = worst.max(tick - last_write);
last_write = tick;
last = v;
}
}
}
assert!(
worst <= 41,
"worst distinct-write gap {worst} ms exceeds the 40 ms declared cadence"
);
}
/// The nudge must stay behind `dedup_jitter`: an off-by-one amplitude on a default-quirks pad
/// would land in Apple's identical-target comparison and Android's one-shot amplitudes.
#[test]
fn default_quirks_pads_get_the_level_verbatim_on_every_renewal() {
let mut e = RumbleEngine::new(); // Apple / Android / plain SDL
let t0 = Instant::now();
e.wire_update(t0, 0, 100, 200, Some(400));
assert_eq!(e.poll(t0).0, Some(cmd(0, 100, 200, 800)));
e.wire_update(t0 + ms(120), 0, 100, 200, Some(400));
assert_eq!(e.poll(t0 + ms(120)).0, Some(cmd(0, 100, 200, 800)));
}
/// Level `(1, 0)` is the one value whose LSB flip is the reserved stop. The nudge steps up
/// instead, so the phase still alternates and no stop is invented under a live lease.
#[test]
fn jitter_never_synthesizes_the_stop_sentinel() {
let mut e = RumbleEngine::new();
e.set_quirks(0, DECK);
let t0 = Instant::now();
e.wire_update(t0, 0, 1, 0, Some(400));
assert_eq!(e.poll(t0).0, Some(cmd(0, 1, 0, 800)));
assert_eq!(e.poll(t0 + ms(40)).0, Some(cmd(0, 3, 0, 800)));
assert_eq!(e.poll(t0 + ms(80)).0, Some(cmd(0, 1, 0, 800)));
}
/// A zero for a pad the engine already believes is silent is dropped: it heals nothing and
/// costs every embedder a command. The deliberate stop-burst heal is unaffected, because a
/// LOST stop leaves the pad buzzing and the re-send therefore does emit.
#[test]
fn a_redundant_stop_is_dropped_but_the_burst_still_heals_a_lost_one() {
let mut e = RumbleEngine::new();
let t0 = Instant::now();
e.wire_update(t0, 0, 100, 200, Some(400));
assert_eq!(drain(&mut e, t0), vec![(100, 200)]);
// First stop reaches the embedder…
e.wire_update(t0 + ms(10), 0, 0, 0, Some(0));
assert_eq!(drain(&mut e, t0 + ms(10)), vec![(0, 0)]);
// …and the burst re-sends behind it are now silent.
e.wire_update(t0 + ms(20), 0, 0, 0, Some(0));
e.wire_update(t0 + ms(30), 0, 0, 0, Some(0));
assert_eq!(drain(&mut e, t0 + ms(30)), Vec::new());
// But if the pad is buzzing (the stop that mattered was lost), a re-send still emits.
e.wire_update(t0 + ms(40), 0, 100, 200, Some(400));
assert_eq!(drain(&mut e, t0 + ms(40)), vec![(100, 200)]);
e.wire_update(t0 + ms(50), 0, 0, 0, Some(0));
assert_eq!(drain(&mut e, t0 + ms(50)), vec![(0, 0)]);
}
/// The client bounds the host's lease. `RUMBLE_TTL_CEIL_MS` is sender-side only, so a modified
/// or third-party host could otherwise stamp a huge TTL and wedge its pump, leaving Apple and
/// the Deck buzzing for the whole of it.
#[test]
fn an_overlong_lease_is_clamped_to_the_ceiling() {
let mut e = RumbleEngine::new();
let t0 = Instant::now();
e.wire_update(t0, 0, 100, 200, Some(u16::MAX));
assert_eq!(e.poll(t0).0, Some(cmd(0, 100, 200, 5000)));
// Silenced at the ceiling, not at the 65 s the sender asked for.
assert!(e.poll(t0 + ms(MAX_LEASE_MS as u64 - 1)).0.is_none());
assert_eq!(
e.poll(t0 + ms(MAX_LEASE_MS as u64)).0,
Some(cmd(0, 0, 0, 0)),
"the lease must end at the ceiling"
);
}
/// A v2 envelope carrying `ttl_ms == 0` on a LIVE level. The audit suspected the zero would be
/// mistaken for the legacy sentinel in `backstop()`; it cannot, because the expiry check
/// preempts the relay branch — the pad silences on the same poll and never reaches a backstop.
/// Pinned so that ordering stays load-bearing rather than incidental.
#[test]
fn a_zero_ttl_envelope_silences_rather_than_taking_the_legacy_backstop() {
let mut e = RumbleEngine::new();
let t0 = Instant::now();
e.wire_update(t0, 0, 100, 200, Some(0));
assert_eq!(
e.poll(t0).0,
Some(cmd(0, 0, 0, 0)),
"a zero-length lease must expire immediately, not emit with a legacy backstop"
);
}
} }