Skip to content

Fixes and small ports from the fork review - #75

Open
trexx wants to merge 6 commits into
mainfrom
fix/fork-review-fixes
Open

trexx wants to merge 6 commits into
mainfrom
fix/fork-review-fixes

Conversation

@trexx

@trexx trexx commented Oct 25, 2026

Copy link
Copy Markdown
Owner

Fixes and small ports found while comparing moonlight-trexx against upstream moonlight-android, Artemis, moonlight-noir, nightjar, Starwire, moonlight-vplus and Axixi2233. One commit per change.

Commit What Found by
Retry a controller arrival the host could not take yet A pad that reports before the stream is up (any USB Pro Controller plugged in before launch) had its arrival dropped with -2 and never resent, so the host built a default pad with no gyro, touchpad, LED or real type. Now retried from the input path, ahead of the next gamepad packet. Axixi2233, moonlight-vplus
Restore upstream's Android 12/13 controller sensor and LED guards getSensorManager() NPE on API 31/32; hasRgbControl() lying on API 31-33. Reaches neither box in hand; kept for other Android TV boxes. Upstream audit
Fail malformed host XML as a parse error instead of crashing Four unchecked exceptions in NvHTTP that escaped every handler. Starwire, moonlight-vplus
Bound the keyboard layout table against out-of-range keycodes AIOOBE in Game.onCreate() from a vendor keyboard layout (API 33+, Homatics). moonlight-vplus
Keep the codec exception as RendererException's cause Crash reports now show which MediaCodec call threw. moonlight-vplus
Claim GameSir's 0x3537 pads in the Xbox USB drivers Checked against Linux xpad.c; the forks' extra init packet and 0x2f24 GIP entry are not taken. Artemis, vplus, Axixi2233

Left out on purpose

  • OkHttp fastFallback(false): excluded as requested.
  • Upstream's minisdl.c sync (c2e224ebf): its Elite and DualSense Edge detection resolves to exactly the IDs trexx already lists, so the only behavioural change would be upstream's default-yes share-button policy, which minisdl.c deliberately declines.

Latency: one boolean read per controller report on the input path (arrival retry); everything else is setup, polling or failure-path only. No allocation added anywhere.

Verification: assembleRelease lintRelease testDebugUnitTest pass locally (314 tests; lint reports two warnings, both pre-existing in files this PR doesn't touch). None of the changed code can load on the JVM (ControllerHandler, KeyboardTranslator and NvHTTP's XML parser need Android), so there are no new unit tests; each commit says why. Not run on hardware: HARDWARE_TESTING.md §33 has the checks for the arrival fix, and the hardware table names the box the API 31-33 guards need.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ShRgaYwZx3XGmFkZvjJaeY

trexx and others added 6 commits October 25, 2026 17:02
Both were removed on the grounds that neither box in hand runs API 31-33. The
target is any Android TV box at API 30+, and both guard real platform bugs on
the versions in between:

- InputDevice.getSensorManager() creates InputDeviceSensorManager, which on
  Android 12 (API 31/32) NPEs on a background thread through bad error checking
  in its InputListener callbacks. Upstream only touches it there for Sony and
  Nintendo pads; so does this again.
- Light.hasRgbControl() returned true for every light before API 34, because
  LIGHT_CAPABILITY_RGB was 0. On API 31-33 every pad was reported to the host
  as having an RGB LED. Upstream's guess - PlayStation pads only - is restored,
  at detection time as upstream now does it, so handleSetControllerLED() obeys
  the same answer as the arrival.

Neither box can exercise either branch: the Shield is API 30 and never reaches
the API 31 gate, the Homatics is API 34 and passes both. HARDWARE_TESTING.md's
hardware table names the box that would.

Latency: none. Both run once per controller at enumeration.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ShRgaYwZx3XGmFkZvjJaeY
A controller is assigned on its first input, and the USB driver service starts
from Game.onCreate(), long before the connection. A Pro Controller plugged in
before launch reports ~120 times a second from the moment it is claimed, so its
arrival went out while the input stream did not exist: LiSendControllerArrival-
Event() returned -2, the result was ignored, and with the pad already marked
assigned nothing sent it again. The host built a default pad for the session -
no gyro, touchpad, LED or real type - which is the likely reason HARDWARE_TESTING
section 2's motion items have never passed. Found independently by Axixi2233 and
moonlight-vplus; upstream Moonlight has the same bug.

The arrival now reports whether the host took it, and a context left pending is
retried from assignControllerNumberIfNeeded() on its next input. That keeps the
ordering the host depends on: the arrival goes out on the same thread directly
before the gamepad packet that follows it, so a packet cannot reach Sunshine
first and lock in the default type, which a resend from connectionStarted()
could not guarantee. The battery monitor now starts only after a successful
arrival, since its first event would otherwise be recorded as reported while the
host never saw it.

Instrumented with one log line on each transition (deferred, then sent after
retry) - not per attempt, which would be ~120 lines a second until the stream is
up. Section 33 lists what to check on hardware.

Latency: one boolean read per controller report on the input path
(ProConController.handleRead() at ~120 Hz reaches it through
sendControllerInputPacket()). The retry itself only runs before the stream is
up. No allocation.

Not JVM-testable: ControllerHandler needs InputManager and a live connection,
and the logic is a single pending flag.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ShRgaYwZx3XGmFkZvjJaeY
Every caller of getXmlString() handles IOException and XmlPullParserException,
which is how a host returning garbage is meant to surface. Four places threw
unchecked exceptions instead, which went past all of them and crashed the
poller - and the poller talks to anything answering on a host's port on the LAN:

- text before the first tag: currentTag.peek() on an empty stack
- a <root> without status_code: Long.parseLong(null)
- a non-numeric currentgame or HttpsPort: Integer.parseInt()
- a non-numeric ServerCodecModeSupport: Long.parseLong()

Each now throws XmlPullParserException. getHttpsPort() already falls back to the
default port on that, so a bad port value now degrades instead of crashing. The
empty-stack guard is the one the app-list parser already carries. From
Starwire's and moonlight-vplus's NvHTTP hardening; stock Sunshine always sends
well-formed replies, so this is robustness against other software on the LAN.

Latency: none; setup and polling only.

Not JVM-tested: getXmlString() uses XmlPullParserFactory, which is a throwing
stub in the test android.jar, and making it reachable would mean adding a kxml
test dependency for four guards.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ShRgaYwZx3XGmFkZvjJaeY
KeyboardMapping sizes its table to KeyEvent.getMaxKeyCode() and then writes at
whatever keycode the layout returns for each location. A vendor keyboard layout
can return one above that, which threw ArrayIndexOutOfBoundsException from
Game.onCreate(). The read side already treats such a keycode as unknown, so it
is now skipped on the write side too and nothing lookup-visible changes. From
moonlight-vplus.

The mapping is API 33+, so this reaches the Homatics only.

Latency: none; built once per keyboard at stream start.

Not JVM-tested: KeyboardTranslator pulls in InputManager and cannot load on the
JVM, and the change is one bound.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ShRgaYwZx3XGmFkZvjJaeY
RendererException formats the codec's diagnostics into its text but dropped the
exception itself, so a crash report's stack ended at the RendererException
constructor and never named the MediaCodec call that threw. It now chains it as
the cause; the text is unchanged. From moonlight-vplus.

Latency: none; only on the failure path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ShRgaYwZx3XGmFkZvjJaeY
GameSir's newer pads use vendor 0x3537, which neither driver listed, so they
fell to Android's generic handling instead of the USB drivers - per moonlight-vplus
a G7 Pro then works but has no rumble. Artemis, moonlight-vplus and Axixi2233
all add it.

Checked against Linux xpad.c rather than taken from the forks: it lists 0x3537
under both XPAD_XBOX360_VENDOR and XPAD_XBOXONE_VENDOR (Nova Lite and T4 Kaleid
as XInput, G7 SE as GIP) and has no GameSir-specific init packets, so the GIP
pads get the wildcard init sequence like any other. Artemis's extra
ONE_S_INIT entry for 0x3537:0x1012 is not taken; neither xpad nor xone carries
it. The forks' 0x2f24 entry for the GIP driver is not taken either: xpad lists
0x2f24 as XInput only, which the 360 driver already claims.

Latency: none; checked once when a USB device attaches. Not verified with a pad.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ShRgaYwZx3XGmFkZvjJaeY
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant