From 1e1a2b7c72b5cfbe474f038e3bf1c21e666ec46f Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Wed, 19 May 2021 14:24:37 +0200 Subject: [PATCH] midi: Make MidiOption a scoped enum class and use QFlags for MidiOptions --- .../controllerinputmappingtablemodel.cpp | 2 +- src/controllers/delegates/controldelegate.cpp | 2 +- .../delegates/midioptionsdelegate.cpp | 54 ++++++------ src/controllers/dlgcontrollerlearning.cpp | 42 ++++++---- src/controllers/learningutils.cpp | 10 +-- ...legacymidicontrollermappingfilehandler.cpp | 62 +++++++------- src/controllers/midi/midicontroller.cpp | 53 ++++++------ src/controllers/midi/midimessage.h | 84 +++++++------------ src/controllers/midi/midiutils.cpp | 66 +++++++-------- src/test/learningutilstest.cpp | 22 ++--- src/test/midicontrollertest.cpp | 12 +-- 11 files changed, 199 insertions(+), 210 deletions(-) diff --git a/src/controllers/controllerinputmappingtablemodel.cpp b/src/controllers/controllerinputmappingtablemodel.cpp index a4eb8e64fa5..5ae6670c0ac 100644 --- a/src/controllers/controllerinputmappingtablemodel.cpp +++ b/src/controllers/controllerinputmappingtablemodel.cpp @@ -196,7 +196,7 @@ QVariant ControllerInputMappingTableModel::data(const QModelIndex& index, case MIDI_COLUMN_OPTIONS: // UserRole is used for sorting. if (role == Qt::UserRole) { - return mapping.options.all; + return QVariant(mapping.options); } return QVariant::fromValue(mapping.options); case MIDI_COLUMN_ACTION: diff --git a/src/controllers/delegates/controldelegate.cpp b/src/controllers/delegates/controldelegate.cpp index 16e09daf5f4..fe9c971db7c 100644 --- a/src/controllers/delegates/controldelegate.cpp +++ b/src/controllers/delegates/controldelegate.cpp @@ -34,7 +34,7 @@ void ControlDelegate::paint(QPainter* painter, QModelIndex optionsColumn = index.sibling(index.row(), m_iMidiOptionsColumn); MidiOptions options = optionsColumn.data().value(); - m_bIsIndexScript = options.script; + m_bIsIndexScript = options.testFlag(MidiOption::Script); } QStyledItemDelegate::paint(painter, option, index); diff --git a/src/controllers/delegates/midioptionsdelegate.cpp b/src/controllers/delegates/midioptionsdelegate.cpp index 30a53e0ec92..8e10fa017b4 100644 --- a/src/controllers/delegates/midioptionsdelegate.cpp +++ b/src/controllers/delegates/midioptionsdelegate.cpp @@ -8,6 +8,28 @@ #include "controllers/midi/midiutils.h" #include "moc_midioptionsdelegate.cpp" +namespace { + +const QList kMidiOptions = { + MidiOption::None, + MidiOption::Invert, + MidiOption::Rot64, + MidiOption::Rot64Invert, + MidiOption::Rot64Fast, + MidiOption::Diff, + MidiOption::Button, + MidiOption::Switch, + MidiOption::Spread64, + MidiOption::HercJog, + MidiOption::SelectKnob, + MidiOption::SoftTakeover, + MidiOption::Script, + MidiOption::FourteenBitMSB, + MidiOption::FourteenBitLSB, +}; + +} + MidiOptionsDelegate::MidiOptionsDelegate(QObject* pParent) : QStyledItemDelegate(pParent) { } @@ -23,27 +45,9 @@ QWidget* MidiOptionsDelegate::createEditor(QWidget* parent, Q_UNUSED(index); QComboBox* pComboBox = new QComboBox(parent); - QList choices; - choices.append(MIDI_OPTION_NONE); - choices.append(MIDI_OPTION_INVERT); - choices.append(MIDI_OPTION_ROT64); - choices.append(MIDI_OPTION_ROT64_INV); - choices.append(MIDI_OPTION_ROT64_FAST); - choices.append(MIDI_OPTION_DIFF); - choices.append(MIDI_OPTION_BUTTON); - choices.append(MIDI_OPTION_SWITCH); - choices.append(MIDI_OPTION_SPREAD64); - choices.append(MIDI_OPTION_HERC_JOG); - choices.append(MIDI_OPTION_SELECTKNOB); - choices.append(MIDI_OPTION_SOFT_TAKEOVER); - choices.append(MIDI_OPTION_SCRIPT); - choices.append(MIDI_OPTION_14BIT_MSB); - choices.append(MIDI_OPTION_14BIT_LSB); - - for (int i = 0; i < choices.size(); ++i) { - MidiOption choice = choices.at(i); + for (const MidiOption choice : kMidiOptions) { pComboBox->addItem(MidiUtils::midiOptionToTranslatedString(choice), - choice); + static_cast(choice)); } return pComboBox; @@ -54,12 +58,10 @@ QString MidiOptionsDelegate::displayText(const QVariant& value, Q_UNUSED(locale); MidiOptions options = value.value(); QStringList optionStrs; - MidiOption option = static_cast(1); - while (option < MIDI_OPTION_MASK) { - if (options.all & option) { + for (const MidiOption option : kMidiOptions) { + if (options.testFlag(option)) { optionStrs.append(MidiUtils::midiOptionToTranslatedString(option)); } - option = static_cast(option << 1); } return optionStrs.join(", "); } @@ -73,7 +75,7 @@ void MidiOptionsDelegate::setEditorData(QWidget* editor, return; } for (int i = 0; i < pComboBox->count(); ++i) { - if (pComboBox->itemData(i).toInt() & options.all) { + if (MidiOptions(pComboBox->itemData(i).toInt()) & options) { pComboBox->setCurrentIndex(i); return; } @@ -88,6 +90,6 @@ void MidiOptionsDelegate::setModelData(QWidget* editor, if (pComboBox == nullptr) { return; } - options.all = pComboBox->itemData(pComboBox->currentIndex()).toInt(); + options = MidiOptions(pComboBox->itemData(pComboBox->currentIndex()).toInt()); model->setData(index, QVariant::fromValue(options), Qt::EditRole); } diff --git a/src/controllers/dlgcontrollerlearning.cpp b/src/controllers/dlgcontrollerlearning.cpp index 478c6882f81..e59140fae54 100644 --- a/src/controllers/dlgcontrollerlearning.cpp +++ b/src/controllers/dlgcontrollerlearning.cpp @@ -364,26 +364,32 @@ void DlgControllerLearning::slotTimerExpired() { foreach (const MidiInputMapping& mapping, m_mappings) { MidiOpCode opCode = MidiUtils::opCodeFromStatus(mapping.key.status); bool twoBytes = MidiUtils::isMessageTwoBytes(opCode); - QString mappingStr = twoBytes ? QString("Status: 0x%1 Control: 0x%2 Options: 0x%03") - .arg(QString::number(mapping.key.status, 16).toUpper(), - QString::number(mapping.key.control, 16).toUpper() - .rightJustified(2, '0'), - QString::number(mapping.options.all, 16).toUpper() - .rightJustified(2, '0')) : - QString("0x%1 0x%2") - .arg(QString::number(mapping.key.status, 16).toUpper(), - QString::number(mapping.options.all, 16).toUpper() - .rightJustified(2, '0')); + QString mappingStr = twoBytes + ? QString("Status: 0x%1 Control: 0x%2 Options: 0x%03") + .arg(QString::number(mapping.key.status, 16) + .toUpper(), + QString::number(mapping.key.control, 16) + .toUpper() + .rightJustified(2, '0'), + QString::number(mapping.options, 16) + .toUpper() + .rightJustified(2, '0')) + : QString("0x%1 0x%2") + .arg(QString::number(mapping.key.status, 16) + .toUpper(), + QString::number(mapping.options, 16) + .toUpper() + .rightJustified(2, '0')); // Set the debug string and "Advanced MIDI Options" group using the // first mapping. if (first) { midiControl = mappingStr; MidiOptions options = mapping.options; - midiOptionInvert->setChecked(options.invert); - midiOptionSelectKnob->setChecked(options.selectknob); - midiOptionSoftTakeover->setChecked(options.soft_takeover); - midiOptionSwitchMode->setChecked(options.sw); + midiOptionInvert->setChecked(options.testFlag(MidiOption::Invert)); + midiOptionSelectKnob->setChecked(options.testFlag(MidiOption::SelectKnob)); + midiOptionSoftTakeover->setChecked(options.testFlag(MidiOption::SoftTakeover)); + midiOptionSwitchMode->setChecked(options.testFlag(MidiOption::Switch)); first = false; } @@ -417,10 +423,10 @@ void DlgControllerLearning::slotMidiOptionsChanged() { for (MidiInputMappings::iterator it = m_mappings.begin(); it != m_mappings.end(); ++it) { MidiOptions& options = it->options; - options.sw = midiOptionSwitchMode->isChecked(); - options.soft_takeover = midiOptionSoftTakeover->isChecked(); - options.invert = midiOptionInvert->isChecked(); - options.selectknob = midiOptionSelectKnob->isChecked(); + options.setFlag(MidiOption::Switch, midiOptionSwitchMode->isChecked()); + options.setFlag(MidiOption::SoftTakeover, midiOptionSoftTakeover->isChecked()); + options.setFlag(MidiOption::Invert, midiOptionInvert->isChecked()); + options.setFlag(MidiOption::SelectKnob, midiOptionSelectKnob->isChecked()); } emit learnTemporaryInputMappings(m_mappings); diff --git a/src/controllers/learningutils.cpp b/src/controllers/learningutils.cpp index 4f99fcfe0d2..1719bd51db5 100644 --- a/src/controllers/learningutils.cpp +++ b/src/controllers/learningutils.cpp @@ -173,7 +173,7 @@ MidiInputMappings LearningUtils::guessMidiInputMappings( // - VCI-400 vinyl toggle button (NOTE_ON) // - Korg nanoKontrol switches (CC) MidiOptions options; - options.sw = true; + options.setFlag(MidiOption::Switch); MidiKey note_on; // The predicate ensures only NOTE_ON or CC messages can trigger this @@ -198,7 +198,7 @@ MidiInputMappings LearningUtils::guessMidiInputMappings( // though it is called 'selectknob' it is actually only two's complement // processing). MidiOptions options; - options.selectknob = true; + options.setFlag(MidiOption::SelectKnob); MidiKey knob; knob.status = MidiUtils::statusFromOpCodeAndChannel( MidiOpCode::ControlChange, *stats.channels.begin()); @@ -208,7 +208,7 @@ MidiInputMappings LearningUtils::guessMidiInputMappings( // A "spread 64" ticker, where 0x40 is zero, positive jog values are // 0x41 and above, and negative jog values are 0x3F and below. MidiOptions options; - options.spread64 = true; + options.setFlag(MidiOption::Spread64); MidiKey knob; knob.status = MidiUtils::statusFromOpCodeAndChannel( @@ -290,7 +290,7 @@ MidiInputMappings LearningUtils::guessMidiInputMappings( MidiOpCode::ControlChange, *stats.channels.begin()); msb.control = msb_control; MidiOptions msb_option; - msb_option.fourteen_bit_msb = true; + msb_option.setFlag(MidiOption::FourteenBitMSB); mappings.append(MidiInputMapping(msb, msb_option, control)); MidiKey lsb; @@ -298,7 +298,7 @@ MidiInputMappings LearningUtils::guessMidiInputMappings( MidiOpCode::ControlChange, *stats.channels.begin()); lsb.control = lsb_control; MidiOptions lsb_option; - lsb_option.fourteen_bit_lsb = true; + lsb_option.setFlag(MidiOption::FourteenBitLSB); mappings.append(MidiInputMapping(lsb, lsb_option, control)); } diff --git a/src/controllers/midi/legacymidicontrollermappingfilehandler.cpp b/src/controllers/midi/legacymidicontrollermappingfilehandler.cpp index 08bec039ae9..e0fcea473cf 100644 --- a/src/controllers/midi/legacymidicontrollermappingfilehandler.cpp +++ b/src/controllers/midi/legacymidicontrollermappingfilehandler.cpp @@ -68,35 +68,35 @@ LegacyMidiControllerMappingFileHandler::load(const QDomElement& root, // "normal" is no options if (strMidiOption == QLatin1String("invert")) { - options.invert = true; + options.setFlag(MidiOption::Invert); } else if (strMidiOption == QLatin1String("rot64")) { - options.rot64 = true; + options.setFlag(MidiOption::Rot64); } else if (strMidiOption == QLatin1String("rot64inv")) { - options.rot64_inv = true; + options.setFlag(MidiOption::Rot64Invert); } else if (strMidiOption == QLatin1String("rot64fast")) { - options.rot64_fast = true; + options.setFlag(MidiOption::Rot64Fast); } else if (strMidiOption == QLatin1String("diff")) { - options.diff = true; + options.setFlag(MidiOption::Diff); } else if (strMidiOption == QLatin1String("button")) { - options.button = true; + options.setFlag(MidiOption::Button); } else if (strMidiOption == QLatin1String("switch")) { - options.sw = true; + options.setFlag(MidiOption::Switch); } else if (strMidiOption == QLatin1String("hercjog")) { - options.herc_jog = true; + options.setFlag(MidiOption::HercJog); } else if (strMidiOption == QLatin1String("hercjogfast")) { - options.herc_jog_fast = true; + options.setFlag(MidiOption::HercJogFast); } else if (strMidiOption == QLatin1String("spread64")) { - options.spread64 = true; + options.setFlag(MidiOption::Spread64); } else if (strMidiOption == QLatin1String("selectknob")) { - options.selectknob = true; + options.setFlag(MidiOption::SelectKnob); } else if (strMidiOption == QLatin1String("soft-takeover")) { - options.soft_takeover = true; + options.setFlag(MidiOption::SoftTakeover); } else if (strMidiOption == QLatin1String("script-binding")) { - options.script = true; + options.setFlag(MidiOption::Script); } else if (strMidiOption == QLatin1String("fourteen-bit-msb")) { - options.fourteen_bit_msb = true; + options.setFlag(MidiOption::FourteenBitMSB); } else if (strMidiOption == QLatin1String("fourteen-bit-lsb")) { - options.fourteen_bit_lsb = true; + options.setFlag(MidiOption::FourteenBitLSB); } optionsNode = optionsNode.nextSiblingElement(); @@ -294,67 +294,67 @@ QDomElement LegacyMidiControllerMappingFileHandler::inputMappingToXML( QDomElement optionsNode = doc->createElement("options"); // "normal" is no options - if (mapping.options.all == 0) { + if (!mapping.options) { QDomElement singleOption = doc->createElement("normal"); optionsNode.appendChild(singleOption); } else { - if (mapping.options.invert) { + if (mapping.options.testFlag(MidiOption::Invert)) { QDomElement singleOption = doc->createElement("invert"); optionsNode.appendChild(singleOption); } - if (mapping.options.rot64) { + if (mapping.options.testFlag(MidiOption::Rot64)) { QDomElement singleOption = doc->createElement("rot64"); optionsNode.appendChild(singleOption); } - if (mapping.options.rot64_inv) { + if (mapping.options.testFlag(MidiOption::Rot64Invert)) { QDomElement singleOption = doc->createElement("rot64inv"); optionsNode.appendChild(singleOption); } - if (mapping.options.rot64_fast) { + if (mapping.options.testFlag(MidiOption::Rot64Fast)) { QDomElement singleOption = doc->createElement("rot64fast"); optionsNode.appendChild(singleOption); } - if (mapping.options.diff) { + if (mapping.options.testFlag(MidiOption::Diff)) { QDomElement singleOption = doc->createElement("diff"); optionsNode.appendChild(singleOption); } - if (mapping.options.button) { + if (mapping.options.testFlag(MidiOption::Button)) { QDomElement singleOption = doc->createElement("button"); optionsNode.appendChild(singleOption); } - if (mapping.options.sw) { + if (mapping.options.testFlag(MidiOption::Switch)) { QDomElement singleOption = doc->createElement("switch"); optionsNode.appendChild(singleOption); } - if (mapping.options.herc_jog) { + if (mapping.options.testFlag(MidiOption::HercJog)) { QDomElement singleOption = doc->createElement("hercjog"); optionsNode.appendChild(singleOption); } - if (mapping.options.herc_jog_fast) { + if (mapping.options.testFlag(MidiOption::HercJogFast)) { QDomElement singleOption = doc->createElement("hercjogfast"); optionsNode.appendChild(singleOption); } - if (mapping.options.spread64) { + if (mapping.options.testFlag(MidiOption::Spread64)) { QDomElement singleOption = doc->createElement("spread64"); optionsNode.appendChild(singleOption); } - if (mapping.options.selectknob) { + if (mapping.options.testFlag(MidiOption::SelectKnob)) { QDomElement singleOption = doc->createElement("selectknob"); optionsNode.appendChild(singleOption); } - if (mapping.options.soft_takeover) { + if (mapping.options.testFlag(MidiOption::SoftTakeover)) { QDomElement singleOption = doc->createElement("soft-takeover"); optionsNode.appendChild(singleOption); } - if (mapping.options.script) { + if (mapping.options.testFlag(MidiOption::Script)) { QDomElement singleOption = doc->createElement("script-binding"); optionsNode.appendChild(singleOption); } - if (mapping.options.fourteen_bit_msb) { + if (mapping.options.testFlag(MidiOption::FourteenBitMSB)) { QDomElement singleOption = doc->createElement("fourteen-bit-msb"); optionsNode.appendChild(singleOption); } - if (mapping.options.fourteen_bit_lsb) { + if (mapping.options.testFlag(MidiOption::FourteenBitLSB)) { QDomElement singleOption = doc->createElement("fourteen-bit-lsb"); optionsNode.appendChild(singleOption); } diff --git a/src/controllers/midi/midicontroller.cpp b/src/controllers/midi/midicontroller.cpp index a7a9af64260..265717f2e21 100644 --- a/src/controllers/midi/midicontroller.cpp +++ b/src/controllers/midi/midicontroller.cpp @@ -256,7 +256,7 @@ void MidiController::processInputMapping(const MidiInputMapping& mapping, unsigned char channel = MidiUtils::channelFromStatus(status); MidiOpCode opCode = MidiUtils::opCodeFromStatus(status); - if (mapping.options.script) { + if (mapping.options.testFlag(MidiOption::Script)) { ControllerScriptEngineLegacy* pEngine = getScriptEngine(); if (pEngine == nullptr) { return; @@ -284,9 +284,8 @@ void MidiController::processInputMapping(const MidiInputMapping& mapping, double newValue = value; - - bool mapping_is_14bit = mapping.options.fourteen_bit_msb || - mapping.options.fourteen_bit_lsb; + bool mapping_is_14bit = mapping.options.testFlag(MidiOption::FourteenBitMSB) || + mapping.options.testFlag(MidiOption::FourteenBitLSB); if (!mapping_is_14bit && !m_fourteen_bit_queued_mappings.isEmpty()) { qWarning() << "MidiController was waiting for the MSB/LSB of a 14-bit" << "message but the next message received was not mapped as 14-bit." @@ -294,15 +293,20 @@ void MidiController::processInputMapping(const MidiInputMapping& mapping, m_fourteen_bit_queued_mappings.clear(); } - //qDebug() << "MIDI Options" << QString::number(mapping.options.all, 2).rightJustified(16,'0'); + //qDebug() << "MIDI Options" << QString::number(mapping.options, 2).rightJustified(16,'0'); if (mapping_is_14bit) { bool found = false; for (auto it = m_fourteen_bit_queued_mappings.begin(); it != m_fourteen_bit_queued_mappings.end(); ++it) { if (it->first.control == mapping.control) { - if ((it->first.options.fourteen_bit_lsb && mapping.options.fourteen_bit_lsb) || - (it->first.options.fourteen_bit_msb && mapping.options.fourteen_bit_msb)) { + if ((it->first.options.testFlag(MidiOption::FourteenBitLSB) && + mapping.options.testFlag( + MidiOption::FourteenBitLSB)) || + (it->first.options.testFlag( + MidiOption::FourteenBitMSB) && + mapping.options.testFlag( + MidiOption::FourteenBitMSB))) { qWarning() << "MidiController: 14-bit MIDI mapping has mis-matched LSB/MSB options." << "Ignoring both messages."; m_fourteen_bit_queued_mappings.erase(it); @@ -310,12 +314,12 @@ void MidiController::processInputMapping(const MidiInputMapping& mapping, } int iValue = 0; - if (mapping.options.fourteen_bit_msb) { + if (mapping.options.testFlag(MidiOption::FourteenBitMSB)) { iValue = (value << 7) | it->second; // qDebug() << "MSB" << value // << "LSB" << it->second // << "Joint:" << iValue; - } else if (mapping.options.fourteen_bit_lsb) { + } else if (mapping.options.testFlag(MidiOption::FourteenBitLSB)) { iValue = (it->second << 7) | value; // qDebug() << "MSB" << it->second // << "LSB" << value @@ -364,11 +368,12 @@ void MidiController::processInputMapping(const MidiInputMapping& mapping, // ControlPushButton ControlObjects only accept NOTE_ON, so if the midi // mapping is