Release 1.4.2: UHID HID input, overlay cursor, and pointer compensation - #44
Merged
Merged
Conversation
The DEX was packaged with no app launcher; keep HID work on experiment/uhid-system-cursor instead of shipping a second unused stack. Co-authored-by: Cursor <cursoragent@cursor.com>
Handshake logs go through Log.* with a JVM-safe wrapper; connection state stays a small class next to the service. Co-authored-by: Cursor <cursoragent@cursor.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @anasvhora284, your pull request is larger than the review limit of 150,000 diff characters
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
… rich Markdown changelog - Native UHID keyboard and mouse attach on Enter and disconnect on Leave - Live mid-session DINF screen size updates and native pointer compensation/edge anchoring - Server cards now reflect real-time connecting states - In-house lightweight Markdown parser and renderer for update changelogs - Fixed accessibility overlay cursor visibility without SYSTEM_ALERT_WINDOW-only gate - Fixed first-enter freeze and Shizuku bind/reconnection lifecycle - Fixed raw Markdown syntax rendering in the Update Available dialog - Added MarkdownContent component and 14 unit test cases in MarkdownParserTest
anasvhora284
force-pushed
the
release/1.4.2
branch
from
September 18, 2026 05:43
a461e70 to
7cfb0ca
Compare
This was referenced Sep 18, 2026
Closed
…nt for 100% Codecov
guaje
self-requested a review
September 21, 2026 12:53
guaje
reviewed
Sep 21, 2026
guaje
left a comment
guaje
left a comment
Collaborator
There was a problem hiding this comment.
Review of the 1.4.2 UHID subsystem and changelog renderer — 5 findings, anchored inline.
UhidProtocol.kt:20— UHID event-type constants diverge from the kernel ABI; the readiness wait happens to work on the wrong-but-adjacent kernel signal, and the unit tests cannot catch it (issue)MarkdownContent.kt:169— Markdown-link URLs from the remote release body bypass the http(s) restriction applied to raw URLs (issue, security)ConnectionService.kt:588— retry-delay logic inlined into a Service method and its boundary tests deleted; the patch gate now passes because the logic is no longer measured (issue)ShizukuInputInjector.kt:170— phantom UHID keyboard/mouse can persist after binder death because kernel cleanup relies on process-kill timing (issue)ShizukuInputInjector.kt:371— wheel-notch truncation plus three smaller low-severity notes (nitpick)
For balance: the UHID_CREATE2 byte layout was checked field-by-field against uhid.h and is exact (incl. rd_size u16 at 260), the module removal and CI teardown are coherent, and the coverage gates were not relaxed.
compose-ui-test-junit4 pulls Espresso 3.5.0 transitively, and nothing in the build declared Espresso at all. 3.5.0 reflects on InputManager.getInstance(), which Android 14 removed, so InputManagerEventInjectionStrategy.initialize() throws NoSuchMethodException and every Compose UI test dies inside Espresso.onIdle before reaching an assertion. On a Pixel-class API 36 device both MainActivitySmokeTest cases failed this way. 3.6.1 does not fix it -- verified by disassembling the artifact, it still calls getInstance() with no fallback. 3.7.0 routes through getInputManager() instead. With this pin the full instrumentation suite passes 13/13, which also means the MainActivitySmokeTest assertions for the two-page onboarding flow are covered for the first time; they had been updated but never actually executed. Test-scope only: no app code and no production dependency is affected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PR #44 review finding. Changelog text is fetched from the GitHub release API, and the Markdown-link branch of parseInline passed the URL straight into LinkAnnotation.Url with no scheme validation -- while the raw-URL branch twenty lines below was already restricted to http(s). The default AndroidUriHandler dispatches whatever it is through ACTION_VIEW, so a tampered, typosquatted or compromised release body containing [label](intent://...) turned one tap in the update dialog into an arbitrary implicit intent. Rather than repeat the check, both link branches now go through a single MarkdownParser.isDispatchableUrl gate, since the cause of the bug was two branches with one policy between them. It fails closed: odd casing or leading whitespace renders as plain text rather than being normalised and dispatched. The @mention branch needed no change -- it builds a hardcoded https://github.com/ prefix and the regex limits the username charset. Tests assert no dispatchable annotation survives for intent/javascript/file/ content/market schemes, that http(s) still linkifies, and that the check fails closed on casing and padding. Verified by mutation: replacing the gate with `if (true)` fails all three. Note for reviewers: a URL containing parens, e.g. javascript:alert(1), leaves a stray ')' in the rendered text, because INLINE_TOKEN_REGEX ends the URL at the first ')'. That is a pre-existing cosmetic quirk of the regex and not a hole in the scheme check -- the link is still suppressed. It is asserted separately so the security test does not depend on it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PR #44 review finding. The constants did not match uapi/linux/uhid.h: START was 4 and OPEN was 6, which are really the kernel's OPEN and OUTPUT. So the reader ignored the real START(2), consumed the real OPEN(4) under the name "START", and its "OPEN" branch watched for OUTPUT(6) -- a host->device report -- and therefore never fired. The tests injected the same wrong constants back into fake streams, so CI proved the file agreed with itself rather than with the kernel. Wire behaviour is deliberately unchanged. The gate was already effectively kernel OPEN(4), and that is the correct gate: START only means hid-core created the device, which is before EventHub attaches, so writing INPUT2 on START would race the very drop the readiness wait exists to prevent. The fix names what was already happening rather than re-pointing the wait at START(2), which would have been a real regression dressed up as a correction. - Constants now carry the kernel's values, plus STOP/CLOSE/OUTPUT/GET_REPORT/ SET_REPORT, so the read path can name every event it may see. - awaitReady waits on OPEN then probes sysfs. The old second window also accepted "OPEN"(=OUTPUT), which for a keyboard is really an LED report; dropping that costs at most presenceTimeoutMs in a rare case and no longer conflates a host report with readiness. - Config renamed startTimeoutMs -> openTimeoutMs, and openGraceMs removed: it only existed to bound a wait for an event that could never arrive. - payloadSize corrected. OUTPUT was treated as zero-payload when it carries data[4096] + u16 size + u8 rtype, so draining one would have left 4099 bytes in the stream and misframed every later read. Only waitForStart consumes this, and that function currently has no production caller -- worth deciding whether to keep it, but it should not carry a wrong table meanwhile. - Tests assert the literal kernel numbers, not the constants, so they guard the ABI instead of restating it. Verified by mutation: restoring START=4/OPEN=6 fails both new tests, and the readiness tests still pass unchanged because the wire value on that path was always 4. 410 unit tests green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…view notes PR #44 review findings #3 and #5. #3 -- RetryDelayCalculator restored. Its lookup had been inlined into ConnectionService.scheduleRetry and its boundary tests (attempts 0/1/2/3/4/5/10 and negative) deleted with it. Logic inside a Service method is unreachable from the JVM suite, so the exact delay per attempt was asserted nowhere and the patch gate passed because the code was no longer measured. Tests restored verbatim. The clamp is now a single coerceIn rather than a branch plus a minOf. #5 -- wheel notches no longer truncate. `yDelta / 120` discarded anything finer than a notch, so a 60-unit delta scrolled nothing while the accessibility branch used float division. Input Leap normalises to 120 units, so only non-conforming servers lost input, silently. The remainder is now banked. Extracted as WheelNotchAccumulator rather than kept inline, for the same reason as #3: ShizukuInputInjector needs a bound AIDL service to exercise, so anything inline there is untestable. Covered for sub-notch banking, negative symmetry, cancellation, multi-notch deltas, reset across attach, and no drift over 1000 conforming events. A partial notch is reset on detach and disconnect so it cannot leak into the next session, and a banked delta still counts as handled so it is not replayed by the fallback path. Review notes: 1. ConnectionService.screenWidth/screenHeight are now @volatile. They are written on the main thread (rotation/DINF) and read from the IO event loop; the HID path is published via HidMouseState.resizeDisplay, but the fallback touch path read them directly and could clamp against stale bounds. 2. MousePointerCompensation.gainForSpeedMmPerS no longer carries an unreachable fallback: the curve is +Inf-terminated, so a segment always matches. 3. UhidChannelReadinessTest no longer races wall-clock timing. The kernel event is written into the pipe up front instead of from a sleeping feeder thread, and the deadline maths runs on a virtual clock through the sleeper/nanoTime seams, so no assertion depends on machine speed. Confirmed stable over five consecutive clean reruns. Two tests added there while the seams were in place: that a missing OPEN degrades within its bounded window rather than hanging (the ColorOS path), and that a kernel START alone is not treated as readiness. 423 unit tests green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PR #44 review finding #4. Nothing wrote UHID_DESTROY when the client went away: the teardown in disconnect() runs in the client, so by the time it fires its closeVirtualKeyboard()/closeVirtualMouse() calls throw DeadObjectException and the runCatching wrappers swallow them. Cleanup then depended entirely on the injector process being reaped, and while that lingers -- reportedly a while on some OEM Shizuku builds -- the devices stay attached and Android goes on believing a physical keyboard is present, keeping the soft keyboard suppressed. The injector now watches the client: attachClient(IBinder) links to death on a Binder owned by the client process and closes both UhidChannels from the death callback, inside the process that owns the /dev/uhid fds. A token that is already dead throws at linkToDeath instead of calling back, so that case tears down inline. Registration is best-effort on the client side: an injector that cannot watch still works, it just falls back to process reaping as before. SERVICE_VERSION 4 -> 5, because a cached v4 UserService from a previous install does not implement attachClient and would throw on every bind. Also adds the openChannel seam to InputInjectorService so the UHID lifecycle can be exercised without a real /dev/uhid; the death path was otherwise untestable. Covered for: DESTROY written for an attached mouse, no device left open afterwards, idempotent death with nothing attached, a null token ignored, and re-attach replacing the previous watch. Verified by mutation -- emptying the death callback fails three of the five. Scope, honestly: this closes the client-death leak, which is reachable by simply force-stopping the app while HID is attached. It does not fully close the case the review described, where the Shizuku *server* dies while the app lives: the client binder is still alive then, so this watch does not fire, and detecting that from inside the injector needs a separate signal. That part is unverified without a device and is left open deliberately rather than claimed as fixed. 428 unit tests green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ocess Crossing onto the phone left the cursor stranded: it flashed at display centre and then behaved as though centre were the entry point, so every later move was offset by that delta for the rest of the session. Cause: every Leave destroys the HID mouse and every Enter recreates it, so AOSP seeds a brand new pointer at display centre each time. The warp was then sent from the app process, back across Binder. AOSP initialises the sprite asynchronously, and when that landed after the warp it overwrote it -- leaving the native pointer at centre while hidMouse.cooked believed it was at the entry point. The two disagreed permanently. The Enter coords are now handed to the injector (onHidMouseEnter/onHidMouseLeave) and HidMouseEnterWarp applies the snap there, immediately after UHID OPEN, in the process that owns the /dev/uhid fd. No Binder hop, so nothing can lose the race. openVirtualMouse reports whether it actually warped (consumeEnterWarpApplied) rather than the client inferring it: the idempotent "already open" branch emits no INPUT2, and guessing from app state would make the client skip a snap that never happened. The client advances its cooked model without replaying the reports (sendHid=false), so the movement is not applied twice. SERVICE_VERSION 5 -> 6 for the AIDL additions. This is a Shizuku-path fix. It was written on feature/root-injection only because that is where the work happened -- nothing here touches libsu, PrivilegeKind or PrivilegedUserServiceHost -- and without it this release ships the stranded cursor. Verified on a OnePlus Nord 4 (Android 16, Shizuku): daemon logs "applied after UHID ready hidReports=2", client logs "daemonWarped=true ... sendHid=false", and the cursor lands where it entered. Known remaining: the pointer is still briefly visible at centre before the warp lands, because AOSP draws the sprite on device creation and the warp is one frame behind. Removing that entirely means not destroying the device on every Leave, which is a separate design change. 439 unit tests green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e is ready Two write-path defects in UhidChannel, both of which delayed or lost the first reports after CREATE2. The readiness reader was never stopped. It stayed blocked in read() on the same /dev/uhid handle used for writing, so INPUT2 and DESTROY could serialise behind a pending read. stopReader now closes a dup'd ParcelFileDescriptor, which is what actually unblocks read(): a FileInputStream built from a PFD does not own the fd, so closing the stream alone is a no-op for the kernel object. If dup fails there is nothing we could close later, so the reader is not started at all rather than left unkillable -- readiness degrades to the presence probe. Reports sent before the device was ready went straight out. AOSP seeds a new pointer at display centre while it enumerates the device, and a report that lands before the seed is simply overwritten -- the app believes the pointer moved, the sprite never did, and every later relative delta is computed from the wrong origin. Reports are now queued until readiness completes and flushed in one go, so they land after the seed rather than before it. CREATE2 also takes the write lock now, so device creation cannot interleave with a report already in flight. Measured on a OnePlus Nord 4 (Android 16, Shizuku): this took Enter from never landing to landing roughly 3 times in 10. It is a real fix and a necessary one, but it is not sufficient on its own -- winning that race reliably needs the device to stop being destroyed on every Leave, which the next commit does. 439 unit tests green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Crossing onto the phone was unreliable: the pointer appeared at display centre and then behaved as though centre were the entry point, so every movement was offset for the rest of the session. Every Leave destroyed the HID mouse and every Enter created a new one, and AOSP seeds a new pointer at display centre each time. Everything before this tried to warp away from that seed before it was noticed, which is a race against AOSP's own asynchronous initialisation -- and one that cannot be won reliably, because nothing signals that the seed has landed. Three separate attempts at winning it got as far as 3 successes in 10. So stop causing it. The mouse now stays registered across a Leave and is only destroyed after the cursor has been away for 30s. A normal crossing creates no device, so there is no seed and no race: the pointer does not move while the cursor is away, the position model stays true, and Enter is an ordinary delta from a known position. Logs confirm it -- phase=ATTACHED and centerSeed=false on every crossing, snapping from the real last position rather than from centre. The keyboard still detaches immediately. While a HID keyboard is registered Android believes a physical keyboard is present and keeps the soft keyboard suppressed, and unlike the mouse it has no respawn problem. The idle timer is cancelled by cancelLeaveDebounce, which Enter already calls, so rapid back-and-forth never trips it. Known remaining: pointer feel is not yet 1:1 under acceleration. That is the compensation model, not this path, and is being investigated separately. 439 unit tests green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
Author
|
@guaje I have made the changes according to your findings and some bugs i encountered |
guaje
approved these changes
Sep 22, 2026
Collaborator
There was a problem hiding this comment.
Hi @anasvhora284,
Approved! I ran a second pass over the fix round (this time reading the full reader lifecycle and the new death-watch logic, and double-checking the UHID stuff against the kernel source), and it holds up. All five findings from the first review are properly fixed — the kernel-pinned ABI tests and the restored RetryDelayCalculator are especially nice. CI is green across the board.
One small thing you might want to fold in before tagging 1.4.2 (it's a one-word change, and it closes a potential crash): in ShizukuInputInjector.onHidMouseEnter, the generic catch runs handleRemoteException() and then falls through into finishPendingSnap(svc) using the binder it just declared dead. If injectHidMouse throws DeadObjectException there, nothing catches it — it escapes into the event-loop collector in ConnectionService, which has no catch either, so it reaches the uncaught handler and takes the app down. The DeadObjectException branch in the same handler already returns; doing the same after handleRemoteException() would make this path safe. If you fold it in, I won't file a follow-up issue.
On the readiness gate — I dug into it a bit since presence got moved behind OPEN, and I think it's worth a look soon, but not worth touching right before the tag:
- From
drivers/hid/uhid.c:UHID_STARTis queued during probe (uhid_hid_start), so it's guaranteed whenever the device is bound.UHID_OPENonly gets queued when a userspace process actually opens the device — stock InputReader does open scanned devices, so on AOSP the OPEN arrives, but nothing in the kernel guarantees an opener exists, and OEM ROMs with input filters / HID security policies are exactly this app's territory. On such a device, the gate waits for an event that never comes and burns the fullopenTimeoutMson every attach. - The old ColorOS comment turned out to be an artifact of the wrong constants: what the old code kept seeing was the real OPEN (it was mislabeled "START"), and the event it "never saw" was value 6 =
UHID_OUTPUT, which never arrives on any device because it's host→device. So we don't have evidence of an OPEN-less device, but we also don't have evidence that every device emits it, and the kernel allows OPEN-less operation, which is what matters for us. - Small thing I noticed: the reader already tracks START (
UhidChannel.kt:220, and there's even a log branch for it), but the loop at:169only exits onsawOpen. And the sysfs presence check now runs after OPEN (:173-178), so when OPEN arrives on time we still pay up topresenceTimeoutMs— where before, whichever signal came first won.
Since createDevice() proceeds either way, this is extra attach latency, not breakage — which is why I'd rather not rush a change to every device's attach path before the tag. Exiting on sawStart || sawOpen and demoting presence to a logged confirmation (plus virtual-clock tests for both exits through the existing seams) should cover it, I think.
Thanks for the quick turnaround on all of this 🙌
Is it possible to reflect the mobile screen size in real-time—specifically the folded state—similar to Samsung's Multi Control?
|
A DeadObjectException from the enter snap escaped the event loop and crashed the process. Co-authored-by: Cursor <cursoragent@cursor.com>
Owner
Author
|
@rlqnwkd This ships with the 1.4.2 release. When the phone switches between the cover display and the inner display, Input Leaf sends the new screen size to the server on the open connection. A half-fold on the same inner screen does not change the reported size, so that posture stays as it is. |
This was referenced Oct 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Release 1.4.2 represents a major architectural and functional upgrade for Input Leaf. It introduces native kernel-level UHID hardware keyboard and mouse emulation via Shizuku, dynamic Android system pointer speed compensation, mid-session display rotation handling, rich in-app Markdown release notes, and accessibility overlay reliability improvements, while completely removing legacy daemon sidecars.
Features Added
Native Hardware Keyboard & Mouse Emulation (UHID via Shizuku)
System Pointer Speed Synchronization & Sub-Pixel Tracking
Settings.System.POINTER_SPEED, range-7to+7).Mid-Session Dynamic Display Rotation (
DINF)Rich Markdown Changelog in Update Dialog
MarkdownContent.kt) that renders GitHub release notes with full support for headings, bullet/numbered lists, bold/italic styling, inline code, and clickable URLs.Accessibility & Connection UX Enhancements
SYSTEM_ALERT_WINDOWas an exclusive blocker.Technical & Architectural Improvements
/dev/uhidCommunication: Completely removed the standaloneuhid-serverJava DEX sidecar daemon and unframed local socket IPC. Input Leaf now writes standard Linux kerneluhid_eventstructs directly into/dev/uhidvia Shizuku (UhidChannel.kt,UhidProtocol.kt).EvdevToHid.ktand standard USB HID keyboard reports (HidKeyboard.kt), providing 1:1 hardware key injection.HidMouseState.kt(delta accumulation),MouseEdgeAnchor.kt(edge snapping and re-entry),MousePointerCompensation.kt(OEM curve compensation including OnePlus/OPPO offsets), andHidAttachmentController.kt.uhid_mouse_backup.patch), and stale documentation (DEVELOPMENT_JOURNEY.md,UI_REDESIGN.md).UhidProtocolTest,HidKeyboardTest,HidMouseTest,MouseEdgeAnchorTest,MousePointerCompensationTest,PointerSpeedTest, andMarkdownParserTest.Issues Closed & Resolved
Settings.System.POINTER_SPEED, range-7to+7) with active compensation curves and fractional sub-pixel accumulation inHidMouseState.uhid-serverJava DEX sidecar daemon and local socket IPC in favor of direct/dev/uhidvia Shizuku.Settings → System → Physical keyboard → Gboard), allowing Android to compose Gujarati, Indic, and other language layouts natively.(Note: #18, #28, and #34 are resolved in Shizuku / UHID mode where a real kernel device is recognized by Android. Pure Accessibility fallback remains touch-drag and InputLeafIME-based).
Test Plan & Verification