Repository navigation
fix(korg): E2 pattern push carries chord notes (was silently dropped) - #40
Merged
Merged
Conversation
stepToE2() in synthstudioToE2Pattern.ts built E2StepInput as only
{active, velocity, note} and never forwarded StepData.chordNotes, even
though the pull path (DrumMachine.applyE2DecodedToActivePattern, v3.309)
actively applies stepChordNotes() from the decoded device body. A
pulled-then-pushed pattern therefore lost every chord on the way back
to the device.
buildE2PatternBody() already knows how to encode chordNotes (bytes
+0x05..+0x07, max 3 slots, 0..127 clamped, only for active steps) — no
new encoder logic needed, just pass the field through.
Found during a code-only audit of the "Geraet"-push/pull buttons for
Omnitribe's docs/hwtest/e2s_native_layer_bringup.md Section 3 (never
run against real hardware). Offsets/CMD-framing checked out against
the proven measurement; this chord-note asymmetry was the one clear,
provable regression in the round-trip. Everything else from that audit
(template-overlay reset of sample-ref/filter on push, unwired isPlaying
guard) is left untouched per explicit decision — separate concerns.
Tests: 5 new cases in korg-e2-pattern-push.test.ts, read back through
decodePatternBody()+stepChordNotes() (the same decode path the actual
DrumMachine pull handler uses, not the chordNotes-blind
e2PatternToSynthstudio() wrapper): normal chord, empty chordNotes, more
notes than the 3 format slots (truncated deterministically, not
silently), inactive step with chord data (not written, matches existing
note/velocity/gate behavior for inactive steps), and no chordNotes at
all. Verified red before the fix via `git stash` on just the source
file (2/14 failing, exactly the two chord assertions) and green after
(14/14).
Full suite: pnpm check clean, pnpm test 526 files / 11493 passed / 37
skipped / 0 failed, pnpm build OK, pnpm test:web (CI_HEADLESS=1) 200
passed / 1 skipped / 0 failed.
package.json + agents/INDEX.js version bumped 3.316.0 -> 3.317.0
(also resyncs INDEX.js's stale 3.311.0 marker).
Co-Authored-By: Claude Opus 5 (1M context) <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.
Der Befund
stepToE2()(client/src/utils/korg/synthstudioToE2Pattern.ts) baute beimPush eines Patterns an ein Korg E2/E2S nur
{active, velocity, note}— diechordNoteseines Steps wurden nie in dasE2StepInputübernommen. DerPull-Pfad (
DrumMachine.applyE2DecodedToActivePattern, v3.309) übernimmtAkkord-Noten dagegen aktiv aus dem gepullten Geräte-Body, und
E2StepInputunterstützt das Feld längst (
chordNotes?: number[], Bytes +0x05..+0x07).Ergebnis: ein Pattern, das vom Gerät gepullt und danach wieder gepusht
wurde, verlor dabei still jeden Akkord — eine stumme Asymmetrie im
Roundtrip, kein Absturz, kein Fehler-Toast.
buildE2PatternBody()kannte die Kodierung (max. 3 Chord-Slots, 0..127geclampt, nur bei aktiven Steps geschrieben) bereits vollständig — der Fix
reicht
chordNotesnur noch durch, ohne neue Encoder-Logik.Herkunft
Gefunden bei einem reinen Code-Audit der Sequencer-Push/Pull-Buttons für
einen offenen Hardware-Test-Punkt aus dem Prüfprotokoll des
(privaten) Firmware-Schwester-Repos — dort ist dokumentiert, dass diese
Buttons implementiert, aber noch nie an echter Hardware ausgeführt wurden.
Die Offsets und das Sysex-Framing selbst stimmten beim Audit mit einer
früher am Gerät bewiesenen Messung überein; diese Chord-Notes-Asymmetrie
war der einzige eindeutige, belegbare Fehler im Roundtrip.
Rot-vor-Fix-Nachweis
Damit der neue Test nicht nur Begleitgrün ist, sondern den Bug wirklich
trifft:
git stash push -- client/src/utils/korg/synthstudioToE2Pattern.ts(nur die Fix-Datei isoliert, die 5 neuen Tests blieben im Baum)
vitest run tests/features/korg-e2-pattern-push.test.ts→2 von 14 Tests rot, exakt die beiden Chord-Assertions
(
expected [] to deeply equal [64,67,71]bzw.[10,20,30])git stash pop→ Fix zurückDie 5 neuen Tests decken zusätzlich die Grenzfälle ab: leeres
chordNotes(kein Akkord), mehr Noten als die 3 Format-Slots fassen(deterministisch auf die ersten 3 gekürzt statt still verworfen),
inaktiver Step mit Akkord-Daten (wird nicht geschrieben — passt zum
bestehenden Verhalten für note/velocity/gate bei inaktiven Steps), und
kein
chordNotesüberhaupt. Gelesen wird überdecodePatternBody()+stepChordNotes()— derselbe Decode-Pfad, dender echte Pull-Handler benutzt, nicht die (chordNotes-blinde)
e2PatternToSynthstudio()-Hülle.Was dieser Fix NICHT tut
sind weiterhin nie an echter Hardware verifiziert.
dem Init-Template und überschreibt nur wenige Felder, wodurch
Sample-Referenz und Filter-Einstellungen eines Parts beim Push auf
Werkswerte zurückfallen können, wenn der Part nicht per
#NNN-Namenbenannt ist. Dokumentiertes Verhalten, bewusst unangetastet.
isPlaying-Guard(
useE2sDeviceStore.ts) — ein laufender Geräte-Sequencer wirdclient-seitig nicht blockiert, nur über die Geräte-NAK sichtbar.
Separate Entscheidung.
Testergebnisse
pnpm check(tsc --noEmit): sauberpnpm test: 526 Dateien, 11493 Tests grün, 37 skipped, 0 rotpnpm build: erfolgreichpnpm test:web(Playwright,CI_HEADLESS=1): 200 passed, 1 skipped🤖 Generated with Claude Code