Skip to content

Commit

Permalink
Improve chunked SysEx exclusive access
Browse files Browse the repository at this point in the history
  • Loading branch information
tttapa committed Jan 23, 2024
1 parent 83dbfa8 commit 1f0d98b
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 24 deletions.
3 changes: 3 additions & 0 deletions src/MIDI_Interfaces/DebugMIDI_Interface.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,9 @@ class StreamDebugMIDI_Interface : public PrintDebugMIDI_Base,
private:
#if !DISABLE_PIPES
void handleStall() override { MIDI_Interface::handleStall(this); }
#ifdef DEBUG_OUT
const char *getName() const override { return "dbg"; }
#endif
#endif

private:
Expand Down
3 changes: 3 additions & 0 deletions src/MIDI_Interfaces/GenericBLEMIDI_Interface.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ class GenericBLEMIDI_Interface : public MIDI_Interface {
private:
#if !DISABLE_PIPES
void handleStall() override { MIDI_Interface::handleStall(this); }
#ifdef DEBUG_OUT
const char *getName() const override { return "ble"; }
#endif
#endif

public:
Expand Down
50 changes: 27 additions & 23 deletions src/MIDI_Interfaces/MIDI_Interface.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,10 @@ class MIDI_Interface : public TrueMIDI_SinkSource,
template <class MIDIInterface_t>
static void dispatchIncoming(MIDIInterface_t *iface, MIDIReadEvent event);
/// Un-stall the given MIDI interface. Assumes the interface has been
/// stalled because of a chunked SysEx messages. Waits untill that message
/// stalled because of a chunked SysEx messages. Waits until that message
/// is finished.
template <class MIDIInterface_t>
static void handleStall(MIDIInterface_t *iface);
static void handleStall(MIDIInterface_t *self);

private:
MIDI_Callbacks *callbacks = nullptr;
Expand All @@ -130,50 +130,52 @@ class MIDI_Interface : public TrueMIDI_SinkSource,
};

template <class MIDIInterface_t>
void MIDI_Interface::updateIncoming(MIDIInterface_t *iface) {
void MIDI_Interface::updateIncoming(MIDIInterface_t *self) {
#if DISABLE_PIPES
MIDIReadEvent event = iface->read();
MIDIReadEvent event = self->read();
while (event != MIDIReadEvent::NO_MESSAGE) {
dispatchIncoming(iface, event);
event = iface->read();
dispatchIncoming(self, event);
event = self->read();
}
#else
if (iface->getStaller() == iface)
iface->unstall(iface);
MIDIReadEvent event = self->read();
if (event == MIDIReadEvent::NO_MESSAGE)
return;
if (self->getStaller() == self)
self->unstall(self);
bool chunked = false;
MIDIReadEvent event = iface->read();
while (event != MIDIReadEvent::NO_MESSAGE) {
dispatchIncoming(iface, event);
dispatchIncoming(self, event);
if (event == MIDIReadEvent::SYSEX_CHUNK)
chunked = true;
if (event == MIDIReadEvent::SYSEX_MESSAGE)
chunked = false;
event = iface->read();
event = self->read();
}
if (chunked)
iface->stall(iface);
self->stall(self);
#endif
// TODO: add logic to detect MIDI messages such as (N)RPN that span over
// multiple channel voice messages and that shouldn't be interrupted.
// For short messages such as (N)RPN, I suggest waiting with a timeout.
}

template <class MIDIInterface_t>
void MIDI_Interface::dispatchIncoming(MIDIInterface_t *iface,
void MIDI_Interface::dispatchIncoming(MIDIInterface_t *self,
MIDIReadEvent event) {
switch (event) {
case MIDIReadEvent::CHANNEL_MESSAGE:
iface->onChannelMessage(iface->getChannelMessage());
self->onChannelMessage(self->getChannelMessage());
break;
case MIDIReadEvent::SYSEX_CHUNK: // fallthrough
case MIDIReadEvent::SYSEX_MESSAGE:
iface->onSysExMessage(iface->getSysExMessage());
self->onSysExMessage(self->getSysExMessage());
break;
case MIDIReadEvent::SYSCOMMON_MESSAGE:
iface->onSysCommonMessage(iface->getSysCommonMessage());
self->onSysCommonMessage(self->getSysCommonMessage());
break;
case MIDIReadEvent::REALTIME_MESSAGE:
iface->onRealTimeMessage(iface->getRealTimeMessage());
self->onRealTimeMessage(self->getRealTimeMessage());
break;
case MIDIReadEvent::NO_MESSAGE: break; // LCOV_EXCL_LINE
default: break; // LCOV_EXCL_LINE
Expand All @@ -182,20 +184,22 @@ void MIDI_Interface::dispatchIncoming(MIDIInterface_t *iface,

#if !DISABLE_PIPES
template <class MIDIInterface_t>
void MIDI_Interface::handleStall(MIDIInterface_t *iface) {
iface->unstall(iface);
void MIDI_Interface::handleStall(MIDIInterface_t *self) {
const char *staller_name = self->getStallerName();
DEBUGFN(F("Handling stall. Cause: ") << staller_name);
self->unstall(self);

unsigned long startTime = millis();
while (millis() - startTime < SYSEX_CHUNK_TIMEOUT) {
MIDIReadEvent event = iface->read();
dispatchIncoming(iface, event);
MIDIReadEvent event = self->read();
dispatchIncoming(self, event);
if (event == MIDIReadEvent::SYSEX_CHUNK)
startTime = millis(); // reset timeout
else if (event == MIDIReadEvent::SYSEX_MESSAGE)
return;
}
DEBUGREF(F("Warning: Unable to un-stall pipes: ")
<< iface->getStallerName());
DEBUGFN(F("Warning: Unable to un-stall pipes. Cause: ") << staller_name);
static_cast<void>(staller_name);
}
#endif

Expand Down
2 changes: 2 additions & 0 deletions src/MIDI_Interfaces/MIDI_Pipes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,11 @@ void MIDI_Source::sourceMIDItoPipe(RealTimeMessage msg) {
void MIDI_Source::stall(MIDIStaller *cause) {
if (hasSinkPipe())
sinkPipe->stallDownstream(cause, this);
DEBUGFN(F("Stalled MIDI source. Cause: ") << getStallerName());
}

void MIDI_Source::unstall(MIDIStaller *cause) {
DEBUGFN(F("Un-stalling MIDI source. Cause: ") << getStallerName());
if (hasSinkPipe())
sinkPipe->unstallDownstream(cause, this);
}
Expand Down
2 changes: 1 addition & 1 deletion src/MIDI_Interfaces/MIDI_Staller.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ BEGIN_CS_NAMESPACE
struct MIDIStaller {
virtual ~MIDIStaller() = default;
/// Get the staller's name for debugging purposes.
virtual const char *getName() const { return "<?>"; };
virtual const char *getName() const { return "<?>"; }
/// Call back that should finish any MIDI messages that are in progress, and
/// un-stall the pipe or MIDI source as quickly as possible.
virtual void handleStall() = 0;
Expand Down
3 changes: 3 additions & 0 deletions src/MIDI_Interfaces/SerialMIDI_Interface.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ class StreamMIDI_Interface : public MIDI_Interface {
protected:
#if !DISABLE_PIPES
void handleStall() override { MIDI_Interface::handleStall(this); }
#ifdef DEBUG_OUT
const char *getName() const override { return "ser"; }
#endif
#endif

protected:
Expand Down
3 changes: 3 additions & 0 deletions src/MIDI_Interfaces/USBMIDI_Interface.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ class GenericUSBMIDI_Interface : public MIDI_Interface {
private:
#if !DISABLE_PIPES
void handleStall() override { MIDI_Interface::handleStall(this); }
#ifdef DEBUG_OUT
const char *getName() const override { return "usb"; }
#endif
#endif

public:
Expand Down
3 changes: 3 additions & 0 deletions src/MIDI_Interfaces/Wrappers/FortySevenEffects.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,9 @@ class FortySevenEffectsMIDI_Interface : public MIDI_Interface {
ERROR(F("Not implemented (stalled by ") << stallername << ')', 0x1349);
(void)stallername;
}
#ifdef DEBUG_OUT
const char *getName() const override { return "47fx"; }
#endif
#endif

private:
Expand Down

0 comments on commit 1f0d98b

Please sign in to comment.