fixup! refactor: move unique_ptr::get() to improve usability

src/test/effects/metronomeeffect_test.cpp

@@ -0,0 +1,165 @@
+#include "effects/backends/builtin/metronomeeffect.h"
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include <algorithm>
+#include <numeric>
+#include <optional>
+
+#include "audio/types.h"
+#include "effects/backends/builtin/metronomeclick.h"
+#include "effects/backends/effectmanifest.h"
+#include "effects/defs.h"
+#include "engine/effects/engineeffectparameter.h"
+#include "engine/effects/groupfeaturestate.h"
+#include "engine/engine.h"
+#include "gmock/gmock.h"
+#include "util/samplebuffer.h"
+#include "util/types.h"
+
+using namespace Qt::Literals;
+
+namespace {
+
+constexpr SINT kBufferSize = 256;
+
+constexpr auto silenceStereo = [] {
+    std::array<CSAMPLE, kBufferSize * mixxx::kEngineChannelOutputCount> buffer;
+    std::fill(buffer.begin(), buffer.end(), 0.0f);
+    return buffer;
+}();
+
+} // namespace
+
+class MetronomeEffectTest : public ::testing::Test {
+  protected:
+    MetronomeEffectTest()
+            : m_effect(),
+              m_engineParameters(mixxx::audio::SampleRate(44100), kBufferSize),
+              m_state(m_engineParameters),
+              m_output(kBufferSize * mixxx::kEngineChannelOutputCount) {
+        // TODO initialize?
+        // m_effect.initialize(const QSet<ChannelHandleAndGroup>
+        // &activeInputChannels, const QSet<ChannelHandleAndGroup>
+        // &registeredOutputChannels, const mixxx::EngineParameters
+        // &engineParameters)
+        const auto parameters = m_effect.getManifest()->parameters();
+        for (const auto& param : parameters) {
+            m_parameters.insert(param->id(), EngineEffectParameterPointer::create(param));
+        }
+        m_effect.loadEngineEffectParameters(m_parameters);
+    }
+
+    MetronomeEffect m_effect;
+    mixxx::EngineParameters m_engineParameters;
+    MetronomeGroupState m_state;
+    mixxx::SampleBuffer m_output;
+    QMap<QString, EngineEffectParameterPointer> m_parameters;
+
+    std::span<const CSAMPLE> getClick() {
+        return clickForSampleRate(m_engineParameters.sampleRate());
+    }
+    void assertOutputMatchesReference(
+            std::span<const CSAMPLE> reference, std::size_t outputOffset = 0) {
+        for (SINT i = 0; i < kBufferSize; ++i) {
+            EXPECT_THAT(m_output.span().subspan(outputOffset *
+                                mixxx::kEngineChannelOutputCount)[i *
+                                mixxx::kEngineChannelOutputCount],
+                    testing::FloatEq(reference[i]));
+            EXPECT_THAT(m_output.span().subspan(outputOffset *
+                                mixxx::kEngineChannelOutputCount)[i *
+                                        mixxx::kEngineChannelOutputCount +
+                                1],
+                    testing::FloatEq(reference[i]));
+        }
+    }
+    template<typename T>
+    void setParameterValue(const QString& id, T value) {
+        m_parameters.value(id)->setValue(static_cast<double>(value));
+    }
+    void processOnSilence(const GroupFeatureState& groupFeatures,
+            EffectEnableState enableState = EffectEnableState::Enabled) {
+        m_effect.processChannel(&m_state,
+                silenceStereo.data(),
+                m_output.data(),
+                m_engineParameters,
+                enableState,
+                groupFeatures);
+    }
+};
+
+// Example test case
+TEST_F(MetronomeEffectTest, ClickEnabledOnSilence) {
+    setParameterValue(u"sync"_s, false);
+    // ensure the effect has gotten a chance to initialize itself
+    processOnSilence({}, EffectEnableState::Enabling);
+    // ensure click is played on enabled
+    assertOutputMatchesReference(getClick());
+    // ensure click continues to play
+    processOnSilence({});
+    assertOutputMatchesReference(getClick().subspan(kBufferSize));
+}
+
+TEST_F(MetronomeEffectTest, SyncedClick) {
+    setParameterValue(u"sync"_s, true);
+    GroupFeatureState groupFeatures = {
+            .beat_length = std::make_optional<GroupFeatureBeatLength>(
+                    // one click every buffer
+                    {.frames = static_cast<double>(
+                             m_engineParameters.framesPerBuffer()),
+                            .scratch_rate = 1.0}),
+            .beat_fraction_buffer_end = 0,
+            .gain = 1.0};
+    processOnSilence(groupFeatures, EffectEnableState::Enabling);
+    m_state.m_framesSinceClickStart = 0; // ugly hack to ensure the next click
+                                         // lines up with the buffer beginning
+    processOnSilence(groupFeatures);
+    assertOutputMatchesReference(getClick());
+}
+
+TEST_F(MetronomeEffectTest, zeroScratchRate) {
+    setParameterValue(u"sync"_s, true);
+    GroupFeatureState groupFeatures = {
+            .beat_length = std::make_optional<GroupFeatureBeatLength>(
+                    // one click every ten buffers
+                    {.frames = static_cast<double>(
+                                       m_engineParameters.framesPerBuffer()) *
+                                    10,
+                            .scratch_rate = 0.0}),
+            .beat_fraction_buffer_end = 0.1,
+            .gain = 1.0};
+    // initialize
+    processOnSilence(groupFeatures, EffectEnableState::Enabling);
+    processOnSilence(groupFeatures);
+    // still no click because due to scratching effectively halting the
+    // transport, we can't predict when the click would be
+    assertOutputMatchesReference(silenceStereo);
+}
+
+TEST_F(MetronomeEffectTest, middleBeatSilence) {
+    setParameterValue(u"sync"_s, true);
+    GroupFeatureState groupFeatures = {
+            .beat_length = std::make_optional<GroupFeatureBeatLength>(
+                    {.frames = static_cast<double>(getClick().size() * 4),
+                            .scratch_rate = 1.0}),
+            .beat_fraction_buffer_end = 0.2,
+            .gain = 1.0};
+    processOnSilence(groupFeatures, EffectEnableState::Enabling);
+    *groupFeatures.beat_fraction_buffer_end += 0.1; // progress buffer by some (arbitrary) amount
+    qWarning() << m_state.m_framesSinceClickStart;
+    processOnSilence(groupFeatures);
+    // no click because we're currently in the middle of a beat that is far larger than the click
+    assertOutputMatchesReference(silenceStereo);
+}
+
+TEST_F(MetronomeEffectTest, negativeFractionalScratchRate) {
+    setParameterValue(u"sync"_s, true);
+    processOnSilence({.beat_length = std::make_optional<GroupFeatureBeatLength>(
+                              {.frames = 1000, .scratch_rate = -0.5}),
+            .beat_fraction_buffer_end = 0.05,
+            .gain = 1.0});
+    // 1/20th of 1000 frames is 20 frames, since this is played backward with 0.5 rate
+    // the click should start 40 frames into the buffer
+    assertOutputMatchesReference(silenceStereo, 40);
+}

src/test/wpushbutton_test.cpp

@@ -8,6 +8,7 @@
 #include "control/controlpushbutton.h"
 #include "util/valuetransformer.h"
 #include "widget/controlwidgetconnection.h"
+#include "widget/wbasewidget.h"
 
 class WPushButtonTest : public ::testing::Test {
   public:
@@ -22,13 +23,14 @@ class WPushButtonTest : public ::testing::Test {
     WPushButtonTest() {
         pushControl.setButtonMode(mixxx::control::ButtonMode::LongPressLatching);
         pushButton.setStates(2);
-        pushButton.addLeftConnection(
+        pushButton.addConnection(
                 std::make_unique<ControlParameterWidgetConnection>(
                         &pushButton,
                         pushControl.getKey(),
                         nullptr,
                         ControlParameterWidgetConnection::DIR_FROM_AND_TO_WIDGET,
-                        ControlParameterWidgetConnection::EMIT_ON_PRESS_AND_RELEASE));
+                        ControlParameterWidgetConnection::EMIT_ON_PRESS_AND_RELEASE),
+                WBaseWidget::ConnectionSide::Left);
     }
 };
 

src/widget/whotcuebutton.cpp

@@ -8,6 +8,7 @@
 #include "track/track.h"
 #include "util/valuetransformer.h"
 #include "widget/controlwidgetconnection.h"
+#include "widget/wbasewidget.h"
 
 namespace {
 constexpr int kDefaultDimBrightThreshold = 127;
@@ -69,21 +70,21 @@ void WHotcueButton::setup(const QDomNode& node, const SkinContext& context) {
     m_pCoType->connectValueChanged(this, &WHotcueButton::slotTypeChanged);
     slotTypeChanged(m_pCoType->get());
 
-    addLeftConnection(std::make_unique<ControlParameterWidgetConnection>(
-            this,
-            getLeftClickConfigKey(), // "activate"
-            nullptr,
-            ControlParameterWidgetConnection::DIR_FROM_WIDGET,
-            ControlParameterWidgetConnection::EMIT_ON_PRESS_AND_RELEASE));
-
-    auto pDisplayConnection = std::make_unique<ControlParameterWidgetConnection>(
-            this,
-            createConfigKey(QStringLiteral("status")),
-            nullptr,
-            ControlParameterWidgetConnection::DIR_TO_WIDGET,
-            ControlParameterWidgetConnection::EMIT_NEVER);
-    setDisplayConnection(pDisplayConnection.get());
-    addConnection(std::move(pDisplayConnection));
+    addConnection(std::make_unique<ControlParameterWidgetConnection>(
+                          this,
+                          getLeftClickConfigKey(), // "activate"
+                          nullptr,
+                          ControlParameterWidgetConnection::DIR_FROM_WIDGET,
+                          ControlParameterWidgetConnection::EMIT_ON_PRESS_AND_RELEASE),
+            WBaseWidget::ConnectionSide::Left);
+
+    setDisplayConnection(std::make_unique<ControlParameterWidgetConnection>(
+                                 this,
+                                 createConfigKey(QStringLiteral("status")),
+                                 nullptr,
+                                 ControlParameterWidgetConnection::DIR_TO_WIDGET,
+                                 ControlParameterWidgetConnection::EMIT_NEVER),
+            WBaseWidget::ConnectionSide::None);
 
     QDomNode con = context.selectNode(node, QStringLiteral("Connection"));
     if (!con.isNull()) {