adafruit / Adafruit_CircuitPython_MIDI

A CircuitPython helper for encoding/decoding MIDI packets over a MIDI or UART connection
MIT License
48 stars 25 forks source link

MIDI UART support #7

Open CedarGroveStudios opened 5 years ago

CedarGroveStudios commented 5 years ago

Would it be possible to expand this to also support "classic" MIDI communications via a UART port?

ladyada commented 5 years ago

im cool with it if you can figure out how :)

CedarGroveStudios commented 5 years ago

Thanks. "I'll take a shot at it," he said nervously.

CedarGroveStudios commented 5 years ago

Status: With just a few changes I was able to create a working UART-only version, but I'm not sure how to combine the two and maintain the original UI -- just yet. I'm looking at other libraries for similar examples and plan to talk to some of the folks on the CPy team for advice. This might take me a while... but I'm having fun.

tannewt commented 5 years ago

@CedarGroveStudios Can you post your changes somewhere? The goal of the USB midi stuff was to act exactly like a UART so this lib would work with both. Maybe something needs to change with it.

CedarGroveStudios commented 5 years ago

@tannewt That was my objective, too. I have fork of the repo with a MIDI_UART class (and the original but renamed MIDI_USB class) and updated examples. Should I just create a PR from the updated fork? (I'm so new at using GitHub for this.)

CedarGroveStudios commented 5 years ago

The fork is here: https://github.com/CedarGroveStudios/Adafruit_CircuitPython_MIDI

tannewt commented 5 years ago

@CedarGroveStudios Could you rename the uart version back to the original filename so that the diff is shown? It looks to me like the only change is taking in midi_in and midi_out as pins instead of serial objects.

Did you try giving a UART as both midi_in and midi_out? I'd expect that to just work.

CedarGroveStudios commented 5 years ago

@tannewt The USB version doesn't handle midi_in if I'm understanding it correctly, so I just duplicated the same output-only functionality by adding the UART pins and unique baud/timeout settings as you noted. I created a second class to facilitate instantiation. Is there a more elegant way of doing it that still allows the creation of a UART instance?

from adafruit_midi import midi_uart

# defaults to transmit out the board.TX pin
midi = midi_uart.MIDI(out_channel=0)

The USB midi_out uses an abbreviated subset of the MIDI protocol stack (CC, Note_On, etc.). Parsing the USB and UART midi_in messages using a similar subset of the MIDI protocol stack would be something for the next version, I'd guess. For this I was just trying to duplicate the existing functionality of the USB code.

tannewt commented 5 years ago

I was thinking:

import board
import busio
import adafruit_midi

uart = busio.UART(board.TX, rx=None, baudrate=31250, timeout=0.001)
midi = adafruit_midi.MIDI(midi_out=uart, out_channel=0)

Check with @kevinjwalters about MIDI in. I think they've done some work on it already.

CedarGroveStudios commented 5 years ago

^ That's an excellent approach that could lead to using any MIDI input/output stream including USB, UART, BLE, and file. In your example, how would USB or BLE in/out be instantiated? I just looked at @kevinjwalters' repo. Their approach is appropriately MIDI message-centric and could be applied to the midi_out functionality as well. Looks to me like a better way to implement than the one I had in mind.

tannewt commented 5 years ago

We don't have BLE support yet.

USB is already instantiated because the USB descriptor is fixed. It is available as usb_midi.ports[0] or usb_midi.ports[1]. You can check direction by checking the object type.

It's a bit weird that those are the default values.

kevinjwalters commented 5 years ago

FYI, I'm hoping to have the MIDI stuff done by Sunday and will look at allowing messages to be sent whilst preserving the existing interface. I'm trying to keep it reasonably tight on memory which is my current area of exploration.

CedarGroveStudios commented 5 years ago

BTW, I successfully tested USB inputs and outputs along with simultaneous UART output using the existing library. My DAW calls the GrandCentral's USB port an Adafruit CircuitPython keyboard. Nice. Since I already have UART input/output code working for the MIDI protocol stack as part of my Crunchable synth project, I'll continue working on converting it to a library. As @kevinjwalters noted, it's a challenge to fit it into memory (it's a protocol stack approach rather than message-centric). However, I think it's doable. I'm a primarily a hardware person so my coding efforts aren't going to win any races. Don't wait for me.

kevinjwalters commented 5 years ago

WRT to comment on default values for midi_in and midi_out, there are now no defaults as of https://github.com/adafruit/Adafruit_CircuitPython_MIDI/issues/11

kevinjwalters commented 5 years ago

Just discussing using UART with @jedgarpark and I think the current library will work for midi_in and almost works with midi_out. The requirement for midi_in object is to support read(len) which busio.UART does, blocking behaviour will be inherited but perhaps that's controlled by timeout optional parameter on constructor? midi_out needs to support write(buf, len) but I see busio.UART has write(buf) with a return value - that could just be wrapped for now until we work out a cleaner solution.

kevinjwalters commented 5 years ago

Where is the midi_uart.py file btw?

CedarGroveStudios commented 5 years ago

BTW, I just threw together a quick/dirty UART MIDI sniffer for @jedgarpark using the library. The timeout was set to 1ms rather than the default 1sec: UART = busio.UART(board.TX, board.RX, baudrate=31250, timeout=0.001) It passed the preliminary tests using CPy 4.0.1 on a FeatherM4Express.

kevinjwalters commented 5 years ago

Having a write_with_len=True parameter on constructor would be a simple/practical way to avoid introspection which MicroPythonCircuitPython may not even support. That could be set to False when busio.UART is being used. Depending on behaviour of PortOut.write the return value could be checked and if ret is not None then the remnants of buffer could be iterated over to accomodate busio.UART.write.

tannewt commented 5 years ago

@kevinjwalters Instead of hacking the library let's just fix midi_out.

kevinjwalters commented 5 years ago

@kevinjwalters Instead of hacking the library let's just fix midi_out.

Can you elaborate a bit on that?

tannewt commented 5 years ago

The recent discussion was about adding a flag to distinguish a midi_out from a UART because "midi_out needs to support write(buf, len) but I see busio.UART has write(buf) with a return value" The two should have identical interfaces so that they can be used interchangeably. Any differences are unintentional and should be fixed.

kevinjwalters commented 5 years ago

It would be certainly be nice if busio.UART.write and PortOut.write had same interface. They are doing slightly different things at the moment. Would also be useful to describe the blocking behaviour and how, if possible, to control that.

Another question is whether support for optionals start and end is warranted like I2CDevice.write().

todbot commented 2 years ago

Also, busio.UART.readinto() and usb_midi.PortIn.readinto() have different method signatures, making it troublesome to implement a fast streaming MIDI parser. e.g. @jedgarpark & I discovered that Winterbloom_SmolMIDI does not work with UART MIDI.