Skip to content
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

MIDI UART support #7

Open
CedarGroveStudios opened this issue Mar 12, 2019 · 22 comments
Open

MIDI UART support #7

CedarGroveStudios opened this issue Mar 12, 2019 · 22 comments

Comments

@CedarGroveStudios
Copy link

@CedarGroveStudios CedarGroveStudios commented Mar 12, 2019

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

@ladyada

This comment has been minimized.

Copy link
Member

@ladyada ladyada commented Mar 12, 2019

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

@CedarGroveStudios

This comment has been minimized.

Copy link
Author

@CedarGroveStudios CedarGroveStudios commented Mar 12, 2019

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

@CedarGroveStudios

This comment has been minimized.

Copy link
Author

@CedarGroveStudios CedarGroveStudios commented Mar 13, 2019

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

This comment has been minimized.

Copy link
Contributor

@tannewt tannewt commented Mar 13, 2019

@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

This comment has been minimized.

Copy link
Author

@CedarGroveStudios CedarGroveStudios commented Mar 13, 2019

@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

This comment has been minimized.

@tannewt

This comment has been minimized.

Copy link
Contributor

@tannewt tannewt commented Mar 19, 2019

@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

This comment has been minimized.

Copy link
Author

@CedarGroveStudios CedarGroveStudios commented Mar 19, 2019

@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

This comment has been minimized.

Copy link
Contributor

@tannewt tannewt commented Mar 20, 2019

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

This comment has been minimized.

Copy link
Author

@CedarGroveStudios CedarGroveStudios commented Mar 20, 2019

^ 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

This comment has been minimized.

Copy link
Contributor

@tannewt tannewt commented Mar 20, 2019

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

This comment has been minimized.

Copy link

@kevinjwalters kevinjwalters commented Mar 20, 2019

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

This comment has been minimized.

Copy link
Author

@CedarGroveStudios CedarGroveStudios commented Mar 21, 2019

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

This comment has been minimized.

Copy link

@kevinjwalters kevinjwalters commented Jun 13, 2019

WRT to comment on default values for midi_in and midi_out, there are now no defaults as of #11

@kevinjwalters

This comment has been minimized.

Copy link

@kevinjwalters kevinjwalters commented Jun 13, 2019

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

This comment has been minimized.

Copy link

@kevinjwalters kevinjwalters commented Jun 13, 2019

Where is the midi_uart.py file btw?

@CedarGroveStudios

This comment has been minimized.

Copy link
Author

@CedarGroveStudios CedarGroveStudios commented Jun 14, 2019

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

This comment has been minimized.

Copy link

@kevinjwalters kevinjwalters commented Jun 14, 2019

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

This comment has been minimized.

Copy link
Contributor

@tannewt tannewt commented Jun 14, 2019

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

@kevinjwalters

This comment has been minimized.

Copy link

@kevinjwalters kevinjwalters commented Jun 14, 2019

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

Can you elaborate a bit on that?

@tannewt

This comment has been minimized.

Copy link
Contributor

@tannewt tannewt commented Jun 17, 2019

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

This comment has been minimized.

Copy link

@kevinjwalters kevinjwalters commented Jun 20, 2019

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().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.