Whats the right way to get correct Midi?

Hey there!

I’ve been digging through the forum trying to find the right way to get stable midi-in but although some threads ended up with ‘that works now’ - I couldn’t find a full solution described.

I got the latest version of the libraries yesterday (10/30). To make sure that nothing on my side is messing things up I’m running a slightly enhanced version of DaisyExamples\patch\Midi which outputs the played notes to the oled. Full code is below.

I’m sending in a sequence of eight eighth notes going up one step at a very slow tempo (40 bpm).
The results I get on the Patch are first stable but then start to jump on top of each others, then get back to stable.
Video capture is here: https://photos.app.goo.gl/ZQMktUwxKd39c8ZBA
Top row on the oled is notes on, bottom notes off.
Would be great to get advice how to get this running stable and probably also good to have a stable version as an example.

Thanks a lot for your help! (and also for having this great product!)
Daniel

#pragma once
#ifndef DSY_MIDI_H
#define DSY_MIDI_H

// TODO: make this adjustable
#define SYSEX_BUFFER_LEN 128

#include <stdint.h>
#include <stdlib.h>
#include "per/uart.h"
#include "util/ringbuffer.h"
#include "hid/MidiEvent.h"
#include "hid/usb_midi.h"
#include "sys/system.h"

namespace daisy
{
/** @brief   Transport layer for sending and receiving MIDI data over UART 
 *  @details This is the mode of communication used for TRS and DIN MIDI
 *  @ingroup midi
*/
class MidiUartTransport
{
  public:
    MidiUartTransport() {}
    ~MidiUartTransport() {}

    struct Config
    {
        UartHandler::Config::Peripheral periph;
        dsy_gpio_pin                    rx;
        dsy_gpio_pin                    tx;

        Config()
        {
            periph = UartHandler::Config::Peripheral::USART_1;
            rx     = {DSY_GPIOB, 7};
            tx     = {DSY_GPIOB, 6};
        }
    };

    inline void Init(Config config)
    {
        UartHandler::Config uart_config;

        //defaults
        uart_config.baudrate   = 31250;
        uart_config.stopbits   = UartHandler::Config::StopBits::BITS_1;
        uart_config.parity     = UartHandler::Config::Parity::NONE;
        uart_config.mode       = UartHandler::Config::Mode::TX_RX;
        uart_config.wordlength = UartHandler::Config::WordLength::BITS_8;

        //user settings
        uart_config.periph        = config.periph;
        uart_config.pin_config.rx = config.rx;
        uart_config.pin_config.tx = config.tx;

        uart_.Init(uart_config);
    }

    inline void    StartRx() { uart_.StartRx(); }
    inline size_t  Readable() { return uart_.Readable(); }
    inline uint8_t Rx() { return uart_.PopRx(); }
    inline bool    RxActive() { return uart_.RxActive(); }
    inline void    FlushRx() { uart_.FlushRx(); }
    inline void    Tx(uint8_t* buff, size_t size) { uart_.PollTx(buff, size); }

  private:
    UartHandler uart_;
};

/** 
    @brief Simple MIDI Handler \n 
    Parses bytes from an input into valid MidiEvents. \n 
    The MidiEvents fill a FIFO queue that the user can pop messages from.
    @author shensley
    @date March 2020
    @ingroup midi
*/
template <typename Transport>
class MidiHandler
{
  public:
    MidiHandler() {}
    ~MidiHandler() {}

    struct Config
    {
        typename Transport::Config transport_config;
    };

    /** Initializes the MidiHandler 
     *  \param config Configuration structure used to define specifics to the MIDI Handler.
     */
    void Init(Config config)
    {
        config_ = config;

        transport_.Init(config_.transport_config);

        event_q_.Init();
        incoming_message_.type = MessageLast;
        pstate_                = ParserEmpty;
    }

    /** Starts listening on the selected input mode(s). MidiEvent Queue will begin to fill, and can be checked with HasEvents() */
    void StartReceive() { transport_.StartRx(); }

    /** Start listening */
    void Listen()
    {
        uint32_t now;
        now = System::GetNow();
        while(transport_.Readable())
        {
            last_read_ = now;
            Parse(transport_.Rx());
        }

        // In case of UART Error, (particularly
        //  overrun error), UART disables itself.
        // Flush the buff, and restart.
        if(!transport_.RxActive())
        {
            pstate_ = ParserEmpty;
            transport_.FlushRx();
            StartReceive();
        }
    }

    /** Checks if there are unhandled messages in the queue 
    \return True if there are events to be handled, else false.
     */
    bool HasEvents() const { return event_q_.readable(); }


    /** Pops the oldest unhandled MidiEvent from the internal queue
    \return The event to be handled
     */
    MidiEvent PopEvent() { return event_q_.Read(); }

    /** SendMessage
    Send raw bytes as message
    */
    void SendMessage(uint8_t* bytes, size_t size)
    {
        transport_.Tx(bytes, size);
    }

    /** Feed in bytes to state machine from a queue.
    Populates internal FIFO queue with MIDI Messages
    For example with uart:
    midi.Parse(uart.PopRx());
    \param byte &
    */
    void Parse(uint8_t byte)
    {
        // reset parser when status byte is received
        if((byte & kStatusByteMask) && pstate_ != ParserSysEx)
        {
            pstate_ = ParserEmpty;
        }
        switch(pstate_)
        {
            case ParserEmpty:
                // check byte for valid Status Byte
                if(byte & kStatusByteMask)
                {
                    // Get MessageType, and Channel
                    incoming_message_.channel = byte & kChannelMask;
                    incoming_message_.type    = static_cast<MidiMessageType>(
                        (byte & kMessageMask) >> 4);

                    // Validate, and move on.
                    if(incoming_message_.type < MessageLast)
                    {
                        pstate_ = ParserHasStatus;
                        // Mark this status byte as running_status
                        running_status_ = incoming_message_.type;

                        if(running_status_ == SystemCommon)
                        {
                            incoming_message_.channel = 0;
                            //system real time = 1111 1xxx
                            if(byte & 0x08)
                            {
                                incoming_message_.type = SystemRealTime;
                                running_status_        = SystemRealTime;
                                incoming_message_.srt_type
                                    = static_cast<SystemRealTimeType>(
                                        byte & kSystemRealTimeMask);

                                //short circuit to start
                                pstate_ = ParserEmpty;
                                event_q_.Write(incoming_message_);
                            }
                            //system common
                            else
                            {
                                incoming_message_.sc_type
                                    = static_cast<SystemCommonType>(byte
                                                                    & 0x07);
                                //sysex
                                if(incoming_message_.sc_type == SystemExclusive)
                                {
                                    pstate_ = ParserSysEx;
                                    incoming_message_.sysex_message_len = 0;
                                }
                                //short circuit
                                else if(incoming_message_.sc_type > SongSelect)
                                {
                                    pstate_ = ParserEmpty;
                                    event_q_.Write(incoming_message_);
                                }
                            }
                        }
                    }
                    // Else we'll keep waiting for a valid incoming status byte
                }
                else
                {
                    // Handle as running status
                    incoming_message_.type    = running_status_;
                    incoming_message_.data[0] = byte & kDataByteMask;
                    //check for single byte running status, really this only applies to channel pressure though
                    if(running_status_ == ChannelPressure
                       || running_status_ == ProgramChange
                       || incoming_message_.sc_type == MTCQuarterFrame
                       || incoming_message_.sc_type == SongSelect)
                    {
                        //Send the single byte update
                        pstate_ = ParserEmpty;
                        event_q_.Write(incoming_message_);
                    }
                    else
                    {
                        pstate_
                            = ParserHasData0; //we need to get the 2nd byte yet.
                    }
                }
                break;
            case ParserHasStatus:
                if((byte & kStatusByteMask) == 0)
                {
                    incoming_message_.data[0] = byte & kDataByteMask;
                    if(running_status_ == ChannelPressure
                       || running_status_ == ProgramChange
                       || incoming_message_.sc_type == MTCQuarterFrame
                       || incoming_message_.sc_type == SongSelect)
                    {
                        //these are just one data byte, so we short circuit back to start
                        pstate_ = ParserEmpty;
                        event_q_.Write(incoming_message_);
                    }
                    else
                    {
                        pstate_ = ParserHasData0;
                    }

                    //ChannelModeMessages (reserved Control Changes)
                    if(running_status_ == ControlChange
                       && incoming_message_.data[0] > 119)
                    {
                        incoming_message_.type = ChannelMode;
                        running_status_        = ChannelMode;
                        incoming_message_.cm_type
                            = static_cast<ChannelModeType>(
                                incoming_message_.data[0] - 120);
                    }
                }
                else
                {
                    // invalid message go back to start ;p
                    pstate_ = ParserEmpty;
                }
                break;
            case ParserHasData0:
                if((byte & kStatusByteMask) == 0)
                {
                    incoming_message_.data[1] = byte & kDataByteMask;

                    //velocity 0 NoteOns are NoteOffs
                    if(running_status_ == NoteOn
                       && incoming_message_.data[1] == 0)
                    {
                        incoming_message_.type = NoteOff;
                    }

                    // At this point the message is valid, and we can add this MidiEvent to the queue
                    event_q_.Write(incoming_message_);
                }
                else
                {
                    // invalid message go back to start ;p
                    pstate_ = ParserEmpty;
                }
                // Regardless, of whether the data was valid or not we go back to empty
                // because either the message is queued for handling or its not.
                pstate_ = ParserEmpty;
                break;
            case ParserSysEx:
                // end of sysex
                if(byte == 0xf7)
                {
                    pstate_ = ParserEmpty;
                    event_q_.Write(incoming_message_);
                }
                else if(incoming_message_.sysex_message_len < SYSEX_BUFFER_LEN)
                {
                    incoming_message_
                        .sysex_data[incoming_message_.sysex_message_len]
                        = byte;
                    incoming_message_.sysex_message_len++;
                }
                break;
            default: break;
        }
    }

  private:
    enum ParserState
    {
        ParserEmpty,
        ParserHasStatus,
        ParserHasData0,
        ParserSysEx,
    };
    UartHandler                uart_;
    ParserState                pstate_;
    MidiEvent                  incoming_message_;
    RingBuffer<MidiEvent, 256> event_q_;
    uint32_t                   last_read_; // time of last byte
    MidiMessageType            running_status_;
    Config                     config_;
    Transport                  transport_;

    // Masks to check for message type, and byte content
    const uint8_t kStatusByteMask     = 0x80;
    const uint8_t kMessageMask        = 0x70;
    const uint8_t kDataByteMask       = 0x7F;
    const uint8_t kSystemCommonMask   = 0xF0;
    const uint8_t kChannelMask        = 0x0F;
    const uint8_t kRealTimeMask       = 0xF8;
    const uint8_t kSystemRealTimeMask = 0x07;
};

/**
 *  @{ 
 *  @ingroup midi
 *  @brief shorthand accessors for MIDI Handlers
 * */
using MidiUartHandler = MidiHandler<MidiUartTransport>;
using MidiUsbHandler  = MidiHandler<MidiUsbTransport>;
/** @} */
} // namespace daisy
#endif

I can’t view your video without giving Google access to all my photos. Bzzzt!

If I open that link in any browser that I’m not logged into Google it just shows the video straight with no questions asked.

On my iPad, it wouldn’t open that link unless I gave google permission to access all my photos. On my Mac, clicking it in Firefox brought me to a Google login screen, clicking it in Safari got me the video without any login.

Anyway, the video showed me something that might be relevant. I don’t know what device you’re using to send the MIDI messages (maybe Synthstrom Deluge?), but Daisy might be having trouble handling incoming MIDI clock messages. There’s a lot more MIDI activity than just your relatively slow NOTE_ON/NOTE_OFF activity.

It comes from the Deluge which only transmits note on/off and timing/transport data at that time.
I’ve done midi programming on Arduino and then Teensy for years and never had any issues like this - well apart from self generated code mess ups.

I’m pretty sure that the daisy’s processor should be capable of reading in the provided data without issues.

Yes, no doubt, should be capable. Teensy software is much more mature, and there’s a much more sophisticated user base.

Has this been corrected and verified?

Realized that I didn’t post the actual example but the midihandler code. Here is the correct modified example:

#include "daisy_patch.h"
#include "daisysp.h"
#include <string>

using namespace daisy;
using namespace daisysp;

DaisyPatch hw;
Oscillator osc;
Svf        filt;

void AudioCallback(AudioHandle::InputBuffer  in,
                   AudioHandle::OutputBuffer out,
                   size_t                    size)
{
    float sig;
    for(size_t i = 0; i < size; i++)
    {
        sig = osc.Process();
        filt.Process(sig);
        for(size_t chn = 0; chn < 4; chn++)
        {
            out[chn][i] = filt.Low();
        }
    }
}

// Typical Switch case for Message Type.
void HandleMidiMessage(MidiEvent m)
{
	bool hasBeenCleared = false;
    switch(m.type)
    {
        case NoteOn:
        {
			hw.display.Fill(false);
			hasBeenCleared = true;
			
            NoteOnEvent p = m.AsNoteOn();
            // This is to avoid Max/MSP Note outs for now..

            if(m.data[1] != 0)
            {
				std::string str  = std::to_string(p.note);
				char*       cstr = &str[0];
				hw.display.SetCursor(20, 10);
				hw.display.WriteString(cstr, Font_7x10, true);

                p = m.AsNoteOn();
                osc.SetFreq(mtof(p.note));
                osc.SetAmp((p.velocity / 127.0f));
            }
        }
        break;
        case NoteOff:
        {
			if(!hasBeenCleared)
				hw.display.Fill(false);

            NoteOffEvent p = m.AsNoteOff();
            // This is to avoid Max/MSP Note outs for now..

			std::string str  = std::to_string(p.note);
			char*       cstr = &str[0];
			hw.display.SetCursor(20, 20);
			hw.display.WriteString(cstr, Font_7x10, true);
        }
        break;
        case ControlChange:
        {
            ControlChangeEvent p = m.AsControlChange();
            switch(p.control_number)
            {
                case 1:
                    // CC 1 for cutoff.
                    filt.SetFreq(mtof((float)p.value));
                    break;
                case 2:
                    // CC 2 for res.
                    filt.SetRes(((float)p.value / 127.0f));
                    break;
                default: break;
            }
            break;
        }
        default: break;
    }
	 hw.display.Update();
}


// Main -- Init, and Midi Handling
int main(void)
{
    // Init
    float samplerate;
    hw.Init();
    samplerate = hw.AudioSampleRate();

    // Synthesis
    osc.Init(samplerate);
    osc.SetWaveform(Oscillator::WAVE_POLYBLEP_SAW);
    filt.Init(samplerate);

    //display
    std::string str  = "Midi";
    char*       cstr = &str[0];
    hw.display.WriteString(cstr, Font_7x10, true);
    hw.display.Update();

    // Start stuff.
    hw.midi.StartReceive();
    hw.StartAdc();
    hw.StartAudio(AudioCallback);
    for(;;)
    {
        hw.midi.Listen();
        // Handle MIDI Events
        while(hw.midi.HasEvents())
        {
            HandleMidiMessage(hw.midi.PopEvent());
        }
    }
}

Anyone? Maybe someone from the development team? This basically renders the Midi note in useless with the current library. — or is that a bug with my unit?

Saw @brbrr and @ben_serge respond here: MIDI issues in Daisy Seed since last libDaisy updates a while back. Maybe you guys have some advice?

Btw: USB midi works correctly but would rather avoid using that.