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

[DONE] Add indication whether controller callback was successfully disconnected #2054

Merged
merged 5 commits into from
May 2, 2019
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
3 changes: 2 additions & 1 deletion src/control/controlobjectscript.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ bool ControlObjectScript::addScriptConnection(const ScriptConnection& conn) {
return true;
}

void ControlObjectScript::removeScriptConnection(const ScriptConnection& conn) {
bool ControlObjectScript::removeScriptConnection(const ScriptConnection& conn) {
bool success = m_scriptConnections.removeOne(conn);
if (success) {
controllerDebug("Disconnected (" +
Expand All @@ -53,6 +53,7 @@ void ControlObjectScript::removeScriptConnection(const ScriptConnection& conn) {
disconnect(this, SIGNAL(trigger(double, QObject*)),
this, SLOT(slotValueChanged(double,QObject*)));
}
return success;
}

void ControlObjectScript::disconnectAllConnectionsToFunction(const QScriptValue& function) {
Expand Down
2 changes: 1 addition & 1 deletion src/control/controlobjectscript.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class ControlObjectScript : public ControlProxy {

bool addScriptConnection(const ScriptConnection& conn);

void removeScriptConnection(const ScriptConnection& conn);
bool removeScriptConnection(const ScriptConnection& conn);

// Required for legacy behavior of ControllerEngine::connectControl
inline int countConnections() {
Expand Down
13 changes: 8 additions & 5 deletions src/controllers/controllerengine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -764,19 +764,22 @@ void ScriptConnection::executeCallback(double value) const {
Purpose: (Dis)connects a ScriptConnection
Input: the ScriptConnection to disconnect
-------- ------------------------------------------------------ */
void ControllerEngine::removeScriptConnection(const ScriptConnection connection) {
bool ControllerEngine::removeScriptConnection(const ScriptConnection connection) {
ControlObjectScript* coScript = getControlObjectScript(connection.key.group,
connection.key.item);

if (m_pEngine == nullptr || coScript == nullptr) {
return;
return false;
}

coScript->removeScriptConnection(connection);
return coScript->removeScriptConnection(connection);
}

void ScriptConnectionInvokableWrapper::disconnect() {
m_scriptConnection.controllerEngine->removeScriptConnection(m_scriptConnection);
bool ScriptConnectionInvokableWrapper::disconnect() {
// if the removeScriptConnection succeeded, the connection has been successfully disconnected
bool success = m_scriptConnection.controllerEngine->removeScriptConnection(m_scriptConnection);
Swiftb0y marked this conversation as resolved.
Show resolved Hide resolved
m_isConnected = !success;
return success;
}

/* -------- ------------------------------------------------------
Expand Down
8 changes: 6 additions & 2 deletions src/controllers/controllerengine.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,18 +59,22 @@ class ScriptConnectionInvokableWrapper : public QObject {
//Q_PROPERTY(ConfigKey key READ key)
// There's little use in exposing the function...
//Q_PROPERTY(QScriptValue function READ function)
Q_PROPERTY(bool isConnected READ readIsConnected)
public:
ScriptConnectionInvokableWrapper(ScriptConnection conn) {
m_scriptConnection = conn;
m_idString = conn.id.toString();
m_isConnected = true;
}
const QString& readId() const { return m_idString; }
Q_INVOKABLE void disconnect();
bool readIsConnected() const { return m_isConnected; }
Q_INVOKABLE bool disconnect();
Q_INVOKABLE void trigger();

private:
ScriptConnection m_scriptConnection;
QString m_idString;
bool m_isConnected;
};

class ControllerEngine : public QObject {
Expand Down Expand Up @@ -99,7 +103,7 @@ class ControllerEngine : public QObject {
const QList<QString>& getScriptFunctionPrefixes() { return m_scriptFunctionPrefixes; };

// Disconnect a ScriptConnection
void removeScriptConnection(const ScriptConnection conn);
bool removeScriptConnection(const ScriptConnection conn);
void triggerScriptConnection(const ScriptConnection conn);

protected:
Expand Down
27 changes: 27 additions & 0 deletions src/test/controllerengine_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,32 @@ TEST_F(ControllerEngineTest, connectionObject_Disconnect) {
// The counter should have been incremented exactly once.
EXPECT_DOUBLE_EQ(1.0, pass->get());
}
TEST_F(ControllerEngineTest, connectionObject_reflectDisconnect) {
// Test that checks if disconnecting yields the appropriate feedback
auto co = std::make_unique<ControlObject>(ConfigKey("[Test]", "co"));
auto pass = std::make_unique<ControlObject>(ConfigKey("[Test]", "passed"));

ScopedTemporaryFile script(makeTemporaryFile(
"var reaction = function(success) { "
" if (success) {"
" var pass = engine.getValue('[Test]', 'passed');"
" engine.setValue('[Test]', 'passed', pass + 1.0); "
" }"
"};"
"var dummy_callback = function(value) {};"
"var connection = engine.makeConnection('[Test]', 'co', dummy_callback);"
"reaction(connection);"
"reaction(connection.isConnected);"
"var successful_disconnect = connection.disconnect();"
"reaction(successful_disconnect);"
"reaction(!connection.isConnected);"
));
cEngine->evaluate(script->fileName());
EXPECT_FALSE(cEngine->hasErrors(script->fileName()));
application()->processEvents();
EXPECT_DOUBLE_EQ(4.0, pass->get());
}


TEST_F(ControllerEngineTest, connectionObject_DisconnectByPassingToConnectControl) {
// Test that passing a connection object back to engine.connectControl
Expand Down Expand Up @@ -563,6 +589,7 @@ TEST_F(ControllerEngineTest, connectionObject_trigger) {
EXPECT_DOUBLE_EQ(1.0, counter->get());
}


TEST_F(ControllerEngineTest, connectionExecutesWithCorrectThisObject) {
// Test that callback functions are executed with JavaScript's
// 'this' keyword referring to the object in which the connection
Expand Down