ci(android): run the kit's unit tests

They were running nowhere. This workflow only assembled, and the screenshot workflow runs the :app module's tests, so nothing enforced :kit's — the pure parsers, migrations and feedback policies could go red without anyone noticing. A couple of seconds against a module the build already produces.
fix(client/android): rumble survives a vibrator fault, and an unplug stops leaking
2026-08-04 18:14:13 +02:00 · 2026-08-04 18:14:07 +02:00
9 changed files with 365 additions and 200 deletions
@@ -160,6 +160,14 @@ jobs:
          key: gradle-${{ hashFiles('clients/android/**/*.gradle.kts', 'clients/android/gradle/wrapper/gradle-wrapper.properties') }}
          restore-keys: gradle-

+      # The kit's JVM unit tests — the pure parsers, migrations and feedback policies. They were
+      # running nowhere: this workflow only assembled, and android-screenshots.yml runs the :app
+      # module's tests, so nothing enforced :kit's. Cheap (a couple of seconds against an already
+      # built module) and it is the only automated cover those behaviours have.
+      - name: kit unit tests
+        working-directory: clients/android
+        run: ./gradlew :kit:testDebugUnitTest --stacktrace
+
      - name: assembleDebug (cargo-ndk → jniLibs → APK)
        working-directory: clients/android
        env:
@@ -116,7 +116,9 @@ class DsCapture(
            // The interfaces are about to release with the kernel driver still detached — a
            // mid-rumble teardown would leave the motors running with nobody to stop them.
            // EP0-direct (the reader thread is stopping; the queue would never drain).
-            usb.writeControl(stopReport(m))
+            // Nothing can retry after this point, so a failure is worth saying out loud: it is
+            // the difference between a quiet pad and one that buzzes until it is unplugged.
+            if (!usb.writeControl(stopReport(m))) Log.w(TAG, "teardown rumble stop was not written")
        }
        disarmBackstop()
        usb.stop()
@@ -145,6 +147,9 @@ class DsCapture(
        val wasActive = model != null
        model = null
        releaseSlot()
+        // Release the transport too: the link only *signals* the drop, so without this an unplug
+        // left its connection open, its interfaces claimed and its detach receiver registered.
+        usb.stop()
        if (wasActive) onActiveChanged?.invoke(false)
    }

@@ -216,17 +221,20 @@ class DsCapture(

    override fun rumble(pad: Int, low: Int, high: Int, backstopMs: Long) {
        val m = model ?: return
-        if (low == 0 && high == 0) {
-            disarmBackstop()
-        } else {
-            armBackstop(backstopMs)
-        }
-        if (m == DsDevice.Model.DUALSHOCK4) {
+        val stop = low == 0 && high == 0
+        if (!stop) armBackstop(backstopMs)
+        val sent = if (m == DsDevice.Model.DUALSHOCK4) {
            ds4Low = low
            ds4High = high
            writeDs4()
        } else {
-            usb.writeRaw(0, DsDevice.ds5RumbleReport(m, low, high))
+            usb.writeRaw(0, DsDevice.ds5RumbleReport(m, low, high), OutReportQueue.KEY_RUMBLE)
+        }
+        if (stop) {
+            // Disarm only once the stop is actually on its way. Dropping the net *before* the
+            // write — as this used to — meant a discarded stop left the motors running with
+            // nothing scheduled to try again; a USB pad holds its last level until told zero.
+            if (sent) disarmBackstop() else armBackstop(STOP_RETRY_MS)
        }
    }

@@ -252,6 +260,9 @@ class DsCapture(
        usb.writeRaw(0, DsDevice.ds5TriggerReport(m, which, effect))
    }

+    // Coalescable: the DS4's write is full-state (motors AND lightbar, rebuilt from the current
+    // fields on every call), so a newer one supersedes an older one wholesale — nothing is lost by
+    // collapsing a backlog of them down to the last.
    private fun writeDs4() = usb.writeRaw(
        0,
        DsDevice.ds4Report(
@@ -261,6 +272,7 @@ class DsCapture(
            (ds4Rgb shr 8) and 0xFF,
            ds4Rgb and 0xFF,
        ),
+        OutReportQueue.KEY_RUMBLE,
    )

    /** The report that stops the motors. The DS4's is a full-state write, so it zeroes the
@@ -284,7 +296,12 @@ class DsCapture(
        backstop?.let { mainHandler.removeCallbacks(it) }
        val r = Runnable {
            backstop = null
-            model?.let { usb.writeRaw(0, stopReport(it)) }
+            val m = model ?: return@Runnable
+            // The net itself can be refused (a full queue, a connection going away). Re-arm rather
+            // than give up: this is the last thing between a stalled poll thread and a pad that
+            // buzzes until it is unplugged. It stops re-arming as soon as the link closes, which
+            // clears `model` and disarms.
+            if (!usb.writeRaw(0, stopReport(m), OutReportQueue.KEY_RUMBLE)) armBackstop(STOP_RETRY_MS)
        }
        backstop = r
        mainHandler.postDelayed(r, ms.coerceAtLeast(1))
@@ -297,5 +314,9 @@ class DsCapture(

    private companion object {
        const val TAG = "DsCapture"
+
+        /** How soon to retry a rumble stop whose write was rejected. Short: the motors are running
+         *  and the host has already moved on, so nothing else is coming to silence them. */
+        const val STOP_RETRY_MS = 100L
    }
 }
@@ -88,6 +88,9 @@ class GamepadFeedback(
        const val TAG_PLAYER_LEDS: Byte = 0x02
        const val TAG_TRIGGER: Byte = 0x03
        const val TAG_HID_RAW: Byte = 0x05
+
+        /** Sparse-log cadence for swallowed render failures — see [noteRenderFailure]. */
+        const val LOG_EVERY = 128L
    }

    /** One controller's rumble binding — VibratorManager (API 31+) OR the legacy single Vibrator (API 28–30). */
@@ -125,6 +128,7 @@ class GamepadFeedback(
    fun start() {
        running = true
        rumbleThread = Thread({
+            var failures = 0L
            while (running) {
                val ev = NativeBridge.nativeNextRumble(handle)
                if (ev < 0L) continue // timeout / closed
@@ -136,26 +140,50 @@ class GamepadFeedback(
                // the backstop (the hardware net under a stalled poll thread).
                val pad = ((ev ushr 49) and 0xFL).toInt()
                val backstopMs = ((ev ushr 32) and 0xFFFF)
-                renderRumble(
-                    pad,
-                    ((ev ushr 16) and 0xFFFF).toInt(),
-                    (ev and 0xFFFF).toInt(),
-                    backstopMs,
-                )
+                // Rendering is binder calls into the vibrator service, and every one of them can
+                // throw unchecked — DeadSystemRuntimeException when system_server goes down, and
+                // the ordinary RuntimeException a dying service wraps its RemoteException in.
+                // Unguarded, ONE of those killed this thread outright: `running` stayed true, so
+                // nothing noticed and nothing restarted it, and rumble was gone for the rest of
+                // the session. Losing a single command is recoverable; losing the loop is not.
+                runCatching {
+                    renderRumble(
+                        pad,
+                        ((ev ushr 16) and 0xFFFF).toInt(),
+                        (ev and 0xFFFF).toInt(),
+                        backstopMs,
+                    )
+                }.onFailure { failures = noteRenderFailure("rumble", it, failures) }
            }
        }, "pf-rumble").apply { isDaemon = true; start() }

        hidoutThread = Thread({
            // 128: the raw as-is passthrough events are [pad][kind tag][report kind][≤64 bytes].
            val buf = ByteBuffer.allocateDirect(128)
+            var failures = 0L
            while (running) {
                val n = NativeBridge.nativeNextHidout(handle, buf)
                if (n < 0) continue // timeout / closed
-                dispatchHidout(buf, n)
+                // Same hazard as the rumble loop above: lights/trigger rendering is binder and USB
+                // calls, and an unchecked throw here would silently end the rich-feedback plane.
+                runCatching { dispatchHidout(buf, n) }
+                    .onFailure { failures = noteRenderFailure("hidout", it, failures) }
            }
        }, "pf-hidout").apply { isDaemon = true; start() }
    }

+    /**
+     * Record a render failure the poll loop swallowed, and return the updated count. Logged on the
+     * first occurrence and sparsely after: a genuinely dead vibrator service fails on *every*
+     * command, which at a rumble plane's rate would bury the log.
+     */
+    private fun noteRenderFailure(plane: String, t: Throwable, seen: Long): Long {
+        if (seen == 0L || seen % LOG_EVERY == 0L) {
+            Log.w(TAG, "$plane render failed (#${seen + 1}) — command dropped, poll loop alive", t)
+        }
+        return seen + 1
+    }
+
    /** Idempotent. Stops + joins the poll threads (must complete before the router is released / handle freed). */
    fun stop() {
        running = false
@@ -269,7 +297,7 @@ class GamepadFeedback(
        val m = bind.vm
        if (m != null) {
            if (lo == 0 && hi == 0) {
-                m.cancel() // (0,0) = stop
+                runCatching { m.cancel() } // (0,0) = stop
                return
            }
            val combo = CombinedVibration.startParallel()
@@ -294,7 +322,7 @@ class GamepadFeedback(
        // API 28–30 legacy single-motor path: blend both motors into one effect.
        val lv = bind.legacy ?: return
        if (lo == 0 && hi == 0) {
-            lv.cancel() // (0,0) = stop
+            runCatching { lv.cancel() } // (0,0) = stop
            return
        }
        val a = (lo * 0.8 + hi * 0.33).toInt().coerceIn(1, 255)
@@ -14,8 +14,8 @@ import android.hardware.usb.UsbRequest
 import android.os.Build
 import android.util.Log
 import java.nio.ByteBuffer
-import java.util.concurrent.ConcurrentLinkedQueue
 import java.util.concurrent.TimeoutException
+import java.util.concurrent.atomic.AtomicBoolean

 /**
 * Generic USB transport for a client-captured HID controller — the device-agnostic half of what
@@ -81,14 +81,20 @@ class HidUsbLink(

    /** Pending OUT reports, submitted by the reader thread — only one thread may drive a
     *  connection's [UsbRequest]s ([UsbDeviceConnection.requestWait] returns ANY completed
-     *  request; a second waiter would steal the reader's completions). */
-    private val outQueue = ConcurrentLinkedQueue<ByteArray>()
+     *  request; a second waiter would steal the reader's completions). See [OutReportQueue] for
+     *  what gets discarded when it fills, and why that is not simply "the oldest". */
+    private val outQueue = OutReportQueue()

    private var reader: Thread? = null
    private var detachReceiver: BroadcastReceiver? = null

    @Volatile private var running = false

+    /** Latches on the first "this link is down" signal so [onClosed] fires exactly once, however
+     *  many of the racing detectors (detach broadcast, reader error streak, failed re-queue) see
+     *  it. Reset by [start]. */
+    private val down = AtomicBoolean(false)
+
    /** First attached matching device, or null. Does not need USB permission to enumerate. */
    fun findDevice(): UsbDevice? = usb.deviceList.values.firstOrNull(config.deviceMatch)

@@ -114,6 +120,7 @@ class HidUsbLink(
        connection = conn
        device = dev
        claims = claimed
+        down.set(false)
        running = true
        Log.i(
            config.tag,
@@ -134,10 +141,7 @@ class HidUsbLink(
                val gone: UsbDevice? = intent.getParcelableExtra(UsbManager.EXTRA_DEVICE)
                if (gone?.deviceName == dev.deviceName) {
                    Log.i(config.tag, "USB detached (${dev.deviceName})")
-                    if (running) {
-                        running = false
-                        onClosed()
-                    }
+                    linkDown()
                }
            }
        }
@@ -221,6 +225,9 @@ class HidUsbLink(
        if (live.isEmpty()) {
            Log.e(config.tag, "no IN request could be queued")
            finishReader(claims)
+            // `start` already returned true, so without this the owner would sit waiting on a
+            // capture that never streams and never reports itself dead.
+            linkDown()
            return
        }
        val scratch = ByteArray(64)
@@ -295,10 +302,23 @@ class HidUsbLink(
        } finally {
            finishReader(claims)
        }
-        if (running) {
-            running = false
-            onClosed()
-        }
+        linkDown()
+    }
+
+    /**
+     * Report the link down, exactly once, from whichever detector noticed first — the detach
+     * broadcast (main thread) or the reader thread on its way out.
+     *
+     * This only *signals*; releasing the connection and the interfaces stays the owner's job, via
+     * the [stop] its `onClosed` handler calls. Previously nothing released them on this path: the
+     * detach receiver flipped a flag and fired the callback, so an unplug left the connection open,
+     * the interfaces claimed (the pad could not return to Android's own input stack) and the
+     * receiver still registered — and a re-plug overwrote the field holding it, leaking a receiver
+     * that stayed live for the process's lifetime.
+     */
+    private fun linkDown() {
+        running = false
+        if (down.compareAndSet(false, true)) onClosed()
    }

    private fun finishReader(claims: List<Claim>) {
@@ -314,28 +334,35 @@ class HidUsbLink(
     * Write one raw report to the device: kind 0 = output report (the active interface's
     * interrupt-OUT, else a `SET_REPORT(Output)` control transfer), kind 1 = feature report
     * (`SET_REPORT(Feature)`). [data] is the full report, id byte first, hidapi framing.
+     *
+     * [coalesce] tells the pending-OUT queue whether a newer report of the same kind may replace
+     * this one — [OutReportQueue.KEY_RUMBLE] for motor levels, the default [OutReportQueue.NO_COALESCE]
+     * for one-shots (lightbar, player LEDs, trigger effects) the sender will not repeat.
+     *
+     * Returns whether the report reached the device or is queued for it. A caller that is writing
+     * a **stop** needs this: a discarded stop has nothing behind it, so it must not be mistaken
+     * for one that landed.
     */
-    fun writeRaw(kind: Int, data: ByteArray) {
-        if (data.isEmpty()) return
-        when (kind) {
+    fun writeRaw(kind: Int, data: ByteArray, coalesce: Int = OutReportQueue.NO_COALESCE): Boolean {
+        if (data.isEmpty()) return false
+        return when (kind) {
            0 -> {
                if ((activeClaim ?: claims.firstOrNull())?.outReq != null) {
-                    // Interrupt-OUT rides UsbRequests submitted by the reader thread. Bounded,
-                    // newest-wins: these are level-styled commands the sender re-sends anyway.
-                    while (outQueue.size >= 32) outQueue.poll()
-                    outQueue.offer(data)
+                    // Interrupt-OUT rides UsbRequests submitted by the reader thread.
+                    outQueue.offer(data, coalesce)
                } else {
                    setReport(REPORT_TYPE_OUTPUT, data)
                }
            }
            1 -> setReport(REPORT_TYPE_FEATURE, data)
+            else -> false
        }
    }

-    private fun setReport(type: Int, data: ByteArray) {
-        val conn = connection ?: return
-        val ifId = (activeClaim ?: claims.firstOrNull())?.iface?.id ?: return
-        sendReport(conn, ifId, type, data)
+    private fun setReport(type: Int, data: ByteArray): Boolean {
+        val conn = connection ?: return false
+        val ifId = (activeClaim ?: claims.firstOrNull())?.iface?.id ?: return false
+        return sendReport(conn, ifId, type, data)
    }

    /**
@@ -344,9 +371,8 @@ class HidUsbLink(
     * queue would never drain (e.g. a rumble stop before the interfaces release). Safe from any
     * thread: EP0 control transfers are independent of the reader's `requestWait`.
     */
-    fun writeControl(data: ByteArray) {
-        if (data.isNotEmpty()) setReport(REPORT_TYPE_OUTPUT, data)
-    }
+    fun writeControl(data: ByteArray): Boolean =
+        data.isNotEmpty() && setReport(REPORT_TYPE_OUTPUT, data)

    private fun sendKeepAlive(conn: UsbDeviceConnection, ifaceId: Int) {
        for (f in config.keepAliveFeatures) sendReport(conn, ifaceId, REPORT_TYPE_FEATURE, f)
@@ -358,27 +384,48 @@ class HidUsbLink(
     * "unnumbered" (id 0 in wValue, id byte stripped from the payload). EP0 is independent of
     * the interrupt endpoints, so this is safe alongside the reader thread's requestWait.
     */
-    private fun sendReport(conn: UsbDeviceConnection, ifaceId: Int, type: Int, data: ByteArray) {
+    private fun sendReport(
+        conn: UsbDeviceConnection,
+        ifaceId: Int,
+        type: Int,
+        data: ByteArray,
+    ): Boolean {
        val id = data[0].toInt() and 0xFF
        val payload = if (id == 0) data.copyOfRange(1, data.size) else data
-        conn.controlTransfer(
-            0x21, // host→device, class, interface
-            0x09, // SET_REPORT
-            (type shl 8) or id,
-            ifaceId,
-            payload,
-            payload.size,
-            WRITE_TIMEOUT_MS,
-        )
+        // controlTransfer returns the byte count, or a negative value on failure — a failed write
+        // must be reported as such, not swallowed (a dropped rumble stop has nothing behind it).
+        val n = runCatching {
+            conn.controlTransfer(
+                0x21, // host→device, class, interface
+                0x09, // SET_REPORT
+                (type shl 8) or id,
+                ifaceId,
+                payload,
+                payload.size,
+                WRITE_TIMEOUT_MS,
+            )
+        }.getOrDefault(-1)
+        return n >= 0
    }

-    /** Stop the read loop and release the interfaces. Idempotent; does not fire [onClosed]. */
+    /**
+     * Stop the read loop and release the interfaces. Idempotent; does not fire [onClosed].
+     *
+     * Safe to call from the `onClosed` handler itself — that is how an unplug now gets cleaned up,
+     * and it arrives on the reader thread, which must not try to join itself.
+     */
    fun stop() {
        running = false
+        // Claim the down-latch so the reader's own exit does not report a close the owner asked for.
+        down.set(true)
        detachReceiver?.let { runCatching { context.unregisterReceiver(it) } }
        detachReceiver = null
-        runCatching { reader?.join(1000) }
-        reader = null
+        if (reader !== Thread.currentThread()) {
+            runCatching { reader?.join(1000) }
+            // Only forget the thread once it is actually gone: clearing it while it still runs
+            // would let a later stop() skip the join and free the connection under it.
+            reader = null
+        }
        outQueue.clear()
        activeClaim = null
        for (c in claims) runCatching { connection?.releaseInterface(c.iface) }
@@ -0,0 +1,89 @@
+package io.unom.punktfunk.kit
+
+/**
+ * The pending interrupt-OUT reports for a captured controller: a bounded FIFO whose overflow
+ * policy knows which reports may be thrown away and which may not.
+ *
+ * The queue exists because only one thread may drive a connection's `UsbRequest`s, so writes from
+ * the feedback threads are handed to the reader thread rather than submitted directly. It has to
+ * be bounded — a stalled or unplugged device would otherwise grow it without limit — and the
+ * question is what to discard when it fills.
+ *
+ * The old policy was "newest wins": drop from the head until there is room. That is right for
+ * rumble, which is *level-styled* — the host re-sends it continuously, so a dropped frame is
+ * replaced milliseconds later and nothing is permanently lost. It is wrong for everything else.
+ * A lightbar colour, a player-LED mask and an adaptive-trigger effect are **one-shots**: the host
+ * sends them on change and never repeats them. Dropping one leaves the pad wrong until the next
+ * time that value happens to change, which may be never.
+ *
+ * So eviction is driven by an explicit [key] supplied by the caller, not by inspecting the bytes.
+ * That distinction cannot be recovered from the report itself: every DualSense output report
+ * carries the *same* report id and differs only in its `valid_flag` bytes, so an id-keyed policy
+ * would happily let a rumble supersede a lightbar — the very bug this replaces, relocated.
+ *
+ * Two rules:
+ *  - A report offered with a coalescing key **replaces** the pending report with that key, in
+ *    place. A burst of rumble collapses to its latest value and never displaces anything else.
+ *  - Only when the queue is full does anything get dropped, and then the oldest *coalescable*
+ *    report goes first. A one-shot is discarded only if the queue is full of nothing but
+ *    one-shots — which needs [cap] distinct one-shots outstanding, far beyond what a real pad
+ *    produces.
+ *
+ * Thread-safe: offered by the feedback threads, drained by the reader thread.
+ */
+internal class OutReportQueue(private val cap: Int = CAP) {
+    private class Entry(val key: Int, val data: ByteArray)
+
+    private val items = ArrayDeque<Entry>()
+
+    /**
+     * Queue [data] for submission. [key] is [NO_COALESCE] for a one-shot, or a caller-chosen
+     * constant identifying a level-styled stream whose newer values supersede older ones.
+     *
+     * Returns false only if the report had to be dropped outright — the caller can then treat the
+     * write as failed rather than assuming it is on its way.
+     */
+    fun offer(data: ByteArray, key: Int = NO_COALESCE): Boolean = synchronized(items) {
+        if (key != NO_COALESCE) {
+            val at = items.indexOfFirst { it.key == key }
+            if (at >= 0) {
+                // Supersede in place: keeping the queue position stops a fast rumble stream from
+                // repeatedly jumping the one-shots queued ahead of it.
+                items[at] = Entry(key, data)
+                return true
+            }
+        }
+        if (items.size >= cap) {
+            val victim = items.indexOfFirst { it.key != NO_COALESCE }
+            if (victim >= 0) {
+                items.removeAt(victim)
+            } else if (key != NO_COALESCE) {
+                // Nothing coalescable to sacrifice and this report is itself replaceable — drop it
+                // rather than a one-shot that will never come again.
+                return false
+            } else {
+                items.removeFirst()
+            }
+        }
+        items.addLast(Entry(key, data))
+        return true
+    }
+
+    /** The next report to submit, or null when nothing is pending. */
+    fun poll(): ByteArray? = synchronized(items) { items.removeFirstOrNull()?.data }
+
+    fun clear() = synchronized(items) { items.clear() }
+
+    val size: Int get() = synchronized(items) { items.size }
+
+    companion object {
+        /** This report is a one-shot: never superseded, evicted only as a last resort. */
+        const val NO_COALESCE = 0
+
+        /** Motor levels — re-sent continuously, so only the newest is worth keeping. */
+        const val KEY_RUMBLE = 1
+
+        /** Deep enough to absorb a burst, small enough that a stalled device cannot bloat us. */
+        const val CAP = 32
+    }
+}
@@ -273,10 +273,20 @@ class Sc2Capture(

    private fun onLinkClosed() {
        Log.i(TAG, "SC2 link closed (unplug / power-off)")
+        // Both transports share this callback, so read which one was live BEFORE clearing it —
+        // releasing the other would tear down a link that never dropped.
+        val dropped = activeLink
        activeLink = LINK_NONE
        dongleLink = false
        releaseSlot()
        releaseUiKeys()
+        // Release the transport too — see the note in DsCapture.onLinkClosed. The Puck makes this
+        // worse than a single leak: it is the pad that gets power-cycled, so the same process can
+        // round-trip a link many times in one session.
+        when (dropped) {
+            LINK_USB -> usb.stop()
+            LINK_BLE -> ble.stop()
+        }
        onActiveChanged?.invoke(false)
    }

@@ -0,0 +1,102 @@
+package io.unom.punktfunk.kit
+
+import org.junit.Assert.assertArrayEquals
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertNull
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+/**
+ * The pending-OUT queue's overflow policy. What is being pinned here is the distinction the old
+ * "drop from the head until there is room" policy did not make: rumble is re-sent continuously and
+ * may be thrown away, while a lightbar/player-LED/trigger report is sent once and never repeated.
+ */
+class OutReportQueueTest {
+    /** A report carrying a 0..255 marker so a test can tell which one came back out. */
+    private fun report(marker: Int) = byteArrayOf(0x02, marker.toByte())
+
+    // Masked: the marker rides in a Byte, and Byte.toInt() sign-extends.
+    private fun drain(q: OutReportQueue): List<Int> =
+        generateSequence { q.poll() }.map { it[1].toInt() and 0xFF }.toList()
+
+    @Test
+    fun `rumble supersedes the pending rumble instead of queueing another`() {
+        val q = OutReportQueue()
+        assertTrue(q.offer(report(1), OutReportQueue.KEY_RUMBLE))
+        assertTrue(q.offer(report(2), OutReportQueue.KEY_RUMBLE))
+        assertTrue(q.offer(report(3), OutReportQueue.KEY_RUMBLE))
+        assertEquals("a rumble burst must collapse to one entry", 1, q.size)
+        assertArrayEquals(report(3), q.poll())
+        assertNull(q.poll())
+    }
+
+    @Test
+    fun `superseding keeps the queue position so a rumble stream cannot jump one-shots`() {
+        val q = OutReportQueue()
+        q.offer(report(1), OutReportQueue.KEY_RUMBLE)
+        q.offer(report(10)) // a one-shot queued behind it
+        q.offer(report(2), OutReportQueue.KEY_RUMBLE)
+        // The newer rumble takes the OLD rumble's slot, so the one-shot does not get starved
+        // behind an endlessly-renewed entry.
+        assertEquals(listOf(2, 10), drain(q))
+    }
+
+    @Test
+    fun `a full queue sacrifices rumble, never a one-shot`() {
+        val q = OutReportQueue(cap = 4)
+        q.offer(report(1), OutReportQueue.KEY_RUMBLE)
+        q.offer(report(10))
+        q.offer(report(11))
+        q.offer(report(12))
+        assertEquals(4, q.size)
+        // Full. The old policy dropped the head — here that is a rumble, but only by luck of
+        // ordering; what matters is that the one-shots all survive.
+        assertTrue(q.offer(report(13)))
+        assertEquals(listOf(10, 11, 12, 13), drain(q))
+    }
+
+    @Test
+    fun `the one-shot the host never repeats survives a rumble storm`() {
+        val q = OutReportQueue(cap = 4)
+        // The exact regression: a lightbar colour queued once, then a flood of rumble. Under the
+        // old newest-wins eviction the colour was dropped from the head and never came back,
+        // leaving the pad lit wrong until the value next happened to change.
+        q.offer(report(200)) // lightbar
+        repeat(50) { q.offer(report(it), OutReportQueue.KEY_RUMBLE) }
+        val out = drain(q)
+        assertTrue("the lightbar report must still be queued, got $out", out.contains(200))
+        assertEquals("rumble must not have accumulated", listOf(200, 49), out)
+    }
+
+    @Test
+    fun `a queue full of one-shots refuses a rumble rather than dropping one`() {
+        val q = OutReportQueue(cap = 2)
+        q.offer(report(10))
+        q.offer(report(11))
+        assertFalse(
+            "with nothing coalescable to sacrifice, the replaceable report yields",
+            q.offer(report(1), OutReportQueue.KEY_RUMBLE),
+        )
+        assertEquals(listOf(10, 11), drain(q))
+    }
+
+    @Test
+    fun `only a queue of nothing but one-shots drops one, and it is the oldest`() {
+        val q = OutReportQueue(cap = 2)
+        q.offer(report(10))
+        q.offer(report(11))
+        assertTrue(q.offer(report(12)))
+        assertEquals(listOf(11, 12), drain(q))
+    }
+
+    @Test
+    fun `clear empties the queue`() {
+        val q = OutReportQueue()
+        q.offer(report(1), OutReportQueue.KEY_RUMBLE)
+        q.offer(report(10))
+        q.clear()
+        assertEquals(0, q.size)
+        assertNull(q.poll())
+    }
+}
@@ -285,21 +285,6 @@ fn set_valve_hidapi(enabled: bool) {
    sdl3::hint::set("SDL_JOYSTICK_HIDAPI_STEAM", v);
 }

-/// Disable the Valve HIDAPI drivers **before SDL exists** — call this alongside the other
-/// pre-`SDL_Init` hints, not after a subsystem is up.
-///
-/// The damage these drivers do happens at *enumeration*, which is part of initialising the
-/// joystick/gamepad subsystem. Setting the hint afterwards does detach the driver, but only after
-/// it has already sent the Deck its `ID_CLEAR_DIGITAL_MAPPINGS` + `TRACKPAD_NONE` — so the
-/// built-in trackpad-mouse dies system-wide and stays dead until the firmware watchdog restores
-/// lizard mode seconds later. The threaded worker ([`run`]) has always done this in the right
-/// order; the caller-pumped path could not, because by the time it receives a
-/// [`sdl3::GamepadSubsystem`] the enumeration has already happened. Hence a separate entry point
-/// its callers can put in the right place.
-pub fn preinit_disable_valve_hidapi() {
-    set_valve_hidapi(false);
-}
-
 /// Map the SDL-reported controller type to the virtual pad we'd ask the host to create.
 fn pref_for_type(t: sdl3::gamepad::GamepadType) -> GamepadPref {
    use sdl3::gamepad::GamepadType as T;
@@ -408,12 +393,9 @@ impl GamepadService {
    /// and calls [`GamepadPump::tick`] once per loop iteration (the threaded worker's
    /// per-wakeup work: ctl drain, chord-hold check, menu repeat, feedback).
    ///
-    /// The Valve HIDAPI drivers are held off here too, but this is **too late to be the only
-    /// place it happens**: the `subsystem` argument means enumeration is already done, and that
-    /// is when the Deck driver kills the trackpad-mouse. The caller must also call
-    /// [`preinit_disable_valve_hidapi`] with its other pre-`SDL_Init` hints. This call still
-    /// earns its place — it re-asserts "off" for a process that ran a session earlier — but on
-    /// its own it only detaches a driver that has already done the damage.
+    /// Like the threaded worker, this disables the Valve HIDAPI drivers up front (their
+    /// mere enumeration kills the Deck's trackpad-mouse system-wide); they are enabled
+    /// for the duration of an attached session only.
    pub fn pumped(subsystem: sdl3::GamepadSubsystem) -> (GamepadService, GamepadPump) {
        set_valve_hidapi(false);
        let pads = Arc::new(Mutex::new(Vec::new()));
@@ -574,38 +556,6 @@ impl GamepadPump {
        self.worker.menu_poll();
        self.worker.render_feedback();
    }
-
-    /// Close every forwarded slot — flush its held wire state, tell the host to remove the pad,
-    /// and physically silence it. Call once on the way out of the caller's event loop.
-    ///
-    /// [`GamepadService::detach`] only *posts* `Ctl::Detach`; the close — the flush, the host-side
-    /// `GamepadRemove`, and the explicit `set_rumble(0, 0)` backstop in `close_slot_at` — happens
-    /// when the pump next drains it. An exit path that detached and then left the loop without
-    /// another [`tick`](Self::tick) therefore skipped all of it, and nothing else would: the slots
-    /// hold no `Drop` that silences them. A pad left mid-buzz stayed buzzing.
-    ///
-    /// This closes the slots directly rather than draining the queued `Ctl::Detach` that would
-    /// have done it. Same physical outcome by a shorter path, and deliberately so: this also runs
-    /// from `Drop`, and `drain_ctl` reaches `Mutex::lock().unwrap()`, which on a poisoned lock
-    /// would panic — during an unwind that aborts the process. Closing a slot touches no lock.
-    ///
-    /// Idempotent, and safe with nothing attached.
-    pub fn shutdown(&mut self) {
-        self.worker.close_all_slots();
-    }
-}
-
-/// The silence backstop of last resort. A caller's loop can also leave by `?` on a fatal overlay
-/// or present error — several paths do — and those would skip an explicit
-/// [`shutdown`](GamepadPump::shutdown) entirely, leaving a forwarded pad buzzing on the way out.
-///
-/// Callers should still call `shutdown` at their normal exit rather than lean on this: the pad
-/// wants to go quiet *before* a long teardown (session join, `vkDeviceWaitIdle`), not after it.
-/// Doing both is free — `shutdown` is idempotent.
-impl Drop for GamepadPump {
-    fn drop(&mut self) {
-        self.shutdown();
-    }
 }

 /// The lowest wire pad index (0..[`MAX_PADS`](punktfunk_core::input::MAX_PADS)) not already held
@@ -1676,11 +1626,6 @@ impl Worker {
                HidOutput::PlayerLeds { bits, .. } if is_ds => {
                    let _ = slot.pad.send_effect(&Ds5Feedback::player_packet(bits));
                }
-                // Every other pad with player LEDs gets them through SDL, which owns the
-                // per-device pattern. This used to fall through and do nothing at all.
-                HidOutput::PlayerLeds { bits, .. } => {
-                    let _ = set_player_leds(&slot.pad, bits);
-                }
                HidOutput::Trigger {
                    which, ref effect, ..
                } if is_ds => {
@@ -1688,43 +1633,12 @@ impl Worker {
                        .pad
                        .send_effect(&Ds5Feedback::trigger_packet(which, effect));
                }
-                // Deliberately unhandled, listed rather than left to a bare `_` so a new
-                // variant cannot join them silently: adaptive triggers exist only on a
-                // DualSense, and the trackpad-haptic / raw-passthrough planes are DS-specific
-                // and carried by `send_effect` above when the pad is one.
-                HidOutput::Trigger { .. }
-                | HidOutput::TrackpadHaptic { .. }
-                | HidOutput::HidRaw { .. } => {}
+                _ => {}
            }
        }
    }
 }

-/// The SDL player index for the wire's positional player-LED `bits`, or `None` for "no player".
-///
-/// The wire carries a bitmask — one bit per LED, low 5 — while SDL wants a player *index* and owns
-/// the per-device pattern. The count bridges them: every convention that reaches this wire spells
-/// "player N" as N lit LEDs, both the DualSense patterns (`0x04`, `0x0A`, `0x15`, `0x1B`, `0x1F`)
-/// and the Switch/XInput run of low bits (`0x01`, `0x03`, `0x07`, `0x0F`). SDL's index is 0-based,
-/// so player 1 is index 0; no lit LED means *no* player rather than player 0.
-///
-/// Split out from [`set_player_leds`] so the mapping is testable — an `sdl3::Gamepad` needs a real
-/// device, so nothing that takes one can be.
-fn player_index_from_bits(bits: u8) -> Option<u16> {
-    match (bits & 0x1F).count_ones() {
-        0 => None,
-        n => Some((n - 1) as u16),
-    }
-}
-
-/// Drive a non-DualSense pad's player LEDs from the wire's positional `bits`.
-fn set_player_leds(pad: &sdl3::gamepad::Gamepad, bits: u8) -> Result<(), sdl3::Error> {
-    match player_index_from_bits(bits) {
-        None => pad.unset_player_index(),
-        Some(i) => pad.set_player_index(i),
-    }
-}
-
 /// The wire pad index a [`HidOutput`] is addressed to (every variant carries `pad`).
 fn hidout_pad(h: &HidOutput) -> u8 {
    match h {
@@ -2094,43 +2008,3 @@ mod slot_tests {
        );
    }
 }
-
-#[cfg(test)]
-mod player_led_tests {
-    use super::*;
-
-    /// Both conventions that reach this wire spell "player N" as N lit LEDs, so the count is the
-    /// player number regardless of WHICH bits a given pad lights. Pinned because the mapping is
-    /// otherwise only obvious once you have seen both patterns side by side.
-    #[test]
-    fn player_index_counts_lit_leds_for_both_conventions() {
-        // DualSense / hid-playstation patterns — non-contiguous, symmetric about the centre LED.
-        assert_eq!(player_index_from_bits(0x04), Some(0)); // player 1
-        assert_eq!(player_index_from_bits(0x0A), Some(1)); // player 2
-        assert_eq!(player_index_from_bits(0x15), Some(2)); // player 3
-        assert_eq!(player_index_from_bits(0x1B), Some(3)); // player 4
-        assert_eq!(player_index_from_bits(0x1F), Some(4)); // player 5
-
-        // Switch/XInput style — a contiguous run of low bits, the same count each time.
-        assert_eq!(player_index_from_bits(0x01), Some(0));
-        assert_eq!(player_index_from_bits(0x03), Some(1));
-        assert_eq!(player_index_from_bits(0x07), Some(2));
-        assert_eq!(player_index_from_bits(0x0F), Some(3));
-    }
-
-    /// No lit LED is "no player", NOT player 0 — the difference between LEDs off and player 1 lit.
-    #[test]
-    fn no_lit_led_is_no_player() {
-        assert_eq!(player_index_from_bits(0x00), None);
-        // Only the low 5 bits are player LEDs; junk above them must not invent a player.
-        assert_eq!(player_index_from_bits(0xE0), None);
-    }
-
-    /// The mask is applied before counting, so out-of-range bits cannot inflate the index past
-    /// the 5 real LEDs.
-    #[test]
-    fn high_bits_are_masked_off_before_counting() {
-        assert_eq!(player_index_from_bits(0xFF), Some(4)); // 0x1F worth of LEDs, not 8
-        assert_eq!(player_index_from_bits(0xE4), Some(0)); // 0x04 with junk on top
-    }
-}
@@ -466,13 +466,6 @@ fn run_inner(mut opts: SessionOpts, mut mode: ModeCtl) -> Result<Option<Outcome>
    #[cfg(windows)]
    crate::win32::set_app_user_model_id();
    sdl3::hint::set("SDL_JOYSTICK_THREAD", "1");
-    // Hold SDL's Valve HIDAPI drivers off BEFORE SDL_Init: the Deck driver clears the pad's
-    // digital mappings at *enumeration*, which is part of bringing the gamepad subsystem up, so a
-    // hint set after `sdl.gamepad()` — where this used to live, inside GamepadService::pumped —
-    // only detached a driver that had already killed the built-in trackpad-mouse system-wide. The
-    // symptom was the Deck losing its trackpad cursor at the start of every session until the
-    // firmware watchdog restored lizard mode. They are still enabled for an attached session.
-    pf_client_core::gamepad::preinit_disable_valve_hidapi();
    // A touchscreen (the Deck's glass) is forwarded as REAL touch passthrough below — so
    // suppress SDL's default synthesis of mouse events from touch. Left on, every touch
    // ALSO warps a synthetic mouse to the touch point, which under the stream's relative
@@ -1902,13 +1895,6 @@ fn run_inner(mut opts: SessionOpts, mut mode: ModeCtl) -> Result<Option<Outcome>
        }
    };

-    // Every exit from the loop above converges here, which is why the gamepad teardown belongs
-    // here and not on the individual `break`s. `gamepad.detach()` only queues the detach; the
-    // close — flush, host-side GamepadRemove, and the explicit rumble-stop backstop — runs when
-    // the pump drains it. Single mode broke out of the loop immediately after detaching and
-    // Event::Quit never detached at all, so both left forwarded pads unflushed and, if the game
-    // was rumbling at the time, still buzzing.
-    pump.shutdown();
    // Join the pump BEFORE the device-wide idle: its decode submissions on the shared
    // device would race vkDeviceWaitIdle otherwise.
    if let Some(st) = stream.take() {