Merge lp:~mixxxdevelopers/mixxx/features_direct_bind into lp:~mixxxdevelopers/mixxx/trunk

[Phillip Whelan]

Call script functions directly as QScriptValues instead of parsing small javascript snippets for invoking MIDI Scripts.

[lp:~mixxxdevelopers/mixxx/features_direct_bind]

2875. By madjester <madjester@voidwalker> on 2012-05-05

Change declaration of ControllerEngine::timerEvent to match trunk.

[Phillip Whelan (pwhelan)]

The runtime evaluation option and file reloading are in the works for after this is merged.

[RJ Skerry-Ryan (rryan)]

Thanks! -- here are my comments sofar:

* ControllerEngine::callFunctionOnObjects should check isFunction() before call()'ing the QScriptValue

* ControllerEngine::resolveFunction should check isFunction() in addition to isValid()

* In ControllerEngine::internalExecute(QScriptValue, QScriptValue) return false if !isFunction() to match execute(QScriptValue, QScriptValueList)

* In ControllerEngine you changed a bunch of debug outputs to say MidiScriptEngine instead of ControllerEngine -- please change those back :)

* In connectControl() in the first if(disconnect) path you set cb.key = key which seems redundant with a few lines above.

* ControllerEngineConnectionScriptValue::disconnect() shadows QObject::disconnect(). We probably shouldn't do that.

* connectControl() connects the valueChanged() signal from the ControlObject, not the ControlObjectThread which causes priority-inversion between the Engine and the Qt main-thread. This was a bug that was fixed in the control-system rewrite so please re-add that.

* MidiController::bindScriptFunction() calling preset() and casting the result -- that's for use by the superclass which doesn't know about MidiControllerPresets. You can just use m_preset. (though see my comment below)

* I think the m_scriptBindings cache in Controller/MidiController isn't really necessary. Instead of the cache living in MixxxControl/Controller and having to pre-cache every function we think we might want to call, why not put the cache in ControllerEngine::resolveFunction. Change ControllerEngine::resolveFunction(QString) to take a boolean shouldCache flag that tells it to first look in the QScriptValue cache and then just call resolveFunction() every time we want to call a method. I think this would be a lot cleaner and this branch would become almost totally contained to ControllerEngine and it would be just as fast as what is written now (and a little more robust to cache misses since it will cache on first use).

* thanks for the tests :)

review: Needs Fixing

[RJ Skerry-Ryan (rryan)]

Any update Phil? If you're too hosed I can make these changes myself --
it'd be great to get these in 1.11.

On Fri, May 11, 2012 at 2:14 PM, RJ Ryan <email address hidden> wrote:

> Review: Needs Fixing
>
> Thanks! -- here are my comments sofar:
>
> * ControllerEngine::callFunctionOnObjects should check isFunction() before
> call()'ing the QScriptValue
>
> * ControllerEngine::resolveFunction should check isFunction() in addition
> to isValid()
>
> * In ControllerEngine::internalExecute(QScriptValue, QScriptValue) return
> false if !isFunction() to match execute(QScriptValue, QScriptValueList)
>
> * In ControllerEngine you changed a bunch of debug outputs to say
> MidiScriptEngine instead of ControllerEngine -- please change those back :)
>
> * In connectControl() in the first if(disconnect) path you set cb.key =
> key which seems redundant with a few lines above.
>
> * ControllerEngineConnectionScriptValue::disconnect() shadows
> QObject::disconnect(). We probably shouldn't do that.
>
> * connectControl() connects the valueChanged() signal from the
> ControlObject, not the ControlObjectThread which causes priority-inversion
> between the Engine and the Qt main-thread. This was a bug that was fixed in
> the control-system rewrite so please re-add that.
>
> * MidiController::bindScriptFunction() calling preset() and casting the
> result -- that's for use by the superclass which doesn't know about
> MidiControllerPresets. You can just use m_preset. (though see my comment
> below)
>
> * I think the m_scriptBindings cache in Controller/MidiController isn't
> really necessary. Instead of the cache living in MixxxControl/Controller
> and having to pre-cache every function we think we might want to call, why
> not put the cache in ControllerEngine::resolveFunction. Change
> ControllerEngine::resolveFunction(QString) to take a boolean shouldCache
> flag that tells it to first look in the QScriptValue cache and then just
> call resolveFunction() every time we want to call a method. I think this
> would be a lot cleaner and this branch would become almost totally
> contained to ControllerEngine and it would be just as fast as what is
> written now (and a little more robust to cache misses since it will cache
> on first use).
>
> * thanks for the tests :)
> --
>
> https://code.launchpad.net/~mixxxdevelopers/mixxx/features_direct_bind/+merge/104529
> You are reviewing the proposed merge of
> lp:~mixxxdevelopers/mixxx/features_direct_bind into lp:mixxx.
>

[Phillip Whelan (pwhelan)]

I'll see if I can get to them tomorrow. Been a bit busy lately with RL.

[lp:~mixxxdevelopers/mixxx/features_direct_bind]

2876. By madjester <madjester@voidwalker> on 2012-05-07

merge from trunk.

2877. By madjester <madjester@voidwalker> on 2012-05-07

merge from trunk.

2878. By madjester <madjester@voidwalker> on 2012-05-09

merge from trunk.

2879. By madjester <madjester@voidwalker> on 2012-05-09

merge from trunk.

2880. By madjester <madjester@voidwalker> on 2012-05-25

merge from trunk.

2881. By madjester <madjester@voidwalker> on 2012-05-25

FIX: check isFunction in ControllerEngine::callFunctionOnObjects before calling a QScriptValue.

2882. By madjester <madjester@voidwalker> on 2012-05-25

FIX: check if object.isFunction in ControllerEngine::resolveFunction.

2883. By madjester <madjester@voidwalker> on 2012-05-25

FIX: return false in ControllerEngine::internalExecute(QScriptValue thisObject, QScriptValue functionObject) if functionObject is not an object to match the calling convention of the other overloads.

2884. By madjester <madjester@voidwalker> on 2012-05-25

Change occurances of MidiScriptEngine to ControllerEngine.

2885. By madjester <madjester@voidwalker> on 2012-05-25

Remove redundant assignment of cb.key in ControllerEngine::connectControl.

2886. By madjester <madjester@voidwalker> on 2012-05-25

Remove m_scriptBindings and bindScriptFunctions() from controllers and use ControllerEngine::resolveFunction to resolve functions (no more caching in the controllers themselves).

2887. By madjester <madjester@voidwalker> on 2012-05-25

FIX: only check if the object is a function right before returning it in ControllerEngine::resolveFunction.

=== modified file 'mixxx/src/controllers/controller.cpp'
--- mixxx/src/controllers/controller.cpp	2012-04-29 02:08:20 +0000
+++ mixxx/src/controllers/controller.cpp	2012-05-25 04:46:23 +0000
@@ -70,14 +70,11 @@
 
     // Load the script code into the engine
     if (m_pEngine != NULL) {
-        const QStringList scriptFunctions = m_pEngine->getScriptFunctions();
-        if (scriptFunctions.isEmpty() && pPreset->scriptFileNames.isEmpty()) {
+        if (pPreset->scriptFileNames.isEmpty()) {
             qWarning() << "No script functions available! Did the XML file(s) load successfully? See above for any errors.";
-        } else {
-            if (scriptFunctions.isEmpty()) {
-                m_pEngine->loadScriptFiles(configPath,
-                                           pPreset->scriptFileNames);
-            }
+        }
+        else {
+            m_pEngine->loadScriptFiles(configPath, pPreset->scriptFileNames);
             m_pEngine->initializeScripts(pPreset->scriptFunctionPrefixes);
         }
     } else {
@@ -131,13 +128,17 @@
         qDebug() << message;
     }
 
-    foreach (QString function, m_pEngine->getScriptFunctionPrefixes()) {
-        if (function == "") {
-            continue;
-        }
-        function.append(".incomingData");
-        if (!m_pEngine->execute(function, data)) {
-            qWarning() << "Controller: Invalid script function" << function;
+    QListIterator<QString> prefixIt(m_pEngine->getScriptFunctionPrefixes());
+    while (prefixIt.hasNext()) {
+        QString function = prefixIt.next();
+        if (function!="") {
+            function.append(".incomingData");
+
+            QScriptValue incomingData = m_pEngine->resolveFunction(function);
+            if (!m_pEngine->execute(incomingData, data)) {
+                qWarning() << "Controller: Invalid script function" << function;
+            }
         }
     }
 }
+

=== modified file 'mixxx/src/controllers/controller.h'
--- mixxx/src/controllers/controller.h	2012-04-29 01:40:37 +0000
+++ mixxx/src/controllers/controller.h	2012-05-25 04:46:23 +0000
@@ -18,6 +18,8 @@
 #include "controllers/controllerpresetfilehandler.h"
 #include "controllers/mixxxcontrol.h"
 
+#include <QScriptValue>
+
 class Controller : public QObject, ControllerPresetVisitor {
     Q_OBJECT
   public:
@@ -118,7 +120,6 @@
         m_controlToLearn = control;
     }
 
-
   private slots:
     virtual int open() = 0;
     virtual int close() = 0;

=== modified file 'mixxx/src/controllers/controllerengine.cpp'
--- mixxx/src/controllers/controllerengine.cpp	2012-05-01 21:42:24 +0000
+++ mixxx/src/controllers/controllerengine.cpp	2012-05-25 04:46:23 +0000
@@ -16,6 +16,10 @@
 
 // #include <QScriptSyntaxCheckResult>
 
+// Used for id's inside controlConnection objects 
+// (closure compatible version of connectControl)
+#include <QUuid>
+
 #ifdef _MSC_VER
     #include <float.h>  // for _isnan() on VC++
     #define isnan(x) _isnan(x)  // VC++ uses _isnan() instead of isnan()
@@ -66,6 +70,62 @@
         m_pEngine = NULL;
         engine->deleteLater();
     }
+
+}
+
+/* -------- ------------------------------------------------------
+Purpose: Calls the same method on a list of JS Objects
+Input:   -
+Output:  -
+-------- ------------------------------------------------------ */
+void ControllerEngine::callFunctionOnObjects(QList<QString> scriptFunctionPrefixes, QString function, QScriptValueList args)
+{
+    QListIterator<QString> prefixIt(scriptFunctionPrefixes);
+    const QScriptValue global = m_pEngine->globalObject();
+    
+    while (prefixIt.hasNext()) {
+        QString prefixName = prefixIt.next();
+        QScriptValue prefix = global.property(prefixName);
+        
+        if ( prefix.isValid() && prefix.isObject()) {
+            
+            QScriptValue init = prefix.property(function);
+            if (init.isValid() && init.isFunction()) {
+                if (m_bDebug) {
+                    qDebug() << "ControllerEngine: Executing" << prefixName << "." << function;
+                }
+                init.call(QScriptValue(), args);
+            }
+            else {
+                qWarning() << "ControllerEngine:" << prefixName << "has no" << function << " method";
+            }
+        }
+        else {
+            qWarning() << "ControllerEngine: No" << prefixName << "object in script";
+        }
+    }
+}
+
+/* -------- ------------------------------------------------------
+Purpose: Resolves a function name to a QScriptValue including 
+            OBJECT.Function calls
+Input:   -
+Output:  -
+-------- ------------------------------------------------------ */
+QScriptValue ControllerEngine::resolveFunction(QString function) const {
+    QScriptValue object = m_pEngine->globalObject();
+    QStringList parts = function.split(".");
+    
+    for (int i = 0; i < parts.size(); i++) {
+        object = object.property(parts.at(i));
+        if (!object.isValid())
+            return QScriptValue();
+    }
+
+    if (!object.isFunction())
+        return QScriptValue();
+
+    return object;
 }
 
 /* -------- ------------------------------------------------------
@@ -85,25 +145,14 @@
     stopAllTimers();
 
     // Call each script's shutdown function if it exists
-    foreach (QString prefix, m_scriptFunctionPrefixes) {
-        if (prefix == "") {
-            continue;
-        }
-        QString shutdownName = QString("%1.shutdown").arg(prefix);
-        if (m_bDebug) {
-            qDebug() << "  Executing" << shutdownName;;
-        }
-        if (!internalExecute(shutdownName)) {
-            qWarning() << "ControllerEngine: No" << shutdownName << "function in script";
-        }
-    }
-
-    // Prevents leaving decks in an unstable state if the controller is shut
-    // down while scratching
+    callFunctionOnObjects(m_scriptFunctionPrefixes, "shutdown");
+    
+    // Prevents leaving decks in an unstable state
+    //  if the controller is shut down while scratching
     QHashIterator<int, int> i(m_scratchTimers);
     while (i.hasNext()) {
         i.next();
-        qDebug() << "  Aborting scratching on deck" << i.value();
+        qDebug() << "Aborting scratching on deck" << i.value();
         // Clear scratch2_enable
         QString group = QString("[Channel%1]").arg(i.value());
         ControlObjectThread *cot = getControlObjectThread(group, "scratch2_enable");
@@ -133,7 +182,7 @@
 }
 
 void ControllerEngine::initializeScriptEngine() {
-    // Create the script engine
+    // Create the Script Engine
     m_pEngine = new QScriptEngine(this);
 
     // Make this ControllerEngine instance available to scripts as 'engine'.
@@ -145,6 +194,7 @@
 
         // Make the Controller instance available to scripts
         engineGlobalObject.setProperty("controller", m_pEngine->newQObject(m_pController));
+
         // ...under the legacy name as well
         engineGlobalObject.setProperty("midi", m_pEngine->newQObject(m_pController));
     }
@@ -251,12 +301,13 @@
 
 
 /* -------- ------------------------------------------------------
-    Purpose: Evaluate & run script code
-    Input:   Code string
-    Output:  false if an exception
-    -------- ------------------------------------------------------ */
-bool ControllerEngine::internalExecute(QString scriptCode) {
-    // A special version of execute since we're evaluating strings, not actual functions
+Purpose: Evaluate & run script code
+Input:   'this' object if applicable, Code string
+Output:  false if an exception
+-------- ------------------------------------------------------ */
+bool ControllerEngine::internalExecute(QScriptValue thisObject,
+                                       QString scriptCode) {
+    // A special version of safeExecute since we're evaluating strings, not actual functions
     //  (execute() would print an error that it's not a function every time a timer fires.)
     if (m_pEngine == NULL)
         return false;
@@ -296,13 +347,25 @@
         return false;
     }
 
+    return internalExecute(thisObject, scriptFunction);
+}
+
+/* -------- ------------------------------------------------------
+Purpose: Evaluate & run script code
+Input:   'this' object if applicable, Code string
+Output:  false if an exception
+-------- ------------------------------------------------------ */
+bool ControllerEngine::internalExecute(QScriptValue thisObject, QScriptValue functionObject) {
+    if(m_pEngine == NULL)
+        return false;
+
     // If it's not a function, we're done.
-    if (!scriptFunction.isFunction()) {
-        return true;
+    if (!functionObject.isFunction()) {
+        return false;
     }
 
     // If it does happen to be a function, call it.
-    scriptFunction.call(QScriptValue());
+    functionObject.call(thisObject);
     if (checkException()) {
         qDebug() << "Exception";
         return false;
@@ -326,16 +389,37 @@
 
     if (checkException())
         return false;
-    if (!scriptFunction.isFunction())
-        return false;
-
-    scriptFunction.call(QScriptValue(), args);
+
+    return execute(scriptFunction, args);
+}
+
+/**-------- ------------------------------------------------------
+   Purpose: Evaluate & call a script function with argument list
+   Input:   Function name, argument list
+   Output:  false if an invalid function or an exception
+   -------- ------------------------------------------------------ */
+bool ControllerEngine::execute(QScriptValue functionObject, QScriptValueList args) {
+
+    if(m_pEngine == NULL) {
+        qDebug() << "ControllerEngine::execute: No script engine exists!";
+        return false;
+    }
+    
+    //qDebug() << "Calling MIDI Script Function";
+    if (!functionObject.isFunction()) {
+        qDebug() << "Not a function";
+        return false;
+    }
+    QScriptValue rc = functionObject.call(m_pEngine->globalObject(), args);
+    if (!rc.isValid()) {
+        qDebug() << "QScriptValue is not a function or ...";
+        return false;
+    }
 
     if (checkException())
         return false;
     return true;
 }
-
 /**-------- ------------------------------------------------------
    Purpose: Evaluate & call a script function
    Input:   Function name, data string (e.g. device ID)
@@ -362,12 +446,7 @@
     QScriptValueList args;
     args << QScriptValue(data);
 
-    scriptFunction.call(QScriptValue(), args);
-    if (checkException()) {
-        qDebug() << "ControllerEngine::execute: Exception";
-        return false;
-    }
-    return true;
+    return execute(scriptFunction, args);
 }
 
 /**-------- ------------------------------------------------------
@@ -389,22 +468,30 @@
 
     if (checkException())
         return false;
-    if (!scriptFunction.isFunction())
-        return false;
-
-//     const char* buffer=reinterpret_cast<const char*>(data);
+
+    return execute(scriptFunction, data);
+}
+
+/**-------- ------------------------------------------------------
+   Purpose: Evaluate & call a script function
+   Input:   Function name, ponter to data buffer, length of buffer
+   Output:  false if an invalid function or an exception
+   -------- ------------------------------------------------------ */
+bool ControllerEngine::execute(QScriptValue function, const QByteArray data) {
+    if (m_pEngine == NULL) {
+        return false;
+    }
+
+    if (checkException())
+        return false;
+    if (!function.isFunction())
+        return false;
 
     QScriptValueList args;
-//     args << QScriptValue(data);
     args << QScriptValue(m_pBaClass->newInstance(data));
     args << QScriptValue(data.size());
-//     args << QScriptValue(m_pBaClass->newInstance(QByteArray::fromRawData(buffer,length)));
-//     args << QScriptValue(length);
 
-    scriptFunction.call(QScriptValue(), args);
-    if (checkException())
-        return false;
-    return true;
+    return execute(function, args);
 }
 
 /* -------- ------------------------------------------------------
@@ -500,46 +587,6 @@
     }
 }
 
-/* -------- ------------------------------------------------------
-   Purpose: Returns a list of functions available in the QtScript
-            code
-   Input:   -
-   Output:  functionList QStringList
-   -------- ------------------------------------------------------ */
-QStringList ControllerEngine::getScriptFunctions() {
-    QStringList ret = m_scriptFunctions;
-    return ret;
-}
-
-void ControllerEngine::generateScriptFunctions(QString scriptCode) {
-//     QStringList functionList;
-    QStringList codeLines = scriptCode.split("\n");
-
-//     qDebug() << "ControllerEngine: m_scriptCode=" << m_scriptCode;
-
-    if (m_bDebug)
-        qDebug() << "ControllerEngine:" << codeLines.count() << "lines of code being searched for functions";
-
-    // grep 'function' midi/midi-mappings-scripts.js|grep -i '(msg)'|sed -e 's/function \(.*\)(msg).*/\1/i' -e 's/[= ]//g'
-    QRegExp rx("*.*function*(*)*");    // Find all lines with function names in them
-    rx.setPatternSyntax(QRegExp::Wildcard);
-
-    int position = codeLines.indexOf(rx);
-
-    while (position != -1) {    // While there are more matches
-
-        QString line = codeLines.takeAt(position);    // Pull & remove the current match from the list.
-
-        if (line.indexOf('#') != 0 && line.indexOf("//") != 0) {    // ignore commented out lines
-            QStringList field = line.split(" ");
-            if (m_bDebug) qDebug() << "ControllerEngine: Found function:" << field[0]
-                                      << "at line" << position;
-            m_scriptFunctions.append(field[0]);
-        }
-        position = codeLines.indexOf(rx);
-    }
-}
-
 ControlObjectThread* ControllerEngine::getControlObjectThread(QString group, QString name) {
     ConfigKey key = ConfigKey(group, name);
 
@@ -620,45 +667,128 @@
                 script function name, true if you want to disconnect
    Output:  true if successful
    -------- ------------------------------------------------------ */
-bool ControllerEngine::connectControl(QString group, QString name, QString function, bool disconnect) {
-    ControlObjectThread* cobj = getControlObjectThread(group, name);
+QScriptValue ControllerEngine::connectControl(QString group, QString name, QScriptValue callback, bool disconnect) {
     ConfigKey key(group, name);
+    ControlObject* cobj = ControlObject::getControl(key);
+    ControllerEngineConnection conn;
+    QScriptValue function;
 
     if (cobj == NULL) {
         qWarning() << "ControllerEngine: script connecting [" << group << "," << name
                    << "], which is non-existent. ignoring.";
-        return false;
-    }
-
-    // Don't add duplicates
-    if (!disconnect && m_connectedControls.contains(key, function)) {
-        return true;
-    }
-
-    if (m_pEngine == NULL) {
-        return false;
-    }
-
-    QScriptValue slot = m_pEngine->evaluate(function);
-
-    if (!checkException() && slot.isFunction()) {
-        if (disconnect) {
-            //qDebug() << "ControllerEngine::connectControl disconnected " << group << name << " from " << function;
-            m_connectedControls.remove(key, function);
-            // Only disconnect the signal if there are no other instances of this control using it
-            if (!m_connectedControls.contains(key)) {
-                this->disconnect(cobj, SIGNAL(valueChanged(double)),
-                                 this, SLOT(slotValueChanged(double)));
-            }
-        } else {
-            //qDebug() << "ControllerEngine::connectControl connected " << group << name << " to " << function;
-            connect(cobj, SIGNAL(valueChanged(double)),
-                    this, SLOT(slotValueChanged(double)));
-            m_connectedControls.insert(key, function);
-        }
-        return true;
-    }
-    return false;
+        return QScriptValue();
+    }
+
+    if(m_pEngine == NULL) {
+        return QScriptValue(FALSE);
+    }
+
+    if (callback.isString()) {
+        ControllerEngineConnection cb;
+        cb.key = key;
+        cb.id = callback.toString();
+        
+        if(disconnect) {
+            disconnectControl(cb);
+            return QScriptValue(TRUE);
+        }
+
+        function = m_pEngine->evaluate(callback.toString());
+        if (checkException() || !function.isFunction()) {
+            return QScriptValue(FALSE);
+        }
+
+        // Do not allow multiple connections to named functions
+        else if (m_connectedControls.contains(key, cb)) {
+            // Return a wrapper to the conn
+            QHash<ConfigKey, ControllerEngineConnection>::iterator i = 
+                m_connectedControls.find(key);
+            
+            ControllerEngineConnection conn = i.value();
+            return m_pEngine->newQObject(
+                        new ControllerEngineConnectionScriptValue(conn), 
+                        QScriptEngine::ScriptOwnership
+                    );
+        }
+    }
+    else if (callback.isFunction()) {
+        function = callback;
+    }
+    // Assume a ControllerEngineConnection
+    else if (callback.isQObject()) {
+        QObject *qobject = callback.toQObject();
+        const QMetaObject *qmeta = qobject->metaObject();
+        
+        if (!strcmp(qmeta->className(), "ControllerEngineConnectionScriptValue")) {
+            ControllerEngineConnectionScriptValue *proxy = 
+                (ControllerEngineConnectionScriptValue *)qobject;
+            proxy->disconnect();
+        }
+    }
+    else {
+        qWarning() << "Invalid callback";
+        return QScriptValue(FALSE);
+    }
+
+    if (function.isFunction()) {
+    	qDebug() << "Connection:" << group << name;
+        connect(cobj, SIGNAL(valueChanged(double)),
+                    this, SLOT(slotValueChanged(double)),
+                    Qt::QueuedConnection);
+
+        ControllerEngineConnection conn;
+        conn.key = key;
+        conn.ce = this;
+        conn.function = function;
+        QScriptContext *ctxt = m_pEngine->currentContext();
+        conn.context = ctxt ? ctxt->thisObject() : QScriptValue();
+
+        if (callback.isString()) {
+            conn.id = callback.toString();
+        }
+        else {
+            QUuid uuid = QUuid::createUuid();
+            conn.id = uuid.toString();
+        }
+
+        m_connectedControls.insert(key, conn);
+        return m_pEngine->newQObject(
+                    new ControllerEngineConnectionScriptValue(conn), 
+                    QScriptEngine::ScriptOwnership
+                );
+    }
+
+    return QScriptValue(FALSE);
+}
+
+/* -------- ------------------------------------------------------
+   Purpose: (Dis)connects a ControlObject valueChanged() signal to/from a script function
+   Input:   Control group (e.g. [Channel1]), Key name (e.g. [filterHigh]),
+                script function name, true if you want to disconnect
+   Output:  true if successful
+   -------- ------------------------------------------------------ */
+void ControllerEngine::disconnectControl(const ControllerEngineConnection conn) {
+    ControlObject* cobj = ControlObject::getControl(ConfigKey(conn.key.group, conn.key.item));
+
+    if(m_pEngine == NULL) {
+        return;
+    }
+
+    if (m_connectedControls.contains(conn.key, conn)) {
+        m_connectedControls.remove(conn.key, conn);
+        // Only disconnect the signal if there are no other instances of this control using it
+        if (!m_connectedControls.contains(conn.key)) {
+            this->disconnect(cobj, SIGNAL(valueChanged(double)),
+                            this, SLOT(slotValueChanged(double)));
+        }
+    }
+    else {
+        qWarning() << "Could not Disconnect connection" << conn.id;
+    }
+}
+
+void ControllerEngineConnectionScriptValue::disconnect() {
+    this->conn.ce->disconnectControl(this->conn);
 }
 
 /**-------- ------------------------------------------------------
@@ -666,39 +796,46 @@
    fires off the appropriate script function.
    -------- ------------------------------------------------------ */
 void ControllerEngine::slotValueChanged(double value) {
-    ControlObjectThread* sender = dynamic_cast<ControlObjectThread*>(this->sender());
-    if (sender == NULL) {
+
+    ControlObject* sender = (ControlObject*)this->sender();
+    if(sender == NULL) {
         qWarning() << "ControllerEngine::slotValueChanged() Shouldn't happen -- sender == NULL";
         return;
     }
-
-    ControlObject* pSenderCO = sender->getControlObject();
-    if (pSenderCO == NULL) {
-        qWarning() << "ControllerEngine::slotValueChanged() The sender's CO is NULL.";
-        return;
-    }
-    ConfigKey key = pSenderCO->getKey();
-
-    if (m_connectedControls.contains(key)) {
-        QMultiHash<ConfigKey, QString>::iterator i = m_connectedControls.find(key);
-        while (i != m_connectedControls.end() && i.key() == key) {
-            QString function = i.value();
-
-            //qDebug() << "ControllerEngine::slotValueChanged() received signal from " << key.group << key.item << " ... firing : " << function;
-
-            // Could branch to execute from here, but for now do it this way.
-            QScriptValue function_value = m_pEngine->evaluate(function);
+    ConfigKey key = sender->getKey();
+
+    qDebug() << "[Controller]: SlotValueChanged" << key.group << key.item;
+
+    if(m_connectedControls.contains(key)) {
+        QHash<ConfigKey, ControllerEngineConnection>::iterator iter = 
+            m_connectedControls.find(key);
+        QList<ControllerEngineConnection> conns;
+
+        // Create a temporary list to allow callbacks to disconnect
+        // -Phillip Whelan
+        while (iter != m_connectedControls.end() && iter.key() == key) {
+            conns.append(iter.value());
+            ++iter;
+        }
+
+        for (int i = 0; i < conns.size(); i++) {
+            ControllerEngineConnection conn = conns.at(i);
             QScriptValueList args;
+
             args << QScriptValue(value);
-            args << QScriptValue(key.group); // Added by Math`
-            args << QScriptValue(key.item);  // Added by Math`
-            QScriptValue result = function_value.call(QScriptValue(), args);
+            args << QScriptValue(key.group);
+            args << QScriptValue(key.item);
+            QScriptValue result = conn.function.call(conn.context, args);
             if (result.isError()) {
-                qWarning()<< "ControllerEngine: Call to " << function << " resulted in an error:  " << result.toString();
-            }
-            ++i;
+                qWarning()<< "ControllerEngine: Call to callback" << conn.id
+                          << "resulted in an error:" << result.toString();
+            }
+            else {
+            	qDebug() << "Called Connection for" << conn.id;
+            }
         }
-    } else {
+    }
+    else {
         qWarning() << "ControllerEngine::slotValueChanged() Received signal from ControlObject that is not connected to a script function.";
     }
 }
@@ -809,9 +946,6 @@
         return false;
     }
 
-    // Add the code we evaluated to our index
-    generateScriptFunctions(scriptCode);
-
     return true;
 }
 
@@ -839,24 +973,37 @@
                 whether it should fire just once
    Output:  The timer's ID, 0 if starting it failed
    -------- ------------------------------------------------------ */
-int ControllerEngine::beginTimer(int interval, QString scriptCode, bool oneShot) {
-    if (interval<20) {
-        qWarning() << "Timer request for" << interval << "ms is too short. Setting to the minimum of 20ms.";
-        interval=20;
-    }
-    // This makes use of every QObject's internal timer mechanism. Nice, clean, and simple.
-    // See http://doc.trolltech.com/4.6/qobject.html#startTimer for details
+int ControllerEngine::beginTimer(int interval, QScriptValue timerCallback,
+                                 bool oneShot) {
+
+    if (!timerCallback.isFunction() && !timerCallback.isString()) {
+        qWarning() << "Invalid timer callback provided to beginTimer."
+                   << "Valid callbacks are strings and functions.";
+        return 0;
+    }
+
+    if (interval < 20) {
+        qWarning() << "Timer request for" << interval
+                   << "ms is too short. Setting to the minimum of 20ms.";
+        interval = 20;
+    }
+    // This makes use of every QObject's internal timer mechanism. Nice, clean,
+    // and simple. See http://doc.trolltech.com/4.6/qobject.html#startTimer for
+    // details
     int timerId = startTimer(interval);
-    QPair<QString, bool> timerTarget;
-    timerTarget.first = scriptCode;
-//     timerTarget.second = oneShot;
-    m_timers[timerId]=timerTarget;
-
+    TimerInfo info;
+    info.callback = timerCallback;
+    QScriptContext *ctxt = m_pEngine->currentContext();
+    info.context = ctxt ? ctxt->thisObject() : QScriptValue();
+    info.oneShot = oneShot;
+    m_timers[timerId] = info;
     if (timerId == 0) {
-        qWarning() << "Controller script timer could not be created";
+        qWarning() << "MIDI Script timer could not be created";
     } else if (m_bDebug) {
-        qDebug() << "Starting" << (oneShot ? "one-shot timer:" : "timer:")
-                 << timerId;
+        if (oneShot)
+            qDebug() << "Starting one-shot timer:" << timerId;
+        else
+            qDebug() << "Starting timer:" << timerId;
     }
     return timerId;
 }
@@ -885,7 +1032,7 @@
    Output:  -
    -------- ------------------------------------------------------ */
 void ControllerEngine::stopAllTimers() {
-    QMutableHashIterator<int, QPair<QString, bool> > i(m_timers);
+    QMutableHashIterator<int, TimerInfo> i(m_timers);
     while (i.hasNext()) {
         i.next();
         stopTimer(i.key());
@@ -911,12 +1058,16 @@
         return;
     }
 
-    QPair<QString, bool> timerTarget = m_timers[timerId];
-    if (timerTarget.second) {
+    TimerInfo timerTarget = m_timers[timerId];
+    if (timerTarget.oneShot) {
         stopTimer(timerId);
     }
 
-    internalExecute(timerTarget.first);
+    if (timerTarget.callback.isString()) {
+        internalExecute(timerTarget.context, timerTarget.callback.toString());
+    } else if (timerTarget.callback.isFunction()) {
+        internalExecute(timerTarget.context, timerTarget.callback);
+    }
 }
 
 /* -------- ------------------------------------------------------

=== modified file 'mixxx/src/controllers/controllerengine.h'
--- mixxx/src/controllers/controllerengine.h	2012-04-27 04:04:33 +0000
+++ mixxx/src/controllers/controllerengine.h	2012-05-25 04:46:23 +0000
@@ -20,6 +20,41 @@
 // Forward declaration(s)
 class Controller;
 class ControlObjectThread;
+class ControllerEngine;
+
+// ControllerEngineConnection class for closure-compatible engine.connectControl
+class ControllerEngineConnection {
+  public:
+    ConfigKey key;
+    QString id;
+    QScriptValue function;
+    ControllerEngine *ce;
+    QScriptValue context;
+};
+
+class ControllerEngineConnectionScriptValue : public QObject {
+    Q_OBJECT
+    Q_PROPERTY(QString id READ readId)
+    // We cannot expose ConfigKey directly since it's not a
+    // QObject
+    //Q_PROPERTY(ConfigKey key READ key)
+    // There's little use in exposing the function...
+    //Q_PROPERTY(QScriptValue function READ function)
+  public:
+    ControllerEngineConnectionScriptValue(ControllerEngineConnection conn) {
+        this->conn = conn;
+    }
+    QString readId() const { return this->conn.id; }
+    Q_INVOKABLE void disconnect();
+    
+  private:
+   ControllerEngineConnection conn;
+};
+
+/* comparison function for ControllerEngineConnection */
+inline bool operator==(const ControllerEngineConnection &c1, const ControllerEngineConnection &c2) {
+    return c1.id == c2.id && c1.key.group == c2.key.group && c1.key.item == c2.key.item;
+}
 
 class ControllerEngine : public QObject {
     Q_OBJECT
@@ -39,20 +74,22 @@
         m_bPopups = bPopups;
     }
 
-    // Look up registered script functions
-    QStringList getScriptFunctions();
-
-    // Look up registered script function prefixes
+    /** Resolve a function name to a QScriptValue. */
+    QScriptValue resolveFunction(QString function) const;
+    /** Look up registered script function prefixes */
     QList<QString>& getScriptFunctionPrefixes() { return m_scriptFunctionPrefixes; };
+    /** Disconnect a ControllerEngineConnection */
+    void disconnectControl(const ControllerEngineConnection conn);
 
   protected:
     Q_INVOKABLE double getValue(QString group, QString name);
     Q_INVOKABLE void setValue(QString group, QString name, double newValue);
-    Q_INVOKABLE bool connectControl(QString group, QString name,
-                                    QString function, bool disconnect = false);
+    Q_INVOKABLE QScriptValue connectControl(QString group, QString name,
+                                    QScriptValue function, bool disconnect = false);
+    // Called indirectly by the objects returned by connectControl
     Q_INVOKABLE void trigger(QString group, QString name);
     Q_INVOKABLE void log(QString message);
-    Q_INVOKABLE int beginTimer(int interval, QString scriptCode, bool oneShot = false);
+    Q_INVOKABLE int beginTimer(int interval, QScriptValue scriptCode, bool oneShot = false);
     Q_INVOKABLE void stopTimer(int timerId);
     Q_INVOKABLE void scratchEnable(int deck, int intervalsPerRev, float rpm,
                                    float alpha, float beta, bool ramp = true);
@@ -67,14 +104,20 @@
     void slotValueChanged(double value);
     // Evaluate a script file
     bool evaluate(QString filepath);
+
     // Execute a particular function
     bool execute(QString function);
     // Execute a particular function with a list of arguments
     bool execute(QString function, QScriptValueList args);
+    bool execute(QScriptValue function, QScriptValueList args);
     // Execute a particular function with a data string (e.g. a device ID)
     bool execute(QString function, QString data);
+    // Execute a particular function with a list of arguments
+    bool execute(QString function, const QByteArray data);
+    bool execute(QScriptValue function, const QByteArray data);
     // Execute a particular function with a data buffer
-    bool execute(QString function, const QByteArray data);
+    //TODO: redo this one
+    //bool execute(QString function, const QByteArray data);
     void loadScriptFiles(QString configPath,
                          QList<QString> scriptFileNames);
     void initializeScripts(const QList<QString> scriptFunctionPrefixes);
@@ -90,12 +133,15 @@
   private:
     bool evaluate(QString scriptName, QList<QString> scriptPaths);
     bool internalExecute(QString scriptCode);
+    bool internalExecute(QScriptValue thisObject, QString scriptCode);
+    bool internalExecute(QScriptValue thisObject, QScriptValue functionObject);
     void initializeScriptEngine();
 
     void scriptErrorDialog(QString detailedError);
     void generateScriptFunctions(QString code);
     void stopAllTimers();
 
+    void callFunctionOnObjects(QList<QString>, QString, QScriptValueList args = QScriptValueList());
     bool checkException();
     QScriptEngine *m_pEngine;
 
@@ -107,12 +153,16 @@
     Controller* m_pController;
     bool m_bDebug;
     bool m_bPopups;
-    QMultiHash<ConfigKey, QString> m_connectedControls;
-    QStringList m_scriptFunctions;
+    QMultiHash<ConfigKey, ControllerEngineConnection> m_connectedControls;
     QList<QString> m_scriptFunctionPrefixes;
     QMap<QString,QStringList> m_scriptErrors;
     QHash<ConfigKey, ControlObjectThread*> m_controlCache;
-    QHash<int, QPair<QString, bool> > m_timers;
+    struct TimerInfo {
+        QScriptValue callback;
+        QScriptValue context;
+        bool oneShot;
+    };
+    QHash<int, TimerInfo> m_timers;
     SoftTakeover m_st;
     ByteArrayClass *m_pBaClass;
     // 256 (default) available virtual decks is enough I would think.

=== modified file 'mixxx/src/controllers/midi/midicontroller.cpp'
--- mixxx/src/controllers/midi/midicontroller.cpp	2012-05-24 21:59:28 +0000
+++ mixxx/src/controllers/midi/midicontroller.cpp	2012-05-25 04:46:23 +0000
@@ -284,6 +284,7 @@
             return;
         }
 
+        QScriptValue function = pEngine->resolveFunction(mc.item());
         QScriptValueList args;
         args << QScriptValue(status & 0x0F);
         args << QScriptValue(control);
@@ -291,7 +292,7 @@
         args << QScriptValue(status);
         args << QScriptValue(mc.group());
 
-        pEngine->execute(mc.item(), args);
+        pEngine->execute(function, args);
         return;
     }
 
@@ -507,7 +508,8 @@
         if (pEngine == NULL) {
             return;
         }
-        if (!pEngine->execute(mc.item(), data)) {
+        QScriptValue function = pEngine->resolveFunction(mc.item());
+        if (!pEngine->execute(function, data)) {
             qDebug() << "MidiController: Invalid script function" << mc.item();
         }
         return;
@@ -520,3 +522,4 @@
             (((unsigned int)byte1) << 8) | status;
     send(word);
 }
+

=== modified file 'mixxx/src/controllers/mixxxcontrol.cpp'
--- mixxx/src/controllers/mixxxcontrol.cpp	2012-04-23 14:16:08 +0000
+++ mixxx/src/controllers/mixxxcontrol.cpp	2012-05-25 04:46:23 +0000
@@ -1,6 +1,12 @@
 #include <QtCore>
 #include <QtXml>
 
+#include "mixxxcontrol.h"
+
+#ifdef __MIDISCRIPT__
+#include <QScriptValue>
+#endif
+
 #include "controllers/mixxxcontrol.h"
 
 MixxxControl::MixxxControl(QString controlobject_group, QString controlobject_item,
@@ -8,6 +14,7 @@
         : m_sGroup(controlobject_group),
           m_sItem(controlobject_item),
           m_sDescription(controlobject_description) {
+    m_scriptFunction = QScriptValue();
 }
 
 /** Constructor that deserializes a MixxxControl object from a <control> or <output>

=== modified file 'mixxx/src/controllers/mixxxcontrol.h'
--- mixxx/src/controllers/mixxxcontrol.h	2012-04-23 14:16:08 +0000
+++ mixxx/src/controllers/mixxxcontrol.h	2012-05-25 04:46:23 +0000
@@ -2,9 +2,12 @@
 #define MIXXXCONTROL_H
 
 #include <QDebug>
+
 #include <QVariant>
+#include <QScriptValue>
 #include "controlobject.h"
 
+
 class MixxxControl {
     public:
         MixxxControl(QString controlobject_group="", QString controlobject_item="",
@@ -18,6 +21,10 @@
         QString group() const { return m_sGroup; };
         QString item() const { return m_sItem; };
         QString description() const { return m_sDescription; };
+
+        QScriptValue getScriptFunction() const { return m_scriptFunction; };
+        void setScriptFunction(QScriptValue func) { m_scriptFunction = func; };
+
         ControlObject* getControlObject() const {
             return ControlObject::getControl(ConfigKey(m_sGroup, m_sItem));
         };
@@ -31,6 +38,7 @@
         QString m_sGroup;
         QString m_sItem;
         QString m_sDescription;
+        QScriptValue m_scriptFunction;
 };
 
 inline bool operator<(const MixxxControl &first, const MixxxControl &second)
@@ -57,3 +65,4 @@
 }*/
 
 #endif
+

=== modified file 'mixxx/src/test/controllerengine_test.cpp'
--- mixxx/src/test/controllerengine_test.cpp	2012-04-07 14:20:50 +0000
+++ mixxx/src/test/controllerengine_test.cpp	2012-05-25 04:46:23 +0000
@@ -4,8 +4,10 @@
 #include <QApplication>
 #include <QObject>
 #include <QFile>
+#include <QThread>
 
 #include "controlobject.h"
+#include "controlpotmeter.h"
 #include "configobject.h"
 #include "controllers/controllerengine.h"
 
@@ -14,10 +16,13 @@
 class ControllerEngineTest : public testing::Test {
   protected:
     virtual void SetUp() {
+        QApplication *app;
         qDebug() << "SetUp";
         static int argc = 1;
-        static char** argv = NULL;
+        static char* argv[2] = { "test", NULL };
+        QThread::currentThread()->setObjectName("Main");
         app = new QApplication(argc, argv);
+        new ControlPotmeter(ConfigKey("[Test]", "potmeter"),-1.,1.);
         Controller* pController = NULL;
         cEngine = new ControllerEngine(pController);
         cEngine->setDebug(true);
@@ -88,6 +93,178 @@
     f.remove();
 }
 
+TEST_F(ControllerEngineTest, scriptConnectDisconnectControlNamedFunction) {
+    QString script = "test.js";
+    QFile f(script);
+    f.open(QIODevice::ReadWrite | QIODevice::Truncate);
+    f.write(
+        "var executed = false;\n"
+        "var connection;\n"
+        "testConnectDisconnectControlCallback = function() {\n"
+        "    executed = true;\n"
+        "};\n"
+        "testConnectDisconnectControl = function() { \n"
+        "    connection = engine.connectControl('[Test]', 'potmeter', \n"
+        "                            'testConnectDisconnectControlCallback');\n"
+        "    engine.trigger('[Test]', 'potmeter');\n"
+        "    return true;\n"
+        "};\n"
+        "checkConnectDisconnectControl = function() {\n"
+        "    connection.disconnect();\n"
+        "    if (!executed) {\n"
+        "        throw 'Did Not Execute Callback';\n"
+        "    }\n"
+        "    return executed;\n"
+        "};\n"
+    );
+    f.close();
+
+    cEngine->evaluate(script);
+    EXPECT_FALSE(cEngine->hasErrors(script));
+
+    EXPECT_TRUE(cEngine->execute("testConnectDisconnectControl"));
+    EXPECT_TRUE(cEngine->execute("checkConnectDisconnectControl"));
+
+    f.remove();
+}
+
+TEST_F(ControllerEngineTest, scriptConnectDisconnectControlClosure) {
+    QString script = "test.js";
+    QFile f(script);
+    f.open(QIODevice::ReadWrite | QIODevice::Truncate);
+    f.write(
+        "var executed = false;\n"
+        "var connection;\n"
+        "testConnectDisconnectControl = function() { \n"
+        "    connection = engine.connectControl('[Test]', 'potmeter', \n"
+        "        function() { executed = true; }\n"
+        "    );\n"
+        "    engine.trigger('[Test]', 'potmeter');\n"
+        "    return true;\n"
+        "};\n"
+        "checkConnectDisconnectControl = function() {\n"
+        "    connection.disconnect();\n"
+        "    if (!executed) {\n"
+        "        throw 'Did Not Execute Callback';\n"
+        "    }\n"
+        "    return executed;\n"
+        "};\n"
+    );
+    f.close();
+
+    cEngine->evaluate(script);
+    EXPECT_FALSE(cEngine->hasErrors(script));
+
+    EXPECT_TRUE(cEngine->execute("testConnectDisconnectControl"));
+    EXPECT_TRUE(cEngine->execute("checkConnectDisconnectControl"));
+
+    f.remove();
+}
+
+TEST_F(ControllerEngineTest, scriptConnectDisconnectControlIsDisconnected) {
+    QString script = "test.js";
+    QFile f(script);
+    f.open(QIODevice::ReadWrite | QIODevice::Truncate);
+    f.write(
+        "var executed = false;\n"
+        "var connection;\n"
+        "testConnectDisconnectControl = function() { \n"
+        "    connection = engine.connectControl('[Test]', 'potmeter', \n"
+        "        function() { executed = true; }\n"
+        "    );\n"
+        "    if (typeof connection == 'undefined')\n"
+        "        throw 'Unable to Connect controller';\n"
+        "    connection.disconnect();\n"
+        "    engine.trigger('[Test]', 'potmeter');\n"
+        "    return true;\n"
+        "};\n"
+        "checkConnectDisconnectControl = function() {\n"
+        "    if (executed) {\n"
+        "        throw 'Callback was executed';\n"
+        "    }\n"
+        "    return executed==false;\n"
+        "};\n"
+    );
+    f.close();
+
+    cEngine->evaluate(script);
+    EXPECT_FALSE(cEngine->hasErrors(script));
+
+    EXPECT_TRUE(cEngine->execute("testConnectDisconnectControl"));
+    EXPECT_TRUE(cEngine->execute("checkConnectDisconnectControl"));
+
+    f.remove();
+}
+
+TEST_F(ControllerEngineTest, scriptConnectDisconnectControlIsDisconnectedByName) {
+    QString script = "test.js";
+    QFile f(script);
+    f.open(QIODevice::ReadWrite | QIODevice::Truncate);
+    f.write(
+        "var executed = false;\n"
+        "var connection;\n"
+        "connectionCallback = function() { executed = true; }\n"
+        "testConnectDisconnectControl = function() { \n"
+        "    connection = engine.connectControl('[Test]', 'potmeter', 'connectionCallback');\n"
+        "    if (typeof connection == 'undefined')\n"
+        "        throw 'Unable to Connect controller';\n"
+        "    engine.connectControl('[Test]', 'potmeter', 'connectionCallback', 1);\n"
+        "    engine.trigger('[Test]', 'potmeter');\n"
+        "    return true;\n"
+        "};\n"
+        "checkConnectDisconnectControl = function() {\n"
+        "    if (executed) {\n"
+        "        throw 'Callback was executed';\n"
+        "    }\n"
+        "    return executed==false;\n"
+        "};\n"
+    );
+    f.close();
+
+    cEngine->evaluate(script);
+    EXPECT_FALSE(cEngine->hasErrors(script));
+
+    EXPECT_TRUE(cEngine->execute("testConnectDisconnectControl"));
+    EXPECT_TRUE(cEngine->execute("checkConnectDisconnectControl"));
+
+    f.remove();
+}
+
+TEST_F(ControllerEngineTest, scriptConnectDisconnectControlIsDisconnectedByObject) {
+    QString script = "test.js";
+    QFile f(script);
+    f.open(QIODevice::ReadWrite | QIODevice::Truncate);
+    f.write(
+        "var executed = false;\n"
+        "var connection;\n"
+        "testConnectDisconnectControl = function() { \n"
+        "    connection = engine.connectControl('[Test]', 'potmeter', \n"
+        "        function() { executed = true; }\n"
+        "    );\n"
+        "    if (typeof connection == 'undefined')\n"
+        "        throw 'Unable to Connect controller';\n"
+        "    engine.connectControl('[Test]', 'potmeter', connection, 1);\n"
+        "    engine.trigger('[Test]', 'potmeter');\n"
+        "    return true;\n"
+        "};\n"
+        "checkConnectDisconnectControl = function() {\n"
+        "    if (executed) {\n"
+        "        throw 'Callback was executed';\n"
+        "    }\n"
+        "    return executed==false;\n"
+        "};\n"
+    );
+    f.close();
+
+    cEngine->evaluate(script);
+    EXPECT_FALSE(cEngine->hasErrors(script));
+
+    EXPECT_TRUE(cEngine->execute("testConnectDisconnectControl"));
+    EXPECT_TRUE(cEngine->execute("checkConnectDisconnectControl"));
+
+    f.remove();
+}
+
 TEST_F(ControllerEngineTest, setInvalidControlObject) {
     QString script = "test.js";
     QFile f(script);