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.
This commit is contained in:
Eric Sherman 2021-08-05 20:38:25 -04:00
parent d501f4bb3d
commit 00206071e6
No known key found for this signature in database
GPG Key ID: 5FF6E54D61748174
6 changed files with 104 additions and 176 deletions

View File

@ -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

View File

@ -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;

View File

@ -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,8 +91,8 @@ void MidiInterface<Transport, Settings, Platform>::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<Transport, Settings, Platform>::read(Channel inChannel
if (channelMatch)
launchCallback();
thruFilter(inChannel);
thruFilter();
return channelMatch;
}
@ -1343,42 +1341,22 @@ 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 void MidiInterface<Transport, Settings, Platform>::setThruFilterMode(Thru::Mode inThruFilterMode)
{
mThruFilterMode = inThruFilterMode;
mThruActivated = mThruFilterMode != Thru::Off;
}
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 void MidiInterface<Transport, Settings, Platform>::turnThruOn(Thru::Mode inThruFilterMode)
inline void MidiInterface<Transport, Settings, Platform>::turnThruOn(ThruFilterCallback fptr)
{
mThruActivated = true;
mThruFilterMode = inThruFilterMode;
mThruFilterCallback = fptr;
}
template<class Transport, class Settings, class Platform>
inline void MidiInterface<Transport, Settings, Platform>::turnThruOff()
{
mThruActivated = false;
mThruFilterMode = Thru::Off;
mThruFilterCallback = thruOff;
}
template<class Transport, class Settings, class Platform>
@ -1397,51 +1375,20 @@ inline void MidiInterface<Transport, Settings, Platform>::UpdateLastSentTime()
// - 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 (!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<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:

View File

@ -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

View File

@ -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

View File

@ -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;
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