From 00206071e639dcd7c304aa0bfd133a368f413121 Mon Sep 17 00:00:00 2001 From: Eric Sherman Date: Thu, 5 Aug 2021 20:38:25 -0400 Subject: [PATCH] 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 | 2 - src/MIDI.h | 17 +- src/MIDI.hpp | 93 +++-------- src/midi_Defs.h | 15 -- test/unit-tests/CMakeLists.txt | 2 + test/unit-tests/tests/unit-tests_MidiThru.cpp | 151 ++++++++---------- 6 files changed, 104 insertions(+), 176 deletions(-) diff --git a/keywords.txt b/keywords.txt index cfb39f4..aa3673b 100644 --- a/keywords.txt +++ b/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 diff --git a/src/MIDI.h b/src/MIDI.h index d15888f..86e2ccb 100644 --- a/src/MIDI.h +++ b/src/MIDI.h @@ -236,15 +236,22 @@ private: // MIDI Soft Thru public: - inline Thru::Mode getFilterMode() const; inline bool getThruState() const; - inline void turnThruOn(Thru::Mode inThruFilterMode = Thru::Full); + using ThruFilterCallback = bool (*)(const MidiMessage& inMessage); + using ThruMapCallback = MidiMessage (*)(const MidiMessage& inMessage); + inline void turnThruOn(ThruFilterCallback fptr = thruOn); inline void turnThruOff(); - inline void setThruFilterMode(Thru::Mode inThruFilterMode); + inline void setThruFilter(ThruFilterCallback fptr) { mThruFilterCallback = fptr; } + inline void setThruMap(ThruMapCallback fptr) { mThruMapCallback = fptr; } 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 +284,6 @@ private: unsigned mPendingMessageIndex; unsigned mCurrentRpnNumber; unsigned mCurrentNrpnNumber; - bool mThruActivated : 1; - Thru::Mode mThruFilterMode : 7; MidiMessage mMessage; unsigned long mLastMessageSentTime; unsigned long mLastMessageReceivedTime; diff --git a/src/MIDI.hpp b/src/MIDI.hpp index 144e40e..5af823a 100644 --- a/src/MIDI.hpp +++ b/src/MIDI.hpp @@ -40,8 +40,6 @@ inline MidiInterface::MidiInterface(Transport& in , mPendingMessageIndex(0) , mCurrentRpnNumber(0xffff) , mCurrentNrpnNumber(0xffff) - , mThruActivated(true) - , mThruFilterMode(Thru::Full) , mLastMessageSentTime(0) , mLastMessageReceivedTime(0) , mSenderActiveSensingPeriodicity(0) @@ -93,8 +91,8 @@ void MidiInterface::begin(Channel inChannel) mMessage.data2 = 0; mMessage.length = 0; - mThruFilterMode = Thru::Full; - mThruActivated = mTransport.thruActivated; + mThruFilterCallback = thruOn; + mThruMapCallback = thruEcho; } // ----------------------------------------------------------------------------- @@ -771,7 +769,7 @@ inline bool MidiInterface::read(Channel inChannel if (channelMatch) launchCallback(); - thruFilter(inChannel); + thruFilter(); return channelMatch; } @@ -1343,42 +1341,22 @@ void MidiInterface::launchCallback() @{ */ -/*! \brief Set the filter for thru mirroring - \param inThruFilterMode a filter mode - - @see Thru::Mode - */ -template -inline void MidiInterface::setThruFilterMode(Thru::Mode inThruFilterMode) -{ - mThruFilterMode = inThruFilterMode; - mThruActivated = mThruFilterMode != Thru::Off; -} - -template -inline Thru::Mode MidiInterface::getFilterMode() const -{ - return mThruFilterMode; -} - template inline bool MidiInterface::getThruState() const { - return mThruActivated; + return mThruFilterCallback != thruOff; } template -inline void MidiInterface::turnThruOn(Thru::Mode inThruFilterMode) +inline void MidiInterface::turnThruOn(ThruFilterCallback fptr) { - mThruActivated = true; - mThruFilterMode = inThruFilterMode; + mThruFilterCallback = fptr; } template inline void MidiInterface::turnThruOff() { - mThruActivated = false; - mThruFilterMode = Thru::Off; + mThruFilterCallback = thruOff; } template @@ -1397,51 +1375,20 @@ inline void MidiInterface::UpdateLastSentTime() // - Channel messages are passed to the output whether their channel // is matching the input channel and the filter setting template -void MidiInterface::thruFilter(Channel inChannel) +void MidiInterface::thruFilter() { - // If the feature is disabled, don't do anything. - if (!mThruActivated || (mThruFilterMode == Thru::Off)) - return; + if (!mThruFilterCallback(mMessage)) + 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) || - (inChannel == MIDI_CHANNEL_OMNI)); - - // Now let's pass it to the output - 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; - } + send(thruMessage.type, + thruMessage.data1, + thruMessage.data2, + thruMessage.channel); } else { @@ -1461,19 +1408,19 @@ void MidiInterface::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: diff --git a/src/midi_Defs.h b/src/midi_Defs.h index ef74621..eb3d236 100644 --- a/src/midi_Defs.h +++ b/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 diff --git a/test/unit-tests/CMakeLists.txt b/test/unit-tests/CMakeLists.txt index eaf1f3f..d98ac8e 100644 --- a/test/unit-tests/CMakeLists.txt +++ b/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 diff --git a/test/unit-tests/tests/unit-tests_MidiThru.cpp b/test/unit-tests/tests/unit-tests_MidiThru.cpp index dc0c0c1..27319a7 100644 --- a/test/unit-tests/tests/unit-tests_MidiThru.cpp +++ b/test/unit-tests/tests/unit-tests_MidiThru.cpp @@ -17,6 +17,7 @@ typedef test_mocks::SerialMock<32> SerialMock; typedef midi::SerialMIDI Transport; typedef midi::MidiInterface MidiInterface; typedef std::vector Buffer; +typedef midi::Message MidiMessage; template 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 MultiByteParsing; typedef midi::MidiInterface 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 Settings; typedef midi::MidiInterface 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; - Transport transport(serial); - MidiInterface midi((Transport&)transport); + Buffer buffer; 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(midi.read(), false); - 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({ + 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