diff --git a/clients/apple/Sources/PunktfunkKit/Gamepad/DualSenseHID.swift b/clients/apple/Sources/PunktfunkKit/Gamepad/DualSenseHID.swift index 7850c254..64b8adf0 100644 --- a/clients/apple/Sources/PunktfunkKit/Gamepad/DualSenseHID.swift +++ b/clients/apple/Sources/PunktfunkKit/Gamepad/DualSenseHID.swift @@ -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. -/// Single-pad model (we forward exactly one controller), so the first match is the right one. +/// Opens one connected Sony DualSense and forwards motor rumble to it over raw HID. +/// +/// 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 - /// be opened (caller then falls back to CoreHaptics). - func open() -> Bool { + /// The IOKit location id of the device this instance opened — the handle a caller correlates + /// with its `GCController`. `nil` until a successful `open`. + 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 ?? [] + 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, 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, - 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() { diff --git a/clients/apple/Sources/PunktfunkKit/Gamepad/GamepadFeedback.swift b/clients/apple/Sources/PunktfunkKit/Gamepad/GamepadFeedback.swift index 9f32ceee..3988b5c2 100644 --- a/clients/apple/Sources/PunktfunkKit/Gamepad/GamepadFeedback.swift +++ b/clients/apple/Sources/PunktfunkKit/Gamepad/GamepadFeedback.swift @@ -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() diff --git a/clients/apple/Sources/PunktfunkKit/Gamepad/RumbleRenderer.swift b/clients/apple/Sources/PunktfunkKit/Gamepad/RumbleRenderer.swift index 563a905f..dada912e 100644 --- a/clients/apple/Sources/PunktfunkKit/Gamepad/RumbleRenderer.swift +++ b/clients/apple/Sources/PunktfunkKit/Gamepad/RumbleRenderer.swift @@ -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) - lastHidWrite = (levels, .now()) + if hid.rumble(low: levels.0, high: levels.1) { + 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 diff --git a/clients/apple/Tests/PunktfunkKitTests/DualSenseHIDTests.swift b/clients/apple/Tests/PunktfunkKitTests/DualSenseHIDTests.swift index 1ea280f2..cc0a0c55 100644 --- a/clients/apple/Tests/PunktfunkKitTests/DualSenseHIDTests.swift +++ b/clients/apple/Tests/PunktfunkKitTests/DualSenseHIDTests.swift @@ -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