Skip to content

Commit

Permalink
MidiController: rename confusingly overloaded "receive" method
Browse files Browse the repository at this point in the history
  • Loading branch information
Be-ing committed Jul 12, 2020
1 parent 8153b3f commit 9363b49
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 75 deletions.
6 changes: 4 additions & 2 deletions src/controllers/midi/hss1394controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,10 @@ int Hss1394Controller::open() {
m_pChannelListener = new DeviceChannelListener(this, getName());
connect(m_pChannelListener, SIGNAL(incomingData(QByteArray, mixxx::Duration)),
this, SLOT(receive(QByteArray, mixxx::Duration)));
connect(m_pChannelListener, SIGNAL(incomingData(unsigned char, unsigned char, unsigned char, mixxx::Duration)),
this, SLOT(receive(unsigned char, unsigned char, unsigned char, mixxx::Duration)));
connect(m_pChannelListener,
&DeviceChannelListener::incomingData,
this,
&Hss1394Controller::receiveShortMessage);

if (!m_pChannel->InstallChannelListener(m_pChannelListener)) {
qDebug() << "HSS1394 channel listener could not be installed for device" << getName();
Expand Down
6 changes: 4 additions & 2 deletions src/controllers/midi/midicontroller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,10 @@ void MidiController::commitTemporaryInputMappings() {
m_temporaryInputMappings.clear();
}

void MidiController::receive(unsigned char status, unsigned char control,
unsigned char value, mixxx::Duration timestamp) {
void MidiController::receiveShortMessage(unsigned char status,
unsigned char control,
unsigned char value,
mixxx::Duration timestamp) {
// The rest of this function is for legacy mappings
unsigned char channel = MidiUtils::channelFromStatus(status);
unsigned char opCode = MidiUtils::opCodeFromStatus(status);
Expand Down
6 changes: 4 additions & 2 deletions src/controllers/midi/midicontroller.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,10 @@ class MidiController : public Controller {
}

protected slots:
virtual void receive(unsigned char status, unsigned char control,
unsigned char value, mixxx::Duration timestamp);
virtual void receiveShortMessage(unsigned char status,
unsigned char control,
unsigned char value,
mixxx::Duration timestamp);
// For receiving System Exclusive messages
void receive(const QByteArray data, mixxx::Duration timestamp) override;
int close() override;
Expand Down
4 changes: 2 additions & 2 deletions src/controllers/midi/portmidicontroller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ bool PortMidiController::poll() {

if ((status & 0xF8) == 0xF8) {
// Handle real-time MIDI messages at any time
receive(status, 0, 0, timestamp);
receiveShortMessage(status, 0, 0, timestamp);
continue;
}

Expand All @@ -163,7 +163,7 @@ bool PortMidiController::poll() {
//unsigned char channel = status & 0x0F;
unsigned char note = Pm_MessageData1(m_midiBuffer[i].message);
unsigned char velocity = Pm_MessageData2(m_midiBuffer[i].message);
receive(status, note, velocity, timestamp);
receiveShortMessage(status, note, velocity, timestamp);
}
}

Expand Down
119 changes: 59 additions & 60 deletions src/test/midicontrollertest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,9 @@ class MidiControllerTest : public MixxxTest {
m_pController->visit(&preset);
}

void receive(unsigned char status, unsigned char control,
unsigned char value) {
void receiveShortMessage(unsigned char status, unsigned char control, unsigned char value) {
// TODO(rryan): This test doesn't care about timestamps.
m_pController->receive(status, control, value, mixxx::Time::elapsed());
m_pController->receiveShortMessage(status, control, value, mixxx::Time::elapsed());
}

MidiControllerPreset m_preset;
Expand All @@ -64,15 +63,15 @@ TEST_F(MidiControllerTest, ReceiveMessage_PushButtonCO_PushOnOff) {
loadPreset(m_preset);

// Receive an on/off, sets the control on/off with each press.
receive(MIDI_NOTE_ON | channel, control, 0x7F);
receiveShortMessage(MIDI_NOTE_ON | channel, control, 0x7F);
EXPECT_LT(0.0, cpb.get());
receive(MIDI_NOTE_OFF | channel, control, 0x00);
receiveShortMessage(MIDI_NOTE_OFF | channel, control, 0x00);
EXPECT_DOUBLE_EQ(0.0, cpb.get());

// Receive an on/off, sets the control on/off with each press.
receive(MIDI_NOTE_ON | channel, control, 0x7F);
receiveShortMessage(MIDI_NOTE_ON | channel, control, 0x7F);
EXPECT_LT(0.0, cpb.get());
receive(MIDI_NOTE_OFF | channel, control, 0x00);
receiveShortMessage(MIDI_NOTE_OFF | channel, control, 0x00);
EXPECT_DOUBLE_EQ(0.0, cpb.get());
}

Expand All @@ -90,15 +89,15 @@ TEST_F(MidiControllerTest, ReceiveMessage_PushButtonCO_PushOnOn) {
loadPreset(m_preset);

// Receive an on/off, sets the control on/off with each press.
receive(MIDI_NOTE_ON | channel, control, 0x7F);
receiveShortMessage(MIDI_NOTE_ON | channel, control, 0x7F);
EXPECT_LT(0.0, cpb.get());
receive(MIDI_NOTE_ON | channel, control, 0x00);
receiveShortMessage(MIDI_NOTE_ON | channel, control, 0x00);
EXPECT_DOUBLE_EQ(0.0, cpb.get());

// Receive an on/off, sets the control on/off with each press.
receive(MIDI_NOTE_ON | channel, control, 0x7F);
receiveShortMessage(MIDI_NOTE_ON | channel, control, 0x7F);
EXPECT_LT(0.0, cpb.get());
receive(MIDI_NOTE_ON | channel, control, 0x00);
receiveShortMessage(MIDI_NOTE_ON | channel, control, 0x00);
EXPECT_DOUBLE_EQ(0.0, cpb.get());
}

Expand All @@ -123,13 +122,13 @@ TEST_F(MidiControllerTest, ReceiveMessage_PushButtonCO_ToggleOnOff_ButtonMidiOpt
// NOTE(rryan): This behavior is broken!

// Toggle the switch on, sets the push button on.
receive(MIDI_NOTE_ON | channel, control, 0x7F);
receiveShortMessage(MIDI_NOTE_ON | channel, control, 0x7F);
EXPECT_LT(0.0, cpb.get());

// The push button is stuck down here!

// Toggle the switch off, sets the push button off.
receive(MIDI_NOTE_OFF | channel, control, 0x00);
receiveShortMessage(MIDI_NOTE_OFF | channel, control, 0x00);
EXPECT_DOUBLE_EQ(0.0, cpb.get());
}

Expand All @@ -154,13 +153,13 @@ TEST_F(MidiControllerTest, ReceiveMessage_PushButtonCO_ToggleOnOff_SwitchMidiOpt
// NOTE(rryan): This behavior is broken!

// Toggle the switch on, sets the push button on.
receive(MIDI_NOTE_ON | channel, control, 0x7F);
receiveShortMessage(MIDI_NOTE_ON | channel, control, 0x7F);
EXPECT_LT(0.0, cpb.get());

// The push button is stuck down here!

// Toggle the switch off, sets the push button on again.
receive(MIDI_NOTE_OFF | channel, control, 0x00);
receiveShortMessage(MIDI_NOTE_OFF | channel, control, 0x00);
EXPECT_LT(0.0, cpb.get());

// NOTE(rryan): What is supposed to happen in this case? It's an open
Expand Down Expand Up @@ -196,15 +195,15 @@ TEST_F(MidiControllerTest, ReceiveMessage_PushButtonCO_PushCC) {
loadPreset(m_preset);

// Receive an on/off, sets the control on/off with each press.
receive(MIDI_CC | channel, control, 0x7F);
receiveShortMessage(MIDI_CC | channel, control, 0x7F);
EXPECT_LT(0.0, cpb.get());
receive(MIDI_CC | channel, control, 0x00);
receiveShortMessage(MIDI_CC | channel, control, 0x00);
EXPECT_DOUBLE_EQ(0.0, cpb.get());

// Receive an on/off, sets the control on/off with each press.
receive(MIDI_CC | channel, control, 0x7F);
receiveShortMessage(MIDI_CC | channel, control, 0x7F);
EXPECT_LT(0.0, cpb.get());
receive(MIDI_CC | channel, control, 0x00);
receiveShortMessage(MIDI_CC | channel, control, 0x00);
EXPECT_DOUBLE_EQ(0.0, cpb.get());
}

Expand All @@ -225,14 +224,14 @@ TEST_F(MidiControllerTest, ReceiveMessage_ToggleCO_PushOnOff) {
loadPreset(m_preset);

// Receive an on/off, toggles the control.
receive(MIDI_NOTE_ON | channel, control, 0x7F);
receive(MIDI_NOTE_OFF | channel, control, 0x00);
receiveShortMessage(MIDI_NOTE_ON | channel, control, 0x7F);
receiveShortMessage(MIDI_NOTE_OFF | channel, control, 0x00);

EXPECT_LT(0.0, cpb.get());

// Receive an on/off, toggles the control.
receive(MIDI_NOTE_ON | channel, control, 0x7F);
receive(MIDI_NOTE_OFF | channel, control, 0x00);
receiveShortMessage(MIDI_NOTE_ON | channel, control, 0x7F);
receiveShortMessage(MIDI_NOTE_OFF | channel, control, 0x00);

EXPECT_DOUBLE_EQ(0.0, cpb.get());
}
Expand All @@ -252,14 +251,14 @@ TEST_F(MidiControllerTest, ReceiveMessage_ToggleCO_PushOnOn) {
loadPreset(m_preset);

// Receive an on/off, toggles the control.
receive(MIDI_NOTE_ON | channel, control, 0x7F);
receive(MIDI_NOTE_ON | channel, control, 0x00);
receiveShortMessage(MIDI_NOTE_ON | channel, control, 0x7F);
receiveShortMessage(MIDI_NOTE_ON | channel, control, 0x00);

EXPECT_LT(0.0, cpb.get());

// Receive an on/off, toggles the control.
receive(MIDI_NOTE_ON | channel, control, 0x7F);
receive(MIDI_NOTE_ON | channel, control, 0x00);
receiveShortMessage(MIDI_NOTE_ON | channel, control, 0x7F);
receiveShortMessage(MIDI_NOTE_ON | channel, control, 0x00);

EXPECT_DOUBLE_EQ(0.0, cpb.get());
}
Expand Down Expand Up @@ -289,12 +288,12 @@ TEST_F(MidiControllerTest, ReceiveMessage_ToggleCO_ToggleOnOff_ButtonMidiOption)

// Toggle the switch on, since it is interpreted as a button press it
// toggles the button on.
receive(MIDI_NOTE_ON | channel, control, 0x7F);
receiveShortMessage(MIDI_NOTE_ON | channel, control, 0x7F);
EXPECT_LT(0.0, cpb.get());

// Toggle the switch off, since it is interpreted as a button release it
// does nothing to the toggle button.
receive(MIDI_NOTE_OFF | channel, control, 0x00);
receiveShortMessage(MIDI_NOTE_OFF | channel, control, 0x00);
EXPECT_LT(0.0, cpb.get());
}

Expand Down Expand Up @@ -324,12 +323,12 @@ TEST_F(MidiControllerTest, ReceiveMessage_ToggleCO_ToggleOnOff_SwitchMidiOption)

// Toggle the switch on, since it is interpreted as a button press it
// toggles the control on.
receive(MIDI_NOTE_ON | channel, control, 0x7F);
receiveShortMessage(MIDI_NOTE_ON | channel, control, 0x7F);
EXPECT_LT(0.0, cpb.get());

// Toggle the switch off, since it is interpreted as a button press it
// toggles the control off.
receive(MIDI_NOTE_OFF | channel, control, 0x00);
receiveShortMessage(MIDI_NOTE_OFF | channel, control, 0x00);
EXPECT_DOUBLE_EQ(0.0, cpb.get());

// Meanwhile, the GUI toggles the control on again.
Expand All @@ -339,12 +338,12 @@ TEST_F(MidiControllerTest, ReceiveMessage_ToggleCO_ToggleOnOff_SwitchMidiOption)

// Toggle the switch on, since it is interpreted as a button press it
// toggles the control off (since it was on).
receive(MIDI_NOTE_ON | channel, control, 0x7F);
receiveShortMessage(MIDI_NOTE_ON | channel, control, 0x7F);
EXPECT_DOUBLE_EQ(0.0, cpb.get());

// Toggle the switch off, since it is interpreted as a button press it
// toggles the control on (since it was off).
receive(MIDI_NOTE_OFF | channel, control, 0x00);
receiveShortMessage(MIDI_NOTE_OFF | channel, control, 0x00);
EXPECT_LT(0.0, cpb.get());
}

Expand All @@ -363,14 +362,14 @@ TEST_F(MidiControllerTest, ReceiveMessage_ToggleCO_PushCC) {
loadPreset(m_preset);

// Receive an on/off, toggles the control.
receive(MIDI_CC | channel, control, 0x7F);
receive(MIDI_CC | channel, control, 0x00);
receiveShortMessage(MIDI_CC | channel, control, 0x7F);
receiveShortMessage(MIDI_CC | channel, control, 0x00);

EXPECT_LT(0.0, cpb.get());

// Receive an on/off, toggles the control.
receive(MIDI_CC | channel, control, 0x7F);
receive(MIDI_CC | channel, control, 0x00);
receiveShortMessage(MIDI_CC | channel, control, 0x7F);
receiveShortMessage(MIDI_CC | channel, control, 0x00);

EXPECT_DOUBLE_EQ(0.0, cpb.get());
}
Expand All @@ -391,15 +390,15 @@ TEST_F(MidiControllerTest, ReceiveMessage_PotMeterCO_7BitCC) {
loadPreset(m_preset);

// Receive a 0, MIDI parameter should map to the min value.
receive(MIDI_CC | channel, control, 0x00);
receiveShortMessage(MIDI_CC | channel, control, 0x00);
EXPECT_DOUBLE_EQ(kMinValue, potmeter.get());

// Receive a 0x7F, MIDI parameter should map to the potmeter max value.
receive(MIDI_CC | channel, control, 0x7F);
receiveShortMessage(MIDI_CC | channel, control, 0x7F);
EXPECT_DOUBLE_EQ(kMaxValue, potmeter.get());

// Receive a 0x40, MIDI parameter should map to the potmeter middle value.
receive(MIDI_CC | channel, control, 0x40);
receiveShortMessage(MIDI_CC | channel, control, 0x40);
EXPECT_DOUBLE_EQ(kMiddleValue, potmeter.get());
}

Expand Down Expand Up @@ -434,58 +433,58 @@ TEST_F(MidiControllerTest, ReceiveMessage_PotMeterCO_14BitCC) {

// Receive a 0x0000 (lsb-first), MIDI parameter should map to the min value.
potmeter.set(0);
receive(MIDI_CC | channel, lsb_control, 0x00);
receive(MIDI_CC | channel, msb_control, 0x00);
receiveShortMessage(MIDI_CC | channel, lsb_control, 0x00);
receiveShortMessage(MIDI_CC | channel, msb_control, 0x00);
EXPECT_DOUBLE_EQ(kMinValue, potmeter.get());

// Receive a 0x0000 (msb-first), MIDI parameter should map to the min value.
potmeter.set(0);
receive(MIDI_CC | channel, msb_control, 0x00);
receive(MIDI_CC | channel, lsb_control, 0x00);
receiveShortMessage(MIDI_CC | channel, msb_control, 0x00);
receiveShortMessage(MIDI_CC | channel, lsb_control, 0x00);
EXPECT_DOUBLE_EQ(kMinValue, potmeter.get());

// Receive a 0x3FFF (lsb-first), MIDI parameter should map to the max value.
potmeter.set(0);
receive(MIDI_CC | channel, lsb_control, 0x7F);
receive(MIDI_CC | channel, msb_control, 0x7F);
receiveShortMessage(MIDI_CC | channel, lsb_control, 0x7F);
receiveShortMessage(MIDI_CC | channel, msb_control, 0x7F);
EXPECT_DOUBLE_EQ(kMaxValue, potmeter.get());

// Receive a 0x3FFF (msb-first), MIDI parameter should map to the max value.
potmeter.set(0);
receive(MIDI_CC | channel, msb_control, 0x7F);
receive(MIDI_CC | channel, lsb_control, 0x7F);
receiveShortMessage(MIDI_CC | channel, msb_control, 0x7F);
receiveShortMessage(MIDI_CC | channel, lsb_control, 0x7F);
EXPECT_DOUBLE_EQ(kMaxValue, potmeter.get());

// Receive a 0x2000 (lsb-first), MIDI parameter should map to the middle
// value.
potmeter.set(0);
receive(MIDI_CC | channel, lsb_control, 0x00);
receive(MIDI_CC | channel, msb_control, 0x40);
receiveShortMessage(MIDI_CC | channel, lsb_control, 0x00);
receiveShortMessage(MIDI_CC | channel, msb_control, 0x40);
EXPECT_DOUBLE_EQ(kMiddleValue, potmeter.get());

// Receive a 0x2000 (msb-first), MIDI parameter should map to the middle
// value.
potmeter.set(0);
receive(MIDI_CC | channel, msb_control, 0x40);
receive(MIDI_CC | channel, lsb_control, 0x00);
receiveShortMessage(MIDI_CC | channel, msb_control, 0x40);
receiveShortMessage(MIDI_CC | channel, lsb_control, 0x00);
EXPECT_DOUBLE_EQ(kMiddleValue, potmeter.get());

// Check the 14-bit resolution is actually present. Receive a 0x2001
// (msb-first), MIDI parameter should map to the middle value plus a tiny
// amount. Scaling is not quite linear for MIDI parameters so just check
// that incrementing the LSB by 1 is greater than the middle value.
potmeter.set(0);
receive(MIDI_CC | channel, msb_control, 0x40);
receive(MIDI_CC | channel, lsb_control, 0x01);
receiveShortMessage(MIDI_CC | channel, msb_control, 0x40);
receiveShortMessage(MIDI_CC | channel, lsb_control, 0x01);
EXPECT_LT(kMiddleValue, potmeter.get());

// Check the 14-bit resolution is actually present. Receive a 0x2001
// (lsb-first), MIDI parameter should map to the middle value plus a tiny
// amount. Scaling is not quite linear for MIDI parameters so just check
// that incrementing the LSB by 1 is greater than the middle value.
potmeter.set(0);
receive(MIDI_CC | channel, lsb_control, 0x01);
receive(MIDI_CC | channel, msb_control, 0x40);
receiveShortMessage(MIDI_CC | channel, lsb_control, 0x01);
receiveShortMessage(MIDI_CC | channel, msb_control, 0x40);
EXPECT_LT(kMiddleValue, potmeter.get());
}

Expand All @@ -505,21 +504,21 @@ TEST_F(MidiControllerTest, ReceiveMessage_PotMeterCO_14BitPitchBend) {
loadPreset(m_preset);

// Receive a 0x0000, MIDI parameter should map to the min value.
receive(MIDI_PITCH_BEND | channel, 0x00, 0x00);
receiveShortMessage(MIDI_PITCH_BEND | channel, 0x00, 0x00);
EXPECT_DOUBLE_EQ(kMinValue, potmeter.get());

// Receive a 0x3FFF, MIDI parameter should map to the potmeter max value.
receive(MIDI_PITCH_BEND | channel, 0x7F, 0x7F);
receiveShortMessage(MIDI_PITCH_BEND | channel, 0x7F, 0x7F);
EXPECT_DOUBLE_EQ(kMaxValue, potmeter.get());

// Receive a 0x2000, MIDI parameter should map to the potmeter middle value.
receive(MIDI_PITCH_BEND | channel, 0x00, 0x40);
receiveShortMessage(MIDI_PITCH_BEND | channel, 0x00, 0x40);
EXPECT_DOUBLE_EQ(kMiddleValue, potmeter.get());

// Check the 14-bit resolution is actually present. Receive a 0x2001, MIDI
// parameter should map to the middle value plus a tiny amount. Scaling is
// not quite linear for MIDI parameters so just check that incrementing the
// LSB by 1 is greater than the middle value.
receive(MIDI_PITCH_BEND | channel, 0x01, 0x40);
receiveShortMessage(MIDI_PITCH_BEND | channel, 0x01, 0x40);
EXPECT_LT(kMiddleValue, potmeter.get());
}
Loading

0 comments on commit 9363b49

Please sign in to comment.