Added missing if-statement for "useRunningStatus", causing a bug with a midi device

[luckyspacetraveller]

Added if statement asking for "useRunningStatus", as it caused a bug
with a certain midi device, causing the arduino to resolve the same midi
data multiple times. I think it was forgotten here, as the comment also
suggests that this section is only for "Activate running status (if enabled...)"

[franky47]

This setting is for TX only, as RX should understand messages with and without running status to be compliant with the spec. The comment could be updated however, as it is indeed not too clear.

Now I'm interested in the bug you describe, what kind of device is it and do you have a MIDI dump of the data it sends ?

[luckyspacetraveller]

Hey, thanks for your reply!

It was actually a couple of weeks ago, so to give you the precise data, I'd have to check again.

I'll wrap up what I can remember for now:

The issue happens with this midi device
https://www.justmusic.de/Keys/Zubehoer-fuer-Tasteninstrumente/MIDI-Tools/Justin-MP-02-USB-MIDI-Interface

it is a simple cheap USB-Midi converter. Now, I am pretty sure that the origin of the bug is caused by this device, not your library, but more below.

The issue happening was, while we've been sending a midi clock from Cubase through this Midi-Interface, sending a note would record multiple values, in fact so fast it could even crash the attached DACs for the note-CV output. But using another USB-Midi Interface like Moog Subsequent37, the crash and multiple notes didn't happen.

We used all sorts of Midi-Monitoring, but nothing unusual was sent there except the 0xF8 clock command or notes, so Midi-wise it is all good, and so far no difference between the two USB-Midi ways.

We digged deeper and analyzed the data stream going through the cable with an oscilloscope and found a mysterious data byte, that the cheap midi device added after every 0xF8 command, but was not there with the obviously much better built Moog.

Now, unfortunately at the moment I can not recall the exact byte-pattern, but it messed up the byte order so with the next note on command, the velocity (3rd byte) was set as note value, and that every 0xF8 byte, which is send 24 times per beat.

One thing is absolutely for sure. that myterious data byte should not be there!! We sent that false Midi signal to the Moog Synthesizer, to see what happens there, and this unit is actually able to filter and throw away that byte.

It would be easy to simply ask for that byte and filter it out too, but we wanted to know whats happening, so we went through all our code, and all your code and finally worked out, that this faulty byte causes the program to store the running status mRunningStatus_RX instead of reseting it to InvalidType. And the next iteration of parse(), it then has a wrong mRunningStatus_RX. By the way is it really a part for Tx only? As it is part of the parse() method.

I have to set up the oscilloscope once more to find out the exact data pattern again if you're interested.

This if-statement probably will not save the day when UseRunningStatus is enabled, then the faulty byte will probably mess it up again, this cheap Midi-Device is just wrong. But I also didn't see a reason why the if-statement shouldn't be there anyway, that's why I just wanted to commit it back to you. :)

Thank you for this great library. If you have any questions, let me know.

Cheers
Luke

// 914:
if (Settings::UseRunningStatus) { // <---- we added this, to prevent saving the status when it shouldn't be

                switch (mMessage.type)
                {
                    case NoteOff:
                    case NoteOn:
                    case AfterTouchPoly:
                    case ControlChange:
                    case ProgramChange:
                    case AfterTouchChannel:
                    case PitchBend:
	                    // Running status enabled: store it from received message
	                    mRunningStatus_RX = mPendingMessage[0];
	                    break;

                    default:
                    // No running status 
	                    mRunningStatus_RX = InvalidType;
	                    break;
                }
            }

            else {

                mRunningStatus_RX = InvalidType; // <---- purposly save InvalidType, when UseRunningStatus mode is false.
            }