rppicomidi / usb_midi_host

An application level TinyUSB USB MIDI Host driver for the RP2040
MIT License
48 stars 5 forks source link

Only 48 bytes max of SysEx are sent out #13

Closed VocasoGK closed 2 weeks ago

VocasoGK commented 3 weeks ago

Describe the bug Can't seem to send out SysEx message bigger than 48 bytes including the F0 and F7 bytes.

To Reproduce Steps to reproduce the behavior: I use this code to sent out 146 bytes of SysEx message.

static void sendSysExAllOn() {
  auto intf = usbhMIDI.getInterfaceFromDeviceAndCable(midiDevAddr, 0);
  if (intf == nullptr)
    return;  // not connected

  const byte PatchAllOn[] = { 0xf0, 0x52, 0x00, 0x58, 0x28, 0x08, 0x61, 0x00, 0x00, 0x24, 0x00, 0x28, 0x00, 0x00, 0x32, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x21, 0x00, 0x00, 0x00, 0x22, 0x01, 0x04, 0x00, 0x32, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x24, 0x00, 0x61, 0x00, 0x00, 0x48, 0x03, 0x78, 0x08, 0x00, 0x37, 0x10, 0x04, 0x22, 0x73, 0x03, 0x00, 0x07, 0x00, 0x40, 0x00, 0x00, 0x01, 0x02, 0x00, 0x00, 0x0c, 0x05, 0x78, 0x00, 0x1e, 0x70, 0x40, 0x60, 0x06, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x11, 0x00, 0x00, 0x50, 0x74, 0x28, 0x79, 0x00, 0x11, 0x20, 0x00, 0x40, 0x16, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x15, 0x0f, 0x54, 0x00, 0x45, 0x53, 0x54, 0x20, 0x43, 0x54, 0x52, 0x00, 0x4c, 0x20, 0x00, 0xf7 };
  unsigned int SysExLength = sizeof(PatchAllOn);

  intf->sendSysEx(SysExLength, PatchAllOn, true);
}

Expected behavior All 146 bytes expected to be received by the slave device.

Screenshots These screenshots are from my Android device acting as a MIDI Monitor. Shorter SysEx sent by RP2040 does work as seen on the 1st-3rd line. But longer SysEx suddenly cut off as seen on the 4th line onwards. Screenshot_20240512_150743_Midi Tools

Expected Behaviour: (sent out via Windows 11 using Bome SendSX) All 146 bytes are received properly. SmartSelect_20240512_151158_Midi Tools

What is your setup like? Arduino 2.3.2 RP2040 3.8.0 Adafruit TinyUSB 3.1.0 (3.1.4 is latest) usb_midi_host 1.0.1 EZ_USB_MIDI_HOST 1.0.1 OS: Windows 11 Board: RP2040 Zero Base Sketch: EZ_USB_MIDI_HOST_PIO_example Using Android Devices as a MIDI Monitor using MIDI monitor app.

Additional context Shorter SysEx does work fine

rppicomidi commented 2 weeks ago

@VocasoGK Thank you for taking the time to submit this issue. Because your sysex message is 147 bytes long, please first be sure to define RPPICOMIDI_TUH_MIDI_MAX_SYSEX in your test code to something at least 147 bytes or larger. If you don't define it, then the default is 128 bytes. Please read the comments in EZ_USB_MIDI_HOST_Config.h. If that fixes your issue, I will try to update the README.md file to make this more clear.

If that doesn't fix your problem, please report back and include enough of your modified sketch to duplicate the issue. It is suspicious to me that you are only getting 48 bytes, because 48 bytes is the maximum size of a MIDI message that can fit in a 64-byte (maximum length) full speed USB bulk transfer packet.

rppicomidi commented 2 weeks ago

@VocasoGK, to be more clear, I will need to see all of the includes and configuration defines the sketch has before it calls sendSysExAllOn()

VocasoGK commented 2 weeks ago

Changing RPPICOMIDI_TUH_MIDI_MAX_SYSEX does solve a problem I had (not mentioned in this issue post) on incoming 146 bytes SysEx messages that was divided as a 128 bytes message, and 20 bytes message. But it still does not solve the outgoing message problem.

Here is my sketch. Changes from the example sketch was just basically calling sendSysExRequestInfo() and sendSysExEditMode() when MIDI device is connected to send out short SysEx messages (works fine), and a timer to call sendSysExAllOn() periodically.

Workaround that comes to my mind currently is dividing the 146 bytes message to a smaller 48 bytes messages and send them out until it reaches the F7 stop.

VocasoGK commented 2 weeks ago

I tried separating the 146 bytes into 4 chunks consisting of 48 bytes each. Then changed

  intf->sendSysEx(SysExLength, PatchAllOn, true);

into

  intf->sendSysEx(sizeof(patchChunks[0]), patchChunks[0], true);
  usbhMIDI.writeFlushAll();
  intf->sendSysEx(sizeof(patchChunks[1]), patchChunks[1], true);
  usbhMIDI.writeFlushAll();  
  intf->sendSysEx(sizeof(patchChunks[2]), patchChunks[2], true);
  usbhMIDI.writeFlushAll();
  intf->sendSysEx(sizeof(patchChunks[3]), patchChunks[3], true);
  usbhMIDI.writeFlushAll();

Code above does send out SysEx, but only the first 96 bytes (patchChunks[0] and patchChunks[1]) are sent out somewhat consistently, and are very unreliable as sometimes it randomly cuts off then spits out the remaining chunks or even just garbage data.

rppicomidi commented 2 weeks ago

@VocasoGK Thanks for the info. Very sorry for the bug. I never tested long sysex messages. I am investigating now.

rppicomidi commented 2 weeks ago

@VocasoGK I understand the problem now. Changing RPPICOMIDI_TUH_MIDI_MAX_SYSEX in EZ_USB_MIDI_HOST_Config.h fixes the receiver because the EX_USB_MIDI_HOST library includes file. However, the usb_midi_host library does not include EZ_USB_MIDI_HOST_Config.h, so properly defining CFG_TUH_MIDI_TX_BUFSIZE in that config file does not help. In the C/C++ projects, all you have to do is add a new definition for CFG_TUH_MIDI_TX_BUFSIZE in tusb_config.h. That option is not practical for Arduino IDE. I need to think about it. Meanwhile, as a short-term workaround, add

#define CFG_TUH_MIDI_TX_BUFSIZE 256

after line 37 in the usb_midi_host.c library file.

Thank you for your patience.

VocasoGK commented 2 weeks ago

It works now! Thank you very much! Really appreciate the quick response. I'll close the issue and let you know if I have any other issues in the future.

rppicomidi commented 2 weeks ago

@VocasoGK I am glad I helped you. If you do not mind, I am going to keep this issue open until I come up with a fix that doesn't require changing library files.

rppicomidi commented 2 weeks ago

I just pushed code that addresses this issue for this library. There is now a function you can call in your sketch tuh_midih_define_limits() that lets you define buffer sizes at runtime. See the Arduino examples for details. However, now I have to make EZ_USBMIDI_HOST library use this function. That is https://github.com/rppicomidi/EZ_USB_MIDI_HOST/issues/1. I am closing this issue for now.

rppicomidi commented 2 weeks ago

Ugh. The good news is that I didn't push new release tags for this. The bad news is I botched the last commit to fix this issue. New one coming soon. I will wait until I push release tags for Arduino before I close this one.

rppicomidi commented 2 weeks ago

This should be fixed with version 1.1.1. https://github.com/rppicomidi/EZ_USB_MIDI_HOST/issues/1 is still open.