From 6a82a602a1b927b348ce37d335d7e77f27f36527 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Fri, 21 Aug 2026 13:10:23 +0200 Subject: [PATCH] fix(clients/android): the button correction fired on pads that never needed it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two field reports (2026-08-21), one shape: a GameSir G8+ and an Xbox Elite Series 2 ("Xbox Wireless Controller" over Bluetooth) with X answering Y, Y answering LB, and the two shoulders answering menu buttons — everything else correct. That is not a stray mapping, it is exactly what `GENERIC_XBOX` does to scancodes `0x133`/`0x134`/`0x136`/`0x137`, so the correction added yesterday was firing on pads whose buttons were already where `Generic.kl` says they are. It fired because it asked the wrong question. `hasKeys(BUTTON_C, BUTTON_Z)` answers for what a device DECLARES, not what it reports: `hid-input` allocates `BTN_A + n` straight through for every button in the descriptor, so BTN_C (`0x132`) and BTN_Z (`0x135`) are set on ANY pad declaring six or more — including a standard-layout pad that never presses either. The signal is therefore identical on the pad that needs correcting and the pad that does not, and no amount of tightening it could have separated them. It is the same pad model in both reports: an Elite Series 2 needed the correction on a Fire TV and another Elite Series 2 was broken by it here. What does separate them is the axes. A HID gamepad describes its triggers either as the Accelerator/Brake usages — which become `ABS_GAS`/`ABS_BRAKE`, names Android has words for — or as two more generic axes on `ABS_Z`/`ABS_RZ`, which it does not. A descriptor well-formed enough to name its triggers puts its buttons at the standard positions too. It is also the firmware line on the pad in the report: an Xbox Wireless Controller over Bluetooth reports GAS/BRAKE after its firmware update and Z/Rz before it, and only the older one was ever wrong. `padButtons` now takes `namedTriggers` and answers NATIVE whenever it is set — no correction of any kind, on buttons or axes, for a pad Android already reads. `padMap` computed that fact one line below and only ever spent it on the axes; it now decides both. `hasKeys` stays for the narrower question it can answer — WHICH straight-through order, once the axes have established there is one — where a false positive costs nothing. Moonlight decides it on the same fact (`ControllerHandler`, `gasRange == null` beside the `"Xbox Wireless Controller"` name); yesterday's commit cited its tables and then replaced its discriminator, which is where this came in. Verified: `:kit:testDebugUnitTest` and `:app:testDebugUnitTest` green (16 cases in PadButtonsTest, 3 new: the gate holds for every vendor/declaration combination, the four reported buttons stay themselves, and the report-order choice past the gate is unchanged), `:app:compileDebugKotlin` clean. The DualSense report filed alongside these — Triangle dead in the client UI and in the stream — is NOT explained by this and is not fixed here: a button that reaches neither is one `buttonBit` maps to nothing, which no branch of the correction produces for Triangle. The Controllers screen prints the raw scancode and keycode of every press; that line off the reporter's pad will pin it. --- .../kotlin/io/unom/punktfunk/kit/Gamepad.kt | 75 +++++++++++++------ .../io/unom/punktfunk/kit/PadButtonsTest.kt | 51 +++++++++++++ 2 files changed, 104 insertions(+), 22 deletions(-) diff --git a/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/Gamepad.kt b/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/Gamepad.kt index 7b3cf6e5..893cb1a5 100644 --- a/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/Gamepad.kt +++ b/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/Gamepad.kt @@ -366,15 +366,26 @@ object Gamepad { // is immune to the layout file — the same reason [Keymap.toVk] reads `scanCode` for keyboards. // Two things keep it from breaking a pad that already works: // - // 1. The correction is applied ONLY when the delivered keycode is what `Generic.kl` would - // have said ([genericKeyCode]). A different keycode means a device-specific layout IS in - // force and already knows this pad better than we do, so we leave it alone. - // 2. Which report order to read is decided from what the DEVICE declares, never a model - // table: a pad numbering straight through claims BUTTON_C and BUTTON_Z ([PadButtons]), - // keycodes no real controller has a button for. + // 1. Nothing is corrected on a pad that names its triggers ([padButtons]). A descriptor + // well-formed enough to call them Accelerator/Brake puts its buttons at the standard + // positions too, and that is the fact — not the model — that separates the two firmwares + // of the SAME Xbox pad, only the older of which needs any of this. + // 2. Past that gate the correction still applies ONLY where the delivered keycode is what + // `Generic.kl` would have said ([genericKeyCode]). A different keycode means a + // device-specific layout IS in force and knows this pad better than we do. // - // Moonlight carries the same two tables (`ControllerHandler`'s `isNonStandardDualShock4` / - // `isNonStandardXboxBtController`), which is why both pads work there on the same box. + // Moonlight carries the same two tables AND the same gate (`ControllerHandler`'s + // `isNonStandardDualShock4` / `isNonStandardXboxBtController`, the latter on `gasRange == null`), + // which is why both pads work there on the same box. + // + // The first cut of this asked `hasKeys(BUTTON_C, BUTTON_Z)` on its own, on the reasoning that a + // pad numbering straight through reaches keycodes no controller has a button for. It does — but + // so does every pad that merely DECLARES six buttons, because `hid-input` allocates `BTN_A + n` + // straight through for the whole descriptor whether or not the pad ever presses them. That fired + // the correction on pads Android was already reading correctly (2026-08-21: an Xbox pad + // answering X with Y, Y with LB, and both shoulders with a menu button), and it could not have + // done otherwise: the signal is identical on the firmware that needs correcting and the one that + // does not. Declaration is not report order. Only the axes tell them apart. /** [MotionEvent] axis id meaning "this pad has no such axis" — see [PadMap]. */ const val AXIS_NONE = -1 @@ -526,22 +537,42 @@ object Gamepad { private val padMaps = ConcurrentHashMap() /** - * Which report order [dev]'s buttons follow, asked of the device rather than a model table. + * Which report order [dev]'s buttons follow — [namedTriggers] is whether the pad reports its + * triggers under a name Android knows (see [padMap]), and [declaresCZ] whether it declares + * BUTTON_C and BUTTON_Z. * - * A pad numbering its HID buttons straight through reaches BUTTON_C and BUTTON_Z, keycodes - * that exist only as `Generic.kl` positions — no controller has a physical C or Z button, and - * a pad with a kernel driver behind it emits the modern Linux gamepad codes, which skip both. - * Declaring the pair is therefore the signature of a pad Android is guessing at. + * `namedTriggers` decides it, and a pad that has them is [PadButtons.NATIVE] whatever else it + * says. A HID gamepad describes its triggers either as the Accelerator/Brake usages, which + * become `ABS_GAS`/`ABS_BRAKE` and axis names Android has words for, or as two more generic + * axes on `ABS_Z`/`ABS_RZ`, which it does not — and a report descriptor well-formed enough to + * name its triggers puts its buttons at the standard positions too, the ones `Generic.kl` + * already reads correctly. It is the same fact Moonlight decides this on (`gasRange == null` + * beside the `"Xbox Wireless Controller"` name), and it is the one that separates the two + * firmwares of the SAME pad: an Xbox Wireless Controller over Bluetooth reports GAS/BRAKE + * after its firmware update and Z/Rz before it, and only the older one needs correcting. + * + * `declaresCZ` cannot make that call and must never be asked to. `hasKeys` answers for what a + * device DECLARES, not what it reports: `hid-input` allocates `BTN_A + n` straight through for + * every button in the descriptor, so BTN_C (`0x132`) and BTN_Z (`0x135`) are set on any pad + * declaring six or more — a standard-layout pad that never presses either included. Read alone + * it fired the correction on pads whose buttons were already right, which is how an Xbox pad + * came to answer X with Y and Y with LB (field reports, 2026-08-21). It stays as the narrower + * question it can answer — WHICH straight-through order, once `namedTriggers` has established + * there is one — where a false positive costs nothing. */ - fun padButtons(dev: InputDevice): PadButtons { + fun padButtons(dev: InputDevice, namedTriggers: Boolean): PadButtons { val has = dev.hasKeys(KeyEvent.KEYCODE_BUTTON_C, KeyEvent.KEYCODE_BUTTON_Z, 0) - val straightThrough = has[0] && has[1] - return when { - straightThrough && dev.vendorId == VID_SONY -> PadButtons.GENERIC_SONY - straightThrough -> PadButtons.GENERIC_XBOX - dev.vendorId == VID_SONY -> PadButtons.SONY_MODERN - else -> PadButtons.NATIVE - } + return padButtons(namedTriggers, dev.vendorId == VID_SONY, declaresCZ = has[0] && has[1]) + } + + /** [padButtons]'s choice over plain facts — the seam its truth table is tested at (an + * [InputDevice] cannot be built off a device). */ + fun padButtons(namedTriggers: Boolean, sony: Boolean, declaresCZ: Boolean): PadButtons = when { + namedTriggers -> PadButtons.NATIVE + declaresCZ && sony -> PadButtons.GENERIC_SONY + declaresCZ -> PadButtons.GENERIC_XBOX + sony -> PadButtons.SONY_MODERN + else -> PadButtons.NATIVE } /** @@ -566,11 +597,11 @@ object Gamepad { fun padMap(dev: InputDevice?): PadMap { if (dev == null) return NATIVE_MAP padMaps[dev.descriptor]?.let { return it } - val buttons = padButtons(dev) fun has(a: Int) = axis(dev, a) != null val named = (has(MotionEvent.AXIS_LTRIGGER) && has(MotionEvent.AXIS_RTRIGGER)) || (has(MotionEvent.AXIS_BRAKE) && has(MotionEvent.AXIS_GAS)) || (has(MotionEvent.AXIS_BRAKE) && has(MotionEvent.AXIS_THROTTLE)) + val buttons = padButtons(dev, namedTriggers = named) val rx = axis(dev, MotionEvent.AXIS_RX) val hasRxRy = rx != null && has(MotionEvent.AXIS_RY) // Whichever pair the fallback is about to pick, ask THAT one where it rests. diff --git a/clients/android/kit/src/test/kotlin/io/unom/punktfunk/kit/PadButtonsTest.kt b/clients/android/kit/src/test/kotlin/io/unom/punktfunk/kit/PadButtonsTest.kt index dc73067d..c7fd7c39 100644 --- a/clients/android/kit/src/test/kotlin/io/unom/punktfunk/kit/PadButtonsTest.kt +++ b/clients/android/kit/src/test/kotlin/io/unom/punktfunk/kit/PadButtonsTest.kt @@ -200,4 +200,55 @@ class PadButtonsTest { assertEquals(generic, Gamepad.PadButtons.NATIVE.correct(scan, generic)) } } + + /** + * The regression that made this gate necessary (field reports, 2026-08-21): an Xbox Wireless + * Controller and a GameSir G8+, both with their buttons at the standard positions and both + * corrected anyway, because `hasKeys` says BUTTON_C and BUTTON_Z for any pad that DECLARES six + * buttons — `hid-input` allocates the whole descriptor `BTN_A + n` straight through whether the + * pad ever presses them or not. Naming the triggers is what tells the two apart. + */ + @Test + fun `a pad that names its triggers is never corrected, whatever it declares`() { + for (sony in listOf(false, true)) { + for (declaresCZ in listOf(false, true)) { + assertEquals( + Gamepad.PadButtons.NATIVE, + Gamepad.padButtons(namedTriggers = true, sony = sony, declaresCZ = declaresCZ), + ) + } + } + } + + /** + * The four buttons the field reports named, on a pad whose report order is already standard: + * X answering Y, Y answering LB, and both shoulders answering a menu button. NATIVE is what + * keeps them themselves — the correction tables are right for the pads they are for, and this + * is about not reaching one of them. + */ + @Test + fun `an Xbox pad at the standard positions keeps X, Y and its shoulders`() { + val native = Gamepad.PadButtons.NATIVE + assertEquals(KeyEvent.KEYCODE_BUTTON_X, native.correct(0x133, KeyEvent.KEYCODE_BUTTON_X)) + assertEquals(KeyEvent.KEYCODE_BUTTON_Y, native.correct(0x134, KeyEvent.KEYCODE_BUTTON_Y)) + assertEquals(KeyEvent.KEYCODE_BUTTON_L1, native.correct(0x136, KeyEvent.KEYCODE_BUTTON_L1)) + assertEquals(KeyEvent.KEYCODE_BUTTON_R1, native.correct(0x137, KeyEvent.KEYCODE_BUTTON_R1)) + // What the old heuristic did to each of them, kept here so the difference stays visible. + val wrong = Gamepad.PadButtons.GENERIC_XBOX + assertEquals(KeyEvent.KEYCODE_BUTTON_Y, wrong.correct(0x133, KeyEvent.KEYCODE_BUTTON_X)) + assertEquals(KeyEvent.KEYCODE_BUTTON_L1, wrong.correct(0x134, KeyEvent.KEYCODE_BUTTON_Y)) + assertEquals(KeyEvent.KEYCODE_BUTTON_SELECT, wrong.correct(0x136, KeyEvent.KEYCODE_BUTTON_L1)) + assertEquals(KeyEvent.KEYCODE_BUTTON_START, wrong.correct(0x137, KeyEvent.KEYCODE_BUTTON_R1)) + } + + /** Past the gate, which straight-through order to read is still the question it always was. */ + @Test + fun `an unnamed-trigger pad still resolves its report order`() { + fun order(sony: Boolean, declaresCZ: Boolean) = + Gamepad.padButtons(namedTriggers = false, sony = sony, declaresCZ = declaresCZ) + assertEquals(Gamepad.PadButtons.GENERIC_SONY, order(sony = true, declaresCZ = true)) + assertEquals(Gamepad.PadButtons.GENERIC_XBOX, order(sony = false, declaresCZ = true)) + assertEquals(Gamepad.PadButtons.SONY_MODERN, order(sony = true, declaresCZ = false)) + assertEquals(Gamepad.PadButtons.NATIVE, order(sony = false, declaresCZ = false)) + } } -- 2.54.0