RP2040 MIDI Host

[atoktoto]

This pull request replaces the previous draft: #1605

This PR adds MIDI host support for RP2040 boards based on work done by @rppicomidi here: https://github.com/rppicomidi/tinyusb/tree/pio-midihost and @Skyler84 in #1434.

It seems to work pretty OK and has no issue with hot-plugging (re-plugging) the device that #1605 had.

[todbot]

About my above comments, since there wasn't an example included in this PR, I took the previous PR's examples/host/midi and tried compiling it.

[todbot]

Still having problems compiling this.
Since there was no Makefile in the PR example, I added the default minimal one:

include ../../../tools/top.mk
include ../../make.mk
INC += \
	src \
	$(TOP)/hw \
# Example source
EXAMPLE_SOURCE += $(wildcard src/*.c)
SRC_C += $(addprefix $(CURRENT_PATH)/, $(EXAMPLE_SOURCE))
include ../../rules.mk

And then I could follow the standard tinyusb build steps but with this PR. Specifically I did:

git clone https://github.com/hathach/tinyusb tinyusb-testmidihost
cd tinyusb-testmidihost
gh pr checkout 1627
git submodule update --init lib
cd examples/host/midi_rx
make BOARD=raspberry_pi_pico get-deps
make BOARD=raspberry_pi_pico all

This fails because all of the unused variables in host/midi_rx/src/main.c if no logging. So, using LOG=1 workaround, I recompile:

make BOARD=raspberry_pi_pico clean && make BOARD=raspberry_pi_pico LOG=1 all

and get the error:

[...]
Scanning dependencies of target midi_rx
make[3]: Leaving directory '/Users/tod/projects/tinyusb/tinyusb-testmidihost/examples/host/midi_rx/_build/raspberry_pi_pico'
make[3]: Entering directory '/Users/tod/projects/tinyusb/tinyusb-testmidihost/examples/host/midi_rx/_build/raspberry_pi_pico'
[ 16%] Building C object CMakeFiles/midi_rx.dir/src/main.c.obj
<command-line>: error: no macro name given in #define directive
compilation terminated due to -Wfatal-errors.
make[3]: *** [CMakeFiles/midi_rx.dir/build.make:76: CMakeFiles/midi_rx.dir/src/main.c.obj] Error 1

Any clues as to what I'm doing wrong?

[atoktoto]

Thanks for being persistent on this one. I'm new at this and was not even aware of the standard way to build TinyUSB.
I'm building this starting from pico-examples CMakeLists.txt as the project includes both pico-sdk and TinyUSB examples.

[todbot]

Do you have a specific recommended build process for this PR?

I have tried both using standard Pico build process inside of midi_rx and moving midi_rx to pico-examples and building there. Specifically I have tried:

cd tinyusb-testmidihost/examples/host/midi_rx
mkdir build
cd build
cmake ..

And I have tried:

cd pico_examples
cp -a ../tinyusb-testmidihost/examples/host/midi_rx .
echo "add_subdirectory(midi_rx)" >> CMakeLists.txt
mkdir build
cd build
cmake ..

[atoktoto]

Generally I'm following this guide: https://datasheets.raspberrypi.com/pico/getting-started-with-pico.pdf and building from CLion IDE, but the commands it's running are:

cmake.exe -G Ninja -S E:\pico\pico-examples -B E:\pico\pico-examples\build
cmake.exe --build E:\pico\pico-examples\build --target tinyusb_host_midi_rx -j 9

(I ommited the env variables specifying the paths to compilers for the second command)

For any other example it would be as simple as:
cd pico-examples
mkdir build
cd build
cmake ..
cd blink (in the build directory)
make -j4

But the TinyUSB examples are pulled in from outside of project root and I don't know where to look for CMake outputs of those.

[todbot]

Seems odd to me you're not following standard tinyusb build process for this.

[atoktoto]

In the meantime (while waiting for #1434 to be merged) I started on adding a nice-looking MIDI API. Turns out the simplest way is to reuse https://github.com/FortySevenEffects/arduino_midi_library. Fortunately it does not depend on Arduino itself and has a transport plug-in architecture.

I wrote a TinyUSB based tramsport plugin for it: https://github.com/atoktoto/pico-midi-usb-transport

void onNote(Channel channel, byte note, byte velocity) {
    printf("Note ON ch=%d, note=%d, vel=%d\n", channel, note, velocity);
}

MIDI.setHandleNoteOn(onNote);

[atoktoto]

@todbot On my machine (Windows, under WSL2) the example now builds with make BOARD=raspberry_pi_pico all in the example directory.

I also verified that the example works as intended when the resulting uf2 is dropped onto RP2040.

I did not get the no macro name given in #define directive error at any point so not sure where to look for the issue.

[todbot]

@atoktoto, I also can get it to compile now (with similar Makefile and changes as your commits, though I had to also fix casts on int -> uint conversion errors cable_num in midi_host.c). Not sure what that no macro name error was but I nuked the directory, recloned and re-pulled and that error is gone.

The pico-midi-usb-transport library is great! The 47effects MIDI library is wonderful.

The CMake files of pico-midi-usb-transport reference to a pico-midi-usb project, like:

add_library(pico-midi-usb INTERFACE)

Where does the pico-midi-usb CMake file come from? (Apologies I don't understand CMake very well)

[atoktoto]

That's a naming inconsistency on my side.

The key fact here is the name of the project is not important. AFAIK add_library(pico-midi-usb INTERFACE) creates a target that can be linked to with target_link_libraries(example-print-notes pico-midi-usb).

[paulhamsh]

I am keen to get this in the master branch so I can use it properly - is this stuck somehow, and can I help?

Seems a failure on this:

/home/runner/work/tinyusb/tinyusb/src/class/midi/midi_host.c:595:29: error: conversion from 'int' to 'uint8_t' {aka 'unsigned char'} may change value [-Werror=conversion]
  595 |         stream->buffer[0] = (cable_num << 4) | msg;
      |                             ^
compilation terminated due to -Wfatal-errors.

I can't really tell why it converts two uint8_t into an int, but wouldn't this fix it?

  595 |         stream->buffer[0] = (uint8_t) ((cable_num << 4) | msg);

[rppicomidi]

@atoktoto I found a fix for hubs not working with this. I made the error in my original pull request. Would you consider adding this change to your pull request?

diff --git a/src/class/midi/midi_host.c b/src/class/midi/midi_host.c
index d242d33c5..bc87898cc 100644
--- a/src/class/midi/midi_host.c
+++ b/src/class/midi/midi_host.c
@@ -490,7 +490,7 @@ bool midih_set_config(uint8_t dev_addr, uint8_t itf_num)
   p_midi_host->configured = true;
 
   // TODO I don't think there are any special config things to do for MIDI
-
+  usbh_driver_set_config_complete(dev_addr, p_midi_host->itf_num);
   return true;
 }

[atoktoto]

Sure, I will to that tonight.

[rppicomidi]

@hathach Set CFG_MIDI_HOST_DEVSTRINGS to 1 to process the virtual cable names of the attached device for MIDI IN and MIDI OUT from the MIDI configuration descriptors and the string descriptors during enumeration and to enable the API for applications to read the strings. For example, on my Arturia KeyLab Essential 88 keyboard, virtual cable 0 is labeled "MIDI" and virtual cable 1 is labeled "DAW" for both MIDI IN and MIDI OUT.

[rppicomidi]

@hathach I recommend that you reference the usb_midi_host project when merging this pull request. I have been maintaining it for quite a while and have fixed a number of issues since this pull request was submitted.

[hathach]

@hathach I recommend that you reference the usb_midi_host project when merging this pull request. I have been maintaining it for quite a while and have fixed a number of issues since this pull request was submitted.

Ah thank you for the head up, I will try to merge this first, then do compare and update later with follow-up PR afterwards. I don't own/use any midi comercial at all, therefore it is all up to you and others to fix its issues. I mostly make sure it is up-to-date with stack API, compiling with other ports and working with simple examples (like midi_rx).

@hathach Set CFG_MIDI_HOST_DEVSTRINGS to 1 to process the virtual cable names of the attached device for MIDI IN and MIDI OUT from the MIDI configuration descriptors and the string descriptors during enumeration and to enable the API for applications to read the strings. For example, on my Arturia KeyLab Essential 88 keyboard, virtual cable 0 is labeled "MIDI" and virtual cable 1 is labeled "DAW" for both MIDI IN and MIDI OUT.

Thank you for explanation

[hathach]

PS: I made more changes to the API and its signature and feeling good for now. I will check the string thing later, need to switch to other works for now. So the string is mostly for displaying the label ? Besides that does it has any behavior or setting impact ?

[rppicomidi]

@hathach Yes, CFG_MIDI_HOST_DEVSTRINGS is for displaying labels for virtual cables. Enabling the setting requires slightly more RAM and code space. Enabling it slows enumeration slightly because the driver needs to parse the Jack Descriptors. The setting does not change run-time behavior after enumeration. Applications with no UI do not need it.

Many MIDI devices do not implement Jack Descriptor strings. Some implement them, but do it poorly. An example RP2040 device that uses Jack Descriptor strings is midi-multistream2usbdev. You may find that project useful to test the Jack Strings when you get to that.

[hathach]

@rppicomidi thank you for your explanation, if string is only used for displaying label. I think we can

• add an optional callback that have all these information when parsing descriptor instead of storing them in the interface. If application need these, they should store it.
• drop it entirely and let the parsing to application, we can expose midi interface descriptor via callback

Making that change now

[hathach]

Thank you very much everyone for putting time and effort for this PR. Especially @atoktoto and @rppicomidi for getting most of the work done. I am sorry for being super late, I have to constant switching works and haven't worked much with rp2040 until now.

Though since there is lots of changes since this PR is created, I have update and also make lots of changes, notably:

• lost of API rename and signature changes to match other class driver consistency e.g num_rx_cables --> rx_cable_count
• remove the devstring and added tuh_midi_descriptor_cb() along with callback data to leave extracting midi jack info to application if needed
• use the new endpoint stream API for streamging data
• allow to turn off stream API CFG_TUH_MIDI_STREAM_API=0 (which consume lots of logic/footprint). Since more recent high level MIDI library can support multiple cables and can parse 4-byte usb packet natively
• change the API input from dev_addr --> interface index, consistency with cdc, but also allow to support more than 1 midi interface per device (rare case).

It is quite a bit of changes, and may break compatible with existing host driver from application space, @rppicomidi let me know what you think and how you want to chagne/resolve the confict. I am open to all feedback, thank you.

[rppicomidi]

@hathach I am just back from travel. I may need a few days to get you feedback. Sorry for the delay.

[hathach]

@hathach I am just back from travel. I may need a few days to get you feedback. Sorry for the delay.

no problems, please take your time.

[rppicomidi]

The midi_rx example does not build. I tried the build with VS Code using the Raspberry Pi pico-examples project and pico-sdk 2.1.1. The reasons the build failed are likely out of scope for this pull request. I am writing them here because I do not know where else to file them.

It does not build for two reasons:

1. The pico-sdk file src/rp2_common/tinyusb/CMakeLists.txt sets BOARD to pico-sdk, which does not work with the latest TinyUSB code. Even the develop branch of the pico-sdk does not set BOARD and FAMILY based on the PICO_BOARD setting for this CMakeLists.txt file; it also does not test if BOARD or FAMILY are already defined on the cmake command line. I believe this is something that needs to be worked out between the Raspberry Pi pico-sdk project and this one.
2. There are 3 examples in the TinyUSB project whose CMakeLists.txt files call family_configure_device_example(${EXE_NAME} ${RTOS}) when they should call family_configure_device_example(${EXE_NAME} noos). The files are examples in the examples/device directory: board_test, msc_dual_lun and cdc_msc. Because these files do not build, the whole pico-examples project appears broken.

Are these known issues? I did not see bugs filed, but it may be my search. People who follow the Raspberry Pi Pico build workflow from current Raspberry Pi Pico C/C++ documentation will have trouble here and may give up.

I will try to get to a test on hardware and a review of the actual driver when I am able.

[hathach]

@rppicomidi how to do build the example, I normally just go to tinyusb/exmaples/ and cmake there. I surely can try to make it backward compatible.

PS: if you create an empty board.h inside the hw/bsp/rp2040/boards/pico_sdk, will it fix the compile issue ?

[rppicomidi]

@hathach Refer to issue #3004 for reproduce steps. Yes, putting an empty board.h file in the pico-sdk directory seems to allow it to build.

[hathach]

@hathach Refer to issue #3004 for reproduce steps. Yes, putting an empty board.h file in the pico-sdk directory seems to allow it to build.

I don't use vscode, but if this fix the compile issue, let just do it. I guess I miss this board config

[rppicomidi]

Also need to fix 3 CMakeList.txt files. I added command line steps to reproduce to #3004.