It just occurred to me that the other transports expect thru to be off by default, and I haven't tested any transport other than the baked-in serial.

I've added a commit on master that adds a copy constructor for the Message object, which will be needed when copying SysEx data (otherwise just the pointers would be copied). Can you cherry-pick it or rebase your commits onto master ?

It just occurred to me that the other transports expect thru to be off by default, and I haven't tested any transport other than the baked-in serial.

Good point, Thru makes sense only for serial transports. That being said, it was initialized as active by default. Maybe we could have transports indicate whether they support Thru, and initialize the callbacks based on a static templated property.

cc @lathoub

Cherry-picked 2ae9d9e

It just occurred to me that the other transports expect thru to be off by default, and I haven't tested any transport other than the baked-in serial.

Good point, Thru makes sense only for serial transports. That being said, it was initialized as active by default. Maybe we could have transports indicate whether they support Thru, and initialize the callbacks based on a static templated property.

cc @lathoub

I was lurking on the thread; indeed: thru filter does not make sense in point to point communication (AppleMIDI, ipMIDI, BLEMIDI, USBMIDI) and is disabled by these transports in code:

// Override default thruActivated. Must be false for all packet based messages
static const bool thruActivated = false;

and taken up here

If this code is deleted, we need to find a why for the transports to switch off Thru

Since each transport already defines their Thru preference as a static const bool, there's no need to copy it in the MidiInterface instance, it can be referenced directly as Transport::thruActivated, so both begin and processThru could use this to return early for non-Thru transports.

begin and processThru now refer to Transport::thruActivated. begin sets the initial filter callback based on its value, and processThru will exit early (prior to calling the filter callback) when false.

One thing that came clear when doing the example in 2fec41e is that it's hard to reference the midi::Message type, because of its template dependency over the SysEx array size, which is defined in the Settings:

using Message = midi::Message<midi::DefaultSettings::SysExSize>;

// Without the typedef above, this gets overly verbose:
bool filter(const Message& message)
{
    return true;
}

MIDI.setThruFilter(filter);

There is some strong coupling between the Settings traits, the MidiInterface and its underlying Message type, because of this centralisation of the static SysEx array size.

There are other open issues and discussion on how to reference types somewhere else in the sketch code, rather than just using the instance object, and I think it would be interesting to extend the macros to add type definitions.

I'm not keen on having a default value for the Message type template argument (pointing to the definition above), I believe it would cause more problems than it tries to solve with mismatching SysEx lengths when using custom settings.

Exporting the message type is definitely a clean, path-of-least-surprise solution.

The only one issue I could see is a name clash with already user-defined types with the same name, although it's very unlikely.