MIDI cleanups, de-dupe logic here and there. Re-add description support.

6 files changed, 44 insertions, 69 deletions

diff --git a/src/controllers/delegates/midicontroldelegate.cpp b/src/controllers/delegates/midicontroldelegate.cpp
index 362d190d1a..c85c94625d 100644
--- a/src/controllers/delegates/midicontroldelegate.cpp
+++ b/src/controllers/delegates/midicontroldelegate.cpp
@@ -21,7 +21,7 @@ QWidget* MidiControlDelegate::createEditor(QWidget* parent,
 QString MidiControlDelegate::displayText(const QVariant& value,
                                         const QLocale& locale) const {
     unsigned char control = static_cast<unsigned char>(value.toInt());
-    return MidiUtils::formatByte(control);
+    return MidiUtils::formatByteAsHex(control);
 }
 
 void MidiControlDelegate::setEditorData(QWidget* editor,
diff --git a/src/controllers/midi/midicontroller.cpp b/src/controllers/midi/midicontroller.cpp
index 56c1d4e028..de104a44a2 100644
--- a/src/controllers/midi/midicontroller.cpp
+++ b/src/controllers/midi/midicontroller.cpp
@@ -244,7 +244,6 @@ void MidiController::receive(unsigned char status, unsigned char control,
                              unsigned char value) {
     unsigned char channel = channelFromStatus(status);
     unsigned char opCode = opCodeFromStatus(status);
-    bool twoBytes = isMessageTwoBytes(opCode);
 
     if (debugging()) {
         qDebug() << formatMidiMessage(status, control, value, channel, opCode);
@@ -252,16 +251,7 @@ void MidiController::receive(unsigned char status, unsigned char control,
 
     //if (m_bReceiveInhibit) return;
 
-    MidiKey mappingKey;
-    mappingKey.status = status;
-
-    // When it's part of the message, include it
-    if (twoBytes) {
-        mappingKey.control = control;
-    } else {
-        // Signifies that the second byte is part of the payload, default
-        mappingKey.control = 0xFF;
-    }
+    MidiKey mappingKey = makeMidiKey(status, control);
 
     bool foundControl = false;
     QPair<MixxxControl, MidiOptions> controlOptions;
@@ -295,7 +285,7 @@ void MidiController::receive(unsigned char status, unsigned char control,
         }
 
         QScriptValueList args;
-        args << QScriptValue(status & 0x0F);
+        args << QScriptValue(channel);
         args << QScriptValue(control);
         args << QScriptValue(value);
         args << QScriptValue(status);
@@ -475,10 +465,7 @@ void MidiController::receive(QByteArray data) {
 
     //if (m_bReceiveInhibit) return;
 
-    MidiKey mappingKey;
-    mappingKey.status = data.at(0);
-    // Signifies that the second byte is part of the payload, default
-    mappingKey.control = 0xFF;
+    MidiKey mappingKey = makeMidiKey(data.at(0), 0xFF);
 
     // TODO(rryan): Need to review how MIDI learn works with sysex messages. I
     // don't think this actually does anything useful.
diff --git a/src/controllers/midi/midicontrollerpresetfilehandler.cpp b/src/controllers/midi/midicontrollerpresetfilehandler.cpp
index 4772c19a0f..7773b37d15 100644
--- a/src/controllers/midi/midicontrollerpresetfilehandler.cpp
+++ b/src/controllers/midi/midicontrollerpresetfilehandler.cpp
@@ -6,7 +6,8 @@
 *
 */
 
-#include "midicontrollerpresetfilehandler.h"
+#include "controllers/midi/midicontrollerpresetfilehandler.h"
+#include "controllers/midi/midiutils.h"
 
 #define DEFAULT_OUTPUT_MAX  1.0f
 #define DEFAULT_OUTPUT_MIN  0.0f    // Anything above 0 is "on"
@@ -33,7 +34,7 @@ ControllerPresetPointer MidiControllerPresetFileHandler::load(const QDomElement
 
     QDomElement control = controller.firstChildElement("controls").firstChildElement("control");
 
-    //Iterate through each <control> block in the XML
+    // Iterate through each <control> block in the XML
     while (!control.isNull()) {
 
         //Unserialize these objects from the XML
@@ -42,10 +43,11 @@ ControllerPresetPointer MidiControllerPresetFileHandler::load(const QDomElement
 
         bool ok = false;
 
-        //Use QString with toInt base of 0 to auto convert hex values
+        // Allow specifying hex, octal, or decimal.
         unsigned char midiStatusByte = strMidiStatus.toInt(&ok, 0);
         if (!ok) midiStatusByte = 0x00;
 
+        // Allow specifying hex, octal, or decimal.
         unsigned char midiControl = midiNo.toInt(&ok, 0);
         if (!ok) midiControl = 0x00;
 
@@ -85,47 +87,17 @@ ControllerPresetPointer MidiControllerPresetFileHandler::load(const QDomElement
 
         // Add the static mapping
         QPair<MixxxControl, MidiOptions> target;
-        target.first = MixxxControl(controlGroup, controlKey);
+        target.first = MixxxControl(controlGroup, controlKey,
+                                    controlDescription);
         target.second = options;
+        MidiKey key = makeMidiKey(midiStatusByte, midiControl);
 
-        unsigned char opCode = midiStatusByte & 0xF0;
-        if (opCode >= 0xF0) opCode = midiStatusByte;
-
-        bool twoBytes = true;
-        switch (opCode) {
-            case MIDI_SONG:
-            case MIDI_NOTE_OFF:
-            case MIDI_NOTE_ON:
-            case MIDI_AFTERTOUCH:
-            case MIDI_CC:
-                break;
-            case MIDI_PITCH_BEND:
-            case MIDI_SONG_POS:
-            case MIDI_PROGRAM_CH:
-            case MIDI_CH_AFTERTOUCH:
-            default:
-                twoBytes = false;
-                break;
-        }
-
-        MidiKey key;
-        key.status = midiStatusByte;
-
-        if (twoBytes) {
-            key.control = midiControl;
-        } else {
-            // Signifies that the second byte is part of the payload, default
-            key.control = 0xFF;
-        }
         // qDebug() << "New mapping:" << QString::number(key.key, 16).toUpper()
         //          << QString::number(key.status, 16).toUpper()
         //          << QString::number(key.control, 16).toUpper()
         //          << target.first.group << target.first.item;
 
         preset->mappings.insert(key.key, target);
-        // Notify the GUI and anyone else who cares
-        //emit(newMapping());  // TODO
-
         control = control.nextSiblingElement("control");
     }
 
@@ -140,9 +112,11 @@ ControllerPresetPointer MidiControllerPresetFileHandler::load(const QDomElement
         //Unserialize the MixxxControl components from the XML
         QDomElement groupNode = output.firstChildElement("group");
         QDomElement keyNode = output.firstChildElement("key");
+        QDomElement descriptionNode = output.firstChildElement("description");
 
         QString controlGroup = groupNode.text();
         QString controlKey = keyNode.text();
+        QString controlDescription = descriptionNode.text();
 
         // Unserialize output message from the XML
         MidiOutput outputMessage;
@@ -201,7 +175,8 @@ ControllerPresetPointer MidiControllerPresetFileHandler::load(const QDomElement
 
         // We use insertMulti because certain tricks are done with multiple
         // entries for the same MixxxControl
-        preset->outputMappings.insertMulti(MixxxControl(controlGroup, controlKey), outputMessage);
+        preset->outputMappings.insertMulti(MixxxControl(
+            controlGroup, controlKey, controlDescription), outputMessage);
 
         output = output.nextSiblingElement("output");
     }
@@ -340,37 +315,43 @@ void MidiControllerPresetFileHandler::mappingToXML(QDomElement& parentNode,
     QDomDocument nodeMaker;
     QDomElement tagNode;
 
-    //Control object group
+    // CO group
     tagNode = nodeMaker.createElement("group");
     text = nodeMaker.createTextNode(mc.group());
     tagNode.appendChild(text);
     parentNode.appendChild(tagNode);
 
-    //Control object name
+    // CO item
     tagNode = nodeMaker.createElement("key"); //WTF worst name ever
     text = nodeMaker.createTextNode(mc.item());
     tagNode.appendChild(text);
     parentNode.appendChild(tagNode);
 
+    // Description
+    if (!mc.description().isEmpty()) {
+        tagNode = nodeMaker.createElement("description");
+        text = nodeMaker.createTextNode(mc.description());
+        tagNode.appendChild(text);
+        parentNode.appendChild(tagNode);
+    }
+
     //MIDI status byte
     tagNode = nodeMaker.createElement("status");
-    text = nodeMaker.createTextNode(
-        QString("0x%1").arg(QString::number(status, 16).toUpper().rightJustified(2,'0')));
+    text = nodeMaker.createTextNode(MidiUtils::formatByteAsHex(status));
     tagNode.appendChild(text);
     parentNode.appendChild(tagNode);
 
     if (control != 0xFF) {
         //MIDI control number
         tagNode = nodeMaker.createElement("midino");
-        text = nodeMaker.createTextNode(
-            QString("0x%1").arg(QString::number(control, 16).toUpper().rightJustified(2,'0')));
+        text = nodeMaker.createTextNode(MidiUtils::formatByteAsHex(control));
         tagNode.appendChild(text);
         parentNode.appendChild(tagNode);
     }
 }
 
 void MidiControllerPresetFileHandler::outputMappingToXML(QDomElement& parentNode, unsigned char on,
-                                        unsigned char off, float max, float min) const {
+                                                         unsigned char off, float max, float min) const {
     QDomText text;
     QDomDocument nodeMaker;
     QDomElement tagNode;
@@ -378,8 +359,7 @@ void MidiControllerPresetFileHandler::outputMappingToXML(QDomElement& parentNode
     // Third MIDI byte for turning on the LED
     if (on != DEFAULT_OUTPUT_ON) {
         tagNode = nodeMaker.createElement("on");
-        text = nodeMaker.createTextNode(
-            QString("0x%1").arg(QString::number(on, 16).toUpper().rightJustified(2,'0')));
+        text = nodeMaker.createTextNode(MidiUtils::formatByteAsHex(on));
         tagNode.appendChild(text);
         parentNode.appendChild(tagNode);
     }
@@ -387,8 +367,7 @@ void MidiControllerPresetFileHandler::outputMappingToXML(QDomElement& parentNode
     // Third MIDI byte for turning off the LED
     if (off != DEFAULT_OUTPUT_OFF) {
         tagNode = nodeMaker.createElement("off");
-        text = nodeMaker.createTextNode(
-            QString("0x%1").arg(QString::number(off, 16).toUpper().rightJustified(2,'0')));
+        text = nodeMaker.createTextNode(MidiUtils::formatByteAsHex(off));
         tagNode.appendChild(text);
         parentNode.appendChild(tagNode);
     }
diff --git a/src/controllers/midi/midimessage.h b/src/controllers/midi/midimessage.h
index 5eea3223b8..37c9ec8ac3 100644
--- a/src/controllers/midi/midimessage.h
+++ b/src/controllers/midi/midimessage.h
@@ -159,6 +159,15 @@ inline bool isMessageTwoBytes(unsigned char opCode) {
     }
 }
 
+inline MidiKey makeMidiKey(unsigned char status, unsigned char control) {
+    unsigned char opCode = opCodeFromStatus(status);
+    bool twoBytes = isMessageTwoBytes(opCode);
+
+    // When it's part of the message, include it. If the message is not two
+    // bytes, signify that the second byte is part of the payload with 0xFF.
+    return MidiKey(status, twoBytes ? control : 0xFF);
+}
+
 inline bool isClockSignal(const MidiKey& mappingKey) {
     return (mappingKey.key & MIDI_TIMING_CLK) == MIDI_TIMING_CLK;
 }
diff --git a/src/controllers/midi/midiutils.cpp b/src/controllers/midi/midiutils.cpp
index 93abb552f9..845ae4cf36 100644
--- a/src/controllers/midi/midiutils.cpp
+++ b/src/controllers/midi/midiutils.cpp
@@ -21,10 +21,10 @@ QString MidiUtils::opCodeToTranslatedString(MidiOpCode code) {
 }
 
 // static
-QString MidiUtils::formatByte(unsigned char value) {
+QString MidiUtils::formatByteAsHex(unsigned char value) {
     // Construct a hex string formatted like 0xFF.
-    return QString("0x") + QString("%1")
-            .arg(value, 2, 16, QLatin1Char('0')).toUpper();
+    return QString("0x%1").arg(QString::number(value, 16)
+                               .toUpper().rightJustified(2,'0'));
 }
 
 // static
diff --git a/src/controllers/midi/midiutils.h b/src/controllers/midi/midiutils.h
index dbcc9a4009..51c77a7411 100644
--- a/src/controllers/midi/midiutils.h
+++ b/src/controllers/midi/midiutils.h
@@ -6,7 +6,7 @@
 class MidiUtils {
   public:
     static QString opCodeToTranslatedString(MidiOpCode code);
-    static QString formatByte(unsigned char value);
+    static QString formatByteAsHex(unsigned char value);
     static QString midiOptionToTranslatedString(MidiOption option);
 };