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

Allow large SysEx Messages to be broken up into smaller packets when transmitted #337

Merged
merged 2 commits into from
Apr 18, 2024

Conversation

insolace
Copy link
Contributor

@insolace insolace commented Apr 9, 2024

We have found that older/slower systems can choke when sending large (> 100k) SysEx firmware payloads to our devices, which can cause problems with devices out in the field. To make our end user device firmware update process safer, we need to be able to slow down the transmission of these payloads and break them into smaller chunks.

Currently both the MacOS and Windows sendMessage methods check to see if the first byte is a SysEx start byte (0xF0) before allowing a > 3 byte message to be sent. If the first byte is not 0xF0, an error message is returned and the message is rejected. This not only inhibits our ability to break up the SysEx payload, it seems antithetical to RtMIDI's role as an agnostic transport that does not parse or format messages. It should be up to the application to determine if messages are formatted correctly, RtMIDI should just send and receive data.

This PR removes the blocking 0xF0 check on MacOS entirely. RtMIDI has not used the CoreMIDI sysex specific message method for years, and the standard send is still perfectly fine for this purpose.

On Windows, we've replaced the 0xF0 check with a size check - messages >3 bytes use the sysex method, otherwise the standard channel/realtime method is used.

The Alsa/Jack send methods do not have a 0xF0 check, so we did not modify them.

We've extensively tested this PR with whole and broken up SysEx payloads, on Windows and MacOS, with newer and older machines. This solved the issues we were having sending firmware updates.

…ecking for SysEx Start Message (RtMIDI should handle transport, it should not police message formatting)
@insolace
Copy link
Contributor Author

Below is an example of how we parse our midi transmit buffer using this PR, breaking up standard messages and sysex into appropriately sized chunks.

#define MAX_MIDI_SYSEX_SIZE 250000 // reject payloads larger than this, also prevents buffer overflow

std::vector<uchar> packet; // the midi TX buffer, put outgoing midi messages here to transmit them

unsigned int sysExTxChunkSize = 48; // this is equivalent to 16 USB MIDI messages, or a full USB MIDI packet 
unsigned int sysExTxChunkDelay = 1; // limit sysex to one packet per 1ms

RtMidiOut *midi_out = new RtMidiOut(); // this is set up when the app is initialized

TimeElapsed syxExTxChunkTimer; // this is an object that tracks time elapsed since it was last started/restarted, initialized when the app loads



// this function is called every 1ms
void slotEmptyMIDIBuffer()
{
    std::vector<uchar> message;
    static bool sendLastChunk = false;

    if (packet.size() == 0)
    {
        return;
    }

    if (packet.size() > MAX_MIDI_SYSEX_SIZE)
    {
        cout << "ERROR: SYSEX TX BUFFER OVERFLOW, DISCARDING";
        packet.clear();
        return;
    }

    // send sysex in chunks
    if (packet.size() > sysExTxChunkSize || sendLastChunk == true)
    {
        if (syxExTxChunkTimer.elapsed() < sysExTxChunkDelay)
        {
            return; // enforce speed limit
        }
        syxExTxChunkTimer.restart();

        unsigned int sizeToSend = sendLastChunk ? (unsigned int)packet.size() : sysExTxChunkSize;

        // Create a sub-vector for the chunk to send
        std::vector<uint8_t> chunkToSend(packet.begin(), packet.begin() + sizeToSend);

        // Send the chunk
        try
        {
            midi_out->sendMessage(&chunkToSend);
        }
        catch (RtMidiError &error)
        {
            cout << "MIDI SEND LARGE SYSEX ERR";
            packet.clear();
            return;
        }

        if (sendLastChunk) // if we just sent the last chunk, clear flag/buffer and exit
        {
            sendLastChunk = false;
            packet.clear();
            return;
        }

        // Remove the sent chunk from the packet
        packet.erase(packet.begin(), packet.begin() + sizeToSend);


        if (packet.size() < sysExTxChunkSize) // if we have one chunk left, loop around and send it
        {
            sendLastChunk = true;
        }
        return;
    }
    else
    {
        for (size_t i = 0; i < packet.size(); ++i)
        {
            if (packet[i] == MIDI_SX_START) // small sysex, less than chunk size
            {
                std::vector<uchar> smallSysExPacket;

                bool stopSearch = false;

                while (!stopSearch)
                {
                    smallSysExPacket.push_back(packet[i++]);

                    if (packet[i] == MIDI_SX_STOP || i >= packet.size())
                    {
                        stopSearch = true;
                    }
                    else if (packet[i] > 127)
                    {
                        i--; // decrement index so the next loop looks at this byte
                        stopSearch = true;
                    }
                }
                smallSysExPacket.push_back(MIDI_SX_STOP); // always end sysex properly even if packet is incomplete

                try
                {
                    midi_out->sendMessage( &smallSysExPacket );
                }
                catch (RtMidiError &error)
                {
                    cout << "MIDI SEND SMALL SYSEX ERR: " << error;
                    packet.clear();
                    return;
                }
                continue; // continue for loop
            }
            else // channel messages
            {
                // Check if the current byte is a status byte
                if (packet[i] >= 0x80)
                {
                    // If there's already a message being constructed, send it
                    if (!message.empty())
                    {
                        try
                        {
                            midi_out->sendMessage( &message );
                        }
                        catch (RtMidiError &error)
                        {
                            cout << "MIDI SEND PACKET ERR: " << error;
                        }
                        message.clear(); // Clear the message vector for the next message
                    }
                }
            }

            // Add the current byte to the message
            message.push_back(packet[i]);

            // If it's the last byte but not a status byte, ensure the message is sent
            if (i == packet.size() - 1 && !message.empty())
            {
                try
                {
                    midi_out->sendMessage( &message );
                }
                catch (RtMidiError &error)
                {
                   cout << "MIDI SEND PACKET ERR: " << error;
                }
            }
        }
    }

    // Clear the packet after processing all messages
    packet.clear();
}

@insolace insolace closed this Apr 10, 2024
@insolace insolace reopened this Apr 10, 2024
@garyscavone garyscavone merged commit 53809fc into thestk:master Apr 18, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants