alda
alda copied to clipboard
alda-lang
Validate MIDI note range
Currently, attempting to play a note that is out of the MIDI note range (0-127) results in a confusing error message:
$ alda play -c 'piano: o10 c'
[27713] Parsing/evaluating...
[27713] ERROR data1 out of range: 132
We should catch this and throw our own error with a clearer message.
I did some investigation on this one. It took me some time to find the source of the "Parsing/evaluating..." message. I eventually found it on alda-client-java//src/alda/AldaServer.java, method play - but now I'm wondering where in the core project this change would take place. If someone can help me and point out on the right direction, I'd appreciate it. Meanwhile I'll try and look into other issues.
We're calculating the MIDI note number here: https://github.com/alda-lang/alda-core/blob/d45039101bf347eb84be7afa471db1b812291914/src/alda/lisp/events/note.clj#L40-L42
I think throwing an ex-info when (> midi-note 127) ought to do the trick.
I don't remember off-hand whether I fixed this in Alda v2 or not. Keeping this issue open until I have a chance to confirm.
I just tried this with Alda 2, and interestingly, playback seems to just silently fail once it gets up to the too-high note. No error message is printed either in the client or in the player logs.
So the disposition is still the same: we should add validation that the MIDI note isn't < 0 or > 127, and I think we should do that validation at score evaluation time instead of playback-time, so that we can display a helpful message earlier.
I think the place to add the validation now is right here after we calculate the MIDI note based on the octave, note letter and accidentals.
I have added the validation at the specified place and submitted a PR in response to this issue. Also ran the addition against the test suite.
Fixed in https://github.com/alda-lang/alda/pull/461. Will keep this issue open until I have a chance to release it.