From 42a0dd52bebf5d41c2e74d16e8eb090d14edd037 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Tue, 4 Aug 2026 22:52:38 +0200 Subject: [PATCH] refactor(haptics): one copy of each thing every rumble path was transcribing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Twelve findings from the sweep's DRY/docs/dead-code tail. Most are small; three found real defects hiding behind the duplication. **The UHID event ABI existed five times.** Every UHID gamepad backend — DualSense, DualShock 4, Switch Pro, Steam Controller, Steam Controller 2 — carried its own verbatim copy of the kernel's constants plus its own `put_cstr`, and they had already drifted: `switch_pro` was missing the SET_REPORT pair entirely, and `steam_controller` read a FIXED 16-byte SET_REPORT window instead of the event's own `size`. That last one is a bug in both directions — a longer report was truncated, and a shorter one had the parser reading whatever the reused event buffer still held past the payload, i.e. acting on rumble values the game never wrote. Now one `uhid_abi` module owns the numbers plus the two accessors that are easy to get subtly wrong, with tests on exactly that. **A dead force-feedback id fallback.** ff-core's `input_ff_upload` picks a free effect slot and writes it into the effect BEFORE uinput forwards the request, so the `id == -1` branch could never run — and allocating from a local counter would have been the wrong answer anyway, since the kernel owns that id space. Removed, with a `debug_assert` where it stood. **Apple's HID path silently dropped weak rumble.** `hidByte` took the top byte with no non-zero floor, so every amplitude below 0x0100 rendered as exactly nothing. Android has always floored it at 1; this was the odd one out. That converter also existed twice byte-identically inside one Gradle module — now one `wireAmplitudeToByte`. Also: the DS5 output-report layout gets named offsets (`dualsense_proto::out_report`) documenting all three transport bases — USB 0, SDL payload −1, Bluetooth +2 — since the differing bases are transport-forced, not drift. `pf-client-core` cannot import them (it and `pf-inject` do not depend on each other, and a DualSense layout has no business in `punktfunk-core`, their only shared crate), so its copy now DERIVES its offsets by explicit subtraction and a test pins the relationship. `PUNKTFUNK_HID_EFFECT_MAX` sizes the struct it describes instead of a second literal 11 — the header now emits `uint8_t effect[PUNKTFUNK_HID_EFFECT_MAX]`. The rumble policy engine's `min_pulse_ms` and `keepalive_ms` docs stop naming cases nothing implements: no in-tree caller sets `min_pulse_ms`, and the macOS DualSense-over-BT keepalive the doc cited CANNOT be served by the quirk, because that renderer skips writes whose levels are unchanged and would swallow the engine's re-emit — it keeps its own keepalive instead. `TrackpadHaptic` is marked as staged scaffolding (the tag is on a shipped wire; removing the variant would not reclaim it). Three ×257-vs-`<<8` doc comments corrected — the scaling itself is fine, both round-trip to 255. `backstop_ms.max(160)` deleted as unreachable (the engine floors at 500). New tests for `Ds5Feedback` and for the Android rumble JNI packing on BOTH sides, with `MAX_PADS <= 16` now a compile-time assertion rather than a comment. Closes S1-S9, S11, T2, T3 (design/haptics-sweep-2026-08-03.md M12). S11's second half is NOT a defect and was left alone: `clients/session/src/main.rs` calls `set_forwarding` unconditionally on every params-build (its own comment explains why — browse mode reuses one service across launches), so `Ctl::Forwarding` routinely arrives unchanged and that early-out is what stops a redundant `sync_open` + Valve-HIDAPI cycle each launch. Verified: pf-inject clippy -D warnings 0 / 91 tests; pf-client-core + punktfunk-core clippy 0 / 437 tests (amd64 container); punktfunk-client-android 7 tests; Android :kit: 6 tests; Apple swift build + 189 tests / 0 failures; cargo fmt --all --check clean. Each new test probed by reverting its fix — the fixed SET_REPORT window fails 3, a broken pack shift fails 3, dropping the amplitude floor fails 1, and a wrong DS5 offset either fails the pin or refuses to compile. --- .../kotlin/io/unom/punktfunk/kit/DsDevice.kt | 14 +- .../io/unom/punktfunk/kit/GamepadFeedback.kt | 33 +--- .../io/unom/punktfunk/kit/RumbleWire.kt | 47 ++++++ .../io/unom/punktfunk/kit/RumbleWireTest.kt | 79 +++++++++ clients/android/native/src/feedback.rs | 92 +++++++++- .../Gamepad/ControllerTester.swift | 2 +- .../Gamepad/GamepadFeedback.swift | 4 +- .../PunktfunkKit/Gamepad/RumbleRenderer.swift | 34 ++-- .../PunktfunkKitTests/RumbleTuningTests.swift | 4 +- crates/pf-client-core/src/gamepad.rs | 159 +++++++++++++++++- .../pf-inject/src/inject/linux/dualsense.rs | 26 +-- .../pf-inject/src/inject/linux/dualshock4.rs | 25 +-- crates/pf-inject/src/inject/linux/gamepad.rs | 14 +- .../src/inject/linux/steam_controller.rs | 35 ++-- .../src/inject/linux/steam_controller2.rs | 24 +-- .../pf-inject/src/inject/linux/switch_pro.rs | 22 +-- crates/pf-inject/src/inject/linux/uhid_abi.rs | 143 ++++++++++++++++ .../src/inject/proto/dualsense_proto.rs | 102 +++++++---- crates/pf-inject/src/inject/uhid_manager.rs | 7 +- crates/pf-inject/src/lib.rs | 5 + crates/punktfunk-core/src/abi.rs | 13 +- crates/punktfunk-core/src/client/rumble.rs | 21 ++- crates/punktfunk-core/src/quic/datagram.rs | 8 + include/punktfunk_core.h | 13 +- 24 files changed, 705 insertions(+), 221 deletions(-) create mode 100644 clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/RumbleWire.kt create mode 100644 clients/android/kit/src/test/kotlin/io/unom/punktfunk/kit/RumbleWireTest.kt create mode 100644 crates/pf-inject/src/inject/linux/uhid_abi.rs diff --git a/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/DsDevice.kt b/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/DsDevice.kt index f0f4f5ea..71ae86af 100644 --- a/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/DsDevice.kt +++ b/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/DsDevice.kt @@ -279,8 +279,8 @@ object DsDevice { fun ds5RumbleReport(model: Model, low: Int, high: Int): ByteArray = newDs5(model).also { it[1] = (DS5_FLAG0_COMPAT_VIBRATION or DS5_FLAG0_HAPTICS_SELECT).toByte() it[39] = DS5_FLAG2_VIBRATION2.toByte() - it[3] = amp8(high).toByte() - it[4] = amp8(low).toByte() + it[3] = wireAmplitudeToByte(high).toByte() + it[4] = wireAmplitudeToByte(low).toByte() } /** @@ -324,17 +324,11 @@ object DsDevice { ByteArray(Model.DUALSHOCK4.outputSize).also { it[0] = 0x05 it[1] = (DS4_FLAG0_MOTORS or DS4_FLAG0_LED).toByte() - it[4] = amp8(high).toByte() - it[5] = amp8(low).toByte() + it[4] = wireAmplitudeToByte(high).toByte() + it[5] = wireAmplitudeToByte(low).toByte() it[6] = r.toByte() it[7] = g.toByte() it[8] = b.toByte() } - // Wire u16 amplitude → motor byte; a nonzero command never collapses to 0 (parity with the - // vibrator path's toAmplitude). - private fun amp8(v16: Int): Int { - val a = (v16 ushr 8) and 0xFF - return if (v16 != 0 && a == 0) 1 else a - } } diff --git a/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/GamepadFeedback.kt b/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/GamepadFeedback.kt index 63d22d57..5746b861 100644 --- a/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/GamepadFeedback.kt +++ b/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/GamepadFeedback.kt @@ -127,21 +127,10 @@ class GamepadFeedback( rumbleThread = Thread({ while (running) { val ev = NativeBridge.nativeNextRumble(handle) - if (ev < 0L) continue // timeout / closed - // ev bits 49..52 = wire pad index; bits 32..47 = backstop duration (ms); - // 16..31 = low; 0..15 = high. These are EFFECTIVE commands from the core's shared - // rumble policy engine — it owns every lease/staleness/close decision (uniform - // across all clients; the old 60 s legacy-host exposure is gone) and emits - // explicit zeros, so apply verbatim: (0, 0) = cancel, non-zero = one-shot for - // 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, - ) + // Layout + semantics live in `unpackRumbleEvent` (RumbleWire.kt), tested there + // against the Rust packer. + val cmd = unpackRumbleEvent(ev) ?: continue // timeout / closed + renderRumble(cmd.pad, cmd.low, cmd.high, cmd.backstopMs) } }, "pf-rumble").apply { isDaemon = true; start() } @@ -264,8 +253,8 @@ class GamepadFeedback( return } val bind = rumbleBindFor(pad) ?: return - val lo = toAmplitude(low) - val hi = toAmplitude(high) + val lo = wireAmplitudeToByte(low) + val hi = wireAmplitudeToByte(high) val m = bind.vm if (m != null) { if (lo == 0 && hi == 0) { @@ -314,8 +303,8 @@ class GamepadFeedback( */ private fun renderDeviceRumble(low: Int, high: Int, durationMs: Long) { val v = deviceVibrator ?: return - val lo = toAmplitude(low) - val hi = toAmplitude(high) + val lo = wireAmplitudeToByte(low) + val hi = wireAmplitudeToByte(high) if (lo == 0 && hi == 0) { runCatching { v.cancel() } // (0,0) = stop return @@ -329,12 +318,6 @@ class GamepadFeedback( } } - // 0..0xFFFF → 1..255 (high byte); a nonzero motor never collapses to 0. - private fun toAmplitude(v16: Int): Int { - val a = (v16 ushr 8) and 0xFF - return if (v16 != 0 && a == 0) 1 else a - } - // One-shot held for `durationMs` — the host's v2 TTL (renewed while the level holds), so it // self-terminates on a lost stop; cancel on zero. Floor the duration at 1 ms: `createOneShot` // throws IllegalArgumentException on a non-positive duration, and a lease can carry ttl_ms==0 diff --git a/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/RumbleWire.kt b/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/RumbleWire.kt new file mode 100644 index 00000000..9516d8b2 --- /dev/null +++ b/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/RumbleWire.kt @@ -0,0 +1,47 @@ +package io.unom.punktfunk.kit + +/** + * The two conversions every rumble path in this module needs, in one place. + * + * Both used to be transcribed per call site: [wireAmplitudeToByte] existed twice, byte-identical, + * in `GamepadFeedback` and `DsDevice`; [unpackRumbleEvent] was inline bit-shifting in the poll loop + * with no test on either side of the JNI boundary. Neither is complicated — which is exactly why a + * silent divergence between copies would have been hard to notice. + */ + +/** + * Wire amplitude (`0..0xFFFF`) → an 8-bit motor/vibrator level. + * + * The high byte, except that a **nonzero command never collapses to zero**: anything below 0x0100 + * would otherwise round to silence, turning a weak-but-real rumble into no rumble at all. 1 is + * imperceptibly light, but it moves. + */ +internal fun wireAmplitudeToByte(v16: Int): Int { + val a = (v16 ushr 8) and 0xFF + return if (v16 != 0 && a == 0) 1 else a +} + +/** One effective rumble command, as packed by the native side's `nativeNextRumble`. */ +internal data class RumbleCmd(val pad: Int, val low: Int, val high: Int, val backstopMs: Long) + +/** + * Unpack `NativeBridge.nativeNextRumble`'s `jlong`, or null for the timeout/closed sentinel. + * + * Layout, mirroring `clients/android/native/src/feedback.rs::pack_rumble`: + * bits 49..52 = wire pad index, 32..47 = backstop duration (ms), 16..31 = low, 0..15 = high. + * The pad field is 4 bits because `punktfunk_core::input::MAX_PADS` is 16 — the Rust side has a + * compile-time assertion tying the two together, so this can't silently start truncating. + * + * These are EFFECTIVE commands from the core's shared rumble policy engine: it owns every + * lease/staleness/close decision and emits explicit zeros, so apply them verbatim — + * `(0, 0)` = cancel, non-zero = one-shot for the backstop. + */ +internal fun unpackRumbleEvent(ev: Long): RumbleCmd? { + if (ev < 0L) return null // timeout / closed + return RumbleCmd( + pad = ((ev ushr 49) and 0xFL).toInt(), + low = ((ev ushr 16) and 0xFFFF).toInt(), + high = (ev and 0xFFFF).toInt(), + backstopMs = (ev ushr 32) and 0xFFFF, + ) +} diff --git a/clients/android/kit/src/test/kotlin/io/unom/punktfunk/kit/RumbleWireTest.kt b/clients/android/kit/src/test/kotlin/io/unom/punktfunk/kit/RumbleWireTest.kt new file mode 100644 index 00000000..30226dde --- /dev/null +++ b/clients/android/kit/src/test/kotlin/io/unom/punktfunk/kit/RumbleWireTest.kt @@ -0,0 +1,79 @@ +package io.unom.punktfunk.kit + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertNull +import org.junit.Test + +/** + * The Kotlin half of the rumble JNI boundary. The Rust half is pinned by `pack_rumble_tests` in + * `clients/android/native/src/feedback.rs`; the two suites describe the same layout from opposite + * sides, which is the only thing that catches one of them drifting. + */ +class RumbleWireTest { + + /** `pack_rumble` from the native side, transcribed — the packer these tests unpack. */ + private fun pack(pad: Int, low: Int, high: Int, backstopMs: Int): Long = + ((pad and 0xF).toLong() shl 49) or + ((backstopMs.coerceAtMost(0xFFFF)).toLong() shl 32) or + (low.toLong() shl 16) or + high.toLong() + + @Test + fun `every field round-trips at its extremes`() { + val cases = listOf( + listOf(0, 0, 0, 0), + listOf(15, 0xFFFF, 0xFFFF, 0xFFFF), + listOf(1, 0x1234, 0x5678, 500), + listOf(7, 0, 0xFFFF, 2000), + ) + for ((pad, low, high, backstop) in cases) { + val cmd = unpackRumbleEvent(pack(pad, low, high, backstop))!! + assertEquals("pad", pad, cmd.pad) + assertEquals("low", low, cmd.low) + assertEquals("high", high, cmd.high) + assertEquals("backstop", backstop.toLong(), cmd.backstopMs) + } + } + + /** MAX_PADS is 16, so all 16 indices must survive the 4-bit field without aliasing. */ + @Test + fun `all sixteen pad indices are distinct`() { + val seen = (0 until 16).map { unpackRumbleEvent(pack(it, 1, 2, 3))!!.pad } + assertEquals((0 until 16).toList(), seen) + } + + @Test + fun `the negative sentinel is not a command`() { + assertNull(unpackRumbleEvent(-1L)) + assertNull(unpackRumbleEvent(Long.MIN_VALUE)) + } + + @Test + fun `a stop is distinguishable from a hold`() { + val stop = unpackRumbleEvent(pack(2, 0, 0, 0))!! + val hold = unpackRumbleEvent(pack(2, 0x8000, 0x8000, 500))!! + assertEquals(0, stop.low) + assertEquals(0, stop.high) + assertNotEquals(stop, hold) + } + + // --- wireAmplitudeToByte (was two byte-identical private copies) --- + + @Test + fun `amplitude takes the high byte`() { + assertEquals(0xFF, wireAmplitudeToByte(0xFFFF)) + assertEquals(0x80, wireAmplitudeToByte(0x8000)) + assertEquals(0x12, wireAmplitudeToByte(0x1234)) + } + + @Test + fun `zero stays silent but a weak nonzero never does`() { + assertEquals("only a real zero may render as silence", 0, wireAmplitudeToByte(0)) + // Everything below 0x0100 has a zero high byte — without the floor these all vanish. + for (v in listOf(1, 0x0042, 0x00FF)) { + assertEquals("wire $v collapsed to silence", 1, wireAmplitudeToByte(v)) + } + assertEquals(1, wireAmplitudeToByte(0x0100)) // first value that reaches 1 on its own + } +} diff --git a/clients/android/native/src/feedback.rs b/clients/android/native/src/feedback.rs index 6833666e..432b050d 100644 --- a/clients/android/native/src/feedback.rs +++ b/clients/android/native/src/feedback.rs @@ -18,6 +18,29 @@ use std::time::Duration; /// observes its `running=false` flag promptly on teardown. const PULL_TIMEOUT: Duration = Duration::from_millis(100); +/// Width of the packed `pad` field in [`pack_rumble`] — 4 bits, i.e. indices 0..15. +const PAD_BITS: u32 = 4; +/// The packing is only lossless while every representable pad index fits in [`PAD_BITS`]. This was +/// a comment before; growing `MAX_PADS` past 16 would have silently aliased pad 16 onto pad 0 +/// rather than failing the build. +const _: () = assert!( + punktfunk_core::input::MAX_PADS <= 1usize << PAD_BITS, + "MAX_PADS no longer fits the 4-bit pad field in the packed rumble long" +); + +/// Pack one effective rumble command into the `jlong` `nativeNextRumble` returns. +/// +/// Layout — mirrored by `unpackRumbleEvent` in `RumbleWire.kt`: bits 49..52 `pad`, 32..47 +/// `backstop_ms`, 16..31 `low`, 0..15 `high`. Always non-negative, so the `-1` timeout/closed +/// sentinel stays unambiguous. Split out from the JNI entry point purely so it can be tested +/// without a live session handle — the shift arithmetic is the part worth pinning. +fn pack_rumble(pad: u16, low: u16, high: u16, backstop_ms: u32) -> jlong { + (jlong::from(pad & ((1 << PAD_BITS) - 1)) << 49) + | (jlong::from(backstop_ms.min(0xFFFF) as u16) << 32) + | (jlong::from(low) << 16) + | jlong::from(high) +} + // HID-output kind tags written into the returned ByteBuffer (Kotlin reads them back). const TAG_LED: u8 = 0x01; const TAG_PLAYER_LEDS: u8 = 0x02; @@ -54,12 +77,7 @@ pub extern "system" fn Java_io_unom_punktfunk_kit_NativeBridge_nativeNextRumble( // handle. let h = unsafe { &*(handle as *const SessionHandle) }; match h.client.next_rumble_command(PULL_TIMEOUT) { - Ok(cmd) => { - (jlong::from(cmd.pad & 0xF) << 49) - | (jlong::from(cmd.backstop_ms.min(0xFFFF) as u16) << 32) - | (jlong::from(cmd.low) << 16) - | jlong::from(cmd.high) - } + Ok(cmd) => pack_rumble(cmd.pad, cmd.low, cmd.high, cmd.backstop_ms), Err(_) => -1, // NoFrame (timeout) or Closed — Kotlin loops on its running flag } }) @@ -160,3 +178,65 @@ pub extern "system" fn Java_io_unom_punktfunk_kit_NativeBridge_nativeNextHidout( n as jint }) } + +#[cfg(test)] +mod pack_rumble_tests { + use super::*; + use punktfunk_core::input::MAX_PADS; + + /// Kotlin's `unpackRumbleEvent`, transcribed — if these two ever disagree the boundary is + /// broken, and nothing else in the build would say so. + fn unpack(ev: jlong) -> (u16, u16, u16, u32) { + let pad = ((ev >> 49) & 0xF) as u16; + let backstop = ((ev >> 32) & 0xFFFF) as u32; + let low = ((ev >> 16) & 0xFFFF) as u16; + let high = (ev & 0xFFFF) as u16; + (pad, low, high, backstop) + } + + #[test] + fn round_trips_every_field_at_its_extremes() { + for &(pad, low, high, backstop) in &[ + (0u16, 0u16, 0u16, 0u32), + (15, 0xFFFF, 0xFFFF, 0xFFFF), + (1, 0x1234, 0x5678, 500), + (7, 0, 0xFFFF, 2000), + ] { + let ev = pack_rumble(pad, low, high, backstop); + assert_eq!(unpack(ev), (pad, low, high, backstop), "pad {pad}"); + } + } + + #[test] + fn every_representable_pad_survives_the_four_bit_field() { + for pad in 0..MAX_PADS as u16 { + let (got, ..) = unpack(pack_rumble(pad, 1, 2, 3)); + assert_eq!(got, pad, "pad {pad} aliased in the packed long"); + } + } + + #[test] + fn a_packed_command_is_never_negative() { + // `-1` is the timeout/closed sentinel; any packed value colliding with it would read as + // "no command" and the rumble would simply vanish. + assert!(pack_rumble(15, 0xFFFF, 0xFFFF, 0xFFFF) >= 0); + assert!(pack_rumble(0, 0, 0, 0) >= 0); + } + + #[test] + fn an_oversized_backstop_saturates_instead_of_corrupting_the_pad_field() { + let ev = pack_rumble(3, 0, 0, u32::MAX); + let (pad, _, _, backstop) = unpack(ev); + assert_eq!(pad, 3, "a huge backstop must not bleed into the pad bits"); + assert_eq!(backstop, 0xFFFF); + } + + #[test] + fn a_stop_is_distinguishable_from_a_hold() { + let stop = pack_rumble(2, 0, 0, 0); + let hold = pack_rumble(2, 0x8000, 0x8000, 500); + assert_ne!(stop, hold); + assert_eq!(unpack(stop).1, 0); + assert_eq!(unpack(stop).2, 0); + } +} diff --git a/clients/apple/Sources/PunktfunkKit/Gamepad/ControllerTester.swift b/clients/apple/Sources/PunktfunkKit/Gamepad/ControllerTester.swift index 422182c8..0b481c4c 100644 --- a/clients/apple/Sources/PunktfunkKit/Gamepad/ControllerTester.swift +++ b/clients/apple/Sources/PunktfunkKit/Gamepad/ControllerTester.swift @@ -12,7 +12,7 @@ import GameController public final class ControllerTester: ObservableObject { // `.manual`: the panel's toggles hold a level until changed — no session wire refreshes // exist here to keep the renderer's staleness watchdog fed. - private let renderer = RumbleRenderer(policy: .manual) + private let renderer = RumbleRenderer() private weak var controller: GCController? /// The rumble backend now in use — "DualSense HID · USB/Bluetooth", "CoreHaptics", or "—" — diff --git a/clients/apple/Sources/PunktfunkKit/Gamepad/GamepadFeedback.swift b/clients/apple/Sources/PunktfunkKit/Gamepad/GamepadFeedback.swift index 9f32ceee..3c63b016 100644 --- a/clients/apple/Sources/PunktfunkKit/Gamepad/GamepadFeedback.swift +++ b/clients/apple/Sources/PunktfunkKit/Gamepad/GamepadFeedback.swift @@ -65,7 +65,7 @@ public final class GamepadFeedback { #if os(iOS) if UserDefaults.standard.bool(forKey: DefaultsKey.rumbleOnDevice), CHHapticEngine.capabilitiesForHardware().supportsHaptics { - deviceRumble = RumbleRenderer(policy: .session, actuator: .device) + deviceRumble = RumbleRenderer(actuator: .device) } else { deviceRumble = nil } @@ -128,7 +128,7 @@ public final class GamepadFeedback { replay(slot) } else { slots[pad] = Slot(controller: controller) - let renderer = RumbleRenderer(policy: .session) + let renderer = RumbleRenderer() renderer.retarget(controller) withRouting { rumbleByPad[pad] = renderer } } diff --git a/clients/apple/Sources/PunktfunkKit/Gamepad/RumbleRenderer.swift b/clients/apple/Sources/PunktfunkKit/Gamepad/RumbleRenderer.swift index 563a905f..7b76f246 100644 --- a/clients/apple/Sources/PunktfunkKit/Gamepad/RumbleRenderer.swift +++ b/clients/apple/Sources/PunktfunkKit/Gamepad/RumbleRenderer.swift @@ -43,8 +43,14 @@ enum RumbleTuning { /// Wire amplitude (0...0xFFFF) → CoreHaptics intensity (0...1). static func amplitude(_ wire: UInt16) -> Float { Float(wire) / 65535 } - /// Wire amplitude → DualSense HID motor byte. - static func hidByte(_ wire: UInt16) -> UInt8 { UInt8(wire >> 8) } + /// Wire amplitude → DualSense HID motor byte. A nonzero command never collapses to silence: + /// the top byte of anything below 0x0100 is 0, so a weak-but-real rumble used to render as + /// nothing at all on this path. Floored at 1 — imperceptibly light, but moving. (Android's + /// `toAmplitude` has always done this; this was the odd one out.) + static func hidByte(_ wire: UInt16) -> UInt8 { + let b = UInt8(wire >> 8) + return wire != 0 && b == 0 ? 1 : b + } /// Single-actuator pads render whichever motor is stronger. static func combined(low: UInt16, high: UInt16) -> UInt16 { max(low, high) } /// Are two baked levels the same (skip the rebuild)? @@ -81,10 +87,11 @@ enum RumbleTuning { /// 4. **Escalating stop.** A throwing `player.stop` means the engine's state is unknown — the /// whole engine is stopped (silencing every player it hosts) and lazily rebuilt behind the /// exponential backoff. -/// 5. **Staleness watchdog** (`Policy.session`): audible with no wire command for -/// `sessionStaleSeconds` → force silence. A lost stop can outlive the host's 500 ms heal -/// only if the channel itself died, and then the pad must not buzz forever. `Policy.manual` -/// (the settings test panel) instead holds a level until it is changed. +/// 5. **No staleness watchdog here.** There was one, keyed off a `Policy` type and a +/// `sessionStaleSeconds`; both are gone. Every liveness decision — lease expiry, legacy-host +/// staleness, session close — now belongs to punktfunk-core's shared policy engine +/// (`client/rumble.rs`), which emits explicit zero commands, so this renderer applies what it +/// is told and never decides on its own when a level should end. /// /// Engines are created lazily on the first nonzero amplitude and torn down on retarget; /// failures (pads without haptics, engine resets) downgrade to silence — rumble is best-effort @@ -93,17 +100,6 @@ enum RumbleTuning { /// `@unchecked Sendable` is sound because every property is read and written only inside /// `queue` closures — the serial queue is the synchronization. final class RumbleRenderer: @unchecked Sendable { - /// Who ends an un-refreshed nonzero target. Session mode applies the core policy engine's - /// commands verbatim — the engine (punktfunk-core `client/rumble.rs`) owns every lease, - /// staleness, and close decision and emits explicit zeros, so the renderer keeps NO - /// staleness policy of its own anymore. The controller test panel (`manual`) holds a slider - /// level indefinitely; both are identical renderer-side today, the distinction is kept for - /// the call sites' intent. - struct Policy { - static let session = Policy() - static let manual = Policy() - } - /// Which physical actuator this renderer drives: the forwarded controller's haptics engine /// (the default), or THIS device's own Taptic Engine (`CHHapticEngine()`) — the opt-in /// "rumble on this device" mirror for phone-clip pads that ship without rumble motors. @@ -115,7 +111,6 @@ final class RumbleRenderer: @unchecked Sendable { } private let queue = DispatchQueue(label: "io.unom.punktfunk.haptics", qos: .userInteractive) - private let policy: Policy private let actuator: Actuator /// One finite haptic play on a motor: the player plus when (engine timeline) it expires. @@ -190,8 +185,7 @@ final class RumbleRenderer: @unchecked Sendable { ((0, 0), DispatchTime(uptimeNanoseconds: 0)) #endif - init(policy: Policy = .session, actuator: Actuator = .controller) { - self.policy = policy + init(actuator: Actuator = .controller) { self.actuator = actuator } diff --git a/clients/apple/Tests/PunktfunkKitTests/RumbleTuningTests.swift b/clients/apple/Tests/PunktfunkKitTests/RumbleTuningTests.swift index 8bbf1a4e..073433c7 100644 --- a/clients/apple/Tests/PunktfunkKitTests/RumbleTuningTests.swift +++ b/clients/apple/Tests/PunktfunkKitTests/RumbleTuningTests.swift @@ -56,7 +56,7 @@ final class RumbleTuningTests: XCTestCase { /// storm, an audible target left to the ticker (watchdog path), then `stop()` — which runs /// `queue.sync` against the same serial queue the ticker fires on and must not deadlock. func testRendererSurvivesCallStormAndTeardownWithoutController() { - let renderer = RumbleRenderer(policy: .session) + let renderer = RumbleRenderer() renderer.retarget(nil) for i in 0..<500 { renderer.apply( @@ -72,7 +72,7 @@ final class RumbleTuningTests: XCTestCase { /// every policy stop (lease expiry, legacy staleness, session close), and the renderer's only /// job is to apply them. Drive the real queue/ticker (no physical pad) and confirm no wedge. func testZeroCommandSilencesAndTeardownDoesNotDeadlock() { - let renderer = RumbleRenderer(policy: .session) + let renderer = RumbleRenderer() renderer.retarget(nil) renderer.apply(low: 0x8000, high: 0x8000) Thread.sleep(forTimeInterval: 0.1) diff --git a/crates/pf-client-core/src/gamepad.rs b/crates/pf-client-core/src/gamepad.rs index a543fc35..f42b4db9 100644 --- a/crates/pf-client-core/src/gamepad.rs +++ b/crates/pf-client-core/src/gamepad.rs @@ -682,13 +682,27 @@ fn axis_value(axis: sdl3::gamepad::Axis, v: i16) -> (u32, i32) { /// host parses off its virtual pad; the wire's 11-byte trigger blocks drop in verbatim. /// Enable bits select only the fields each update touches, so rumble (driven separately /// through SDL) and untouched fields keep their state. +/// +/// The offsets below are the USB output report's, **minus one**: SDL's payload carries no leading +/// report id. `pf-inject`'s `dualsense_proto::out_report` is where that layout is written down and +/// explained (including the Bluetooth `+2` base), but this crate cannot import it — `pf-inject` is +/// host-side and neither crate depends on the other, and a DualSense report layout has no business +/// in `punktfunk-core`, the only crate they share. So this is a deliberate second copy, and +/// [`ds5_offsets_track_the_usb_report`](ds5_feedback_tests) pins the `−1` relationship rather than +/// leaving it to a comment. struct Ds5Feedback; impl Ds5Feedback { - const RIGHT_TRIGGER: usize = 10; - const LEFT_TRIGGER: usize = 21; - const PAD_LIGHTS: usize = 43; - const LED_RGB: usize = 44; + /// The USB report offsets these are derived from — see the type doc. Kept beside the derived + /// values so the subtraction is visible at the point of definition. + const REPORT_ID_LEN: usize = 1; + const RIGHT_TRIGGER: usize = 11 - Self::REPORT_ID_LEN; + const LEFT_TRIGGER: usize = 22 - Self::REPORT_ID_LEN; + const PAD_LIGHTS: usize = 44 - Self::REPORT_ID_LEN; + const LED_RGB: usize = 45 - Self::REPORT_ID_LEN; + /// One adaptive-trigger parameter block: a mode byte plus 10 parameters. Mirrors + /// `PUNKTFUNK_HID_EFFECT_MAX`, which is the same number at the C-ABI boundary. + const TRIGGER_LEN: usize = punktfunk_core::abi::PUNKTFUNK_HID_EFFECT_MAX as usize; fn trigger_packet(which: u8, effect: &[u8]) -> [u8; 47] { let mut p = [0u8; 47]; @@ -698,7 +712,7 @@ impl Ds5Feedback { (0x08, Self::LEFT_TRIGGER) }; p[0] = flag; - let n = effect.len().min(11); + let n = effect.len().min(Self::TRIGGER_LEN); p[off..off + n].copy_from_slice(&effect[..n]); p } @@ -1837,7 +1851,12 @@ impl Worker { let dur_ms: u32 = if (low, high) == (0, 0) { 100 // a stop takes effect immediately; the duration is irrelevant } else { - backstop_ms.max(160) // floor: a jittered renewal can never gap the actuator + // No local floor. There was a `.max(160)` here, and it could never do anything: the + // engine's own `backstop()` returns `(2 * ttl).clamp(500, 5000)` or the 2000 ms legacy + // value, so a non-zero command's backstop is never below 500. A floor that belongs to a + // particular actuator belongs in its `ActuatorQuirks::min_pulse_ms`, which the engine + // already applies — not re-invented per renderer where it can silently disagree. + backstop_ms }; // Surface a failed SDL rumble write: a swallowed error here (DualSense not in the right // HIDAPI mode, etc.) reads exactly like "rumble doesn't work". The host logs the send side @@ -2387,3 +2406,131 @@ mod slot_tests { ); } } + +/// [`Ds5Feedback`]'s three packet builders. The host-side parser, the Android writer and the Apple +/// writer are all pinned by their own suites; this writer had nothing, despite being the one that +/// hand-shifts every offset by the report-id length. +#[cfg(test)] +mod ds5_feedback_tests { + use super::*; + + /// The USB output report offsets, written out independently of the implementation. A DS5 + /// effects payload is the same block with the leading report id removed, so every offset is + /// exactly one lower — this is the relationship the derived constants encode. + #[test] + fn ds5_offsets_track_the_usb_report() { + for (usb, payload) in [ + (11usize, Ds5Feedback::RIGHT_TRIGGER), + (22, Ds5Feedback::LEFT_TRIGGER), + (44, Ds5Feedback::PAD_LIGHTS), + (45, Ds5Feedback::LED_RGB), + ] { + assert_eq!(payload, usb - 1, "payload offset for USB byte {usb}"); + } + assert_eq!(Ds5Feedback::TRIGGER_LEN, 11); + } + + #[test] + fn lightbar_sets_only_its_enable_bit_and_its_three_bytes() { + let p = Ds5Feedback::lightbar_packet(0x11, 0x22, 0x33); + assert_eq!(p.len(), 47); + assert_eq!(p[1], 0x04, "valid_flag1 lightbar bit"); + assert_eq!(p[0], 0, "must not claim any valid_flag0 field"); + assert_eq!( + ( + p[Ds5Feedback::LED_RGB], + p[Ds5Feedback::LED_RGB + 1], + p[Ds5Feedback::LED_RGB + 2] + ), + (0x11, 0x22, 0x33) + ); + // Everything else stays zero — an over-broad packet would blank the triggers/player LEDs + // it never meant to touch. + let touched = [ + 1, + Ds5Feedback::LED_RGB, + Ds5Feedback::LED_RGB + 1, + Ds5Feedback::LED_RGB + 2, + ]; + assert!(p + .iter() + .enumerate() + .all(|(i, &b)| touched.contains(&i) || b == 0)); + } + + #[test] + fn player_leds_are_masked_to_five_bits() { + let p = Ds5Feedback::player_packet(0xFF); + assert_eq!(p[1], 0x10, "valid_flag1 player-indicator bit"); + assert_eq!( + p[Ds5Feedback::PAD_LIGHTS], + 0x1F, + "high bits are not ours to set" + ); + let p = Ds5Feedback::player_packet(0b0000_0101); + assert_eq!(p[Ds5Feedback::PAD_LIGHTS], 0b0000_0101); + } + + /// which 1 = R2 and which 0 = L2 — and the RIGHT block sits FIRST in the report, which is the + /// pairing most likely to be transcribed backwards. + #[test] + fn trigger_which_selects_the_right_flag_and_offset() { + let eff: Vec = (1..=11).collect(); + + let r = Ds5Feedback::trigger_packet(1, &eff); + assert_eq!(r[0], 0x04, "valid_flag0 R2 bit"); + assert_eq!( + &r[Ds5Feedback::RIGHT_TRIGGER..Ds5Feedback::RIGHT_TRIGGER + 11], + &eff[..] + ); + assert_eq!( + r[Ds5Feedback::LEFT_TRIGGER], + 0, + "the other trigger is untouched" + ); + + let l = Ds5Feedback::trigger_packet(0, &eff); + assert_eq!(l[0], 0x08, "valid_flag0 L2 bit"); + assert_eq!( + &l[Ds5Feedback::LEFT_TRIGGER..Ds5Feedback::LEFT_TRIGGER + 11], + &eff[..] + ); + assert_eq!(l[Ds5Feedback::RIGHT_TRIGGER], 0); + } + + #[test] + fn an_oversized_effect_is_clamped_rather_than_overflowing_into_the_next_field() { + let long = vec![0xAAu8; 40]; + let p = Ds5Feedback::trigger_packet(1, &long); + assert_eq!(p.len(), 47); + // Exactly TRIGGER_LEN bytes written; the left block must not be scribbled on. + assert_eq!(p[Ds5Feedback::RIGHT_TRIGGER + 10], 0xAA); + assert_eq!(p[Ds5Feedback::RIGHT_TRIGGER + 11], 0); + assert_eq!(p[Ds5Feedback::LEFT_TRIGGER], 0); + } + + #[test] + fn a_short_effect_leaves_the_rest_of_the_block_zeroed() { + let p = Ds5Feedback::trigger_packet(0, &[0x02, 0x99]); + assert_eq!(p[Ds5Feedback::LEFT_TRIGGER], 0x02); + assert_eq!(p[Ds5Feedback::LEFT_TRIGGER + 1], 0x99); + assert!( + p[Ds5Feedback::LEFT_TRIGGER + 2..Ds5Feedback::LEFT_TRIGGER + 11] + .iter() + .all(|&b| b == 0) + ); + } + + /// An empty effect is a well-formed all-zero block: mode 0x00 = release. It must still assert + /// its enable bit, or the pad keeps whatever effect it was holding. + #[test] + fn an_empty_effect_is_a_release_not_a_no_op() { + let p = Ds5Feedback::trigger_packet(1, &[]); + assert_eq!(p[0], 0x04); + assert!( + p[Ds5Feedback::RIGHT_TRIGGER..Ds5Feedback::RIGHT_TRIGGER + 11] + .iter() + .all(|&b| b == 0) + ); + } +} diff --git a/crates/pf-inject/src/inject/linux/dualsense.rs b/crates/pf-inject/src/inject/linux/dualsense.rs index 121388e4..2e90f5f0 100644 --- a/crates/pf-inject/src/inject/linux/dualsense.rs +++ b/crates/pf-inject/src/inject/linux/dualsense.rs @@ -17,6 +17,11 @@ use super::dualsense_proto::{ DS_EDGE_PRODUCT, DS_FEATURE_CALIBRATION, DS_FEATURE_FIRMWARE, DS_INPUT_REPORT_LEN, DS_PRODUCT, DS_TOUCH_H, DS_TOUCH_W, DS_VENDOR, DUALSENSE_EDGE_RDESC, DUALSENSE_RDESC, }; +use crate::uhid_abi::{ + put_cstr, BUS_USB, HID_MAX_DESCRIPTOR_SIZE, UHID_CREATE2, UHID_DESTROY, UHID_EVENT_SIZE, + UHID_GET_REPORT, UHID_GET_REPORT_REPLY, UHID_INPUT2, UHID_OUTPUT, UHID_PATH, UHID_SET_REPORT, + UHID_SET_REPORT_REPLY, +}; use crate::uhid_manager::{PadFeedback, PadProto, UhidManager}; use anyhow::{Context, Result}; use punktfunk_core::quic::RichInput; @@ -24,27 +29,6 @@ use std::fs::{File, OpenOptions}; use std::io::{Read, Write}; use std::os::unix::fs::OpenOptionsExt; -// /dev/uhid event ABI (linux/uhid.h). `struct uhid_event` is __packed__: a u32 `type` then a -// union whose largest member is uhid_create2_req (128+64+64 + 2+2 + 4*4 + rd_data[4096] = 4372). -const UHID_PATH: &str = "/dev/uhid"; -const UHID_DESTROY: u32 = 1; -const UHID_OUTPUT: u32 = 6; -const UHID_GET_REPORT: u32 = 9; -const UHID_GET_REPORT_REPLY: u32 = 10; -const UHID_CREATE2: u32 = 11; -const UHID_INPUT2: u32 = 12; -const UHID_SET_REPORT: u32 = 13; -const UHID_SET_REPORT_REPLY: u32 = 14; -const HID_MAX_DESCRIPTOR_SIZE: usize = 4096; -const UHID_EVENT_SIZE: usize = 4 + 4372; // type + union (create2) -const BUS_USB: u16 = 0x03; - -/// Copy a NUL-padded C string field into the event buffer. -fn put_cstr(ev: &mut [u8], off: usize, cap: usize, s: &str) { - let n = s.len().min(cap - 1); - ev[off..off + n].copy_from_slice(&s.as_bytes()[..n]); // rest already zero (NUL-terminated) -} - /// The UHID identity a [`DualSensePad`] is created with — the plain DualSense or the Edge (same /// driver, same report codec; the Edge differs by PID + descriptor and carries the four extra /// `buttons[2]` bits). Mirrors the uinput pad's `PadIdentity` shape. diff --git a/crates/pf-inject/src/inject/linux/dualshock4.rs b/crates/pf-inject/src/inject/linux/dualshock4.rs index 1f9555d2..fd37d9ee 100644 --- a/crates/pf-inject/src/inject/linux/dualshock4.rs +++ b/crates/pf-inject/src/inject/linux/dualshock4.rs @@ -18,6 +18,11 @@ use super::dualshock4_proto::{ parse_ds4_output, serialize_state, Ds4Feedback, DS4_INPUT_REPORT_LEN, DS4_PRODUCT, DS4_TOUCH_H, DS4_TOUCH_W, DS4_VENDOR, }; +use crate::uhid_abi::{ + put_cstr, BUS_USB, HID_MAX_DESCRIPTOR_SIZE, UHID_CREATE2, UHID_DESTROY, UHID_EVENT_SIZE, + UHID_GET_REPORT, UHID_GET_REPORT_REPLY, UHID_INPUT2, UHID_OUTPUT, UHID_PATH, UHID_SET_REPORT, + UHID_SET_REPORT_REPLY, +}; use crate::uhid_manager::{PadFeedback, PadProto, UhidManager}; use anyhow::{Context, Result}; use punktfunk_core::quic::{HidOutput, RichInput}; @@ -25,20 +30,6 @@ use std::fs::{File, OpenOptions}; use std::io::{Read, Write}; use std::os::unix::fs::OpenOptionsExt; -// /dev/uhid event ABI (linux/uhid.h) — identical to the DualSense backend's; see `super::dualsense`. -const UHID_PATH: &str = "/dev/uhid"; -const UHID_DESTROY: u32 = 1; -const UHID_OUTPUT: u32 = 6; -const UHID_GET_REPORT: u32 = 9; -const UHID_GET_REPORT_REPLY: u32 = 10; -const UHID_CREATE2: u32 = 11; -const UHID_INPUT2: u32 = 12; -const UHID_SET_REPORT: u32 = 13; -const UHID_SET_REPORT_REPLY: u32 = 14; -const HID_MAX_DESCRIPTOR_SIZE: usize = 4096; -const UHID_EVENT_SIZE: usize = 4 + 4372; // type + union (create2) -const BUS_USB: u16 = 0x03; - // Feature reports `hid-playstation` GET_REPORTs during DS4 init. The PAIRING report (0x12) is // MANDATORY — without a valid reply `dualshock4_create()` aborts and creates NO input devices; the // kernel reads the 6-byte device MAC from bytes 1..7. CALIBRATION (0x02) and FIRMWARE (0xa3) are @@ -144,12 +135,6 @@ const DS4_RDESC: &[u8] = &[ 0xB1, 0x02, 0xC0, ]; -/// Copy a NUL-padded C string field into the event buffer. -fn put_cstr(ev: &mut [u8], off: usize, cap: usize, s: &str) { - let n = s.len().min(cap - 1); - ev[off..off + n].copy_from_slice(&s.as_bytes()[..n]); // rest already zero (NUL-terminated) -} - /// A virtual DualShock 4 backed by `/dev/uhid` (hand-rolled codec mirroring the DualSense pad's). /// Dropping it destroys the device (the kernel tears down the bound `hid-playstation` interface). pub struct DualShock4Pad { diff --git a/crates/pf-inject/src/inject/linux/gamepad.rs b/crates/pf-inject/src/inject/linux/gamepad.rs index 435cf87d..6e7eef98 100644 --- a/crates/pf-inject/src/inject/linux/gamepad.rs +++ b/crates/pf-inject/src/inject/linux/gamepad.rs @@ -268,7 +268,6 @@ struct Effect { /// the policy is pure and unit-testable without a live uinput fd. struct FfState { effects: HashMap, - next_effect_id: i16, gain: u32, /// Last `(low, high)` reported, to dedup. last_mix: (u16, u16), @@ -284,7 +283,6 @@ impl FfState { fn new() -> FfState { FfState { effects: HashMap::new(), - next_effect_id: 0, gain: 0xFFFF, last_mix: (0, 0), last_activity: Instant::now(), @@ -531,11 +529,13 @@ impl VirtualPad { let mut up: UinputFfUpload = unsafe { std::mem::zeroed() }; up.request_id = ev.value as u32; if ioctl_ptr(raw, UI_BEGIN_FF_UPLOAD, &mut up, "UI_BEGIN_FF_UPLOAD").is_ok() { - let mut e = up.effect; - if e.id == -1 { - e.id = self.ff.next_effect_id; - self.ff.next_effect_id = self.ff.next_effect_id.wrapping_add(1); - } + let e = up.effect; + // No `id == -1` fallback: ff-core's `input_ff_upload` picks a free slot and + // writes it into the effect BEFORE handing the request to uinput, so what + // arrives here is always an assigned id. The fallback that used to allocate + // one from a local counter could therefore never run, and a local counter is + // the wrong answer anyway — the kernel owns that id space. + debug_assert!(e.id >= 0, "uinput handed us an unassigned FF effect id"); if e.type_ == FF_RUMBLE { let strong = u16::from_ne_bytes([e.u[0], e.u[1]]); let weak = u16::from_ne_bytes([e.u[2], e.u[3]]); diff --git a/crates/pf-inject/src/inject/linux/steam_controller.rs b/crates/pf-inject/src/inject/linux/steam_controller.rs index 73d6f1ec..12e716bb 100644 --- a/crates/pf-inject/src/inject/linux/steam_controller.rs +++ b/crates/pf-inject/src/inject/linux/steam_controller.rs @@ -23,6 +23,11 @@ use super::steam_proto::{ btn, parse_steam_output, sc_from_gamepad, serial_reply, serialize_deck_state, serialize_sc_state, SteamModel, SteamState, STEAMDECK_RDESC, STEAM_REPORT_LEN, STEAM_VENDOR, }; +use crate::uhid_abi::{ + put_cstr, request_id, set_report_data, BUS_USB, HID_MAX_DESCRIPTOR_SIZE, UHID_CREATE2, + UHID_DESTROY, UHID_EVENT_SIZE, UHID_GET_REPORT, UHID_GET_REPORT_REPLY, UHID_INPUT2, + UHID_OUTPUT, UHID_PATH, UHID_SET_REPORT, UHID_SET_REPORT_REPLY, +}; use crate::uhid_manager::{PadFeedback, PadProto, UhidManager}; use anyhow::{Context, Result}; use punktfunk_core::quic::RichInput; @@ -32,20 +37,6 @@ use std::os::unix::fs::OpenOptionsExt; use std::sync::atomic::{AtomicBool, Ordering}; use std::time::{Duration, Instant}; -// /dev/uhid event ABI — same layout as the DualSense backend. -const UHID_PATH: &str = "/dev/uhid"; -const UHID_DESTROY: u32 = 1; -const UHID_OUTPUT: u32 = 6; -const UHID_GET_REPORT: u32 = 9; -const UHID_GET_REPORT_REPLY: u32 = 10; -const UHID_CREATE2: u32 = 11; -const UHID_INPUT2: u32 = 12; -const UHID_SET_REPORT: u32 = 13; -const UHID_SET_REPORT_REPLY: u32 = 14; -const HID_MAX_DESCRIPTOR_SIZE: usize = 4096; -const UHID_EVENT_SIZE: usize = 4 + 4372; -const BUS_USB: u16 = 0x03; - /// Hold the `b9.6` mode-switch this long at creation to toggle `gamepad_mode` on (the kernel needs /// ~450 ms continuous; give margin). const MODE_ENTER: Duration = Duration::from_millis(650); @@ -53,11 +44,6 @@ const MODE_ENTER: Duration = Duration::from_millis(650); /// we insert a one-frame release so an in-game long-Start-hold can't toggle `gamepad_mode` off. const MENU_HOLD_CAP: Duration = Duration::from_millis(350); -fn put_cstr(ev: &mut [u8], off: usize, cap: usize, s: &str) { - let n = s.len().min(cap - 1); - ev[off..off + n].copy_from_slice(&s.as_bytes()[..n]); -} - /// Best-effort, once per process: clear `hid_steam`'s `lizard_mode` so `steam_do_deck_input_event` /// stops gating on `gamepad_mode` (gamepad events then always flow). Needs root; on failure the /// per-pad `b9.6` pulse + guard handle it instead. @@ -214,10 +200,13 @@ impl SteamDeckPad { let _ = self.reply_get_report(id, &serial_reply("PUNKTFUNK01")); } UHID_SET_REPORT => { - let id = u32::from_ne_bytes([ev[4], ev[5], ev[6], ev[7]]); - // SET_REPORT data: [report-id 0, cmd, …] at ev[12..]. Surface rumble, then ack. - let end = (12 + 16).min(UHID_EVENT_SIZE); - if let Some(r) = parse_steam_output(&ev[12..end]).rumble { + let id = request_id(&ev); + // SET_REPORT data: [report-id 0, cmd, …]. Take exactly the bytes the kernel + // declared — this used to read a fixed 16-byte window, which truncated any + // longer report and, for a shorter one, fed the parser whatever the reused + // event buffer still held past the payload. Every sibling backend that parses + // SET_REPORT already read the size field; this one didn't. + if let Some(r) = parse_steam_output(set_report_data(&ev)).rumble { rumble = Some(r); } let _ = self.reply_set_report(id); diff --git a/crates/pf-inject/src/inject/linux/steam_controller2.rs b/crates/pf-inject/src/inject/linux/steam_controller2.rs index 4dc9a18a..ad9c0f9a 100644 --- a/crates/pf-inject/src/inject/linux/steam_controller2.rs +++ b/crates/pf-inject/src/inject/linux/steam_controller2.rs @@ -23,6 +23,11 @@ use super::triton_proto::{ triton_serial, triton_unit_id, TritonState, TRITON_RDESC, TRITON_STATE_LEN, TRITON_VENDOR, TRITON_WIRED_PRODUCT, }; +use crate::uhid_abi::{ + put_cstr, BUS_USB, HID_MAX_DESCRIPTOR_SIZE, UHID_CREATE2, UHID_DESTROY, UHID_EVENT_SIZE, + UHID_GET_REPORT, UHID_GET_REPORT_REPLY, UHID_INPUT2, UHID_OUTPUT, UHID_PATH, UHID_SET_REPORT, + UHID_SET_REPORT_REPLY, +}; use crate::uhid_manager::{PadFeedback, PadProto, UhidManager}; use anyhow::{Context, Result}; use punktfunk_core::quic::{HidOutput, RichInput, HID_RAW_FEATURE, HID_RAW_OUTPUT}; @@ -30,25 +35,6 @@ use std::fs::{File, OpenOptions}; use std::io::{Read, Write}; use std::os::unix::fs::OpenOptionsExt; -// /dev/uhid event ABI — same layout as the Deck/DualSense backends. -const UHID_PATH: &str = "/dev/uhid"; -const UHID_DESTROY: u32 = 1; -const UHID_OUTPUT: u32 = 6; -const UHID_GET_REPORT: u32 = 9; -const UHID_GET_REPORT_REPLY: u32 = 10; -const UHID_CREATE2: u32 = 11; -const UHID_INPUT2: u32 = 12; -const UHID_SET_REPORT: u32 = 13; -const UHID_SET_REPORT_REPLY: u32 = 14; -const HID_MAX_DESCRIPTOR_SIZE: usize = 4096; -const UHID_EVENT_SIZE: usize = 4 + 4372; -const BUS_USB: u16 = 0x03; - -fn put_cstr(ev: &mut [u8], off: usize, cap: usize, s: &str) { - let n = s.len().min(cap - 1); - ev[off..off + n].copy_from_slice(&s.as_bytes()[..n]); -} - /// A virtual Steam Controller 2 backed by `/dev/uhid`. Dropping it destroys the device. pub struct TritonPad { fd: File, diff --git a/crates/pf-inject/src/inject/linux/switch_pro.rs b/crates/pf-inject/src/inject/linux/switch_pro.rs index c4f31f94..c6e5e104 100644 --- a/crates/pf-inject/src/inject/linux/switch_pro.rs +++ b/crates/pf-inject/src/inject/linux/switch_pro.rs @@ -22,6 +22,10 @@ use super::switch_proto::{ serialize_report_0x30, spi_flash_read, switch_mac, SwitchOutput, SwitchState, PROCON_RDESC, SWITCH_PRODUCT, SWITCH_REPORT_LEN, SWITCH_VENDOR, }; +use crate::uhid_abi::{ + put_cstr, BUS_USB, HID_MAX_DESCRIPTOR_SIZE, UHID_CREATE2, UHID_DESTROY, UHID_EVENT_SIZE, + UHID_GET_REPORT, UHID_GET_REPORT_REPLY, UHID_INPUT2, UHID_OUTPUT, UHID_PATH, +}; use crate::uhid_manager::{PadFeedback, PadProto, UhidManager}; use anyhow::{Context, Result}; use punktfunk_core::quic::{HidOutput, RichInput}; @@ -29,24 +33,6 @@ use std::fs::{File, OpenOptions}; use std::io::{Read, Write}; use std::os::unix::fs::OpenOptionsExt; -// /dev/uhid event ABI (linux/uhid.h) — identical to the DualSense backend's; see `super::dualsense`. -const UHID_PATH: &str = "/dev/uhid"; -const UHID_DESTROY: u32 = 1; -const UHID_OUTPUT: u32 = 6; -const UHID_GET_REPORT: u32 = 9; -const UHID_GET_REPORT_REPLY: u32 = 10; -const UHID_CREATE2: u32 = 11; -const UHID_INPUT2: u32 = 12; -const HID_MAX_DESCRIPTOR_SIZE: usize = 4096; -const UHID_EVENT_SIZE: usize = 4 + 4372; // type + union (create2) -const BUS_USB: u16 = 0x03; - -/// Copy a NUL-padded C string field into the event buffer. -fn put_cstr(ev: &mut [u8], off: usize, cap: usize, s: &str) { - let n = s.len().min(cap - 1); - ev[off..off + n].copy_from_slice(&s.as_bytes()[..n]); // rest already zero (NUL-terminated) -} - /// A virtual Pro Controller backed by `/dev/uhid`. Dropping it destroys the device (the kernel /// tears down the bound `hid-nintendo` interface). pub struct SwitchProPad { diff --git a/crates/pf-inject/src/inject/linux/uhid_abi.rs b/crates/pf-inject/src/inject/linux/uhid_abi.rs new file mode 100644 index 00000000..2dbd2c12 --- /dev/null +++ b/crates/pf-inject/src/inject/linux/uhid_abi.rs @@ -0,0 +1,143 @@ +//! The `/dev/uhid` event ABI (`linux/uhid.h`), in one place. +//! +//! Every UHID gamepad backend — DualSense, DualShock 4, Switch Pro, Steam Controller and Steam +//! Controller 2 — speaks the same kernel protocol, and each carried its own verbatim copy of these +//! constants plus its own `put_cstr`. Five copies of one kernel ABI is five chances to drift from +//! it, and they already had: `switch_pro` was missing the SET_REPORT pair entirely, and one backend +//! read a fixed-size SET_REPORT payload instead of the length the kernel gave it (see +//! [`set_report_data`]). +//! +//! `struct uhid_event` is `__packed__`: a `u32` `type` followed by a union whose largest member is +//! `uhid_create2_req` (name 128 + phys 64 + uniq 64 + rd_size 2 + bus 2 + 4×u32 + rd_data 4096 = +//! 4372 bytes). Nothing here allocates or parses a whole event — the backends still drive their own +//! read/write loops; this module owns the numbers and the two field accessors that are easy to get +//! subtly wrong. + +/// The character device every backend opens. +pub const UHID_PATH: &str = "/dev/uhid"; + +// Event types (`enum uhid_event_type`). Only the ones the backends actually use. +pub const UHID_DESTROY: u32 = 1; +pub const UHID_OUTPUT: u32 = 6; +pub const UHID_GET_REPORT: u32 = 9; +pub const UHID_GET_REPORT_REPLY: u32 = 10; +pub const UHID_CREATE2: u32 = 11; +pub const UHID_INPUT2: u32 = 12; +pub const UHID_SET_REPORT: u32 = 13; +pub const UHID_SET_REPORT_REPLY: u32 = 14; + +/// `HID_MAX_DESCRIPTOR_SIZE` — also the cap on a report payload we will copy out of an event. +pub const HID_MAX_DESCRIPTOR_SIZE: usize = 4096; +/// `size_of::()`: the `u32` type tag plus the create2 union. +pub const UHID_EVENT_SIZE: usize = 4 + 4372; +/// `BUS_USB` from `linux/input.h`. +pub const BUS_USB: u16 = 0x03; + +/// Offset of the `id` field shared by the GET_REPORT / SET_REPORT request and reply structs. +const OFF_ID: usize = 4; +/// Offset of `uhid_set_report_req::size` (after `id: u32`, `rnum: u8`, `rtype: u8`). +const OFF_SET_REPORT_SIZE: usize = 10; +/// Offset of the payload in a SET_REPORT request — and of `data` in the reply structs. +const OFF_DATA: usize = 12; +/// Offset of `uhid_output_req::size` (the payload follows `data[4096]`). +const OFF_OUTPUT_SIZE: usize = 4 + HID_MAX_DESCRIPTOR_SIZE; + +/// Copy a NUL-padded C string field into the event buffer. The buffer is zeroed by the caller, so +/// truncation still leaves a NUL terminator. +pub fn put_cstr(ev: &mut [u8], off: usize, cap: usize, s: &str) { + let n = s.len().min(cap - 1); + ev[off..off + n].copy_from_slice(&s.as_bytes()[..n]); // rest already zero (NUL-terminated) +} + +/// The request id of a GET_REPORT / SET_REPORT event — what the matching reply must echo. +pub fn request_id(ev: &[u8]) -> u32 { + u32::from_ne_bytes([ev[OFF_ID], ev[OFF_ID + 1], ev[OFF_ID + 2], ev[OFF_ID + 3]]) +} + +/// The payload of a `UHID_SET_REPORT` event: exactly the bytes the kernel says are there. +/// +/// Read the length from the event's own `size` field. Assuming a fixed window instead is wrong in +/// both directions — a longer report is silently truncated, and a shorter one is parsed together +/// with whatever stale bytes the reused event buffer still holds past its end, which for a rumble +/// report means acting on numbers the game never wrote. +pub fn set_report_data(ev: &[u8]) -> &[u8] { + let size = u16::from_ne_bytes([ev[OFF_SET_REPORT_SIZE], ev[OFF_SET_REPORT_SIZE + 1]]) as usize; + let end = (OFF_DATA + size.min(HID_MAX_DESCRIPTOR_SIZE)).min(ev.len()); + &ev[OFF_DATA.min(end)..end] +} + +/// The payload of a `UHID_OUTPUT` event (`uhid_output_req`: `data[4096]` then `size`). +pub fn output_data(ev: &[u8]) -> &[u8] { + let size = u16::from_ne_bytes([ev[OFF_OUTPUT_SIZE], ev[OFF_OUTPUT_SIZE + 1]]) as usize; + let end = (4 + size.min(HID_MAX_DESCRIPTOR_SIZE)).min(ev.len()); + &ev[4.min(end)..end] +} + +#[cfg(test)] +mod tests { + use super::*; + + fn blank() -> Vec { + vec![0u8; UHID_EVENT_SIZE] + } + + #[test] + fn set_report_data_honours_the_events_own_size() { + let mut ev = blank(); + ev[OFF_SET_REPORT_SIZE..OFF_SET_REPORT_SIZE + 2].copy_from_slice(&5u16.to_ne_bytes()); + for (i, b) in [1u8, 2, 3, 4, 5].iter().enumerate() { + ev[OFF_DATA + i] = *b; + } + // Stale bytes past the payload — a fixed-window read would hand these to the parser. + ev[OFF_DATA + 5] = 0xAA; + ev[OFF_DATA + 15] = 0xBB; + assert_eq!(set_report_data(&ev), &[1, 2, 3, 4, 5]); + } + + #[test] + fn set_report_data_is_not_truncated_at_sixteen() { + let mut ev = blank(); + let n = 40usize; + ev[OFF_SET_REPORT_SIZE..OFF_SET_REPORT_SIZE + 2].copy_from_slice(&(n as u16).to_ne_bytes()); + for i in 0..n { + ev[OFF_DATA + i] = i as u8; + } + let d = set_report_data(&ev); + assert_eq!( + d.len(), + n, + "a report longer than 16 bytes must survive whole" + ); + assert_eq!(d[39], 39); + } + + #[test] + fn oversized_and_empty_sizes_stay_in_bounds() { + let mut ev = blank(); + ev[OFF_SET_REPORT_SIZE..OFF_SET_REPORT_SIZE + 2].copy_from_slice(&u16::MAX.to_ne_bytes()); + assert!(set_report_data(&ev).len() <= HID_MAX_DESCRIPTOR_SIZE); + assert!(OFF_DATA + set_report_data(&ev).len() <= UHID_EVENT_SIZE); + + let ev0 = blank(); // size = 0 + assert!(set_report_data(&ev0).is_empty()); + assert!(output_data(&ev0).is_empty()); + } + + #[test] + fn output_data_reads_its_trailing_size_field() { + let mut ev = blank(); + ev[OFF_OUTPUT_SIZE..OFF_OUTPUT_SIZE + 2].copy_from_slice(&3u16.to_ne_bytes()); + ev[4] = 0x02; + ev[5] = 0x11; + ev[6] = 0x22; + ev[7] = 0x33; // past the declared size + assert_eq!(output_data(&ev), &[0x02, 0x11, 0x22]); + } + + #[test] + fn request_id_round_trips() { + let mut ev = blank(); + ev[OFF_ID..OFF_ID + 4].copy_from_slice(&0xDEAD_BEEFu32.to_ne_bytes()); + assert_eq!(request_id(&ev), 0xDEAD_BEEF); + } +} diff --git a/crates/pf-inject/src/inject/proto/dualsense_proto.rs b/crates/pf-inject/src/inject/proto/dualsense_proto.rs index 32852914..46582338 100644 --- a/crates/pf-inject/src/inject/proto/dualsense_proto.rs +++ b/crates/pf-inject/src/inject/proto/dualsense_proto.rs @@ -471,7 +471,14 @@ fn pack_touch(dst: &mut [u8], t: &Touch) { #[derive(Default)] pub struct DsFeedback { pub hidout: Vec, - /// `(low, high)` motor levels (0..=0xFFFF), if a report carried them. + /// `(low, high)` motor levels, if a report carried them. + /// + /// This parser widens the device's 8-bit motor bytes by `<< 8`, so the values it produces are + /// `0..=0xFF00` in steps of 0x100 — NOT `0..=0xFFFF`, which is what this said before. The + /// Windows backend widens the same bytes by `× 257` and does reach 0xFFFF. Both are correct: + /// every consumer narrows with `>> 8`, and 0xFF00 and 0xFFFF both narrow back to 255. Do not + /// "fix" one to match the other — see [`crate::uhid_manager::PadFeedback::rumble`], which is + /// the type that sees both. pub rumble: Option<(u16, u16)>, /// The driver's output-report ring overflowed this poll — pending reports were DISCARDED and /// feedback state is unknown; the [`UhidManager`](crate::uhid_manager) must resync (silence + @@ -479,64 +486,101 @@ pub struct DsFeedback { pub resync: bool, } -/// Parse a DualSense USB output report (`0x02`) into a [`DsFeedback`]. The byte layout below is -/// the USB DualSense common report; only the well-understood fields (motor rumble, lightbar RGB, -/// player LEDs) are surfaced — adaptive-trigger blocks are forwarded raw for the client. +/// Field offsets in the DualSense **output** report, as indices into a whole USB report — i.e. +/// including the leading report id at `[0]`. This is the one place in Rust the layout is written +/// down; index off these rather than repeating the numbers. +/// +/// **The same fields sit at different offsets per transport, and that is not drift.** Every writer +/// lays out one common block; what changes is how much header precedes it: +/// +/// | base | where | first payload byte | +/// |---|---|---| +/// | `0` | USB report, id included — what these constants describe, and what this parser reads | `[1]` | +/// | `−1` | SDL `DS5EffectsState_t` — a 47-byte payload with NO report id (`pf-client-core`'s `Ds5Feedback`) | `[0]` | +/// | `+2` | Bluetooth report `0x31` — id, sequence, magic, then the block; CRC32 in the last 4 bytes | `[3]` | +/// +/// Subtract or add the base to translate. Mirrors that cannot import this module — Kotlin +/// (`DsDevice.kt`, USB base 0) and Swift (`DualSenseHID.swift`, which handles both the USB and +/// Bluetooth bases) — carry a pointer back here; keep them in step by hand. +pub mod out_report { + /// `valid_flag0`: BIT0 compat vibration, BIT1 haptics select, BIT2 R2, BIT3 L2. + pub const VALID_FLAG0: usize = 1; + /// `valid_flag1`: BIT2 lightbar, BIT4 player indicators. + pub const VALID_FLAG1: usize = 2; + /// High-frequency (small / right) motor. + pub const MOTOR_RIGHT: usize = 3; + /// Low-frequency (big / left) motor. + pub const MOTOR_LEFT: usize = 4; + /// First byte of the RIGHT trigger's parameter block — it precedes the left one in the report. + pub const RIGHT_TRIGGER: usize = 11; + /// First byte of the LEFT trigger's parameter block. + pub const LEFT_TRIGGER: usize = 22; + /// One adaptive-trigger parameter block: a mode byte plus 10 parameters. + pub const TRIGGER_LEN: usize = 11; + /// `valid_flag2`: BIT2 = `COMPATIBLE_VIBRATION2` (the firmware ≥ 2.24 rumble signal). + pub const VALID_FLAG2: usize = 39; + /// Lit player-indicator bits (low 5). + pub const PLAYER_LEDS: usize = 44; + /// Lightbar red; green and blue follow. + pub const LED_RGB: usize = 45; +} + +/// Parse a DualSense USB output report (`0x02`) into a [`DsFeedback`], indexed off +/// [`out_report`]. Only the well-understood fields (motor rumble, lightbar RGB, player LEDs) are +/// surfaced — adaptive-trigger blocks are forwarded raw for the client. /// /// Every field is gated on the report's valid-flags (`valid_flag0` at data[1], `valid_flag1` /// at data[2]) — writers only set the bits for fields they mean to change (the rest is zeroed), /// so an ungated parse would turn every plain rumble write into a lightbar-off + triggers-off /// broadcast. pub fn parse_ds_output(pad: u8, data: &[u8], fb: &mut DsFeedback) { + use out_report as o; // data[0] is the report id (0x02). Be defensive about short reports. if data.first() != Some(&0x02) || data.len() < 48 { return; } - let flag0 = data[1]; // BIT0 compat vibration, BIT1 haptics select, BIT2 R2, BIT3 L2 - let flag1 = data[2]; // BIT2 lightbar, BIT4 player indicators - // Motor rumble: high-frequency (small/right) motor at data[3], low-frequency (big/left) at - // data[4]. Scale 0..255 → 0..0xFFFF, same (low, high) convention as the uinput pad's mixer, - // and route to the universal rumble plane (0xCA). - // Writers on firmware ≥ 2.24 signal rumble via COMPATIBLE_VIBRATION2 in valid_flag2 - // (data[39] BIT2) instead of flag0 BIT0. Our feature report advertises a version - // above 2.24 (DS_FEATURE_FIRMWARE bytes 44..46, chosen to keep Sony's updater - // quiet), so the kernel and SDL write the v2 flag — while older writers, and any - // that never read the version, stay on flag0. Both conventions must land here: a - // rumble dropped on either — including stops — is silently ignored, and a missed - // stop buzzes for the rest of the session (the 500 ms refresh re-sends stale state - // forever). - if flag0 & 0x03 != 0 || data[39] & 0x04 != 0 { - let high = (data[3] as u16) << 8; - let low = (data[4] as u16) << 8; + let flag0 = data[o::VALID_FLAG0]; // BIT0 compat vibration, BIT1 haptics select, BIT2 R2, BIT3 L2 + let flag1 = data[o::VALID_FLAG1]; // BIT2 lightbar, BIT4 player indicators + // Motor rumble: high-frequency (small/right) motor first, low-frequency (big/left) second. + // Widened 0..255 → 0..0xFF00 by `<< 8` (NOT 0xFFFF — see `DsFeedback::rumble`), same + // (low, high) convention as the uinput pad's mixer, and routed to the 0xCA plane. + // Writers on firmware ≥ 2.24 signal rumble via COMPATIBLE_VIBRATION2 in valid_flag2 + // instead of flag0 BIT0. Our feature report advertises a version above 2.24 + // (DS_FEATURE_FIRMWARE bytes 44..46, chosen to keep Sony's updater quiet), so the + // kernel and SDL write the v2 flag — while older writers, and any that never read the + // version, stay on flag0. Both conventions must land here: a rumble dropped on either + // — including stops — is silently ignored, and a missed stop buzzes for the rest of + // the session (the 500 ms refresh re-sends stale state forever). + if flag0 & 0x03 != 0 || data[o::VALID_FLAG2] & 0x04 != 0 { + let high = (data[o::MOTOR_RIGHT] as u16) << 8; + let low = (data[o::MOTOR_LEFT] as u16) << 8; fb.rumble = Some((low, high)); } - // Lightbar RGB (USB common report: bytes 45..48). Player LEDs at byte 44. if flag1 & 0x04 != 0 { - let (r, g, b) = (data[45], data[46], data[47]); + let (r, g, b) = (data[o::LED_RGB], data[o::LED_RGB + 1], data[o::LED_RGB + 2]); fb.hidout.push(HidOutput::Led { pad, r, g, b }); } if flag1 & 0x10 != 0 { fb.hidout.push(HidOutput::PlayerLeds { pad, - bits: data[44] & 0x1F, + bits: data[o::PLAYER_LEDS] & 0x1F, }); } - // Adaptive-trigger parameter blocks, 11 bytes each: the RIGHT trigger comes FIRST in the - // report (bytes 11..22), the left at 22..33 — per SDL's DS5EffectsState_t / inputtino's - // ps5.hpp. Wire convention: which 0 = L2, 1 = R2. - if data.len() >= 33 { + // The RIGHT trigger block comes FIRST in the report — per SDL's DS5EffectsState_t / + // inputtino's ps5.hpp. Wire convention: which 0 = L2, 1 = R2. + if data.len() >= o::LEFT_TRIGGER + o::TRIGGER_LEN { if flag0 & 0x04 != 0 { fb.hidout.push(HidOutput::Trigger { pad, which: 1, - effect: data[11..22].to_vec(), + effect: data[o::RIGHT_TRIGGER..o::RIGHT_TRIGGER + o::TRIGGER_LEN].to_vec(), }); } if flag0 & 0x08 != 0 { fb.hidout.push(HidOutput::Trigger { pad, which: 0, - effect: data[22..33].to_vec(), + effect: data[o::LEFT_TRIGGER..o::LEFT_TRIGGER + o::TRIGGER_LEN].to_vec(), }); } } diff --git a/crates/pf-inject/src/inject/uhid_manager.rs b/crates/pf-inject/src/inject/uhid_manager.rs index 8f91f0a9..c537fdd8 100644 --- a/crates/pf-inject/src/inject/uhid_manager.rs +++ b/crates/pf-inject/src/inject/uhid_manager.rs @@ -18,7 +18,12 @@ use std::time::{Duration, Instant}; /// 0xCD feedback events (lightbar / player LEDs / adaptive triggers), deduped via [`HidoutDedup`]. #[derive(Default)] pub struct PadFeedback { - /// `(low, high)` motor levels (0..=0xFF00), if the pass saw a rumble report. + /// `(low, high)` motor levels, if the pass saw a rumble report. + /// + /// Range is `0..=0xFFFF` — this said `0..=0xFF00`, which is only true of the backends that + /// widen the device's 8-bit motor byte by `<< 8` (the UHID/DualSense path). The Windows + /// backend widens by `× 257` and does reach 0xFFFF, and this type carries both. Neither is a + /// defect: consumers narrow with `>> 8`, and 0xFF00 and 0xFFFF both narrow back to 255. pub rumble: Option<(u16, u16)>, pub hidout: Vec, /// Whether the game drove this pad's RUMBLE plane this poll — at least one output report diff --git a/crates/pf-inject/src/lib.rs b/crates/pf-inject/src/lib.rs index 3dde4586..ed946eb1 100644 --- a/crates/pf-inject/src/lib.rs +++ b/crates/pf-inject/src/lib.rs @@ -457,6 +457,11 @@ pub mod triton_proto; #[cfg(target_os = "linux")] #[path = "inject/linux/triton_usbip.rs"] pub mod triton_usbip; +/// Linux: the `/dev/uhid` event ABI shared by every UHID gamepad backend — the constants each +/// used to transcribe for itself, plus the field accessors that read a payload's real length. +#[cfg(target_os = "linux")] +#[path = "inject/linux/uhid_abi.rs"] +pub mod uhid_abi; /// The generic stateful virtual-pad manager ([`uhid_manager::UhidManager`]) — event routing, frame /// merge, heartbeat, and feedback pump shared by the five UHID/UMDF backends; each supplies only /// its per-controller protocol via [`uhid_manager::PadProto`] (G12). diff --git a/crates/punktfunk-core/src/abi.rs b/crates/punktfunk-core/src/abi.rs index b2db9524..5a6653b7 100644 --- a/crates/punktfunk-core/src/abi.rs +++ b/crates/punktfunk-core/src/abi.rs @@ -698,7 +698,10 @@ pub struct PunktfunkHidOutput { /// Trigger: number of valid bytes in `effect` (≤ `PUNKTFUNK_HID_EFFECT_MAX`). pub effect_len: u8, /// Trigger: the raw DualSense trigger parameter block (mode + params). - pub effect: [u8; 11], + /// Sized off [`PUNKTFUNK_HID_EFFECT_MAX`] rather than a second literal `11` — the constant is + /// exported precisely so embedders can size their own buffers against it, and it declaring one + /// number while the struct it describes hardcoded another was the whole hazard. + pub effect: [u8; PUNKTFUNK_HID_EFFECT_MAX as usize], } #[cfg(feature = "quic")] @@ -2497,10 +2500,12 @@ pub unsafe extern "C" fn punktfunk_connection_next_rumble_cmd( /// Declare a physical actuator's quirks for wire pad `pad` — how a platform parameterizes the /// shared rumble policy engine instead of forking it (typically called at controller attach). /// `keepalive_ms`: re-emit an unchanged non-zero level at this cadence for actuators whose -/// hardware output decays between wire renewals (Steam Deck ≈ 40, DualSense-over-BT raw HID -/// ≈ 900); `0` = none. `min_pulse_ms`: floor for `backstop_ms` on non-zero commands. `flags`: +/// hardware output decays between wire renewals (the Steam Deck's ≈ 40 is the one in-tree user); +/// `0` = none. `min_pulse_ms`: floor for `backstop_ms` on non-zero commands — no in-tree caller +/// sets it, it exists for embedders whose duration-taking API rejects short values. `flags`: /// [`PUNKTFUNK_RUMBLE_QUIRK_DEDUP_JITTER`]. All-zero (the initial state) describes a well-behaved -/// actuator. +/// actuator. See [`ActuatorQuirks`](crate::client::rumble::ActuatorQuirks) for why a renderer that +/// dedupes its own writes (the Apple HID path) cannot use `keepalive_ms` and keeps its own. /// /// # Safety /// `c` is a valid connection handle. Callable from any thread. diff --git a/crates/punktfunk-core/src/client/rumble.rs b/crates/punktfunk-core/src/client/rumble.rs index e3f024d9..452d40a8 100644 --- a/crates/punktfunk-core/src/client/rumble.rs +++ b/crates/punktfunk-core/src/client/rumble.rs @@ -53,10 +53,25 @@ pub struct RumbleCommand { #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] pub struct ActuatorQuirks { /// Re-emit an unchanged non-zero level every this many ms — for actuators whose hardware - /// output decays between wire renewals (Steam Deck ≈ 40, macOS DualSense-over-HID BT ≈ 900). - /// `0` = no keepalive (the common case). + /// output decays between wire renewals. `0` = no keepalive (the common case). + /// + /// The one in-tree producer is the Steam Deck's ≈ 40 ms (`pf-client-core`'s slot open, paired + /// with `dedup_jitter`). The macOS DualSense-over-HID Bluetooth decay is NOT served by this + /// quirk, though it reads like the obvious second example: the Apple client keeps its own + /// ≈ 900 ms keepalive down in `RumbleRenderer` (`RumbleTuning.hidKeepaliveSeconds`) because + /// the re-emit has to happen BELOW the command layer. An engine keepalive arrives as a + /// command carrying the same levels, and that renderer skips a HID write whose levels are + /// unchanged — so the re-emit would be swallowed by the very dedupe it exists to defeat + /// (`dedup_jitter` is the Deck's answer to the same problem one layer up). pub keepalive_ms: u16, - /// Floor for `backstop_ms` on non-zero commands (Android's `createOneShot` throws on 0). + /// Floor for `backstop_ms` on non-zero commands. + /// + /// **No in-tree producer sets this non-zero** — it is reachable only through the C ABI + /// (`punktfunk_connection_set_rumble_quirks`), for embedders whose duration-taking API + /// rejects short values. The case it was written for is handled elsewhere: Android's + /// `createOneShot` does throw on a non-positive duration, but the Kotlin renderer floors the + /// duration itself at the call, and that path never declares quirks at all. Kept because it + /// is exported ABI, and because a floor belongs here rather than re-invented per embedder. pub min_pulse_ms: u16, /// Alternate the low motor's LSB on keepalive re-emits (imperceptible) so an SDL-class layer /// that no-ops identical values still writes the device — the Deck's dedupe-defeat. diff --git a/crates/punktfunk-core/src/quic/datagram.rs b/crates/punktfunk-core/src/quic/datagram.rs index 0567f241..42368596 100644 --- a/crates/punktfunk-core/src/quic/datagram.rs +++ b/crates/punktfunk-core/src/quic/datagram.rs @@ -431,6 +431,14 @@ pub enum HidOutput { /// A trackpad haptic pulse for a Steam Controller's voice-coil actuators (its only "rumble"). /// `side` 0 = right pad, 1 = left pad; `amplitude` + `period` (µs off-time) + `count` (pulses) /// synthesize a buzz. A client without trackpad coils drops it (or maps it to ordinary rumble). + /// + /// **STAGED SCAFFOLDING — deliberately unreachable today, do not delete.** Nothing on the host + /// produces this variant and no client renders it; it codes/decodes and round-trips in tests + /// and nothing else. It stays because `HIDOUT_TRACKPAD_HAPTIC` is an allocated tag on a + /// SHIPPED wire: removing the variant would not reclaim the tag (a future peer could still + /// send it), it would only lose the decoder that keeps such a datagram from being mistaken + /// for something else. The producer is the Steam Controller coil path; the renderer is the + /// client-side coil write. Wire up either half and this becomes live with no format change. TrackpadHaptic { pad: u8, side: u8, diff --git a/include/punktfunk_core.h b/include/punktfunk_core.h index 62d53b32..59134bce 100644 --- a/include/punktfunk_core.h +++ b/include/punktfunk_core.h @@ -1649,7 +1649,10 @@ typedef struct { // Trigger: number of valid bytes in `effect` (≤ `PUNKTFUNK_HID_EFFECT_MAX`). uint8_t effect_len; // Trigger: the raw DualSense trigger parameter block (mode + params). - uint8_t effect[11]; + // Sized off [`PUNKTFUNK_HID_EFFECT_MAX`] rather than a second literal `11` — the constant is + // exported precisely so embedders can size their own buffers against it, and it declaring one + // number while the struct it describes hardcoded another was the whole hazard. + uint8_t effect[PUNKTFUNK_HID_EFFECT_MAX]; } PunktfunkHidOutput; #endif @@ -2445,10 +2448,12 @@ PunktfunkStatus punktfunk_connection_next_rumble_cmd(PunktfunkConnection *c, // Declare a physical actuator's quirks for wire pad `pad` — how a platform parameterizes the // shared rumble policy engine instead of forking it (typically called at controller attach). // `keepalive_ms`: re-emit an unchanged non-zero level at this cadence for actuators whose -// hardware output decays between wire renewals (Steam Deck ≈ 40, DualSense-over-BT raw HID -// ≈ 900); `0` = none. `min_pulse_ms`: floor for `backstop_ms` on non-zero commands. `flags`: +// hardware output decays between wire renewals (the Steam Deck's ≈ 40 is the one in-tree user); +// `0` = none. `min_pulse_ms`: floor for `backstop_ms` on non-zero commands — no in-tree caller +// sets it, it exists for embedders whose duration-taking API rejects short values. `flags`: // [`PUNKTFUNK_RUMBLE_QUIRK_DEDUP_JITTER`]. All-zero (the initial state) describes a well-behaved -// actuator. +// actuator. See [`ActuatorQuirks`](crate::client::rumble::ActuatorQuirks) for why a renderer that +// dedupes its own writes (the Apple HID path) cannot use `keepalive_ms` and keeps its own. // // # Safety // `c` is a valid connection handle. Callable from any thread.