thru filter overhaul

* resolves #40 with franky47's proposed thru filter overhaul
* removes thru filter modes
* adds thru filter callback
* adds thru map callback
* old thru filter unit tests have been replicated with filter callbacks
* does not yet include documentation changes

I believe this implements the latest proposal for #40 and exercises
everything necessary in the unit tests, including the immutability of
`mMessage` after a thru map callback has modified the outgoing message.

The thru filter callbacks in the unit tests are not suitable for copying
and pasting by end-users due to the difference in the MIDI namespace
when setup via the unit tests vs via `MIDI_CREATE_DEFAULT_INSTANCE()`.

If the changes here are deemed suitable, I'll work on documentation.

keywords.txt

@@ -55,14 +55,12 @@ getData1	KEYWORD2
 getData2	KEYWORD2
 getSysExArray	KEYWORD2
 getSysExArrayLength	KEYWORD2
-getFilterMode	KEYWORD2
 getThruState	KEYWORD2
 getInputChannel	KEYWORD2
 check	KEYWORD2
 setInputChannel	KEYWORD2
 turnThruOn	KEYWORD2
 turnThruOff	KEYWORD2
-setThruFilterMode	KEYWORD2
 disconnectCallbackFromType	KEYWORD2
 setHandleNoteOff	KEYWORD2
 setHandleNoteOn	KEYWORD2

src/MIDI.h

@@ -236,15 +236,30 @@ private:
     // MIDI Soft Thru
 
 public:
-    inline Thru::Mode getFilterMode() const;
     inline bool getThruState() const;
 
-    inline MidiInterface& turnThruOn(Thru::Mode inThruFilterMode = Thru::Full);
+    using ThruFilterCallback = bool (*)(const MidiMessage& inMessage);
+    using ThruMapCallback = MidiMessage (*)(const MidiMessage& inMessage);
+    inline MidiInterface& turnThruOn(ThruFilterCallback fptr = thruOn);
     inline MidiInterface& turnThruOff();
-    inline MidiInterface& setThruFilterMode(Thru::Mode inThruFilterMode);
+    inline MidiInterface& setThruFilter(ThruFilterCallback fptr)
+    {
+        mThruFilterCallback = fptr;
+        return *this;
+    }
+    inline MidiInterface& setThruMap(ThruMapCallback fptr)
+    {
+        mThruMapCallback = fptr;
+        return *this;
+    }
 
 private:
-    void thruFilter(byte inChannel);
+    void thruFilter();
+    static inline bool thruOn(const MidiMessage& inMessage) { (void)inMessage; return true; }
+    static inline bool thruOff(const MidiMessage& inMessage) { (void)inMessage; return false; }
+    static inline MidiMessage thruEcho(const MidiMessage& inMessage) { return inMessage; }
+    ThruFilterCallback mThruFilterCallback;
+    ThruMapCallback mThruMapCallback;
 
     // -------------------------------------------------------------------------
     // MIDI Parsing
@@ -277,8 +292,6 @@ private:
     unsigned        mPendingMessageIndex;
     unsigned        mCurrentRpnNumber;
     unsigned        mCurrentNrpnNumber;
-    bool            mThruActivated  : 1;
-    Thru::Mode      mThruFilterMode : 7;
     MidiMessage     mMessage;
     unsigned long   mLastMessageSentTime;
     unsigned long   mLastMessageReceivedTime;

src/MIDI.hpp

@@ -40,8 +40,6 @@ inline MidiInterface<Transport, Settings, Platform>::MidiInterface(Transport& in
     , mPendingMessageIndex(0)
     , mCurrentRpnNumber(0xffff)
     , mCurrentNrpnNumber(0xffff)
-    , mThruActivated(true)
-    , mThruFilterMode(Thru::Full)
     , mLastMessageSentTime(0)
     , mLastMessageReceivedTime(0)
     , mSenderActiveSensingPeriodicity(0)
@@ -93,9 +91,8 @@ MidiInterface<Transport, Settings, Platform>& MidiInterface<Transport, Settings,
     mMessage.data2   = 0;
     mMessage.length  = 0;
 
-    mThruFilterMode = Thru::Full;
+    mThruFilterCallback = thruOn;
-    mThruActivated  = mTransport.thruActivated;
+    mThruMapCallback = thruEcho;
-
     return *this;
 }
 
@@ -819,7 +816,7 @@ inline bool MidiInterface<Transport, Settings, Platform>::read(Channel inChannel
     if (channelMatch)
         launchCallback();
 
-    thruFilter(inChannel);
+    thruFilter();
 
     return channelMatch;
 }
@@ -1399,51 +1396,26 @@ void MidiInterface<Transport, Settings, Platform>::launchCallback()
  @{
  */
 
-/*! \brief Set the filter for thru mirroring
- \param inThruFilterMode a filter mode
-
- @see Thru::Mode
- */
-template<class Transport, class Settings, class Platform>
-inline MidiInterface<Transport, Settings, Platform>& MidiInterface<Transport, Settings, Platform>::setThruFilterMode(Thru::Mode inThruFilterMode)
-{
-    mThruFilterMode = inThruFilterMode;
-    mThruActivated  = mThruFilterMode != Thru::Off;
-
-    return *this;
-}
-
-template<class Transport, class Settings, class Platform>
-inline Thru::Mode MidiInterface<Transport, Settings, Platform>::getFilterMode() const
-{
-    return mThruFilterMode;
-}
-
 template<class Transport, class Settings, class Platform>
 inline bool MidiInterface<Transport, Settings, Platform>::getThruState() const
 {
-    return mThruActivated;
+    return mThruFilterCallback != thruOff;
 }
 
 template<class Transport, class Settings, class Platform>
-inline MidiInterface<Transport, Settings, Platform>& MidiInterface<Transport, Settings, Platform>::turnThruOn(Thru::Mode inThruFilterMode)
+inline MidiInterface<Transport, Settings, Platform>& MidiInterface<Transport, Settings, Platform>::turnThruOn(ThruFilterCallback fptr)
 {
-    mThruActivated = true;
+    mThruFilterCallback = fptr;
-    mThruFilterMode = inThruFilterMode;
-
     return *this;
 }
 
 template<class Transport, class Settings, class Platform>
 inline MidiInterface<Transport, Settings, Platform>& MidiInterface<Transport, Settings, Platform>::turnThruOff()
 {
-    mThruActivated = false;
+    mThruFilterCallback = thruOff;
-    mThruFilterMode = Thru::Off;
-
     return *this;
 }
 
-
 /*! @} */ // End of doc group MIDI Thru
 
 // This method is called upon reception of a message
@@ -1453,51 +1425,20 @@ inline MidiInterface<Transport, Settings, Platform>& MidiInterface<Transport, Se
 // - Channel messages are passed to the output whether their channel
 //   is matching the input channel and the filter setting
 template<class Transport, class Settings, class Platform>
-void MidiInterface<Transport, Settings, Platform>::thruFilter(Channel inChannel)
+void MidiInterface<Transport, Settings, Platform>::thruFilter()
 {
-    // If the feature is disabled, don't do anything.
+   if (!mThruFilterCallback(mMessage))
-    if (!mThruActivated || (mThruFilterMode == Thru::Off))
+      return;
-        return;
+
+   MidiMessage thruMessage = mThruMapCallback(mMessage);
 
     // First, check if the received message is Channel
     if (mMessage.type >= NoteOff && mMessage.type <= PitchBend)
     {
-        const bool filter_condition = ((mMessage.channel == inChannel) ||
+       send(thruMessage.type,
-                                       (inChannel == MIDI_CHANNEL_OMNI));
+            thruMessage.data1,
-
+            thruMessage.data2,
-        // Now let's pass it to the output
+            thruMessage.channel);
-        switch (mThruFilterMode)
-        {
-            case Thru::Full:
-                send(mMessage.type,
-                     mMessage.data1,
-                     mMessage.data2,
-                     mMessage.channel);
-                break;
-
-            case Thru::SameChannel:
-                if (filter_condition)
-                {
-                    send(mMessage.type,
-                         mMessage.data1,
-                         mMessage.data2,
-                         mMessage.channel);
-                }
-                break;
-
-            case Thru::DifferentChannel:
-                if (!filter_condition)
-                {
-                    send(mMessage.type,
-                         mMessage.data1,
-                         mMessage.data2,
-                         mMessage.channel);
-                }
-                break;
-
-            default:
-                break;
-        }
     }
     else
     {
@@ -1517,19 +1458,19 @@ void MidiInterface<Transport, Settings, Platform>::thruFilter(Channel inChannel)
 
             case SystemExclusive:
                 // Send SysEx (0xf0 and 0xf7 are included in the buffer)
-                sendSysEx(getSysExArrayLength(), getSysExArray(), true);
+                sendSysEx(thruMessage.getSysExSize(), thruMessage.sysexArray, true);
                 break;
 
             case SongSelect:
-                sendSongSelect(mMessage.data1);
+                sendSongSelect(thruMessage.data1);
                 break;
 
             case SongPosition:
-                sendSongPosition(mMessage.data1 | ((unsigned)mMessage.data2 << 7));
+                sendSongPosition(thruMessage.data1 | ((unsigned)thruMessage.data2 << 7));
                 break;
 
             case TimeCodeQuarterFrame:
-                sendTimeCodeQuarterFrame(mMessage.data1,mMessage.data2);
+                sendTimeCodeQuarterFrame(thruMessage.data1,thruMessage.data2);
                 break;
 
             default:

src/midi_Defs.h

@@ -56,7 +56,6 @@ static const uint16_t ActiveSensingTimeout = 300;
 typedef byte StatusByte;
 typedef byte DataByte;
 typedef byte Channel;
-typedef byte FilterMode;
 
 // -----------------------------------------------------------------------------
 // Errors
@@ -123,20 +122,6 @@ enum MidiType: uint8_t
 
 // -----------------------------------------------------------------------------
 
-/*! Enumeration of Thru filter modes */
-struct Thru
-{
-    enum Mode
-    {
-        Off                   = 0,  ///< Thru disabled (nothing passes through).
-        Full                  = 1,  ///< Fully enabled Thru (every incoming message is sent back).
-        SameChannel           = 2,  ///< Only the messages on the Input Channel will be sent back.
-        DifferentChannel      = 3,  ///< All the messages but the ones on the Input Channel will be sent back.
-    };
-};
-
-// -----------------------------------------------------------------------------
-
 /*! \brief Enumeration of Control Change command numbers.
  See the detailed controllers numbers & description here:
  http://www.somascape.org/midi/tech/spec.html#ctrlnums

test/unit-tests/CMakeLists.txt

@@ -23,6 +23,8 @@ add_executable(unit-tests
     tests/unit-tests_MidiThru.cpp
 )
 
+set_source_files_properties(tests/unit-tests_MidiThru.cpp PROPERTIES COMPILE_FLAGS -Wno-shadow)
+
 target_link_libraries(unit-tests
     gtest
     gmock

test/unit-tests/tests/unit-tests_MidiThru.cpp

@@ -17,6 +17,7 @@ typedef test_mocks::SerialMock<32> SerialMock;
 typedef midi::SerialMIDI<SerialMock> Transport;
 typedef midi::MidiInterface<Transport> MidiInterface;
 typedef std::vector<byte> Buffer;
+typedef midi::Message<midi::DefaultSettings::SysExMaxSize> MidiMessage;
 
 template<unsigned Size>
 struct VariableSysExSettings : midi::DefaultSettings
@@ -24,75 +25,66 @@ struct VariableSysExSettings : midi::DefaultSettings
     static const unsigned SysExMaxSize = Size;
 };
 
+SerialMock serial;
+Transport transport(serial);
+MidiInterface midi((Transport&)transport);
+
+bool thruFilterSameChannel(const MidiMessage& inMessage)
+{
+    if (!midi.isChannelMessage(inMessage.type))
+        return true;
+
+    return MIDI_CHANNEL_OMNI == midi.getInputChannel() ||
+           inMessage.channel == midi.getInputChannel();
+}
+
+bool thruFilterDifferentChannel(const MidiMessage& inMessage)
+{
+    if (!midi.isChannelMessage(inMessage.type))
+        return true;
+
+    return MIDI_CHANNEL_OMNI != midi.getInputChannel() &&
+           inMessage.channel != midi.getInputChannel();
+}
+
+MidiMessage thruMapNoteOnFullVelocity(const MidiMessage& inMessage)
+{
+    if (inMessage.type != midi::MidiType::NoteOn)
+        return inMessage;
+
+    MidiMessage modified = inMessage;
+    modified.data2 = 127;
+    return modified;
+}
+
 // -----------------------------------------------------------------------------
 
 TEST(MidiThru, defaultValues)
 {
-    SerialMock serial;
-    Transport transport(serial);
-    MidiInterface midi((Transport&)transport);
-
     EXPECT_EQ(midi.getThruState(),  true);
-    EXPECT_EQ(midi.getFilterMode(), midi::Thru::Full);
     midi.begin(); // Should not change the state
     EXPECT_EQ(midi.getThruState(),  true);
-    EXPECT_EQ(midi.getFilterMode(), midi::Thru::Full);
 }
 
 TEST(MidiThru, beginEnablesThru)
 {
-    SerialMock serial;
-    Transport transport(serial);
-    MidiInterface midi((Transport&)transport);
-
     midi.turnThruOff();
     EXPECT_EQ(midi.getThruState(),  false);
-    EXPECT_EQ(midi.getFilterMode(), midi::Thru::Off);
     midi.begin();
     EXPECT_EQ(midi.getThruState(),  true);
-    EXPECT_EQ(midi.getFilterMode(), midi::Thru::Full);
 }
 
 TEST(MidiThru, setGet)
 {
-    SerialMock serial;
-    Transport transport(serial);
-    MidiInterface midi((Transport&)transport);
-
     midi.turnThruOff();
     EXPECT_EQ(midi.getThruState(),  false);
-    EXPECT_EQ(midi.getFilterMode(), midi::Thru::Off);
 
     midi.turnThruOn();
     EXPECT_EQ(midi.getThruState(),  true);
-    EXPECT_EQ(midi.getFilterMode(), midi::Thru::Full);
-    midi.turnThruOn(midi::Thru::SameChannel);
-    EXPECT_EQ(midi.getThruState(),  true);
-    EXPECT_EQ(midi.getFilterMode(), midi::Thru::SameChannel);
-    midi.turnThruOn(midi::Thru::DifferentChannel);
-    EXPECT_EQ(midi.getThruState(),  true);
-    EXPECT_EQ(midi.getFilterMode(), midi::Thru::DifferentChannel);
-
-    midi.setThruFilterMode(midi::Thru::Full);
-    EXPECT_EQ(midi.getThruState(),  true);
-    EXPECT_EQ(midi.getFilterMode(), midi::Thru::Full);
-    midi.setThruFilterMode(midi::Thru::SameChannel);
-    EXPECT_EQ(midi.getThruState(),  true);
-    EXPECT_EQ(midi.getFilterMode(), midi::Thru::SameChannel);
-    midi.setThruFilterMode(midi::Thru::DifferentChannel);
-    EXPECT_EQ(midi.getThruState(),  true);
-    EXPECT_EQ(midi.getFilterMode(), midi::Thru::DifferentChannel);
-    midi.setThruFilterMode(midi::Thru::Off);
-    EXPECT_EQ(midi.getThruState(),  false);
-    EXPECT_EQ(midi.getFilterMode(), midi::Thru::Off);
 }
 
 TEST(MidiThru, off)
 {
-    SerialMock serial;
-    Transport transport(serial);
-    MidiInterface midi((Transport&)transport);
-
     midi.begin(MIDI_CHANNEL_OMNI);
     midi.turnThruOff();
 
@@ -110,14 +102,9 @@ TEST(MidiThru, off)
 
 TEST(MidiThru, full)
 {
-    SerialMock serial;
-    Transport transport(serial);
-    MidiInterface midi((Transport&)transport);
-
     Buffer buffer;
 
     midi.begin(MIDI_CHANNEL_OMNI);
-    midi.setThruFilterMode(midi::Thru::Full);
 
     static const unsigned rxSize = 6;
     static const byte rxData[rxSize] = { 0x9b, 12, 34, 0x9c, 56, 78 };
@@ -154,14 +141,10 @@ TEST(MidiThru, full)
 
 TEST(MidiThru, sameChannel)
 {
-    SerialMock serial;
-    Transport transport(serial);
-    MidiInterface midi((Transport&)transport);
-
     Buffer buffer;
 
     midi.begin(12);
-    midi.setThruFilterMode(midi::Thru::SameChannel);
+    midi.setThruFilter(thruFilterSameChannel);
 
     static const unsigned rxSize = 6;
     static const byte rxData[rxSize] = { 0x9b, 12, 34, 0x9c, 56, 78 };
@@ -185,14 +168,10 @@ TEST(MidiThru, sameChannel)
 
 TEST(MidiThru, sameChannelOmni) // Acts like full
 {
-    SerialMock serial;
-    Transport transport(serial);
-    MidiInterface midi((Transport&)transport);
-
     Buffer buffer;
 
     midi.begin(MIDI_CHANNEL_OMNI);
-    midi.setThruFilterMode(midi::Thru::SameChannel);
+    midi.setThruFilter(thruFilterSameChannel);
 
     static const unsigned rxSize = 6;
     static const byte rxData[rxSize] = { 0x9b, 12, 34, 0x9c, 56, 78 };
@@ -229,14 +208,10 @@ TEST(MidiThru, sameChannelOmni) // Acts like full
 
 TEST(MidiThru, differentChannel)
 {
-    SerialMock serial;
-    Transport transport(serial);
-    MidiInterface midi((Transport&)transport);
-
     Buffer buffer;
 
     midi.begin(12);
-    midi.setThruFilterMode(midi::Thru::DifferentChannel);
+    midi.setThruFilter(thruFilterDifferentChannel);
 
     static const unsigned rxSize = 6;
     static const byte rxData[rxSize] = { 0x9b, 12, 34, 0x9c, 56, 78 };
@@ -260,14 +235,10 @@ TEST(MidiThru, differentChannel)
 
 TEST(MidiThru, differentChannelOmni) // Acts like off
 {
-    SerialMock serial;
-    Transport transport(serial);
-    MidiInterface midi((Transport&)transport);
-
     Buffer buffer;
 
     midi.begin(MIDI_CHANNEL_OMNI);
-    midi.setThruFilterMode(midi::Thru::DifferentChannel);
+    midi.setThruFilter(thruFilterDifferentChannel);
 
     static const unsigned rxSize = 6;
     static const byte rxData[rxSize] = { 0x9b, 12, 34, 0x9c, 56, 78 };
@@ -293,14 +264,11 @@ TEST(MidiThru, multiByteThru)
     typedef VariableSettings<false, false> MultiByteParsing;
     typedef midi::MidiInterface<Transport, MultiByteParsing> MultiByteMidiInterface;
 
-    SerialMock serial;
-    Transport transport(serial);
     MultiByteMidiInterface midi((Transport&)transport);
     
     Buffer buffer;
 
     midi.begin(MIDI_CHANNEL_OMNI);
-    midi.setThruFilterMode(midi::Thru::Full);
 
     static const unsigned rxSize = 6;
     static const byte rxData[rxSize] = { 0x9b, 12, 34, 56, 78 };
@@ -324,14 +292,11 @@ TEST(MidiThru, withTxRunningStatus)
     typedef VariableSettings<true, true> Settings;
     typedef midi::MidiInterface<Transport, Settings> RsMidiInterface;
 
-    SerialMock serial;
-    Transport transport(serial);
     RsMidiInterface midi((Transport&)transport);
     
     Buffer buffer;
 
     midi.begin(MIDI_CHANNEL_OMNI);
-    midi.setThruFilterMode(midi::Thru::Full);
 
     static const unsigned rxSize = 5;
     static const byte rxData[rxSize] = { 0x9b, 12, 34, 56, 78 };
@@ -364,26 +329,52 @@ TEST(MidiThru, withTxRunningStatus)
     }));
 }
 
-TEST(MidiThru, invalidMode)
+TEST(MidiThru, mapNoteOnFullVelocity)
 {
-    SerialMock serial;
+    Buffer buffer;
-    Transport transport(serial);
-    MidiInterface midi((Transport&)transport);
 
     midi.begin(MIDI_CHANNEL_OMNI);
-    midi.setThruFilterMode(midi::Thru::Mode(42));
+    midi.setThruMap(thruMapNoteOnFullVelocity);
 
     static const unsigned rxSize = 6;
     static const byte rxData[rxSize] = { 0x9b, 12, 34, 0x9c, 56, 78 };
     serial.mRxBuffer.write(rxData, rxSize);
+
     EXPECT_EQ(midi.read(), false);
+    EXPECT_EQ(serial.mTxBuffer.getLength(), 0);
     EXPECT_EQ(midi.read(), false);
-    EXPECT_EQ(midi.read(), true);
+    EXPECT_EQ(serial.mTxBuffer.getLength(), 0);
-    EXPECT_EQ(midi.read(), false);
-    EXPECT_EQ(midi.read(), false);
     EXPECT_EQ(midi.read(), true);
 
+    buffer.clear();
+    buffer.resize(3);
+    EXPECT_EQ(serial.mTxBuffer.getLength(), 3);
+    serial.mTxBuffer.read(&buffer[0], 3);
+    EXPECT_THAT(buffer, ElementsAreArray({
+        0x9b, 12, 127                                  // thru message full velocity
+    }));
+    EXPECT_EQ(midi.getType(),       midi::NoteOn);
+    EXPECT_EQ(midi.getChannel(),    12);
+    EXPECT_EQ(midi.getData1(),      12);
+    EXPECT_EQ(midi.getData2(),      34);               // mMessage velocity unchanged
+
+    EXPECT_EQ(midi.read(), false);
     EXPECT_EQ(serial.mTxBuffer.getLength(), 0);
+    EXPECT_EQ(midi.read(), false);
+    EXPECT_EQ(serial.mTxBuffer.getLength(), 0);
+    EXPECT_EQ(midi.read(), true);
+
+    buffer.clear();
+    buffer.resize(3);
+    EXPECT_EQ(serial.mTxBuffer.getLength(), 3);
+    serial.mTxBuffer.read(&buffer[0], 3);
+    EXPECT_THAT(buffer, ElementsAreArray({
+        0x9c, 56, 127                                  // thru message full velocity
+    }));
+    EXPECT_EQ(midi.getType(),       midi::NoteOn);
+    EXPECT_EQ(midi.getChannel(),    13);
+    EXPECT_EQ(midi.getData1(),      56);
+    EXPECT_EQ(midi.getData2(),      78);               // mMessage velocity unchanged
 }
 
 END_UNNAMED_NAMESPACE