MIDI Library not working

Hi there!

I’m having some trouble getting the MIDI Library receiving correctly some SysEx messages. Basically, I send a Dump Voice request to my keyboard which in turn returns 26 sysex messages containing the information of the selected voice. I tried this function in a simple sketch and it did work fine.
Here’s the code I wrote:

#include <MIDI.h>

MIDI_CREATE_INSTANCE(HardwareSerial, Serial1, midi1);

byte* receivedSysExArray;
int usedSysExBytes = 0;
boolean incomingSysEx = false;
unsigned long sysExPreviousMillis;
unsigned long MIDIWaitInterval = 3000;
long previousMillis;
long interval = 5000;

void setup() {
  Serial.begin(115200);
  
  receivedSysExArray = (byte*)malloc(16);
  midi1.begin();
  midi1.setHandleSystemExclusive(handleSystemExclusive);
}


void loop() {
  
  midi1.read();
  
  checkIncomingMIDI();

  if (millis() - previousMillis >= interval) {
    previousMillis = millis();
    requestSysExDump();
  }

}

int checkIncomingMIDI() {
if ((incomingSysEx) && (millis() - sysExPreviousMillis >= MIDIWaitInterval)) {
  //print the array and reset it after 3 seconds from the last midi message

    printReceivedArray(); // It just prints the array
    
    incomingSysEx = false;
    free(receivedSysExArray);
    receivedSysExArray = NULL;
    usedSysExBytes = 0;
    receivedSysExArray = (byte*)malloc(16);
  }
  return 0;
}

void handleSystemExclusive(byte* array, unsigned size_) {
  sysExPreviousMillis = millis();
  incomingSysEx = true;
  receivedSysExArray = (byte*)realloc(receivedSysExArray, usedSysExBytes + size_);

  for (int i = 0; i < size_; i++) {
    receivedSysExArray[usedSysExBytes] = array[i];
    usedSysExBytes++;
  }
  
}

void requestSysExDump() {
  byte dumpRequestArray[] = {0xF0, 0x43, 0x20, 0x7F, 0x1C, 0x00, 0x0E, 0x0F, 0x00, 0xF7};
  midi1.sendSysEx(10, dumpRequestArray, true);
  incomingSysEx = true;
}

However when I put this same code in the real project’s sketch, the handleSystemExclusive callback function is not called correctly and only about half of the times (13/26). The only difference is in the main loop function which is:

void loop() {
  
  midi1.read();
  
  checkIncomingMIDI();

  if (millis() - previousMillis >= interval) {
    previousMillis = millis();
    requestSysExDump();
  }

  if (!(incomingSysEx)){
     checkPotentiometers();
  }

}

If I comment out the checkPotentiometers() function the program runs fine. Even though that function should not be called unless 3 seconds passed from the last MIDI message, it seems to be causing this big issue. Adding a delay(50); after checkPotentiometers() worsen the malfunction as just 1/26 sysex message is correctly received.

What on earth is this about?

It is quite an urgent project, can somebody help me?

It seems possible the problem is with the checkPotentiometers() function that you haven't shown us.

It really is useful to post the WHOLE program not just the fragment where you can't find any problem.

Steve

Alright I’ll post the entire program:

#include <MIDI.h>

byte muxPins[] = {2, 3, 4, 5};
byte buttonsIndexes[] = {0x03, 0x04, 0x05, 0x0A, 0x0B, 0x0E, 0x0F, 0x10, 0x11, 0x1B,
                         0x1C, 0x1E, 0x1F, 0x20, 0x24, 0x25, 0x29, 0x2A
                        }; //First digit = muxInputNumber   second digit = buttonAddress
byte potsIndexes[] = {0x00, 0x01, 0x02, 0x06, 0x06, 0x07, 0x08, 0x09, 0x0C, 0x0D,
                      0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1A, 0x1D,
                      0x21, 0x22, 0x23, 0x26, 0x27, 0x28, 0x2B, 0x2C, 0x2D, 0x2E,
                      0x2F
                     }; //First digit = muxInputNumber   second digit = potAddress
int keyboardValues[100] = {};
int zeroValues[][16] = {{0, 0},
  {0, 0},
  {0, 0},
  {0, 0}
};
int previousValues[][16] = {{0, 0},
  {0, 0},
  {0, 0},
  {0, 0}
};
int previousMappedValues[][16] = {{0, 0},
  {0, 0},
  {0, 0},
  {0, 0}
};
int potValueThreshold = 1;
int ADCToMidiRatio = 8;

MIDI_CREATE_INSTANCE(HardwareSerial, Serial1, midi1);

byte* receivedSysExArray;
int usedSysExBytes = 0;
boolean incomingSysEx = false;
unsigned long sysExPreviousMillis;
unsigned long MIDIWaitInterval = 3000;

long previousMillis;
long interval = 8000;

boolean relativeValues = true;

byte header[] = {0x43, 0x10, 0x7f, 0x1c, 0x00}; //Default header for Yamaha SysEx
byte sendArray[20];

boolean serialPanelDebug = false;



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

  for (int i = 0; i < 4; i++) pinMode(muxPins[i], OUTPUT);

  for (int i = 0; i < sizeof(potsIndexes); i++) {  //Initializing previous values to initial status of potentiometers
    int value = readPotFromIndex(potsIndexes[i]);
    byte muxInput = muxInputFromIndex(potsIndexes[i]);
    byte potAddress = potAddressFromIndex(potsIndexes[i]);
    previousValues[muxInput][potAddress] = value;
  }

  receivedSysExArray = (byte*)malloc(16);
  midi1.begin();
  midi1.setHandleSystemExclusive(handleSystemExclusive);
}

void loop() {

  midi1.read();

  checkIncomingMIDI();

  if (!(incomingSysEx)) {
    if (millis() - previousMillis >= interval) {
      previousMillis = millis();
      requestSysExDump();
    }

    checkAllPots();

    checkAllButtons();
  }
}

void checkIncomingMIDI() {
  if ((incomingSysEx) && (millis() - sysExPreviousMillis >= MIDIWaitInterval)) {

    for (int i = 0; i < usedSysExBytes; i++) { //Prints the messages in a readable way
      Serial.print(receivedSysExArray[i], HEX);
      Serial.print(" ");
      if (receivedSysExArray[i] == 0xF7) {
        Serial.println();
      }
    }
    Serial.println();

    incomingSysEx = false;
    free(receivedSysExArray);
    receivedSysExArray = NULL;
    usedSysExBytes = 0;
    receivedSysExArray = (byte*)malloc(16);
  }
}

void handleSystemExclusive(byte* array, unsigned size_) {
  sysExPreviousMillis = millis();
  incomingSysEx = true;
  receivedSysExArray = (byte*)realloc(receivedSysExArray, usedSysExBytes + size_);

  for (int i = 0; i < size_; i++) {
    receivedSysExArray[usedSysExBytes] = array[i];
    usedSysExBytes++;
  }

}

void sendMIDI(byte arrayLength, byte address[], byte data1, byte data2 = 0) {
  byte totalLength = arrayLength + sizeof(header);
  byte dataLength = 1;
  if (data2) dataLength = 2;

  byte data[2];
  data[0] = data1;
  data[1] = data2;

  memcpy(sendArray, header, sizeof(header));
  memcpy(sendArray + sizeof(header), address, arrayLength - dataLength);
  memcpy(sendArray + sizeof(header) + (arrayLength - dataLength), data, dataLength);

  midi1.sendSysEx(totalLength, sendArray, false);
}

void requestSysExDump() {
  byte dumpRequestArray[] = {0xF0, 0x43, 0x20, 0x7F, 0x1C, 0x00, 0x0E, 0x0F, 0x00, 0xF7};
  midi1.sendSysEx(10, dumpRequestArray, true);
  incomingSysEx = true;
}


void sendElementLevel(int element, int level) {
  if ((element > 8) || (element < 0)) {
    return;
  }
  byte address[3] = {0x41, 0x00, 0x1A};
  address[1] = element - 1;

  sendMIDI(4, address, level);
}

void checkAllButtons() {
  for (int i = 0; i < sizeof(buttonsIndexes); i++) {
    byte muxInput = muxInputFromIndex(buttonsIndexes[i]);
    byte buttonAddress = potAddressFromIndex(buttonsIndexes[i]); //It is the same function for pots and buttons
    int buttonState = readButtonValue(muxInput, buttonAddress);
    if (buttonState) {  //The button is pressed
      if (muxInput == 2) {
        if (buttonAddress == 0) {
          if (relativeValues) {
            saveZeroes();
          }
        }
      }
      if (serialPanelDebug) {
        Serial.print(muxInput, HEX);
        Serial.print(buttonAddress, HEX);
        Serial.print(" : ");
        Serial.println(buttonState);
      }
    }
  }
}

int value = 0;
void checkAllPots() {
  for (int i = 0; i < sizeof(potsIndexes); i++) {
    if (relativeValues) {
      value = zeroedValue(potsIndexes[i], readPotFromIndex(potsIndexes[i]));
    }
    else {
      value = readPotFromIndex(potsIndexes[i]);
    }

    byte muxInput = muxInputFromIndex(potsIndexes[i]);
    byte potAddress = potAddressFromIndex(potsIndexes[i]);

    if ((value < previousValues[muxInput][potAddress] - potValueThreshold) || (value > previousValues[muxInput][potAddress] + potValueThreshold)) {
      // Pot value has changed
      previousValues[muxInput][potAddress] = value; //Must stay before the following condition
      if ((value >= -1) && (value <= 1))  return;

      if (muxInput == 0) {
        value = map(value, 0, 1024, 0, 127);
        if (checkMappedValueChange(value, muxInput, potAddress)) {
          sendElementLevel(1, value);
        }
      }
      if (serialPanelDebug) {
        Serial.print(muxInput, HEX);
        Serial.print(potAddress, HEX);
        Serial.print(" : ");
        Serial.println(value);
      }
    }

  }
}

boolean checkMappedValueChange(int value, byte muxInput, byte potAddress) {
  if ((value < previousMappedValues[muxInput][potAddress]) || (value > previousMappedValues[muxInput][potAddress])) {
    previousMappedValues[muxInput][potAddress] = value;
    return true;
  }
  else {
    return false;
  }
}

int zeroedValue(byte potIndex, int potValue) {
  byte muxInput = muxInputFromIndex(potIndex);
  byte potAddress = potAddressFromIndex(potIndex);
  return potValue - zeroValues[muxInput][potAddress];
}

void saveZeroes() {
  //Zeroes the pots out
  for (int i = 0; i < sizeof(potsIndexes); i++) {
    byte muxInput = muxInputFromIndex(potsIndexes[i]);
    byte potAddress = potAddressFromIndex(potsIndexes[i]);
    zeroValues[muxInput][potAddress] = readPotValue(muxInput, potAddress);
  }
}

int readPotFromIndex(byte index) {
  byte muxInput = muxInputFromIndex(index);
  byte potAddress = potAddressFromIndex(index);
  return readPotValue(muxInput, potAddress);
}

int readPotValue(int muxInput, int potAddress) {
  writeAddress(potAddress);
  int potValue = analogRead(muxInput);
  return potValue;
}

void writeAddress(byte address) {
  digitalWrite(muxPins[0], (address & B0001));
  digitalWrite(muxPins[1], (address & B0010));
  digitalWrite(muxPins[2], (address & B0100));
  digitalWrite(muxPins[3], (address & B1000));
}

byte muxInputFromIndex(byte index) {
  return (index & B11110000) >> 4;
}

byte potAddressFromIndex(byte index) {
  return (index & B00001111);
}
int lastButtonState = LOW;
unsigned long lastDebounceTime = 0;
unsigned long debounceDelay = 100;

boolean readButtonValue(byte muxInput, byte buttonAddress) {
  writeAddress(buttonAddress);
  int reading = digitalRead(54 + muxInput); //A0 = 54 on Mega
  if ((reading != previousValues[muxInput][buttonAddress]) && ((millis() - lastDebounceTime) > debounceDelay)) {
    previousValues[muxInput][buttonAddress] = reading;
    lastDebounceTime = millis();
    return (boolean) reading;
  }
  else {
    lastButtonState = reading;
    return false;
  }
}

All the checking of changing values is done by the two functions checkAllPots() and checkAllButtons() which are called constantly unless the Arduino has sent a SysEx dump request to the keyboard and it is waiting for a response (incomingSysEx = true when requestSysExDump() is called).

In that case the program calls midi1.read() as fast as possible. However just the presence of the two (slow) check functions, even if they are not called, seems to cause an erratic and wrong receiving process of the 26 consecutive SysEx messages sent by the keyboard.

A bit complicated for me to follow but it more or less has to be that IncomingSysEx is false more often than you think so you are calling the other routines more often too.

To add to my confusion I notice readButtonValue() is declared as boolean but the value it returns is declared as an int (probably works o.k. but looks odd). Also readPotFromIndex() declared as int apparently returns nothing. But neither of those should cause problems if the functions that use them are never called.

Steve

Why malloc a 16 byte array to read the sysex message and then free it and immediately malloc another?
Just declare a global 16-byte array and use that.

Pete

slipstick:
A bit complicated for me to follow but it more or less has to be that IncomingSysEx is false more often than you think so you are calling the other routines more often too.

  1. I put a serial print message in the loop and I faked the call of the handle function to check if what you said was the case. Now the loop looks something like this :
void loop() {

  midi1.read();

  checkIncomingMIDI();


  if (!(incomingSysEx)) {
    Serial.println("Hello world");

    if (millis() - previousMillis >= interval) {
      previousMillis = millis();
      Serial.println("Requesting");
      byte arr[3] = {0x31, 0x34, 0xF7};
      handleSystemExclusive(arr, 3);
    }

    checkAllPots();

    checkAllButtons();
  }
}

The results are as expected : when the Arduino is not awaiting a response (incomingSysEx = false) it prints "Hello world" normally. When handleSystemExclusive() is called every 8 seconds (interval=8000), the program prints "Requesting" waits for 3 seconds (incomingSysEx = true), prints the fake array and resumes printing "Hello world". Sadly, nothing changed :drooling_face:

slipstick:
To add to my confusion I notice readButtonValue() is declared as boolean but the value it returns is declared as an int (probably works o.k. but looks odd). Also readPotFromIndex() declared as int apparently returns nothing. But neither of those should cause problems if the functions that use them are never called.

Yes, you're right, I corrected the mistakes in the code however they were not related to this problem.

el_supremo:
Why malloc a 16 byte array to read the sysex message and then free it and immediately malloc another?
Just declare a global 16-byte array and use that.

Yes and no, 16 bytes is a random size number I put as default, however if you look at the handleSystemExclusive() function the array is dynamically resized (realloc) as much as it is needed by the length of the SysEx message that is being received. I still don't think that is the main problem I'm struggling with.

So the problem persists and I still can't receive all the messages correctly as the lengthy analogRead functions slow down the call of midi1.read() even though they are not called. What is causing this?

Thank you for the help so far,
Archi

What is the real, maximum size of a sysex message sent by the keyboard? Some sysex messages can be far larger than the MIDI library allows. If I read the code correctly the default maximum is 128 bytes but it can be overridden. Or the message may be larger than the available free ram. Which Arduino are you using?
You may be running into a problem with malloc not being able to maintain sufficient unfragmented memory. You also don't check that receivedSysExArray contains a valid pointer - i.e. that it is not NULL - which will happen if you ask for too much space. You may find it safer to figure out the maximum possible size of a received sysex message and then allocate a static array of that size.

Pete

el_supremo:
What is the real, maximum size of a sysex message sent by the keyboard? Some sysex messages can be far larger than the MIDI library allows. If I read the code correctly the default maximum is 128 bytes but it can be overridden. Or the message may be larger than the available free ram. Which Arduino are you using?

The maximum size of the SysEx message is 111 bytes, just within the limit imposed by the Library. I’m using an Arduino Mega, 8K ram I suppose?

el_supremo:
You may find it safer to figure out the maximum possible size of a received sysex message and then allocate a static array of that size.

I followed your advice and I replaced the dynamic array with a static array. May I ask, what do you mean by unfragmented memory? Memory not used or allocated for other variables?

I finally solved it! The tweak was as simple as putting sysExPreviousMillis = millis(); in the requestSysExDump() function. Basically, not doing so caused the program to call checkIncomingMIDI() and wasting time printing the array before 3 seconds elapsed. Thank you for the help to everybody, the code improved in speed and stability as well! :slight_smile:

Should I correct the code above?

Archi