standardize facelift dbus communication

Fix service type
Pelagicore · Sep 13, 2020 · 6680a1e · 6680a1e
1 parent d451419
commit 6680a1e
Show file tree

Hide file tree

Showing 17 changed files with 446 additions and 227 deletions.
diff --git a/codegen/facelift/templates/IPCDBusServiceAdapter.template.h b/codegen/facelift/templates/IPCDBusServiceAdapter.template.h
@@ -110,13 +110,6 @@ class {{classExport}} {{className}}: public {{baseClass}}
     {% for property in interface.properties %}
     {% if property.type.is_model %}
     ::facelift::IPCAdapterModelPropertyHandler<ThisType, {{property.nestedType.interfaceCppType}}> m_{{property.name}}Handler;
-    {% elif property.type.is_interface %}
-    QString m_previous{{property.name}}ObjectPath;
-    {% else %}
-    {{property.interfaceCppType}} m_previous{{property.name}} {};
-    {% endif %}
-    {% if property.type.is_interface %}
-    InterfacePropertyIPCAdapterHandler<{{property.cppType}}, {{property.cppType}}{{proxyTypeNameSuffix}}> m_{{property.name}};
     {% endif %}
     {% endfor %}
 };

diff --git a/codegen/facelift/templates/IPCProxyAdapter.template.cpp b/codegen/facelift/templates/IPCProxyAdapter.template.cpp
@@ -43,9 +43,6 @@
 
 {{className}}::{{className}}(QObject *parent) : BaseType(parent)
     {% for property in interface.properties %}
-    {% if property.type.is_interface %}
-    , m_{{property.name}}Proxy(*this)
-    {% endif %}
     {% if property.type.is_model %}
     , m_{{property.name}}(*this)
     {% endif %}
@@ -61,47 +58,33 @@ void {{className}}::unmarshalPropertyValues(InputIPCMessage &msg)
 {
     QListIterator<QVariant> argumentsIterator(msg.arguments());
     if (argumentsIterator.hasNext()) {
-        QMap<QString, QVariant> values = castArgument<QMap<QString, QVariant>>(argumentsIterator.next());
+        QMap<QString, QVariant> values = castFromVariant<QMap<QString, QVariant>>(argumentsIterator.next());
         for (const QString &propertyName: values.keys()) {
             {% for property in interface.properties %}
-            {% if property.type.is_interface %}
             if (propertyName == QStringLiteral("{{property.name}}")) {
-                bool emit_{{property.name}}ChangeSignal = false;
-                QString {{property.name}}_objectPath = castDBusVariantArgument<QString>(values[propertyName]);
-                m_{{property.name}}Proxy.update({{property.name}}_objectPath);
-                m_{{property.name}} = m_{{property.name}}Proxy.getValue();
-                emit_{{property.name}}ChangeSignal = true;
-                if (emit_{{property.name}}ChangeSignal)
-                    emit {{property.name}}Changed();
-            }
+            {% if property.type.is_interface %}
+                const {{property.interfaceCppType}} previous_{{property.name}}_Value = m_{{property.name}};
+                m_{{property.name}} = castFromDBusVariant<{{property.interfaceCppType}}>(values[propertyName]);
+                bool emit_{{property.name}}ChangeSignal = ((previous_{{property.name}}_Value != m_{{property.name}}));
             {% elif property.type.is_model %}
-            if (propertyName == QStringLiteral("{{property.name}}")) {
                 bool emit_{{property.name}}ChangeSignal = false;
-                int {{property.name}}Size = castDBusVariantArgument<int>(values[propertyName]);
+                int {{property.name}}Size = castFromDBusVariant<int>(values[propertyName]);
                 m_{{property.name}}.beginResetModel();
                 m_{{property.name}}.reset({{property.name}}Size, std::bind(&ThisType::{{property.name}}Data, this, std::placeholders::_1));
                 m_{{property.name}}.endResetModel();
                 emit_{{property.name}}ChangeSignal = true;
-                if (emit_{{property.name}}ChangeSignal)
-                    emit {{property.name}}Changed();
-            }
             {% else %}
-            if (propertyName == QStringLiteral("{{property.name}}")) {
                 const auto previous_{{property.name}}_Value = m_{{property.name}};
-                {% if (property.type.is_list and property.nestedType.interfaceCppType == 'QString') %}
-                m_{{property.name}} = castDBusVariantArgument<QStringList>(values[propertyName]);
-                {% else %}
-                m_{{property.name}} = castDBusVariantArgument<{{property.interfaceCppType}}>(values[propertyName]);
-                {% endif %}
+                m_{{property.name}} = castFromDBusVariant<{{property.interfaceCppType}}>(values[propertyName]);
                 bool emit_{{property.name}}ChangeSignal = ((previous_{{property.name}}_Value != m_{{property.name}}));
+            {% endif %}
                 if (emit_{{property.name}}ChangeSignal)
                     emit {{property.name}}Changed();
             }
-            {% endif %}
             {% endfor %}
             if (propertyName == QStringLiteral("ready")) {
                 bool previousIsReady = this->ready();
-                m_serviceReady = castDBusVariantArgument<bool>(values[propertyName]);
+                m_serviceReady = castFromDBusVariant<bool>(values[propertyName]);
                 bool emit_ReadyChangeSignal = (previousIsReady != m_serviceReady);
                 if (emit_ReadyChangeSignal)
                     emit readyChanged();
@@ -117,7 +100,7 @@ void {{className}}::handleSignals(InputIPCMessage& msg)
         QListIterator<QVariant> argumentsIterator(msg.arguments());
         {% for parameter in event.parameters %}
         {{parameter.interfaceCppType}} param_{{parameter.name}};
-        param_{{parameter.name}} = (argumentsIterator.hasNext() ? castArgument<{{parameter.interfaceCppType}}>(argumentsIterator.next()): {{parameter.interfaceCppType}}());
+        param_{{parameter.name}} = (argumentsIterator.hasNext() ? castFromVariant<{{parameter.interfaceCppType}}>(argumentsIterator.next()):{% if not parameter.type.is_interface %}{{parameter.interfaceCppType}}(){% else %}nullptr{% endif %});
         {% endfor %}
         emit {{event}}(
         {%- set comma = joiner(", ") -%}
@@ -154,7 +137,7 @@ const QList<QString>& {{className}}::getSignals() const
 void {{className}}::onModelUpdateEvent(const InputIPCMessage& msg)
 {
     QListIterator<QVariant> argumentsIterator(msg.arguments());
-    const QString& modelPropertyName = (argumentsIterator.hasNext() ? castArgument<QString>(argumentsIterator.next()): QString());
+    const QString& modelPropertyName = (argumentsIterator.hasNext() ? castFromVariant<QString>(argumentsIterator.next()): QString());
     {% for property in interface.properties %}
     {% if property.type.is_model %}
     if (modelPropertyName == QStringLiteral("{{property.name}}")) {
@@ -168,22 +151,17 @@ void {{className}}::onModelUpdateEvent(const InputIPCMessage& msg)
 void {{className}}::unmarshalPropertiesChanged(InputIPCMessage &msg)
 {
     QListIterator<QVariant> argumentsIterator(msg.arguments());
-    QString interfaceName = (argumentsIterator.hasNext() ? castArgument<QString>(argumentsIterator.next()): QString());
-    QVariantMap changedProperties = (argumentsIterator.hasNext() ? castArgument<QVariantMap>(argumentsIterator.next()): QVariantMap());
+    QString interfaceName = (argumentsIterator.hasNext() ? castFromVariant<QString>(argumentsIterator.next()): QString());
+    QVariantMap changedProperties = (argumentsIterator.hasNext() ? castFromVariant<QVariantMap>(argumentsIterator.next()): QVariantMap());
     for (const QString &propertyName: changedProperties.keys()) {
         {% for property in interface.properties %}
-        {% if property.type.is_interface %}
-        {% elif property.type.is_model %}
-        {% else %}
         if (propertyName == QStringLiteral("{{property.name}}")) {
-            {% if (property.type.is_list and property.nestedType.interfaceCppType == 'QString') %}
-            m_{{property.name}} = castArgument<QStringList>(changedProperties[propertyName]);
-            {% else %}
-            m_{{property.name}} = castArgument<{{property.cppType}}>(changedProperties[propertyName]);
-            {% endif %}
+        {% if property.type.is_model %}
+        {% else %}
+            m_{{property.name}} = castFromDBusVariant<{{property.interfaceCppType}}>(changedProperties[propertyName]);
             emit {{property.name}}Changed(); // trust the propertiesChanged signal and emit without checking
-        }
         {% endif %}
+        }
         {% endfor %}
     }
 }
@@ -233,7 +211,7 @@ void {{className}}::{{operation.name}}(
             {%- for parameter in operation.parameters -%}
             , {{parameter.name}}
             {%- endfor -%} );
-        return (!args.isEmpty() ? castArgument<{{operation.interfaceCppType}}>(args[0]): {{operation.interfaceCppType}}());
+        return (!args.isEmpty() ? castFromVariant<{{operation.interfaceCppType}}>(args[0]):{% if not (operation.type.is_interface) %}{{operation.cppType}}(){% else %}nullptr{% endif %});
         {% else %}
         ipc()->sendMethodCall(memberID(MethodID::{{operation.name}}, "{{operation.name}}")
         {%- for parameter in operation.parameters -%}

diff --git a/codegen/facelift/templates/IPCProxyAdapter.template.h b/codegen/facelift/templates/IPCProxyAdapter.template.h
@@ -162,9 +162,6 @@ class {{classExport}} {{className}} : public {{baseClass}}
     {% endif %}
 private:
     {% for property in interface.properties %}
-    {% if property.type.is_interface %}
-    InterfacePropertyIPCProxyHandler<{{property.cppType}}{{proxyTypeNameSuffix}}> m_{{property.name}}Proxy;
-    {% endif %}
     {% if property.type.is_model %}
     facelift::IPCProxyModelProperty<ThisType, {{property.nestedType.interfaceCppType}}> m_{{property.name}};
     {% endif %}

diff --git a/codegen/facelift/templates/IPCServiceAdapter.template.cpp b/codegen/facelift/templates/IPCServiceAdapter.template.cpp
@@ -57,7 +57,7 @@ facelift::IPCHandlingResult {{className}}::handleMethodCallMessage(InputIPCMessa
     {% for operation in interface.operations %}
     if (member == memberID(MethodID::{{operation.name}}, "{{operation.name}}")) {
         {% for parameter in operation.parameters %}
-        {{parameter.cppType}} param_{{parameter.name}} = (argumentsIterator.hasNext() ? castArgument<{{parameter.cppType}}>(argumentsIterator.next()): {{parameter.cppType}}());
+        {{parameter.cppType}} param_{{parameter.name}} = (argumentsIterator.hasNext() ? castFromVariant<{{parameter.cppType}}>(argumentsIterator.next()):{% if not parameter.type.is_interface %}{{parameter.cppType}}(){% else %}nullptr{% endif %});
         {% endfor %}
         {% if operation.isAsync %}
         theService->{{operation.name}}({% for parameter in operation.parameters %} param_{{parameter.name}}, {%- endfor -%}
@@ -75,7 +75,7 @@ facelift::IPCHandlingResult {{className}}::handleMethodCallMessage(InputIPCMessa
             {{ comma() }}param_{{parameter.name}}
         {%- endfor -%});
         {% if operation.hasReturnValue %}
-        replyMessage << QVariant::fromValue(returnValue);
+        replyMessage << castToVariant(returnValue);
         {% endif %}
         {% endif %}
     } else
@@ -157,12 +157,6 @@ void {{className}}::connectSignals()
 
     // Properties
     {% for property in interface.properties %}
-    {% if property.type.is_interface %}
-    m_{{property.name}}.update(this, theService->{{property.name}}());
-    QObject::connect(theService, &ServiceType::{{property.name}}Changed, this, [this, theService] () {
-        m_{{property.name}}.update(this, theService->{{property.name}}());
-    });
-    {% endif %}
     {% if (not property.type.is_model) %}
     QObject::connect(theService, &ServiceType::{{property.name}}Changed, this, [this, theService] () {
         this->sendPropertiesChanged("{{property.name}}", theService->{{property.name}}());
@@ -183,7 +177,7 @@ void {{className}}::connectSignals()
 void {{className}}::marshalPropertyValues(const QList<QVariant>& arguments, OutputIPCMessage& msg)
 {
     QListIterator<QVariant> argumentsIterator(arguments);
-    auto msgInterfaceName = (argumentsIterator.hasNext() ? castArgument<QString>(argumentsIterator.next()): QString());
+    auto msgInterfaceName = (argumentsIterator.hasNext() ? castFromVariant<QString>(argumentsIterator.next()): QString());
     if (msgInterfaceName == interfaceName()) {
         auto theService = service();
         QMap<QString, QDBusVariant> ret;
@@ -192,61 +186,55 @@ void {{className}}::marshalPropertyValues(const QList<QVariant>& arguments, Outp
         {#% endif %#}
 
         {% for property in interface.properties %}
-        {% if property.type.is_interface %}
-        ret["{{property.name}}"] = QDBusVariant(QVariant::fromValue(m_{{property.name}}.objectPath()));
-        {% elif property.type.is_model %}
-        ret["{{property.name}}"] = QDBusVariant(QVariant::fromValue(theService->{{property.name}}().size()));
-        {% elif (property.type.is_list and property.nestedType.interfaceCppType == 'QString') %}
-        ret["{{property.name}}"] = QDBusVariant(QVariant::fromValue(QStringList(theService->{{property.name}}())));
+        {% if property.type.is_model %}
+        ret["{{property.name}}"] = castToDBusVariant(theService->{{property.name}}().size());
         {% else %}
-        ret["{{property.name}}"] = QDBusVariant(QVariant::fromValue(theService->{{property.name}}()));
+        ret["{{property.name}}"] = castToDBusVariant(theService->{{property.name}}());
         {% endif %}
         {% endfor %}
-        ret["ready"] = QDBusVariant(QVariant::fromValue(theService->ready()));
-        msg << QVariant::fromValue(ret);
+        ret["ready"] = castToDBusVariant(theService->ready());
+        msg << castToVariant(ret);
     }
 }
 
 void {{className}}::marshalProperty(const QList<QVariant>& arguments, OutputIPCMessage& msg)
 {
     QListIterator<QVariant> argumentsIterator(arguments);
-    auto msgInterfaceName = (argumentsIterator.hasNext() ? castArgument<QString>(argumentsIterator.next()): QString());
+    auto msgInterfaceName = (argumentsIterator.hasNext() ? castFromVariant<QString>(argumentsIterator.next()): QString());
     if (msgInterfaceName == interfaceName()) {
-        auto propertyName = (argumentsIterator.hasNext() ? castArgument<QString>(argumentsIterator.next()): QString());
+        auto propertyName = (argumentsIterator.hasNext() ? castFromVariant<QString>(argumentsIterator.next()): QString());
         {% for property in interface.properties %}
-        {% if property.type.is_interface %}
-
-        {% elif property.type.is_model %}
+        if (propertyName == QStringLiteral("{{property.name}}")) {
+        {% if property.type.is_model %}
 
         {% else %}
-        if (propertyName == QStringLiteral("{{property.name}}")) {
-            msg << QVariant::fromValue(service()->{{property.name}}());
-        }
+            msg << castToVariant(service()->{{property.name}}());
         {% endif %}
+        }
         {% endfor %}
         if (propertyName == QStringLiteral("ready")) {
-            msg << QVariant::fromValue(service()->ready());
+            msg << castToVariant(service()->ready());
         }
     }
 }
 
 void {{className}}::setProperty(const QList<QVariant>& arguments)
 {
     QListIterator<QVariant> argumentsIterator(arguments);
-    auto msgInterfaceName = (argumentsIterator.hasNext() ? castArgument<QString>(argumentsIterator.next()): QString());
+    auto msgInterfaceName = (argumentsIterator.hasNext() ? castFromVariant<QString>(argumentsIterator.next()): QString());
     if (msgInterfaceName == interfaceName()) {
-        auto propertyName = (argumentsIterator.hasNext() ? castArgument<QString>(argumentsIterator.next()): QString());
+        auto propertyName = (argumentsIterator.hasNext() ? castFromVariant<QString>(argumentsIterator.next()): QString());
         if (argumentsIterator.hasNext()) {
             {% for property in interface.properties %}
+            if (propertyName == QStringLiteral("{{property.name}}")) {
             {% if property.type.is_interface %}
-
+                Q_ASSERT(false); // Writable interface properties are unsupported
             {% elif property.type.is_model %}
 
             {% elif (not property.readonly) %}
-            if (propertyName == QStringLiteral("{{property.name}}")) {
-                service()->set{{property.name}}(castDBusVariantArgument<{{property.cppType}}>(argumentsIterator.next()));
-            }
+                service()->set{{property.name}}(castFromDBusVariant<{{property.cppType}}>(argumentsIterator.next()));
             {% endif %}
+	    }
             {% endfor %}
         }
     }

diff --git a/codegen/facelift/templates/IPCServiceAdapter.template.h b/codegen/facelift/templates/IPCServiceAdapter.template.h
@@ -106,13 +106,6 @@ class {{classExport}} {{className}}: public {{baseClass}}
     {% for property in interface.properties %}
     {% if property.type.is_model %}
     ::facelift::IPCAdapterModelPropertyHandler<ThisType, {{property.nestedType.interfaceCppType}}> m_{{property.name}}Handler;
-    {% elif property.type.is_interface %}
-    QString m_previous{{property.name}}ObjectPath;
-    {% else %}
-    {{property.interfaceCppType}} m_previous{{property.name}} {};
-    {% endif %}
-    {% if property.type.is_interface %}
-    InterfacePropertyIPCAdapterHandler<{{property.cppType}}, {{property.cppType}}{{proxyTypeNameSuffix}}> m_{{property.name}};
     {% endif %}
     {% endfor %}
 };

diff --git a/src/ipc/dbus/DBusIPCCommon.h b/src/ipc/dbus/DBusIPCCommon.h
@@ -38,22 +38,11 @@
 
 #include <QtDBus>
 
-template<typename T>
-const char* typeToSignature()
-{
-  return QDBusMetaType::typeToSignature(qMetaTypeId<T>());
-}
-
-template<>
-inline const char* typeToSignature<QList<QString>>()
-{
-  return QDBusMetaType::typeToSignature(qMetaTypeId<QStringList>());
-}
-
 namespace facelift {
 namespace dbus {
 using namespace facelift;
 
+
 struct FaceliftIPCLibDBus_EXPORT DBusIPCCommon {
     static constexpr const char *GET_ALL_PROPERTIES = "GetAll";
     static constexpr const char *GET_PROPERTY = "Get";

diff --git a/src/ipc/dbus/DBusIPCProxy.h b/src/ipc/dbus/DBusIPCProxy.h
@@ -56,8 +56,6 @@ class DBusIPCProxy : public IPCProxyBase<InterfaceType>, protected DBusRequestHa
     using InputIPCMessage = ::facelift::dbus::DBusIPCMessage;
     using OutputIPCMessage = ::facelift::dbus::DBusIPCMessage;
 
-    template<typename T> struct type { };
-
     template<typename Type>
     using IPCProxyType = typename Type::IPCDBusProxyType;
 
@@ -108,29 +106,20 @@ class DBusIPCProxy : public IPCProxyBase<InterfaceType>, protected DBusRequestHa
     }
 
     template<typename T>
-    T castArgument(const QVariant& value) {
-        return castArgumentPrivate(type<T>(), value);
+    T castFromVariant(const QVariant& value) {
+        return m_ipcBinder.castFromVariant<T>(value);
     }
 
     template<typename T>
-    T castDBusVariantArgument(const QVariant& value) {
-        return qdbus_cast<T>(value);
+    T castFromDBusVariant(const QVariant& value) {
+        return m_ipcBinder.castFromDBusVariant<T>(value);
     }
 
 protected:
     bool m_serviceRegistered = false;
 private:
     DBusIPCProxyBinder m_ipcBinder;
 
-    template<typename T>
-    T castArgumentPrivate(type<T>, const QVariant& value) {
-        return qdbus_cast<T>(value);
-    }
-
-    QList<QString> castArgumentPrivate(type<QList<QString>>, const QVariant& value) {
-        return qdbus_cast<QStringList>(value); // workaround to use QList<QString> since its signature matches the QStringList
-    }
-
 };
 
 } // end namespace dbus