search

FortySevenEffects / arduino_midi_library

MIDI for Arduino
MIT License
1.67k stars 263 forks source link

wrong midi CC received with keyboard #363

Open Genesis2921 opened 1 month ago

Genesis2921 commented 1 month ago

Context

Please answer a few questions to help us understand your problem better and guide you to a solution:

Describe your project and what you expect to happen:

I try to turn on lights when I receive a CC command. set to CC 11 on channel 1 the value is sent to the leds.

Describe your problem (what does not work):

I randomly receive wrong CC number when I turn only the CC11 knob on my M-audio oxygen keyboard

Steps to reproduce

I've check the hardware on scope for delay, and all works fine, I've tried the keyboard under midi ox with usb adapter > the keyboard works fine. it's as if there was a buffer overflow and in this case, a bad CC was sent. the test code is as simple as this (the led is used for local debug):

void handleCC(byte channel, byte controler, byte value) { controler_channel = channel; controler_number = controler; controler_value = value;
cc_flag = 1; // value received if(controler_number != 11 ) { digitalWrite(LED, HIGH); delay(100); digitalWrite(LED, LOW);
} } main loop: void loop() { MIDI.read(); }

when I play quickly with the button, making mini maxi jumps, the blue LED flashes while everything starts from the keyboard in CC11 channel 1 test with callbacks and with cortex M4 power > same thing.

Image

franky47 commented 1 month ago

it's as if there was a buffer overflow and in this case,

I think your intuition was correct. Blocking the main thread with delay(100) is likely the cause here.

Doing a bit of back of the envelope maths, MIDI runs at 320µs per byte, so about 1ms for a 3-byte message. Blocking during 100ms leaves about 300 bytes having to go in the RX buffer (which IIRC is 1024 bytes by default). The issue is that once the delay clears, if the next message in the queue is also a good CC, then the delay blocks again, having only cleared 3 bytes and leaving up to 297 bytes in the queue, leading to accumulation of data and a buffer overflow.

You could try a non-blocking way of turning your LED on and off by:

  1. Keeping a record (using millis()) of the last time you turned the LED on, when receiving a valid CC and turning the LED on
  2. In the main loop, comparing the current time to that of that lastTurnedOnAt variable, and if greater than 100ms, turn the LED off (you could set another flag to not do that operation again, but digitalWrite(LED, LOW) on an already low pin won't do much and is faster than adding another check).

Hope this helps.

Genesis2921 commented 1 month ago

Thanks Franky47 ;) but as I've specified above, the led is for test directly inside the interrupt routine. to check if even here the value is different from 11. my main loop check for the volatile interrupt flag "cc_flag" and the bug appears too. even if I do nothing in the "if" test. a friend checked the code too. it's a very strange behaviour. if the itsy bitsy cortex M4 120MHz is doing nothing in the main loop how I can fill the buffer only when turning a knob. even if the buffer is full, it's still weird to receive values ​​on a bad controler number. it's like a bad data parsing.

Image

Image

franky47 commented 1 month ago

Do you have a minimally reproducible sketch I could try?

Genesis2921 commented 1 month ago

here is the code used for debug. a scope is connected to A6 it toggles A6 output when the received controler value is different from 11

// ARDUINO MICRO PRO CARD

include <avr/pgmspace.h>

include

include "MIDI/src/midi.h" //load the library from sketch subfolder

volatile unsigned char channel_set = 1;//listening channel volatile unsigned char channel_in; volatile unsigned char note_in; volatile unsigned char controler_channel =0; volatile unsigned char controler_number =0; volatile unsigned char controler_value =0;

volatile boolean cc_flag = 0;

define SCOPE A6

//MIDI_CREATE_INSTANCE(HardwareSerial, Serial1, MIDI)// start midi on uart 1 MIDI_CREATE_DEFAULT_INSTANCE()// start midi on uart 1

// -----------------------------------------------------------------------------

// This function will be automatically called when a NoteOn is received. // It must be a void-returning function with the correct parameters, // see documentation here: // https://github.com/FortySevenEffects/arduino_midi_library/wiki/Using-Callbacks

void handleCC(byte channel, byte controler, byte value) { controler_channel = channel; controler_number = controler; controler_value = value;
cc_flag = 1; // value received }

// -----------------------------------------------------------------------------

void setup() { pinMode(SCOPE, OUTPUT); digitalWrite(SCOPE, LOW);

// Initiate MIDI communications, listen to all channels
MIDI.begin(MIDI_CHANNEL_OMNI);

}

void loop() {

 if (cc_flag)                // Is there a MIDI message incoming ?
{
      if(controler_number != 11 )
      {
        digitalWrite(SCOPE, HIGH);
        delayMicroseconds(10);
        digitalWrite(SCOPE, LOW);
        delayMicroseconds(10);        
      }
 cc_flag = 0;

}

}

franky47 commented 1 month ago

Thanks. The volatile specifier is not needed, the callbacks are also running in the main thread (within MIDI.read), they are not interrupt based.

Could it be that you're receiving messages from another channel (ie: you might want to check the channel + CC number combination, or listen to channel 1 specifically) ?

Genesis2921 commented 1 month ago

yes I have tried with and without the volatile specifier to be sure. the bug appears when the keyboard is set to channel 1 and only the knob set to CC10 is used. (but do the same thing on another channel) in the original code, I check the channel too and the channel is ok. and under midi ox the keyboard works fine (no other channel midi message) an idea would be to do a selective parsing, indicating what you want to receive and rejecting everything else to save CPU time. I think my next step will be to make my own parsing from serial and buffer to see if I have the same issue.