diff --git a/crates/pf-inject/src/inject/linux/gamepad.rs b/crates/pf-inject/src/inject/linux/gamepad.rs index cac39a69..435cf87d 100644 --- a/crates/pf-inject/src/inject/linux/gamepad.rs +++ b/crates/pf-inject/src/inject/linux/gamepad.rs @@ -669,6 +669,11 @@ impl GamepadManager { /// 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)) { + // 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 + // rumble internally); if that ever changes, consume it like the other two backends do. + 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); diff --git a/crates/pf-inject/src/inject/pad_slots.rs b/crates/pf-inject/src/inject/pad_slots.rs index 6f595ff6..8538fab7 100644 --- a/crates/pf-inject/src/inject/pad_slots.rs +++ b/crates/pf-inject/src/inject/pad_slots.rs @@ -62,15 +62,30 @@ impl

PadSlots

{ self.label } - /// Drop every allocated pad whose `active_mask` bit has stayed clear for [`SWEEP_GRACE`] (the - /// unplug sweep run on each state frame), logging each. Returns the swept indices as a bitmask - /// so the caller resets its per-index sibling state; an index another manager owns is `None` - /// here, so it is never swept. The grace is the devnode-churn debounce: a mask that glitches - /// clear for a few frames and returns re-arms nothing. + /// Fold one state frame's `active_mask` into the grace clocks, then drop whatever has run out + /// (see [`Self::reap`]). Returns the dropped indices as a bitmask so the caller resets its + /// per-index sibling state; an index another manager owns is `None` here, so it is never + /// touched. The grace is the devnode-churn debounce: a mask that glitches clear for a few + /// frames and returns re-arms nothing. + /// + /// A frame can only ARM the grace, never complete it — no time has passed at the instant the + /// clock starts. Since the producer emits exactly ONE frame per detach, [`Self::reap`] on the + /// manager's periodic pump is what actually finishes the unplug; a backend that only ever + /// called `sweep` would keep the detached pad alive for the rest of the session. pub fn sweep(&mut self, active_mask: u16) -> u16 { self.sweep_at(active_mask, Instant::now()) } + /// Drop every allocated pad whose grace has run out, logging each — the half of the unplug + /// that needs no state frame. Returns the dropped indices as a bitmask, same as [`Self::sweep`]. + /// + /// This can only ever *complete* an unplug some frame already started: it never arms a clock, + /// so however often it runs it cannot drop a pad whose `active_mask` bit never went clear. + /// That is what makes it safe to call from a hot pump loop. + pub fn reap(&mut self) -> u16 { + self.reap_at(Instant::now()) + } + /// Backdate every armed grace clock by [`SWEEP_GRACE`], so the NEXT sweep drops the pads /// whose bits are still clear — consumer tests (the managers') drive the debounce without /// wall-clock sleeps. Test-only: production code has no business expiring the grace. @@ -81,26 +96,37 @@ impl

PadSlots

{ } } - /// [`Self::sweep`] with an injectable clock (unit tests drive the grace window). + /// [`Self::sweep`] with an injectable clock (unit tests drive the grace window): arm or disarm + /// each slot's clock from the mask, then reap whatever has already run out. fn sweep_at(&mut self, active_mask: u16, now: Instant) -> u16 { - let mut swept = 0u16; - for (i, slot) in self.pads.iter_mut().enumerate() { + for i in 0..MAX_PADS { if active_mask & (1 << i) != 0 { self.inactive_since[i] = None; // active (again): a glitch never reaches the drop + } else if self.pads[i].is_some() && self.inactive_since[i].is_none() { + self.inactive_since[i] = Some(now); // newly inactive — start the grace + } + } + self.reap_at(now) + } + + /// [`Self::reap`] with an injectable clock. Deliberately arms nothing — it only ever reads + /// `inactive_since` and clears it, so a pad whose bit never went clear has no clock to run out + /// and cannot be dropped here. + fn reap_at(&mut self, now: Instant) -> u16 { + let mut swept = 0u16; + for i in 0..MAX_PADS { + let Some(since) = self.inactive_since[i] else { + continue; // active, or never went clear — nothing to complete + }; + if self.pads[i].is_none() { + self.inactive_since[i] = None; // the slot went away by some other route continue; } - if slot.is_none() { - continue; - } - match self.inactive_since[i] { - None => self.inactive_since[i] = Some(now), // newly inactive — start the grace - Some(since) if now.duration_since(since) >= SWEEP_GRACE => { - tracing::info!(index = i, "controller unplugged ({})", self.label); - *slot = None; - self.inactive_since[i] = None; - swept |= 1 << i; - } - Some(_) => {} // inside the grace — hold + if now.duration_since(since) >= SWEEP_GRACE { + tracing::info!(index = i, "controller unplugged ({})", self.label); + self.pads[i] = None; + self.inactive_since[i] = None; + swept |= 1 << i; } } swept @@ -161,6 +187,56 @@ mod tests { PadSlots::new("Test", "test pad", "") } + #[test] + fn a_single_frame_plus_a_reap_completes_the_unplug() { + // The shape production actually produces: ONE cleared-mask frame, then time, then a reap + // with no further frame. Before the arm/reap split the pad survived here forever. + let mut s = slots(); + assert!(s.ensure(2, |i| Ok(i as u32))); + assert_eq!( + s.sweep(0b0), + 0, + "a frame arms the grace but cannot itself drop" + ); + assert!(s.get(2).is_some()); + s.expire_grace(); + assert_eq!(s.reap(), 1 << 2, "the reap did not complete the unplug"); + assert!(s.get(2).is_none()); + assert_eq!(s.reap(), 0, "nothing left to reap"); + } + + #[test] + fn reap_never_drops_a_pad_no_frame_ever_deactivated() { + // Reaping COMPLETES an unplug; it must never invent one. A pad whose bit never went clear + // has no armed clock, so any number of reaps — even with the clock backdated — leaves it. + let mut s = slots(); + assert!(s.ensure(0, |i| Ok(i as u32))); + for _ in 0..10 { + assert_eq!(s.reap(), 0); + s.expire_grace(); + } + assert!( + s.get(0).is_some(), + "reap dropped a pad that never went inactive" + ); + } + + #[test] + fn a_glitch_that_returns_inside_the_grace_never_drops_the_pad() { + // The anti-flap guarantee, now that reaps are frequent: a client mask that blips clear and + // comes back must not churn a PnP devnode. + let mut s = slots(); + assert!(s.ensure(0, |i| Ok(i as u32))); + assert_eq!(s.sweep(0b0), 0); // bit clears — arms only + for _ in 0..5 { + assert_eq!(s.reap(), 0, "dropped a pad inside its grace"); + } + assert_eq!(s.sweep(0b1), 0); // the bit returns — disarms + s.expire_grace(); + assert_eq!(s.reap(), 0, "a returned bit must leave nothing armed"); + assert!(s.get(0).is_some()); + } + #[test] fn ensure_creates_once_and_reports_freshness() { let mut s = slots(); diff --git a/crates/pf-inject/src/inject/uhid_manager.rs b/crates/pf-inject/src/inject/uhid_manager.rs index cdb95fd4..8f91f0a9 100644 --- a/crates/pf-inject/src/inject/uhid_manager.rs +++ b/crates/pf-inject/src/inject/uhid_manager.rs @@ -217,13 +217,10 @@ impl UhidManager { if idx >= MAX_PADS { return; } - // Unplugs: drop any allocated pad whose mask bit cleared, resetting its state. + // Unplugs: arm the grace for any pad whose mask bit cleared (the drop itself lands + // on a later `pump` tick — this frame is the only one the producer sends). let swept = self.slots.sweep(f.active_mask); - for i in 0..MAX_PADS { - if swept & (1 << i) != 0 { - self.reset_pad(i); - } - } + self.reset_swept(swept); if f.active_mask & (1 << idx) == 0 { return; // this event WAS the unplug } @@ -282,6 +279,12 @@ impl UhidManager { mut hidout: impl FnMut(HidOutput), ) { let now = Instant::now(); + // Finish any unplug whose removal frame only armed the grace. The producer emits that + // frame exactly once, so without this a detached pad — the single-pad session being the + // common case — would never be destroyed. Runs BEFORE the loop so a reaped index is + // already gone for `get_mut` here and for `heartbeat`'s `get` later in the same tick. + let swept = self.slots.reap(); + self.reset_swept(swept); for i in 0..MAX_PADS { let Some(pad) = self.slots.get_mut(i) else { continue; @@ -360,6 +363,18 @@ impl UhidManager { } } + /// Reset the sibling state of every index a sweep or reap just dropped. Both halves of the + /// unplug land here, so a pad torn down on the pump tick clears exactly what one torn down on + /// a state frame would — in particular `hidout_dedup`, which has no watchdog to re-arm it and + /// would otherwise swallow an identical lightbar/trigger re-assert after a re-plug. + fn reset_swept(&mut self, swept: u16) { + for i in 0..MAX_PADS { + if swept & (1 << i) != 0 { + self.reset_pad(i); + } + } + } + /// Reset one pad's sibling state (on create and unplug) so the first frame/feedback after a /// (re)connect starts from scratch and is always forwarded. fn reset_pad(&mut self, idx: usize) { @@ -494,18 +509,36 @@ mod tests { } #[test] - fn removal_frame_never_recreates_the_pad_it_swept() { + fn one_removal_frame_plus_a_pump_tick_completes_the_unplug() { + // The producer emits the cleared-mask frame exactly ONCE — `native/input.rs` guards it on + // the bit still being set — so the teardown has to finish on the periodic pump. The + // previous version of this test hand-fed a SECOND removal frame, which is what let the + // never-reaped pad hide: with one frame and no pump, the device outlived the session. let mut m = mgr(); m.handle(&frame(1, 0b10, 0)); assert!(m.slots.get(1).is_some()); - // Bit 1 cleared: the first sweep only ARMS the devnode-churn grace — the pad holds (a - // mask glitch must not flap PnP devices; see pad_slots::SWEEP_GRACE). + // The one removal frame: arms the devnode-churn grace, drops nothing. m.handle(&frame(1, 0b00, 0)); assert!(m.slots.get(1).is_some(), "inside the grace — not yet swept"); - // Grace elapsed: the frame IS pad 1's removal — sweep, then early-return (no ensure). + // A tick inside the grace must NOT flap the devnode (pad_slots::SWEEP_GRACE). + 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(|_, _, _| {}, |_| {}); + assert!( + m.slots.get(1).is_none(), + "the pump tick never completed the unplug" + ); + // …and a further cleared-mask frame must not resurrect it (the arm branch early-returns). m.handle(&frame(1, 0b00, 0)); - assert!(m.slots.get(1).is_none()); + assert!( + m.slots.get(1).is_none(), + "a cleared-mask frame recreated the pad" + ); } #[test] @@ -551,10 +584,15 @@ mod tests { assert_eq!(collect(&mut m), vec![(0, 100, 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. - m.handle(&frame(0, 0b0, 0)); // arms the sweep grace + // 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(); - m.handle(&frame(0, 0b0, 0)); // grace elapsed — actually swept + assert_eq!(collect(&mut m), vec![]); // this tick reaps; nothing queued to forward + assert!( + m.slots.get(0).is_none(), + "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)]); diff --git a/crates/pf-inject/src/inject/windows/gamepad_windows.rs b/crates/pf-inject/src/inject/windows/gamepad_windows.rs index 3cfe7803..d0191c72 100644 --- a/crates/pf-inject/src/inject/windows/gamepad_windows.rs +++ b/crates/pf-inject/src/inject/windows/gamepad_windows.rs @@ -318,14 +318,10 @@ impl GamepadManager { if idx >= MAX_PADS { return; } - // Unplugs: drop any allocated pad whose mask bit cleared. + // Unplugs: arm the grace for any pad whose mask bit cleared (the drop itself lands + // on a later `pump_rumble` tick — this frame is the only one the producer sends). let swept = self.slots.sweep(f.active_mask); - for i in 0..MAX_PADS { - if swept & (1 << i) != 0 { - self.last_rumble[i] = (0, 0); - self.last_active[i] = Instant::now(); - } - } + self.reset_swept(swept); if f.active_mask & (1 << idx) == 0 { return; } @@ -345,10 +341,25 @@ impl GamepadManager { } } + /// Reset the sibling state of every index a sweep or reap just dropped, so both halves of the + /// unplug clear the same things. + fn reset_swept(&mut self, swept: u16) { + for i in 0..MAX_PADS { + if swept & (1 << i) != 0 { + self.last_rumble[i] = (0, 0); + self.last_active[i] = Instant::now(); + } + } + } + /// 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)) { + // 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(); + self.reset_swept(swept); for (i, pad) in self.slots.iter_mut() { if let Some((large, small)) = pad.service() { // The game drove the pad this poll (SET_STATE bumped the seq) — refresh the