Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Event queue compressing proxy #4566

Merged
merged 12 commits into from
Dec 20, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL
src/control/control.cpp
src/control/controlaudiotaperpot.cpp
src/control/controlbehavior.cpp
src/control/controlcompressingproxy.cpp
src/control/controleffectknob.cpp
src/control/controlencoder.cpp
src/control/controlindicator.cpp
Expand Down Expand Up @@ -1589,6 +1590,7 @@ add_executable(mixxx-test
src/test/controller_mapping_validation_test.cpp
src/test/controllerscriptenginelegacy_test.cpp
src/test/controlobjecttest.cpp
src/test/controlobjectscripttest.cpp
src/test/coreservicestest.cpp
src/test/coverartcache_test.cpp
src/test/coverartutils_test.cpp
Expand Down
39 changes: 39 additions & 0 deletions src/control/controlcompressingproxy.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
#include "control/controlcompressingproxy.h"

#include "moc_controlcompressingproxy.cpp"

// Event queue compressing proxy
CompressingProxy::CompressingProxy(QObject* parent)
: QObject(parent) {
}

// This function is called recursive by QCoreApplication::sendPostedEvents, until no more events are in the queue.
// When the last event is found, QCoreApplication::sendPostedEvents returns for the processQueuedEvents() instance of this event,
// and m_recursiveSearchForLastEventOngoing is set to false, while returning true itself.
// All previous started instances of processQueuedEvents() will return false in consequence,
// because the return value depends on the member variable and not on the stack of the instance.
stateOfprocessQueuedEvent CompressingProxy::processQueuedEvents() {
m_recursiveSearchForLastEventOngoing = true;
JoergAtGithub marked this conversation as resolved.
Show resolved Hide resolved

// sendPostedEvents recursive executes slotValueChanged until no more events for this slot are in the queue
// Each call of QCoreApplication::sendPostedEvents triggers the processing of the next event in the queue,
// by sending the signal to execute slotValueChanged again
QCoreApplication::sendPostedEvents(this, QEvent::MetaCall);

// Execution continues here, when last event for this slot is processed

stateOfprocessQueuedEvent returnValue;
if (m_recursiveSearchForLastEventOngoing == true) {
JoergAtGithub marked this conversation as resolved.
Show resolved Hide resolved
returnValue = stateOfprocessQueuedEvent::last_event;
} else {
returnValue = stateOfprocessQueuedEvent::outdated_event;
}
m_recursiveSearchForLastEventOngoing = false;
return returnValue;
}

void CompressingProxy::slotValueChanged(double value, QObject* obj) {
if (processQueuedEvents() == stateOfprocessQueuedEvent::last_event) {
emit signalValueChanged(value, obj);
}
}
26 changes: 26 additions & 0 deletions src/control/controlcompressingproxy.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#pragma once

#include <QApplication>
#include <QMetaObject>

enum class stateOfprocessQueuedEvent { last_event,
outdated_event };

class CompressingProxy : public QObject {
Q_OBJECT
private:
stateOfprocessQueuedEvent processQueuedEvents();

bool m_recursiveSearchForLastEventOngoing;

public slots:
void slotValueChanged(double value, QObject* obj);

signals:
void signalValueChanged(double, QObject*);

public:
// No default constructor, since the proxy must be a child of the
// target object.
explicit CompressingProxy(QObject* parent);
};
27 changes: 19 additions & 8 deletions src/control/controlobjectscript.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,21 @@ ControlObjectScript::ControlObjectScript(
m_logger(logger) {
}

bool ControlObjectScript::addScriptConnection(const ScriptConnection& conn) {
bool ControlObjectScript::addScriptConnection(ScriptConnection* const conn) {
if (m_scriptConnections.isEmpty()) {
// Only connect the slots when they are actually needed
// by script connections.
conn->proxy = new CompressingProxy(this);
connect(m_pControl.data(),
&ControlDoublePrivate::valueChanged,
conn->proxy,
&CompressingProxy::slotValueChanged,
Qt::QueuedConnection);
connect(conn->proxy,
&CompressingProxy::signalValueChanged,
this,
&ControlObjectScript::slotValueChanged,
Qt::QueuedConnection);
Qt::DirectConnection);
connect(this,
&ControlObjectScript::trigger,
this,
Expand All @@ -25,19 +31,19 @@ bool ControlObjectScript::addScriptConnection(const ScriptConnection& conn) {
}

for (const auto& priorConnection : qAsConst(m_scriptConnections)) {
if (conn == priorConnection) {
qCWarning(m_logger) << "Connection " + conn.id.toString() +
if (*conn == priorConnection) {
qCWarning(m_logger) << "Connection " + conn->id.toString() +
" already connected to (" +
conn.key.group + ", " + conn.key.item +
conn->key.group + ", " + conn->key.item +
"). Ignoring attempt to connect again.";
return false;
}
}

m_scriptConnections.append(conn);
m_scriptConnections.append(*conn);
qCDebug(m_logger) << "Connected (" +
conn.key.group + ", " + conn.key.item +
") to connection " + conn.id.toString();
conn->key.group + ", " + conn->key.item +
") to connection " + conn->id.toString();
return true;
}

Expand All @@ -56,12 +62,17 @@ bool ControlObjectScript::removeScriptConnection(const ScriptConnection& conn) {
// no ScriptConnections left, so disconnect signals
disconnect(m_pControl.data(),
&ControlDoublePrivate::valueChanged,
conn.proxy,
&CompressingProxy::slotValueChanged);
disconnect(conn.proxy,
&CompressingProxy::signalValueChanged,
this,
&ControlObjectScript::slotValueChanged);
disconnect(this,
&ControlObjectScript::trigger,
this,
&ControlObjectScript::slotValueChanged);
delete (conn.proxy);
JoergAtGithub marked this conversation as resolved.
Show resolved Hide resolved
}
return success;
}
Expand Down
4 changes: 2 additions & 2 deletions src/control/controlobjectscript.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class ControlObjectScript : public ControlProxy {
const RuntimeLoggingCategory& logger,
QObject* pParent = nullptr);

bool addScriptConnection(const ScriptConnection& conn);
bool addScriptConnection(ScriptConnection* const conn);
JoergAtGithub marked this conversation as resolved.
Show resolved Hide resolved

bool removeScriptConnection(const ScriptConnection& conn);

Expand All @@ -36,7 +36,7 @@ class ControlObjectScript : public ControlProxy {

protected slots:
// Receives the value from the master control by a unique queued connection
void slotValueChanged(double v, QObject*);
virtual void slotValueChanged(double v, QObject*);
JoergAtGithub marked this conversation as resolved.
Show resolved Hide resolved

private:
QVector<ScriptConnection> m_scriptConnections;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ QJSValue ControllerScriptInterfaceLegacy::makeConnection(
connection.callback = callback;
connection.id = QUuid::createUuid();

if (coScript->addScriptConnection(connection)) {
if (coScript->addScriptConnection(&connection)) {
return pJsEngine->newQObject(
new ScriptConnectionJSProxy(connection));
}
Expand Down
2 changes: 2 additions & 0 deletions src/controllers/scripting/legacy/scriptconnection.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <QJSValue>
#include <QUuid>

#include "control/controlcompressingproxy.h"
#include "preferences/configobject.h"

class ControllerScriptEngineLegacy;
Expand All @@ -18,6 +19,7 @@ class ScriptConnection {
QJSValue callback;
ControllerScriptInterfaceLegacy* engineJSProxy;
ControllerScriptEngineLegacy* controllerEngine;
CompressingProxy* proxy;
JoergAtGithub marked this conversation as resolved.
Show resolved Hide resolved

void executeCallback(double value) const;

Expand Down
147 changes: 147 additions & 0 deletions src/test/controlobjectscripttest.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
#include <gmock/gmock.h>
#include <gtest/gtest.h>

#include <QtDebug>

#include "control/controlobject.h"
#include "control/controlobjectscript.h"
#include "test/mixxxtest.h"
#include "util/memory.h"

using ::testing::_;
using ::testing::DoubleEq;
using ::testing::Invoke;
using ::testing::Return;
using ::testing::StrictMock;

namespace {

const RuntimeLoggingCategory k_logger(QString("test").toLocal8Bit());

class MockControlObjectScript : public ControlObjectScript {
public:
MockControlObjectScript(const ConfigKey& key,
const RuntimeLoggingCategory& logger,
QObject* pParent)
: ControlObjectScript(key, logger, pParent) {
}
~MockControlObjectScript() override = default;
MOCK_METHOD2(slotValueChanged, void(double value, QObject*));
};

class ControlObjectScriptTest : public MixxxTest {
protected:
void SetUp() override {
ck1 = ConfigKey("[Channel1]", "co1");
ck2 = ConfigKey("[Channel1]", "co2");
co1 = std::make_unique<ControlObject>(ck1);
co2 = std::make_unique<ControlObject>(ck2);

coScript1 = std::make_unique<MockControlObjectScript>(ck1, k_logger, nullptr);
coScript2 = std::make_unique<MockControlObjectScript>(ck2, k_logger, nullptr);

conn1.key = ck1;
conn1.engineJSProxy = nullptr;
conn1.controllerEngine = nullptr;
conn1.callback = "mock_callback1";
conn1.id = QUuid::createUuid();

conn2.key = ck2;
conn2.engineJSProxy = nullptr;
conn2.controllerEngine = nullptr;
conn2.callback = "mock_callback2";
conn2.id = QUuid::createUuid();

coScript1->addScriptConnection(&conn1);
coScript2->addScriptConnection(&conn2);
}

void TearDown() override {
coScript1->removeScriptConnection(conn1);
coScript2->removeScriptConnection(conn2);
}

ConfigKey ck1, ck2;
std::unique_ptr<ControlObject> co1;
std::unique_ptr<ControlObject> co2;

std::unique_ptr<MockControlObjectScript> coScript1;
std::unique_ptr<MockControlObjectScript> coScript2;

ScriptConnection conn1, conn2;
};

TEST_F(ControlObjectScriptTest, CompressingProxyCompareCount1) {
// Check that slotValueChanged callback is never called for conn2
EXPECT_CALL(*coScript2, slotValueChanged(_, _))
.Times(0)
.WillOnce(Return());
// Check that slotValueChanged callback is called only once (independent of the value)
EXPECT_CALL(*coScript1, slotValueChanged(_, _))
.Times(1)
.WillOnce(Return());
co1->set(1.0);
application()->processEvents();
}

TEST_F(ControlObjectScriptTest, CompressingProxyCompareValue1) {
EXPECT_CALL(*coScript1, slotValueChanged(2.0, _))
.Times(1)
.WillOnce(Return());

co1->set(2.0);
application()->processEvents();
}

TEST_F(ControlObjectScriptTest, CompressingProxyCompareCount2) {
// Check that slotValueChanged callback is never called for conn2
EXPECT_CALL(*coScript2, slotValueChanged(_, _))
.Times(0)
.WillOnce(Return());
// Check that slotValueChanged callback for conn1 is called only once (independent of the value)
EXPECT_CALL(*coScript1, slotValueChanged(_, _))
.Times(1)
.WillOnce(Return());
co1->set(3.0);
co1->set(4.0);
application()->processEvents();
}

TEST_F(ControlObjectScriptTest, CompressingProxyCompareValue2) {
// Check that slotValueChanged callback is called with the last set value
EXPECT_CALL(*coScript1, slotValueChanged(6.0, _))
.Times(1)
.WillOnce(Return());
co1->set(5.0);
co1->set(6.0);
application()->processEvents();
}

TEST_F(ControlObjectScriptTest, CompressingProxyCompareCountMulti) {
// Check that slotValueChanged callback for conn1 and conn2 is called only once (independent of the value)
EXPECT_CALL(*coScript1, slotValueChanged(_, _)).Times(1).WillOnce(Return());
// Check that slotValueChanged callback for conn1 and conn2 is called only once (independent of the value)
EXPECT_CALL(*coScript2, slotValueChanged(_, _)).Times(1).WillOnce(Return());
co1->set(10.0);
co2->set(11.0);
co1->set(12.0);
co2->set(13.0);
application()->processEvents();
}

TEST_F(ControlObjectScriptTest, CompressingProxyCompareValueMulti) {
// Check that slotValueChanged callback is called with the last set value
EXPECT_CALL(*coScript1, slotValueChanged(22.0, _))
.Times(1)
.WillOnce(Return());
EXPECT_CALL(*coScript2, slotValueChanged(23.0, _))
.Times(1)
.WillOnce(Return());
co1->set(20.0);
co2->set(21.0);
co1->set(22.0);
co2->set(23.0);
application()->processEvents();
}

} // namespace