Skip to content

Commit

Permalink
Made CompressingProxy by default disabled. Now you need to specify th…
Browse files Browse the repository at this point in the history 
…e additional boolean argument skipSuperseded as true, to enable it.

In case of multiple callback functions connected to the same CO, with contradicting state for skipSuperseded, the CompressingProxy will be disabled and an warning message is printed.
  • Loading branch information
@JoergAtGithub
JoergAtGithub committed Dec 20, 2021
1 parent 9eab7d9 commit aba7730
Show file tree
Hide file tree
Showing 6 changed files with 142 additions and 23 deletions.
76 changes: 57 additions & 19 deletions src/control/controlobjectscript.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,59 @@ ControlObjectScript::ControlObjectScript(
const ConfigKey& key, const RuntimeLoggingCategory& logger, QObject* pParent)
: ControlProxy(key, pParent, ControlFlag::AllowMissingOrInvalid),
m_logger(logger),
m_proxy(key, logger, this) {
m_proxy(key, logger, this),
m_skipSuperseded(false) {
}

bool ControlObjectScript::addScriptConnection(const ScriptConnection& conn) {
if (m_scriptConnections.isEmpty()) {
// Only connect the slots when they are actually needed
// by script connections.
connect(m_pControl.data(),
&ControlDoublePrivate::valueChanged,
&m_proxy,
&CompressingProxy::slotValueChanged,
Qt::QueuedConnection);
connect(&m_proxy,
&CompressingProxy::signalValueChanged,
this,
&ControlObjectScript::slotValueChanged,
Qt::DirectConnection);
m_skipSuperseded = conn.skipSuperseded;
if (conn.skipSuperseded) {
connect(m_pControl.data(),
&ControlDoublePrivate::valueChanged,
&m_proxy,
&CompressingProxy::slotValueChanged,
Qt::QueuedConnection);
connect(&m_proxy,
&CompressingProxy::signalValueChanged,
this,
&ControlObjectScript::slotValueChanged,
Qt::DirectConnection);
} else {
connect(m_pControl.data(),
&ControlDoublePrivate::valueChanged,
this,
&ControlObjectScript::slotValueChanged,
Qt::QueuedConnection);
}
connect(this,
&ControlObjectScript::trigger,
this,
&ControlObjectScript::slotValueChanged,
Qt::QueuedConnection);
} else {
// At least one callback function is already connected to this CO
if (conn.skipSuperseded == false && m_skipSuperseded == true) {
// Disconnect proxy if this is first callback function connected with skipSuperseded false
m_skipSuperseded = false;
qCWarning(m_logger) << conn.key.group + ", " + conn.key.item +
"is connected to different callback functions with differing state of the skipSuperseded. Disable skipping of superseded events for all these callback functions.";
disconnect(m_pControl.data(),
&ControlDoublePrivate::valueChanged,
&m_proxy,
&CompressingProxy::slotValueChanged);
disconnect(&m_proxy,
&CompressingProxy::signalValueChanged,
this,
&ControlObjectScript::slotValueChanged);
connect(m_pControl.data(),
&ControlDoublePrivate::valueChanged,
this,
&ControlObjectScript::slotValueChanged,
Qt::QueuedConnection);
}
}

for (const auto& priorConnection : qAsConst(m_scriptConnections)) {
Expand Down Expand Up @@ -60,14 +91,21 @@ bool ControlObjectScript::removeScriptConnection(const ScriptConnection& conn) {
}
if (m_scriptConnections.isEmpty()) {
// no ScriptConnections left, so disconnect signals
disconnect(m_pControl.data(),
&ControlDoublePrivate::valueChanged,
&m_proxy,
&CompressingProxy::slotValueChanged);
disconnect(&m_proxy,
&CompressingProxy::signalValueChanged,
this,
&ControlObjectScript::slotValueChanged);
if (m_skipSuperseded) {
disconnect(m_pControl.data(),
&ControlDoublePrivate::valueChanged,
&m_proxy,
&CompressingProxy::slotValueChanged);
disconnect(&m_proxy,
&CompressingProxy::signalValueChanged,
this,
&ControlObjectScript::slotValueChanged);
} else {
disconnect(m_pControl.data(),
&ControlDoublePrivate::valueChanged,
this,
&ControlObjectScript::slotValueChanged);
}
disconnect(this,
&ControlObjectScript::trigger,
this,
Expand Down
1 change: 1 addition & 0 deletions src/control/controlobjectscript.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,5 @@ class ControlObjectScript : public ControlProxy {
QVector<ScriptConnection> m_scriptConnections;
const RuntimeLoggingCategory m_logger;
CompressingProxy m_proxy;
bool m_skipSuperseded; // This flag is combined for all connections of this Control Object
};
3 changes: 2 additions & 1 deletion src/controllers/scripting/legacy/controllerscriptinterfacelegacy.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ double ControllerScriptInterfaceLegacy::getDefaultParameter(
}

QJSValue ControllerScriptInterfaceLegacy::makeConnection(
const QString& group, const QString& name, const QJSValue& callback) {
const QString& group, const QString& name, const QJSValue& callback, bool skipSuperseded) {
auto pJsEngine = m_pScriptEngineLegacy->jsEngine();
VERIFY_OR_DEBUG_ASSERT(pJsEngine) {
return QJSValue();
Expand Down Expand Up @@ -245,6 +245,7 @@ QJSValue ControllerScriptInterfaceLegacy::makeConnection(
connection.controllerEngine = m_pScriptEngineLegacy;
connection.callback = callback;
connection.id = QUuid::createUuid();
connection.skipSuperseded = skipSuperseded;

if (coScript->addScriptConnection(connection)) {
return pJsEngine->newQObject(
Expand Down
2 changes: 1 addition & 1 deletion src/controllers/scripting/legacy/controllerscriptinterfacelegacy.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class ControllerScriptInterfaceLegacy : public QObject {
Q_INVOKABLE double getDefaultValue(const QString& group, const QString& name);
Q_INVOKABLE double getDefaultParameter(const QString& group, const QString& name);
Q_INVOKABLE QJSValue makeConnection(
const QString& group, const QString& name, const QJSValue& callback);
const QString& group, const QString& name, const QJSValue& callback, bool skipSuperseded = false);
// DEPRECATED: Use makeConnection instead.
Q_INVOKABLE QJSValue connectControl(const QString& group,
const QString& name,
Expand Down
1 change: 1 addition & 0 deletions src/controllers/scripting/legacy/scriptconnection.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class ScriptConnection {
QJSValue callback;
ControllerScriptInterfaceLegacy* engineJSProxy;
ControllerScriptEngineLegacy* controllerEngine;
bool skipSuperseded;

void executeCallback(double value) const;

Expand Down
82 changes: 80 additions & 2 deletions src/test/controlobjectscripttest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,47 +35,64 @@ class ControlObjectScriptTest : public MixxxTest {
void SetUp() override {
ck1 = ConfigKey("[Channel1]", "co1");
ck2 = ConfigKey("[Channel1]", "co2");
ck4 = ConfigKey("[Channel1]", "co4");
co1 = std::make_unique<ControlObject>(ck1);
co2 = std::make_unique<ControlObject>(ck2);
co4 = std::make_unique<ControlObject>(ck4);

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

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

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

conn3.key = ck2;
conn3.engineJSProxy = nullptr;
conn3.controllerEngine = nullptr;
conn3.callback = "mock_callback3";
conn3.id = QUuid::createUuid();
conn3.skipSuperseded = true;

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

coScript1->addScriptConnection(conn1);
coScript2->addScriptConnection(conn2);
coScript4->addScriptConnection(conn4);
}

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

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

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

ScriptConnection conn1, conn2, conn3;
ScriptConnection conn1, conn2, conn3, conn4;
};

TEST_F(ControlObjectScriptTest, CompressingProxyCompareCount1) {
Expand Down Expand Up @@ -124,6 +141,32 @@ TEST_F(ControlObjectScriptTest, CompressingProxyCompareValue2) {
application()->processEvents();
}

TEST_F(ControlObjectScriptTest, QueuedCompareCount2) {
// Check that slotValueChanged callback is never called for conn4
EXPECT_CALL(*coScript2, slotValueChanged(_, _))
.Times(0)
.WillOnce(Return());
// Check that slotValueChanged callback for conn1 is called only twice (independent of the value), because proxy is disabled
EXPECT_CALL(*coScript4, slotValueChanged(_, _))
.Times(2)
.WillOnce(Return());
co4->set(53.0);
co4->set(54.0);
application()->processEvents();
}

TEST_F(ControlObjectScriptTest, QueuedCompareValue2) {
// Check that slotValueChanged callback is called for each value, because proxy is disabled
EXPECT_CALL(*coScript4, slotValueChanged(55.0, _))
.Times(1)
.WillOnce(Return());
EXPECT_CALL(*coScript4, slotValueChanged(56.0, _))
.Times(1)
.WillOnce(Return());
co4->set(55.0);
co4->set(56.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());
Expand Down Expand Up @@ -170,6 +213,41 @@ TEST_F(ControlObjectScriptTest, CompressingProxyMultiConnection) {
coScript2->removeScriptConnection(conn3);
}

TEST_F(ControlObjectScriptTest, QueuedFallbackMultiConnection) {
// Check that slotValueChanged callback is called 1 time if multiple connections exist forthe same slot
EXPECT_CALL(*coScript1, slotValueChanged(62.0, _))
.Times(1)
.WillOnce(Return());
EXPECT_CALL(*coScript2, slotValueChanged(63.0, _))
.Times(1)
.WillOnce(Return());
EXPECT_CALL(*coScript1, slotValueChanged(66.0, _))
.Times(1)
.WillOnce(Return());
EXPECT_CALL(*coScript2, slotValueChanged(65.0, _))
.Times(1)
.WillOnce(Return());
EXPECT_CALL(*coScript2, slotValueChanged(67.0, _))
.Times(1)
.WillOnce(Return());

co1->set(60.0);
co2->set(61.0);
co1->set(62.0);
co2->set(63.0);
application()->processEvents();
conn3.skipSuperseded = false;
coScript2->addScriptConnection(conn3);
co1->set(64.0);
co2->set(65.0);
co1->set(66.0);
co2->set(67.0);
application()->processEvents();

coScript2->removeScriptConnection(conn3);
conn3.skipSuperseded = true;
}

TEST_F(ControlObjectScriptTest, CompressingProxyManyEvents) {
// Check maximum number of recursions
EXPECT_CALL(*coScript1, slotValueChanged(kMaxNumOfRecursions, _))
Expand Down

0 comments on commit aba7730

Please sign in to comment.