lichen-community-systems / flocking-midi

A collection of MIDI components for Infusion and Flocking
1 stars 2 forks source link

Add the ability to send messages to a MIDI device. #8

Closed the-t-in-rtf closed 4 years ago

the-t-in-rtf commented 6 years ago

Currently, flock.midi.connection.send is set to call flock.fail with an error message. I have put together a branch that adds write support and a demo harness for the same. I will submit a PR once I add a few tests.

colinbdclark commented 6 years ago

This will be awesome, @the-t-in-rtf! I have just recently merged my continuing-creativity/Flocking#214 branch, which ensures that all of Flocking's unit tests run well in both Node.js and across supported browsers. So whenever you have time (no rush) to make a pull request against master, I'll be keen to review it.

the-t-in-rtf commented 6 years ago

I just realised that I can (and should) at least create unit tests that roundtrip between raw MIDI and JSON using the encoding and decoding functions. I'll comment when that's ready.

colinbdclark commented 6 years ago

Yes, good point. One suggestion I was thinking of is to record some examples of MIDI output using MIDI Monitor or a similar tool, and then write your tests so that they verify that Flocking can produce the same raw messages. I have quite a number of tests like this in osc.js, where I ended up having to hand-create raw OSC bytes in order to test both its reading and writing functionality. In the case of MIDI it should be a lot simpler, you can just sit down at your favourite controller or sequencer and grab examples of the kind of output you need to test with.

Round-trip tests are good too, definitely. But if we have some sample raw MIDI bytes we can test both sides of the system separately from the other, avoiding potential bugs that emerge from incorrect assumptions that are implemented consistently on both sides of the conversion process.

colinbdclark commented 5 years ago

Fixed via continuing-creativity/Flocking#224 and continuing-creativity/Flocking#237. Thanks again, @the-t-in-rtf!