Skip to content

note_output: don't send notes from a patch being stored - #1195

Merged
dpwe merged 1 commit into
mainfrom
note-output-stored-patch
Sep 28, 2026
Merged

dpwe merged 1 commit into
mainfrom
note-output-stored-patch

Conversation

@dpwe

@dpwe dpwe commented Sep 28, 2026 •

Copy link
Copy Markdown
Collaborator

Follow-up noted in #1191 and #1194.

The bug. patches_store_patch parses a patch string into the patch's own delta list. A segment like i1n60l1 names a synth and a note, so it goes down patches_event_has_voices, and note_output acted on it. As a result, storing a patch:

  • sent a real note-on out of the MIDI port, or raised a CV gate, at the moment the patch was saved;
  • if the patch string contained i1l0, ran ALL NOTES OFF, dropping a gate that a note being played was holding up.

Reproduction on main:

i1iv1in1
i1iG2,1           # synth 1: MIDI note output
K1024ui1n60l1     # store a patch  ->  MIDI OUT 90 60 127

The fix. note_output_handle_event(e, live), where live means queue == &amy_global.delta_queue, the same test #1194 uses for midi_cc_output. For a stored event:

  • a note is still claimed, so a voiceless note-output synth skips the voice path exactly as it does when playing;
  • nothing is sent, and the held-note stack isn't touched.

Tests. test_note_output.c gains test_storing_a_patch_sends_nothing, which covers:

  • MIDI mode;
  • CV mode, including that the next played note still raises its own gate edge;
  • a stored panic leaving a held gate alone, while a played panic still drops it.

4 of its 6 checks fail on main and all pass with the fix. make ctest passes; make test is unchanged (90 / 43 at ~-99 dB, same as main here).

This and #1194 both touch patches.c, but in different places (yield_synth_commands there, patches_event_has_voices here), so either can merge first.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PykXkeeWw2aRpC6PLfpqTQ


Generated by Claude Code

patches_store_patch parses a patch string into the patch's own delta
list, and a segment such as "i1n60l1" names a synth and a note, so it
went down patches_event_has_voices and note_output sent a real note-on
out of the MIDI port (or raised a CV gate) at the moment the patch was
saved. A stored "i1l0" likewise ran ALL NOTES OFF, dropping a gate that
a played note was holding up.

note_output_handle_event now takes `live` (the event is headed for
amy_global.delta_queue). A stored note is still claimed, so a voiceless
note-output synth skips the voice path as before, but nothing is sent
and the held-note stack is untouched.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PykXkeeWw2aRpC6PLfpqTQ
@github-actions

Copy link
Copy Markdown
Contributor

🎛️ AMY HW CI (AMYboard bench)

Flashed this PR's AMY (LoadTestChord: 6-voice Juno patch=1, one held note every 2 s) onto the physical AMYboard and measured the smoothed render load as the chord grows — back-to-back with the same sketch built at the PR's merge base, so Δ is this PR's own cost.

✅ PASS — the bench ran the test to completion.

notes held main @ 1e3b49d this PR Δ
1 993 997 +4
2 1143 1147 +4
3 1721 1728 +7
4 1877 1885 +8
5 2497 2504 +7
6 2626 2630 +4

Full chord settled render μs: 2627 (was 2619, Δ +0.3%) (peak 2632, 39 samples)

⬇️ Artifacts: serial log · load trace · report

Self-hosted bench (amyboardci). FAIL means only that the test could not run — the load values are informational, with no threshold and no audio compare. See tools/arduino_loadsweep/.

@dpwe
dpwe merged commit fbc8721 into main Sep 28, 2026
12 checks passed
@bwhitman

Copy link
Copy Markdown
Collaborator

⛓️ tulipcc integration PR opened

This merge was pinned into tulipcc for full-system CI: shorepine/tulipcc#1380

Test it there and merge that PR to move tulipcc onto this AMY.

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.

3 participants