Midi unit tests, and refactor internal handling to std::variant

Syl Morrison · Syl Morrison · commit 01cb08cb942a · 2025-11-29T15:00:21.000Z
diff --git a/include/mostly_harmless/CMakeLists.txt b/include/mostly_harmless/CMakeLists.txt
@@ -15,6 +15,7 @@ set(MOSTLYHARMLESS_HEADERS
         ${CMAKE_CURRENT_SOURCE_DIR}/events/mostlyharmless_InputEventContext.h
         ${CMAKE_CURRENT_SOURCE_DIR}/events/mostlyharmless_WebEvent.h
         ${CMAKE_CURRENT_SOURCE_DIR}/events/mostlyharmless_ParamEvent.h
+        ${CMAKE_CURRENT_SOURCE_DIR}/events/mostlyharmless_MidiEvent.h
         ${CMAKE_CURRENT_SOURCE_DIR}/mostlyharmless_Parameters.h
         ${CMAKE_CURRENT_SOURCE_DIR}/gui/mostlyharmless_WebviewBase.h
         ${CMAKE_CURRENT_SOURCE_DIR}/gui/mostlyharmless_WebviewEditor.h
@@ -27,6 +28,7 @@ set(MOSTLYHARMLESS_HEADERS
         ${CMAKE_CURRENT_SOURCE_DIR}/utils/mostlyharmless_Proxy.h
         ${CMAKE_CURRENT_SOURCE_DIR}/utils/mostlyharmless_NoDenormals.h
         ${CMAKE_CURRENT_SOURCE_DIR}/utils/mostlyharmless_Logging.h
+        ${CMAKE_CURRENT_SOURCE_DIR}/utils/mostlyharmless_Visitor.h
         ${CMAKE_CURRENT_SOURCE_DIR}/data/mostlyharmless_DatabaseState.h
         ${CMAKE_CURRENT_SOURCE_DIR}/data/mostlyharmless_DatabasePropertyWatcher.h
         ${PLATFORM_HEADERS}
diff --git a/include/mostly_harmless/core/mostlyharmless_IEngine.h b/include/mostly_harmless/core/mostlyharmless_IEngine.h
@@ -142,13 +142,12 @@ namespace mostly_harmless::core {
 
         /**
          * Called if the plugin receives a pitch wheel event - not pure virtual, as this function isn't relevant if you haven't requested midi functionality.
-         * Value here is a 14-bit value representing the pitch wheel's position, in the range 0x0 to 0x3FFF. Generally, 0x2000 is treated as the centre, negative pitch wheel pos is below 0x2000, and positive is above etc etc etc.
          * Called on the audio thread, in response to a pitch wheel event.
          * @param portIndex The clap port index the event originated from.
          * @param channel The midi channel the event was passed to
-         * @param value The 14-bit value representing pitch wheel pos between 0x0 and 0x3FFF.
+         * @param value The value, between -1.0 and 1.0
          */
-        virtual void handlePitchWheel([[maybe_unused]] std::uint8_t portIndex, [[maybe_unused]] std::uint8_t channel, [[maybe_unused]] std::uint16_t value) {}
+        virtual void handlePitchWheel([[maybe_unused]] std::uint8_t portIndex, [[maybe_unused]] std::uint8_t channel, [[maybe_unused]] double value) {}
     };
 } // namespace mostly_harmless::core
 #endif // MOSTLYHARMLESS_MOSTLYHARMLESS_IENGINE_H
diff --git a/include/mostly_harmless/events/mostlyharmless_MidiEvent.h b/include/mostly_harmless/events/mostlyharmless_MidiEvent.h
@@ -0,0 +1,60 @@
+#ifndef MOSTLY_HARMLESS_MIDI_EVENT
+#define MOSTLY_HARMLESS_MIDI_EVENT
+#include <variant>
+#include <optional>
+/// @internal
+namespace mostly_harmless::events::midi {
+    /// @internal
+    struct NoteOn final {
+        std::uint8_t channel;
+        std::uint8_t note;
+        double velocity;
+    };
+
+    /// @internal
+    struct NoteOff final {
+        std::uint8_t channel;
+        std::uint8_t note;
+        double velocity;
+    };
+
+    /// @internal
+    struct PolyAftertouch final {
+        std::uint8_t channel;
+        std::uint8_t note;
+        std::uint8_t pressure;
+    };
+
+    /// @internal
+    struct ControlChange final {
+        std::uint8_t channel;
+        std::uint8_t controllerNumber;
+        std::uint8_t data;
+    };
+
+    /// @internal
+    struct ProgramChange final {
+        std::uint8_t channel;
+        std::uint8_t programNumber;
+    };
+
+    /// @internal
+    struct ChannelAftertouch final {
+        std::uint8_t channel;
+        std::uint8_t pressure;
+    };
+
+    /// @internal
+    struct PitchWheel final {
+        std::uint8_t channel;
+        double value;
+    };
+
+    /// @internal
+    using MidiEvent = std::variant<NoteOn, NoteOff, PolyAftertouch, ControlChange, ProgramChange, ChannelAftertouch, PitchWheel>;
+
+    /// @internal
+    auto parse(std::uint8_t b0, std::uint8_t b1, std::uint8_t b2) -> std::optional<MidiEvent>;
+} // namespace mostly_harmless::events::midi
+
+#endif
diff --git a/include/mostly_harmless/utils/mostlyharmless_Visitor.h b/include/mostly_harmless/utils/mostlyharmless_Visitor.h
@@ -0,0 +1,26 @@
+//
+// Created by Syl Morrison on 29/11/2025.
+//
+
+#ifndef MOSTLYHARMLESS_VISITOR_H
+#define MOSTLYHARMLESS_VISITOR_H
+namespace mostly_harmless::utils {
+    /**
+     * \brief Util for use with `std::visit` to avoid a bunch of `if constexpr(...)`s.
+     *
+     * Usage::
+     * ```
+     *      std::variant<int, double> data;
+     *      std::visit(Visitor{
+     *          [](int x) { std::cout << "Was int!\n"; },
+     *          [](double x) { std::cout << "Was double!\n"; }
+     *      }, data);
+     * ```
+     * @tparam Callable
+     */
+    template <typename... Callable>
+    struct Visitor : Callable... {
+        using Callable::operator()...;
+    };
+} // namespace mostly_harmless::utils
+#endif // GLEO_MOSTLYHARMLESS_VISITOR_H
diff --git a/source/CMakeLists.txt b/source/CMakeLists.txt
@@ -23,6 +23,7 @@ set(MOSTLYHARMLESS_SOURCES
         ${CMAKE_CURRENT_SOURCE_DIR}/events/mostlyharmless_InputEventContext.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/events/mostlyharmless_ParamEvent.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/events/mostlyharmless_WebEvent.cpp
+        ${CMAKE_CURRENT_SOURCE_DIR}/events/mostlyharmless_MidiEvent.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/gui/mostlyharmless_WebviewBase.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/gui/mostlyharmless_WebviewEditor.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/gui/mostlyharmless_Colour.cpp
diff --git a/source/events/mostlyharmless_MidiEvent.cpp b/source/events/mostlyharmless_MidiEvent.cpp
@@ -0,0 +1,38 @@
+#include <mostly_harmless/events/mostlyharmless_MidiEvent.h>
+namespace mostly_harmless::events::midi {
+    constexpr static auto s_note_off{ 0x80 };
+    constexpr static auto s_note_on{ 0x90 };
+    constexpr static auto s_poly_aftertouch{ 0xA0 };
+    constexpr static auto s_control_change{ 0xB0 };
+    constexpr static auto s_program_change{ 0xC0 };
+    constexpr static auto s_channel_aftertouch{ 0xD0 };
+    constexpr static auto s_pitch_wheel{ 0xE0 };
+
+    auto parse(std::uint8_t b0, std::uint8_t b1, std::uint8_t b2) -> std::optional<MidiEvent> {
+        const std::uint8_t message = b0 & 0xF0;
+        const std::uint8_t channel = b0 & 0x0F;
+        switch (message) {
+            case s_note_on: [[fallthrough]];
+            case s_note_off: {
+                const std::uint8_t note = b1;
+                const std::uint8_t velocity = b2;
+                const auto floatVel = static_cast<double>(velocity) / 127.0;
+                if (message == s_note_on && velocity != 0) {
+                    return NoteOn{ .channel = channel, .note = note, .velocity = floatVel };
+                }
+                return NoteOff{ .channel = channel, .note = note, .velocity = floatVel };
+            }
+            case s_poly_aftertouch: return PolyAftertouch{ .channel = channel, .note = b1, .pressure = b2 };
+            case s_control_change: return ControlChange{ .channel = channel, .controllerNumber = b1, .data = b2 };
+            case s_program_change: return ProgramChange{ .channel = channel, .programNumber = b1 };
+            case s_channel_aftertouch: return ChannelAftertouch{ .channel = channel, .pressure = b1 };
+            case s_pitch_wheel: {
+                const std::int16_t combined = b1 | (b2 << 7);
+                double res = static_cast<double>(combined - 8192) / 8192.0;
+                return PitchWheel{ .channel = channel, .value = res };
+            }
+            default: return {};
+        }
+    }
+
+} // namespace mostly_harmless::events::midi
diff --git a/source/mostlyharmless_PluginBase.cpp b/source/mostlyharmless_PluginBase.cpp
@@ -1,22 +1,17 @@
 //
 // Created by Syl Morrison on 20/10/2024.
 //
+#include "mostly_harmless/utils/mostlyharmless_Visitor.h"
+
+
 #include <mostly_harmless/mostlyharmless_PluginBase.h>
 #include <mostly_harmless/utils/mostlyharmless_Macros.h>
 #include <mostly_harmless/audio/mostlyharmless_AudioHelpers.h>
 #include <mostly_harmless/utils/mostlyharmless_NoDenormals.h>
+#include <mostly_harmless/events/mostlyharmless_MidiEvent.h>
 #include <clap/helpers/plugin.hxx>
 
 namespace mostly_harmless::internal {
-    namespace midi_headers {
-        constexpr static auto s_note_off{ 0x80 };
-        constexpr static auto s_note_on{ 0x90 };
-        constexpr static auto s_poly_aftertouch{ 0xA0 };
-        constexpr static auto s_control_change{ 0xB0 };
-        constexpr static auto s_program_change{ 0xC0 };
-        constexpr static auto s_channel_aftertouch{ 0xD0 };
-        constexpr static auto s_pitch_wheel{ 0xE0 };
-    } // namespace midi_headers
     PluginBase::PluginBase(const clap_host* host) : clap::helpers::Plugin<clap::helpers::MisbehaviourHandler::Ignore, clap::helpers::CheckingLevel::Maximal>(&getDescriptor(), host) {
         MH_LOG("PROC: Creating plugin instance...");
         m_pluginEntry = core::createPluginEntry();
@@ -177,53 +172,33 @@ namespace mostly_harmless::internal {
                 // 0PPP PPPP
                 // 0VVV VVVV
                 const auto* midiEvent = reinterpret_cast<const clap_event_midi*>(event);
-                const std::uint8_t message = midiEvent->data[0] & 0xF0; // SSSS 0000 - Message will be << 4
-                const std::uint8_t channel = midiEvent->data[0] & 0x0F; // 0000 CCCC
-                switch (message) {
-                    case midi_headers::s_note_on: [[fallthrough]];
-                    case midi_headers::s_note_off: {
-                        const std::uint8_t note = midiEvent->data[1];     // 0PPP PPPP
-                        const std::uint8_t velocity = midiEvent->data[2]; // 0VVV VVVV
-                        const auto fpVelocity = static_cast<double>(velocity) / 127.0;
-                        if (message == midi_headers::s_note_on) {
-                            m_engine->handleNoteOn(midiEvent->port_index, channel, note, fpVelocity);
-                        } else {
-                            m_engine->handleNoteOff(midiEvent->port_index, channel, note, fpVelocity);
-                        }
-                        break;
-                    }
-                    case midi_headers::s_poly_aftertouch: {
-                        const std::uint8_t note = midiEvent->data[1];
-                        const std::uint8_t pressure = midiEvent->data[2];
-                        m_engine->handlePolyAftertouch(midiEvent->port_index, channel, note, pressure);
-                        break;
-                    }
-                    case midi_headers::s_control_change: {
-                        const std::uint8_t controllerNumber = midiEvent->data[1];
-                        const std::uint8_t data = midiEvent->data[2];
-                        m_engine->handleControlChange(midiEvent->port_index, channel, controllerNumber, data);
-                        break;
-                    }
-                    case midi_headers::s_program_change: {
-                        const std::uint8_t programNumber = midiEvent->data[1];
-                        m_engine->handleProgramChange(midiEvent->port_index, channel, programNumber);
-                        break;
-                    }
-                    case midi_headers::s_channel_aftertouch: {
-                        const std::uint8_t pressure = midiEvent->data[1];
-                        m_engine->handleChannelAftertouch(midiEvent->port_index, channel, pressure);
-                        break;
-                    }
-                    case midi_headers::s_pitch_wheel: {
-                        const std::uint8_t lsb = midiEvent->data[1];
-                        const std::uint8_t msb = midiEvent->data[2];
-                        const std::uint16_t combined = lsb & (msb << 7);
-                        m_engine->handlePitchWheel(midiEvent->port_index, channel, combined);
-                        break;
-                    }
-                    default: break;
+                auto res = mostly_harmless::events::midi::parse(midiEvent->data[0], midiEvent->data[1], midiEvent->data[2]);
+                if (!res) {
+                    return;
                 }
-                break;
+                std::visit(utils::Visitor{
+                               [this, &midiEvent](events::midi::NoteOff x) {
+                                   m_engine->handleNoteOff(midiEvent->port_index, x.channel, x.note, x.velocity);
+                               },
+                               [this, &midiEvent](events::midi::NoteOn x) {
+                                   m_engine->handleNoteOn(midiEvent->port_index, x.channel, x.note, x.velocity);
+                               },
+                               [this, &midiEvent](events::midi::PolyAftertouch x) {
+                                   m_engine->handlePolyAftertouch(midiEvent->port_index, x.channel, x.note, x.pressure);
+                               },
+                               [this, &midiEvent](events::midi::ControlChange x) {
+                                   m_engine->handleControlChange(midiEvent->port_index, x.channel, x.controllerNumber, x.data);
+                               },
+                               [this, &midiEvent](events::midi::ProgramChange x) {
+                                   m_engine->handleProgramChange(midiEvent->port_index, x.channel, x.programNumber);
+                               },
+                               [this, &midiEvent](events::midi::ChannelAftertouch x) {
+                                   m_engine->handleChannelAftertouch(midiEvent->port_index, x.channel, x.pressure);
+                               },
+                               [this, &midiEvent](events::midi::PitchWheel x) {
+                                   m_engine->handlePitchWheel(midiEvent->port_index, x.channel, x.value);
+                               } },
+                           *res);
             }
             case CLAP_EVENT_TRANSPORT: {
                 if (const auto* transportEvent = reinterpret_cast<const clap_event_transport_t*>(event)) {
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@ -5,4 +5,5 @@ set(MOSTLYHARMLESS_TEST_SOURCE
         ${CMAKE_CURRENT_SOURCE_DIR}/utils/mostlyharmless_TimerTests.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/events/mostlyharmless_InputEventContextTests.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/data/mostlyharmless_DatabaseStateTests.cpp
+        ${CMAKE_CURRENT_SOURCE_DIR}/events/mostlyharmless_MidiEventTests.cpp
         PARENT_SCOPE)
diff --git a/tests/data/mostlyharmless_DatabaseStateTests.cpp b/tests/data/mostlyharmless_DatabaseStateTests.cpp
diff --git a/tests/events/mostlyharmless_MidiEventTests.cpp b/tests/events/mostlyharmless_MidiEventTests.cpp
