fix(core/rumble): the Deck's keepalive stops being swallowed by its own renewals #30

Merged
enricobuehler merged 1 commits from worktree-haptics-m3-rumble-engine into main 2026-08-04 21:01:33 +00:00
Owner

Workstream M3 of the 2026-08-03 force-feedback sweep — closes B12, B22, R9 and T1. All in crates/punktfunk-core/src/client/rumble.rs.

One change of shape, three faults

The free-running jitter: bool phase becomes last_emit: (u16, u16) — the exact value last handed to an embedder — and every emit routes through one emit() helper. That single field answers all three live questions: would re-sending this be a no-op device write? (B12), is this stop redundant? (R9-A), would the nudge invent a stop? (B22).

B12 — the keepalive the Deck declares was not the cadence it got

The Deck declares keepalive_ms: 40 with dedup_jitter, because an SDL-class layer discards a write identical to the last one. The nudge lived only in the keepalive branch (:243), so every host renewal re-emitted the raw level from the dirty branch (:214), collided with the last jittered write, was discarded — and re-anchored next_keepalive a full 40 ms out.

renewal cadence before after
120 ms (400 ms default TTL) 40, 40, 80 — every renewal swallowed 40, 40, 40, 40
60 ms (TTL hatch floor) 20, 100 — renewals alternate 40, 20, 40, 20

Verified against the real numbers: max(ttl*3/10, 60) = 120 ms at the 400 ms default, 60 ms at the floor, and SDL_joystick.c:2050 short-circuits identical values with no device write.

Severity is low, and the sweep's original framing overstated it. It leaned on "40 ms is the declared decay cadence", but gamepad.rs:65-71 says the actuator "decays inside SDL's ~2 s internal rumble resend" and that 40 ms merely mirrors SDL's sibling Steam-Controller driver keep-alive — a borrowed constant, not a measurement. Nothing establishes 80–100 ms is above the Deck's real decay threshold. What is unambiguous is that the engine did not deliver the cadence it declares, and the fix is one line.

B22 — the nudge could synthesize the reserved stop

Level (1, 0) XORs to (0, 0) — the value rumble.rs:39 and client/mod.rs define as "stop now" — and it went out with backstop_ms: 800, under a live lease. It is the only such level: high must already be 0, and low ^ 1 == 0 implies low == 1. The nudge now steps the LSB up (1 ↔ 3, two parts in 65535), so the phase still alternates and no stop is invented.

R9-A — a stop for a pad already believed silent

Branch 3 fired on dirty alone with no already-silent test. Under PUNKTFUNK_RUMBLE_ENVELOPE=0 the host re-sends zeros for every latched pad every 500 ms for the rest of the session — which cost Android an unconditional Log.i plus a binder cancel() at 2 Hz per pad.

The deliberate RUMBLE_STOP_BURST heal is untouched: a stop that was lost leaves the pad buzzing, so last_emit != (0, 0), which is exactly the guard's pass condition.

R9-B — the client now bounds the lease

RUMBLE_TTL_CEIL_MS is sender-side only. A modified or third-party host could stamp a long TTL and wedge its renewal pump, leaving buzzing 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 until the lease ends. Defence in depth; no behaviour change against any host in this tree.

One suspicion that did not survive

I flagged, on first reading, that ttl_ms looked overloaded — a v2 envelope with ttl_ms == 0 storing the same 0 that backstop() reads as "legacy". Refuted: the expiry check preempts the relay branch, so such a pad silences on the same poll and never reaches a backstop. No fix. Pinned with a test so that ordering stays load-bearing rather than incidental.

Tests — every one proven able to fail

Seven added. Each was run with its own fix reverted:

test revert result
renewal_keeps_the_dedupe_jitter_alternating dirty branch emits raw level FAILED
renewal_never_gaps_distinct_writes_at_the_60ms_floor same FAILED (gap → 100 ms)
jitter_never_synthesizes_the_stop_sentinel sentinel guard removed FAILED — emits low: 0, high: 0, backstop_ms: 800
a_redundant_stop_is_dropped_but_the_burst_still_heals_a_lost_one guard removed FAILED
an_overlong_lease_is_clamped_to_the_ceiling clamp removed FAILED
default_quirks_pads_get_the_level_verbatim_on_every_renewal jitter made unconditional FAILED

The last is an over-reach guard: a default-quirks pad must still receive the level verbatim, or an off-by-one amplitude would land in Apple's identical-target comparison (RumbleRenderer.swift:239) and Android's one-shot amplitudes.

My first probe run reported the B22 test as vacuous. It wasn't — rustfmt had split the ternary across four lines and my single-line search string silently matched nothing. Re-run against the real text, it fails correctly.

Verification

  • cargo test -p punktfunk-core --features quic --lib client::rumble17 passed, 0 failed
  • cargo clippy -p punktfunk-core --all-targets --features quic --locked -- -D warnings0
  • cargo fmt --all --check → clean
  • Generated C header unchanged. MAX_LEASE_MS is deliberately not pub: every pub const in this crate is emitted into include/punktfunk_core.h as an unprefixed #define, which is finding R21 (~170 of them already), and this had no reason to add another.

⚠ Note for anyone verifying locally: this module is behind the non-default quic feature, so a bare cargo test -p punktfunk-core compiles none of it and reports a healthy-looking "0 tests". CI is fine — 15 workspace members enable quic, so --workspace unifies it.

c_abi_harness_round_trips fails on macOS with a clang linker error — verified identical on an unmodified tree, so pre-existing and environmental.

Not on glass: the Deck cadence change is worth one session with a real Deck before merge.

🤖 Generated with Claude Code

Workstream **M3** of the [2026-08-03 force-feedback sweep](https://claude.ai/code/artifact/aa75b7f8-736a-4723-b20f-cef5155ed120) — closes **B12**, **B22**, **R9** and **T1**. All in `crates/punktfunk-core/src/client/rumble.rs`. ## One change of shape, three faults The free-running `jitter: bool` phase becomes **`last_emit: (u16, u16)`** — the exact value last handed to an embedder — and every emit routes through one `emit()` helper. That single field answers all three live questions: *would re-sending this be a no-op device write?* (B12), *is this stop redundant?* (R9-A), *would the nudge invent a stop?* (B22). ### B12 — the keepalive the Deck declares was not the cadence it got The Deck declares `keepalive_ms: 40` with `dedup_jitter`, because an SDL-class layer discards a write identical to the last one. The nudge lived only in the keepalive branch (`:243`), so every host renewal re-emitted the **raw** level from the dirty branch (`:214`), collided with the last jittered write, was discarded — *and* re-anchored `next_keepalive` a full 40 ms out. | renewal cadence | before | after | |---|---|---| | 120 ms (400 ms default TTL) | 40, 40, **80** — every renewal swallowed | 40, 40, 40, 40 | | 60 ms (TTL hatch floor) | 20, **100** — renewals alternate | 40, 20, 40, 20 | Verified against the real numbers: `max(ttl*3/10, 60)` = 120 ms at the 400 ms default, 60 ms at the floor, and `SDL_joystick.c:2050` short-circuits identical values with no device write. **Severity is low, and the sweep's original framing overstated it.** It leaned on "40 ms is the declared decay cadence", but `gamepad.rs:65-71` says the actuator "decays inside SDL's ~2 s internal rumble resend" and that 40 ms merely *mirrors SDL's sibling Steam-Controller driver keep-alive* — a borrowed constant, not a measurement. Nothing establishes 80–100 ms is above the Deck's real decay threshold. What is unambiguous is that the engine did not deliver the cadence it declares, and the fix is one line. ### B22 — the nudge could synthesize the reserved stop Level `(1, 0)` XORs to `(0, 0)` — the value `rumble.rs:39` and `client/mod.rs` define as "stop now" — and it went out with `backstop_ms: 800`, under a live lease. It is the *only* such level: `high` must already be 0, and `low ^ 1 == 0` implies `low == 1`. The nudge now steps the LSB **up** (1 ↔ 3, two parts in 65535), so the phase still alternates and no stop is invented. ### R9-A — a stop for a pad already believed silent Branch 3 fired on `dirty` alone with no already-silent test. Under `PUNKTFUNK_RUMBLE_ENVELOPE=0` the host re-sends zeros for every latched pad every 500 ms for the rest of the session — which cost Android an unconditional `Log.i` **plus a binder `cancel()`** at 2 Hz per pad. The deliberate `RUMBLE_STOP_BURST` heal is untouched: a stop that was **lost** leaves the pad buzzing, so `last_emit != (0, 0)`, which is exactly the guard's pass condition. ### R9-B — the client now bounds the lease `RUMBLE_TTL_CEIL_MS` is sender-side only. A modified or third-party host could stamp a long TTL and wedge its renewal pump, leaving buzzing 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 until the lease ends. Defence in depth; no behaviour change against any host in this tree. ## One suspicion that did not survive I flagged, on first reading, that `ttl_ms` looked overloaded — a v2 envelope with `ttl_ms == 0` storing the same `0` that `backstop()` reads as "legacy". **Refuted:** the expiry check preempts the relay branch, so such a pad silences on the same poll and never reaches a backstop. No fix. Pinned with a test so that ordering stays load-bearing rather than incidental. ## Tests — every one proven able to fail Seven added. Each was run with **its own fix reverted**: | test | revert | result | |---|---|---| | `renewal_keeps_the_dedupe_jitter_alternating` | dirty branch emits raw level | FAILED | | `renewal_never_gaps_distinct_writes_at_the_60ms_floor` | same | FAILED (gap → 100 ms) | | `jitter_never_synthesizes_the_stop_sentinel` | sentinel guard removed | FAILED — emits `low: 0, high: 0, backstop_ms: 800` | | `a_redundant_stop_is_dropped_but_the_burst_still_heals_a_lost_one` | guard removed | FAILED | | `an_overlong_lease_is_clamped_to_the_ceiling` | clamp removed | FAILED | | `default_quirks_pads_get_the_level_verbatim_on_every_renewal` | jitter made unconditional | FAILED | The last is an **over-reach** guard: a default-quirks pad must still receive the level verbatim, or an off-by-one amplitude would land in Apple's identical-target comparison (`RumbleRenderer.swift:239`) and Android's one-shot amplitudes. My first probe run reported the B22 test as vacuous. It wasn't — rustfmt had split the ternary across four lines and my single-line search string silently matched nothing. Re-run against the real text, it fails correctly. ## Verification - `cargo test -p punktfunk-core --features quic --lib client::rumble` → **17 passed, 0 failed** - `cargo clippy -p punktfunk-core --all-targets --features quic --locked -- -D warnings` → **0** - `cargo fmt --all --check` → clean - **Generated C header unchanged.** `MAX_LEASE_MS` is deliberately not `pub`: every `pub` const in this crate is emitted into `include/punktfunk_core.h` as an *unprefixed* `#define`, which is finding R21 (~170 of them already), and this had no reason to add another. ⚠ Note for anyone verifying locally: this module is behind the non-default **`quic`** feature, so a bare `cargo test -p punktfunk-core` compiles none of it and reports a healthy-looking "0 tests". CI is fine — 15 workspace members enable `quic`, so `--workspace` unifies it. ⚠ `c_abi_harness_round_trips` fails on macOS with a `clang` linker error — verified identical on an unmodified tree, so pre-existing and environmental. ⏳ Not on glass: the Deck cadence change is worth one session with a real Deck before merge. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
enricobuehler added 1 commit 2026-08-04 05:41:08 +00:00
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
ec4bf75a6e
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).
enricobuehler marked the pull request as ready for review 2026-08-04 21:01:06 +00:00
enricobuehler merged commit ffd5a33598 into main 2026-08-04 21:01:33 +00:00
Sign in to join this conversation.
No Reviewers
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: unom/punktfunk#30