add settingChangedHandler, fix some issues, cleanup some warnings

libnymea-core/integrations/python/pything.h

@@ -45,6 +45,7 @@ typedef struct _thing {
     PyObject *pyNameChangedHandler = nullptr;
     PyObject *pySettingChangedHandler = nullptr;
     PyObject *pyStates = nullptr; // A copy of the things states
+    PyThreadState *threadState = nullptr; // The python threadstate this thing belongs to
 } PyThing;
 
 
@@ -58,9 +59,10 @@ static PyObject* PyThing_new(PyTypeObject *type, PyObject */*args*/, PyObject */
     return (PyObject*)self;
 }
 
-static void PyThing_setThing(PyThing *self, Thing *thing)
+static void PyThing_setThing(PyThing *self, Thing *thing, PyThreadState *threadState)
 {
     self->thing = thing;
+    self->threadState = threadState;
 
     // Creating a copy because we cannot access the actual thing from the python thread
     self->thingClass = new ThingClass(thing->thingClass());
@@ -87,28 +89,36 @@ static void PyThing_setThing(PyThing *self, Thing *thing)
     // Those lambdas Will be executed in the main thread context. This means we
     // can access self->thing, but need to hold the GIL for interacting with python
     QObject::connect(thing, &Thing::nameChanged, [=](){
-        PyGILState_STATE s = PyGILState_Ensure();
+        PyEval_RestoreThread(self->threadState);
         Py_XDECREF(self->pyName);
         self->pyName = PyUnicode_FromString(self->thing->name().toUtf8().data());
         if (self->pyNameChangedHandler) {
-            PyObject_CallFunctionObjArgs(self->pyNameChangedHandler, self, nullptr);
+            PyObject *ret = PyObject_CallFunctionObjArgs(self->pyNameChangedHandler, self, nullptr);
+            if (PyErr_Occurred()) {
+                PyErr_Print();
+            }
+            Py_XDECREF(ret);
         }
-        PyGILState_Release(s);
+        PyEval_ReleaseThread(self->threadState);
     });
 
 
     QObject::connect(thing, &Thing::settingChanged, [=](const ParamTypeId &paramTypeId, const QVariant &value){
-        PyGILState_STATE s = PyGILState_Ensure();
+        PyEval_RestoreThread(self->threadState);
         Py_XDECREF(self->pySettings);
         self->pySettings = PyParams_FromParamList(self->thing->settings());
         if (self->pySettingChangedHandler) {
-            PyObject_CallFunctionObjArgs(self->pySettingChangedHandler, self, PyUnicode_FromString(paramTypeId.toString().toUtf8().data()), QVariantToPyObject(value), nullptr);
+            PyObject * ret = PyObject_CallFunctionObjArgs(self->pySettingChangedHandler, self, PyUnicode_FromString(paramTypeId.toString().toUtf8().data()), QVariantToPyObject(value), nullptr);
+            if (PyErr_Occurred()) {
+                PyErr_Print();
+            }
+            Py_XDECREF(ret);
         }
-        PyGILState_Release(s);
+        PyEval_ReleaseThread(self->threadState);
     });
 
     QObject::connect(thing, &Thing::stateValueChanged, [=](const StateTypeId &stateTypeId, const QVariant &value){
-        PyGILState_STATE s = PyGILState_Ensure();
+        PyEval_RestoreThread(self->threadState);
         for (int i = 0; i < PyList_Size(self->pyStates); i++) {
             PyObject *pyState = PyList_GetItem(self->pyStates, i);
             PyObject *pyStateTypeId = PyDict_GetItemString(pyState, "stateTypeId");
@@ -121,7 +131,7 @@ static void PyThing_setThing(PyThing *self, Thing *thing)
                 break;
             }
         }
-        PyGILState_Release(s);
+        PyEval_ReleaseThread(self->threadState);
     });
 }
 
@@ -342,6 +352,7 @@ static PyMethodDef PyThing_methods[] = {
 static PyMemberDef PyThing_members[] = {
     {"params", T_OBJECT_EX, offsetof(PyThing, pyParams), READONLY, "Thing params"},
     {"nameChangedHandler", T_OBJECT_EX, offsetof(PyThing, pyNameChangedHandler), 0, "Set a callback for when the thing name changes"},
+    {"settingChangedHandler", T_OBJECT_EX, offsetof(PyThing, pySettingChangedHandler), 0, "Set a callback for when a thing setting changes"},
     {nullptr, 0, 0, 0, nullptr}  /* Sentinel */
 };
 
@@ -409,7 +420,6 @@ static void registerThingType(PyObject *module)
     for (int i = 0; i < thingErrorEnum.keyCount(); i++) {
         PyModule_AddObject(module, thingErrorEnum.key(i), PyLong_FromLong(thingErrorEnum.value(i)));
     }
-
 }
 
 

libnymea-core/integrations/python/pythingdescriptor.h

@@ -36,7 +36,7 @@ static int PyThingDescriptor_init(PyThingDescriptor *self, PyObject *args, PyObj
     static char *kwlist[] = {"thingClassId", "name", "description", "thingId", "params", nullptr};
     PyObject *thingClassId = nullptr, *name = nullptr, *description = nullptr, *thingId = nullptr, *params = nullptr;
 
-    qWarning() << "++++ PyThingDescriptor";
+    qCDebug(dcPythonIntegrations()) << "+++ PyThingDescriptor";
     if (!PyArg_ParseTupleAndKeywords(args, kwds, "|OOOOO", kwlist, &thingClassId, &name, &description, &thingId, &params))
         return -1;
 
@@ -65,7 +65,7 @@ static int PyThingDescriptor_init(PyThingDescriptor *self, PyObject *args, PyObj
 
 static void PyThingDescriptor_dealloc(PyThingDescriptor * self)
 {
-    qWarning() << "---- PyThingDescriptor";
+    qCDebug(dcPythonIntegrations()) << "--- PyThingDescriptor";
     Py_XDECREF(self->pyThingClassId);
     Py_XDECREF(self->pyName);
     Py_XDECREF(self->pyDescription);

libnymea-core/integrations/python/pythingdiscoveryinfo.h

@@ -49,7 +49,7 @@ static PyObject* PyThingDiscoveryInfo_new(PyTypeObject *type, PyObject */*args*/
     if (self == NULL) {
         return nullptr;
     }
-    qWarning() << "++++ PyThingDiscoveryInfo";
+    qCDebug(dcPythonIntegrations()) << "+++ PyThingDiscoveryInfo";
     return (PyObject*)self;
 }
 
@@ -62,7 +62,7 @@ void PyThingDiscoveryInfo_setInfo(PyThingDiscoveryInfo *self, ThingDiscoveryInfo
 
 static void PyThingDiscoveryInfo_dealloc(PyThingDiscoveryInfo * self)
 {
-    qWarning() << "---- PyThingDiscoveryInfo";
+    qCDebug(dcPythonIntegrations()) << "--- PyThingDiscoveryInfo";
     Py_DECREF(self->pyThingClassId);
     Py_DECREF(self->pyParams);
     Py_TYPE(self)->tp_free(self);

libnymea-core/integrations/python/pythingpairinginfo.h

@@ -60,7 +60,7 @@ static PyMemberDef PyThingPairingInfo_members[] = {
 
 static int PyThingPairingInfo_init(PyThingPairingInfo */*self*/, PyObject */*args*/, PyObject */*kwds*/)
 {
-    qWarning() << "++++ ThingPairingInfo";
+    qCDebug(dcPythonIntegrations()) << "+++ ThingPairingInfo";
     return 0;
 }
 
@@ -77,7 +77,7 @@ void PyThingPairingInfo_setInfo(PyThingPairingInfo *self, ThingPairingInfo *info
 
 static void PyThingPairingInfo_dealloc(PyThingPairingInfo * self)
 {
-    qWarning() << "---- ThingPairingInfo";
+    qCDebug(dcPythonIntegrations()) << "--- ThingPairingInfo";
     Py_XDECREF(self->pyTransactionId);
     Py_XDECREF(self->pyThingClassId);
     Py_XDECREF(self->pyThingId);

libnymea-core/integrations/python/pyutils.h

@@ -69,7 +69,6 @@ QVariant PyObjectToQVariant(PyObject *pyObject)
     return QVariant();
 }
 
-
 // Write to stdout
 PyObject* pyLog_write(PyObject* /*self*/, PyObject* args)
 {

libnymea-core/integrations/pythonintegrationplugin.cpp

@@ -108,7 +108,6 @@ PyObject *PythonIntegrationPlugin::pyAutoThingsAppeared(PyObject *self, PyObject
 
     PyObject *iter = PyObject_GetIter(pyDescriptors);
     if (!iter) {
-        Py_DECREF(pyDescriptors);
         qCWarning(dcThingManager()) << "Error parsing args. Not a param list";
         return nullptr;
     }
@@ -124,6 +123,7 @@ PyObject *PythonIntegrationPlugin::pyAutoThingsAppeared(PyObject *self, PyObject
 
         if (next->ob_type != &PyThingDescriptorType) {
             PyErr_SetString(PyExc_ValueError, "Invalid argument. Not a ThingDescriptor.");
+            Py_DECREF(next);
             continue;
         }
         PyThingDescriptor *pyDescriptor = (PyThingDescriptor*)next;
@@ -155,8 +155,6 @@ PyObject *PythonIntegrationPlugin::pyAutoThingsAppeared(PyObject *self, PyObject
     QMetaObject::invokeMethod(plugin, "autoThingsAppeared", Qt::QueuedConnection, Q_ARG(ThingDescriptors, descriptors));
 
     Py_DECREF(iter);
-    Py_DECREF(pyDescriptors);
-
     Py_RETURN_NONE;
 }
 
@@ -432,13 +430,15 @@ void PythonIntegrationPlugin::setupThing(ThingSetupInfo *info)
 {
     PyEval_RestoreThread(m_threadState);
 
+    Thing *thing = info->thing();
+
     PyThing *pyThing = nullptr;
-    if (m_things.contains(info->thing())) {
+    if (m_things.contains(thing)) {
-        pyThing = m_things.value(info->thing());
+        pyThing = m_things.value(thing);
     } else {
         pyThing = (PyThing*)PyObject_CallObject((PyObject*)&PyThingType, NULL);
-        PyThing_setThing(pyThing, info->thing());
+        PyThing_setThing(pyThing, thing, m_threadState);
-        m_things.insert(info->thing(), pyThing);
+        m_things.insert(thing, pyThing);
     }
 
     PyObject *args = PyTuple_New(1);
@@ -457,6 +457,7 @@ void PythonIntegrationPlugin::setupThing(ThingSetupInfo *info)
 
     connect(info->thing(), &Thing::destroyed, this, [=](){
         PyEval_RestoreThread(m_threadState);
+        m_things.remove(thing);
         pyThing->thing = nullptr;
         Py_DECREF(pyThing);
         m_threadPool->setMaxThreadCount(m_threadPool->maxThreadCount() - 1);
@@ -515,31 +516,6 @@ void PythonIntegrationPlugin::thingRemoved(Thing *thing)
 {
     PyThing *pyThing = m_things.value(thing);
     callPluginFunction("thingRemoved", reinterpret_cast<PyObject*>(pyThing));
-
-    m_mutex.lock();
-    m_things.remove(thing);
-    m_mutex.unlock();
-}
-
-void PythonIntegrationPlugin::dumpError()
-{
-    if (!PyErr_Occurred()) {
-        return;
-    }
-
-    PyObject *ptype, *pvalue, *ptraceback;
-    PyErr_Fetch(&ptype, &pvalue, &ptraceback);
-    if (pvalue) {
-        PyObject *pstr = PyObject_Str(pvalue);
-        if (pstr) {
-            const char* err_msg = PyUnicode_AsUTF8(pstr);
-            if (pstr) {
-                qCWarning(dcThingManager()) << QString(err_msg);
-            }
-
-        }
-        PyErr_Restore(ptype, pvalue, ptraceback);
-    }
 }
 
 void PythonIntegrationPlugin::exportIds()
@@ -658,10 +634,10 @@ bool PythonIntegrationPlugin::callPluginFunction(const QString &function, PyObje
         PyEval_RestoreThread(threadState);
 
         PyObject *pluginFunctionResult = PyObject_CallFunctionObjArgs(pluginFunction, param1, param2, param3, nullptr);
-        dumpError();
 
         if (PyErr_Occurred()) {
             qCWarning(dcThingManager()) << "Error calling python method:" << function << "on plugin" << pluginName();
+            PyErr_Print();
         }
 
         Py_DECREF(pluginFunction);

libnymea-core/integrations/pythonintegrationplugin.h

@@ -39,7 +39,6 @@ public:
     void thingRemoved(Thing *thing) override;
 
 
-    static void dumpError();
     static PyObject* pyConfiguration(PyObject* self, PyObject* args);
     static PyObject* pyConfigValue(PyObject* self, PyObject* args);
     static PyObject* pySetConfigValue(PyObject* self, PyObject* args);

plugins/pymock/integrationpluginpymock.py

@@ -96,6 +96,8 @@ def confirmPairing(info, username, secret):
 def setupThing(info):
     logger.log("setupThing for", info.thing.name)
     info.finish(nymea.ThingErrorNoError)
+    info.thing.nameChangedHandler = thingNameChanged
+    info.thing.settingChangedHandler = thingSettingChanged
 
 
 def postSetupThing(thing):
@@ -123,6 +125,14 @@ def autoThings():
     return autoThings
 
 
+def thingNameChanged(thing, name):
+    logger.log("Thing name changed:", thing.name)
+
+
+def thingSettingChanged(thing, paramTypeId, value):
+    logger.log("Thing setting changed:", thing.name, paramTypeId, value)
+
+
 # Intentionally commented out to also have a test case for unimplmented functions
 # def thingRemoved(thing):
 #    logger.log("thingRemoved for", thing.name)

python-todos.txt

@@ -1,3 +1,2 @@
 
-* thing.settingChangedHandler missing
 * pluginStorage missing