MIDI - support for virtual wires/jacks about adafruit_tinyusb_arduino HOT 23 CLOSED

Here is the first pull request, which is the preparation for the virtual wire code. It should not change any behavior.

adafruit/ArduinoCore-samd#179

from adafruit_tinyusb_arduino.

@hathach thanks for merging all the PRs.

Just a short update. I have rebased everything, these are the remaining changes now:
https://github.com/kaysievers/ArduinoCore-samd/tree/tinyusb-midi-packets
https://github.com/kaysievers/Adafruit_TinyUSB_Arduino/tree/midi-packet-interface

I've merged the example sketch into midi-packet-interface. It all seems to work fine

The tinyusb-midi-packets change still needs some cleanup work. I'll do this this week, and submit both as PRs. Thanks!

from adafruit_tinyusb_arduino.

I should have done this earlier :)

No worries, I am not in a hurry. :)

Both changes are submitted now. The midi_test example using the Stream interface, and the midi_virtual_ports example using the packet interface seem to work fine here.

from adafruit_tinyusb_arduino.

You mentioned the PRs were not against the right repositories. I closed the original PRs now and deleted the original repositories.

I maintain the current code in these branches:
https://github.com/versioduo/ArduinoCore-samd/tree/tinyusb-midi-packets
https://github.com/versioduo/Adafruit_TinyUSB_Arduino/tree/midi-packet-interface

I use this MIDI implementation on top of the added raw packet interface:
https://github.com/versioduo/V2MIDI

Just to show that it all works fine, here is a recently built instrument using all of the above :)
https://versioduo.com/#glockenspiel-37

I did not submit any new PRs. Feel free to cherry-pick anything from these repos, if you want to incorporate the changes yourself.

from adafruit_tinyusb_arduino.

@kaysievers I am really sorry if I did say something offended

Oh no, there is nothing wrong with anything. I'm not offended at all, or in any hurry. It's just that I didn't use the old repositories (kaysievers) anymore, and I'm only working on the new ones (versioduo) now. So I just deleted the old repositories, if we need to re-submit the PRs anyway.

Let me know when you are ready to look at the MIDI changes and when things have settled. I can open new PRs against the new repos.

The code is still there and maintained. Until we have figured out how to proceed, I'll continue working with repos I mentioned above.

from adafruit_tinyusb_arduino.

Both repositories are updated. The USB side appears to work correctly. The raw device has the identical descriptors with the old and new code, when only one wire/jack is used. I can send messages to all ports and receive them, but I can't distinguish them inside the MIDI device.

To make use of the wire/jack data, tinyusb MIDI needs to be extended/changed. If I read the code correctly, the current tinyusb MIDI device code throws away the first byte of the USB packet and the information about the cable/jack number gets lost:

cores/arduino/Adafruit_TinyUSB_Core/tinyusb/src/class/midi/midi_device.c
tu_fifo_write_n(&midi->rx_ff, &buffer[i + 1], count);

The FortySevenEffects MIDI library uses the streaming format because its focus is the legacy serial line MIDI format. The Arduino MIDIUSB library always had the full USB packet to read.

The future/upcoming MIDI2.0 protocol is a pure packet-based transport, there is no streaming support at all, and it will not work over legacy serial lines. It would be nice to have a raw interface to the packets instead of stuffing it into the old serial line format.

@hathach any ideas what's the best way forward here? Can we change the tinyusb MIDI buffering to store all 4 bytes in the FIFO and move the packing of the legacy streaming format to the read() method?

from adafruit_tinyusb_arduino.

@kaysievers thank you, this is a great addition to add. I am totally OK to change to writing full 4 bytes format and move away from streaming format as well. In fact, I am open to alter all the usb stack with a good reason. Though I am not too familiar with MIDI usage, do you happen to know an library to conveniently replace FortySevenEffects when we switch to the packet format.

What I need is only an proof-of-concept example to work upon, would you mind adding a new midi example based on this https://github.com/adafruit/Adafruit_TinyUSB_Arduino/blob/master/examples/MIDI/midi_test/midi_test.ino which

• Has 2 virtual pair (in & out) of ports ( more is better but 2 is convincing enough)
• Each output port has an distinguish note sequence so that we could notice the difference when attaching speaker to chanel0, chanel1
• If possible: library to replace FortySevenEffects to parse incoming event for each chanel.

Make all changes that you think is needed, then submit your PR on both 2 repos. I will review, merge, and will refactor if needed. Also tested to make sure it works on other platform such as nrf5x and its bare metal C example in my core repo as well. Let's me know if you need an more detail or clarification on the usb stack.

Notice: don't use C++ static_cast with file inside Adafruit_TinyUSB_Core/tinyusb/src, those are intended to compile with both C and C++.

from adafruit_tinyusb_arduino.

do you happen to know an library to conveniently replace FortySevenEffects when we switch to the packet format

For my own use, I have a tiny wrapper around MIDIUSB in my devices. I would be totally fine to just port that to a new Adafruit_USBD_MIDI device packet interface and not use any another library.

The code to convert from packets to a stream and the stream class interface for the FortySevenEffects lib is probably still the easiest way for people to build devices. At least as long as there are no need to support MIDI2.0 protocol devices.

If we add a new packet interface, would you like to keep the stream conversion code in the tinyusb core code or move it to the Adafruit_USBD_MIDI code?

from adafruit_tinyusb_arduino.

Here is a packet interface for tinyusb, the streaming methods do the packet mangling now, the FIFO stores the original packets:
https://github.com/kaysievers/ArduinoCore-samd/tree/tinyusb-midi-packets

Here is the Adafruit_TinyUSB part:
https://github.com/kaysievers/Adafruit_TinyUSB_Arduino/tree/midi-packet-interface

from adafruit_tinyusb_arduino.

With all the 4 branches applied:
https://github.com/kaysievers/Adafruit_TinyUSB_Arduino/tree/midi-jacks
https://github.com/kaysievers/ArduinoCore-samd/tree/tinyusb-midi-jacks
https://github.com/kaysievers/ArduinoCore-samd/tree/tinyusb-midi-packets
https://github.com/kaysievers/Adafruit_TinyUSB_Arduino/tree/midi-packet-interface

Three virtual ports are requested:

  Adafruit_USBD_MIDI V2MIDI(3);

No further MIDI library is used, it just sends the raw USB packet to the Adafruit_USBD_MIDI device on the virtual port 1:

        uint8_t msg[] = {
          0x10 | MIDI_STATUS_NOTE_ON,
          (uint8_t)((MIDI_STATUS_NOTE_ON << 4) | 0),
          (uint8_t)MIDI_PERCUSSION_ACOUSTIC_SNARE,
          (uint8_t)FSR.note_velocity
        };

        V2MIDI.send(msg, 4);

And two USB packets received note-on ,note-off and the first byte correctly shows virtual port 2:

2-9 144 71 78
2-8 128 71 64

The OS and the music application recognizes the 3 separate ports of the device properly:

@hathach, it would be nice if you can have a look over the code, if it looks fine I can submit the PRs.

from adafruit_tinyusb_arduino.

Here is an example sketch that uses the virtual ports:

Every port continuously sends out notes:
Port #1, channel 6: monochord (single tone)
port #2, channel 9: dichord (two tones)
port #3, channel 12: trichord (three tones)

Notes received on the ports let the built-in LED blink one, two,
or three times.

https://github.com/kaysievers/Adafruit_TinyUSB_Arduino/tree/midi-jacks-example

Tested on ItsyBitsy M0.

from adafruit_tinyusb_arduino.

@kaysievers sorry, I have been busy with other thing lately, will test this out soon enough :)

from adafruit_tinyusb_arduino.

Here is the counterpart which adds an interface to request more than one MIDI port/wire.

#31

from adafruit_tinyusb_arduino.

@kaysievers thank you very much for your contribution for the issue and PR. Though I am currently in the middle of other works. I will try to review and work on this as soon as I could.

from adafruit_tinyusb_arduino.

@kaysievers I should be the one to say thanks for your work, and your patient. I should have done this earlier :) .

from adafruit_tinyusb_arduino.

@kaysievers I am really sorry if I did say something offended. I am really appreciated your work and PR to push forward the MIDI library and other USB related PR. Unfortunately MIDI is not my expertise at all. Therefore I am very slow catching up with your PR (while still struggling with other long overdue works :( )

Currently, Tinyusb files are all over the places, we are trying to re-organize the Arduino Tinyusb file in BSP core folder into its own repo as a submodule at
https://github.com/adafruit/Adafruit_TinyUSB_ArduinoCore
These files are used by both SAMD and nRF core and maybe other cores such as STM32 in the future as well. And it is a bit of headache (and error-prone) to keep them synced together with the original stack project as it is now. I have submitted 2 PR for the re-org
adafruit/ArduinoCore-samd#190 (waiting for review)
adafruit/Adafruit_nRF52_Arduino#396 (merged)

I probably confuse you when saying the issue is not target the right repo since above PR is not merged yet, but once it is merged the tinyusb files aren't located on the samd core anymore. I hope that I explain it clearly now. Hopefully you would change your mind and continue to submit more awesome PRs to https://github.com/adafruit/Adafruit_TinyUSB_ArduinoCore or https://github.com/hathach/tinyusb (original stack).

Thanks for all the PRs so far

from adafruit_tinyusb_arduino.

thank you very much, I am looking forward to see your upcoming PRs :)

from adafruit_tinyusb_arduino.

Here are the updated patches for the tinyusb part:
hathach/tinyusb#258

from adafruit_tinyusb_arduino.

This issue is already supported by series of recent PRs. We have release BSP SAMD 1.5.12 and this library 0.9.0 @kaysievers Please checkout the latest version to see if we could close this by now. Thanks

from adafruit_tinyusb_arduino.

I've updated this library and it seems to work fine.

Then I've updated ArduinoCore-samd and Adafruit_TinyUSB_ArduinoCore, and the MIDI devices work fine for simple messages. But as soon as I send a large SysEx to the device, the device reads it, and replies with a large Sysex which fails to reach the host.

It seems that streaming-out larger data over MIDI does not work the same way with the updated tinyusb code. I'll try to figure it out what's going on ...

from adafruit_tinyusb_arduino.

It appears streaming out data, sending MIDI packets in a loop causes problems now. It seems like a race condition, when I add Serial.println() statements to the loop, it works approximately half of the time, the behavior is not really predictable.

Do you have an idea if there have been related changes in tinyusb?

from adafruit_tinyusb_arduino.

I see the outgoing FIFO filled up, and not flushed to the host.

If I add this:
https://github.com/versioduo/Adafruit_TinyUSB_ArduinoCore/tree/versioduo

it works without the added timeouts or debug prints, but half of the SysEx (~4kb blobs in size) messages are still corrupted at random positions.

If I revert to the old tinyusb code before the sync, with the same MIDI code on top, all is fine.

from adafruit_tinyusb_arduino.

@kaysievers this happens with the tinyusb stack repo as well. I will close this once since this is for multiple cables issue. I will open an issue for discussion to fix this on tinyusb repo.

hathach/tinyusb#377

from adafruit_tinyusb_arduino.