Repository navigation
feat(input): route ROLI touch and button input from blocksd - #323
Merged
Merged
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing (3)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (36)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThis change adds driver-fed device touch and button input. It defines input events and per-device leases, tracks and routes device state, connects daemon demand to device input, and adds a ROLI Blocks event-stream producer. ChangesDevice Input Pipeline
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~60 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant BlocksBackend
participant BlocksInput
participant blocksdEventSocket
participant DeviceInputPublisher
BlocksBackend->>BlocksInput: notify device connection
BlocksInput->>DeviceInputPublisher: attach device lease
BlocksInput->>blocksdEventSocket: subscribe to device, touch, and button events
blocksdEventSocket->>BlocksInput: deliver event lines
BlocksInput->>DeviceInputPublisher: publish converted input edges
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 54.40% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 250 functions across 35 files. (1 skipped… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly and concisely identifies the main change: routing ROLI touch and button input from blocksd. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Docstring Coverage
Explanation
Docstring coverage is 54.40% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 250 functions across 35 files. (1 skipped: 1 unsupported.)
- Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
- Commit to this branch
- Create a new PR
- Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Hardware that Hypercolor lights can also be played, but nothing in the input pipeline could say which device a touch or button came from. The shared vocabulary now has a driver-facing DeviceInputEdge (touch began, moved, ended, and named buttons, in normalized device coordinates) and two routed InputEvent variants, Touch and DeviceButton, that carry the DeviceId. Touch positions use Q16.16 like PointerScroll so InputEvent keeps deriving Eq, and only begin, end, and cancel are events; movement will be held state, as pointer motion already is. Drivers reach the pipeline through a new DriverHost::device_input() capability returning a DeviceInputSink. attach() hands back a per-device publisher lease. Hosts without device input return None, so existing drivers and test hosts are unaffected. Until device sources exist, effect payloads drop the new events the way they drop MIDI, and synthetic releases cancel touches and release device buttons. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Driver-owned devices need a place to publish input that is outside the InputManager (which allows one host interaction source) and outside the host capture consent gate. DeviceInputHandle follows the browser preview registry: one routable child publication per attached DeviceId, so a device's lifetime is one source incarnation and a busy surface cannot evict another device's edges from a shared ring. Each child always tracks what is physically held but only announces a press while some consumer demands interaction. Announced touches publish begin and end edges, with movement folded into held state on the new InteractionData.device field. Losing demand cancels every announced hold with explicit Cancelled and Released edges, and a press that began while undemanded stays invisible until it lifts. The router therefore never sees a hold or release it did not observe being pressed, which would otherwise leave stale quarantine entries that swallow a later tap. Attaching a device again supersedes the earlier lease with an incarnation-fenced close, so a stale lease can never retire its successor, and DeviceInputPublisher::publish now reports supersession. Positions are clamped, a non-finite begin is dropped and counted, and an end is never dropped: a bad final position falls back to the last good one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Device registry children now join the interaction route catalog as a Device source class in their own incarnation namespace, listed after host and browser sources. Device sources are selected under the Host and Merge policies and never under Browser, so physical device input behaves like physical host input: the daemon consumer sees it by default and previews stay isolated unless a user chooses Merge. Touches and device buttons become held controls keyed by DeviceId, so contacts on different surfaces never alias. event_control maps touch begin to pressed and end or cancel to released with explicit arms in place of the old fall-through, and snapshot holds, quarantine, and held output extend to both. Held positions always come from the latest snapshot. Rebuilding device held output stays allocation-free: touch lookups use a Copy key, button lookups scan, and output vectors are refilled in place. The allocation contract test now holds a device touch and button through every measured frame. Input availability counts only host sources, so an attached Lightpad cannot make a keyboard effect report input as available when host capture is off. The status API likewise no longer counts the device backend as host capture. The daemon's routing control owns the device registry and passes its snapshot to every catalog refresh. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
DaemonDriverHost now answers DriverHost::device_input() with the device registry owned by the interaction routing control, so drivers built at startup, reconcile, or rebind publish into the same registry the render thread routes. The input publication pump mirrors the interaction bit of its aggregate capture demand into that registry at the top of every iteration, ahead of the screen transition and capture reconcile steps that can retry on Busy or Stale, and clears it when the pump stops. The demand is the union of the authoritative render, preview, and input_events WebSocket consumers. Device input therefore flows exactly while something wants interaction, whether or not host keyboard capture is enabled, and nothing reaches the bus otherwise. InputPublicationPump::start takes the optional device handle directly; tests pass None. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Blocks backend now subscribes to blocksd's device, touch, and button events on a second connection that only writes its subscribe request. Events never share the frame connection, which pairs each request with the next line or byte it reads and would misread an interleaved event as a frame acknowledgement. The builtin factory hands the host's device input sink to the backend. connect() attaches a publisher lease under the host's canonical DeviceId for the adopted uid and disconnect() drops it, so blocks the host never adopted are ignored and no uid is re-minted into an id. That replaces the unread uid_map. blocksd touch start, move, and end become device touch edges with the touch index as the contact, and buttons use blocksd's SDK function name, then the protocol index. Anything that could hide a lift supersedes leases so held input cancels: device_removed for that device, and an undecodable line or a lost connection for every device. device_added payloads are ignored, so schema drift there cancels nothing. After a lost connection the stream resubscribes with backoff from 250 ms to 5 s without cancelling again while blocksd stays unreachable. The stream runs only while a device is connected, the last disconnect closes it, and dropping the backend aborts it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Records the device input design as built: the driver-api lease seam, the demand-gated registry that only announces presses it can end, Device route selection under Host and Merge, allocation-free held output, and the blocksd event stream. It also names the known limits and the open LUMI keybed mapping question, and lays out phase 2 (effects) and phase 3 (more producers). A fresh-context design review shaped the lease fencing, the announced hold model, and resolving Blocks devices from the adopted set. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The pump cleared device input demand only after its worker loop exited normally. When shutdown passed its one second deadline and aborted the supervisor, or the worker panicked, that reset never ran and device input stayed demanded with no pump left to follow it, contrary to spec 82 section 6. A DeviceInputDemand guard now owns the handle for the worker's lifetime and withdraws demand on drop, so a clean exit, an abort, and an unwinding panic all clear it. The clean exit still drops it before releasing host capture, preserving the previous ordering. A new test spawns the worker directly, aborts it while interaction is demanded, and checks demand is withdrawn. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Aborting the Blocks event stream only takes effect at the task's next await. A stream that had already decoded an event, or was about to supersede leases after losing its connection, could therefore run on after a fast disconnect and reconnect, find the reconnected uid in the shared state, and publish into (or reattach) the new stream's lease. The host's lease supersession cannot catch this, because the stale task reaches the new lease through the map rather than holding an old one. Each stream now carries a generation that every start and abort bumps under the input state lock, and every state access the stream makes goes through a check of that generation under the same lock. A superseded stream drops what it decoded and exits. Dropping the backend bumps the generation as well, so a stream that outlives its backend cannot supersede a lease a replacement backend attached for the same device. Unit tests hold an aborted stream across a disconnect and reconnect, and across a backend drop, and confirm it publishes and reattaches nothing while a replacement stream still delivers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Blocks event reader framed lines with read_line, so a peer that sent bytes without a newline grew the line buffer without limit. Lines are now framed by hand over the buffered reader and capped at 64 KiB. The largest event on the subscribed categories is device_added at a few hundred bytes, so the cap leaves two orders of magnitude for payload growth. A longer line is discarded through its newline without being buffered, warned about once per connection, and treated like an undecodable line: every lease is superseded, since the skipped line could have been a touch end, and reading continues on the next line. Lines are now decoded from bytes, so invalid UTF-8 also counts as an undecodable line instead of failing the read and reconnecting. A new fake-blocksd test sends a valid touch padded to twice the cap followed by a normal touch, and checks that the padded one is dropped, leases are superseded, and the next line still decodes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
hyperb1iss
force-pushed
the
nova/device-input
branch
from
October 9, 2026 03:17
40b02fc to
274af46
Compare
Owner
Author
|
Rebased onto main (no conflicts, the six original commits replay unchanged) and pushed three fixes from an independent Codex review. Each new test fails with its fix reverted.
Spec 82 §6, §7, and Testing match the new behavior. Local gates on 274af46 all pass: fmt, ~ via nova ⚡ |
2 of 15 tasks
Main now starts the input publication pump inside a startup progress step, so the device input routing argument this branch adds moves into that step's closure. No other behavior changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
1 participant
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.
What this changes
Touching a ROLI Lightpad or pressing a LUMI key now reaches Hypercolor's interaction pipeline, attributed to the device that was touched. Touches and device buttons arrive in
FrameInputs.interactionwith live positions and as routedInputEvents on the bus, beside host keyboard and mouse input. Effects cannot read them yet; that is the next phase.hypercolor-typesDeviceInputEdge(touch began, moved, ended, named button) for drivers, plus routedInputEvent::TouchandInputEvent::DeviceButtoncarrying theDeviceId. Touch positions are Q16.16 soInputEventkeeps derivingEq.hypercolor-driver-apiDriverHost::device_input()returns aDeviceInputSink.attach()hands a driver a per-device publisher lease; hosts without device input returnNone.hypercolor-core::input::deviceDeviceInputHandle, a registry modeled on browser preview input: one routable child per attached device, outsideInputManagerand outside the host capture consent gate.hypercolor-core::input::routingDevicesource class, selected under theHostandMergepolicies and never underBrowser, with touches and device buttons as held controls keyed byDeviceId.hypercolor-daemonTouch movement is held state, like pointer motion: only begin, end, and cancel cross the event ring and the bus, while
InteractionData.device.touchescarries the live position every frame. Previews default to theBrowserpolicy, so they stay isolated from device input just as they are from host keyboard input.Why
The Blocks driver only lit devices. It never subscribed to blocksd's event stream, and no driver had any way to publish input, so hardware that Hypercolor lights could not also be played. The seam is driver neutral, so Push 2 pads or any other controller Hypercolor drives can publish the same way later.
The design is spec 82. The blocksd side is blocksd #14, which adds button identity and corrects touch velocity; this PR also works against older blocksd releases, where every button on a block shares one name.
Verification
just verifypasses locally (Rust fmt + lint + test)just denypasses (required for dependency or license changes)just ui-testandjust ui-buildpass (required forcrates/hypercolor-ui/)just sdk-lint,just sdk-check, andjust sdk-buildpass (required forsdk/)just python-verifypasses (required forpython/)just compat-checkpasses (required fordata/drivers/vendors/*.toml)just docs-buildpasses (required for docs or README changes)cd docs && zola checkpasses (required for docs link/content changes)scripts/orpackaging/)just e2e-buildpasses with the normal Servo stack (required for daemon/UI/effect integration changes)just e2e-build-cpupasses when validating the CPU smoke fallbackjust e2epasses against the Servo stack (required for end-to-end behavior changes; starts daemon/browser)just verifyis unchecked because its workspace test step was run in parts. On this workstation the wgpu tests stall when they share a parallel run with the rest of the daemon suite, and each passes alone; the same stall reproduces on the base commit. Every otherverifycomponent that this change touches passed on the final head40b02fc96, after rebasing onto currentmain:just oss-boundary-check-strict,just api-doc-route-check, andjust macos-gpu-only-checkcargo fmt --check, andcargo clippy --workspace --all-targets --features hypercolor-daemon/persistence-test-hooks -- -D warningshypercolor-core:device_input_tests(20, including end-to-end routing),routing_tests(16),blocks_input_tests(5, stable over 12 repeated runs),blocks_backend_tests(6),input_tests(114), the library tests (353), and the allocation contract, which now holds a device touch and button through every measured framehypercolor-types,hypercolor-driver-api, andhypercolor-driver-builtintestshypercolor-daemonlibrary: all 1,091 tests, as 855 in parallel and the 236 wgpu tests seriallyNo dependencies, UI, SDK, packaging, or published docs changed; spec 82 lives under
docs/specs, outside the Zola site. The e2e paths are left to CI.An independent verifier reviewed the branch and passed it; its findings shaped the last round of fixes, each with a regression test. It compiled every intermediate commit of the pre-rebase branch with zero warnings, and ran adversarial routing scenarios with no stuck, doubled, or permanently quarantined controls: ring overflow mid-touch, supersede mid-touch, Host to Browser to Host policy flips, and a second consumer attaching mid-touch. A stress run of three injecting threads against demand toggling every 300 µs and supersedes every 3 ms routed about 665k edges with no malformed edge streams or deadlocks. It also confirmed the excluded
hypercolor-uicrate still builds for wasm, and that no SDK, TUI, or platform crate matches exhaustively onInputEvent. The blocksd stream is covered by a fake blocksd harness.Not yet tested on hardware: nothing here has run against a physical Lightpad or LUMI. The next step is touching both with an interactive effect active and watching
input_eventson the WebSocket.Notes for reviewers
The router never sees a hold or release it did not observe being pressed. The router clears a quarantined control only on a release edge, so a producer that silently drops holds leaves quarantine that swallows the next tap. Each device child therefore tracks what is physically held but only announces a press while something demands interaction. Losing demand cancels every announced hold with explicit
CancelledandReleasededges, and a press that began while undemanded stays invisible until it lifts.A stale lease can never retire its successor. Attaching a device again supersedes the earlier lease with an incarnation-fenced close; the old lease's
publishreturnsfalseand its drop does nothing.Device input never stands in for host input. An attached Lightpad does not make a keyboard effect report input as available when host capture is off.
Demand, not consent, gates device input. It is not keystroke capture, so
[input].enableddoes not gate it. The input pump mirrors aggregate interaction demand (an interactive effect, a preview, or aninput_eventsWebSocket subscriber) at the top of every iteration, independently of the host capture reconcile, and clears it when the pump stops. While nothing demands interaction, no device traffic reaches the bus.blocksd events use their own connection. The frame connection pairs each request with the next line or byte it reads, so an interleaved event would be misread as a frame acknowledgement. Anything that could hide a lift cancels held input:
device_removedfor that device, and an undecodable line or a lost connection for every device.device_addedpayloads are ignored, so schema drift there cancels nothing.Start with
crates/hypercolor-core/src/input/device.rs(EdgeFoldandsync_demand), then the device arms inrouting.rs(event_control,reconcile_snapshots,build_interaction_into). The lock order is status control, registry writer, then child state.Known limits: a touch whose end blocksd never sends stays held until its contact index is reused or the device detaches, and how LUMI touch
xmaps onto the 24 key LEDs still needs a hardware capture. Phase 2 exposes touches to effects with canvas coordinates and ships a touch-ripple effect.🤖 Generated with Claude Code
Summary by CodeRabbit