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

SysEx message issue with 5.0.2 #188

Open
0ba-pia opened this issue Dec 17, 2020 · 9 comments
Open

SysEx message issue with 5.0.2 #188

0ba-pia opened this issue Dec 17, 2020 · 9 comments
Labels
bug SysEx Issue relates to System Exclusive messages

Comments

@0ba-pia
Copy link

0ba-pia commented Dec 17, 2020

I had to revert to 4.3.1 release since using v5.0.2 I had Sysex messages corruption.
Difficult to debug but basically my setup is device (32U4 based) receives sysex "heartbeat" message every 2s from hardware serial (note that MIDIUSB v1.0.4 is also used by this device).
When using MIDI v4.3.1 it works perfectly but compiling with 5.0.2 I receive sysex messages with wrong size (2 bytes instead of 3) after some time.

@0ba-pia 0ba-pia added the bug label Dec 17, 2020
@franky47
Copy link
Member

Thanks, can you post your code here so we can see what could be the problem ?

@lathoub lathoub added the SysEx Issue relates to System Exclusive messages label Dec 18, 2020
@lathoub
Copy link
Collaborator

lathoub commented Dec 18, 2020

Thanks @0ba-pia for reporting. Can you provide the byte array(s) of the heartbeat SysEx

@0ba-pia
Copy link
Author

0ba-pia commented Dec 18, 2020

Sorry .. code on sender side (uses MIDI v4.3.1):

keepAliveBuf[0] = SYSEX_KEEPALIVE; //0x54
keepAliveBuf[1] = 1;
keepAliveBuf[2] = 24;
MIDI.sendSysEx(3, keepAliveBuf, false);

And on receiver side (uses MIDI v5.0.2):

void handleSystemExclusive(byte *buf, unsigned bufSize) {
  if (bufSize > 2) {
    switch (buf[1]) {
      case SYSEX_KEEPALIVE :
        handleKeepAlive(buf[2],buf[3]);
        break;
      case ...
    }
  } else {
    logging = String();
    logging = logging + "Invalid SysEx size:"+bufSize;
    Serial.println(logging);
  }
}

@lathoub
Copy link
Collaborator

lathoub commented Dec 19, 2020

I'm unable to reproduce the error. I have 1 Leonardos sending 3 bytes SysEx every second (0xF0, 0x54, 1, 24, 0xF7), another one receiving the SysEx.

I noticed in your code that you dynamically allocate memory (logging = String();) which could lead to heap fragmentation (or memory leaks). Could it be that your keepAliveBuf gets corrupted?

Here is the code for both Leonardos:

#include <MIDI.h>

unsigned long t1 = millis();
byte sysex0bapia [] = { 0xF0, 0x54, 1, 24, 0xF7 };

MIDI_CREATE_DEFAULT_INSTANCE();

// -----------------------------------------------------------------------------
//
// -----------------------------------------------------------------------------
void setup()
{
  Serial.begin(115200);
  while (!Serial);

  MIDI.begin();
  MIDI.setHandleSystemExclusive(OnMidiSysEx);

  Serial.println(F("Send SysEx every second"));
}

// -----------------------------------------------------------------------------
//
// -----------------------------------------------------------------------------
void loop()
{
  MIDI.read();

  if ((millis() - t1) > 1000)
  {
    MIDI.sendSysEx(sizeof(sysex0bapia), sysex0bapia, true);
    t1 = millis();
  }
}

void OnMidiSysEx(byte* data, unsigned length) {
  Serial.print(F("SYSEX: ("));
  Serial.print(getSysExStatus(data, length));
  Serial.print(F(", "));
  Serial.print(length);
  Serial.print(F(" bytes) "));
  for (uint16_t i = 0; i < length; i++)
  {
    Serial.print(data[i], HEX);
    Serial.print(" ");
  }
  Serial.println();
}

char getSysExStatus(const byte* data, uint16_t length)
{
  if (data[0] == 0xF0 && data[length - 1] == 0xF7)
    return 'F'; // Full SysEx Command
  else if (data[0] == 0xF0 && data[length - 1] != 0xF7)
    return 'S'; // Start of SysEx-Segment
  else if (data[0] != 0xF0 && data[length - 1] != 0xF7)
    return 'M'; // Middle of SysEx-Segment
  else
    return 'E'; // End of SysEx-Segment
}

@0ba-pia
Copy link
Author

0ba-pia commented Dec 19, 2020

Can you try hot unplugging and replugging the UART connection between those 2 Leonardos? Trying this several times, with v5.0.2 you may get the wrong packed size issue. And not with 4.3.1

@franky47
Copy link
Member

This might be your issue: establishing the connection mid-stream will cause false readings no matter what firmware is used.

Now if all subsequent messages received have the wrong size, this is indeed a problem. But it is expected that connecting in the middle of a message will have it be incomplete.

@lathoub
Copy link
Collaborator

lathoub commented Dec 19, 2020

I tried the scenario of disconnecting the connection a then re-establishing so. First I send (using MIDI-OX) a complete SysEx (F0 54 01 18 F7), then a partial SysEx (F0 53 01), then a complete again (F0 54 01 18 F7) - using version 4.3.1 and 5.0.2 (using the Library Manager - keeping the COM* window open).

Here is the log, showing the same output for both versions:

21:23:15.209 -> 40300
21:23:15.209 -> Receive SysEx
21:23:23.827 -> SYSEX: (F, 5 bytes) F0 54 1 18 F7 
21:23:32.429 -> SYSEX: (F, 4 bytes) F0 54 1 F7 
21:23:32.429 -> SYSEX: (F, 5 bytes) F0 54 1 18 F7 
21:24:20.108 -> 50000
21:24:20.108 -> Receive SysEx
21:24:27.581 -> SYSEX: (F, 5 bytes) F0 54 1 18 F7 
21:24:35.300 -> SYSEX: (F, 4 bytes) F0 54 1 F7 
21:24:35.300 -> SYSEX: (F, 5 bytes) F0 54 1 18 F7 

@franky47 MIDI_LIBRARY_VERSION & MIDI_LIBRARY_VERSION_PATCH need to be updated (for next release)

Here is the code:

#include <MIDI.h>

MIDI_CREATE_DEFAULT_INSTANCE();

void setup()
{
  Serial.begin(115200);
  while (!Serial);

  MIDI.begin();
  MIDI.setHandleSystemExclusive(OnMidiSysEx);

  Serial.println(MIDI_LIBRARY_VERSION, HEX);
  Serial.println(F("Receive SysEx"));
}

void loop()
{
  // Listen to incoming notes
  MIDI.read();
}

void OnMidiSysEx(byte* data, unsigned length) {
  Serial.print(F("SYSEX: ("));
  Serial.print(getSysExStatus(data, length));
  Serial.print(F(", "));
  Serial.print(length);
  Serial.print(F(" bytes) "));
  for (uint16_t i = 0; i < length; i++)
  {
    Serial.print(data[i], HEX);
    Serial.print(" ");
  }
  Serial.println();
}

char getSysExStatus(const byte* data, uint16_t length)
{
  if (data[0] == 0xF0 && data[length - 1] == 0xF7)
    return 'F'; // Full SysEx Command
  else if (data[0] == 0xF0 && data[length - 1] != 0xF7)
    return 'S'; // Start of SysEx-Segment
  else if (data[0] != 0xF0 && data[length - 1] != 0xF7)
    return 'M'; // Middle of SysEx-Segment
  else
    return 'E'; // End of SysEx-Segment
}

@lathoub
Copy link
Collaborator

lathoub commented Dec 19, 2020

@0ba-pia Have you looked at ActiveSensing to see if the connection is alive?

@lathoub
Copy link
Collaborator

lathoub commented May 13, 2021

@0ba-pia Was the above useful?

See also franky47's note:
#188 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug SysEx Issue relates to System Exclusive messages
Projects
None yet
Development

No branches or pull requests

3 participants