Skip to content

midi: drop the dead time argument, and inject_midi with it - #1040

Merged
dpwe merged 1 commit into
mainfrom
dpwe/drop-midi-time
Aug 2, 2026
Merged

midi: drop the dead time argument, and inject_midi with it#1040
dpwe merged 1 commit into
mainfrom
dpwe/drop-midi-time

Conversation

@dpwe

@dpwe dpwe commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

The time argument was already dead

In amy_midi.c, time is declared AMY_UNSET_VALUE(time) (lines 321, 395) and never reassigned — so every real MIDI path (USB, gadget, UART, mac, sysex) has always passed it unset. amy_received_control_change() didn't even read its copy. Nothing outside src/ called these at all. The one caller that ever supplied a real value was pyamy's inject_midi(), i.e. the test suite.

That reflects the truth of the thing: a live MIDI message has no time of its own — it plays when it arrives. So the argument is gone from amy_event_midi_message_received() and the five amy_received_* handlers.

inject_midi goes with it

It was the weaker of the two Python entry points:

  • It hardcoded len=3, so it could not send a 2-byte message at all — test.py already used inject_midi_bytes([0xC0, 5]) for program change.
  • It handed a pre-formed message straight past the byte-stream parser.

inject_midi_bytes() runs the real convert_midi_bytes_to_messages(), so the tests now exercise running status and real-time interleaving the way actual MIDI input does. Net gain in coverage.

The ~16 test call sites that passed an absolute millisecond time move to a new amy_inject_midi_at(), the MIDI twin of the existing amy_send_at(): render up to the moment, then inject.

What deliberately did NOT change

midi_message_handler_to_queue() keeps its time parameter. It lives in midi_mappings.c, and patches.c:1081 drives it with a real e->time for the wave=AMY_MIDI osc — that's how a tick-scheduled event emits MIDI on the beat. amy_event_midi_message_received() now passes it an explicit unset instead of a laundered one.

Drive-by fixes

  • pyamy.c had three bare return Py_None; (missing INCREF — each call over-decremented None's refcount). Now Py_RETURN_NONE.
  • The Daisy port's midi_polyphony() demo was staggering notes 1000ms apart with the argument that never did anything; it now says so instead of pretending.

Testing

make test: 122 tests pass, and all 116 waveform comparisons are bit-exact (err=-100.0 dB) — no reference audio needed regenerating, despite the tests now routing through the byte-stream parser and the render-forward clock.

make check-c-api: no generated-API drift. Clean make with no new warnings.

🤖 Generated with Claude Code

The time parameter threaded through amy_midi was already dead: in
amy_midi.c it is declared AMY_UNSET_VALUE(time) and never reassigned, so
every real MIDI path -- USB, gadget, UART, mac, sysex -- has always passed
it unset. amy_received_control_change() didn't even read it. Nothing
outside src/ called these at all. The single caller that ever supplied a
real value was pyamy's inject_midi(), i.e. the test suite.

That reflects the truth of the thing: a live MIDI message has no time of
its own, it plays when it arrives. So remove the argument from
amy_event_midi_message_received() and the five amy_received_* handlers.

inject_midi() goes too. It was the weaker of the two Python entry points --
it hardcoded len=3, so it could not send a 2-byte message at all (test.py
already used inject_midi_bytes for program change), and it handed a
pre-formed message straight past the byte-stream parser. inject_midi_bytes()
runs the real parser, so tests now exercise running status and real-time
interleaving the way actual MIDI input does.

The ~16 test call sites that passed an absolute millisecond time move to a
new amy_inject_midi_at(), the MIDI twin of amy_send_at(): render up to the
moment, then inject. All 122 tests pass and all 116 waveform comparisons are
bit-exact, so no reference audio needed regenerating.

midi_message_handler_to_queue() KEEPS its time parameter -- it lives in
midi_mappings.c and patches.c drives it with a real e->time for the
wave=AMY_MIDI osc, which is how a tick-scheduled event emits MIDI on the
beat. amy_event_midi_message_received() now passes it an explicit unset.

Also fixes three bare `return Py_None` in pyamy.c (missing INCREF, so each
call over-decremented None's refcount) and updates the Daisy port, whose
midi_polyphony() demo had been staggering notes with the argument that never
did anything.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 2, 2026

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 @ 5309014 this PR Δ
1 1050 1045 -5
2 1200 1198 -2
3 1800 1799 -1
4 1962 1952 -10
5 2609 2610 +1
6 2766 2764 -2

Full chord settled render μs: 2765 (was 2767, Δ -0.1%) (peak 2771, 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 a0f74f5 into main Aug 2, 2026
12 checks passed
@bwhitman

bwhitman commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

⛓️ tulipcc integration PR opened

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

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.

2 participants