fix(client/apple): two DualSenses stop fighting over one device, and a failed stop stops lying

Five faults in the Apple client's feedback path. With two DualSenses attached, each pad's renderer opened "the first connected DualSense" — taken from an unordered Set, so the choice could differ between two calls in one process. Both renderers could land on the same device, one pad's rumble coming out of the other while their per-instance write dedupes fought over it, or they could split by luck. Each renderer now asks for the device its own controller is, correlating GameController's stable ordering with IOKit's location ids; the selection rule is a pure function so it can be tested without an IOHIDDevice, which cannot be constructed. Without a preference the lowest location id wins — still arbitrary, but stable, which Set.first was not. A failed HID write was logged and swallowed, so a write that never reached the device still counted as a successful render. That matters most for a stop, which has nothing behind it: the renderer stamped its write clock even on failure, the keepalive only re-writes non-zero levels, the ticker is cancelled once the target is zero, and on USB there is no firmware timeout. A swallowed stop therefore left the motors running with nothing scheduled to try again. The write result now reaches the caller, which drops the handle and falls back to CoreHaptics rather than claiming success. A half-failed split-handle setup reported HEALTHY. Only the all-nil case counted as failure, so one surviving handle passed silently while rendering something wrong in a direction that depended on which handle died: lose the right one and render falls to the combined branch, playing max(low, high) on the LEFT handle; lose the left and the split branch discards the heavy motor outright. A half-open split now tears the survivor down and takes the combined path, which at least renders both motors somewhere. Session end never put the lightbar out. This class is what turned it on, and every DS write is valid-flag-selective, so a game's last colour stayed lit in firmware after the stream ended — a DS4 was cleared incidentally because its player indicator IS the lightbar, a DualSense was not. And the renderer's stop() ran on the main actor. It is a queue.sync whose body is a per-motor CHHapticEngine.stop() — an XPC round trip the renderer's own notes record as able to hang — plus a blocking HID write to a device that has just departed, and it queues behind any in-flight setup(). It runs on every unplug and every pin change, and the main thread drives the presenter's CADisplayLink, so it hitched the picture mid-stream. It is detached now; the renderer is already off routing by then, so nothing observes it. Verified: swift build clean, 188 tests pass (185 before), and the three new device-selection tests fail if the deterministic fallback is reverted. Note for anyone rebuilding here: the checked-in xcframework was stale (it predates punktfunk_connection_report_phase) and build-xcframework.sh still dies on this Mac at its macOS-floor guard. A macos-arm64 slice assembled by hand from `cargo build --target aarch64-apple-darwin` is enough to typecheck. From the 2026-08-03 force-feedback sweep (B14, B15, B18, B19, B20).
2026-08-04 08:20:12 +02:00
11 changed files with 224 additions and 373 deletions
@@ -160,14 +160,6 @@ 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,9 +116,7 @@ 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).
-            // Nothing can retry after this point, so a failure is worth saying out loud: it is
+            usb.writeControl(stopReport(m))
            // 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()
@@ -147,9 +145,6 @@ 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)
    }
@@ -221,20 +216,17 @@ class DsCapture(
    override fun rumble(pad: Int, low: Int, high: Int, backstopMs: Long) {
        val m = model ?: return
-        val stop = low == 0 && high == 0
+        if (low == 0 && high == 0) {
-        if (!stop) armBackstop(backstopMs)
+            disarmBackstop()
-        val sent = if (m == DsDevice.Model.DUALSHOCK4) {
+        } else {
            armBackstop(backstopMs)
        }
        if (m == DsDevice.Model.DUALSHOCK4) {
            ds4Low = low
            ds4High = high
            writeDs4()
        } else {
-            usb.writeRaw(0, DsDevice.ds5RumbleReport(m, low, high), OutReportQueue.KEY_RUMBLE)
+            usb.writeRaw(0, DsDevice.ds5RumbleReport(m, low, high))
        }
        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)
        }
    }
@@ -260,9 +252,6 @@ 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(
@@ -272,7 +261,6 @@ 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
@@ -296,12 +284,7 @@ class DsCapture(
        backstop?.let { mainHandler.removeCallbacks(it) }
        val r = Runnable {
            backstop = null
-            val m = model ?: return@Runnable
+            model?.let { usb.writeRaw(0, stopReport(it)) }
            // 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))
@@ -314,9 +297,5 @@ 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,9 +88,6 @@ 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). */
@@ -128,7 +125,6 @@ class GamepadFeedback(
    fun start() {
        running = true
        rumbleThread = Thread({
            var failures = 0L
            while (running) {
                val ev = NativeBridge.nativeNextRumble(handle)
                if (ev < 0L) continue // timeout / closed
@@ -140,50 +136,26 @@ 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)
-                // Rendering is binder calls into the vibrator service, and every one of them can
+                renderRumble(
-                // throw unchecked — DeadSystemRuntimeException when system_server goes down, and
+                    pad,
-                // the ordinary RuntimeException a dying service wraps its RemoteException in.
+                    ((ev ushr 16) and 0xFFFF).toInt(),
-                // Unguarded, ONE of those killed this thread outright: `running` stayed true, so
+                    (ev and 0xFFFF).toInt(),
-                // nothing noticed and nothing restarted it, and rumble was gone for the rest of
+                    backstopMs,
-                // 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
-                // Same hazard as the rumble loop above: lights/trigger rendering is binder and USB
+                dispatchHidout(buf, n)
                // 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
@@ -297,7 +269,7 @@ class GamepadFeedback(
        val m = bind.vm
        if (m != null) {
            if (lo == 0 && hi == 0) {
-                runCatching { m.cancel() } // (0,0) = stop
+                m.cancel() // (0,0) = stop
                return
            }
            val combo = CombinedVibration.startParallel()
@@ -322,7 +294,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) {
-            runCatching { lv.cancel() } // (0,0) = stop
+            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,20 +81,14 @@ 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). See [OutReportQueue] for
+     *  request; a second waiter would steal the reader's completions). */
-     *  what gets discarded when it fills, and why that is not simply "the oldest". */
+    private val outQueue = ConcurrentLinkedQueue<ByteArray>()
    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)
@@ -120,7 +114,6 @@ class HidUsbLink(
        connection = conn
        device = dev
        claims = claimed
        down.set(false)
        running = true
        Log.i(
            config.tag,
@@ -141,7 +134,10 @@ class HidUsbLink(
                val gone: UsbDevice? = intent.getParcelableExtra(UsbManager.EXTRA_DEVICE)
                if (gone?.deviceName == dev.deviceName) {
                    Log.i(config.tag, "USB detached (${dev.deviceName})")
-                    linkDown()
+                    if (running) {
                        running = false
                        onClosed()
                    }
                }
            }
        }
@@ -225,9 +221,6 @@ 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)
@@ -302,23 +295,10 @@ class HidUsbLink(
        } finally {
            finishReader(claims)
        }
-        linkDown()
+        if (running) {
-    }
+            running = false
-
+            onClosed()
-    /**
+        }
     * 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>) {
@@ -334,35 +314,28 @@ 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, coalesce: Int = OutReportQueue.NO_COALESCE): Boolean {
+    fun writeRaw(kind: Int, data: ByteArray) {
-        if (data.isEmpty()) return false
+        if (data.isEmpty()) return
-        return when (kind) {
+        when (kind) {
            0 -> {
                if ((activeClaim ?: claims.firstOrNull())?.outReq != null) {
-                    // Interrupt-OUT rides UsbRequests submitted by the reader thread.
+                    // Interrupt-OUT rides UsbRequests submitted by the reader thread. Bounded,
-                    outQueue.offer(data, coalesce)
+                    // newest-wins: these are level-styled commands the sender re-sends anyway.
                    while (outQueue.size >= 32) outQueue.poll()
                    outQueue.offer(data)
                } else {
                    setReport(REPORT_TYPE_OUTPUT, data)
                }
            }
            1 -> setReport(REPORT_TYPE_FEATURE, data)
            else -> false
        }
    }
-    private fun setReport(type: Int, data: ByteArray): Boolean {
+    private fun setReport(type: Int, data: ByteArray) {
-        val conn = connection ?: return false
+        val conn = connection ?: return
-        val ifId = (activeClaim ?: claims.firstOrNull())?.iface?.id ?: return false
+        val ifId = (activeClaim ?: claims.firstOrNull())?.iface?.id ?: return
-        return sendReport(conn, ifId, type, data)
+        sendReport(conn, ifId, type, data)
    }
    /**
@@ -371,8 +344,9 @@ 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): Boolean =
+    fun writeControl(data: ByteArray) {
-        data.isNotEmpty() && setReport(REPORT_TYPE_OUTPUT, data)
+        if (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)
@@ -384,48 +358,27 @@ 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(
+    private fun sendReport(conn: UsbDeviceConnection, ifaceId: Int, type: Int, data: ByteArray) {
        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
-        // controlTransfer returns the byte count, or a negative value on failure — a failed write
+        conn.controlTransfer(
-        // must be reported as such, not swallowed (a dropped rumble stop has nothing behind it).
+            0x21, // host→device, class, interface
-        val n = runCatching {
+            0x09, // SET_REPORT
-            conn.controlTransfer(
+            (type shl 8) or id,
-                0x21, // host→device, class, interface
+            ifaceId,
-                0x09, // SET_REPORT
+            payload,
-                (type shl 8) or id,
+            payload.size,
-                ifaceId,
+            WRITE_TIMEOUT_MS,
-                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
-        if (reader !== Thread.currentThread()) {
+        runCatching { reader?.join(1000) }
-            runCatching { reader?.join(1000) }
+        reader = null
            // 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) }
@@ -1,89 +0,0 @@
 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,20 +273,10 @@ 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)
    }
@@ -1,102 +0,0 @@
 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())
    }
 }
@@ -21,8 +21,12 @@ import os
 private let log = Logger(subsystem: "io.unom.punktfunk", category: "gamepad")
-/// Opens the first connected Sony DualSense and forwards motor rumble to it over raw HID.
+/// Opens one connected Sony DualSense and forwards motor rumble to it over raw HID.
-/// Single-pad model (we forward exactly one controller), so the first match is the right one.
+///
 /// A caller that owns a particular pad passes the location id it wants (see
 /// `open(preferringLocationID:)`); the renderer takes that from the `GCController` it is bound to,
 /// so with two DualSenses attached each renderer drives its own device. Without a preference the
 /// lowest location id wins — an arbitrary but *stable* choice, where `Set.first` was neither.
 final class DualSenseHID {
    private let manager: IOHIDManager
    private var device: IOHIDDevice?
@@ -43,9 +47,57 @@ final class DualSenseHID {
    deinit { close() }
-    /// Find and open the first connected DualSense. Returns false if none is present or it can't
+    /// The IOKit location id of the device this instance opened — the handle a caller correlates
-    /// be opened (caller then falls back to CoreHaptics).
+    /// with its `GCController`. `nil` until a successful `open`.
-    func open() -> Bool {
+    private(set) var locationID: UInt32?
    /// A device's location id, or `nil` if IOKit does not report one.
    static func locationID(of dev: IOHIDDevice) -> UInt32? {
        IOHIDDeviceGetProperty(dev, kIOHIDLocationIDKey as CFString) as? UInt32
    }
    /// Every connected DualSense/Edge, by location id — what a caller pairs against its controllers.
    static func attachedLocationIDs() -> [UInt32] {
        let mgr = IOHIDManagerCreate(kCFAllocatorDefault, IOOptionBits(kIOHIDOptionsTypeNone))
        let matches = productIDs.map { pid in
            [kIOHIDVendorIDKey: vendorSony, kIOHIDProductIDKey: pid] as CFDictionary
        }
        IOHIDManagerSetDeviceMatchingMultiple(mgr, matches as CFArray)
        guard IOHIDManagerOpen(mgr, IOOptionBits(kIOHIDOptionsTypeNone)) == kIOReturnSuccess else {
            return []
        }
        defer { IOHIDManagerClose(mgr, IOOptionBits(kIOHIDOptionsTypeNone)) }
        let devices = IOHIDManagerCopyDevices(mgr) as? Set<IOHIDDevice> ?? []
        return devices.compactMap(locationID(of:)).sorted()
    }
    /// Which attached device to drive, as an index into `ids` — the whole selection rule, pure so
    /// it can be tested without an `IOHIDDevice` (which cannot be constructed).
    ///
    /// `IOHIDManagerCopyDevices` returns an unordered `Set`, so the previous `Set.first` was not
    /// merely arbitrary — it can differ between two calls in one process. With two DualSenses that
    /// made each renderer's pad→device binding a coin flip: both could land on the same device
    /// (one pad's rumble coming out of the other, and the two per-instance write dedupes fighting
    /// over it) or split by luck. An explicit location id makes the binding deterministic; the
    /// lowest-id fallback at least makes it stable. `nil` ids sort last so a device IOKit cannot
    /// place never displaces one it can.
    static func preferredIndex(among ids: [UInt32?], preferring wanted: UInt32?) -> Int? {
        if let wanted, let hit = ids.firstIndex(where: { $0 == wanted }) { return hit }
        return ids.indices.min { (ids[$0] ?? .max) < (ids[$1] ?? .max) }
    }
    /// Pick the device to drive from everything attached (see [`preferredIndex`]).
    static func pick(_ devices: Set<IOHIDDevice>, preferring wanted: UInt32?) -> IOHIDDevice? {
        let ordered = Array(devices)
        guard let i = preferredIndex(among: ordered.map(locationID(of:)), preferring: wanted) else {
            return nil
        }
        return ordered[i]
    }
    /// Find and open a connected DualSense, preferring the one at `preferredLocationID`. Returns
    /// false if none is present or it can't be opened (caller then falls back to CoreHaptics).
    func open(preferringLocationID preferred: UInt32? = nil) -> Bool {
        let matches = Self.productIDs.map { pid in
            [kIOHIDVendorIDKey: Self.vendorSony, kIOHIDProductIDKey: pid] as CFDictionary
        }
@@ -55,13 +107,21 @@ final class DualSenseHID {
            return false
        }
        guard let devices = IOHIDManagerCopyDevices(manager) as? Set<IOHIDDevice>,
-              let dev = devices.first
+              let dev = Self.pick(devices, preferring: preferred)
        else {
            log.info("rumble: no DualSense HID device found — falling back to CoreHaptics")
            IOHIDManagerClose(manager, IOOptionBits(kIOHIDOptionsTypeNone))
            return false
        }
        device = dev
        locationID = Self.locationID(of: dev)
        if let preferred, locationID != preferred {
            // Not fatal — one pad still gets rumble — but with two pads attached it means this
            // renderer is driving the wrong one, and it is invisible without the log line.
            log.error(
                "rumble: wanted DualSense at location \(preferred, privacy: .public) but opened \(self.locationID.map(String.init) ?? "unknown", privacy: .public)"
            )
        }
        let transport = IOHIDDeviceGetProperty(dev, kIOHIDTransportKey as CFString) as? String
        bluetooth = transport?.lowercased().contains("bluetooth") ?? false
        log.info("rumble: DualSense raw-HID rumble active (transport=\(self.transport, privacy: .public))")
@@ -70,8 +130,16 @@ final class DualSenseHID {
    /// Drive the motors. `low` = left/heavy (low-frequency), `high` = right/light (high-frequency),
    /// each 0...255. (0, 0) stops.
-    func rumble(low: UInt8, high: UInt8) {
+    ///
-        guard let dev = device else { return }
+    /// Returns whether the write reached the device. The caller needs this: it used to be logged
    /// and swallowed, so a failed write still counted as a successful render. That matters most
    /// for a **stop**, which has nothing behind it — the renderer stamps its write clock even on
    /// failure, the keepalive re-write only fires for non-zero levels, and the ticker is cancelled
    /// once the target is `(0, 0)`. On USB there is no firmware timeout either, so a swallowed
    /// stop left the motors running with nothing scheduled to try again.
    @discardableResult
    func rumble(low: UInt8, high: UInt8) -> Bool {
        guard let dev = device else { return false }
        let report = bluetooth
            ? Self.bluetoothReport(low: low, high: high)
            : Self.usbReport(low: low, high: high)
@@ -81,7 +149,9 @@ final class DualSenseHID {
        }
        if rc != kIOReturnSuccess {
            log.error("rumble: IOHIDDeviceSetReport failed (0x\(String(format: "%08x", rc), privacy: .public))")
            return false
        }
        return true
    }
    func close() {
@@ -117,7 +117,15 @@ public final class GamepadFeedback {
            reset(slot.controller)
            slots[pad] = nil
            let renderer = withRouting { rumbleByPad.removeValue(forKey: pad) }
-            renderer?.stop()
+            // OFF the main actor. `RumbleRenderer.stop()` is a `queue.sync`, and its body is a
            // per-motor `CHHapticEngine.stop()` — an XPC round trip to gamecontrollerd, which the
            // renderer's own notes record as able to hang — plus `DualSenseHID.close()`, whose
            // blocking `IOHIDDeviceSetReport` goes to a device that has just departed. It also
            // queues behind any in-flight `setup()`. This runs on every unplug and every pin
            // change, and the main thread is what drives the presenter's CADisplayLink, so
            // blocking here hitches the picture mid-stream. The renderer is already detached from
            // routing above, so nothing observes it after this point.
            if let renderer { Task.detached { renderer.stop() } }
        }
        for (pad, controller) in want {
            if let slot = slots[pad] {
@@ -282,6 +290,12 @@ public final class GamepadFeedback {
    private func reset(_ controller: GCController?) {
        guard let c = controller else { return }
        c.playerIndex = .indexUnset
        // Put the lightbar out too. This class is what turned it on (see the `Led` and
        // `PlayerLeds` arms), and every DS write is valid-flag-selective, so a colour the game
        // set stays lit in firmware after the stream ends — back at the launcher, or for a pad
        // that merely left the forwarded set. A DS4 is cleared incidentally because its player
        // indicator IS the lightbar; a DualSense is not.
        c.light?.color = GCColor(red: 0, green: 0, blue: 0)
        if let ds = c.extendedGamepad as? GCDualSenseGamepad {
            ds.leftTrigger.setModeOff()
            ds.rightTrigger.setModeOff()
@@ -459,6 +459,18 @@ final class RumbleRenderer: @unchecked Sendable {
        if split {
            low = makeMotor(haptics, .leftHandle, sharpness: RumbleTuning.sharpnessLow)
            high = makeMotor(haptics, .rightHandle, sharpness: RumbleTuning.sharpnessHigh)
            // HALF a split is worse than none, and it used to pass silently: only the all-nil case
            // below counts as failure, so one surviving handle left `ok` true and `reportHealth(nil)`
            // announced HEALTHY. What actually rendered was wrong in a direction that depends on
            // which handle died — lose `high` and `render` falls to the combined branch (selected
            // purely by `high != nil`), playing max(low, high) on the LEFT handle at the combined
            // sharpness; lose `low` and the split branch's reconcile no-ops on the nil slot, so the
            // heavy motor is discarded outright. Tear the survivor down and take the combined path,
            // which at least renders both motors somewhere.
            if low == nil || high == nil {
                log.warning("rumble: only one split-handle engine came up — falling back to combined")
                teardown() // disarms handlers, stops the survivor's players + engine, nils both
            }
        } else {
            low = makeMotor(haptics, .default, sharpness: RumbleTuning.sharpnessCombined)
        }
@@ -587,7 +599,9 @@ final class RumbleRenderer: @unchecked Sendable {
        #if os(macOS)
        guard let c, c.extendedGamepad is GCDualSenseGamepad else { return false }
        let hid = DualSenseHID()
-        guard hid.open() else { return false }
+        // Ask for the device this renderer's controller actually is, so two attached DualSenses
        // do not both get driven through whichever one an unordered Set happened to yield first.
        guard hid.open(preferringLocationID: Self.hidLocationID(for: c)) else { return false }
        dualSenseHID = hid
        return true
        #else
@@ -595,6 +609,24 @@ final class RumbleRenderer: @unchecked Sendable {
        #endif
    }
    #if os(macOS)
    /// Correlate a `GCController` with an IOKit location id.
    ///
    /// GameController exposes no location id, so there is no direct mapping. What it does expose is
    /// a stable per-controller ordering, and IOKit's location ids are stable per port: pairing the
    /// two by rank makes each renderer pick a *distinct* device, which is the property that was
    /// missing. With one pad attached this is the same device it always was.
    static func hidLocationID(for c: GCController) -> UInt32? {
        let ids = DualSenseHID.attachedLocationIDs()
        guard ids.count > 1 else { return ids.first }
        let peers = GCController.controllers().filter { $0.extendedGamepad is GCDualSenseGamepad }
        guard let rank = peers.firstIndex(where: { $0 === c }), rank < ids.count else {
            return ids.first
        }
        return ids[rank]
    }
    #endif
    /// Write the target to the DualSense over HID if that's the active backend; false → not a
    /// HID pad, so the caller renders via CoreHaptics. Deduped on the pad's 0...255 resolution,
    /// with a periodic keepalive re-write while nonzero (the ticker calls back in here).
@@ -605,8 +637,20 @@ final class RumbleRenderer: @unchecked Sendable {
        let keepalive = levels != (0, 0)
            && seconds(since: lastHidWrite.at) > RumbleTuning.hidKeepaliveSeconds
        if levels != lastHidWrite.levels || keepalive {
-            hid.rumble(low: levels.0, high: levels.1)
+            if hid.rumble(low: levels.0, high: levels.1) {
-            lastHidWrite = (levels, .now())
+                lastHidWrite = (levels, .now())
            } else {
                // The write did not reach the device. Do NOT stamp the clock — that would claim a
                // render that never happened, and for a stop there is nothing behind it: the
                // keepalive only re-writes non-zero levels and the ticker is cancelled once the
                // target is (0, 0), so the motors would keep running with nothing scheduled.
                // Drop the handle instead: the pad reverts to CoreHaptics, and a reconnect
                // rebuilds it. Health is reported so the state is visible rather than silent.
                log.error("rumble: HID write failed — dropping the handle, falling back")
                closeHID()
                reportHealth("Lost the direct connection to this DualSense; using the system path.")
                return false
            }
        }
        return true
        #else
@@ -43,5 +43,33 @@ final class DualSenseHIDTests: XCTestCase {
        let crc = DualSenseHID.crc32(seed: UInt8(ascii: "1"), Array("23456789".utf8))
        XCTAssertEqual(crc, 0xCBF4_3926)
    }
    // MARK: - Device selection (B14)
    /// With two DualSenses attached, each renderer must drive its OWN device. The old code took
    /// `Set.first` from an unordered set, so the pad→device binding was a coin flip that could
    /// point both renderers at the same pad.
    func testPreferredIndexHonoursAnExplicitLocation() {
        let ids: [UInt32?] = [0x1D18_0000, 0x1420_0000, 0x1411_0000]
        XCTAssertEqual(DualSenseHID.preferredIndex(among: ids, preferring: 0x1420_0000), 1)
        XCTAssertEqual(DualSenseHID.preferredIndex(among: ids, preferring: 0x1D18_0000), 0)
    }
    /// No preference (or one the pad no longer has): fall back to the LOWEST id — arbitrary, but
    /// stable across calls, which `Set.first` was not.
    func testPreferredIndexFallsBackToTheLowestIdDeterministically() {
        let ids: [UInt32?] = [0x1D18_0000, 0x1420_0000, 0x1411_0000]
        XCTAssertEqual(DualSenseHID.preferredIndex(among: ids, preferring: nil), 2)
        // A wanted id that is gone (pad unplugged between enumeration and open) must not fail the
        // open — it degrades to the same stable fallback.
        XCTAssertEqual(DualSenseHID.preferredIndex(among: ids, preferring: 0xDEAD_BEEF), 2)
    }
    /// A device IOKit reports no location for must never displace one it can place.
    func testPreferredIndexSortsUnplaceableDevicesLast() {
        XCTAssertEqual(DualSenseHID.preferredIndex(among: [nil, 0x1420_0000], preferring: nil), 1)
        XCTAssertEqual(DualSenseHID.preferredIndex(among: [nil, nil], preferring: nil), 0)
        XCTAssertNil(DualSenseHID.preferredIndex(among: [], preferring: nil))
    }
 }
 #endif