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
diff --git a/crates/punktfunk-host/src/native/input.rs b/crates/punktfunk-host/src/native/input.rs
index a71cde1c..29e773b5 100644
--- a/crates/punktfunk-host/src/native/input.rs
+++ b/crates/punktfunk-host/src/native/input.rs
@@ -629,6 +629,20 @@ const RUMBLE_RENEW_FLOOR_MS: u64 = 60;
/// own expiry. `3` total zero sends = the immediate one + this many renewal re-sends.
const RUMBLE_STOP_BURST: u8 = 2;
+/// Clear a removed pad's rumble bookkeeping — the level, the "we have seen a level" flag, and any
+/// stop re-sends still owed. Together these end the pad's lease, so a re-plug on the same wire
+/// index inherits nothing that could buzz the new device.
+///
+/// The per-pad rumble **sequence is deliberately not a parameter**: it must stay monotonic for the
+/// 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);
+ *seen = false;
+ *stop_burst = 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.
@@ -824,11 +838,22 @@ pub(super) fn input_thread(
tracing::info!(pad = idx, "gamepad unplugged (native detach)");
}
// Fresh feedback bookkeeping so a later re-plug on this index inherits no
- // stale rumble lease/seq (a lease still ticking would buzz the new pad).
- rumble_state[idx] = (0, 0);
- rumble_seen[idx] = false;
- rumble_seq[idx] = 0;
- rumble_stop_burst[idx] = 0;
+ // stale rumble lease (a lease still ticking would buzz the new pad).
+ //
+ // `rumble_seq` deliberately SURVIVES — do not reset it here. The client's
+ // rumble reorder gate (`client/pump/datagram_task.rs`) is per-CONNECTION
+ // and has no reset path, so restarting this counter strands every later
+ // envelope for the re-plugged pad until the host climbs back past the
+ // value the client already stored (up to 128 sends ≈ 15 s of continuous
+ // rumble, or dozens of separate rumble events). The three clears below are
+ // what actually kill a stale lease; the sibling `pad_seq` gate keeps its
+ // value across a removal for exactly the same reason (see the comment at
+ // the top of this arm).
+ clear_pad_feedback(
+ &mut rumble_state[idx],
+ &mut rumble_seen[idx],
+ &mut rumble_stop_burst[idx],
+ );
}
}
InputKind::GamepadArrival => {
@@ -1071,6 +1096,72 @@ mod tests {
}
}
+ /// A pad re-plug must not strand the client's rumble reorder gate.
+ ///
+ /// The client's `rumble_last_seq` lives for the whole QUIC connection and has no reset path
+ /// (`punktfunk-core/src/client/pump/datagram_task.rs`), so this host's per-pad rumble counter
+ /// has to stay monotonic across a `GamepadRemove`. Regression: the removal arm used to do
+ /// `rumble_seq[idx] = 0`, which made every envelope after a re-plug fail `seq_newer` until the
+ /// counter climbed back past the value the client had already stored — up to 128 sends.
+ ///
+ /// Drives the real wire encoder and the real gate, so it fails if either side's rule moves.
+ #[test]
+ fn rumble_seq_survives_a_removal_so_the_client_gate_accepts() {
+ use punktfunk_core::input::GamepadSnapshot;
+ use punktfunk_core::quic::{decode_rumble_envelope, encode_rumble_datagram_v2};
+
+ // The client half: one per-pad slot, per connection, never reset.
+ let deliver = |seq: u8, gate: &mut Option| {
+ let d = encode_rumble_datagram_v2(0, 0x4000, 0x8000, seq, 400);
+ let env = decode_rumble_envelope(&d)
+ .expect("v2 envelope decodes")
+ .envelope
+ .expect("v2 tail present");
+ if GamepadSnapshot::seq_newer(env.seq, *gate) {
+ *gate = Some(env.seq);
+ true
+ } else {
+ false
+ }
+ };
+
+ // The host half: one wrapping counter, bumped on every change and every renewal.
+ let mut gate: Option = None;
+ let mut seq = 0u8;
+
+ // A long rumble before the unplug pushes the client's stored seq well past zero.
+ for _ in 0..100 {
+ seq = seq.wrapping_add(1);
+ assert!(deliver(seq, &mut gate));
+ }
+ 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);
+ clear_pad_feedback(&mut state, &mut seen, &mut burst);
+ assert_eq!(
+ (state, seen, burst),
+ ((0, 0), false, 0),
+ "lease not cleared"
+ );
+
+ // It returns on the same wire index and the game rumbles again: the very first envelope
+ // has to reach the actuator.
+ seq = seq.wrapping_add(1);
+ assert!(
+ deliver(seq, &mut gate),
+ "first envelope after a re-plug was dropped by the client's reorder gate"
+ );
+
+ // Non-vacuity: the pre-fix behaviour (counter restarted at 0) really is rejected, and
+ // stays rejected for the whole forward window — this is the bug, reproduced.
+ let mut stranded = Some(100u8);
+ assert!(
+ (1..=100).all(|s| !deliver(s, &mut stranded)),
+ "test is vacuous — a restarted counter should have been gated out"
+ );
+ }
+
/// Incremental wire events accumulate into the full pad frame the virtual xpad applies.
#[test]
fn gamepad_accumulator() {