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