From bd5735b80390870c96edb90bb57bf0aa19e52d4f Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Sun, 9 Aug 2026 21:10:08 +0200 Subject: [PATCH] feat(pads/windows): make the HID Xbox pad the default, and carry the trigger motors on the wire MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three changes that only make sense together: the HID backend becomes the default now that it is a superset of the XUSB one, the rumble datagram grows the two Xbox impulse-trigger motors, and the INF-shape tests learn about the Xbox identity's own install section. WP-E — `PUNKTFUNK_XBOX_BACKEND` now defaults to `hid`; `=xusb` is the escape hatch. The knob existed for exactly one reason, recorded in its own doc comment: the HID pad could not reach classic XInput, so defaulting to it would trade a known-working path for an unproven one. That objection is gone — with the `xinputhid` bus filter the INF now attaches, the HID pad is promoted like real hardware and keeps classic XInput while gaining everything XUSB never had (Steam, SDL, RawInput, DirectInput, joy.cpl, WGI) plus rumble, which XUSB could not source at all. The escape hatch stays because promotion leans on Microsoft's inbox `xinputhid.inf`; if a servicing update changes it, one env var restores the old behaviour with no reinstall. An unrecognised value takes the DEFAULT rather than the opt-out, so a typo cannot silently drop a user onto the path with no HID collection. WP-D — the `0xCA` rumble datagram gains a v3 form: v1 7 B: [0xCA][u16 pad][u16 low][u16 high] v2 10 B: … [u8 seq][u16 ttl_ms] v3 14 B: … [u16 lt][u16 rt] v3 is built FROM v2's bytes rather than restating the layout, so the prefix relationship is structural instead of a convention two encoders have to keep agreeing on, and every reader gates with `>=`. The four levels share one seq and one ttl on purpose: they are one statement of the pad's feedback at one instant, and sharing means the whole v2 apparatus — renewal cadence, stop burst, the client's seq gate, the lease clamp — governs the triggers with no new code. The new `RumbleUpdate` fields are plain `u16`, not `Option`: on a level-triggered plane "absent" must mean zero, because "absent → keep the previous value" is the stuck-rumble bug in a new costume. Only one backend can ever source them — the Windows HID Xbox pad, whose output report 0x03 carries them. `XINPUT_VIBRATION` and evdev `FF_RUMBLE` have two members and no third, so every other producer sends `lt = rt = 0`. ⚠️ The two TRIGGER `enable`-mask bits remain CONJECTURE. Bits 2/3 = left/right handle are measured; bit 0/1 = the triggers are inferred from field order and nothing else. `parse_xbox_output` says so inline, and no test asserts them — every test vector uses masks (0xFF, 0x00, 0x0C, 0xF3) whose expectations hold whichever bits turn out to be right. XInput cannot settle this: it has two motors. The INF tests — `hwid_matches_inf` matched the install section by the exact string `=pfGamepad,` and so stopped seeing the Xbox hardware ids the moment that identity moved to its own `pfGamepadXbox` section. It failed loudly, which is the good outcome; it is now prefix-matched and tolerant of further per-identity sections. Added `only_the_xbox_identity_installs_the_xinputhid_section`, which asserts the split in BOTH directions: the Xbox line must not install the shared section, and no other line may install the Xbox one. Merging them back is a one-line edit that looks like tidying and would hand a DualSense to Microsoft's Xbox translator. VERIFIED * ON WINDOWS (.173, the only place this code compiles): `cargo test -p pf-inject --lib` 104/104, including the new trigger tests and both INF tests; `cargo check -p punktfunk-host` clean. * macOS: `cargo fmt --all --check` clean; `cargo test -p punktfunk-core --features quic` rumble suite 22/22, including v3 round-trip and v3<->v2 cross-version parsing. * The pre-existing `c_abi_harness_round_trips` failure on macOS is `ld: library 'opus' not found` and reproduces with these changes stashed. NOT VERIFIED * No trigger rumble has ever been observed end to end — nothing can drive it yet (see the conjecture note above), and no client renders it. * The default flip has NOT been exercised in a real streaming session; every measurement so far came from the devtest harness. That is the on-glass run. * Non-Rust clients do not decode v3. They are blocked on a C ABI entry point first (`punktfunk_connection_next_rumble_cmd` has fixed out-params, ABI_VERSION 17); Apple could render it via GCHapticsLocality.leftTrigger/.rightTrigger, Android structurally cannot (its packed jlong is full) and has no trigger actuators anyway. --- clients/probe/src/main.rs | 7 +- .../pf-inject/src/inject/linux/dualsense.rs | 6 +- .../pf-inject/src/inject/linux/dualshock4.rs | 3 +- crates/pf-inject/src/inject/linux/gamepad.rs | 13 +- .../src/inject/linux/steam_controller.rs | 6 +- .../src/inject/linux/steam_controller2.rs | 3 +- .../pf-inject/src/inject/linux/switch_pro.rs | 5 +- crates/pf-inject/src/inject/uhid_manager.rs | 152 +++++++++---- .../inject/windows/dualsense_edge_windows.rs | 3 +- .../src/inject/windows/dualsense_windows.rs | 84 ++++++- .../src/inject/windows/dualshock4_windows.rs | 3 +- .../src/inject/windows/gamepad_windows.rs | 11 +- .../src/inject/windows/steam_deck_windows.rs | 3 +- .../src/inject/windows/xbox_windows.rs | 97 +++++--- crates/pf-inject/src/lib.rs | 2 +- crates/punktfunk-core/cbindgen.toml | 1 + .../src/client/pump/datagram_task.rs | 9 + crates/punktfunk-core/src/quic/datagram.rs | 163 +++++++++++++- crates/punktfunk-host/src/devtest.rs | 16 +- .../punktfunk-host/src/gamestream/control.rs | 10 +- crates/punktfunk-host/src/native.rs | 11 +- crates/punktfunk-host/src/native/gamepad.rs | 41 ++-- crates/punktfunk-host/src/native/input.rs | 212 +++++++++++++----- include/punktfunk_core.h | 9 + 24 files changed, 686 insertions(+), 184 deletions(-) diff --git a/clients/probe/src/main.rs b/clients/probe/src/main.rs index 399704f0..bb38de21 100644 --- a/clients/probe/src/main.rs +++ b/clients/probe/src/main.rs @@ -1274,13 +1274,18 @@ async fn session(args: Args) -> Result<()> { } } else if let Some(u) = punktfunk_core::quic::decode_rumble_envelope(&d) { // Log the first rumble so a loopback test can see the self-terminating v2 - // envelope tail (seq + TTL) arrived, not just the level. + // envelope tail (seq + TTL) arrived, not just the level. `lt`/`rt` are the v3 + // impulse-trigger levels: printed beside the envelope because the wire-leg + // check for trigger rumble is exactly "non-zero lt/rt AND the envelope still + // present" — i.e. the trigger tail did not displace the seq/TTL tail. if !rumble_logged { rumble_logged = true; tracing::info!( pad = u.pad, low = u.low, high = u.high, + lt = u.left_trigger, + rt = u.right_trigger, envelope = ?u.envelope, "rumble (0xCA)" ); diff --git a/crates/pf-inject/src/inject/linux/dualsense.rs b/crates/pf-inject/src/inject/linux/dualsense.rs index b852a523..39af7025 100644 --- a/crates/pf-inject/src/inject/linux/dualsense.rs +++ b/crates/pf-inject/src/inject/linux/dualsense.rs @@ -299,7 +299,8 @@ impl PadProto for DsLinuxProto { fn service(&self, pad: &mut DualSensePad, idx: u8) -> PadFeedback { let fb = pad.service(idx); PadFeedback { - rumble: fb.rumble, + // No trigger motors on this protocol — see `PadFeedback::rumble`. + rumble: fb.rumble.map(|(low, high)| (low, high, 0, 0)), hidout: fb.hidout, // Rumble-plane liveness (arms the shared abandoned-rumble force-off). evdev-FF games // going through hid-playstation get their stops surfaced reliably, but Steam Input @@ -401,7 +402,8 @@ impl PadProto for DsEdgeLinuxProto { fn service(&self, pad: &mut DualSensePad, idx: u8) -> PadFeedback { let fb = pad.service(idx); PadFeedback { - rumble: fb.rumble, + // No trigger motors on this protocol — see `PadFeedback::rumble`. + rumble: fb.rumble.map(|(low, high)| (low, high, 0, 0)), hidout: fb.hidout, // Rumble-plane liveness (arms the shared abandoned-rumble force-off). evdev-FF games // going through hid-playstation get their stops surfaced reliably, but Steam Input diff --git a/crates/pf-inject/src/inject/linux/dualshock4.rs b/crates/pf-inject/src/inject/linux/dualshock4.rs index 8929b2ec..aa4575de 100644 --- a/crates/pf-inject/src/inject/linux/dualshock4.rs +++ b/crates/pf-inject/src/inject/linux/dualshock4.rs @@ -314,7 +314,8 @@ impl PadProto for Ds4LinuxProto { fn service(&self, pad: &mut DualShock4Pad, idx: u8) -> PadFeedback { let fb = pad.service(idx); PadFeedback { - rumble: fb.rumble, + // No trigger motors on this protocol — see `PadFeedback::rumble`. + rumble: fb.rumble.map(|(low, high)| (low, high, 0, 0)), hidout: fb .led .map(|(r, g, b)| HidOutput::Led { pad: idx, r, g, b }) diff --git a/crates/pf-inject/src/inject/linux/gamepad.rs b/crates/pf-inject/src/inject/linux/gamepad.rs index 4b87784b..0f557a4c 100644 --- a/crates/pf-inject/src/inject/linux/gamepad.rs +++ b/crates/pf-inject/src/inject/linux/gamepad.rs @@ -705,9 +705,14 @@ impl GamepadManager { .ensure(idx, |i| VirtualPad::create(i as usize, identity)); } - /// Service every pad's FF protocol; `send(index, low, high)` is invoked for each pad whose - /// mixed rumble level changed. Call frequently (games block in `EVIOCSFF` until answered). - pub fn pump_rumble(&mut self, mut send: impl FnMut(u16, u16, u16)) { + /// Service every pad's FF protocol; `send(index, low, high, left_trigger, right_trigger)` is + /// invoked for each pad whose mixed rumble level changed. Call frequently (games block in + /// `EVIOCSFF` until answered). + /// + /// The two trigger levels are always zero here and always will be: evdev's `FF_RUMBLE` effect + /// is `{ u16 strong_magnitude, u16 weak_magnitude }` and has no third field, so impulse-trigger + /// rumble is unreachable through this backend no matter what the client can render. + pub fn pump_rumble(&mut self, mut send: impl FnMut(u16, u16, u16, u16, u16)) { // Finish any unplug whose removal frame only armed the grace — the producer sends that // frame once, so without this the uinput node would outlive the controller. The swept // mask is discarded because this manager keeps no per-index sibling state (the pads mix @@ -715,7 +720,7 @@ impl GamepadManager { self.slots.reap(); for (i, pad) in self.slots.iter_mut() { if let Some((low, high)) = pad.pump_ff() { - send(i as u16, low, high); + send(i as u16, low, high, 0, 0); } } } diff --git a/crates/pf-inject/src/inject/linux/steam_controller.rs b/crates/pf-inject/src/inject/linux/steam_controller.rs index 5bd26231..a58da15c 100644 --- a/crates/pf-inject/src/inject/linux/steam_controller.rs +++ b/crates/pf-inject/src/inject/linux/steam_controller.rs @@ -440,7 +440,8 @@ impl PadProto for SteamProto { fn service(&self, pad: &mut DeckTransport, _idx: u8) -> PadFeedback { let rumble = pad.service(); PadFeedback { - rumble, + // No trigger motors on this protocol — see `PadFeedback::rumble`. + rumble: rumble.map(|(low, high)| (low, high, 0, 0)), hidout: Vec::new(), // Rumble-plane liveness: a `0xEB` rumble command this poll. Steam Input drives this // pad over hidraw (the same abandonment semantics as the Windows Deck backend), so @@ -570,7 +571,8 @@ impl PadProto for ScProto { fn service(&self, pad: &mut SteamDeckPad, _idx: u8) -> PadFeedback { let rumble = pad.service(); PadFeedback { - rumble, + // No trigger motors on this protocol — see `PadFeedback::rumble`. + rumble: rumble.map(|(low, high)| (low, high, 0, 0)), hidout: Vec::new(), // Rumble-plane liveness: the kernel registers no FF device for the classic SC, so // rumble only ever arrives from a hidraw writer (`0xEB`) — which is exactly the diff --git a/crates/pf-inject/src/inject/linux/steam_controller2.rs b/crates/pf-inject/src/inject/linux/steam_controller2.rs index acd8efd0..3b7d559f 100644 --- a/crates/pf-inject/src/inject/linux/steam_controller2.rs +++ b/crates/pf-inject/src/inject/linux/steam_controller2.rs @@ -369,7 +369,8 @@ impl PadProto for TritonProto { }) .collect(); PadFeedback { - rumble, + // No trigger motors on this protocol — see `PadFeedback::rumble`. + rumble: rumble.map(|(low, high)| (low, high, 0, 0)), hidout, // Rumble-plane liveness: Steam is a hidraw writer here too, so the shared // abandoned-rumble force-off applies (the raw 0xCD passthrough plane is unaffected). diff --git a/crates/pf-inject/src/inject/linux/switch_pro.rs b/crates/pf-inject/src/inject/linux/switch_pro.rs index 015169ee..1dce0936 100644 --- a/crates/pf-inject/src/inject/linux/switch_pro.rs +++ b/crates/pf-inject/src/inject/linux/switch_pro.rs @@ -173,7 +173,8 @@ impl SwitchProPad { let _ = self.write_report(&build_usb_ack(cmd)); } Some(SwitchOutput::Subcmd { id, args, rumble }) => { - fb.rumble = Some(rumble); + // No trigger motors on this protocol — see `PadFeedback::rumble`. + fb.rumble = Some((rumble.0, rumble.1, 0, 0)); if id == 0x30 { // Player lights ride the subcommand itself; still ack it. if let Some(&arg) = args.first() { @@ -185,7 +186,7 @@ impl SwitchProPad { } self.answer_subcmd(id, &args); } - Some(SwitchOutput::Rumble(r)) => fb.rumble = Some(r), + Some(SwitchOutput::Rumble(r)) => fb.rumble = Some((r.0, r.1, 0, 0)), None => {} } } diff --git a/crates/pf-inject/src/inject/uhid_manager.rs b/crates/pf-inject/src/inject/uhid_manager.rs index e43ab836..9ceb5a41 100644 --- a/crates/pf-inject/src/inject/uhid_manager.rs +++ b/crates/pf-inject/src/inject/uhid_manager.rs @@ -18,13 +18,21 @@ use std::time::{Duration, Instant}; /// 0xCD feedback events (lightbar / player LEDs / adaptive triggers), deduped via [`HidoutDedup`]. #[derive(Default)] pub struct PadFeedback { - /// `(low, high)` motor levels, if the pass saw a rumble report. + /// `(low, high, left_trigger, right_trigger)` motor levels, if the pass saw a rumble report. /// /// Range is `0..=0xFFFF` — this said `0..=0xFF00`, which is only true of the backends that /// widen the device's 8-bit motor byte by `<< 8` (the UHID/DualSense path). The Windows /// backend widens by `× 257` and does reach 0xFFFF, and this type carries both. Neither is a /// defect: consumers narrow with `>> 8`, and 0xFF00 and 0xFFFF both narrow back to 255. - pub rumble: Option<(u16, u16)>, + /// + /// The two trailing fields are the Xbox impulse-trigger motors, which ride the 0xCA plane's + /// v3 tail (design/trigger-rumble-plane.md). **Exactly one backend can ever set them non-zero** + /// — the Windows HID Xbox pad, whose output report `0x03` has fields for them. Every other + /// backend reports `(low, high, 0, 0)` because the packet it parses has nowhere to carry them: + /// XUSB's `SET_STATE` is `rumble_large`/`rumble_small`, evdev's `FF_RUMBLE` is strong/weak, + /// and a DualSense's trigger actuators are *adaptive* (force resistance, on the 0xCD plane) + /// rather than motors. That is a permanent property of those protocols, not a gap to fill. + pub rumble: Option<(u16, u16, u16, u16)>, pub hidout: Vec, /// Whether the game drove this pad's RUMBLE plane this poll — at least one output report /// asserted the vibration fields (valid-flag set, including an explicit zero), not merely any @@ -119,8 +127,11 @@ pub struct UhidManager { slots: PadSlots, /// Each pad's current full report — buttons/sticks merged with persisted rich-plane fields. state: Vec, - /// Last rumble forwarded per pad, so a report that only changes rich feedback doesn't re-send it. - last_rumble: Vec<(u16, u16)>, + /// Last rumble forwarded per pad, so a report that only changes rich feedback doesn't re-send + /// it. All FOUR levels, deliberately: dedup on the handle pair alone would swallow a + /// trigger-only change — a racing title's impulse-trigger stream against silent handles — and + /// the pad would never rumble, with nothing logged anywhere. + last_rumble: Vec<(u16, u16, u16, u16)>, /// Last rich feedback forwarded per pad, so an output report that only changed the rumble /// doesn't re-send unchanged lightbar/LED/trigger state. hidout_dedup: Vec, @@ -254,7 +265,7 @@ impl UhidManager { backend, slots: PadSlots::new(B::LABEL, B::DEVICE, B::CREATE_HINT), state, - last_rumble: vec![(0, 0); MAX_PADS], + last_rumble: vec![(0, 0, 0, 0); MAX_PADS], hidout_dedup: vec![HidoutDedup::default(); MAX_PADS], last_write: vec![Instant::now(); MAX_PADS], last_active: vec![Instant::now(); MAX_PADS], @@ -339,13 +350,14 @@ impl UhidManager { } /// Service every pad: answer any pending driver/kernel handshake and route a game's feedback - /// back out. `rumble` is invoked `(index, low, high)` only when the motor level *changes* (the - /// universal 0xCA plane); `hidout` is invoked per rich feedback event that isn't an exact - /// repeat of the last-forwarded value (the 0xCD plane). Call frequently — kernel/driver init - /// handshakes block until answered. + /// back out. `rumble` is invoked `(index, low, high, left_trigger, right_trigger)` only when + /// the motor level *changes* (the universal 0xCA plane — the trigger pair is non-zero only on + /// the Windows HID Xbox pad, see [`PadFeedback::rumble`]); `hidout` is invoked per rich + /// feedback event that isn't an exact repeat of the last-forwarded value (the 0xCD plane). + /// Call frequently — kernel/driver init handshakes block until answered. pub fn pump( &mut self, - mut rumble: impl FnMut(u16, u16, u16), + mut rumble: impl FnMut(u16, u16, u16, u16, u16), mut hidout: impl FnMut(HidOutput), ) { let now = Instant::now(); @@ -369,9 +381,9 @@ impl UhidManager { // the next LED/trigger state re-forwards. WARN through the per-pad rate limiter — // a storm overflows every poll and the raw line once flooded a whole log export. self.overflow_warn[i].note(now, B::LABEL, i); - if self.last_rumble[i] != (0, 0) { - self.last_rumble[i] = (0, 0); - rumble(i as u16, 0, 0); + if self.last_rumble[i] != (0, 0, 0, 0) { + self.last_rumble[i] = (0, 0, 0, 0); + rumble(i as u16, 0, 0, 0, 0); } self.hidout_dedup[i] = HidoutDedup::default(); } @@ -385,9 +397,9 @@ impl UhidManager { if let Some(r) = fb.rumble { if self.last_rumble[i] != r { self.last_rumble[i] = r; - rumble(i as u16, r.0, r.1); + rumble(i as u16, r.0, r.1, r.2, r.3); } - } else if self.last_rumble[i] != (0, 0) + } else if self.last_rumble[i] != (0, 0, 0, 0) && rumble_idle_timeout() .is_some_and(|t| now.duration_since(self.last_active[i]) >= t) { @@ -400,10 +412,12 @@ impl UhidManager { index = i, prev_low = self.last_rumble[i].0, prev_high = self.last_rumble[i].1, + prev_lt = self.last_rumble[i].2, + prev_rt = self.last_rumble[i].3, "rumble: stale residual (game stopped driving the rumble plane) — forcing off" ); - self.last_rumble[i] = (0, 0); - rumble(i as u16, 0, 0); + self.last_rumble[i] = (0, 0, 0, 0); + rumble(i as u16, 0, 0, 0, 0); } for h in fb.hidout { // Skip rich feedback that repeats the last-forwarded value (a game's output report @@ -469,7 +483,7 @@ impl UhidManager { /// (re)connect starts from scratch and is always forwarded. fn reset_pad(&mut self, idx: usize) { self.state[idx] = self.backend.neutral(); - self.last_rumble[idx] = (0, 0); + self.last_rumble[idx] = (0, 0, 0, 0); self.hidout_dedup[idx].clear(); self.last_write[idx] = Instant::now(); self.last_active[idx] = Instant::now(); @@ -733,14 +747,14 @@ mod tests { m.handle(&frame(1, 0b00, 0)); assert!(m.slots.get(1).is_some(), "inside the grace — not yet swept"); // A tick inside the grace must NOT flap the devnode (pad_slots::SWEEP_GRACE). - m.pump(|_, _, _| {}, |_| {}); + m.pump(|_, _, _, _, _| {}, |_| {}); assert!( m.slots.get(1).is_some(), "a tick inside the grace dropped it" ); // Grace elapsed: the next tick completes the unplug, with no further frame. m.slots.expire_grace(); - m.pump(|_, _, _| {}, |_| {}); + m.pump(|_, _, _, _, _| {}, |_| {}); assert!( m.slots.get(1).is_none(), "the pump tick never completed the unplug" @@ -783,7 +797,10 @@ mod tests { m.handle(&frame(0, 0b1, 0)); let collect = |m: &mut UhidManager| { let out = RefCell::new(Vec::new()); - m.pump(|i, lo, hi| out.borrow_mut().push((i, lo, hi)), |_| {}); + m.pump( + |i, lo, hi, lt, rt| out.borrow_mut().push((i, lo, hi, lt, rt)), + |_| {}, + ); out.into_inner() }; let rumble = |r| PadFeedback { @@ -792,12 +809,16 @@ mod tests { rumble_drove: Some(true), resync: false, }; - *m.backend.feedback.borrow_mut() = vec![rumble((100, 0)), rumble((100, 0)), rumble((7, 7))]; - assert_eq!(collect(&mut m), vec![(0, 100, 0)]); // first value forwards + *m.backend.feedback.borrow_mut() = vec![ + rumble((100, 0, 0, 0)), + rumble((100, 0, 0, 0)), + rumble((7, 7, 0, 0)), + ]; + assert_eq!(collect(&mut m), vec![(0, 100, 0, 0, 0)]); // first value forwards assert_eq!(collect(&mut m), vec![]); // exact repeat deduped - assert_eq!(collect(&mut m), vec![(0, 7, 7)]); // change forwards - // Unplug + recreate re-arms the dedup: the same level forwards again. The unplug completes - // on a PUMP tick, not on a second frame — that is all production ever sends. + assert_eq!(collect(&mut m), vec![(0, 7, 7, 0, 0)]); // change forwards + // Unplug + recreate re-arms the dedup: the same level forwards again. The unplug completes + // on a PUMP tick, not on a second frame — that is all production ever sends. m.handle(&frame(0, 0b0, 0)); // the one removal frame — arms the grace m.slots.expire_grace(); assert_eq!(collect(&mut m), vec![]); // this tick reaps; nothing queued to forward @@ -806,8 +827,43 @@ mod tests { "the pump tick completed the unplug" ); m.handle(&frame(0, 0b1, 0)); - *m.backend.feedback.borrow_mut() = vec![rumble((7, 7))]; - assert_eq!(collect(&mut m), vec![(0, 7, 7)]); + *m.backend.feedback.borrow_mut() = vec![rumble((7, 7, 0, 0))]; + assert_eq!(collect(&mut m), vec![(0, 7, 7, 0, 0)]); + } + + /// The dedup compares all FOUR levels. Comparing only the handle pair would swallow a + /// trigger-only change — which is the *normal* shape of impulse-trigger content, since racing + /// titles drive the triggers continuously against near-silent handles — and the pad would + /// simply never rumble, with nothing logged and nothing on the wire to look at. + #[test] + fn a_trigger_only_change_is_forwarded_not_deduped_away() { + let mut m = mgr(); + m.handle(&frame(0, 0b1, 0)); + let collect = |m: &mut UhidManager| { + let out = RefCell::new(Vec::new()); + m.pump( + |i, lo, hi, lt, rt| out.borrow_mut().push((i, lo, hi, lt, rt)), + |_| {}, + ); + out.into_inner() + }; + let rumble = |r| PadFeedback { + rumble: Some(r), + hidout: Vec::new(), + rumble_drove: Some(true), + resync: false, + }; + // Handles silent throughout; only the trigger motors move. + *m.backend.feedback.borrow_mut() = vec![ + rumble((0, 0, 0x8000, 0)), + rumble((0, 0, 0x8000, 0)), + rumble((0, 0, 0x8000, 0x4000)), + rumble((0, 0, 0, 0)), + ]; + assert_eq!(collect(&mut m), vec![(0, 0, 0, 0x8000, 0)]); + assert_eq!(collect(&mut m), vec![], "exact repeat still dedups"); + assert_eq!(collect(&mut m), vec![(0, 0, 0, 0x8000, 0x4000)]); + assert_eq!(collect(&mut m), vec![(0, 0, 0, 0, 0)], "the stop forwards"); } #[test] @@ -816,17 +872,20 @@ mod tests { m.handle(&frame(0, 0b1, 0)); let collect = |m: &mut UhidManager| { let out = RefCell::new(Vec::new()); - m.pump(|i, lo, hi| out.borrow_mut().push((i, lo, hi)), |_| {}); + m.pump( + |i, lo, hi, lt, rt| out.borrow_mut().push((i, lo, hi, lt, rt)), + |_| {}, + ); out.into_inner() }; // The game latches a non-zero rumble (a fresh report drove the pad). *m.backend.feedback.borrow_mut() = vec![PadFeedback { - rumble: Some((200, 0)), + rumble: Some((200, 0, 0, 0)), hidout: Vec::new(), rumble_drove: Some(true), resync: false, }]; - assert_eq!(collect(&mut m), vec![(0, 200, 0)]); + assert_eq!(collect(&mut m), vec![(0, 200, 0, 0, 0)]); // The game stops driving the RUMBLE plane — no output report at all, or (equivalently, the // confirmed stuck-ON case) a stream of LED/adaptive-trigger reports that never assert the @@ -845,7 +904,7 @@ mod tests { // exactly once, then stays off (no repeated zero spam). m.last_active[0] = Instant::now() - (RUMBLE_IDLE_TIMEOUT + Duration::from_millis(50)); *m.backend.feedback.borrow_mut() = vec![idle(), idle()]; - assert_eq!(collect(&mut m), vec![(0, 0, 0)]); // forced off + assert_eq!(collect(&mut m), vec![(0, 0, 0, 0, 0)]); // forced off assert_eq!(collect(&mut m), vec![]); // already zero — no repeat } @@ -855,16 +914,19 @@ mod tests { m.handle(&frame(0, 0b1, 0)); let collect = |m: &mut UhidManager| { let out = RefCell::new(Vec::new()); - m.pump(|i, lo, hi| out.borrow_mut().push((i, lo, hi)), |_| {}); + m.pump( + |i, lo, hi, lt, rt| out.borrow_mut().push((i, lo, hi, lt, rt)), + |_| {}, + ); out.into_inner() }; *m.backend.feedback.borrow_mut() = vec![PadFeedback { - rumble: Some((200, 0)), + rumble: Some((200, 0, 0, 0)), hidout: Vec::new(), rumble_drove: Some(true), resync: false, }]; - assert_eq!(collect(&mut m), vec![(0, 200, 0)]); + assert_eq!(collect(&mut m), vec![(0, 200, 0, 0, 0)]); // Even with a stale clock, a poll where the game drove the rumble plane refreshes // activity, so the held rumble is NOT cut. Backends report that as @@ -872,7 +934,7 @@ mod tests { // the manager also honors the bare `rumble_drove: Some(true)` shape defensively. m.last_active[0] = Instant::now() - (RUMBLE_IDLE_TIMEOUT + Duration::from_millis(50)); *m.backend.feedback.borrow_mut() = vec![PadFeedback { - rumble: Some((200, 0)), + rumble: Some((200, 0, 0, 0)), hidout: Vec::new(), rumble_drove: Some(true), resync: false, @@ -906,7 +968,7 @@ mod tests { }]; let out = RefCell::new(0u32); m.pump( - |_, _, _| {}, + |_, _, _, _, _| {}, |_| { *out.borrow_mut() += 1; }, @@ -976,7 +1038,7 @@ mod tests { let rumbles = RefCell::new(Vec::new()); let hidouts = RefCell::new(0u32); m.pump( - |i, lo, hi| rumbles.borrow_mut().push((i, lo, hi)), + |i, lo, hi, lt, rt| rumbles.borrow_mut().push((i, lo, hi, lt, rt)), |_| *hidouts.borrow_mut() += 1, ); (rumbles.into_inner(), hidouts.into_inner()) @@ -984,12 +1046,12 @@ mod tests { // Latch a rumble + an LED. *m.backend.feedback.borrow_mut() = vec![PadFeedback { - rumble: Some((100, 0)), + rumble: Some((100, 0, 0, 0)), hidout: vec![led(10)], rumble_drove: Some(true), resync: false, }]; - assert_eq!(collect(&mut m), (vec![(0, 100, 0)], 1)); + assert_eq!(collect(&mut m), (vec![(0, 100, 0, 0, 0)], 1)); // Overflow poll: no reports survived, resync flagged → forced stop, exactly once. *m.backend.feedback.borrow_mut() = vec![PadFeedback { @@ -998,22 +1060,22 @@ mod tests { rumble_drove: Some(false), resync: true, }]; - assert_eq!(collect(&mut m), (vec![(0, 0, 0)], 0)); + assert_eq!(collect(&mut m), (vec![(0, 0, 0, 0, 0)], 0)); // The game re-asserts the SAME rumble + LED state: both must re-forward (the rumble // because the forced stop reset `last_rumble`, the LED because the dedup was re-armed). *m.backend.feedback.borrow_mut() = vec![PadFeedback { - rumble: Some((100, 0)), + rumble: Some((100, 0, 0, 0)), hidout: vec![led(10)], rumble_drove: Some(true), resync: false, }]; - assert_eq!(collect(&mut m), (vec![(0, 100, 0)], 1)); + assert_eq!(collect(&mut m), (vec![(0, 100, 0, 0, 0)], 1)); // A resync with nothing latched forwards no spurious stop. *m.backend.feedback.borrow_mut() = vec![ PadFeedback { - rumble: Some((0, 0)), + rumble: Some((0, 0, 0, 0)), hidout: Vec::new(), rumble_drove: Some(true), resync: false, @@ -1025,7 +1087,7 @@ mod tests { resync: true, }, ]; - assert_eq!(collect(&mut m), (vec![(0, 0, 0)], 0)); // the explicit stop + assert_eq!(collect(&mut m), (vec![(0, 0, 0, 0, 0)], 0)); // the explicit stop assert_eq!(collect(&mut m), (vec![], 0)); // resync at zero — silent } } diff --git a/crates/pf-inject/src/inject/windows/dualsense_edge_windows.rs b/crates/pf-inject/src/inject/windows/dualsense_edge_windows.rs index acac31a1..1947f4d6 100644 --- a/crates/pf-inject/src/inject/windows/dualsense_edge_windows.rs +++ b/crates/pf-inject/src/inject/windows/dualsense_edge_windows.rs @@ -85,7 +85,8 @@ impl PadProto for DsEdgeWinProto { fn service(&self, pad: &mut DsWinPad, idx: u8) -> PadFeedback { let fb = pad.service(idx); PadFeedback { - rumble: fb.rumble, + // No trigger motors on this protocol — see `PadFeedback::rumble`. + rumble: fb.rumble.map(|(low, high)| (low, high, 0, 0)), hidout: fb.hidout, // Rumble-plane liveness, not any-report liveness — see the plain DualSense backend. rumble_drove: Some(fb.rumble.is_some()), diff --git a/crates/pf-inject/src/inject/windows/dualsense_windows.rs b/crates/pf-inject/src/inject/windows/dualsense_windows.rs index ac7325d9..7ae6e69c 100644 --- a/crates/pf-inject/src/inject/windows/dualsense_windows.rs +++ b/crates/pf-inject/src/inject/windows/dualsense_windows.rs @@ -686,7 +686,8 @@ impl PadProto for DsWinProto { // feed the abandoned-rumble force-off's activity clock (the historical unbounded // stuck-ON path, now doubly closed by the lossless report ring). rumble_drove: Some(fb.rumble.is_some()), - rumble: fb.rumble, + // No trigger motors on this protocol — see `PadFeedback::rumble`. + rumble: fb.rumble.map(|(low, high)| (low, high, 0, 0)), hidout: fb.hidout, resync: fb.resync, } @@ -995,14 +996,26 @@ mod drain_tests { "/../../packaging/windows/drivers/pf-gamepad/pf_gamepad.inx" ); let inf = std::fs::read_to_string(inx).expect("read pf_gamepad.inx"); - // The [Models] lines: `%DeviceDesc…%=pfGamepad, [, …]`. + // The [Models] lines: `%DeviceDesc…%=, [, …]`. + // + // ⚠️ Match the install section by PREFIX, not by the exact string `pfGamepad,`. The Xbox + // line installs `pfGamepadXbox` — a section of its own, because that identity additionally + // attaches the `xinputhid` bus filter and the four PlayStation/Deck identities must not get + // it. An exact match silently stopped seeing the Xbox ids the moment that split happened, + // which is precisely the "this test went vacuous" failure the assert below guards against, + // except it failed loudly instead. Keep this tolerant of further per-identity sections. let declared: Vec = inf .lines() .map(str::trim) .filter(|l| !l.starts_with(';')) - .filter_map(|l| l.split_once("=pfGamepad,")) - .flat_map(|(_, ids)| { - ids.split(',') + .filter_map(|l| l.split_once('=')) + .filter(|(_, rhs)| rhs.trim_start().starts_with("pfGamepad")) + .flat_map(|(_, rhs)| { + // `pfGamepad[Suffix], [, …]` — drop the section name, keep the ids. + // `AddReg=pfGamepadXbox_HW_AddReg` reaches here too and contributes nothing, + // because it has no comma. + rhs.split(',') + .skip(1) .map(|id| id.trim().to_ascii_lowercase()) .collect::>() }) @@ -1033,6 +1046,67 @@ mod drain_tests { } } + /// The Xbox identity must install its OWN section, and the PlayStation/Deck identities must + /// not install that one. + /// + /// `pfGamepadXbox` attaches Microsoft's `xinputhid` as an upper filter and sets + /// `DevicePropertyFlags=1` (`BusDevice`), which is what makes Windows promote our Xbox pad — + /// it mints the `IG_00` token, registers an XUSB interface, and lets classic XInput and rumble + /// through. Applied to a DualSense, DualShock 4, Edge or Steam Deck it would hand a + /// PlayStation pad to Microsoft's **Xbox** translator, which claims the HID collection + /// exclusively and would take a working pad away from Steam and SDL. + /// + /// Merging the two sections back together is a one-line edit that looks like tidying and is + /// not, so assert the split rather than trusting a comment to survive. + #[test] + fn only_the_xbox_identity_installs_the_xinputhid_section() { + let inx = concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../packaging/windows/drivers/pf-gamepad/pf_gamepad.inx" + ); + let inf = std::fs::read_to_string(inx).expect("read pf_gamepad.inx"); + let xbox = super::super::xbox_windows::XBOX_HWID.to_ascii_lowercase(); + + let mut saw_xbox_model = false; + for line in inf.lines().map(str::trim).filter(|l| !l.starts_with(';')) { + let Some((_, rhs)) = line.split_once('=') else { + continue; + }; + let rhs = rhs.trim_start(); + let Some((section, ids)) = rhs.split_once(',') else { + continue; + }; + if !section.starts_with("pfGamepad") { + continue; + } + let ids: Vec = ids + .split(',') + .map(|i| i.trim().to_ascii_lowercase()) + .collect(); + let mentions_xbox = ids.iter().any(|i| i.contains(&xbox)); + if mentions_xbox { + saw_xbox_model = true; + assert_ne!( + section, "pfGamepad", + "the Xbox model line installs the SHARED section, so the xinputhid filter \ + would be attached to every PlayStation and Deck pad too" + ); + } else { + assert_eq!( + section, "pfGamepad", + "a non-Xbox model line ({ids:?}) installs {section:?}; if that section carries \ + the xinputhid filter, this pad is about to be handed to Microsoft's Xbox \ + translator" + ); + } + } + assert!( + saw_xbox_model, + "no [Models] line mentions {xbox:?} — the parse went vacuous; fix it rather than \ + deleting the assert" + ); + } + /// The driver reads its HID identity back off the same hardware id — that mapping is what /// decides which report descriptor and which VID/PID a pad enumerates with, and it is settled /// at `EvtDeviceAdd`, before the sealed channel can possibly say anything (its delivery goes diff --git a/crates/pf-inject/src/inject/windows/dualshock4_windows.rs b/crates/pf-inject/src/inject/windows/dualshock4_windows.rs index 1418291a..6055901e 100644 --- a/crates/pf-inject/src/inject/windows/dualshock4_windows.rs +++ b/crates/pf-inject/src/inject/windows/dualshock4_windows.rs @@ -232,7 +232,8 @@ impl PadProto for Ds4WinProto { fn service(&self, pad: &mut Ds4WinPad, idx: u8) -> PadFeedback { let fb = pad.service(); PadFeedback { - rumble: fb.rumble, + // No trigger motors on this protocol — see `PadFeedback::rumble`. + rumble: fb.rumble.map(|(low, high)| (low, high, 0, 0)), hidout: fb .led .map(|(r, g, b)| HidOutput::Led { pad: idx, r, g, b }) diff --git a/crates/pf-inject/src/inject/windows/gamepad_windows.rs b/crates/pf-inject/src/inject/windows/gamepad_windows.rs index b6b8ed91..22c54bbb 100644 --- a/crates/pf-inject/src/inject/windows/gamepad_windows.rs +++ b/crates/pf-inject/src/inject/windows/gamepad_windows.rs @@ -356,7 +356,12 @@ impl GamepadManager { /// Relay any changed rumble level to the client. XUSB motors are 0..255; the wire carries /// 0..65535, so scale by 257. `large` (low-frequency) → the datagram's `low`, `small` /// (high-frequency) → `high` — matching the other backends. - pub fn pump_rumble(&mut self, mut send: impl FnMut(u16, u16, u16)) { + /// + /// The two trigger levels `send` also takes are always zero here and always will be: the XUSB + /// `SET_STATE` packet this backend parses carries `rumble_large`/`rumble_small` and nothing + /// else, mirroring `XINPUT_VIBRATION`'s two members. Impulse-trigger rumble is only reachable + /// through the HID-visible Xbox identity (WGI / GameInput), never through the XUSB companion. + pub fn pump_rumble(&mut self, mut send: impl FnMut(u16, u16, u16, u16, u16)) { // Finish any unplug whose removal frame only armed the grace — the producer sends that // frame once, so without this the XUSB devnode would outlive the controller. let swept = self.slots.reap(); @@ -369,7 +374,7 @@ impl GamepadManager { self.last_active[i] = Instant::now(); if self.last_rumble[i] != (large, small) { self.last_rumble[i] = (large, small); - send(i as u16, large as u16 * 257, small as u16 * 257); + send(i as u16, large as u16 * 257, small as u16 * 257, 0, 0); } } else if self.last_rumble[i] != (0, 0) && crate::uhid_manager::rumble_idle_timeout() @@ -386,7 +391,7 @@ impl GamepadManager { "rumble: stale residual (game stopped driving the pad) — forcing off" ); self.last_rumble[i] = (0, 0); - send(i as u16, 0, 0); + send(i as u16, 0, 0, 0, 0); } } } diff --git a/crates/pf-inject/src/inject/windows/steam_deck_windows.rs b/crates/pf-inject/src/inject/windows/steam_deck_windows.rs index 5e6e0e15..1f56ccbc 100644 --- a/crates/pf-inject/src/inject/windows/steam_deck_windows.rs +++ b/crates/pf-inject/src/inject/windows/steam_deck_windows.rs @@ -231,7 +231,8 @@ impl PadProto for DeckWinProto { // presence is the rumble-plane activity signal, even at an unchanged level. let (rumble, resync) = pad.service(); PadFeedback { - rumble, + // No trigger motors on this protocol — see `PadFeedback::rumble`. + rumble: rumble.map(|(low, high)| (low, high, 0, 0)), hidout: Vec::new(), rumble_drove: Some(rumble.is_some()), resync, diff --git a/crates/pf-inject/src/inject/windows/xbox_windows.rs b/crates/pf-inject/src/inject/windows/xbox_windows.rs index 21c063c2..ef347e48 100644 --- a/crates/pf-inject/src/inject/windows/xbox_windows.rs +++ b/crates/pf-inject/src/inject/windows/xbox_windows.rs @@ -128,7 +128,7 @@ impl XboxWinPad { /// Poll the section's output slot for a game's rumble, tick the sealed-channel delivery and /// feed the driver-attach health watcher. - fn service(&mut self) -> (Option<(u16, u16)>, bool) { + fn service(&mut self) -> (Option<(u16, u16, u16, u16)>, bool) { self.channel.pump(); // SAFETY: base points at SHM_SIZE bytes. let proto = unsafe { @@ -146,34 +146,46 @@ impl XboxWinPad { } } -/// Parse an Xbox output report into `(low, high)` motor levels on the wire's 0..65535 scale. +/// Parse an Xbox output report into `(low, high, left_trigger, right_trigger)` motor levels on the +/// wire's 0..65535 scale. /// /// The Bluetooth Xbox rumble report is id `0x03`: `[id, enable, left_trigger, right_trigger, /// left, right, duration, delay, loop]`, with magnitudes on a **0..100** scale (not 0..255 — a /// detail that silently costs 60 % of the rumble range if you assume otherwise). The `enable` /// mask picks which motors the values apply to; bit 2 is the left (low-frequency) motor and bit 3 /// the right (high-frequency) one, matching how the wire's `low`/`high` pair is used elsewhere. -/// The two trigger motors have no wire representation and are ignored. +/// +/// Bytes 2/3 are the two impulse-trigger motors, which ride the 0xCA plane's v3 tail +/// (design/trigger-rumble-plane.md). This pad is the only backend in the tree that can ever source +/// them: XUSB's `SET_STATE` carries `rumble_large`/`rumble_small` and evdev's `FF_RUMBLE` carries +/// strong/weak, so neither packet has a field to lose. They are scaled by the same 0..100 closure +/// as the handles rather than a copy of it — assuming 0..255 here would read a full-scale `100` as +/// ~39 %, which on a real pad reads as "trigger rumble works but is weirdly weak", the hardest +/// class of bug to attribute. /// /// ⚠️ Never seen a real report — this shape is from the documented protocol, not a capture. -fn parse_xbox_output(bytes: &[u8]) -> Option<(u16, u16)> { +/// +/// ⚠️ **The two TRIGGER `enable` bits are conjecture, not measurement.** Bits 2/3 = left/right +/// handle are known; bit 0 = left trigger and bit 1 = right trigger are inferred from the report's +/// field order (triggers first, handles second) and from nothing else. A live capture (design WP0) +/// settles it. Getting it wrong yields "the triggers buzz when the game asked for the handles", +/// so nothing downstream may treat this assignment as established — and the tests below are +/// deliberately written with mask vectors that hold whichever bits turn out to be right. +fn parse_xbox_output(bytes: &[u8]) -> Option<(u16, u16, u16, u16)> { // The driver republishes output reports report-id-prefixed, like the PS backends. if bytes.len() < 6 || bytes[0] != 0x03 { return None; } let enable = bytes[1]; let scale = |v: u8| -> u16 { (v.min(100) as u32 * 65535 / 100) as u16 }; - let low = if enable & 0x04 != 0 { - scale(bytes[4]) - } else { - 0 - }; - let high = if enable & 0x08 != 0 { - scale(bytes[5]) - } else { - 0 - }; - Some((low, high)) + let gated = |bit: u8, v: u8| if enable & bit != 0 { scale(v) } else { 0 }; + Some(( + gated(0x04, bytes[4]), + gated(0x08, bytes[5]), + // UNVERIFIED bit assignment — see the second ⚠️ above before trusting either of these. + gated(0x01, bytes[2]), + gated(0x02, bytes[3]), + )) } /// The Windows-Xbox half of the shared stateful manager (see [`PadProto`]). Lifecycle (slot table, @@ -251,30 +263,65 @@ pub type XboxWindowsManager = UhidManager; mod tests { use super::*; + // Every `enable` vector in this module is chosen so its assertion holds whichever bits the + // TRIGGER actuators turn out to use — the assignment is conjecture (see the ⚠️ on + // `parse_xbox_output`) and a test asserting it would pin a guess as if it were the contract. + // The safe masks: `0xFF` enables everything that exists, `0x00` enables nothing, and + // `0x0C` / `0xF3` split the two MEASURED handle bits from every other bit. No vector below + // names a trigger enable bit. + + /// Both handle motors at full scale, with the triggers idle. #[test] fn rumble_scales_off_the_zero_to_hundred_protocol_range() { - // Both motors enabled, full scale. let full = [0x03, 0x0F, 0, 0, 100, 100, 0, 0, 1]; - assert_eq!(parse_xbox_output(&full), Some((65535, 65535))); + assert_eq!(parse_xbox_output(&full), Some((65535, 65535, 0, 0))); // Half on the left motor only. let half = [0x03, 0x04, 0, 0, 50, 100, 0, 0, 1]; - assert_eq!(parse_xbox_output(&half), Some((32767, 0))); + assert_eq!(parse_xbox_output(&half), Some((32767, 0, 0, 0))); } - /// A value above the protocol's 0..100 range must clamp, not wrap past full scale. + /// The trigger magnitudes are on the SAME 0..100 protocol range as the handles, so a + /// full-scale `100` is `65535` — not `25700`, which is what reading them as 0..255 would give + /// and which reads on a real pad as "trigger rumble works but is weirdly weak". Named for the + /// regression so it cannot be "fixed" the wrong way later. + #[test] + fn trigger_magnitudes_are_not_a_zero_to_255_range() { + let full = [0x03, 0xFF, 100, 100, 0, 0, 0, 0, 1]; + assert_eq!(parse_xbox_output(&full), Some((0, 0, 65535, 65535))); + let half = [0x03, 0xFF, 50, 25, 0, 0, 0, 0, 1]; + assert_eq!(parse_xbox_output(&half), Some((0, 0, 32767, 16383))); + } + + /// A value above the protocol's 0..100 range must clamp, not wrap past full scale — on all + /// four actuators, since the triggers reuse the handles' scale closure. #[test] fn out_of_range_magnitudes_clamp() { - let over = [0x03, 0x0F, 0, 0, 255, 255, 0, 0, 1]; - assert_eq!(parse_xbox_output(&over), Some((65535, 65535))); + let over = [0x03, 0xFF, 255, 255, 255, 255, 0, 0, 1]; + assert_eq!(parse_xbox_output(&over), Some((65535, 65535, 65535, 65535))); } - /// The enable mask gates each motor independently — a report that enables neither is a stop. + /// The enable mask gates each motor independently — a report that enables nothing is a stop. #[test] fn the_enable_mask_gates_each_motor() { - let none = [0x03, 0x00, 0, 0, 100, 100, 0, 0, 1]; - assert_eq!(parse_xbox_output(&none), Some((0, 0))); + let none = [0x03, 0x00, 100, 100, 100, 100, 0, 0, 1]; + assert_eq!(parse_xbox_output(&none), Some((0, 0, 0, 0))); let right_only = [0x03, 0x08, 0, 0, 100, 100, 0, 0, 1]; - assert_eq!(parse_xbox_output(&right_only), Some((0, 65535))); + assert_eq!(parse_xbox_output(&right_only), Some((0, 65535, 0, 0))); + } + + /// The case the whole trigger-rumble plane exists for, and the one nothing else in the tree + /// can produce: a racing title driving the impulse triggers hard while the handles stay + /// silent. `0x0C` is the two measured handle bits; `0xF3` is every OTHER bit, so this pair + /// isolates the handles from the triggers without claiming which bits the triggers are. + #[test] + fn a_trigger_only_report_leaves_the_handles_silent() { + let triggers_only = [0x03, 0xF3, 100, 40, 100, 100, 0, 0, 1]; + assert_eq!( + parse_xbox_output(&triggers_only), + Some((0, 0, 65535, 26214)) + ); + let handles_only = [0x03, 0x0C, 100, 100, 100, 100, 0, 0, 1]; + assert_eq!(parse_xbox_output(&handles_only), Some((65535, 65535, 0, 0))); } /// Anything that is not the rumble report — or is truncated — is ignored rather than parsed diff --git a/crates/pf-inject/src/lib.rs b/crates/pf-inject/src/lib.rs index ac91d2e8..01260fd0 100644 --- a/crates/pf-inject/src/lib.rs +++ b/crates/pf-inject/src/lib.rs @@ -500,7 +500,7 @@ pub mod gamepad { GamepadManager } pub fn handle(&mut self, _ev: &punktfunk_core::input::GamepadEvent) {} - pub fn pump_rumble(&mut self, _send: impl FnMut(u16, u16, u16)) {} + pub fn pump_rumble(&mut self, _send: impl FnMut(u16, u16, u16, u16, u16)) {} } } /// Linux: the "Punktfunk Pen" uinput virtual tablet (design/pen-tablet-input.md §5) — the diff --git a/crates/punktfunk-core/cbindgen.toml b/crates/punktfunk-core/cbindgen.toml index 428cd854..11de6b6d 100644 --- a/crates/punktfunk-core/cbindgen.toml +++ b/crates/punktfunk-core/cbindgen.toml @@ -204,6 +204,7 @@ include = ["PunktfunkEndReason"] "RICH_INPUT_MAGIC" = "PUNKTFUNK_RICH_INPUT_MAGIC" "RUMBLE_V1_LEN" = "PUNKTFUNK_RUMBLE_V1_LEN" "RUMBLE_V2_LEN" = "PUNKTFUNK_RUMBLE_V2_LEN" +"RUMBLE_V3_LEN" = "PUNKTFUNK_RUMBLE_V3_LEN" "SETUP_FAILED_CLOSE_CODE" = "PUNKTFUNK_SETUP_FAILED_CLOSE_CODE" "TAG_LEN" = "PUNKTFUNK_TAG_LEN" "TRIGGER_EFFECT_MAX" = "PUNKTFUNK_TRIGGER_EFFECT_MAX" diff --git a/crates/punktfunk-core/src/client/pump/datagram_task.rs b/crates/punktfunk-core/src/client/pump/datagram_task.rs index eceb6474..a08ea68b 100644 --- a/crates/punktfunk-core/src/client/pump/datagram_task.rs +++ b/crates/punktfunk-core/src/client/pump/datagram_task.rs @@ -91,6 +91,15 @@ pub(super) async fn run( let ttl = u.envelope.map(|e| e.ttl_ms); // Both consumers are fed; an embedder drains exactly one of them // (the legacy queue, or the policy engine's command API). + // + // `u.left_trigger`/`u.right_trigger` (the v3 tail) are decoded and + // deliberately NOT forwarded yet: neither consumer has a slot for them. + // Widening them is the client-engine work package — `RumbleCommand` grows + // two fields, `ActuatorQuirks` learns whether the physical pad has trigger + // motors, and the C ABI gains a `next_rumble_cmd2` beside the existing + // fixed-out-param puller. Dropping them here is exactly what the §5 + // compatibility table calls "new host, old client": the handle motors + // behave identically and the trigger levels are silently discarded. let _ = rumble_tx.try_send((u.pad, u.low, u.high, ttl)); rumble_feed.wire_update(u.pad, u.low, u.high, ttl); } diff --git a/crates/punktfunk-core/src/quic/datagram.rs b/crates/punktfunk-core/src/quic/datagram.rs index c73243a8..8e907651 100644 --- a/crates/punktfunk-core/src/quic/datagram.rs +++ b/crates/punktfunk-core/src/quic/datagram.rs @@ -122,8 +122,9 @@ pub fn decode_audio_red_datagram(b: &[u8]) -> Option<(u32, u64, &[u8], Option<&[ /// Legacy rumble datagram (v1), host → client: `[0xCA][u16 pad LE][u16 low LE][u16 high LE]`. /// Force-feedback state for pad `pad` (0xFFFF amplitudes, 0/0 = stop) as *level-triggered* state /// — it persists until superseded, which is why the host re-sends it periodically as its loss -/// heal. New hosts emit the self-terminating [`encode_rumble_datagram_v2`] instead; this is kept -/// for the loopback tests and as the wire an old host still speaks (a new client decodes both via +/// heal. New hosts emit the self-terminating [`encode_rumble_datagram_v3`] instead; this is kept +/// for the loopback tests, as the wire an old host still speaks, and as what the +/// `PUNKTFUNK_RUMBLE_ENVELOPE=0` bisect hatch drops to (a new client decodes every form via /// [`decode_rumble_envelope`]). pub fn encode_rumble_datagram(pad: u16, low: u16, high: u16) -> [u8; 7] { let mut b = [0u8; 7]; @@ -141,6 +142,12 @@ pub const RUMBLE_V1_LEN: usize = 7; /// first 7 bytes as a plain level and ignores the tail, so no wire-version bump is needed — the /// same dual-size idiom the HDR-luminance `AddRequest` tail uses. pub const RUMBLE_V2_LEN: usize = 10; +/// Wire length of a v3 (envelope + impulse-trigger motors) rumble datagram — the v2 form plus a +/// `[u16 left_trigger LE][u16 right_trigger LE]` tail (see [`encode_rumble_datagram_v3`]). Second +/// use of the same append-extension the v2 tail introduced, and for the same reason: every reader +/// on this plane gates with `>=`, so a 14-byte datagram satisfies the v1 predicate (level only), +/// the v2 predicate (level + envelope) and this one, and each peer takes the prefix it knows. +pub const RUMBLE_V3_LEN: usize = 14; /// Rumble envelope datagram (v2), host → client: /// `[0xCA][u16 pad LE][u16 low LE][u16 high LE][u8 seq][u16 ttl_ms LE]`. @@ -163,6 +170,41 @@ pub fn encode_rumble_datagram_v2(pad: u16, low: u16, high: u16, seq: u8, ttl_ms: b } +/// Rumble envelope datagram with the impulse-trigger motors (v3), host → client: +/// `[0xCA][u16 pad LE][u16 low LE][u16 high LE][u8 seq][u16 ttl_ms LE][u16 lt LE][u16 rt LE]`. +/// +/// The [`encode_rumble_datagram_v2`] envelope with the Xbox trigger motors appended, on the same +/// `0..=0xFFFF` scale as `low`/`high` (design/trigger-rumble-plane.md §4). +/// +/// **The four levels share ONE `seq` and ONE `ttl_ms`, deliberately.** They are a single statement +/// of the pad's feedback state at one instant; a second sequence space would let a reordered +/// datagram apply the handles from moment *t* and the triggers from *t−1*, a glitch nothing else +/// in the system can currently produce. Sharing also means the whole v2 apparatus — the renewal +/// cadence, the post-stop burst, the client's wrapping half-space `seq` gate, the receiver-side +/// lease clamp — governs the trigger motors with no new code, so a trigger rumble whose host dies +/// self-silences on the same lease as the handles. +/// +/// Exactly one backend can ever source non-zero trigger levels: the Windows HID Xbox pad, whose +/// output report `0x03` carries them. Classic XInput's `XINPUT_VIBRATION` and evdev's `FF_RUMBLE` +/// have two members and no third, so every other producer passes `lt = rt = 0` — for those this is +/// a v2 datagram with four zero bytes on the end, which is exactly what the length tolerance is +/// for. +pub fn encode_rumble_datagram_v3( + pad: u16, + low: u16, + high: u16, + seq: u8, + ttl_ms: u16, + lt: u16, + rt: u16, +) -> [u8; RUMBLE_V3_LEN] { + let mut b = [0u8; RUMBLE_V3_LEN]; + b[..RUMBLE_V2_LEN].copy_from_slice(&encode_rumble_datagram_v2(pad, low, high, seq, ttl_ms)); + b[10..12].copy_from_slice(<.to_le_bytes()); + b[12..14].copy_from_slice(&rt.to_le_bytes()); + b +} + /// The self-termination tail of a v2 rumble envelope (see [`encode_rumble_datagram_v2`]). #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct RumbleEnvelope { @@ -174,17 +216,28 @@ pub struct RumbleEnvelope { /// A decoded rumble update. `envelope` is `None` for a legacy 7-byte datagram (an old host, which /// has no seq/ttl — the client applies its own staleness policy), `Some` for a v2 envelope. +/// +/// `left_trigger`/`right_trigger` are the Xbox impulse-trigger motors from a v3 datagram, on the +/// same `0..=0xFFFF` scale as `low`/`high`, and they are **plain fields, not `Option`** even though +/// only a v3 datagram carries them. A v1/v2 datagram decodes to `left_trigger = right_trigger = 0`. +/// The temptation is to mirror `envelope` so a consumer could tell "old host" from "new host, +/// triggers idle", but `Option` invites "absent → keep the previous value", and on a +/// level-triggered plane that is the stuck-rumble bug in a new costume: `0xCA` means *these are the +/// levels now*, so an absent field is zero. (`envelope` is genuinely optional because its absence +/// selects a different *policy*, not a different level.) #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct RumbleUpdate { pub pad: u16, pub low: u16, pub high: u16, + pub left_trigger: u16, + pub right_trigger: u16, pub envelope: Option, } -/// Parse a rumble datagram → `(pad, low, high)`, tolerating (and ignoring) a v2 envelope tail. -/// `None` on bad tag/length. Kept for callers that only need the level (the probe, the loopback -/// assertions); clients that honor TTL use [`decode_rumble_envelope`]. +/// Parse a rumble datagram → `(pad, low, high)`, tolerating (and ignoring) the v2 envelope and v3 +/// trigger tails. `None` on bad tag/length. Kept for callers that only need the handle level (the +/// probe, the loopback assertions); clients that honor TTL use [`decode_rumble_envelope`]. pub fn decode_rumble_datagram(b: &[u8]) -> Option<(u16, u16, u16)> { if b.len() < RUMBLE_V1_LEN || b[0] != RUMBLE_MAGIC { return None; @@ -193,10 +246,15 @@ pub fn decode_rumble_datagram(b: &[u8]) -> Option<(u16, u16, u16)> { Some((u16at(1), u16at(3), u16at(5))) } -/// Parse a rumble datagram → [`RumbleUpdate`], detecting the v2 envelope tail by length. A -/// `>= RUMBLE_V2_LEN` buffer carries `seq`/`ttl_ms`; a 7..RUMBLE_V2_LEN buffer is a legacy level +/// Parse a rumble datagram → [`RumbleUpdate`], detecting each appended tail by length. A +/// `>= RUMBLE_V2_LEN` buffer carries `seq`/`ttl_ms`; a `>= RUMBLE_V3_LEN` buffer additionally +/// carries the two impulse-trigger levels; a 7..RUMBLE_V2_LEN buffer is a legacy level /// (`envelope: None`) — the same tolerance as an old client would apply, so a torn/short tail /// degrades to a level rather than dropping. `None` on bad tag/length. +/// +/// The one decoder for all three forms: v3 is not a separate wire, it is the same wire with more +/// of it present. Absent trigger bytes read as zero rather than "unchanged" — see +/// [`RumbleUpdate`] for why that is not negotiable on a level-triggered plane. pub fn decode_rumble_envelope(b: &[u8]) -> Option { if b.len() < RUMBLE_V1_LEN || b[0] != RUMBLE_MAGIC { return None; @@ -206,10 +264,13 @@ pub fn decode_rumble_envelope(b: &[u8]) -> Option { seq: b[7], ttl_ms: u16::from_le_bytes([b[8], b[9]]), }); + let triggers = b.len() >= RUMBLE_V3_LEN; Some(RumbleUpdate { pad: u16at(1), low: u16at(3), high: u16at(5), + left_trigger: if triggers { u16at(10) } else { 0 }, + right_trigger: if triggers { u16at(12) } else { 0 }, envelope, }) } @@ -1196,6 +1257,8 @@ mod tests { pad: 2, low: 0x4000, high: 0x8000, + left_trigger: 0, + right_trigger: 0, envelope: Some(RumbleEnvelope { seq: 7, ttl_ms: 400 @@ -1215,6 +1278,8 @@ mod tests { pad: 3, low: 0x1111, high: 0x2222, + left_trigger: 0, + right_trigger: 0, envelope: None, }) ); @@ -1237,6 +1302,90 @@ mod tests { assert!(decode_rumble_envelope(&wrong_tag).is_none()); } + /// v3 (design/trigger-rumble-plane.md §4) is the v2 envelope with the two impulse-trigger + /// levels appended, and the prefix discipline the 0xCF plane uses three times over holds here + /// too: the first 10 bytes must be byte-identical to what v2 would have produced, or the + /// envelope a v2-era client reads is displaced and every TTL/seq guarantee on this plane + /// silently changes meaning. + #[test] + fn rumble_v3_roundtrips_and_keeps_the_v2_envelope_in_place() { + let v2 = encode_rumble_datagram_v2(2, 0x4000, 0x8000, 7, 400); + let v3 = encode_rumble_datagram_v3(2, 0x4000, 0x8000, 7, 400, 0x1234, 0xFFFF); + assert_eq!(v3.len(), RUMBLE_V3_LEN); + assert_eq!(&v3[..RUMBLE_V2_LEN], &v2[..], "v2 is a strict prefix of v3"); + // The exact tail layout, LE, pinned as bytes: an endianness slip here reads a 0x1234 + // trigger as 0x3412 and is invisible in a round-trip that uses the same encoder both ways. + assert_eq!(&v3[10..14], &[0x34, 0x12, 0xFF, 0xFF]); + assert_eq!( + decode_rumble_envelope(&v3), + Some(RumbleUpdate { + pad: 2, + low: 0x4000, + high: 0x8000, + left_trigger: 0x1234, + right_trigger: 0xFFFF, + envelope: Some(RumbleEnvelope { + seq: 7, + ttl_ms: 400 + }), + }) + ); + // A trigger-only rumble (racing titles drive the triggers hard and the handles not at all) + // is expressible and survives the trip with the handles at rest. + let trig_only = encode_rumble_datagram_v3(0, 0, 0, 3, 400, 0x8000, 0); + let u = decode_rumble_envelope(&trig_only).unwrap(); + assert_eq!((u.low, u.high), (0, 0)); + assert_eq!((u.left_trigger, u.right_trigger), (0x8000, 0)); + assert_eq!(u.envelope.unwrap().ttl_ms, 400); + } + + /// Cross-version tolerance, both directions — the compatibility table in + /// design/trigger-rumble-plane.md §5, as code. + #[test] + fn rumble_v3_and_v2_parse_each_others_datagrams() { + let v3 = encode_rumble_datagram_v3(1, 0x1111, 0x2222, 9, 250, 0xAAAA, 0xBBBB); + + // NEW host → OLD client: the v2-era readers see exactly what they saw before. The level + // decoder ignores both tails; the envelope decoder reads the same seq/ttl off bytes 7..10. + assert_eq!(decode_rumble_datagram(&v3), Some((1, 0x1111, 0x2222))); + assert_eq!( + decode_rumble_envelope(&v3).unwrap().envelope, + Some(RumbleEnvelope { + seq: 9, + ttl_ms: 250 + }) + ); + + // OLD host → NEW client: v1 and v2 decode with the triggers SILENT, not "unchanged". + for (form, d) in [ + ("v1", encode_rumble_datagram(1, 0x1111, 0x2222).to_vec()), + ( + "v2", + encode_rumble_datagram_v2(1, 0x1111, 0x2222, 9, 250).to_vec(), + ), + ] { + let u = decode_rumble_envelope(&d).unwrap(); + assert_eq!( + (u.left_trigger, u.right_trigger), + (0, 0), + "{form} must decode to idle triggers" + ); + assert_eq!((u.pad, u.low, u.high), (1, 0x1111, 0x2222)); + } + + // A torn trigger tail (11..14 bytes — the host never emits these, a truncating middlebox + // might) degrades to the v2 decode rather than reading half a level: a 13-byte buffer must + // not surface `rt` from one byte of it. + let v2 = decode_rumble_envelope(&encode_rumble_datagram_v2(1, 0x1111, 0x2222, 9, 250)); + for n in RUMBLE_V2_LEN..RUMBLE_V3_LEN { + assert_eq!( + decode_rumble_envelope(&v3[..n]), + v2, + "partial trigger tail ({n} B) must degrade to the v2 decode" + ); + } + } + #[test] fn rumble_envelope_seq_gate_drops_reordered_stale_start() { use crate::input::GamepadSnapshot; diff --git a/crates/punktfunk-host/src/devtest.rs b/crates/punktfunk-host/src/devtest.rs index 47509241..269c76ea 100644 --- a/crates/punktfunk-host/src/devtest.rs +++ b/crates/punktfunk-host/src/devtest.rs @@ -270,8 +270,10 @@ pub fn switchpro_test(args: &[String]) -> Result<()> { let (mut i, mut last_write) = (0i32, Instant::now()); while Instant::now() < deadline { let fb = pad.service(0); - if let Some((low, high)) = fb.rumble { - println!(" rumble from kernel/game: low={low} high={high}"); + // `lt`/`rt` are structurally always zero here — a Switch Pro has no trigger motors — + // but this harness reads the shared `PadFeedback`, so it prints all four levels. + if let Some((low, high, lt, rt)) = fb.rumble { + println!(" rumble from kernel/game: low={low} high={high} lt={lt} rt={rt}"); } for o in fb.hidout { println!(" hid output from kernel/game: {o:?}"); @@ -397,7 +399,9 @@ pub fn dualsense_windows_test(args: &[String]) -> Result<()> { let (mut i, mut last) = (0i32, Instant::now()); while Instant::now() < deadline { mgr.pump( - |pad, lo, hi| println!(" rumble from game: pad={pad} low={lo} high={hi}"), + |pad, lo, hi, lt, rt| println!( + " rumble from game: pad={pad} low={lo} high={hi} lt={lt} rt={rt}" + ), |o| println!(" hid output from game: {o:?}"), ); if last.elapsed() >= Duration::from_millis(400) { @@ -442,8 +446,10 @@ pub fn dualsense_windows_test(args: &[String]) -> Result<()> { let deadline = Instant::now() + Duration::from_secs(secs); let mut t = 0i32; while Instant::now() < deadline { - mgr.pump_rumble(|pad, lo, hi| { - println!(" rumble from game: pad={pad} low={lo} high={hi}") + // `lt`/`rt` are structurally always zero on XUSB (see `pump_rumble`); printed so + // the harness output is comparable line-for-line with the HID Xbox backend's. + mgr.pump_rumble(|pad, lo, hi, lt, rt| { + println!(" rumble from game: pad={pad} low={lo} high={hi} lt={lt} rt={rt}") }); t += 1; let lx = (((t % 200) - 100) * 327).clamp(-32768, 32767) as i16; // sweep ±32700 diff --git a/crates/punktfunk-host/src/gamestream/control.rs b/crates/punktfunk-host/src/gamestream/control.rs index a2206fa1..bd23f550 100644 --- a/crates/punktfunk-host/src/gamestream/control.rs +++ b/crates/punktfunk-host/src/gamestream/control.rs @@ -255,7 +255,13 @@ pub fn spawn(state: Arc) -> Result<()> { hdr_sent = true; } } - pads.pump_rumble(|index, low, high| { + // The GameStream leg carries the handle motors only: Moonlight's + // trigger-rumble message (`ConnListenerRumbleTriggers`) is a separate + // control-stream id we have not read out of moonlight-common-c yet, and + // `low`/`high` here are already what `rumble_plaintext` (0x010B) encodes. + // The uinput backend cannot source triggers anyway (evdev `FF_RUMBLE` has + // two fields), so nothing is dropped today. + pads.pump_rumble(|index, low, high, _lt, _rt| { let pt = super::gamepad::rumble_plaintext(index, low, high); out.push(encrypt_control(&key, &scheme, host_seq, &pt)); host_seq = host_seq.wrapping_add(1); @@ -269,7 +275,7 @@ pub fn spawn(state: Arc) -> Result<()> { } } else { // No client/scheme yet: still answer FF uploads so games don't block. - pads.pump_rumble(|_, _, _| {}); + pads.pump_rumble(|_, _, _, _, _| {}); } // ENet needs frequent servicing for handshake/keepalive/retransmit. std::thread::sleep(Duration::from_millis(2)); diff --git a/crates/punktfunk-host/src/native.rs b/crates/punktfunk-host/src/native.rs index be01a1f9..578e6f51 100644 --- a/crates/punktfunk-host/src/native.rs +++ b/crates/punktfunk-host/src/native.rs @@ -1452,9 +1452,14 @@ async fn serve_session( && std::env::var("PUNKTFUNK_TEST_FEEDBACK").as_deref() == Ok("1") { use punktfunk_core::quic::HidOutput; - // v2 envelope (seq 0, 400 ms TTL) so the loopback/probe assertion covers the self- - // terminating tail, not just the level. - let d = punktfunk_core::quic::encode_rumble_datagram_v2(0, 0x4000, 0x8000, 0, 400); + // v3 envelope (seq 0, 400 ms TTL, both impulse-trigger motors asserted) so the + // loopback/probe assertion covers the self-terminating tail AND the trigger tail behind + // it, not just the level. The trigger levels are deliberately DIFFERENT from each other + // and from the handles: a decoder that reads the wrong offset produces a plausible-looking + // number rather than a zero, so identical values would hide the mistake. + let d = punktfunk_core::quic::encode_rumble_datagram_v3( + 0, 0x4000, 0x8000, 0, 400, 0x2000, 0x6000, + ); let _ = conn.send_datagram(d.to_vec().into()); for h in [ HidOutput::Led { diff --git a/crates/punktfunk-host/src/native/gamepad.rs b/crates/punktfunk-host/src/native/gamepad.rs index 12c9ec15..b990a507 100644 --- a/crates/punktfunk-host/src/native/gamepad.rs +++ b/crates/punktfunk-host/src/native/gamepad.rs @@ -223,26 +223,41 @@ fn degrade_steam_on_conflict(chosen: GamepadPref) -> GamepadPref { /// Whether an Xbox-family pad should be built as a real **HID** device /// ([`crate::inject::xbox_windows`]) instead of the **XUSB** companion -/// ([`crate::inject::gamepad`]). Windows only; `PUNKTFUNK_XBOX_BACKEND=hid` opts in. +/// ([`crate::inject::gamepad`]). Windows only. **HID is the default**; set +/// `PUNKTFUNK_XBOX_BACKEND=xusb` to go back to the companion. /// -/// **Why this is a knob and not simply the new default.** The XUSB companion registers only -/// `GUID_DEVINTERFACE_XUSB` and exposes no HID collection, so Steam's hidapi enumeration, -/// DirectInput, `joy.cpl` and WGI/GameInput cannot see it at all — only classic `XInputGetState` -/// via xinput1_4's interface walk does. That is what left a reporter with a dead controller for two -/// weeks (2026-08-09) until they switched the client to DualSense, a real HID pad. +/// **Why HID is now the default.** The XUSB companion registers only `GUID_DEVINTERFACE_XUSB` and +/// exposes no HID collection, so Steam's hidapi enumeration, DirectInput, `joy.cpl` and +/// WGI/GameInput cannot see it at all — only classic `XInputGetState` via xinput1_4's interface walk +/// does. That is what left a reporter with a dead controller for two weeks (2026-08-09) until they +/// switched the client to DualSense, a real HID pad. /// -/// But the converse is not yet proven: classic-XInput games DO read the XUSB pad today, and whether -/// Windows promotes our HID pad into an Xbox-profile device that XInput and WGI `Gamepad` accept is -/// exactly the open question. Until that is settled on glass, flipping the default would trade a -/// known-working path for an unproven one. Opt in, measure, then decide. +/// This was an opt-in knob for exactly one reason: the HID pad could not reach classic XInput, so +/// defaulting to it would have traded a known-working path for an unproven one. **That objection is +/// gone.** `pf_gamepad.inx`'s `pfGamepadXbox` section now attaches the `xinputhid` bus filter +/// (`UpperFilters` + `DevicePropertyFlags=1`), and with it the HID pad is promoted exactly like real +/// hardware: measured on `.173` 2026-08-09 it gains the `IG_00` token and an XUSB interface, classic +/// XInput reads it live (full stick range and buttons), `XInputSetState` rumble round-trips, and it +/// keeps everything the XUSB companion never had — Steam, SDL, RawInput, DirectInput, `joy.cpl`. +/// ⇒ the HID backend is now a **superset** of the XUSB one, which is the condition the old comment +/// set for flipping. +/// +/// ⚠️ `xusb` stays as an escape hatch because the promotion depends on Microsoft's inbox +/// `xinputhid.inf` and its hardware-id allow-list. If a Windows servicing update changes that, or a +/// box has a third-party filter on the stack, one env var restores the previous behaviour without a +/// reinstall. /// /// The two backends are mutually exclusive per pad by construction (one match arm or the other) — /// presenting both would hand a game two controllers for one pair of hands. #[cfg(target_os = "windows")] pub(super) fn windows_xbox_hid() -> bool { - std::env::var("PUNKTFUNK_XBOX_BACKEND") - .map(|v| v.trim().eq_ignore_ascii_case("hid")) - .unwrap_or(false) + match std::env::var("PUNKTFUNK_XBOX_BACKEND") { + Ok(v) if v.trim().eq_ignore_ascii_case("xusb") => false, + // Anything else — unset, empty, "hid", or a typo — takes the default. A misspelled opt-out + // silently landing on the OLD path is the worse failure: it is invisible, and it is the + // path with no HID collection. + _ => true, + } } /// Resolve the client's gamepad-backend preference (the env/logging shell around diff --git a/crates/punktfunk-host/src/native/input.rs b/crates/punktfunk-host/src/native/input.rs index 3a1a56bd..bf0d4fc7 100644 --- a/crates/punktfunk-host/src/native/input.rs +++ b/crates/punktfunk-host/src/native/input.rs @@ -298,10 +298,11 @@ impl Pads { .steamdeck_win .get_or_insert_with(crate::inject::steam_deck_windows::SteamDeckWindowsManager::new) .handle(ev), - // The Xbox pad, as a real HID device rather than the XUSB companion. Opt-in for now - // (see `windows_xbox_hid`): XUSB is what classic-XInput games read today, and this - // trades that for the Steam / WGI / GameInput / DirectInput visibility XUSB can never - // have — a swap that has to be proven on glass before it becomes the default. + // The Xbox pad, as a real HID device rather than the XUSB companion. This is now the + // DEFAULT (see `windows_xbox_hid`; `PUNKTFUNK_XBOX_BACKEND=xusb` reverts it). It is no + // longer a trade: with the `xinputhid` bus filter the INF attaches, the HID pad keeps + // classic XInput AND gains everything XUSB never had — Steam, SDL, RawInput, + // DirectInput, `joy.cpl`, WGI — plus rumble, which the XUSB path could not source. #[cfg(target_os = "windows")] GamepadPref::Xbox360 | GamepadPref::XboxOne if super::gamepad::windows_xbox_hid() => { self.xbox_hid @@ -425,12 +426,17 @@ impl Pads { } /// Service feedback for every instantiated backend each cycle. `rumble` carries motor - /// force-feedback on the universal plane (every backend, tagged with its own pad index); - /// `hidout` carries rich feedback (lightbar / player LEDs / adaptive triggers) for the UHID/UMDF - /// pads. The `&mut` closure re-borrows satisfy `FnMut` for each backend. + /// force-feedback on the universal plane (every backend, tagged with its own pad index) as + /// `(pad, low, high, left_trigger, right_trigger)`; `hidout` carries rich feedback (lightbar / + /// player LEDs / adaptive triggers) for the UHID/UMDF pads. The `&mut` closure re-borrows + /// satisfy `FnMut` for each backend. + /// + /// Only the Windows HID Xbox backend (`xbox_hid`) can ever report non-zero trigger levels — no + /// other backend's source packet has a field for them (see `PadFeedback::rumble`), so they pass + /// zeros and the v3 datagram they produce is a v2 datagram with a zero tail. fn pump( &mut self, - mut rumble: impl FnMut(u16, u16, u16), + mut rumble: impl FnMut(u16, u16, u16, u16, u16), mut hidout: impl FnMut(punktfunk_core::quic::HidOutput), ) { if let Some(m) = &mut self.xbox360 { @@ -753,26 +759,58 @@ const RUMBLE_STOP_BURST: u8 = 2; /// life of the connection because the client gates on it with a wrapping half-space compare and /// never resets its side (`punktfunk-core/src/client/pump/datagram_task.rs`). Resetting it here is /// the bug pinned by [`tests::rumble_seq_survives_a_removal_so_the_client_gate_accepts`]. -fn clear_pad_feedback(state: &mut (u16, u16), seen: &mut bool, stop_burst: &mut u8) { - *state = (0, 0); +fn clear_pad_feedback(state: &mut RumbleLevels, seen: &mut bool, stop_burst: &mut u8) { + *state = (0, 0, 0, 0); *seen = false; *stop_burst = 0; } +/// One pad's four motor levels as the 0xCA plane orders them: +/// `(low, high, left_trigger, right_trigger)`, all `0..=0xFFFF`. Kept as one value rather than four +/// parallel arrays because they are a single statement of the pad's feedback state at one instant — +/// the same reason they share one `seq` and one TTL on the wire. +type RumbleLevels = (u16, u16, u16, u16); + +/// Is this pad's feedback fully silent? **All four** motors, and that is the whole point of it +/// being a named predicate rather than an inline comparison repeated at each site. +/// +/// Every "is this pad quiet?" decision in the rumble path routes through here: whether to log the +/// silent→active transition, whether to arm the post-stop burst, and — the one that decides +/// whether the feature works at all — whether the envelope gets a live TTL or the `0` that means +/// *stop*. Written as a two-field test, a trigger-only rumble (the normal shape of +/// impulse-trigger content: racing titles drive the triggers continuously while the handles stay +/// near-silent) is stamped `ttl = 0`, the client reads an already-expired lease and silences on +/// arrival, and nothing anywhere logs an error. See +/// [`tests::a_trigger_only_rumble_gets_a_live_ttl`]. +fn rumble_silent(lv: RumbleLevels) -> bool { + lv == (0, 0, 0, 0) +} + /// Send one rumble datagram on the universal 0xCA plane. `envelope_on` picks the self-terminating -/// v2 form (`[level][seq][ttl_ms]`, the default) or the legacy v1 level datagram (the -/// `PUNKTFUNK_RUMBLE_ENVELOPE=0` bisect hatch). Best-effort like every side-plane datagram. +/// v3 form (`[level][seq][ttl_ms][trigger levels]`, the default) or the legacy v1 level datagram +/// (the `PUNKTFUNK_RUMBLE_ENVELOPE=0` bisect hatch). Best-effort like every side-plane datagram. +/// +/// v3 goes out **unconditionally** while the envelope is on — not "only when a trigger level is +/// non-zero". A wire form that depends on history is how you get a bug that reproduces only after a +/// specific sequence of events, and the four extra bytes cost nothing: a client that predates v3 +/// reads the 10-byte prefix and ignores them. +/// +/// ⚠️ The bisect hatch drops to v1, which takes trigger rumble down with it (v1 has no tail at +/// all). That is correct for a hatch whose job is to reproduce the pre-envelope wire, but it means +/// "trigger rumble stopped working" is an expected symptom of setting it — do not bisect a trigger +/// bug into this hatch and conclude the hatch fixed it. fn send_rumble( conn: &quinn::Connection, envelope_on: bool, pad: u16, - low: u16, - high: u16, + lv: RumbleLevels, seq: u8, ttl_ms: u16, ) { + let (low, high, lt, rt) = lv; let d: Vec = if envelope_on { - punktfunk_core::quic::encode_rumble_datagram_v2(pad, low, high, seq, ttl_ms).to_vec() + punktfunk_core::quic::encode_rumble_datagram_v3(pad, low, high, seq, ttl_ms, lt, rt) + .to_vec() } else { punktfunk_core::quic::encode_rumble_datagram(pad, low, high).to_vec() }; @@ -789,11 +827,14 @@ fn send_rumble( /// the session; the pointer/keyboard injector (and its portal grant) lives in the service, /// across sessions. /// -/// Rumble is emitted as self-terminating 0xCA v2 envelopes (`[level][seq][ttl_ms]`): the host owns -/// the timeline, renewing an active level every ~`RUMBLE_TTL_MS × 3/10` ms and letting an -/// abandoned one expire client-side, so "stuck rumble" is inexpressible on the wire (see -/// `punktfunk-planning/design/rumble-envelope-plan.md`). `PUNKTFUNK_RUMBLE_ENVELOPE=0` reverts to -/// legacy v1 level datagrams + the flat 500 ms refresh (bisect hatch). +/// Rumble is emitted as self-terminating 0xCA v3 envelopes +/// (`[level][seq][ttl_ms][trigger levels]`): the host owns the timeline, renewing an active level +/// every ~`RUMBLE_TTL_MS × 3/10` ms and letting an abandoned one expire client-side, so "stuck +/// rumble" is inexpressible on the wire (see `punktfunk-planning/design/rumble-envelope-plan.md` +/// and `design/trigger-rumble-plane.md`). The four motors share one `seq` and one TTL, so the +/// trigger pair inherits the whole envelope apparatus unchanged. +/// `PUNKTFUNK_RUMBLE_ENVELOPE=0` reverts to legacy v1 level datagrams + the flat 500 ms refresh +/// (bisect hatch — which drops trigger rumble with it, see [`send_rumble`]). pub(super) fn input_thread( rx: std::sync::mpsc::Receiver, conn: quinn::Connection, @@ -814,14 +855,20 @@ pub(super) fn input_thread( // Last applied snapshot seq per pad (`None` until the first one): the reorder gate for // `InputKind::GamepadState` — a late datagram with an older seq must not roll held state back. let mut pad_seq: [Option; MAX_WIRE_PADS] = [None; MAX_WIRE_PADS]; - // Rumble self-terminating envelopes (0xCA v2). Each non-zero level is authorized for + // Rumble self-terminating envelopes (0xCA v3). Each non-zero level is authorized for // `rumble_ttl_ms`; the host renews an active pad every `rumble_renew` and lets an abandoned // one expire on the client, so a dropped transition heals on the next renewal and a stop that // is lost heals via the stop burst (or the client's own TTL expiry). `rumble_seq` is the // per-pad wrapping reorder counter (bumped on changes AND renewals) the client gates on; // `rumble_stop_burst` counts the post-stop zero re-sends still owed. `PUNKTFUNK_RUMBLE_ENVELOPE=0` // reverts to legacy v1 datagrams re-sent flat every 500 ms. - let mut rumble_state = [(0u16, 0u16); MAX_WIRE_PADS]; + // + // `rumble_state` holds ALL FOUR levels (see `RumbleLevels`), and every "is this pad silent?" + // test below is an all-four-zero test for one specific reason: a trigger-only rumble — the + // normal shape of impulse-trigger content, since racing titles drive the triggers continuously + // against near-silent handles — would otherwise be stamped `ttl = 0`, which the client reads as + // an instantly-expired lease. That is trigger rumble that never plays, with no error anywhere. + let mut rumble_state = [(0u16, 0u16, 0u16, 0u16); MAX_WIRE_PADS]; let mut rumble_seen = [false; MAX_WIRE_PADS]; let mut rumble_seq = [0u8; MAX_WIRE_PADS]; let mut rumble_stop_burst = [0u8; MAX_WIRE_PADS]; @@ -1036,43 +1083,50 @@ pub(super) fn input_thread( // EVIOCSFF, and HID handshakes must be answered promptly). Rumble → the universal 0xCA // plane; rich/raw HID feedback → 0xCD. pads.pump( - |pad, low, high| { + |pad, low, high, lt, rt| { + let lv: RumbleLevels = (low, high, lt, rt); + let silent = rumble_silent(lv); let idx = pad as usize; if idx < MAX_WIRE_PADS { let prev = rumble_state[idx]; // Log the silent→active transition (once per buzz) so a live test can tell // "host never gets rumble from the game" apart from "client doesn't render it". - if prev == (0, 0) && (low != 0 || high != 0) { - tracing::debug!(pad, low, high, "rumble: forwarding to client (0xCA)"); + // It carries `lt`/`rt` because it is the attribution line for exactly the + // trigger case too — without them a "triggers never buzzed" report cannot be + // split into "the host never saw them" and "the client never rendered them". + if rumble_silent(prev) && !silent { + tracing::debug!( + pad, + low, + high, + lt, + rt, + "rumble: forwarding to client (0xCA)" + ); } - rumble_state[idx] = (low, high); + rumble_state[idx] = lv; rumble_seen[idx] = true; // Bump the reorder counter on every change, then arm the stop burst on a // transition to zero (so a lost stop still reaches a legacy client) and clear // it when the game re-asserts a non-zero level. rumble_seq[idx] = rumble_seq[idx].wrapping_add(1); - if (low, high) == (0, 0) { - rumble_stop_burst[idx] = if prev != (0, 0) { RUMBLE_STOP_BURST } else { 0 }; + if silent { + rumble_stop_burst[idx] = if !rumble_silent(prev) { + RUMBLE_STOP_BURST + } else { + 0 + }; } else { rumble_stop_burst[idx] = 0; } - let ttl = if (low, high) == (0, 0) { - 0 - } else { - rumble_ttl_ms - }; - send_rumble( - &conn, - rumble_envelope_on, - pad, - low, - high, - rumble_seq[idx], - ttl, - ); + // A pad with ANY of its four motors asserted gets a live lease. Testing only + // `(low, high)` here would stamp a trigger-only rumble `ttl = 0` — an + // already-expired lease the client silences on arrival. + let ttl = if silent { 0 } else { rumble_ttl_ms }; + send_rumble(&conn, rumble_envelope_on, pad, lv, rumble_seq[idx], ttl); } else { // Out-of-range pad (a backend never produces these) — forward without gating. - send_rumble(&conn, rumble_envelope_on, pad, low, high, 0, rumble_ttl_ms); + send_rumble(&conn, rumble_envelope_on, pad, lv, 0, rumble_ttl_ms); } }, |h| { @@ -1092,27 +1146,21 @@ pub(super) fn input_thread( if !rumble_seen[i] { continue; } - let (low, high) = rumble_state[i]; - if (low, high) != (0, 0) { + let lv = rumble_state[i]; + if !rumble_silent(lv) { rumble_seq[i] = rumble_seq[i].wrapping_add(1); - send_rumble( - &conn, - true, - i as u16, - low, - high, - rumble_seq[i], - rumble_ttl_ms, - ); + send_rumble(&conn, true, i as u16, lv, rumble_seq[i], rumble_ttl_ms); } else if rumble_stop_burst[i] > 0 { rumble_stop_burst[i] -= 1; rumble_seq[i] = rumble_seq[i].wrapping_add(1); - send_rumble(&conn, true, i as u16, 0, 0, rumble_seq[i], 0); + send_rumble(&conn, true, i as u16, (0, 0, 0, 0), rumble_seq[i], 0); } } } else { - // Legacy: re-send the current level of every seen pad every 500 ms (v1). - for (i, &(low, high)) in rumble_state.iter().enumerate() { + // Legacy: re-send the current level of every seen pad every 500 ms (v1). The + // trigger levels are dropped here by construction — v1 has no tail (see + // `send_rumble`). + for (i, &(low, high, _, _)) in rumble_state.iter().enumerate() { if rumble_seen[i] { let d = punktfunk_core::quic::encode_rumble_datagram(i as u16, low, high); let _ = conn.send_datagram(d.to_vec().into()); @@ -1281,11 +1329,12 @@ mod tests { assert_eq!(gate, Some(100)); // The pad is unplugged mid-buzz: the lease is cleared, the counter is not. - let (mut state, mut seen, mut burst) = ((0x1234u16, 0x5678u16), true, RUMBLE_STOP_BURST); + let (mut state, mut seen, mut burst) = + ((0x1234, 0x5678, 0x9ABC, 0xDEF0), true, RUMBLE_STOP_BURST); clear_pad_feedback(&mut state, &mut seen, &mut burst); assert_eq!( (state, seen, burst), - ((0, 0), false, 0), + ((0, 0, 0, 0), false, 0), "lease not cleared" ); @@ -1327,4 +1376,49 @@ mod tests { assert_eq!(s.left_trigger, 255); assert!(!s.apply(&gp(InputKind::GamepadAxis, 42, 1, 0))); } + + /// The single most likely way to ship trigger rumble broken (design/trigger-rumble-plane.md + /// §5): a rumble that drives ONLY the impulse triggers must still get a live lease. + /// + /// The pre-existing silence test was `(low, high) == (0, 0)`, and a trigger-only level passes + /// it. Stamped `ttl = 0`, the envelope reaches the client as an already-expired lease, which + /// it silences on arrival — trigger rumble that never plays, with no error on either side. + /// Drives the real predicate and the real encoder/decoder pair, so it fails if either moves. + #[test] + fn a_trigger_only_rumble_gets_a_live_ttl() { + use punktfunk_core::quic::{decode_rumble_envelope, encode_rumble_datagram_v3}; + + // What a racing title's impulse-trigger stream looks like: handles at rest throughout. + let trigger_only: RumbleLevels = (0, 0, 0x8000, 0); + assert!( + !rumble_silent(trigger_only), + "a trigger-only level was read as silence — the ttl=0 trap" + ); + let ttl = if rumble_silent(trigger_only) { + 0 + } else { + RUMBLE_TTL_MS + }; + let d = encode_rumble_datagram_v3(0, 0, 0, 1, ttl, trigger_only.2, trigger_only.3); + let u = decode_rumble_envelope(&d).expect("v3 envelope decodes"); + assert_eq!( + u.envelope.expect("v3 carries the v2 tail").ttl_ms, + RUMBLE_TTL_MS, + "trigger-only rumble was stamped with a dead lease" + ); + assert_eq!((u.left_trigger, u.right_trigger), (0x8000, 0)); + assert_eq!((u.low, u.high), (0, 0), "handles stay at rest"); + + // The reserved stop is still expressible, and is still the ONLY thing that gets ttl = 0. + assert!(rumble_silent((0, 0, 0, 0))); + for lv in [ + (1, 0, 0, 0), + (0, 1, 0, 0), + (0, 0, 1, 0), + (0, 0, 0, 1), + (0xFFFF, 0xFFFF, 0xFFFF, 0xFFFF), + ] { + assert!(!rumble_silent(lv), "{lv:?} must not read as a stop"); + } + } } diff --git a/include/punktfunk_core.h b/include/punktfunk_core.h index 079fccd8..8aea2f81 100644 --- a/include/punktfunk_core.h +++ b/include/punktfunk_core.h @@ -1196,6 +1196,15 @@ #define PUNKTFUNK_RUMBLE_V2_LEN 10 #endif +#if defined(PUNKTFUNK_FEATURE_QUIC) +// Wire length of a v3 (envelope + impulse-trigger motors) rumble datagram — the v2 form plus a +// `[u16 left_trigger LE][u16 right_trigger LE]` tail (see [`encode_rumble_datagram_v3`]). Second +// use of the same append-extension the v2 tail introduced, and for the same reason: every reader +// on this plane gates with `>=`, so a 14-byte datagram satisfies the v1 predicate (level only), +// the v2 predicate (level + envelope) and this one, and each peer takes the prefix it knows. +#define PUNKTFUNK_RUMBLE_V3_LEN 14 +#endif + #if defined(PUNKTFUNK_FEATURE_QUIC) // Longest raw HID report a [`RichInput::HidReport`] / [`HidOutput::HidRaw`] can carry — the // 64-byte interrupt/feature report size every Valve controller uses (Triton input reports are