-
-
Notifications
You must be signed in to change notification settings - Fork 134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add MIDI Program Change, SysEx, NRPN, PitchBend and AfterTouch Output #1244
base: main
Are you sure you want to change the base?
Conversation
This commit adds a second argument to the midi() command: mapping. This argument should be an object containing a key-value map of MIDI controls used by an external synthesizer. If any control is used that matches the mapping, a CC message is sent.
split sysex message into sysexid and sysexdata sysexid is a device identification number or array sysexdata is an array of data to be sent to the device
- add example sysex data for setting NSX-39(Pocket Miku) voice data.
…d-program-change
37f826f
to
a880378
Compare
great stuff! would it make sense to add this to the existing |
wow also nice work on the docs |
Thank you for bringing this to my attention (#710). I overlooked the existing midicmd control completely.
|
Regarding sysex (System Exclusive) messages, While ideally I'd want to structure it as:
Currently, I've split it into:
This split approach largely follows webmidijs's sendSysex method specification, sendSysex([id],[data],[option]) If we adopt
here are some relevant topic on discord channel. Or |
- MiniRepl for input-output.mdx - addresses tidalcycles#789 tidalcycles#710
- add progNum keyword handler - update midicmd handler to handle 'progNum' case
- sysex(id, data) and both arguments are patternable
I have managed to get Here I have added a sysex method in controls.mjs using export const sysex = register('sysex', (args, pat) => {
if (!Array.isArray(args)) {
throw new Error('sysex expects an array of [id, data]');
}
const [id, data] = args;
return pat.sysexid(id).sysexdata(data); //return a chain of controls (which returns a Pattern)
}); |
thanks! looking forward to testing this. i hope this can be part of the next release (happening soonish) |
so i assume we have to merge #1212 first, right? |
I tried adding API references, but it’s causing errors in the test code. I’ll fix it tomorrow morning by adding some dummy code for testing MIDI in and out. |
- add mock 'midin' and 'sysex' - typo in sysex example
#1274 is now merged, so this PR could now be updated to use the new API |
…d-program-change
- Move MIDI API logic into separate functions following tidalcycles#1274 (sendCC, sendProgramChange, sendPitchBend, etc.)
@nkymut is this one ready for a review? |
Yes, I've been regularly using the setup with multiple midi devices (two NSX-39 units, an OP-LAB, a NUX TapeEcho, and a Behringer RD-6), and haven't encountered any issues so far. It's ready for a review. Thank you! |
|
||
//TODO: MIDI mapping related | ||
if (typeof output === 'object') { | ||
const { port, controller = false, ...remainingProps } = output; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't that obsolete now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, its obsolete. As #1274 merged and we can use midimaps
instead.
not sure if its necessary to keep it for backward compatibility? @Bubobubobubobubo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
afaik "output" being an object was never in main
packages/midi/midi.mjs
Outdated
function mapCC(mapping, value) { | ||
console.log('mapping', mapping); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this log useful here, or an artifact of debugging?
packages/midi/midi.mjs
Outdated
@@ -254,22 +342,57 @@ Pattern.prototype.midi = function (output) { | |||
velocity = gain * velocity; | |||
// if midimap is set, send a cc messages from defined controls | |||
if (midicontrolMap.has(midimap)) { | |||
console.log('midimap', midimap); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need that log?
const ccs = mapCC(midicontrolMap.get(midimap), hap.value); | ||
ccs.forEach(({ ccn, ccv }) => sendCC(ccn, ccv, device, midichan, timeOffsetString)); | ||
} | ||
|
||
// note off messages will often a few ms arrive late, try to prevent glitching by subtracting from the duration length | ||
const duration = (hap.duration.valueOf() / cps) * 1000 - 10; | ||
if (note != null) { | ||
if (note != null && !isController) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need the isController flag? iirc it was part of the old midimapping format? how would it be set with the new format?
i've read the code and added some comments / questions. all in all this looks good :) |
…d-program-change
…into add-program-change
This update adds support for sending MIDI Program Change and System Exclusive (SysEx) messages for @strudel/midi package. It builds upon the changes made in #1212; however, MIDI mapping and Hi-res CC have not been tested yet.
Examples:
Program Change:
SysEx Messages:
midibend && miditouch
midibend
sets MIDI pitch bend (-1 - 1)miditouch
sets MIDI key after touch (0-1)cc,progNum, sysex for
midicmd
Messages:cc
- sends MIDI control change messages.progNum
- sends MIDI program change messages.sysex
- sends MIDI system exclusive messages.Test & ToDo
pc
toprogNum
progNum
support tomidicmd
sysex
support tomidicmd