diff --git a/clients/apple/Sources/PunktfunkKit/Connection/PunktfunkConnection.swift b/clients/apple/Sources/PunktfunkKit/Connection/PunktfunkConnection.swift index 94e68868..e209cbae 100644 --- a/clients/apple/Sources/PunktfunkKit/Connection/PunktfunkConnection.swift +++ b/clients/apple/Sources/PunktfunkKit/Connection/PunktfunkConnection.swift @@ -1002,22 +1002,34 @@ public final class PunktfunkConnection { /// Pull the next EFFECTIVE rumble command from the core's shared rumble policy engine — the /// uniform replacement for per-platform rumble policy. The engine owns every decision /// (v2 lease expiry, legacy-host staleness at a uniform 1 s, connection-close drain zeros), - /// so apply commands verbatim: `(0, 0)` = stop now, non-zero = run at this level. + /// so apply commands verbatim: all-zero = stop now, non-zero = run at this level. /// `backstopMs` is a safety-net duration for duration-parameterized platform APIs — the /// CoreHaptics renderer ignores it (its finite segment ceiling is the equivalent net). /// Drain from the (single) feedback thread, alongside `nextHidOutput`. + /// + /// A command carries FOUR motor levels: the two handles plus the two Xbox impulse-trigger + /// motors (`leftTrigger`/`rightTrigger`, same 0...0xFFFF scale), which arrive on the 0xCA + /// plane's v3 tail. This calls the core's `_cmd2` entry point — `_cmd` is the frozen + /// two-handle form kept for out-of-tree embedders, and there is no reason for this client to + /// stay on it: a pad that reports no `GCHapticsLocality.leftTrigger`/`.rightTrigger` simply + /// has no engine for those levels and they go nowhere, which is the normal case. public func nextRumbleCommand(timeoutMs: UInt32 = 0) throws - -> (pad: UInt16, low: UInt16, high: UInt16, backstopMs: UInt32)? + -> ( + pad: UInt16, low: UInt16, high: UInt16, leftTrigger: UInt16, rightTrigger: UInt16, + backstopMs: UInt32 + )? { feedbackLock.lock() defer { feedbackLock.unlock() } guard let h = liveHandle() else { throw PunktfunkClientError.closed } var pad: UInt16 = 0, low: UInt16 = 0, high: UInt16 = 0, backstop: UInt32 = 0 - let rc = punktfunk_connection_next_rumble_cmd(h, &pad, &low, &high, &backstop, timeoutMs) + var lt: UInt16 = 0, rt: UInt16 = 0 + let rc = punktfunk_connection_next_rumble_cmd2( + h, &pad, &low, &high, <, &rt, &backstop, timeoutMs) switch rc { case statusOK: - return (pad, low, high, backstop) + return (pad, low, high, lt, rt, backstop) case statusNoFrame: return nil case statusClosed: diff --git a/clients/apple/Sources/PunktfunkKit/Gamepad/GamepadFeedback.swift b/clients/apple/Sources/PunktfunkKit/Gamepad/GamepadFeedback.swift index 83bdc73e..e36c279a 100644 --- a/clients/apple/Sources/PunktfunkKit/Gamepad/GamepadFeedback.swift +++ b/clients/apple/Sources/PunktfunkKit/Gamepad/GamepadFeedback.swift @@ -172,7 +172,8 @@ public final class GamepadFeedback { while rumbleBurst < 64, !flag.isStopped, let c = try connection.nextRumbleCommand(timeoutMs: 0) { self?.routeRumble( - pad: UInt8(truncatingIfNeeded: c.pad), low: c.low, high: c.high) + pad: UInt8(truncatingIfNeeded: c.pad), low: c.low, high: c.high, + leftTrigger: c.leftTrigger, rightTrigger: c.rightTrigger) rumbleBurst += 1 } // Drain a BOUNDED burst of hidout events so sustained 0xCD traffic (a game writing @@ -225,12 +226,21 @@ public final class GamepadFeedback { /// Route one engine command to its pad's renderer (drain thread). A command for a pad with no /// live renderer — one that just left the forwarded set — is dropped. - private func routeRumble(pad: UInt8, low: UInt16, high: UInt16) { + private func routeRumble( + pad: UInt8, low: UInt16, high: UInt16, leftTrigger: UInt16, rightTrigger: UInt16 + ) { let renderer = withRouting { rumbleByPad[pad] } - renderer?.apply(low: low, high: high) + renderer?.apply(low: low, high: high, leftTrigger: leftTrigger, rightTrigger: rightTrigger) // The opt-in device mirror follows controller 1 unconditionally — the pads it exists for // have no motors (their renderer above no-ops), and mirroring deliberately isn't gated on // that: capability probing can't see a motor-less MFi pad, and the user opted in. + // + // HANDLES ONLY, deliberately. A phone body is one actuator with no trigger analogue, so + // the trigger levels would have to be folded to arrive at all — and folding continuous + // impulse-trigger content (a racing title's engine RPM / tyre slip) onto the one motor + // this mirror has would buzz the phone flat-out for the whole race at a level the game + // never requested. Dropping them matches the core engine's policy for every pad without + // trigger motors. if pad == 0 { deviceRumble?.apply(low: low, high: high) } } diff --git a/clients/apple/Sources/PunktfunkKit/Gamepad/RumbleRenderer.swift b/clients/apple/Sources/PunktfunkKit/Gamepad/RumbleRenderer.swift index 8dd45af7..1e8386d1 100644 --- a/clients/apple/Sources/PunktfunkKit/Gamepad/RumbleRenderer.swift +++ b/clients/apple/Sources/PunktfunkKit/Gamepad/RumbleRenderer.swift @@ -36,7 +36,9 @@ enum RumbleTuning { /// classic Xbox ERM rotor ignores it. On split-handle pads the wire's two motors render at /// distinct frequencies mirroring the real hardware they emulate — low/left ≈ the heavy /// low-frequency rotor, high/right ≈ the light buzzer; a single combined actuator keeps the - /// proven mid value. + /// proven mid value. The impulse-trigger motors are small and light — the same character as + /// the high/right buzzer — so they reuse `sharpnessHigh` rather than introduce a number + /// nobody has measured on real trigger hardware. static let sharpnessLow: Float = 0.3 static let sharpnessHigh: Float = 0.7 static let sharpnessCombined: Float = 0.5 @@ -140,9 +142,21 @@ final class RumbleRenderer: @unchecked Sendable { private var controller: GCController? private var low: Motor? private var high: Motor? - /// Wire-truth target (raw wire units) — the engine command's level, applied verbatim; the - /// core policy engine owns when it ends (explicit zero commands), so no deadline lives here. - private var target: (low: UInt16, high: UInt16) = (0, 0) + /// The two Xbox impulse-trigger motors, when the pad offers + /// `GCHapticsLocality.leftTrigger`/`.rightTrigger`. **Nil is the normal case** — every pad but + /// an Xbox One/Series/Elite has no such actuator, and the tree has already observed Xbox pads + /// on Apple exposing no haptics engine at all — so their absence is never logged and never + /// counts as a setup failure. Independent of the handle split: a pad may offer trigger + /// localities with or without split handles, and losing one does not implicate the other. + private var leftTrigger: Motor? + private var rightTrigger: Motor? + /// Wire-truth target (raw wire units) — the engine command's four levels, applied verbatim; + /// the core policy engine owns when it ends (explicit zero commands), so no deadline lives + /// here. The trigger levels are only ever non-zero against a Windows HID Xbox host pad; every + /// other backend on every OS lacks the channel entirely (XInput's `XINPUT_VIBRATION` and + /// evdev's `FF_RUMBLE` each carry exactly two magnitudes). + private var target: (low: UInt16, high: UInt16, leftTrigger: UInt16, rightTrigger: UInt16) = + (0, 0, 0, 0) /// Runs while anything is (or should be) audible: staleness watchdog, segment re-arm, /// throttled-level catch-up, engine rebuild after a reset, HID keepalive. Nil while silent, /// so an idle controller costs no timer wakeups and no radio traffic. @@ -216,22 +230,28 @@ final class RumbleRenderer: @unchecked Sendable { } } - /// Set the wire-truth target. Called with every 0xCA state the host sends — level changes AND - /// renewals (v2) / 500 ms refreshes (legacy); both stamp liveness and, for v2, refresh the - /// self-termination deadline. `ttlMs` is the envelope lease in ms, or [`RumbleTuning.noTTL`] - /// against a legacy host (no lease → the staleness watchdog is the backstop). Renewals at an - /// unchanged level extend the deadline before the idempotence guard, so a held rumble never - /// lapses mid-effect. - func apply(low lowAmp: UInt16, high highAmp: UInt16) { + /// Set the wire-truth target: one policy-engine command's four motor levels, applied verbatim. + /// Called with every 0xCA state the host sends — level changes AND renewals — and the core + /// engine owns when a level ends (it emits explicit zero commands), so nothing here decides. + /// + /// `leftTrigger`/`rightTrigger` are the Xbox impulse-trigger motors. They default to zero so + /// handle-only callers (the debug test panel, the tuning tests) read unchanged, which is also + /// the wire's own rule: on a level-triggered plane an absent level is off, never "keep what + /// you had". + func apply( + low lowAmp: UInt16, high highAmp: UInt16, leftTrigger ltAmp: UInt16 = 0, + rightTrigger rtAmp: UInt16 = 0 + ) { queue.async { - let active = lowAmp != 0 || highAmp != 0 + let next = (lowAmp, highAmp, ltAmp, rtAmp) + let active = next != (0, 0, 0, 0) if active != self.wasActive { self.wasActive = active log.debug( - "rumble: \(active ? "active" : "stop", privacy: .public) low=\(lowAmp, privacy: .public) high=\(highAmp, privacy: .public)") + "rumble: \(active ? "active" : "stop", privacy: .public) low=\(lowAmp, privacy: .public) high=\(highAmp, privacy: .public) lt=\(ltAmp, privacy: .public) rt=\(rtAmp, privacy: .public)") } - guard (lowAmp, highAmp) != self.target else { return } - self.target = (lowAmp, highAmp) + guard next != self.target else { return } + self.target = next self.render() } } @@ -241,7 +261,7 @@ final class RumbleRenderer: @unchecked Sendable { queue.sync { self.ticker?.cancel() self.ticker = nil - self.target = (0, 0) + self.target = (0, 0, 0, 0) self.wasActive = false self.teardown() self.closeHID() @@ -256,7 +276,7 @@ final class RumbleRenderer: @unchecked Sendable { defer { updateTicker() } if renderHID() { return } guard !broken else { return } - let audible = target.low != 0 || target.high != 0 + let audible = target != (0, 0, 0, 0) if audible, low == nil, high == nil, DispatchTime.now() >= retryAfter { setup() } @@ -274,6 +294,18 @@ final class RumbleRenderer: @unchecked Sendable { let mixed = RumbleTuning.combined(low: target.low, high: target.high) ok = reconcile(&low, to: RumbleTuning.amplitude(mixed)) } + // Impulse triggers: rendered ONLY where the hardware has the actuators, never folded into + // the handles. `reconcile` on a nil slot is a no-op returning true, so a pad without them + // silently drops the levels — which is the correct degrade and the common case. + // + // Their outcome is deliberately kept OUT of `ok`: a trigger engine erroring must not tear + // down the handle engines (which are what the pad's rumble mostly is) nor flip + // `preferCombined`, which is a statement about the handle split and nothing else. Nothing + // is orphaned by that — a failed reconcile leaves the slot's Motor in place, so the next + // tick simply retries it, and an engine that is genuinely dead fires its + // stopped/reset handler, which tears down all four slots for a lazy rebuild. + _ = reconcile(&leftTrigger, to: RumbleTuning.amplitude(target.leftTrigger)) + _ = reconcile(&rightTrigger, to: RumbleTuning.amplitude(target.rightTrigger)) if !ok { let wasSplit = high != nil teardown() @@ -410,9 +442,11 @@ final class RumbleRenderer: @unchecked Sendable { /// The ticker runs only while something needs tending — any nonzero target (watchdog, /// throttle catch-up, HID keepalive, post-reset engine rebuild) or segments still alive. private func updateTicker() { - let needed = target != (0, 0) + let needed = target != (0, 0, 0, 0) || low?.current != nil || low?.retiring != nil || high?.current != nil || high?.retiring != nil + || leftTrigger?.current != nil || leftTrigger?.retiring != nil + || rightTrigger?.current != nil || rightTrigger?.retiring != nil if needed, ticker == nil { let t = DispatchSource.makeTimerSource(queue: queue) t.schedule( @@ -477,6 +511,26 @@ final class RumbleRenderer: @unchecked Sendable { preferCombined = true log.info("rumble: split-handle engines failing — will retry with one combined engine") } + // Return before the trigger engines: the retry path re-enters setup() on the same + // `low == nil, high == nil` condition, so building them here would leak a fresh pair + // on every attempt (teardown() only runs on the failure paths above, and this is not + // one of them). + return + } + // Impulse-trigger motors, built last and best-effort. Independent of the handle split — + // the localities are separate and a pad can offer either, both or neither — and NOT part + // of the failure test above: nil here is the ordinary state of every pad that is not an + // Xbox One/Series/Elite, so it must not read as "engine setup failed", back off the handle + // engines, or produce a log line on a path that runs per controller attach. + // + // Whether a given pad + OS pair actually reports these localities is UNVERIFIED on glass. + // The degrade needs no code: `createEngine(withLocality:)` returns nil, the slots stay nil, + // and `reconcile` no-ops on them. + if localities.contains(.leftTrigger) { + leftTrigger = makeMotor(haptics, .leftTrigger, sharpness: RumbleTuning.sharpnessHigh) + } + if localities.contains(.rightTrigger) { + rightTrigger = makeMotor(haptics, .rightTrigger, sharpness: RumbleTuning.sharpnessHigh) } } @@ -563,7 +617,7 @@ final class RumbleRenderer: @unchecked Sendable { } private func teardown() { - for m in [low, high].compactMap({ $0 }) { + for m in [low, high, leftTrigger, rightTrigger].compactMap({ $0 }) { // Disarm the handlers before stopping so stop() can't re-enter teardown via them. // (Both properties are non-optional closures on this SDK, so assign no-ops, not nil.) m.engine.stoppedHandler = { _ in } @@ -577,6 +631,8 @@ final class RumbleRenderer: @unchecked Sendable { } low = nil high = nil + leftTrigger = nil + rightTrigger = nil } private func seconds(since t: DispatchTime) -> TimeInterval { @@ -624,6 +680,16 @@ final class RumbleRenderer: @unchecked Sendable { /// 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). + /// + /// **The impulse-trigger levels are deliberately dropped here, and there is no mapping to + /// invent.** A DualSense has *adaptive* triggers — force resistance on a trigger you press, + /// driven by the separate 0xCD `HidOutput.Trigger` plane — and no trigger *motors*. The two + /// features are unrelated hardware that only share a word: an Xbox Series pad has trigger + /// motors and no adaptive triggers, a DualSense has the reverse. Routing wire trigger rumble + /// into either the DS5 rumble bytes (which are the two handles) or the adaptive-trigger + /// parameter block would fabricate feedback the game never asked for. This path returning + /// `true` also means a macOS DualSense never reaches the CoreHaptics trigger localities above, + /// which is correct for the same reason. private func renderHID() -> Bool { #if os(macOS) guard let hid = dualSenseHID else { return false } diff --git a/crates/punktfunk-core/src/abi.rs b/crates/punktfunk-core/src/abi.rs index f9c67603..1723a8be 100644 --- a/crates/punktfunk-core/src/abi.rs +++ b/crates/punktfunk-core/src/abi.rs @@ -1186,7 +1186,10 @@ pub const PUNKTFUNK_GAMEPAD_XBOX360: u32 = 1; pub const PUNKTFUNK_GAMEPAD_DUALSENSE: u32 = 2; /// uinput X-Box One / Series pad — the X-Box 360 backend with the One/Series USB identity, so /// games show One/Series glyphs. XInput-identical to `XBOX360` otherwise (no game-visible gain; -/// impulse-trigger rumble is unreachable through a virtual pad). Useful for glyph-matching a +/// impulse-trigger rumble is unreachable through THIS pad — evdev's `FF_RUMBLE` is two +/// magnitudes and has no third, so a uinput backend can never source it. The Windows HID Xbox +/// backend can, off its output report `0x03`; see +/// [`punktfunk_connection_next_rumble_cmd2`]). Useful for glyph-matching a /// physical X-Box One/Series controller on the client. pub const PUNKTFUNK_GAMEPAD_XBOXONE: u32 = 3; /// UHID DualShock 4 (kernel `hid-playstation` ≥ 6.2): lightbar, touchpad, motion, rumble — the @@ -2723,8 +2726,20 @@ pub const PUNKTFUNK_RUMBLE_QUIRK_DEDUP_JITTER: u32 = 1; /// [`PunktfunkStatus::NoFrame`] on timeout; [`PunktfunkStatus::Closed`] once the session ended AND /// every close-drain stop was delivered — silence all actuators on it. /// -/// An embedder uses EITHER this or `next_rumble`/`next_rumble2` for a connection's lifetime, -/// never both (they consume the same wire plane). +/// **Handle motors only.** A pad also carries two Xbox impulse-trigger levels, which this entry +/// point has no out-params for and never will — +/// [`punktfunk_connection_next_rumble_cmd2`] is the four-motor pull. Staying here is a supported +/// choice, not a deprecation: for a controller with no trigger motors — every pad but an Xbox +/// One/Series/Elite — the two views are identical, and where they differ, "the handles are silent" +/// is exactly the right instruction for the motors this API owns. +/// +/// The one observable difference against a trigger-driving host: a rumble that moves only the +/// triggers still produces commands here, carrying `low == high == 0`. They are idempotent stops +/// for the handles; the engine's redundant-stop suppression cannot fold them away, because the +/// command is not silent — some motor on that pad is running. +/// +/// An embedder uses EITHER this (or its `2` form) or `next_rumble`/`next_rumble2` for a +/// connection's lifetime, never both (they consume the same wire plane). /// /// # Safety /// `c` is a valid connection handle; out pointers are writable (NULLs are skipped). At most one @@ -2775,6 +2790,95 @@ pub unsafe extern "C" fn punktfunk_connection_next_rumble_cmd( }) } +/// [`punktfunk_connection_next_rumble_cmd`] with the two Xbox impulse-trigger motors: the same +/// command, all four of its levels. `*left_trigger` / `*right_trigger` are on the same +/// `0..=0xFFFF` scale as `low`/`high`, and a stop is all four at zero. +/// +/// A NEW symbol rather than a wider signature on the old one, following the +/// `next_rumble` → `next_rumble2` precedent in this file: an exported entry point's parameter list +/// is part of the contract, and silently growing one breaks every out-of-tree embedder at once, +/// with a stack-corruption signature rather than a link error. Old callers keep the old symbol and +/// simply never see the trigger levels. +/// +/// **Render the trigger levels only on a pad that actually has trigger motors, and drop them +/// otherwise** — do not fold them into the handles. Impulse-trigger content is continuous +/// (a racing title drives engine RPM and tyre slip into the triggers while the handles stay near +/// silent), so folding it produces a handle motor droning flat-out for the whole race at a level +/// the game never asked for. Query the hardware: SDL's +/// `SDL_PROP_GAMEPAD_CAP_TRIGGER_RUMBLE_BOOLEAN`, Apple's `GCDeviceHaptics.supportedLocalities` +/// (`GCHapticsLocalityLeftTrigger`/`…RightTrigger`). A pad without them is the common case and not +/// an error — do not log per command. +/// +/// **Nothing has driven these levels non-zero end to end yet, and that is structural, not an +/// oversight.** Exactly one producer can ever source them — the Windows HID Xbox pad's output +/// report `0x03` — because classic XInput's `XINPUT_VIBRATION` has two members and evdev's +/// `FF_RUMBLE` has two, so no other host backend on any OS has the channel. That producer is +/// reachable only through GameInput/WGI, and an xinputhid-promoted Xbox pad is not enumerated by +/// GameInput at all (measured against a real Microsoft Elite, which is equally invisible there +/// while XInput reads it live). So this delivery path is deliberately built ahead of its producer: +/// the wire, the engine and this entry point are exercised only by synthetic levels. +/// +/// Same threading, timeout and close semantics as +/// [`punktfunk_connection_next_rumble_cmd`]; the two share one wire plane and one policy engine, +/// so an embedder calls exactly one of them. +/// +/// # Safety +/// `c` is a valid connection handle; out pointers are writable (NULLs are skipped). At most one +/// thread pulls rumble — it may run concurrently with the video/audio pullers. +#[cfg(feature = "quic")] +#[no_mangle] +pub unsafe extern "C" fn punktfunk_connection_next_rumble_cmd2( + c: *mut PunktfunkConnection, + pad: *mut u16, + low: *mut u16, + high: *mut u16, + left_trigger: *mut u16, + right_trigger: *mut u16, + backstop_ms: *mut u32, + timeout_ms: u32, +) -> PunktfunkStatus { + guard(|| { + // SAFETY: per the ABI contract - an opaque handle from a `*_new`/`*_pair` that the caller + // has not yet freed, or null, which `as_mut`/`as_ref` reports as `None` and the `match` + // here handles. + let c = match unsafe { c.as_ref() } { + Some(c) => c, + None => return PunktfunkStatus::NullPointer, + }; + match c + .inner + .next_rumble_command(std::time::Duration::from_millis(timeout_ms as u64)) + { + Ok(cmd) => { + // SAFETY: per the ABI contract - each out-param below is OPTIONAL, so it is null- + // checked before it is written; a non-null one is a caller-owned writable slot. + unsafe { + if !pad.is_null() { + *pad = cmd.pad; + } + if !low.is_null() { + *low = cmd.low; + } + if !high.is_null() { + *high = cmd.high; + } + if !left_trigger.is_null() { + *left_trigger = cmd.left_trigger; + } + if !right_trigger.is_null() { + *right_trigger = cmd.right_trigger; + } + if !backstop_ms.is_null() { + *backstop_ms = cmd.backstop_ms; + } + } + PunktfunkStatus::Ok + } + Err(e) => e.status(), + } + }) +} + /// 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 diff --git a/crates/punktfunk-core/src/client/mod.rs b/crates/punktfunk-core/src/client/mod.rs index aff3892c..258df579 100644 --- a/crates/punktfunk-core/src/client/mod.rs +++ b/crates/punktfunk-core/src/client/mod.rs @@ -1232,10 +1232,15 @@ impl NativeClient { /// the engine emits the level on every wire update (renewals re-arm duration-parameterized /// APIs), an explicit zero at lease expiry / legacy staleness / connection close, and /// quirk-declared keepalives ([`NativeClient::set_rumble_quirks`]). Apply commands verbatim: - /// `(0, 0)` = stop now; non-zero = run at this level, with `backstop_ms` as the safety-net + /// all-zero = stop now; non-zero = run at this level, with `backstop_ms` as the safety-net /// duration for APIs that take one. [`PunktfunkError::NoFrame`] on timeout; /// [`PunktfunkError::Closed`] once the session ended AND every close-drain stop was delivered. /// + /// A command carries FOUR levels: the two handle motors plus the two Xbox impulse-trigger + /// motors ([`RumbleCommand`]). Render the trigger pair only on a pad that has trigger motors + /// (SDL: `has_rumble_triggers()`); dropping them otherwise is the correct degrade, and folding + /// them into a handle is specifically not — see [`RumbleCommand`] for why. + /// /// One puller thread, and one API: an embedder uses EITHER this or /// `next_rumble`/`next_rumble_ttl` for a connection's lifetime, never both (both consume the /// same wire plane; the raw queue keeps filling harmlessly while this API is used). diff --git a/crates/punktfunk-core/src/client/pump/datagram_task.rs b/crates/punktfunk-core/src/client/pump/datagram_task.rs index a08ea68b..02c38bf7 100644 --- a/crates/punktfunk-core/src/client/pump/datagram_task.rs +++ b/crates/punktfunk-core/src/client/pump/datagram_task.rs @@ -92,16 +92,23 @@ pub(super) async fn run( // Both consumers are fed; an embedder drains exactly one of them // (the legacy queue, or the policy engine's command API). // - // `u.left_trigger`/`u.right_trigger` (the v3 tail) are decoded and - // deliberately NOT forwarded yet: neither consumer has a slot for them. - // Widening them is the client-engine work package — `RumbleCommand` grows - // two fields, `ActuatorQuirks` learns whether the physical pad has trigger - // motors, and the C ABI gains a `next_rumble_cmd2` beside the existing - // fixed-out-param puller. Dropping them here is exactly what the §5 - // compatibility table calls "new host, old client": the handle motors - // behave identically and the trigger levels are silently discarded. + // Only the policy engine carries `u.left_trigger`/`u.right_trigger` (the + // v3 impulse-trigger tail). The legacy queue's tuple is the shape two + // frozen C entry points read through fixed out-params + // (`punktfunk_connection_next_rumble`/`_next_rumble2`), so it stays at the + // two handle levels forever: an out-of-tree embedder on those symbols must + // keep behaving exactly as it did. That is the §5 compatibility table's + // "new host, old client" cell, and it is now a per-API property rather + // than a per-client one — the same session can serve both. let _ = rumble_tx.try_send((u.pad, u.low, u.high, ttl)); - rumble_feed.wire_update(u.pad, u.low, u.high, ttl); + rumble_feed.wire_update( + u.pad, + u.low, + u.high, + u.left_trigger, + u.right_trigger, + ttl, + ); } } } diff --git a/crates/punktfunk-core/src/client/rumble.rs b/crates/punktfunk-core/src/client/rumble.rs index fa028876..056a9e3f 100644 --- a/crates/punktfunk-core/src/client/rumble.rs +++ b/crates/punktfunk-core/src/client/rumble.rs @@ -22,6 +22,14 @@ //! a per-pad mailbox and commands are generated on demand, so a stalled embedder wakes to ONE //! current-level command instead of a backlog — and a stop can never be the update that an //! overflowing queue drops. +//! +//! A pad carries FOUR motor levels ([`Levels`]): the two handles plus the two Xbox impulse-trigger +//! motors off the 0xCA v3 tail (`design/trigger-rumble-plane.md`). They deliberately share one +//! lease, one seq and one policy — they are a single statement of the pad's feedback state at one +//! instant, so the whole apparatus above (expiry, staleness, keepalives, close drain) governs the +//! trigger motors with no second timeline. Every liveness test is therefore against all four +//! levels, not the handles: a trigger-only rumble is the *normal* shape of impulse-trigger +//! content, and a two-field test would silence it on arrival. use crate::input::MAX_PADS; use std::sync::{Condvar, Mutex}; @@ -52,18 +60,41 @@ const BACKSTOP_LEGACY_MS: u32 = 2000; /// header already has ~170 instances of, and one this has no reason to add to. const MAX_LEASE_MS: u16 = 5_000; -/// One effective actuator command. `(0, 0)` means stop now. `backstop_ms` is a safety-net -/// duration for platform APIs that take one (SDL rumble, Android one-shots): the engine emits -/// explicit zeros at every policy stop, so the backstop only matters if the embedder thread itself -/// stalls; platforms with explicit-stop APIs ignore it. Zero commands carry `backstop_ms == 0`. +/// One effective actuator command: four motor levels for one pad at one instant. All-zero means +/// stop now. `backstop_ms` is a safety-net duration for platform APIs that take one (SDL rumble, +/// Android one-shots): the engine emits explicit zeros at every policy stop, so the backstop only +/// matters if the embedder thread itself stalls; platforms with explicit-stop APIs ignore it. Zero +/// commands carry `backstop_ms == 0`. +/// +/// `left_trigger`/`right_trigger` are the Xbox impulse-trigger motors off the 0xCA v3 tail +/// (`design/trigger-rumble-plane.md`), on the same `0..=0xFFFF` scale as `low`/`high`. A renderer +/// on a pad without trigger motors ignores them — that is the *normal* case, not an error, and the +/// engine deliberately does not fold them into the handles (folding a racing title's continuous +/// trigger stream onto a handle motor drones flat-out for the whole race; §8 of the design). +/// +/// A pre-trigger embedder reading only `(low, high)` stays correct: the four levels are one +/// statement of the pad's state, so a trigger-only rumble reads as "handles silent", which is what +/// its actuator should do. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct RumbleCommand { pub pad: u16, pub low: u16, pub high: u16, + pub left_trigger: u16, + pub right_trigger: u16, pub backstop_ms: u32, } +/// One pad's four motor levels, in wire order: `(low, high, left_trigger, right_trigger)`. The two +/// handle motors first so the pre-trigger `(low, high)` reading is a literal prefix of this one. +type Levels = (u16, u16, u16, u16); + +/// The reserved "this actuator group is silent" value. Every liveness test in the engine is +/// against ALL FOUR levels: a rumble that drives only the impulse triggers — the normal shape of +/// racing-title content, where the handles stay at rest — must read as LIVE, or it would be +/// silenced on arrival by a two-field test that never saw its levels. +const SILENT: Levels = (0, 0, 0, 0); + /// A physical actuator's declared quirks — how a platform parameterizes the shared policy instead /// of forking it. Defaults (all zero/false) describe a well-behaved actuator. #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] @@ -96,7 +127,7 @@ pub struct ActuatorQuirks { #[derive(Clone, Copy)] struct PadState { - level: (u16, u16), + level: Levels, /// v2 lease expiry — `None` for a zero level or a legacy pad. deadline: Option, /// Last v2 TTL (drives the backstop); 0 ⇔ legacy. @@ -106,23 +137,23 @@ struct PadState { /// A wire update landed since the last emit (level change OR renewal — renewals re-emit). dirty: bool, next_keepalive: Option, - /// The exact value last handed to an embedder. `(0, 0)` ⇔ the engine believes this actuator is - /// silent. It replaces a free-running jitter phase because one field answers all three live - /// questions: would re-sending this be a no-op device write (the dedupe nudge), is a stop - /// redundant, and would the nudge synthesize the reserved stop. - last_emit: (u16, u16), + /// The exact value last handed to an embedder. [`SILENT`] ⇔ the engine believes this pad's + /// actuators are all silent. It replaces a free-running jitter phase because one field answers + /// all three live questions: would re-sending this be a no-op device write (the dedupe nudge), + /// is a stop redundant, and would the nudge synthesize the reserved stop. + last_emit: Levels, quirks: ActuatorQuirks, } impl PadState { const NEUTRAL: PadState = PadState { - level: (0, 0), + level: SILENT, deadline: None, ttl_ms: 0, legacy_wire: None, dirty: false, next_keepalive: None, - last_emit: (0, 0), + last_emit: SILENT, quirks: ActuatorQuirks { keepalive_ms: 0, min_pulse_ms: 0, @@ -139,18 +170,22 @@ impl PadState { b.max(self.quirks.min_pulse_ms as u32) } - /// Zero the pad's level + timers and produce the stop command. + /// Zero the pad's levels + timers and produce the stop command — all four motors, so a policy + /// stop silences the impulse triggers on the same event as the handles (which is the whole + /// reason they share one lease and one seq). fn silence(&mut self, pad: u16) -> RumbleCommand { - self.level = (0, 0); + self.level = SILENT; self.deadline = None; self.legacy_wire = None; self.next_keepalive = None; self.dirty = false; - self.last_emit = (0, 0); + self.last_emit = SILENT; RumbleCommand { pad, low: 0, high: 0, + left_trigger: 0, + right_trigger: 0, backstop_ms: 0, } } @@ -166,25 +201,39 @@ impl PadState { /// gap between *distinct* device writes to 80 ms at the default cadence and 100 ms at the /// floor, on an actuator whose quirk declares 40. /// - /// The nudge is refused when it would synthesize the reserved `(0, 0)` stop. That is level - /// `(1, 0)` and only that: `high` must already be 0, and `low ^ 1 == 0` implies `low == 1`. + /// The nudge is refused when it would synthesize the reserved all-zero stop. **Re-derived for + /// four levels, not mechanically widened** — the old proof reasoned about exactly two fields. + /// `emit` is only ever reached with `level != SILENT` (every caller in [`RumbleEngine::poll`] + /// guards on it), the nudge touches `low` alone, and it changes `low` by ±1 in the LSB. So the + /// nudged tuple can equal [`SILENT`] only when the three untouched levels are already zero AND + /// `low ^ 1 == 0`, i.e. exactly level `(1, 0, 0, 0)` — the same single case as before, now + /// conditioned on `high`, `left_trigger` and `right_trigger` together instead of `high` alone. /// There the LSB steps up instead, so the phase still alternates (1 ↔ 3, two parts in 65535) /// and the pad never receives a stop the policy did not order. + /// + /// The nudge stays on `low` even for a trigger-only level, where it lifts a resting handle + /// motor from 0 to 1. That is not new behaviour in kind — a `(0, high)` level has always been + /// nudged to `(1, high)` — and one part in 65535 is below any actuator's threshold. Moving it + /// to whichever level is non-zero would make the dedupe phase depend on which motors a + /// particular command happens to drive, which is exactly the free-running-phase failure + /// `last_emit` was introduced to remove. fn emit(&mut self, pad: u16) -> RumbleCommand { - let (mut low, high) = self.level; - if self.quirks.dedup_jitter && (low, high) == self.last_emit { + let (mut low, high, lt, rt) = self.level; + if self.quirks.dedup_jitter && self.level == self.last_emit { let alt = low ^ 1; - low = if (alt, high) == (0, 0) { + low = if (alt, high, lt, rt) == SILENT { low | 0b10 } else { alt }; } - self.last_emit = (low, high); + self.last_emit = (low, high, lt, rt); RumbleCommand { pad, low, high, + left_trigger: lt, + right_trigger: rt, backstop_ms: self.backstop(), } } @@ -210,18 +259,26 @@ impl RumbleEngine { /// Fold one seq-gated wire update in. Every update dirties the pad (renewals re-emit so /// platform duration timers re-arm); a v2 update replaces the lease deadline, a legacy update /// refreshes the staleness clock. + /// + /// `lt`/`rt` are the v3 impulse-trigger levels — zero for a v1/v2 datagram, because on a + /// level-triggered plane an absent field means "off now", never "keep what you had". + // Four levels, a pad index, a clock and a lease: grouping them would move the field list one + // hop from the two call sites (the demux feed and the tests) for nothing. + #[allow(clippy::too_many_arguments)] pub(crate) fn wire_update( &mut self, now: Instant, pad: u16, low: u16, high: u16, + lt: u16, + rt: u16, ttl_ms: Option, ) { let Some(p) = self.pads.get_mut(pad as usize) else { return; }; - p.level = (low, high); + p.level = (low, high, lt, rt); p.dirty = true; match ttl_ms { Some(t) => { @@ -229,7 +286,10 @@ impl RumbleEngine { let t = t.min(MAX_LEASE_MS); p.ttl_ms = t; p.legacy_wire = None; - p.deadline = if (low, high) != (0, 0) { + // All four levels decide whether there is a lease to run: a trigger-only rumble + // against silent handles is a LIVE level and must get a deadline, not the + // instantly-expired `None` a two-field test would have handed it. + p.deadline = if p.level != SILENT { Some(now + Duration::from_millis(t as u64)) } else { None @@ -261,7 +321,7 @@ impl RumbleEngine { for i in 0..MAX_PADS { let p = &mut self.pads[i]; let pad = i as u16; - if p.level != (0, 0) { + if p.level != SILENT { // 1) v2 lease expiry — the host stopped renewing (died / stopped caring). This // firing in the wild is the signature of a host-side bug: worth a log line. if let Some(d) = p.deadline { @@ -284,7 +344,7 @@ impl RumbleEngine { // 3) a wire update to relay (level change or renewal re-arm). if p.dirty { p.dirty = false; - if p.level == (0, 0) { + if p.level == SILENT { // Relay a stop only if the actuator is, as far as the engine knows, still // buzzing. A zero on an already-silent pad heals nothing and costs every // embedder a command — Android an unconditional log line plus a binder @@ -293,8 +353,8 @@ impl RumbleEngine { // `PUNKTFUNK_RUMBLE_ENVELOPE=0`) the legacy flat 500 ms refresh, which re-sends // zeros for every latched pad for the rest of the session. The burst still // heals the case it exists for: a LOST first stop leaves the pad buzzing, so - // `last_emit != (0, 0)` and the re-send does emit. - if p.last_emit != (0, 0) { + // `last_emit != SILENT` and the re-send does emit. + if p.last_emit != SILENT { return (Some(p.silence(pad)), None); } continue; @@ -308,7 +368,7 @@ impl RumbleEngine { // 4) actuator-decay keepalive, bounded by (1)/(2) above by construction: an expired // or stale pad was silenced before reaching here, so a keepalive can never sustain a // level the policy has ended. - if p.level != (0, 0) && p.quirks.keepalive_ms > 0 { + if p.level != SILENT && p.quirks.keepalive_ms > 0 { let ka = Duration::from_millis(p.quirks.keepalive_ms as u64); let due = *p.next_keepalive.get_or_insert(now + ka); if now >= due { @@ -325,7 +385,7 @@ impl RumbleEngine { /// silences every platform by contract instead of by per-client accident. pub(crate) fn close_drain(&mut self) -> Option { for i in 0..MAX_PADS { - if self.pads[i].level != (0, 0) { + if self.pads[i].level != SILENT { return Some(self.pads[i].silence(i as u16)); } } @@ -349,9 +409,18 @@ struct SharedState { pub(crate) struct RumbleFeed(pub(crate) std::sync::Arc); impl RumbleFeed { - pub(crate) fn wire_update(&self, pad: u16, low: u16, high: u16, ttl_ms: Option) { + pub(crate) fn wire_update( + &self, + pad: u16, + low: u16, + high: u16, + lt: u16, + rt: u16, + ttl_ms: Option, + ) { let mut g = self.0.inner.lock().unwrap(); - g.engine.wire_update(Instant::now(), pad, low, high, ttl_ms); + g.engine + .wire_update(Instant::now(), pad, low, high, lt, rt, ttl_ms); drop(g); self.0.cv.notify_all(); } @@ -425,7 +494,30 @@ mod tests { dedup_jitter: true, }; - /// Drain the engine the way an embedder does: poll until nothing is due. + /// Feed a HANDLE-ONLY wire update — what every producer but the Windows HID Xbox pad emits + /// (XInput's `XINPUT_VIBRATION` and evdev's `FF_RUMBLE` have two members and no third), so it + /// is also what the pre-v3 tests below are all about. Trigger cases call `wire4` instead. + fn wire(e: &mut RumbleEngine, t: Instant, pad: u16, low: u16, high: u16, ttl: Option) { + e.wire_update(t, pad, low, high, 0, 0, ttl); + } + + /// Feed a full v3 wire update, all four levels. + #[allow(clippy::too_many_arguments)] + fn wire4( + e: &mut RumbleEngine, + t: Instant, + pad: u16, + low: u16, + high: u16, + lt: u16, + rt: u16, + ttl: Option, + ) { + e.wire_update(t, pad, low, high, lt, rt, ttl); + } + + /// Drain the engine the way an embedder does: poll until nothing is due. Handle levels only — + /// `drain4` is the four-level view. fn drain(e: &mut RumbleEngine, t: Instant) -> Vec<(u16, u16)> { let mut out = Vec::new(); while let (Some(c), _) = e.poll(t) { @@ -434,15 +526,30 @@ mod tests { out } + fn drain4(e: &mut RumbleEngine, t: Instant) -> Vec { + let mut out = Vec::new(); + while let (Some(c), _) = e.poll(t) { + out.push((c.low, c.high, c.left_trigger, c.right_trigger)); + } + out + } + fn ms(v: u64) -> Duration { Duration::from_millis(v) } + /// A handle-only expected command — the shape every pre-v3 assertion below is written in. fn cmd(pad: u16, low: u16, high: u16, backstop_ms: u32) -> RumbleCommand { + cmd4(pad, low, high, 0, 0, backstop_ms) + } + + fn cmd4(pad: u16, low: u16, high: u16, lt: u16, rt: u16, backstop_ms: u32) -> RumbleCommand { RumbleCommand { pad, low, high, + left_trigger: lt, + right_trigger: rt, backstop_ms, } } @@ -451,7 +558,7 @@ mod tests { fn v2_level_emits_and_expires_at_the_lease() { let mut e = RumbleEngine::new(); let t0 = Instant::now(); - e.wire_update(t0, 0, 0x4000, 0x8000, Some(400)); + wire(&mut e, t0, 0, 0x4000, 0x8000, Some(400)); assert_eq!(e.poll(t0).0, Some(cmd(0, 0x4000, 0x8000, 800))); // backstop = 2×ttl // No renewal: at the deadline the engine self-silences — the host-died safety net. let (c, wake) = e.poll(t0 + ms(200)); @@ -465,11 +572,11 @@ mod tests { fn renewal_re_emits_and_extends_the_deadline() { let mut e = RumbleEngine::new(); let t0 = Instant::now(); - e.wire_update(t0, 0, 100, 0, Some(400)); + wire(&mut e, t0, 0, 100, 0, Some(400)); assert!(e.poll(t0).0.is_some()); // A same-level renewal at t+300 re-emits (platform duration timers re-arm) and pushes the // deadline to t+700 — so t+500 (past the ORIGINAL deadline) still rumbles. - e.wire_update(t0 + ms(300), 0, 100, 0, Some(400)); + wire(&mut e, t0 + ms(300), 0, 100, 0, Some(400)); assert_eq!(e.poll(t0 + ms(300)).0, Some(cmd(0, 100, 0, 800))); assert_eq!(e.poll(t0 + ms(500)).0, None); assert_eq!(e.poll(t0 + ms(700)).0, Some(cmd(0, 0, 0, 0))); @@ -479,9 +586,9 @@ mod tests { fn explicit_stop_is_immediate_and_cancels_the_lease() { let mut e = RumbleEngine::new(); let t0 = Instant::now(); - e.wire_update(t0, 2, 500, 500, Some(400)); + wire(&mut e, t0, 2, 500, 500, Some(400)); assert!(e.poll(t0).0.is_some()); - e.wire_update(t0 + ms(50), 2, 0, 0, Some(0)); + wire(&mut e, t0 + ms(50), 2, 0, 0, Some(0)); assert_eq!(e.poll(t0 + ms(50)).0, Some(cmd(2, 0, 0, 0))); assert_eq!(e.poll(t0 + ms(600)), (None, None)); // no phantom expiry later } @@ -490,10 +597,10 @@ mod tests { fn legacy_host_gets_the_uniform_staleness_bound() { let mut e = RumbleEngine::new(); let t0 = Instant::now(); - e.wire_update(t0, 0, 300, 0, None); // legacy: no TTL + wire(&mut e, t0, 0, 300, 0, None); // legacy: no TTL assert_eq!(e.poll(t0).0, Some(cmd(0, 300, 0, 2000))); // The legacy 500 ms refresh keeps it alive… - e.wire_update(t0 + ms(500), 0, 300, 0, None); + wire(&mut e, t0 + ms(500), 0, 300, 0, None); assert_eq!(e.poll(t0 + ms(500)).0, Some(cmd(0, 300, 0, 2000))); assert_eq!(e.poll(t0 + ms(1400)).0, None); // 900 ms since last wire — inside the bound // …and one second of silence cuts it, on every platform alike. @@ -512,7 +619,7 @@ mod tests { }, ); let t0 = Instant::now(); - e.wire_update(t0, 0, 100, 200, Some(400)); + wire(&mut e, t0, 0, 100, 200, Some(400)); assert_eq!(e.poll(t0).0, Some(cmd(0, 100, 200, 800))); // Keepalives at the quirk cadence, alternating the low LSB to defeat SDL's dedupe. assert_eq!(e.poll(t0 + ms(40)).0, Some(cmd(0, 101, 200, 800))); @@ -526,7 +633,7 @@ mod tests { fn quirk_registered_mid_rumble_starts_keepalives() { let mut e = RumbleEngine::new(); let t0 = Instant::now(); - e.wire_update(t0, 0, 100, 0, Some(400)); + wire(&mut e, t0, 0, 100, 0, Some(400)); assert!(e.poll(t0).0.is_some()); e.set_quirks( 0, @@ -555,7 +662,7 @@ mod tests { }, ); let t0 = Instant::now(); - e.wire_update(t0, 0, 100, 0, Some(100)); + wire(&mut e, t0, 0, 100, 0, Some(100)); assert_eq!(e.poll(t0).0, Some(cmd(0, 100, 0, 5000))); } @@ -563,8 +670,8 @@ mod tests { fn close_drain_silences_every_buzzing_pad_once() { let mut e = RumbleEngine::new(); let t0 = Instant::now(); - e.wire_update(t0, 0, 100, 0, Some(400)); - e.wire_update(t0, 3, 0, 900, Some(400)); + wire(&mut e, t0, 0, 100, 0, Some(400)); + wire(&mut e, t0, 3, 0, 900, Some(400)); let _ = e.poll(t0); let _ = e.poll(t0); let a = e.close_drain().unwrap(); @@ -581,7 +688,7 @@ mod tests { // 20 renewals landed while the embedder was stalled — state, not a queue: exactly one // command comes out, carrying the latest level. for k in 0..20u64 { - e.wire_update(t0 + ms(k * 120), 0, 100 + k as u16, 0, Some(400)); + wire(&mut e, t0 + ms(k * 120), 0, 100 + k as u16, 0, Some(400)); } let t = t0 + ms(20 * 120); assert_eq!(e.poll(t).0, Some(cmd(0, 119, 0, 800))); @@ -592,7 +699,7 @@ mod tests { fn shared_close_delivers_drain_zero_then_closed() { let shared = std::sync::Arc::new(RumbleShared::new()); let feed = RumbleFeed(shared.clone()); - feed.wire_update(1, 100, 0, Some(400)); + feed.wire_update(1, 100, 0, 0, 0, Some(400)); assert_eq!( shared.next_command(ms(100)).unwrap().unwrap(), cmd(1, 100, 0, 800) @@ -613,12 +720,12 @@ mod tests { let mut e = RumbleEngine::new(); e.set_quirks(0, DECK); let t0 = Instant::now(); - e.wire_update(t0, 0, 100, 200, Some(400)); + wire(&mut e, t0, 0, 100, 200, Some(400)); assert_eq!(drain(&mut e, t0), vec![(100, 200)]); assert_eq!(drain(&mut e, t0 + ms(40)), vec![(101, 200)]); assert_eq!(drain(&mut e, t0 + ms(80)), vec![(100, 200)]); // The renewal at the 120 ms default cadence: same level, must still be a distinct write. - e.wire_update(t0 + ms(120), 0, 100, 200, Some(400)); + wire(&mut e, t0 + ms(120), 0, 100, 200, Some(400)); assert_eq!(drain(&mut e, t0 + ms(120)), vec![(101, 200)]); assert_eq!(drain(&mut e, t0 + ms(160)), vec![(100, 200)]); } @@ -634,7 +741,7 @@ mod tests { for tick in 0..=360u64 { let t = t0 + ms(tick); if tick % 60 == 0 { - e.wire_update(t, 0, 100, 200, Some(400)); + wire(&mut e, t, 0, 100, 200, Some(400)); } for v in drain(&mut e, t) { assert_ne!(v, (0, 0), "a live lease must never emit the stop sentinel"); @@ -657,9 +764,9 @@ mod tests { fn default_quirks_pads_get_the_level_verbatim_on_every_renewal() { let mut e = RumbleEngine::new(); // Apple / Android / plain SDL let t0 = Instant::now(); - e.wire_update(t0, 0, 100, 200, Some(400)); + wire(&mut e, t0, 0, 100, 200, Some(400)); assert_eq!(e.poll(t0).0, Some(cmd(0, 100, 200, 800))); - e.wire_update(t0 + ms(120), 0, 100, 200, Some(400)); + wire(&mut e, t0 + ms(120), 0, 100, 200, Some(400)); assert_eq!(e.poll(t0 + ms(120)).0, Some(cmd(0, 100, 200, 800))); } @@ -670,7 +777,7 @@ mod tests { let mut e = RumbleEngine::new(); e.set_quirks(0, DECK); let t0 = Instant::now(); - e.wire_update(t0, 0, 1, 0, Some(400)); + wire(&mut e, t0, 0, 1, 0, Some(400)); assert_eq!(e.poll(t0).0, Some(cmd(0, 1, 0, 800))); assert_eq!(e.poll(t0 + ms(40)).0, Some(cmd(0, 3, 0, 800))); assert_eq!(e.poll(t0 + ms(80)).0, Some(cmd(0, 1, 0, 800))); @@ -683,20 +790,20 @@ mod tests { fn a_redundant_stop_is_dropped_but_the_burst_still_heals_a_lost_one() { let mut e = RumbleEngine::new(); let t0 = Instant::now(); - e.wire_update(t0, 0, 100, 200, Some(400)); + wire(&mut e, t0, 0, 100, 200, Some(400)); assert_eq!(drain(&mut e, t0), vec![(100, 200)]); // First stop reaches the embedder… - e.wire_update(t0 + ms(10), 0, 0, 0, Some(0)); + wire(&mut e, t0 + ms(10), 0, 0, 0, Some(0)); assert_eq!(drain(&mut e, t0 + ms(10)), vec![(0, 0)]); // …and the burst re-sends behind it are now silent. - e.wire_update(t0 + ms(20), 0, 0, 0, Some(0)); - e.wire_update(t0 + ms(30), 0, 0, 0, Some(0)); + wire(&mut e, t0 + ms(20), 0, 0, 0, Some(0)); + wire(&mut e, t0 + ms(30), 0, 0, 0, Some(0)); assert_eq!(drain(&mut e, t0 + ms(30)), Vec::new()); // But if the pad is buzzing (the stop that mattered was lost), a re-send still emits. - e.wire_update(t0 + ms(40), 0, 100, 200, Some(400)); + wire(&mut e, t0 + ms(40), 0, 100, 200, Some(400)); assert_eq!(drain(&mut e, t0 + ms(40)), vec![(100, 200)]); - e.wire_update(t0 + ms(50), 0, 0, 0, Some(0)); + wire(&mut e, t0 + ms(50), 0, 0, 0, Some(0)); assert_eq!(drain(&mut e, t0 + ms(50)), vec![(0, 0)]); } @@ -707,7 +814,7 @@ mod tests { fn an_overlong_lease_is_clamped_to_the_ceiling() { let mut e = RumbleEngine::new(); let t0 = Instant::now(); - e.wire_update(t0, 0, 100, 200, Some(u16::MAX)); + wire(&mut e, t0, 0, 100, 200, Some(u16::MAX)); assert_eq!(e.poll(t0).0, Some(cmd(0, 100, 200, 5000))); // Silenced at the ceiling, not at the 65 s the sender asked for. assert!(e.poll(t0 + ms(MAX_LEASE_MS as u64 - 1)).0.is_none()); @@ -726,11 +833,112 @@ mod tests { fn a_zero_ttl_envelope_silences_rather_than_taking_the_legacy_backstop() { let mut e = RumbleEngine::new(); let t0 = Instant::now(); - e.wire_update(t0, 0, 100, 200, Some(0)); + wire(&mut e, t0, 0, 100, 200, Some(0)); assert_eq!( e.poll(t0).0, Some(cmd(0, 0, 0, 0)), "a zero-length lease must expire immediately, not emit with a legacy backstop" ); } + + /// **The single most likely way to ship trigger rumble broken** (design §5): a rumble that + /// drives ONLY the impulse triggers is the normal shape of the content — racing titles run the + /// triggers continuously against silent handles. Every liveness test in the engine used to be + /// `(low, high) == (0, 0)`; left that way, a trigger-only update is read as a stop, dropped as + /// redundant on a silent pad, and the feature is dead with no error anywhere. + #[test] + fn a_trigger_only_rumble_is_a_live_level_not_a_stop() { + let mut e = RumbleEngine::new(); + let t0 = Instant::now(); + wire4(&mut e, t0, 0, 0, 0, 0x8000, 0, Some(400)); + assert_eq!( + e.poll(t0).0, + Some(cmd4(0, 0, 0, 0x8000, 0, 800)), + "a trigger-only level must emit with a live backstop" + ); + // It runs on the pad's ONE shared lease, exactly like the handles: no renewal, so the + // whole group silences at the deadline. + assert_eq!(e.poll(t0 + ms(200)), (None, Some(t0 + ms(400)))); + assert_eq!(e.poll(t0 + ms(400)).0, Some(cmd(0, 0, 0, 0))); + assert_eq!(e.poll(t0 + ms(500)), (None, None)); + } + + /// Backward compatibility for the pre-trigger C entry point + /// (`punktfunk_connection_next_rumble_cmd`, which writes `pad`/`low`/`high`/`backstop_ms` and + /// has no slot for the other two). Its embedder sees the same command, truncated to its first + /// two levels — and that truncation is CORRECT rather than merely tolerable: with no trigger + /// motors to drive, "handles silent" is what its actuator should do. The one visible + /// difference is that trigger traffic now produces commands where before the demux dropped it, + /// so such an embedder sees redundant handle stops while a trigger-only rumble runs. They are + /// idempotent; the redundant-stop suppression cannot apply, because the command is not silent. + #[test] + fn the_old_two_field_view_of_a_trigger_command_is_silent_handles() { + let mut e = RumbleEngine::new(); + let t0 = Instant::now(); + wire4(&mut e, t0, 0, 0x1111, 0, 0x8000, 0x4000, Some(400)); + let c = e.poll(t0).0.unwrap(); + assert_eq!((c.pad, c.low, c.high, c.backstop_ms), (0, 0x1111, 0, 800)); + assert_eq!((c.left_trigger, c.right_trigger), (0x8000, 0x4000)); + // Handles released, triggers still driven: the old view reads (0, 0) — a stop for the + // motors it owns — while the new view keeps the triggers alive. + wire4(&mut e, t0 + ms(50), 0, 0, 0, 0x8000, 0x4000, Some(400)); + let c = e.poll(t0 + ms(50)).0.unwrap(); + assert_eq!((c.low, c.high), (0, 0)); + assert_eq!((c.left_trigger, c.right_trigger), (0x8000, 0x4000)); + assert_ne!( + c.backstop_ms, 0, + "not a stop command — the pad is still live" + ); + } + + /// The trigger levels ride the pad's ONE seq/lease/keepalive apparatus, so a Deck-class + /// actuator's re-kicks carry them unchanged — and the dedupe nudge still only ever moves + /// `low`, never a trigger level (which would be a device write the policy did not order). + #[test] + fn keepalives_carry_the_trigger_levels_and_only_nudge_low() { + let mut e = RumbleEngine::new(); + e.set_quirks(0, DECK); + let t0 = Instant::now(); + wire4(&mut e, t0, 0, 100, 200, 300, 400, Some(400)); + assert_eq!(drain4(&mut e, t0), vec![(100, 200, 300, 400)]); + assert_eq!(drain4(&mut e, t0 + ms(40)), vec![(101, 200, 300, 400)]); + assert_eq!(drain4(&mut e, t0 + ms(80)), vec![(100, 200, 300, 400)]); + } + + /// The four-field re-derivation of the jitter proof (design §8): the reserved stop is now + /// all-four-zero, so the nudge must refuse only at `(1, 0, 0, 0)` — and must NOT refuse at + /// `(1, 0, lt, rt)`, where flipping the LSB is perfectly safe because the triggers keep the + /// command non-silent. A mechanical widening that kept testing `high` alone would get the + /// first case right and the second one wrong in the harmless direction; testing `(alt, high)` + /// against `(0, 0)` would get the first case wrong and send a Deck a stop nobody ordered. + #[test] + fn the_jitter_never_synthesizes_the_four_field_stop_sentinel() { + let mut e = RumbleEngine::new(); + e.set_quirks(0, DECK); + let t0 = Instant::now(); + // (1, 0, 0, 0): the ONE level whose LSB flip is the reserved stop — step up instead. + wire(&mut e, t0, 0, 1, 0, Some(400)); + assert_eq!(drain4(&mut e, t0), vec![(1, 0, 0, 0)]); + assert_eq!(drain4(&mut e, t0 + ms(40)), vec![(3, 0, 0, 0)]); + assert_eq!(drain4(&mut e, t0 + ms(80)), vec![(1, 0, 0, 0)]); + // (1, 0, lt, 0): a live trigger level, so the plain LSB flip to 0 is safe and taken. + let mut e = RumbleEngine::new(); + e.set_quirks(0, DECK); + wire4(&mut e, t0, 0, 1, 0, 0x8000, 0, Some(400)); + assert_eq!(drain4(&mut e, t0), vec![(1, 0, 0x8000, 0)]); + assert_eq!(drain4(&mut e, t0 + ms(40)), vec![(0, 0, 0x8000, 0)]); + assert_eq!(drain4(&mut e, t0 + ms(80)), vec![(1, 0, 0x8000, 0)]); + } + + /// A pad still buzzing on the triggers alone must be silenced by the close drain — the same + /// contract the handles have, and the reason `close_drain` tests all four levels. + #[test] + fn close_drain_silences_a_trigger_only_pad() { + let mut e = RumbleEngine::new(); + let t0 = Instant::now(); + wire4(&mut e, t0, 2, 0, 0, 0, 0x9000, Some(400)); + assert!(e.poll(t0).0.is_some()); + assert_eq!(e.close_drain(), Some(cmd(2, 0, 0, 0))); + assert_eq!(e.close_drain(), None); + } } diff --git a/crates/punktfunk-core/src/lib.rs b/crates/punktfunk-core/src/lib.rs index 60b559dd..ef61b356 100644 --- a/crates/punktfunk-core/src/lib.rs +++ b/crates/punktfunk-core/src/lib.rs @@ -145,7 +145,18 @@ pub use stats::Stats; /// connection was simply lost. Purely a read of state the core already had: no new call is required /// of an embedder, a client that never calls it is unchanged, and the host sends exactly the same /// bytes either way, so [`WIRE_VERSION`] is unchanged. -pub const ABI_VERSION: u32 = 17; +/// v18: added `punktfunk_connection_next_rumble_cmd2` — the policy engine's rumble command with the +/// two Xbox impulse-trigger motor levels off the 0xCA v3 tail +/// (`design/trigger-rumble-plane.md`), which the fixed out-params of +/// `punktfunk_connection_next_rumble_cmd` have no room for. A NEW symbol, not a widened one: an +/// exported parameter list is part of the contract, and growing one in place breaks every +/// out-of-tree embedder at once. The old entry point is unchanged in signature AND in the levels +/// it reports — it keeps writing the two handle motors, which is the correct instruction for the +/// actuators it owns, so an embedder that never adopts the new symbol behaves exactly as before. +/// Additive and client-local: the v3 tail has been on the wire (and length-tolerant in both +/// decoders) since it landed, and the host sends the same bytes either way, so [`WIRE_VERSION`] is +/// unchanged. +pub const ABI_VERSION: u32 = 18; /// The punktfunk/1 **wire** version — what `Hello`/`Welcome` carry and hosts equality-check. /// Deliberately its own constant: [`ABI_VERSION`] tracks the embeddable **C surface** diff --git a/docs/embedding-the-c-abi.md b/docs/embedding-the-c-abi.md index c7d76014..d3cf0c76 100644 --- a/docs/embedding-the-c-abi.md +++ b/docs/embedding-the-c-abi.md @@ -484,6 +484,29 @@ Pull these on your feedback thread (or poll with `timeout_ms = 0`). Same Amplitudes 0..0xFFFF; `(0,0)` = stop. `ttl_ms` is a host-supplied self-terminating lease — render the level for that long unless renewed; `PUNKTFUNK_RUMBLE_NO_TTL` means fall back to your own staleness timeout. (The v1 `_next_rumble` drops the TTL — prefer v2.) +- **Rumble, policy-engine form** — `punktfunk_connection_next_rumble_cmd(c, &pad, &low, &high, + &backstop_ms, timeout)` hands you **effective commands** instead of raw wire state: the core owns + lease expiry, legacy-host staleness and close-drain zeros, so you apply what you are told and keep + no staleness policy of your own. `backstop_ms` is a safety net for APIs that take a duration + (ignored by explicit-stop APIs; `0` on stops). Pick **one** rumble API per connection — they + consume the same plane. +- **Rumble with trigger motors** (ABI ≥ 18) — `punktfunk_connection_next_rumble_cmd2(c, &pad, &low, + &high, &left_trigger, &right_trigger, &backstop_ms, timeout)` is the same command with the two + Xbox impulse-trigger levels, on the same 0..0xFFFF scale; a stop is all four at zero. It is a + **new symbol, not a wider `_cmd`** — `_cmd` keeps its signature and its two-handle view forever, + so existing embedders need no change. Render the trigger pair only on a pad that has trigger + motors (Windows: `IGameInputDevice::SetRumbleState`'s `leftTrigger`/`rightTrigger`, or WGI's + `GamepadVibration`; SDL: `SDL_RumbleGamepadTriggers` gated on + `SDL_PROP_GAMEPAD_CAP_TRIGGER_RUMBLE_BOOLEAN`; Apple: `GCHapticsLocalityLeftTrigger` / + `…RightTrigger`) and **drop them otherwise — never fold them into a handle motor**: impulse-trigger + content is continuous (a racing title drives it off engine RPM and tyre slip while the handles stay + near silent), so folding drones a handle flat-out for the whole race at a level the game never + asked for. A pad without trigger motors is the common case, not an error; do not log per command. + Note that on a trigger-driving host a `_cmd` caller now sees commands carrying `low == high == 0` + while only the triggers run — correct (its motors *should* be silent) and idempotent. + Nothing sources non-zero trigger levels end to end yet: only the Windows HID Xbox pad has the + channel at all (XInput's `XINPUT_VIBRATION` and evdev's `FF_RUMBLE` each have exactly two + members), and it is reachable only through GameInput/WGI. - **DualSense HID output** — `punktfunk_connection_next_hidout(c, &out, timeout)`. `out.kind` selects lightbar RGB / player LEDs / adaptive-trigger effect / trackpad haptic. Replay on a real DualSense via the platform's controller API. Only a DualSense-backend session emits these. @@ -629,7 +652,10 @@ shared-mode render. Request 6/8 channels at connect for surround. and emit `GAMEPAD_BUTTON`/`GAMEPAD_AXIS` events. Because a real Xbox pad drives this, connect with `PUNKTFUNK_GAMEPAD_XBOXONE` for matching glyphs. Rumble comes **back** from the host — feed `punktfunk_connection_next_rumble2` into `IGameInputDevice::SetRumbleState` (map `low`→ -low-frequency, `high`→high-frequency motors). +low-frequency, `high`→high-frequency motors). `GameInputRumbleParams` has two more members, +`leftTrigger`/`rightTrigger`, and this is the one platform API that can drive them: use +`punktfunk_connection_next_rumble_cmd2` (ABI ≥ 18) instead and fill all four. The host can only +ever source non-zero trigger levels from its Windows HID Xbox pad, so expect zeros elsewhere. **Skeleton (C++):** diff --git a/include/punktfunk_core.h b/include/punktfunk_core.h index d7a44760..2f62ab3d 100644 --- a/include/punktfunk_core.h +++ b/include/punktfunk_core.h @@ -83,7 +83,18 @@ // connection was simply lost. Purely a read of state the core already had: no new call is required // of an embedder, a client that never calls it is unchanged, and the host sends exactly the same // bytes either way, so [`WIRE_VERSION`] is unchanged. -#define PUNKTFUNK_ABI_VERSION 17 +// v18: added `punktfunk_connection_next_rumble_cmd2` — the policy engine's rumble command with the +// two Xbox impulse-trigger motor levels off the 0xCA v3 tail +// (`design/trigger-rumble-plane.md`), which the fixed out-params of +// `punktfunk_connection_next_rumble_cmd` have no room for. A NEW symbol, not a widened one: an +// exported parameter list is part of the contract, and growing one in place breaks every +// out-of-tree embedder at once. The old entry point is unchanged in signature AND in the levels +// it reports — it keeps writing the two handle motors, which is the correct instruction for the +// actuators it owns, so an embedder that never adopts the new symbol behaves exactly as before. +// Additive and client-local: the v3 tail has been on the wire (and length-tolerant in both +// decoders) since it landed, and the host sends the same bytes either way, so [`WIRE_VERSION`] is +// unchanged. +#define PUNKTFUNK_ABI_VERSION 18 // The punktfunk/1 **wire** version — what `Hello`/`Welcome` carry and hosts equality-check. // Deliberately its own constant: [`ABI_VERSION`] tracks the embeddable **C surface** @@ -195,7 +206,10 @@ // uinput X-Box One / Series pad — the X-Box 360 backend with the One/Series USB identity, so // games show One/Series glyphs. XInput-identical to `XBOX360` otherwise (no game-visible gain; -// impulse-trigger rumble is unreachable through a virtual pad). Useful for glyph-matching a +// impulse-trigger rumble is unreachable through THIS pad — evdev's `FF_RUMBLE` is two +// magnitudes and has no third, so a uinput backend can never source it. The Windows HID Xbox +// backend can, off its output report `0x03`; see +// [`punktfunk_connection_next_rumble_cmd2`]). Useful for glyph-matching a // physical X-Box One/Series controller on the client. #define PUNKTFUNK_GAMEPAD_XBOXONE 3 @@ -2774,8 +2788,20 @@ PunktfunkStatus punktfunk_connection_next_rumble2(PunktfunkConnection *c, // [`PunktfunkStatus::NoFrame`] on timeout; [`PunktfunkStatus::Closed`] once the session ended AND // every close-drain stop was delivered — silence all actuators on it. // -// An embedder uses EITHER this or `next_rumble`/`next_rumble2` for a connection's lifetime, -// never both (they consume the same wire plane). +// **Handle motors only.** A pad also carries two Xbox impulse-trigger levels, which this entry +// point has no out-params for and never will — +// [`punktfunk_connection_next_rumble_cmd2`] is the four-motor pull. Staying here is a supported +// choice, not a deprecation: for a controller with no trigger motors — every pad but an Xbox +// One/Series/Elite — the two views are identical, and where they differ, "the handles are silent" +// is exactly the right instruction for the motors this API owns. +// +// The one observable difference against a trigger-driving host: a rumble that moves only the +// triggers still produces commands here, carrying `low == high == 0`. They are idempotent stops +// for the handles; the engine's redundant-stop suppression cannot fold them away, because the +// command is not silent — some motor on that pad is running. +// +// An embedder uses EITHER this (or its `2` form) or `next_rumble`/`next_rumble2` for a +// connection's lifetime, never both (they consume the same wire plane). // // # Safety // `c` is a valid connection handle; out pointers are writable (NULLs are skipped). At most one @@ -2788,6 +2814,52 @@ PunktfunkStatus punktfunk_connection_next_rumble_cmd(PunktfunkConnection *c, uint32_t timeout_ms); #endif +#if defined(PUNKTFUNK_FEATURE_QUIC) +// [`punktfunk_connection_next_rumble_cmd`] with the two Xbox impulse-trigger motors: the same +// command, all four of its levels. `*left_trigger` / `*right_trigger` are on the same +// `0..=0xFFFF` scale as `low`/`high`, and a stop is all four at zero. +// +// A NEW symbol rather than a wider signature on the old one, following the +// `next_rumble` → `next_rumble2` precedent in this file: an exported entry point's parameter list +// is part of the contract, and silently growing one breaks every out-of-tree embedder at once, +// with a stack-corruption signature rather than a link error. Old callers keep the old symbol and +// simply never see the trigger levels. +// +// **Render the trigger levels only on a pad that actually has trigger motors, and drop them +// otherwise** — do not fold them into the handles. Impulse-trigger content is continuous +// (a racing title drives engine RPM and tyre slip into the triggers while the handles stay near +// silent), so folding it produces a handle motor droning flat-out for the whole race at a level +// the game never asked for. Query the hardware: SDL's +// `SDL_PROP_GAMEPAD_CAP_TRIGGER_RUMBLE_BOOLEAN`, Apple's `GCDeviceHaptics.supportedLocalities` +// (`GCHapticsLocalityLeftTrigger`/`…RightTrigger`). A pad without them is the common case and not +// an error — do not log per command. +// +// **Nothing has driven these levels non-zero end to end yet, and that is structural, not an +// oversight.** Exactly one producer can ever source them — the Windows HID Xbox pad's output +// report `0x03` — because classic XInput's `XINPUT_VIBRATION` has two members and evdev's +// `FF_RUMBLE` has two, so no other host backend on any OS has the channel. That producer is +// reachable only through GameInput/WGI, and an xinputhid-promoted Xbox pad is not enumerated by +// GameInput at all (measured against a real Microsoft Elite, which is equally invisible there +// while XInput reads it live). So this delivery path is deliberately built ahead of its producer: +// the wire, the engine and this entry point are exercised only by synthetic levels. +// +// Same threading, timeout and close semantics as +// [`punktfunk_connection_next_rumble_cmd`]; the two share one wire plane and one policy engine, +// so an embedder calls exactly one of them. +// +// # Safety +// `c` is a valid connection handle; out pointers are writable (NULLs are skipped). At most one +// thread pulls rumble — it may run concurrently with the video/audio pullers. +PunktfunkStatus punktfunk_connection_next_rumble_cmd2(PunktfunkConnection *c, + uint16_t *pad, + uint16_t *low, + uint16_t *high, + uint16_t *left_trigger, + uint16_t *right_trigger, + uint32_t *backstop_ms, + uint32_t timeout_ms); +#endif + #if defined(PUNKTFUNK_FEATURE_QUIC) // 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).