diff --git a/CHANGELOG.md b/CHANGELOG.md index 280a0994c..624f0bffe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,13 @@ ## [1.2.x] +### Changed +- Note names throughout the UI now use scientific pitch notation (middle C = C4), matching the convention used by most DAWs. + ### Fixed - Blocks embedded in the graph return to their embedded state when the plugin window is closed. +- MIDI Monitor note names displayed two octaves too high. +- MIDI Monitor logged Start/Stop/Continue messages twice. ## [1.2.0] diff --git a/src/nodes/midimonitor.cpp b/src/nodes/midimonitor.cpp index 0cda6cdbb..dafd8ae4f 100644 --- a/src/nodes/midimonitor.cpp +++ b/src/nodes/midimonitor.cpp @@ -2,6 +2,7 @@ // SPDX-License-Identifier: GPL-3.0-or-later #include "nodes/midimonitor.hpp" +#include "utils.hpp" namespace element { @@ -72,6 +73,32 @@ void MidiMonitorNode::clearMessages() messagesLogged(); } +String MidiMonitorNode::describe (const MidiMessage& msg) +{ + if (msg.isMidiClock()) + return {}; + + if (msg.isMidiStart()) + return "Start"; + if (msg.isMidiStop()) + return "Stop"; + if (msg.isMidiContinue()) + return "Continue"; + + if (msg.isNoteOn() || msg.isNoteOff()) + { + String text; + text << (msg.isNoteOn() ? "Note On " : "Note Off ") + << Util::noteValueToString (msg.getNoteNumber()) + << " (" << msg.getNoteNumber() << ")" + << " Velocity " << (int) msg.getVelocity() + << " Channel " << msg.getChannel(); + return text; + } + + return msg.getDescription(); +} + void MidiMonitorNode::timerCallback() { midiTemp.clear(); @@ -80,48 +107,13 @@ void MidiMonitorNode::timerCallback() return; int numLogged = 0; - String text; for (auto m : midiTemp) { - auto msg = m.getMessage(); - if (msg.isMidiClock()) - { - text.clear(); + const auto text = describe (m.getMessage()); + if (text.isEmpty()) continue; - } - - if (msg.isMidiStart()) - text << "Start"; - else if (msg.isMidiStop()) - text << "Stop"; - else if (msg.isMidiContinue()) - text << "Continue"; - - if (text.isNotEmpty()) - { - midiLog.add (text); - } - else if (msg.isNoteOn()) - { - text.clear(); - text << "Note On " - << msg.getMidiNoteName (msg.getNoteNumber(), true, true, 5) - << " (" << msg.getNoteNumber() << ") " - << " Velocity " << msg.getVelocity() - << " Channel " << msg.getChannel(); - } - else if (msg.isNoteOff()) - { - text.clear(); - text << "Note Off " - << msg.getMidiNoteName (msg.getNoteNumber(), true, true, 5) - << " (" << msg.getNoteNumber() << ") " - << " Velocity " << msg.getVelocity() - << " Channel " << msg.getChannel(); - } - - midiLog.add (text.isNotEmpty() ? text : msg.getDescription()); - text.clear(); + + midiLog.add (text); ++numLogged; } diff --git a/src/nodes/midimonitor.hpp b/src/nodes/midimonitor.hpp index 8d9f8ccb1..31a447446 100644 --- a/src/nodes/midimonitor.hpp +++ b/src/nodes/midimonitor.hpp @@ -58,6 +58,14 @@ class MidiMonitorNode : public MidiFilterNode, const StringArray& logger() const { return midiLog; } + /** Formats a MIDI message the way it appears in the monitor log. + + @param msg the message to describe + @return the display text, or an empty string for messages that are not + logged (MIDI clock) + */ + static juce::String describe (const juce::MidiMessage& msg); + private: friend class MidiMonitorNodeEditor; friend class MidiMonitorBlock; diff --git a/src/ui/virtualkeyboardview.cpp b/src/ui/virtualkeyboardview.cpp index 01659d650..1d1698237 100644 --- a/src/ui/virtualkeyboardview.cpp +++ b/src/ui/virtualkeyboardview.cpp @@ -4,6 +4,7 @@ #include #include "ui/guicommon.hpp" #include "ui/virtualkeyboardview.hpp" +#include "utils.hpp" namespace element { @@ -36,6 +37,7 @@ static int getOctaveOffsetForKeyPress (const KeyPress& key, const int fallback = VirtualKeyboardComponent::VirtualKeyboardComponent (MidiKeyboardState& s, Orientation o) : MidiKeyboardComponent (s, o) { + setOctaveForMiddleC (Util::middleCOctave); } void VirtualKeyboardComponent::setKeypressOctaveOffset (int offset) diff --git a/src/utils.hpp b/src/utils.hpp index 6eef0d192..29863c0eb 100644 --- a/src/utils.hpp +++ b/src/utils.hpp @@ -34,22 +34,36 @@ inline static String secondsToString (const double input) return minutesToString (input / 60.0); } +/** The octave number given to middle C (MIDI note 60) everywhere note names are + displayed or parsed. + + Element uses scientific pitch notation, middle C = C4, which is what most DAWs + show. Anything naming a MIDI note should go through noteValueToString() or use + this constant rather than passing its own number to juce::MidiMessage. +*/ +inline constexpr int middleCOctave = 4; + +/** Converts a MIDI note number to its note name, e.g. "C4", "F#2", "A0". + + @param value the MIDI note number, rounded to the nearest integer + @return the note name including its octave number +*/ inline static String noteValueToString (double value) { - return juce::MidiMessage::getMidiNoteName (juce::roundToInt (value), true, true, 3); + return juce::MidiMessage::getMidiNoteName (juce::roundToInt (value), true, true, middleCOctave); } -/** Parses a note name (e.g. "C3", "F#2", "Eb4") or a raw MIDI number into a +/** Parses a note name (e.g. "C4", "F#2", "Eb4") or a raw MIDI number into a MIDI note number 0-127. - Uses the same octave-for-middle-C convention as noteValueToString (middle C = C3), + Uses the same octave-for-middle-C convention as noteValueToString (middle C = C4), so this is the inverse of that function. @param text the text to parse @param octaveForMiddleC the octave number assigned to middle C (note 60) @return the MIDI note number 0-127, or -1 if the text cannot be parsed */ -inline static int noteValueFromString (const juce::String& text, int octaveForMiddleC = 3) +inline static int noteValueFromString (const juce::String& text, int octaveForMiddleC = middleCOctave) { const auto trimmed = text.trim(); if (trimmed.isEmpty()) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index d883e81ae..7f2329bb2 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -77,6 +77,7 @@ add_test(NAME "MidiMappingSessionTests" COMMAND test_element --run_test=MidiMapp add_test(NAME "MappingEngineTests" COMMAND test_element --run_test=MappingEngineTests) add_test(NAME "MappingLearnTests" COMMAND test_element --run_test=MappingLearnTests) add_test(NAME "MidiClockTest" COMMAND test_element --run_test=MidiClockTest) +add_test(NAME "MidiMonitorTests" COMMAND test_element --run_test=MidiMonitorTests) add_test(NAME "MidiProgramMapTests" COMMAND test_element --run_test=MidiProgramMapTests) add_test(NAME "MidiProgramTests" COMMAND test_element --run_test=MidiProgramTests) add_test(NAME "MidiScriptTests" COMMAND test_element --run_test=MidiScriptTests) diff --git a/test/MidiMonitorTests.cpp b/test/MidiMonitorTests.cpp new file mode 100644 index 000000000..cc2fc963f --- /dev/null +++ b/test/MidiMonitorTests.cpp @@ -0,0 +1,62 @@ +// SPDX-FileCopyrightText: Copyright (C) Kushview, LLC. +// SPDX-License-Identifier: GPL-3.0-or-later + +#include + +#include "nodes/midimonitor.hpp" +#include "utils.hpp" + +using namespace element; +using namespace juce; + +BOOST_AUTO_TEST_SUITE (MidiMonitorTests) + +BOOST_AUTO_TEST_CASE (NoteOnUsesMiddleCAsC4) +{ + const auto text = MidiMonitorNode::describe (MidiMessage::noteOn (1, 60, 1.0f)); + BOOST_REQUIRE (text.startsWith ("Note On C4")); + BOOST_REQUIRE (text.contains ("(60)")); + BOOST_REQUIRE (text.contains ("Channel 1")); +} + +BOOST_AUTO_TEST_CASE (NoteNamesAtOctaveEdges) +{ + BOOST_REQUIRE (MidiMonitorNode::describe (MidiMessage::noteOn (1, 21, 1.0f)) + .startsWith ("Note On A0")); + BOOST_REQUIRE (MidiMonitorNode::describe (MidiMessage::noteOn (1, 0, 1.0f)) + .startsWith ("Note On C-1")); + BOOST_REQUIRE (MidiMonitorNode::describe (MidiMessage::noteOn (1, 127, 1.0f)) + .startsWith ("Note On G9")); +} + +BOOST_AUTO_TEST_CASE (NoteOff) +{ + const auto text = MidiMonitorNode::describe (MidiMessage::noteOff (2, 60)); + BOOST_REQUIRE (text.startsWith ("Note Off C4")); + BOOST_REQUIRE (text.contains ("Channel 2")); +} + +BOOST_AUTO_TEST_CASE (ClockIsNotLogged) +{ + BOOST_REQUIRE (MidiMonitorNode::describe (MidiMessage::midiClock()).isEmpty()); +} + +BOOST_AUTO_TEST_CASE (TransportMessagesLogOnce) +{ + BOOST_REQUIRE_EQUAL (MidiMonitorNode::describe (MidiMessage::midiStart()).toStdString(), "Start"); + BOOST_REQUIRE_EQUAL (MidiMonitorNode::describe (MidiMessage::midiStop()).toStdString(), "Stop"); + BOOST_REQUIRE_EQUAL (MidiMonitorNode::describe (MidiMessage::midiContinue()).toStdString(), "Continue"); +} + +BOOST_AUTO_TEST_CASE (AgreesWithNoteValueConversions) +{ + BOOST_REQUIRE_EQUAL (Util::middleCOctave, 4); + BOOST_REQUIRE_EQUAL (Util::noteValueToString (60).toStdString(), "C4"); + BOOST_REQUIRE_EQUAL (Util::noteValueToString (21).toStdString(), "A0"); + BOOST_REQUIRE_EQUAL (Util::noteValueFromString ("C4"), 60); + BOOST_REQUIRE_EQUAL (Util::noteValueFromString ("A0"), 21); + BOOST_REQUIRE_EQUAL (Util::noteValueFromString ("C-1"), 0); + BOOST_REQUIRE_EQUAL (Util::noteValueFromString ("G9"), 127); +} + +BOOST_AUTO_TEST_SUITE_END()