[Proposal] add interface for callback methods

[joshka]

I had difficulty creating a new class that will listen to MIDI callbacks as C++ doesn't support casting class instance method function pointers to (void*) function pointers .

I propose adding an observer and refactoring the built in to call this.

class MidiObserver
{
public:
    virtual ~MidiCallback() {}
    virtual void handleNoteOff(byte channel, byte note, byte velocity) {}
    virtual void handleNoteOn(byte channel, byte note, byte velocity) {}
    virtual void handleAfterTouchPoly(byte channel, byte note, byte pressure) {}
    virtual void handleControlChange(byte channel, byte number, byte value) {}
    virtual void handleProgramChange(byte channel, byte number) {}
    virtual void handleAfterTouchChannel(byte channel, byte pressure) {}
    virtual void handlePitchBend(byte channel, int bend) {}
    virtual void handleSystemExclusive(byte *array, unsigned size) {}
    virtual void handleTimeCodeQuarterFrame(byte data) {}
    virtual void handleSongPosition(unsigned beats) {}
    virtual void handleSongSelect(byte songnumber) {}
    virtual void handleTuneRequest() {}
    virtual void handleClock() {}
    virtual void handleStart() {}
    virtual void handleContinue() {}
    virtual void handleStop() {}
    virtual void handleActiveSensing() {}
    virtual void handleSystemReset() {}
}

...

MidiInterface::attachObserver(MidiObserver observer)
{
    // note, could easily be an array of observers instead of a single observer
    _observer = observer;
}

...

void MidiInterface<SerialPort, Settings>::launchCallback()
{
    // The order is mixed to allow frequent messages to trigger their callback faster.
    switch (mMessage.type)
    {
            // Notes
        case NoteOff: if (observer != 0) observer->handleNoteOff(mMessage.channel, mMessage.data1, mMessage.data2);   break;
        case NoteOn:  if (observer != 0) observer->handleNoteOn(mMessage.channel, mMessage.data1, mMessage.data2);    break;
        ...
    }
}

// to support the existing setHandleXXX() methods
class LegacyCallback
{
public:
    inline void setHandleNoteOff(void (*fptr)(byte channel, byte note, byte velocity));
    inline void setHandleNoteOn(void (*fptr)(byte channel, byte note, byte velocity));
    inline void setHandleAfterTouchPoly(void (*fptr)(byte channel, byte note, byte pressure));
    inline void setHandleControlChange(void (*fptr)(byte channel, byte number, byte value));
    inline void setHandleProgramChange(void (*fptr)(byte channel, byte number));
    inline void setHandleAfterTouchChannel(void (*fptr)(byte channel, byte pressure));
    inline void setHandlePitchBend(void (*fptr)(byte channel, int bend));
    inline void setHandleSystemExclusive(void (*fptr)(byte * array, unsigned size));
    inline void setHandleTimeCodeQuarterFrame(void (*fptr)(byte data));
    inline void setHandleSongPosition(void (*fptr)(unsigned beats));
    inline void setHandleSongSelect(void (*fptr)(byte songnumber));
    inline void setHandleTuneRequest(void (*fptr)(void));
    inline void setHandleClock(void (*fptr)(void));
    inline void setHandleStart(void (*fptr)(void));
    inline void setHandleContinue(void (*fptr)(void));
    inline void setHandleStop(void (*fptr)(void));
    inline void setHandleActiveSensing(void (*fptr)(void));
    inline void setHandleSystemReset(void (*fptr)(void));

private:
    void (*mNoteOffCallback)(byte channel, byte note, byte velocity);
    void (*mNoteOnCallback)(byte channel, byte note, byte velocity);
    void (*mAfterTouchPolyCallback)(byte channel, byte note, byte velocity);
    void (*mControlChangeCallback)(byte channel, byte, byte);
    void (*mProgramChangeCallback)(byte channel, byte);
    void (*mAfterTouchChannelCallback)(byte channel, byte);
    void (*mPitchBendCallback)(byte channel, int);
    void (*mSystemExclusiveCallback)(byte * array, unsigned size);
    void (*mTimeCodeQuarterFrameCallback)(byte data);
    void (*mSongPositionCallback)(unsigned beats);
    void (*mSongSelectCallback)(byte songnumber);
    void (*mTuneRequestCallback)(void);
    void (*mClockCallback)(void);
    void (*mStartCallback)(void);
    void (*mContinueCallback)(void);
    void (*mStopCallback)(void);
    void (*mActiveSensingCallback)(void);
    void (*mSystemResetCallback)(void);

public:
    void handleNoteOff(byte channel, byte note, byte velocity) { if (mNoteOffCallback != 0) mNoteOffCallback(channel, byte, velocity); }
    ...
}

This will allow something like e.g.:

class Arp : public MidiObserver
{
public:
    void handleStart();
    void handleStop();
    void handleClock();
    void handleNoteOn(byte channel, byte note, byte velocity);
    void handleNoteOff(byte channel, byte note, byte velocity);
}

class Seq : public MidiObserver 
{
public:
    void handleStart();
    void handleStop();
    void handleContinue();
    void handleClock();
    void handleNoteOn(byte channel, byte note, byte velocity);
    void handleNoteOff(byte channel, byte note, byte velocity);
}

Arp arp;
Seq seq;

void setup()
{
    MIDI.begin();
    arp.begin();
    seq.begin();
    MIDI.attachObserver(arp);
}

void loop()
{
  switch (mode)
  {
    case Mode::arpeggiator: MIDI.attachObserver(arp); break;
    case Mode::sequencer: MIDI.attachObserver(seq); break;
  }
  MIDI.read();
}

[franky47]

One way to attach callbacks using the existing system is to use static class methods:

#include <MIDI.h>

MIDI_CREATE_DEFAULT_INSTANCE();

class Arpeggiator
{
public:
    static void handleNoteOn(byte inChannel, byte inNote, byte inVelocity);

public:
    inline void attachCallbacks()
    {
        MIDI.setHandleNoteOn(Arpeggiator::handleNoteOn);
    }

private:
    inline void process()
    {
    }
};

class Sequencer
{
public:
    static void handleNoteOn(byte inChannel, byte inNote, byte inVelocity);

public:
    inline void attachCallbacks()
    {
        MIDI.setHandleNoteOn(Sequencer::handleNoteOn);
    }

private:
    void process()
    { 
    }
};

// --

Arpeggiator arp;
Sequencer seq;

void Arpeggiator::handleNoteOn(byte inChannel, byte inNote, byte inVelocity)
{
    // You can call private members of your global instance here
    arp.process();
}

void Sequencer::handleNoteOn(byte inChannel, byte inNote, byte inVelocity)
{
    // You can call private members of your global instance here
    seq.process();
}

// --

void setup()
{
    MIDI.begin();
    arp.attachCallbacks();
}

void loop()
{
    MIDI.read();
}

For the sake of simplicity, I'd like to keep function pointers, as not every user is familiar or comfortable with the concept of classes, virtual inheritance and other C++ features in the Arduino community, even though your implementation is solid on an architectural point of view.

[joshka]

The static method approach only works for a single instance of each class. Not multiple. Hence, I can’t have multiple sequencers that handle different channels, instead I have to have a single sequencer that handles multiple channels. I see this as adding functionality not replacing it. No change to the existing users that only know how to use function pointers. The LegacyCallback would be the default observer that enables this.

I suppose that this could be implemented as my own static callback class that hooks this up against the other observer classes as an easy workaround.