feat: Add USB-MIDI 2.0 Device and Host class drivers#3571
feat: Add USB-MIDI 2.0 Device and Host class drivers#3571sauloverissimo wants to merge 7 commits intohathach:masterfrom
Conversation
Add native USB-MIDI 2.0 Device class driver to TinyUSB. Implements the USB-MIDI 2.0 specification with both Alt Setting 0 (MIDI 1.0 fallback) and Alt Setting 1 (UMP native) descriptor support. Driver features: - UMP (Universal MIDI Packet) read/write with atomic message framing - Protocol negotiation: Endpoint Discovery, Config Request/Notify, Function Block Discovery (embedded in driver) - Group Terminal Block descriptor via GET_DESCRIPTOR - Alt Setting switch handler with endpoint re-arm - Static allocation, no dynamic memory, ISR-safe Build system: - Register midi2d_* in usbd.c driver table - Add TUD_MIDI2_DESCRIPTOR macros to usbd.h - Add config defaults (CFG_TUD_MIDI2_*) to tusb_option.h - Add midi2_ump_word_count() to midi.h (shared by Device and Host) - Add midi2_device.c/h to family.cmake and CMakeLists.txt - Add midi2_device.h include to tusb.h All changes guarded by #if CFG_TUD_MIDI2 (default 0). Zero impact on existing drivers and examples. Tested: Raspberry Pi Pico (RP2040), Linux ALSA, Windows MIDI Services
Add native USB-MIDI 2.0 Host class driver to TinyUSB. Implements reactive architecture: enumerate, detect MIDI 2.0 capability, inform application via callbacks. Driver features: - Parse both Alt Setting 0 (MIDI 1.0) and Alt Setting 1 (UMP) - Detect bcdMSC version from descriptor - Auto-select highest protocol (Alt 1 preferred if available) - UMP read/write via endpoint streams - Proper Audio Control interface skip (loop-based, following midi_host.c pattern) - Endpoint open with tuh_edpt_open/tu_edpt_stream_open/clear - usbh_driver_set_config_complete for USBH state machine - Handle Audio Control itf_num in set_config gracefully - 5 weak callback stubs (descriptor, mount, unmount, rx, tx) Build system: - Register midih2_* in usbh.c driver table - Add midi2_host.h include to tusb.h - Add CFG_TUH_MIDI2_LOG_LEVEL to tusb_option.h All changes guarded by #if CFG_TUH_MIDI2 (default 0). Zero impact on existing drivers and examples. Tested: Waveshare RP2350-USB-A (Host) receiving UMP from Raspberry Pi Pico (Device) via PIO-USB, board-to-board
Add unit tests for MIDI 2.0 drivers: - Device: UMP word count (all 16 message types), descriptor macro validation (length, byte layout, alt settings, endpoints), CS endpoint subtypes, traversal integrity - Host: UMP word count, callback struct validation, CS endpoint subtypes Also add Sphinx documentation for MIDI 2.0 class drivers (Device and Host API reference, lifecycle, configuration, examples). Tests: 60/60 PASS (FIFO 26/26, USBD 5/5, MIDI2 Device 18/18, MIDI2 Host 6/6, USBD internal 5/5)
Add Device example that plays Twinkle Twinkle Little Star using native UMP format. Demonstrates all MIDI 2.0 Channel Voice message types: 16-bit velocity, 32-bit CC, 32-bit pitch bend, 32-bit channel/poly pressure, per-note management, program change with bank select, and JR timestamps. USB descriptor exposes both Alt Setting 0 (MIDI 1.0) and Alt Setting 1 (UMP) per USB-MIDI 2.0 specification. Tested on: Raspberry Pi Pico (RP2040), Linux (ALSA), Windows (MIDI Services)
Add Host example that receives UMP from a MIDI 2.0 Device via PIO-USB and displays received messages on a SSD1306 OLED (I2C, 128x64). Features: - Boot checklist on display (PWR, TinyUSB, USB bus, Device, Descriptor, Alt Setting UMP, Mount, Receiving) - Decode and display all MIDI 2.0 Channel Voice messages - PIO-USB Host on rhport 1 (GP12/GP13 for Waveshare RP2350-USB-A) - SSD1306 display via I2C0 (GP4=SDA, GP5=SCL) Also add board definition for Waveshare RP2350-USB-A. Tested: Waveshare RP2350-USB-A (Host) receiving UMP from Raspberry Pi Pico (Device) via USB cable, notes displayed on SSD1306 OLED
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d5c50a274
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/class/midi/midi2_host.c
Outdated
| midih2_parse_descriptors_alt1(p_midi, desc_itf, desc_end); | ||
| } | ||
|
|
||
| return max_len; |
There was a problem hiding this comment.
midih2_open() currently returns max_len unconditionally, but USB host enumeration expects each class driver to return only the number of descriptor bytes it consumed. In enum_parse_configuration_desc() (src/host/usbh.c), this return value is used to bind interfaces/endpoints to the class and advance parsing; returning the full remaining length makes MIDI2 claim all following interfaces in the configuration, breaking composite devices that have additional functions after MIDI2.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. Fixed in 9f1001e -- midih2_open() now returns the actual parsed length. Both parsers (alt0/alt1) were refactored to return a const uint8_t* end pointer, and open() computes the delta. Follows the same pattern as midih_open() in midi_host.c.
src/class/midi/midi2_device.c
Outdated
| if (dtype != TUSB_DESC_CS_INTERFACE && dtype != TUSB_DESC_CS_ENDPOINT && | ||
| dtype != TUSB_DESC_INTERFACE && dtype != TUSB_DESC_ENDPOINT) { | ||
| break; |
There was a problem hiding this comment.
The descriptor-skip loop in midi2d_open() continues through any TUSB_DESC_INTERFACE, not just MIDI2 alternate-setting descriptors. During process_set_config() (src/device/usbd.c), the returned length is used to bind all enclosed interfaces/endpoints to this driver, so this logic can consume unrelated interfaces that follow MIDI2 in a composite device and prevent their own class drivers from being opened.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9f1001e. The skip loop now checks bInterfaceNumber and stops at any interface that doesn't belong to our MIDI Streaming interface, preventing consumption of unrelated interfaces in composite devices.
src/class/midi/midi2_host.c
Outdated
| // Signal USBH that configuration is complete | ||
| usbh_driver_set_config_complete(dev_addr, itf_num); |
There was a problem hiding this comment.
midih2_set_config() selects alt_setting_current and immediately completes configuration without issuing a standard SET_INTERFACE control request for alternate setting 1. USB interfaces default to alt 0, so MIDI2-capable devices can be reported as mounted while still running alt 0 semantics, which prevents proper MIDI2/UMP operation on devices that require alt 1 activation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9f1001e. midih2_set_config() now issues tuh_interface_set() to activate Alt Setting 1, with an async callback (midih2_set_interface_cb) that falls back to Alt 0 if the request fails. Mount is only reported after the control transfer completes.
There was a problem hiding this comment.
Pull request overview
Adds USB-MIDI 2.0 support to TinyUSB by introducing new device/host class drivers, descriptor templates, configuration defaults, documentation, examples, and unit tests—while keeping the feature opt-in via CFG_TUD_MIDI2 / CFG_TUH_MIDI2.
Changes:
- Introduce new USB-MIDI 2.0 device and host class drivers (
midi2_device.*,midi2_host.*) and register them with USBD/USBH. - Add USB-MIDI 2.0 descriptor templates/macros and new configuration defaults for buffers/EP sizing.
- Add examples (device + host) plus unit tests and reference documentation for the new drivers.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit-test/test/host/midi2/test_midi2_host.c | Adds unit tests for shared UMP helper and callback struct field layout. |
| test/unit-test/test/device/midi2/test_midi2_device.c | Adds unit tests for UMP helper and descriptor macro composition/bytes. |
| src/tusb_option.h | Adds default config knobs for MIDI 2.0 device/host. |
| src/tusb.h | Exposes new MIDI2 headers behind CFG_TUD_MIDI2 / CFG_TUH_MIDI2. |
| src/host/usbh.c | Registers the new MIDI2 host class driver. |
| src/device/usbd.h | Adds TUD_MIDI2_DESCRIPTOR and related MIDI2 descriptor templates. |
| src/device/usbd.c | Registers the new MIDI2 device class driver. |
| src/class/midi/midi2_host.h | Declares the MIDI2 host public API and callback structs. |
| src/class/midi/midi2_host.c | Implements MIDI2 host enumeration + stream I/O API. |
| src/class/midi/midi2_device.h | Declares the MIDI2 device public API and callbacks. |
| src/class/midi/midi2_device.c | Implements MIDI2 device streaming + negotiation + GTB descriptor handling. |
| src/class/midi/midi.h | Adds shared midi2_ump_word_count() helper. |
| src/CMakeLists.txt | Adds MIDI2 sources to the core TinyUSB build. |
| hw/bsp/rp2040/family.cmake | Adds MIDI2 sources to the RP2040 family build targets. |
| hw/bsp/rp2040/boards/waveshare_rp2350_usb_a/board.h | Adds a new RP2350 USB-A host board definition stub. |
| hw/bsp/rp2040/boards/waveshare_rp2350_usb_a/board.cmake | Adds board CMake metadata for Waveshare RP2350-USB-A. |
| examples/host/midi2_host/src/tusb_config.h | Host example configuration enabling CFG_TUH_MIDI2. |
| examples/host/midi2_host/src/main.c | Host example that receives/decodes UMP and displays status/logs. |
| examples/host/midi2_host/src/font5x7.h | Adds a minimal font for the OLED example UI. |
| examples/host/midi2_host/src/display.h | Declares SSD1306 helper API for the host example. |
| examples/host/midi2_host/src/display.c | Implements SSD1306 text UI (checklist/log/status). |
| examples/host/midi2_host/CMakeLists.txt | Builds the host example (RP2350 PIO-USB + I2C display). |
| examples/host/CMakeLists.txt | Registers the new midi2_host example. |
| examples/device/midi2_device/src/usb_descriptors.c | Device example descriptors using TUD_MIDI2_DESCRIPTOR. |
| examples/device/midi2_device/src/tusb_config.h | Device example configuration enabling CFG_TUD_MIDI2. |
| examples/device/midi2_device/src/main.c | Device example sending MIDI 2.0 UMP “Twinkle Twinkle” sequence. |
| examples/device/midi2_device/README.md | Documents the new device example and testing steps. |
| examples/device/midi2_device/CMakeLists.txt | Builds the device example. |
| examples/device/CMakeLists.txt | Registers the new midi2_device example. |
| docs/reference/index.rst | Adds class_drivers to the reference docs TOC. |
| docs/reference/class_drivers.rst | Adds reference documentation for MIDI 2.0 device + host drivers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/class/midi/midi2_host.c
Outdated
| const midi_desc_cs_endpoint_t *p_csep = (const midi_desc_cs_endpoint_t *) p_desc; | ||
|
|
||
| if (tu_edpt_dir(p_ep->bEndpointAddress) == TUSB_DIR_OUT) { | ||
| tx_cable_count = p_csep->bNumEmbMIDIJack; | ||
| } else { | ||
| rx_cable_count = p_csep->bNumEmbMIDIJack; |
There was a problem hiding this comment.
In the Alt 1 descriptor parser, the CS_ENDPOINT subtype is MIDI_CS_ENDPOINT_GENERAL_2_0, but the code casts it to midi_desc_cs_endpoint_t and reads bNumEmbMIDIJack. For USB-MIDI 2.0 this field represents Group Terminal Block association (bNumGrpTrmBlk and following block IDs), not embedded jack/cable count, so the parsed rx/tx counts (and callback fields) are likely incorrect. Define/parse the correct USB-MIDI 2.0 CS endpoint descriptor layout and expose the appropriate GTB/group information (or rename fields if you intend to surface GTB count).
| const midi_desc_cs_endpoint_t *p_csep = (const midi_desc_cs_endpoint_t *) p_desc; | |
| if (tu_edpt_dir(p_ep->bEndpointAddress) == TUSB_DIR_OUT) { | |
| tx_cable_count = p_csep->bNumEmbMIDIJack; | |
| } else { | |
| rx_cable_count = p_csep->bNumEmbMIDIJack; | |
| // For MIDI 2.0, the CS Endpoint General 2.0 descriptor uses bNumGrpTrmBlk | |
| // at offset 4 instead of the MIDI 1.0 bNumEmbMIDIJack. Use protocol_version | |
| // to decide which layout to apply. | |
| if (p_midi->protocol_version == 1) { | |
| const uint8_t *p_csep = p_desc; | |
| // Ensure descriptor is long enough to contain bNumGrpTrmBlk (offset 4). | |
| if (p_csep[0] >= 5) { | |
| uint8_t num_grp_trm_blk = p_csep[4]; | |
| if (tu_edpt_dir(p_ep->bEndpointAddress) == TUSB_DIR_OUT) { | |
| tx_cable_count = num_grp_trm_blk; | |
| } else { | |
| rx_cable_count = num_grp_trm_blk; | |
| } | |
| } | |
| } else { | |
| const midi_desc_cs_endpoint_t *p_csep = (const midi_desc_cs_endpoint_t *) p_desc; | |
| if (tu_edpt_dir(p_ep->bEndpointAddress) == TUSB_DIR_OUT) { | |
| tx_cable_count = p_csep->bNumEmbMIDIJack; | |
| } else { | |
| rx_cable_count = p_csep->bNumEmbMIDIJack; | |
| } |
There was a problem hiding this comment.
Fixed in 9f1001e. Alt 1 parser now reads bNumGrpTrmBlk at offset 3 with a MIDI_CS_ENDPOINT_GENERAL_2_0 subtype check, instead of casting to the MIDI 1.0 midi_desc_cs_endpoint_t layout.
src/device/usbd.h
Outdated
| #define TUD_MIDI2_DESC_ALT1_HEAD_LEN (9 + 7) | ||
| #define TUD_MIDI2_DESC_ALT1_HEAD(_itfnum, _stridx) \ | ||
| /* MIDI Streaming Interface, Alt Setting 1 */\ | ||
| 9, TUSB_DESC_INTERFACE, (uint8_t)((_itfnum) + 1), 1, 2, TUSB_CLASS_AUDIO, AUDIO_SUBCLASS_MIDI_STREAMING, AUDIO_FUNC_PROTOCOL_CODE_UNDEF, _stridx,\ |
There was a problem hiding this comment.
TUD_MIDI2_DESC_ALT1_HEAD sets the Alt 1 MS Header wTotalLength to 7 (header-only) and uses _stridx as iInterface for the streaming interface. In the existing MIDI 1.0 template, wTotalLength covers the full MS descriptor set for the interface and iInterface for the streaming interface is 0. Consider updating wTotalLength to include the Alt 1 class-specific descriptors (and keep iInterface consistent with Alt 0), or splitting the parameters so Audio Control and MIDI Streaming string indices are not conflated.
| 9, TUSB_DESC_INTERFACE, (uint8_t)((_itfnum) + 1), 1, 2, TUSB_CLASS_AUDIO, AUDIO_SUBCLASS_MIDI_STREAMING, AUDIO_FUNC_PROTOCOL_CODE_UNDEF, _stridx,\ | |
| 9, TUSB_DESC_INTERFACE, (uint8_t)((_itfnum) + 1), 1, 2, TUSB_CLASS_AUDIO, AUDIO_SUBCLASS_MIDI_STREAMING, AUDIO_FUNC_PROTOCOL_CODE_UNDEF, 0,\ |
There was a problem hiding this comment.
Fixed in 9f1001e. wTotalLength now uses TUD_MIDI2_DESC_ALT1_CS_LEN(_numgtbs) which accounts for the MS Header plus both CS Endpoint descriptors. iInterface for the streaming interface is set to 0, consistent with the MIDI 1.0 template.
src/device/usbd.h
Outdated
| #define TUD_MIDI2_DESC_ALT1_EP_LEN(_numgtbs) (7 + 4 + (_numgtbs)) | ||
| #define TUD_MIDI2_DESC_ALT1_EP(_ep, _epsize, _numgtbs) \ | ||
| 7, TUSB_DESC_ENDPOINT, _ep, TUSB_XFER_BULK, U16_TO_U8S_LE(_epsize), 0, \ | ||
| (uint8_t)(4 + (_numgtbs)), TUSB_DESC_CS_ENDPOINT, MIDI_CS_ENDPOINT_GENERAL_2_0, _numgtbs |
There was a problem hiding this comment.
TUD_MIDI2_DESC_ALT1_EP(_numgtbs) sets bLength to (4 + _numgtbs) but the macro itself only emits the 4-byte CS endpoint header; the required _numgtbs bytes of bAssoGrpTrmBlkID must be appended manually. Since the macro signature suggests it is self-contained, it is easy to generate malformed descriptors when _numgtbs != 1. Consider either (a) hard-coding the template to 1 GTB and removing the parameter, or (b) extending the macro to accept/emit the GTB ID list so the output size matches the declared bLength.
There was a problem hiding this comment.
Fixed in 9f1001e. The macro now accepts GTB ID bytes via variadic arguments and emits them inline, so bLength matches the actual output. Usage: TUD_MIDI2_DESC_ALT1_EP(_ep, _epsize, 1, 1 /* bAssoGrpTrmBlkID */).
| case TUSB_REQ_SET_INTERFACE: { | ||
| uint8_t itf_num = tu_u16_low(request->wIndex); | ||
| uint8_t alt = tu_u16_low(request->wValue); | ||
|
|
||
| uint8_t idx = find_midi2_itf_by_num(itf_num); | ||
| if (idx >= CFG_TUD_MIDI2) return false; | ||
|
|
||
| midi2d_interface_t* p_midi = &_midi2d_itf[idx]; | ||
| p_midi->alt_setting = alt; | ||
|
|
||
| tu_edpt_stream_clear(&p_midi->ep_stream.rx); | ||
| tu_edpt_stream_clear(&p_midi->ep_stream.tx); | ||
|
|
||
| if (alt == 1) { | ||
| p_midi->negotiated = false; | ||
| p_midi->protocol = MIDI_PROTOCOL_MIDI2; | ||
| } | ||
|
|
||
| // Re-arm RX endpoint for receiving data after alt setting change | ||
| tu_edpt_stream_read_xfer(&p_midi->ep_stream.rx); | ||
|
|
||
| tud_midi2_set_itf_cb(idx, alt); | ||
| tud_control_status(rhport, request); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
midi2d_control_xfer_cb handles SET_INTERFACE but accepts any alt value and updates p_midi->alt_setting without validation. Since this interface only defines Alt 0 and Alt 1, unsupported alternate settings should be rejected (stall/return false) to avoid leaving the driver in an undefined state. Also consider explicitly resetting negotiated/protocol state when switching back to Alt 0 if the driver relies on those fields elsewhere.
There was a problem hiding this comment.
Fixed in 9f1001e. midi2d_control_xfer_cb() now rejects alt > 1 with return false (STALL). State reset on switch back to Alt 0 is handled by the existing alt_setting update path.
|
|
||
| if (log_count >= LOG_LINES) { | ||
| for (int i = 0; i < LOG_LINES - 1; i++) { | ||
| strncpy(log_lines[i], log_lines[i + 1], CHARS_PER_LINE); |
There was a problem hiding this comment.
When the log buffer scrolls, the strncpy() copies CHARS_PER_LINE bytes but does not ensure a trailing '\0' on the destination lines. fb_string() relies on NUL-terminated strings, so this can lead to reading past the end of log_lines[i]. After shifting, explicitly set log_lines[i][CHARS_PER_LINE] = '\0' (or use memcpy + manual terminator) for each moved line.
| strncpy(log_lines[i], log_lines[i + 1], CHARS_PER_LINE); | |
| strncpy(log_lines[i], log_lines[i + 1], CHARS_PER_LINE); | |
| log_lines[i][CHARS_PER_LINE] = '\0'; |
There was a problem hiding this comment.
Fixed in 9f1001e. Added explicit NUL termination (log_lines[i][CHARS_PER_LINE] = '\0') after each strncpy() in the scroll loop.
| - **Reactive Architecture**: Auto-detects Alt Setting 1 (MIDI 2.0) capability during enumeration | ||
| - **Auto-Selection**: Automatically selects the highest available protocol (MIDI 2.0 preferred) | ||
| - **Transparent Stream Messages**: All data (UMP packets + Stream Messages) flow through callbacks |
There was a problem hiding this comment.
The host driver documentation states it will "Auto-Select" the highest available protocol, which matches the current implementation (midih2_auto_select_alt_setting). However, the PR description says the driver makes no protocol decisions and the application controls everything. Please align the PR description and public API/docs with the actual behavior (either document the auto-selection as the supported behavior, or add an application-controlled selection mechanism and update the code/docs accordingly).
There was a problem hiding this comment.
Fixed in 9f1001e. Documentation now reflects the actual behavior: auto-selects highest protocol and issues SET_INTERFACE to activate Alt Setting 1 when MIDI 2.0 is detected. PR description aligned accordingly.
src/class/midi/midi2_host.c
Outdated
| midih2_parse_descriptors_alt1(p_midi, desc_itf, desc_end); | ||
| } | ||
|
|
||
| return max_len; |
There was a problem hiding this comment.
midih2_open() returns max_len, which will cause the USBH configuration parser to skip the rest of the configuration descriptor (and potentially prevent other interfaces/alt settings from being opened). The host class driver open() callback is expected to return the number of bytes consumed for this interface (see midih_open()). Compute and return the parsed length (e.g., advance a local descriptor pointer until the next TUSB_DESC_INTERFACE and return the delta from desc_start).
| return max_len; | |
| // Compute and return number of bytes consumed for this interface. | |
| // Walk descriptors from desc_itf until the next interface descriptor | |
| // or the end of the provided buffer. | |
| uint8_t const *desc_start = (uint8_t const *) desc_itf; | |
| uint8_t const *p = desc_start; | |
| uint8_t const *end = desc_start + max_len; | |
| while (p + 2 <= end) { | |
| uint8_t const len = p[0]; // bLength | |
| uint8_t const type = p[1]; // bDescriptorType | |
| // Stop at the next interface descriptor (but skip the first one, | |
| // which is desc_itf itself). | |
| if (type == TUSB_DESC_INTERFACE && p != desc_start) { | |
| break; | |
| } | |
| if (len == 0 || p + len > end) { | |
| // Malformed descriptor or would run past end; stop here. | |
| break; | |
| } | |
| p += len; | |
| } | |
| uint16_t const parsed_len = (uint16_t) (p - desc_start); | |
| return parsed_len; |
There was a problem hiding this comment.
Fixed in 9f1001e -- same fix as noted in the other thread. midih2_open() now returns the actual parsed length computed from descriptor end pointers, following the same pattern as midih_open() in midi_host.c.
src/class/midi/midi2_host.c
Outdated
| // Auto-select alt setting | ||
| midih2_auto_select_alt_setting(p_midi); | ||
|
|
||
| // Invoke descriptor_cb | ||
| tuh_midi2_descriptor_cb_t desc_cb = { | ||
| .protocol_version = p_midi->protocol_version, | ||
| .bcdMSC_hi = p_midi->bcdMSC_hi, | ||
| .bcdMSC_lo = p_midi->bcdMSC_lo, | ||
| .rx_cable_count = (p_midi->alt_setting_current == 0) ? | ||
| p_midi->rx_cable_count_alt0 : p_midi->rx_cable_count_alt1, | ||
| .tx_cable_count = (p_midi->alt_setting_current == 0) ? | ||
| p_midi->tx_cable_count_alt0 : p_midi->tx_cable_count_alt1, | ||
| }; | ||
| tuh_midi2_descriptor_cb(idx, &desc_cb); | ||
|
|
||
| // Mark as mounted | ||
| TU_LOG_DRV("MIDI2 mounted addr = %u, alt = %u, protocol = %u\r\n", | ||
| dev_addr, p_midi->alt_setting_current, p_midi->protocol_version); | ||
| p_midi->mounted = true; | ||
|
|
||
| // Invoke mount_cb | ||
| tuh_midi2_mount_cb_t mount_cb = { | ||
| .daddr = p_midi->daddr, | ||
| .bInterfaceNumber = p_midi->bInterfaceNumber, | ||
| .protocol_version = p_midi->protocol_version, | ||
| .alt_setting_active = p_midi->alt_setting_current, | ||
| .rx_cable_count = desc_cb.rx_cable_count, | ||
| .tx_cable_count = desc_cb.tx_cable_count, | ||
| }; | ||
| tuh_midi2_mount_cb(idx, &mount_cb); | ||
|
|
||
| // Prepare RX transfer | ||
| tu_edpt_stream_read_xfer(&p_midi->ep_stream.rx); | ||
|
|
||
| // Signal USBH that configuration is complete | ||
| usbh_driver_set_config_complete(dev_addr, itf_num); | ||
|
|
||
| return true; |
There was a problem hiding this comment.
midih2_set_config() sets alt_setting_current to 1 when a MIDI 2.0-capable device is detected, but the driver never issues a SET_INTERFACE request to actually activate alternate setting 1. As a result the device will remain in Alt 0 while callbacks report Alt 1, and UMP read/write will interpret legacy MIDI 1.0 event packets as UMP words. Consider calling tuh_interface_set(dev_addr, p_midi->bInterfaceNumber, desired_alt, ...) and only marking mounted / invoking mount_cb (and usbh_driver_set_config_complete) after that control transfer completes successfully.
There was a problem hiding this comment.
Fixed in 9f1001e. midih2_set_config() now calls tuh_interface_set() and only marks mounted after the control transfer completes successfully in the async callback. Falls back to Alt 0 on failure.
|
OK. Working on these adjustments. 🔧 |
Host driver (midi2_host.c): - midih2_open() now returns actual parsed length instead of max_len, preventing composite device interface conflicts - Parsers (alt0/alt1) refactored to return const uint8_t* end pointer following midi_host.c switch/case pattern - Alt 1 CS Endpoint now parses MIDI 2.0 layout (bNumGrpTrmBlk at offset 3 with MIDI_CS_ENDPOINT_GENERAL_2_0 subtype check) instead of reusing MIDI 1.0 struct (bNumEmbMIDIJack) - midih2_set_config() now issues SET_INTERFACE control request via tuh_interface_set() before completing configuration. Falls back to alt 0 if SET_INTERFACE fails - Extracted midih2_set_config_complete() and midih2_set_interface_cb() for async SET_INTERFACE handling Device driver (midi2_device.c): - midi2d_open() skip loop now checks bInterfaceNumber, stopping at interfaces that belong to other functions in composite devices - SET_INTERFACE handler now rejects alt > 1 (returns false/stall) - Named constants for GTB descriptor types and MIDI protocol values Descriptor macros (usbd.h): - TUD_MIDI2_DESC_ALT1_HEAD: iInterface set to 0 (consistent with Alt 0), wTotalLength now uses TUD_MIDI2_DESC_ALT1_CS_LEN to cover all Alt 1 class-specific descriptors - TUD_MIDI2_DESC_ALT1_EP: now accepts GTB ID list via variadic args, emitting complete CS endpoint descriptor Host example: - CMakeLists.txt restricted to rp2040 family (display.c requires Pico SDK headers) - display.c: null terminator after strncpy in log scroll Documentation: - class_drivers.rst updated to reflect SET_INTERFACE behavior and auto-select with fallback Addresses: Codex P1 (hathach#1, hathach#2, hathach#3), Copilot (hathach#4-hathach#9)
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| // -- Utility Messages (MT=0x0, 32-bit) -- | ||
|
|
||
| static inline void ump_noop(void) { |
Check notice
Code scanning / CodeQL
Unused static function Note
There was a problem hiding this comment.
Removed -- this function no longer exists in the current code.
- Use UINT32_C(1) instead of 1u for bit shifts >= 16 in midi2_device.c to avoid shift-count-overflow on 16-bit platforms (MSP430) - Add Makefiles for midi2_device and midi2_host examples with family guard (skip if FAMILY != rp2040). These examples require Pico SDK and board-specific hardware - Restrict midi2_device CMakeLists.txt to rp2040 family (matching midi2_host)
Size Difference ReportBecause TinyUSB code size varies by port and configuration, the metrics below represent the averaged totals across all example builds. Note: If there is no change, only one value is shown. Changes >1% in size
Changes <1% in size
No changes
|
|
Thank you for the automated review feedback. I pushed two commits addressing all of it: 9f1001e - PR review feedback:
b8fa96a - CI build fixes:
All CI checks passing now. |
|
superb! Thank you very much, I am in the middle of reworking rp2 driver, give me a bit of time to catch up. Will review this asap. |
|
@hathach thank you for your response! I'm available to explain and adjust anything that's needed. 🌟 |
I have been following TinyUSB for a while and have deep respect for the work @hathach built here over the years. The care with static memory, the clarity of APIs, the callback patterns -- all of that directly influenced how I approached this implementation.
What I tried to do was bring MIDI 2.0 to TinyUSB following the same patterns as MIDI 1.0, as a natural extension of the code and architecture. Anyone familiar with
midi_device.candmidi_host.cwill recognize the structure. Same naming conventions (midi2d_*,midih2_*,tud_midi2_*,tuh_midi2_*), same endpoint stream patterns, sameTU_VERIFY/TU_ASSERTusage, same init/open/xfer_cb/close logic.Device driver
Implements USB-MIDI 2.0 with both Alt Settings: Alt 0 (MIDI 1.0 fallback) and Alt 1 (native UMP). Protocol negotiation (Endpoint Discovery, Config Request/Notify, Function Block Discovery) is embedded in the driver, responding automatically when the host requests it. Group Terminal Block descriptor is served via
GET_DESCRIPTOR. UMP read/write does atomic framing by message type.Host driver
Follows a reactive architecture: enumerate, detect MIDI 2.0 via
bcdMSCand Alt Setting 1 presence, inform the application via callbacks. No protocol decisions are made by the driver. The application controls everything. Five weak callbacks: descriptor, mount, unmount, rx, tx.Zero impact on existing code
Everything is guarded by
#if CFG_TUD_MIDI2/#if CFG_TUH_MIDI2(default 0). If nobody enables these flags, the MIDI 2.0 code does not exist in the binary. I confirmed by running the existing unit tests: FIFO 26/26 PASS, USBD 5/5 PASS.Examples
midi2_device plays Twinkle Twinkle Little Star using all MIDI 2.0 Channel Voice messages: genuine 16-bit velocity (values with no 7-bit equivalent), 32-bit CC, 32-bit pitch bend, channel pressure, per-note poly pressure, per-note management, program change with bank select, JR timestamps. The USB descriptor exposes both Alt Settings per the USB-MIDI 2.0 specification.
midi2_host receives UMP via PIO-USB and shows on a SSD1306 OLED display a boot checklist (PWR, TinyUSB init, USB bus, Device connected, Descriptor parsed, Alt Setting, Mount, Receiving) followed by decoded notes in real time.
Test results
Note about the Waveshare RP2350-USB-A
During development I needed
CFG_TUSB_DEBUG=2for this specific board because it has a known hardware issue (R13 pull-up on D+ that interferes with hot-plug detection, ref: https://qsantos.fr/2025/11/21/fixing-the-rp2350-usb-a-not-working-as-usb-host/). This is board-specific, not driver-related. The Host example ships withCFG_TUSB_DEBUG=0(default). I built this with what I had in the lab while my Adafruit Feather RP2040 USB Host is still on the way. If anyone has a Feather available and could test, I would appreciate the feedback. I believe it should work out of the box since the Feather's USB Host circuit (GP16, VBUSEN GP18) is what TinyUSB uses as reference.Files
New files:
src/class/midi/midi2_device.c/h-- Device driver (604+125 lines)src/class/midi/midi2_host.c/h-- Host driver (502+99 lines)examples/device/midi2_device/-- Device exampleexamples/host/midi2_host/-- Host example with displayhw/bsp/rp2040/boards/waveshare_rp2350_usb_a/-- board definitiontest/unit-test/test/device/midi2/test_midi2_device.ctest/unit-test/test/host/midi2/test_midi2_host.cdocs/reference/class_drivers.rstModified files (all within
#ifguards):src/device/usbd.c-- driver table registrationsrc/device/usbd.h--TUD_MIDI2_DESCRIPTORmacrossrc/host/usbh.c-- driver table registrationsrc/tusb.h-- includessrc/tusb_option.h-- config defaultssrc/class/midi/midi.h--midi2_ump_word_counthelperhw/bsp/rp2040/family.cmake-- sourcessrc/CMakeLists.txt-- sourcesWhat is next
I already have some ideas for a possible next step, but I am still studying the best approach before proposing anything. When it is more mature, I will bring it in a separate PR.
For context on my MIDI 2.0 work: I have been contributing to the MIDI 2.0 ecosystem including the official MIDI Association library (AM_MIDI2.0Lib -- merged, Flex Data + per-note expression + bug fixes) and collaborating on USB-MIDI 2.0 topics in the tusb_ump project. This PR is a natural continuation of that work, bringing native MIDI 2.0 directly into TinyUSB.
Demo
Device test video:
Board-to-board test video (Device + Host):
Linux: ALSA enumeration + aseqdump
Windows: Device Manager
Windows: MIDI Services - Devices and Endpoints
Windows: MIDI Services - UMP Monitor
