Repository navigation
feat(push2): publish pad, strip, and button touches onto the canvas - #438
Merged
Merged
Conversation
A device with several LED outputs in a layout had no touch placement at all, because one unnamed touch could land on any of them. Touches can now name the device segment they landed on, and DeviceSurfaceMap resolves a named touch through the one LED output bound to that segment. A touch that names no segment keeps the single-output rule, so single-surface devices such as the Lightpad resolve exactly as before. A named touch on a device whose only output names no segment lands there, since that output stands for the whole device. Strip outputs now follow their direction the way matrix outputs follow their start corner: a strip's surface x axis runs from its first LED, so a touch at a strip position lands on the LED the sampler reads there in every direction. Before, a strip took its surface frame as the output's local frame, which only agreed with the LEDs for left-to-right strips; that direction maps exactly as it did. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Placing a touch per segment needs the segment to survive the trip from the driver to the effect payload. TouchBegan now carries an optional segment name, the device input fold keeps it with the held contact, and every Touch event that contact produces, its begin, end, and cancel, names it. Held TouchContact snapshots carry it as a shared Arc<str>, so the router still rebuilds held output each frame without allocating, and the LightScript payload resolves canvas points through the segment the touch names. The segment rides as an optional field that serializes only when set, so a Lightpad touch keeps the event shape it always had. Blocks touches name no segment, since every block has one touch surface. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Driver-owned USB transports had no way to report input from the devices they drive, and the HAL cannot reach the driver API's input sink without breaking the crate graph. The HAL now defines TransportInputSink, a callback a transport hands decoded DeviceInputEdges to, along with the two moments only the transport can see: input was interrupted, so a lift may have been lost, or input ended for good. A binding opts in with publishes_input, and the open request carries the sink. The core USB backend owns the lease. For a binding that publishes input, it passes the transport a bridge at open, attaches the host lease under the device's canonical DeviceId only once connect succeeds, and retires it when shutdown starts, so held touches cancel before the shutdown sequence runs. An interruption supersedes the lease and a close retires it, so the host cancels whatever the device held either way, the same way the Blocks backend handles a lost blocksd stream. Input that arrives before connect finishes is dropped, and a device that never finishes connecting never appears as an input source. The builtin USB module hands the host's device input sink to the backend. No binding sets publishes_input yet. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The Push 2 transport already held a midir input connection on the User port, but only for sysex replies; pad notes were discarded. Its callback now hands each message to the reply ingress first, so acknowledgement latency is unchanged, then to a relay that decodes it and publishes the edges through the transport input seam. Reading input sends no MIDI and changes no device mode, so it adds nothing to the output path that has wedged the firmware before, and the relay takes no lock the output or display paths share. The decoder follows Ableton's MIDI and Display Interface Manual. Pads (notes 36 to 99) begin on Note On with velocity as pressure, end on Note Off or zero velocity, and move on poly or channel pressure. Pads report in the matrix's LED frame and the strip along its LEDs from the bottom, so a press lands on the LED under it through the layout's start corner or strip direction; touches name the Pads or Touch Strip segment. The strip's autoreturn sends center just before every lift, so center only moves a touch once a later value confirms the finger, and a lift ends at the last real position. Buttons use stable names from the driver spec's tables, and relative encoder turns and encoder touches are ignored. A message cut short or a Set MIDI Mode reply announcing Live mode reports an interruption, and the relay's drop with the input connection reports that input ended, so the host cancels what the Push 2 held. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Spec 28 listed input events as future work and spec 82 named Push 2 pads as the next producer. Spec 28 now documents the input path: the MIDI map and where it comes from, the LED frames touches report in, button names, strip autoreturn handling, and what counts as lost input. Spec 82 gains the segment field on touch edges, events, and held contacts, per-segment placement and the strip direction rule, the USB transport input seam and its lease lifecycle, and the new tests and known limits. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The device input fold capped held contacts at 32 per device, sized for touch hardware whose contact indices fit in five bits. The Push 2 can hold 65 at once, its 64 pads plus the touch strip, so a palm across the grid dropped every later press, the strip included. The cap now sits at 128, the range of seven-bit note numbers MIDI controllers count contacts by, so it bounds a misbehaving driver without dropping real presses. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
LED routing binds an output's segment name to the device by exact name, then by alias, ignoring case and slug punctuation, so a layout naming the pads "pads" still lights them. Touch placement compared names exactly, so the same layout lit the pads while every touch on them lost its canvas point. A named touch now resolves through the exact match, or else the only alias match, and two alias matches stay ambiguous. A named touch also no longer falls back to a device's single unnamed output. That output's LED order follows the whole device, not the touched segment, so projecting a pad touch through it would land on the wrong LED; an unplaced touch is the honest answer. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
An interruption re-attached the lease without checking that it was still current. If a newer connection of the same device had attached since, the stale bridge would supersede it, stranding the live connection's input. The Push 2's MIDI lifecycle lease keeps two opens of one device apart today, but the bridge should not rely on that. It now asks its lease whether it is still current with an empty publish before re-attaching, and closes itself instead when it is not, as it also does when a publish reports the lease superseded. The driver API documents that publishing no edges changes nothing and reports whether a lease is current, which the bridge relies on. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
TouchPosition's field docs still said y counts from the surface's top edge, while a matrix surface counts from the row holding its first LED, which on the Push 2 is the physical bottom row. The relay claimed no allocation once warm, which holds for decoding but not for what the host sink does with published edges. The Push 2 touch surface test now covers all four matrix start corners and says plainly that it builds its layouts from the Push 2's zone list rather than through the UI seeding or auto layout code. Spec 82 records the raised contact cap, alias matching, and the lease fence. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A held touch whose segment has no exact match in a zone's layout went through channel_name_matches_slot_alias every frame, which builds several slug strings per comparison. A scene zone that places some of a device's segments but not the touched one takes that path for every held touch on every frame, as a Push 2 pad does in a zone that places only its white buttons and display. Two names alias exactly when their slot alias keys are equal, so the surface map now builds each output's key once, when the map is built, and slot_alias_key_matches walks the touched segment's name against it in place. A test checks the new comparison agrees with the old one over every pair of a set of names, and the allocation contract now places a held touch that misses its exact segment, and one that matches only by alias, with zero allocations. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
An interrupted USB input bridge asked whether its lease was still current with an empty publish, then attached a replacement. The check and the attach were separate steps, so a fresh bridge for the same device could activate between them; the stale bridge then superseded it, took the device's input back, and the fresh bridge closed itself for good on its next publish. DeviceInputPublisher gains renew(), which replaces a lease with a fresh one only while it is still current. The core registry checks the publication incarnation and swaps the child under the writer lock, the same lock attach takes, so whichever bridge gets there second loses cleanly: a stale lease renews to None and its bridge closes. The trait method defaults to None, so publishers that cannot renew atomically retire the lease instead of racing. A regression test forces a fresh activation between a stale bridge deciding to renew and the renewal, and fails against the old probe-then-attach logic. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Aftertouch replaced a held pad's pressure outright. The Push 2 reports aftertouch only above its pads' threshold and can send 0 once a strike settles, so a held pad's pressure collapsed right after the hit, and Touch Ripple's held glow shrank under a finger still on the pad. A held pad now keeps its strike velocity, and its pressure is the greater of that and its latest poly or channel aftertouch. Pressing in harder still swells it, easing off settles back to the strike rather than below it, and a pad moves only when that pressure changes. The decoder test and spec 28 state the rule. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The SDK's TouchContact doc still said x and y run from the surface's left and top edge, and that canvas is null when a device is placed more than once. Touches now report in the frame the touched segment's LED order defines, so a Push 2 pad's y of 0 is its bottom row, and a device with several segments is placed per segment, so a touch loses its canvas point only when its own segment is unplaced or placed twice. Spec 28 also claimed the Push 2 relay allocates nothing once warm. That holds for decoding, but publishing into the host sink allocates in the core fold, and the spec now says so. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
This review includes 39 billable files and costs up to $9.75. Or wait 34 minutes for your next included review. View limit details
Comment |
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.
Pressing an Ableton Push 2 pad now reaches effects as a device touch, with a canvas point that lands on that pad's own LED. Touch Ripple, and any effect that reads
input.touch, plays the Push 2 the way it already plays a ROLI Lightpad: a hit ripples from the pad, pressing in harder swells the glow, the touch strip drags a contact along its LEDs, and the buttons arrive as device buttons.Two gaps stood in the way. The Push 2 driver read MIDI input only for sysex replies, so pad notes went nowhere. And even with touches published, the canvas projection treated any device with more than one LED output in a layout as ambiguous and gave its touches no canvas point. The Push 2 has pads, buttons, and a touch strip, so every touch would have projected to
null.💡 What changes
Touches name the segment they land on.
DeviceInputEdge::TouchBegancarries an optionalsegment, the core fold keeps it with the held contact, andDeviceSurfaceMapresolves a named touch only through the layout output bound to that segment. Matching is exact first, then by the same case and punctuation rules LED routing uses, and alias keys are computed once when the map is built, so resolution allocates nothing per frame. A touch with no segment keeps the old rule (the device's only LED output), so the Lightpad path is unchanged.InputEvent::Touchgainssegment, omitted from JSON when unset, so a ROLI touch serializes exactly as before.Driver-owned USB transports can publish input.
hypercolor-halcannot depend on the driver API, so the HAL defines a small callback trait,TransportInputSink(publish,interrupted,closed). A transport opts in withpublishes_inputon its binding. The core USB backend receives the host's input sink throughwith_device_input, wired indriver-builtin, and owns the lease through a bridge that follows the Blocks semantics:interruptedrenews the lease, which cancels held touches, through a newDeviceInputPublisher::renewthat swaps only a lease that is still current, under the same registry lockattachtakes;The Push 2 decodes its MIDI input. The existing midir input callback still hands every message to sysex reply handling first, so acknowledgement timing is unchanged, then passes it to
Push2InputDecoder. Decoding allocates nothing once warm, takes no lock the output or display paths use, sends no MIDI, and changes no device mode.Pads,x = col/7,y = row/7with row 0 the bottom rowTouch Stripplay,shift,track_1)The driver configures the strip with autoreturn, and the manual says the center value arrives just before the release note, so the decoder treats a center reading as ambiguous while touched and ends a lift at the last real position. A Set MIDI Mode reply that reports Live mode is treated as lost input, because this port then stops receiving pad releases.
Strip outputs honour their direction. A strip output used the touch position as is, which only lined up with the LEDs on left-to-right strips. Strips now follow their direction the way matrices follow their start corner. Left-to-right strips, including the rig's LUMI Keys, map exactly as before.
🎯 Invariant
A press on pad N projects to the exact canvas point pad N's LED samples.
push2_touch_surface_testsproves it without re-deriving the mapping: for each case it lights one LED through the realPush2Protocol::encode_frameto find the note that drives it, presses that note through the real decoder, and compares the touch's canvas point with the real sampler's point for that LED. It covers all four pad corners and an interior pad in all four start corners, plus strip positions in all four directions, and it fails if the pad y axis is flipped.🧪 Validation
cargo fmt --all -- --check,cargo check --workspacecargo clippy -D warnings --all-targetson types, hal, core, driver-api, driver-builtin, daemoncargo test --no-fail-faston types, hal, driver-api, driver-builtincargo test -p hypercolor-core --no-fail-fasthtml_loader_testsalloc_contract_tests,--test-threads=1device_surfaces,device_input,interaction_routing,input_publication,input_event,--test-threads=1just sdk-lint,just sdk-checkThe two
html_loader_testsfailures need effect HTML fromjust effects-build; they fail the same way on the base commit in the same environment, and this branch does not touch them.An independent verifier checked every MIDI fact against Ableton's
AbletonPush2MIDIDisplayInterface.ascand its mapping figure, ran ten decoder edge-case probes (sysex inside a press and release, running status, truncated sysex, out-of-range pitch bend), and ran the real decoder and frame encoder against the owner's saved scene: all 64 pads and 7 strip positions landed on their sampled LEDs. Its first pass found four non-blocking issues (the lease renew race, per-frame allocation in alias matching, the pressure overwrite, and stale docs); the last four commits fix them, and the verifier's second pass closed each one, including showing the race test fails on the old code and that a 40,000-pair fuzz of the alias matcher agrees with routing's.On hardware: this branch runs the owner's rig with a Push 2 connected. The device connects, the daemon logs no errors, and frames hold at 2.9 ms with every LED zone sampled on the GPU. The owner played the pads with Touch Ripple on top, and each ripple started from the pad that was hit.
connect(), which enumerates real devices; the bridge and shutdown paths are tested directly.🔍 Review focus
Start with
DeviceSurfaceMap::segment_canvas_positionincrates/hypercolor-core/src/spatial/device_surface.rsand the bridge incrates/hypercolor-core/src/device/usb_backend/input.rs; the bridge's lock order is bridge then registry writer, never the reverse. Thencrates/hypercolor-hal/src/drivers/push2/protocol/input.rsfor the decode. Specs 28 (§11.4) and 82 (§9) describe the input path and the segment vocabulary.🤖 Generated with Claude Code