Go Down

Topic: MIDI input - messages getting dropped/corrupted when multiple notes received (Read 590 times) previous topic - next topic

batmundo

Hi,

I'm working on getting a MIDI-controlled LED infinity mirror setup going, and having some trouble with the Arduino (I have the Mega) dropping MIDI messages when receiving multiple notes at once.

In order to make programming sequences easier, I've broken up the LED strip into different chunks, each controlled by a specific MIDI note. Single notes seem fine, but I run into problems when trying to trigger more than one section at once. I'm finding I have to stagger the notes slightly if I want things to light up correctly - using MIDI Monitor (I'm on OSX), I was able to determine that the notes need to be around 8-9ms apart; the LED triggering gets sporadic for anything under that. I happened to notice that in Logic, this generally translated to the MIDI notes being 0 0 1 0 apart (not sure how the third value translates in terms of beat subdivisions - 1/48th of a quarter note I think?) - this applies to all Note On AND Note Off messages, which gets really annoying when having to stagger both ends of the notes...


A few other things I should perhaps mention:

- Not only do messages get dropped, sometimes they get corrupted or something as well, as other sections of the LED will sometimes trigger, even though there was no note value corresponding to those.
- I initially built the MIDI circuit using the schematic here:
http://libremusicproduction.com/tutorials/arduino-and-midi-in
- After doing some research, it seemed possible it was an optocoupler speed issue (or something...I'm not entirely certain what an octocoupler does really - still somewhat of an electronics noob here), so based on a forum suggestion that a MIDI shield might work better, I ended up getting a Sparkfun MIDI shield - still getting the same issues though.


Just to compare, I set up MIDI notes 11-14 to trigger the four sets of LEDs at the same time, and the Arduino handles the four different LED sets just fine - all turn on instantly with no issues, as it's just one MIDI note.

I had all sorts of other code going on (the whole thing is actually a three piece mirror), but I've stripped down the code to just one mirror section, and the problem is still there:


Code: [Select]

void loop() {

  if (MIDI.read()) {
    location_byte = MIDI.getType();
    note = MIDI.getData1();
    velocity = MIDI.getData2();
    if (location_byte == midi_on || location_byte == midi_off) newNote = true;
  }
 
 
  if (newNote) {
    if (location_byte == midi_on) {
      midistate[note] = 1;
    }
    if (location_byte == midi_off) {
      midistate[note] = 0;
    }
    newNote = false;
  }
 
  EVERY_N_MILLISECONDS(20) {
    fadeToBlackBy(leds_L1, NUM_LEDS_LR, 64);
  }

 
  if (midistate[11] == 1) {
    leds_L1_UL_B = CRGB::Aqua;
    leds_L1_UL_L = CRGB::Aqua;
    leds_L1_UL_T = CRGB::Aqua;
    leds_L1_UL_R = CRGB::Aqua;
  }
  if (midistate[12] == 1) {
    leds_L1_UR_B = CRGB::Aqua;
    leds_L1_UR_L = CRGB::Aqua;
    leds_L1_UR_T = CRGB::Aqua;
    leds_L1_UR_R = CRGB::Aqua;
  }
  if (midistate[13] == 1) {
    leds_L1_LL_B = CRGB::Aqua;
    leds_L1_LL_L = CRGB::Aqua;
    leds_L1_LL_T = CRGB::Aqua;
    leds_L1_LL_R = CRGB::Aqua;
  }
  if (midistate[14] == 1) {
    leds_L1_LR_B = CRGB::Aqua;
    leds_L1_LR_L = CRGB::Aqua;
    leds_L1_LR_T = CRGB::Aqua;
    leds_L1_LR_R = CRGB::Aqua;
  }
 
  if (midistate[15] == 1) { leds_L1_UL_B = CRGB::Aqua; }
  if (midistate[16] == 1) { leds_L1_UL_L = CRGB::Aqua; }
  if (midistate[17] == 1) { leds_L1_UL_T = CRGB::Aqua; }
  if (midistate[18] == 1) { leds_L1_UL_R = CRGB::Aqua; }
  if (midistate[19] == 1) { leds_L1_UR_B = CRGB::Aqua; }
  if (midistate[20] == 1) { leds_L1_UR_L = CRGB::Aqua; }
  if (midistate[21] == 1) { leds_L1_UR_T = CRGB::Aqua; }
  if (midistate[22] == 1) { leds_L1_UR_R = CRGB::Aqua; }
  if (midistate[23] == 1) { leds_L1_LL_B = CRGB::Aqua; }
  if (midistate[24] == 1) { leds_L1_LL_L = CRGB::Aqua; }
  if (midistate[25] == 1) { leds_L1_LL_T = CRGB::Aqua; }
  if (midistate[26] == 1) { leds_L1_LL_R = CRGB::Aqua; }
  if (midistate[27] == 1) { leds_L1_LR_B = CRGB::Aqua; }
  if (midistate[28] == 1) { leds_L1_LR_L = CRGB::Aqua; }
  if (midistate[29] == 1) { leds_L1_LR_T = CRGB::Aqua; }
  if (midistate[30] == 1) { leds_L1_LR_R = CRGB::Aqua; }
 

  FastLED.show();
}



Is it an issue with the way I'm reading and storing the MIDI data? Or is there a limitation on the Arduino's capability to receive and process MIDI? MIDI can't possibly be THAT taxing can it?

Grumpy_Mike

Quote
Or is there a limitation on the Arduino's capability to receive and process MIDI?
No.

It would help if you posted all the code. What you posted is not compilable.

Basically this could be a few things.
1) your sending device going into running status mode when notes are close together.
2) the delay in the unshown fading routines
3) the MIDI library you use, why use a library any way?
4) the neopixel driver disabling the interrupts for long enough to miss an incoming serial byte.

Grumpy_Mike

Thanks.
I think your loop function is too long and a bit mixed up. The newNote variable did not seem to be doing much for you. I think the start would be better like this:-
Code: [Select]
if (MIDI.read()) {
    location_byte = MIDI.getType();
    channel = MIDI.getChannel();
    note = MIDI.getData1();
    velocity = MIDI.getData2();
   if (location_byte == midi_on) midistate[note] = 1;
   if (location_byte == midi_off) midistate[note] = 0;     
  }

Also the compiler warns you are running out of memory and the midistate array uses 128 bytes of memory and you only seem to use index values up to 30.

I also worry about the EVERY_N_MILLISECONDS line I did not understand it. I did some searching and found this:- https://arduino.stackexchange.com/questions/19771/every-n-milliseconds so not very helpful but that might be a reason why the libiary might be dropping bits. Especially if it involves interrupts and delays.

When you write a chunk of code where all the lines are nearly identical that tells you their must be a better way of writing that code. Learn how to use arrays and loops to shorten that bit at the end to just a few lines.

However what might be the problem is that after each message you are going through setting up all the lights and then displaying them. Why not do all that only when you have not read a MIDI message and you haven't done it before. To do that set a boolean variable called refresh to true in the midi read part and then only do all the setting of LEDs and outputting them if this variable is true. Remember to make it false after calling the show method to stop you refreshing the lights when no messages are being received.

Grumpy_Mike

Just noticed something very odd about this code:-

Code: [Select]

CRGB rawLeds_L1[NUM_LEDS_LR];
  CRGBSet leds_L1 (rawLeds_L1, NUM_LEDS_LR);
    CRGBSet leds_L1_UL (leds_L1(0,69));


At first I thought this was a function but you are using square braces, so it looks like an array declaration. The other lines of code are not in any function so what do you think they will do.

Grumpy_Mike

I have been trying your code and have modified it like I said in reply #3 so it only refreshes the LEDs every MIDI event. Even so it takes 8.48mS to output all your data to the LEDs. Now with a MIDI rate of 31250 baud, that means you get 3125 bytes per second at MIDI rates. So one byte takes 0.32mS to be received.

While you are outputting data to the LEDs the interrupts have to be disabled to achieve the correct timing, that means the serial port can not transfer incoming data to the serial buffer. So after the next byte is receiver the UART will loose that first byte. So basically when you say:- 
Quote
I was able to determine that the notes need to be around 8-9ms apart;
You are quite right, you do because it takes that long to write the data out to the LEDs. This is the potential problem I pointed out in reply#1 - section 4.

One solution would be to switch to the APA102C type of LED strip. These have a clock and data signal and the timing of the data signals is not critical so they can be driven without disabling the interrupts and so allow incoming serial data ( MIDI ) to be placed in the buffer for later processing.

batmundo

Crap, I accidentally mis-clicked and deleted the code!

Putting it back up just for reference.


Code: [Select]

#include "FastLED.h"
#include <MIDI.h>


FASTLED_USING_NAMESPACE

#if defined(FASTLED_VERSION) && (FASTLED_VERSION < 3001000)
#warning "Requires FastLED 3.1 or later; check github for latest code."
#endif

MIDI_CREATE_DEFAULT_INSTANCE();

#define DATA_PIN_C 3
#define DATA_PIN_L 4
#define DATA_PIN_R 5
#define COLOR_ORDER GRB
#define LED_TYPE WS2813
#define NUM_LEDS_LR 280
#define NUM_LEDS_LR_Q 70
#define BRIGHTNESS  255


CRGB rawLeds_L1[NUM_LEDS_LR];
  CRGBSet leds_L1 (rawLeds_L1, NUM_LEDS_LR);
    CRGBSet leds_L1_UL (leds_L1(0,69));
      CRGBSet leds_L1_UL_B (leds_L1_UL(0,16));  //17
      CRGBSet leds_L1_UL_L (leds_L1_UL(17,34));  //18
      CRGBSet leds_L1_UL_T (leds_L1_UL(35,52));  //18
      CRGBSet leds_L1_UL_R (leds_L1_UL(53,69));  //17
    CRGBSet leds_L1_UR (leds_L1(70,139));
      CRGBSet leds_L1_UR_B (leds_L1_UR(0,16));  //17
      CRGBSet leds_L1_UR_L (leds_L1_UR(17,34));  //18
      CRGBSet leds_L1_UR_T (leds_L1_UR(35,52));  //18
      CRGBSet leds_L1_UR_R (leds_L1_UR(53,69));  //17
    CRGBSet leds_L1_LL (leds_L1(140,209));
      CRGBSet leds_L1_LL_B (leds_L1_LL(0,16));  //17
      CRGBSet leds_L1_LL_L (leds_L1_LL(17,34));  //18
      CRGBSet leds_L1_LL_T (leds_L1_LL(35,52));  //18
      CRGBSet leds_L1_LL_R (leds_L1_LL(53,69));  //17
    CRGBSet leds_L1_LR (leds_L1(210,280));
      CRGBSet leds_L1_LR_B (leds_L1_LR(0,16));  //17
      CRGBSet leds_L1_LR_L (leds_L1_LR(17,34));  //18
      CRGBSet leds_L1_LR_T (leds_L1_LR(35,52));  //18
      CRGBSet leds_L1_LR_R (leds_L1_LR(53,69));  //17



//MIDI variables
byte midi_on = 0x90;
byte midi_off = 0x80;
byte location_byte;
byte channel;
byte note;
byte velocity;
byte midistate[128];
boolean newNote = false;



void setup() {
  delay(3000); // 3 second delay for recovery

  MIDI.begin(MIDI_CHANNEL_OMNI);

  // tell FastLED about the LED strip configuration
  FastLED.addLeds<LED_TYPE, DATA_PIN_L, COLOR_ORDER>(leds_L1, NUM_LEDS_LR).setCorrection(TypicalLEDStrip);

  // set master brightness control
  FastLED.setBrightness(BRIGHTNESS);
  set_max_power_in_volts_and_milliamps(5, 4000);              // FastLED Power management set at 5V, 500mA
}


void loop() {
  int setChannel = 1;
 
  if (MIDI.read()) {
    location_byte = MIDI.getType();
    channel = MIDI.getChannel();
    note = MIDI.getData1();
    velocity = MIDI.getData2();
    if (location_byte == midi_on || location_byte == midi_off) newNote = true;
  }
 
 
  if (newNote) {
    if (location_byte == midi_on) {
      midistate[note] = 1;
    }
    if (location_byte == midi_off) {
      midistate[note] = 0;
    }
    newNote = false;
  }
 
  EVERY_N_MILLISECONDS(20) {
    fadeToBlackBy(leds_L1, NUM_LEDS_LR, 64);
  }

 
  if (midistate[11] == 1) {
    leds_L1_UL_B = CRGB::Aqua;
    leds_L1_UL_L = CRGB::Aqua;
    leds_L1_UL_T = CRGB::Aqua;
    leds_L1_UL_R = CRGB::Aqua;
  }
  if (midistate[12] == 1) {
    leds_L1_UR_B = CRGB::Aqua;
    leds_L1_UR_L = CRGB::Aqua;
    leds_L1_UR_T = CRGB::Aqua;
    leds_L1_UR_R = CRGB::Aqua;
  }
  if (midistate[13] == 1) {
    leds_L1_LL_B = CRGB::Aqua;
    leds_L1_LL_L = CRGB::Aqua;
    leds_L1_LL_T = CRGB::Aqua;
    leds_L1_LL_R = CRGB::Aqua;
  }
  if (midistate[14] == 1) {
    leds_L1_LR_B = CRGB::Aqua;
    leds_L1_LR_L = CRGB::Aqua;
    leds_L1_LR_T = CRGB::Aqua;
    leds_L1_LR_R = CRGB::Aqua;
  }
 
  if (midistate[15] == 1) { leds_L1_UL_B = CRGB::Aqua; }
  if (midistate[16] == 1) { leds_L1_UL_L = CRGB::Aqua; }
  if (midistate[17] == 1) { leds_L1_UL_T = CRGB::Aqua; }
  if (midistate[18] == 1) { leds_L1_UL_R = CRGB::Aqua; }
  if (midistate[19] == 1) { leds_L1_UR_B = CRGB::Aqua; }
  if (midistate[20] == 1) { leds_L1_UR_L = CRGB::Aqua; }
  if (midistate[21] == 1) { leds_L1_UR_T = CRGB::Aqua; }
  if (midistate[22] == 1) { leds_L1_UR_R = CRGB::Aqua; }
  if (midistate[23] == 1) { leds_L1_LL_B = CRGB::Aqua; }
  if (midistate[24] == 1) { leds_L1_LL_L = CRGB::Aqua; }
  if (midistate[25] == 1) { leds_L1_LL_T = CRGB::Aqua; }
  if (midistate[26] == 1) { leds_L1_LL_R = CRGB::Aqua; }
  if (midistate[27] == 1) { leds_L1_LR_B = CRGB::Aqua; }
  if (midistate[28] == 1) { leds_L1_LR_L = CRGB::Aqua; }
  if (midistate[29] == 1) { leds_L1_LR_T = CRGB::Aqua; }
  if (midistate[30] == 1) { leds_L1_LR_R = CRGB::Aqua; }
 
 


  FastLED.show();
}

batmundo

Thanks.
I think your loop function is too long and a bit mixed up. The newNote variable did not seem to be doing much for you. I think the start would be better like this:-
Code: [Select]
if (MIDI.read()) {
    location_byte = MIDI.getType();
    channel = MIDI.getChannel();
    note = MIDI.getData1();
    velocity = MIDI.getData2();
   if (location_byte == midi_on) midistate[note] = 1;
   if (location_byte == midi_off) midistate[note] = 0;     
  }

Also the compiler warns you are running out of memory and the midistate array uses 128 bytes of memory and you only seem to use index values up to 30.

I also worry about the EVERY_N_MILLISECONDS line I did not understand it. I did some searching and found this:- https://arduino.stackexchange.com/questions/19771/every-n-milliseconds so not very helpful but that might be a reason why the libiary might be dropping bits. Especially if it involves interrupts and delays.

When you write a chunk of code where all the lines are nearly identical that tells you their must be a better way of writing that code. Learn how to use arrays and loops to shorten that bit at the end to just a few lines.

However what might be the problem is that after each message you are going through setting up all the lights and then displaying them. Why not do all that only when you have not read a MIDI message and you haven't done it before. To do that set a boolean variable called refresh to true in the midi read part and then only do all the setting of LEDs and outputting them if this variable is true. Remember to make it false after calling the show method to stop you refreshing the lights when no messages are being received.
- Good point, I guess the newNote isn't really needed!

- I had initially set the array at 128 to cover MIDI values 0-127, but at this point I've figured out I only need 55 notes to cover the different subdivisons of the mirror, so I should probably change that.

- Yeah, I assumed there must be a better way of mapping the notes to functions, I just haven't figured out how to do it yet.

- Oh, I have other LED functions I'm using that would need constant refreshing between notes, this particular LED display setup was mainly for troubleshooting.


batmundo

Just noticed something very odd about this code:-

Code: [Select]

CRGB rawLeds_L1[NUM_LEDS_LR];
  CRGBSet leds_L1 (rawLeds_L1, NUM_LEDS_LR);
    CRGBSet leds_L1_UL (leds_L1(0,69));


At first I thought this was a function but you are using square braces, so it looks like an array declaration. The other lines of code are not in any function so what do you think they will do.
This is how I'm divvying up the LED strip - rawLeds_L1 is the whole strip, leds_L1 is the full length of rawLeds_L1, leds_L1_UL a quarter section of leds_L1, etc.

Now that I think about it, I still don't understand why I had to declare the full strip again as leds_L1. My guess is that since I'm declaring a function to accept CRGBSets, I wouldn't be able to send a CRGB rawLed value, and would have to assign a CRGBSet value to it as well? Which kind of caused a little issue in one of FastLED's functions I was using where I couldn't figure out how pass it a CRGBSet value...

batmundo

I have been trying your code and have modified it like I said in reply #3 so it only refreshes the LEDs every MIDI event. Even so it takes 8.48mS to output all your data to the LEDs. Now with a MIDI rate of 31250 baud, that means you get 3125 bytes per second at MIDI rates. So one byte takes 0.32mS to be received.

While you are outputting data to the LEDs the interrupts have to be disabled to achieve the correct timing, that means the serial port can not transfer incoming data to the serial buffer. So after the next byte is receiver the UART will loose that first byte. So basically when you say:-  You are quite right, you do because it takes that long to write the data out to the LEDs. This is the potential problem I pointed out in reply#1 - section 4.

One solution would be to switch to the APA102C type of LED strip. These have a clock and data signal and the timing of the data signals is not critical so they can be driven without disabling the interrupts and so allow incoming serial data ( MIDI ) to be placed in the buffer for later processing.
Does this mean with the current LED strip I have right now, it just won't be possible to read the notes simultaneously?

batmundo

For reference, here's a video so you get an idea of what the output is:

https://www.dropbox.com/s/8bitz28alzehovy/20171007_211115.mp4?dl=0


I set up three sets of MIDI notes, each cycling through one quadrant of the square:

1) all notes quantized to 1/4 notes
2) each set of 4 notes (for each side of the squares) slightly offset from each other - you can actually see the edges come in with a little bit of delay from each other
3) 4 single notes, each assigned a single function to display the four edges of each square in a single call


Oh, and the goal is to be able to do this using MIDI to design the sequences, rather than having to program each sequence per note (i.e. example 3 in the previous video):

https://www.dropbox.com/s/g3hwmqobrk5zxbp/20170928_004345.mp4?dl=0

Grumpy_Mike

Quote
Does this mean with the current LED strip I have right now, it just won't be possible to read the notes simultaneously?
Basically yes. Not that you will ever be able to read notes simultaneously at all because the MIDI protocol is serial so you only send one note at a time, but you will not be able to send the notes as close together as they will go.

With the amount of time it takes to feed that many LEDs with the data and the fact that the interrupts must be off to generate the precise timing you need then the notes need to be spaced 8.5mS apart.

It might be possible, in theory, to use a board with the NRF52 processor with the Adafruit Neopixel libiary because that processor has a DMA capability and so does not need the interrupts disabled to achieve the correct timing. I say "in theory" because I don't know what product this is, I only know by reading the libiary code. I know that this product is not the Adafruit Bluefruit nRF52 Feather because that caused a compile error when I tried it.

batmundo

Bummer! Understood, I guess I'll just have to work with what I have for now then. Thanks for the help!

Go Up