Merge branch 'main' into worktree-haptics-m1-rumble-seq

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);

crates/pf-inject/src/inject/pad_slots.rs

@@ -62,15 +62,30 @@ impl<P> PadSlots<P> {
         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<P> PadSlots<P> {
         }
     }
 
-    /// [`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();

crates/pf-inject/src/inject/uhid_manager.rs

@@ -217,13 +217,10 @@ impl<B: PadProto> UhidManager<B> {
                 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<B: PadProto> UhidManager<B> {
         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<B: PadProto> UhidManager<B> {
         }
     }
 
+    /// 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)]);

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