diff --git a/libguh/devicemanager.cpp b/libguh/devicemanager.cpp index 3a98b9db..36c04170 100644 --- a/libguh/devicemanager.cpp +++ b/libguh/devicemanager.cpp @@ -330,7 +330,7 @@ DeviceClass DeviceManager::findDeviceClass(const DeviceClassId &deviceClassId) c /*! Execute the given \{Action}. This will find the \l{Device} \a action refers to in \l{Action::deviceId()} and its \l{DevicePlugin}. Then will dispatch the execution to the \l{DevicePlugin}.*/ -DeviceManager::DeviceError DeviceManager::executeAction(const Action &action) +QPair DeviceManager::executeAction(const Action &action) { foreach (Device *device, m_configuredDevices) { if (action.deviceId() == device->id()) { @@ -345,21 +345,21 @@ DeviceManager::DeviceError DeviceManager::executeAction(const Action &action) qDebug() << "checking params" << actionType.parameters().count() << action.params().count(); qDebug() << "action params:" << action.params(); - if (actionType.parameters().count() > action.params().count()) { - return DeviceErrorMissingParameter; + QPair paramCheck = verifyParams(actionType.parameters(), action.params()); + if (!paramCheck.first) { + return qMakePair(DeviceErrorMissingParameter, paramCheck.second); } - continue; } } if (!found) { - return DeviceErrorActionTypeNotFound; + return qMakePair(DeviceErrorActionTypeNotFound, action.actionTypeId().toString()); } return m_devicePlugins.value(device->pluginId())->executeAction(device, action); } } - return DeviceErrorDeviceNotFound; + return qMakePair(DeviceErrorDeviceNotFound, action.deviceId().toString()); } void DeviceManager::loadPlugins() @@ -562,3 +562,23 @@ bool DeviceManager::setupDevice(Device *device) connect(device, SIGNAL(stateValueChanged(QUuid,QVariant)), this, SLOT(slotDeviceStateValueChanged(QUuid,QVariant))); return true; } + +QPair DeviceManager::verifyParams(const QList paramTypes, const QList params) +{ + foreach (const ParamType ¶mType, paramTypes) { + qDebug() << "checking paramType" << paramType.name(); + bool found = false; + foreach (const Param ¶m, params) { + qDebug() << "checking param" << param.name(); + if (param.name() == paramType.name()) { + if (param.value().canConvert(paramType.type())) { + found = true; + } + } + } + if (!found) { + return qMakePair(false, paramType.name()); + } + } + return qMakePair(true, QString()); +} diff --git a/libguh/devicemanager.h b/libguh/devicemanager.h index 098780b7..a6977b47 100644 --- a/libguh/devicemanager.h +++ b/libguh/devicemanager.h @@ -87,7 +87,7 @@ signals: void devicesDiscovered(const DeviceClassId &deviceClassId, const QList &devices); public slots: - DeviceError executeAction(const Action &action); + QPair executeAction(const Action &action); private slots: void loadPlugins(); @@ -105,6 +105,9 @@ private slots: private: DeviceError addConfiguredDeviceInternal(const DeviceClassId &deviceClassId, const QList ¶ms, const DeviceId id = DeviceId::createDeviceId()); bool setupDevice(Device *device); + QPair verifyParams(const QList paramTypes, const QList params); + +private: QHash m_supportedVendors; QHash > m_vendorDeviceMap; diff --git a/libguh/plugin/deviceplugin.cpp b/libguh/plugin/deviceplugin.cpp index 424b662c..ae92549e 100644 --- a/libguh/plugin/deviceplugin.cpp +++ b/libguh/plugin/deviceplugin.cpp @@ -66,6 +66,15 @@ pure virtual methods: \l{DevicePlugin::pluginName()}, \l{DevicePlugin::pluginId( \fn void DevicePlugin::executeAction(Device *device, const Action &action) This will be called to actually execute actions on the hardware. The \{Device} and the \{Action} are contained in the \a device and \a action parameters. + Use \l{DevicePlugin::report()} to report the result. If everything worked out, + just return report(). Otherwise fill in the error code and a short message + describing the offending part. E.g: + If the action couldn't be executed because the device can't be reached (e.g. it is unplugged) + then report the appropriate error code and give the device id as message: + return report(DeviceManager::DeviceErrorSetupFailed, device->id()); + Keep the message short, the DeviceManager will format it for you. + + \sa DevicePlugin::report() */ /*! @@ -251,3 +260,14 @@ void DevicePlugin::transmitData(QList rawData) } } +/*! + Constructs a status report to be returned. By default (when called without + arguments) this will report \l{DeviceManager::DeviceErrorNoError} and an + empty message. + Keep the message short, the DeviceManager will format it for you. + */ +QPair DevicePlugin::report(DeviceManager::DeviceError error, const QString &message) +{ + return qMakePair(error, message); +} + diff --git a/libguh/plugin/deviceplugin.h b/libguh/plugin/deviceplugin.h index 3d31fbf6..89451c0a 100644 --- a/libguh/plugin/deviceplugin.h +++ b/libguh/plugin/deviceplugin.h @@ -62,9 +62,9 @@ public: virtual void setConfiguration(const QVariantMap &configuration); public slots: - virtual DeviceManager::DeviceError executeAction(Device *device, const Action &action) { + virtual QPair executeAction(Device *device, const Action &action) { Q_UNUSED(device) Q_UNUSED(action) - return DeviceManager::DeviceErrorNoError; + return qMakePair(DeviceManager::DeviceErrorNoError, ""); } signals: @@ -78,6 +78,7 @@ protected: void transmitData(QList rawData); + QPair report(DeviceManager::DeviceError error = DeviceManager::DeviceErrorNoError, const QString &message = QString()); private: void initPlugin(DeviceManager *deviceManager); diff --git a/plugins/deviceplugins/boblight/devicepluginboblight.cpp b/plugins/deviceplugins/boblight/devicepluginboblight.cpp index 959c179b..113a7c58 100644 --- a/plugins/deviceplugins/boblight/devicepluginboblight.cpp +++ b/plugins/deviceplugins/boblight/devicepluginboblight.cpp @@ -133,21 +133,21 @@ void DevicePluginBoblight::setConfiguration(const QVariantMap &configuration) connectToBoblight(); } -DeviceManager::DeviceError DevicePluginBoblight::executeAction(Device *device, const Action &action) +QPair DevicePluginBoblight::executeAction(Device *device, const Action &action) { if (!m_bobClient->connected()) { - return DeviceManager::DeviceErrorSetupFailed; + return report(DeviceManager::DeviceErrorSetupFailed, device->id().toString()); } QColor newColor = action.param("color").value().value(); if (!newColor.isValid()) { - return DeviceManager::DeviceErrorActionParameterError; + return report(DeviceManager::DeviceErrorActionParameterError, "color"); } qDebug() << "executing boblight action" << newColor; m_bobClient->setColor(device->paramValue("channel").toInt(), newColor); m_bobClient->sync(); device->setStateValue(colorStateTypeId, newColor); - return DeviceManager::DeviceErrorNoError; + return report(); } void DevicePluginBoblight::connectToBoblight() diff --git a/plugins/deviceplugins/boblight/devicepluginboblight.h b/plugins/deviceplugins/boblight/devicepluginboblight.h index f6dc81b7..96fbdaac 100644 --- a/plugins/deviceplugins/boblight/devicepluginboblight.h +++ b/plugins/deviceplugins/boblight/devicepluginboblight.h @@ -48,7 +48,7 @@ public: void setConfiguration(const QVariantMap &configuration) override; public slots: - DeviceManager::DeviceError executeAction(Device *device, const Action &action); + QPair executeAction(Device *device, const Action &action); private slots: void connectToBoblight(); diff --git a/plugins/deviceplugins/conrad/devicepluginconrad.cpp b/plugins/deviceplugins/conrad/devicepluginconrad.cpp index 78d9a5b8..bbef79ca 100644 --- a/plugins/deviceplugins/conrad/devicepluginconrad.cpp +++ b/plugins/deviceplugins/conrad/devicepluginconrad.cpp @@ -141,13 +141,13 @@ PluginId DevicePluginConrad::pluginId() const return PluginId("1fd1a076-f229-4ec6-b501-48ddd15935e4"); } -DeviceManager::DeviceError DevicePluginConrad::executeAction(Device *device, const Action &action) +QPair DevicePluginConrad::executeAction(Device *device, const Action &action) { QList rawData; QByteArray binCode; - return DeviceManager::DeviceErrorNoError; + return report(); } void DevicePluginConrad::radioData(QList rawData) diff --git a/plugins/deviceplugins/conrad/devicepluginconrad.h b/plugins/deviceplugins/conrad/devicepluginconrad.h index dd3793d6..73e10ffd 100644 --- a/plugins/deviceplugins/conrad/devicepluginconrad.h +++ b/plugins/deviceplugins/conrad/devicepluginconrad.h @@ -41,7 +41,7 @@ public: void radioData(QList rawData) override; public slots: - DeviceManager::DeviceError executeAction(Device *device, const Action &action) override; + QPair executeAction(Device *device, const Action &action) override; }; diff --git a/plugins/deviceplugins/elro/devicepluginelro.cpp b/plugins/deviceplugins/elro/devicepluginelro.cpp index 18ee99d2..a0bebd85 100644 --- a/plugins/deviceplugins/elro/devicepluginelro.cpp +++ b/plugins/deviceplugins/elro/devicepluginelro.cpp @@ -218,7 +218,7 @@ PluginId DevicePluginElro::pluginId() const return PluginId("2b267f81-d9ae-4f4f-89a0-7386b547cfd3"); } -DeviceManager::DeviceError DevicePluginElro::executeAction(Device *device, const Action &action) +QPair DevicePluginElro::executeAction(Device *device, const Action &action) { QList rawData; @@ -311,7 +311,7 @@ DeviceManager::DeviceError DevicePluginElro::executeAction(Device *device, const //qDebug() << "rawData" << rawData; qDebug() << "transmit" << pluginName() << action.param("power").value().toBool(); transmitData(rawData); - return DeviceManager::DeviceErrorNoError; + return report(); } void DevicePluginElro::radioData(QList rawData) diff --git a/plugins/deviceplugins/elro/devicepluginelro.h b/plugins/deviceplugins/elro/devicepluginelro.h index bbe71b80..05cee206 100644 --- a/plugins/deviceplugins/elro/devicepluginelro.h +++ b/plugins/deviceplugins/elro/devicepluginelro.h @@ -41,7 +41,7 @@ public: void radioData(QList rawData) override; public slots: - DeviceManager::DeviceError executeAction(Device *device, const Action &action) override; + QPair executeAction(Device *device, const Action &action) override; }; diff --git a/plugins/deviceplugins/googlemail/deviceplugingooglemail.cpp b/plugins/deviceplugins/googlemail/deviceplugingooglemail.cpp index df26877e..72a9e5c8 100644 --- a/plugins/deviceplugins/googlemail/deviceplugingooglemail.cpp +++ b/plugins/deviceplugins/googlemail/deviceplugingooglemail.cpp @@ -218,19 +218,19 @@ DeviceManager::HardwareResources DevicePluginGoogleMail::requiredHardware() cons return DeviceManager::HardwareResourceNone; } -DeviceManager::DeviceError DevicePluginGoogleMail::executeAction(Device *device, const Action &action) +QPair DevicePluginGoogleMail::executeAction(Device *device, const Action &action) { qDebug() << "execute action " << sendMailActionTypeId.toString(); if(action.actionTypeId() == sendMailActionTypeId){ if(!m_smtpClient->login(device->paramValue("user").toString(), device->paramValue("password").toString())){ qDebug() << "ERROR: could nt login for sending mail"; - return DeviceManager::DeviceErrorDeviceParameterError; + return report(DeviceManager::DeviceErrorDeviceParameterError, "user, password"); } m_smtpClient->sendMail(device->paramValue("user").toString(), device->paramValue("sendTo").toString(), action.param("subject").value().toString(), action.param("body").value().toString()); m_smtpClient->logout(); } - return DeviceManager::DeviceErrorNoError; + return report(); } QString DevicePluginGoogleMail::pluginName() const diff --git a/plugins/deviceplugins/googlemail/deviceplugingooglemail.h b/plugins/deviceplugins/googlemail/deviceplugingooglemail.h index a9991545..3730ec19 100644 --- a/plugins/deviceplugins/googlemail/deviceplugingooglemail.h +++ b/plugins/deviceplugins/googlemail/deviceplugingooglemail.h @@ -38,7 +38,7 @@ public: bool deviceCreated(Device *device) override; DeviceManager::HardwareResources requiredHardware() const override; - DeviceManager::DeviceError executeAction(Device *device, const Action &action) override; + QPair executeAction(Device *device, const Action &action) override; QString pluginName() const override; PluginId pluginId() const override; diff --git a/plugins/deviceplugins/intertechno/devicepluginintertechno.cpp b/plugins/deviceplugins/intertechno/devicepluginintertechno.cpp index cbdb3972..29ab73d4 100644 --- a/plugins/deviceplugins/intertechno/devicepluginintertechno.cpp +++ b/plugins/deviceplugins/intertechno/devicepluginintertechno.cpp @@ -348,7 +348,7 @@ PluginId DevicePluginIntertechno::pluginId() const return PluginId("e998d934-0397-42c1-ad63-9141bcac8563"); } -DeviceManager::DeviceError DevicePluginIntertechno::executeAction(Device *device, const Action &action) +QPair DevicePluginIntertechno::executeAction(Device *device, const Action &action) { QList rawData; @@ -391,7 +391,7 @@ DeviceManager::DeviceError DevicePluginIntertechno::executeAction(Device *device }else if(familyCode == "P"){ binCode.append("01010101"); }else{ - return DeviceManager::DeviceErrorDeviceParameterError; + return report(); } QString buttonCode = device->paramValue("buttonCode").toString(); @@ -431,7 +431,7 @@ DeviceManager::DeviceError DevicePluginIntertechno::executeAction(Device *device }else if(familyCode == "16"){ binCode.append("01010101"); }else{ - return DeviceManager::DeviceErrorDeviceParameterError; + return report(); } // ======================================= @@ -470,7 +470,7 @@ DeviceManager::DeviceError DevicePluginIntertechno::executeAction(Device *device qDebug() << "transmit" << pluginName() << familyCode << buttonCode << action.param("power").value().toBool(); transmitData(rawData); - return DeviceManager::DeviceErrorNoError; + return report(); } diff --git a/plugins/deviceplugins/intertechno/devicepluginintertechno.h b/plugins/deviceplugins/intertechno/devicepluginintertechno.h index 2f9703ec..4757954d 100644 --- a/plugins/deviceplugins/intertechno/devicepluginintertechno.h +++ b/plugins/deviceplugins/intertechno/devicepluginintertechno.h @@ -41,7 +41,7 @@ public: void radioData(QList rawData) override; public slots: - DeviceManager::DeviceError executeAction(Device *device, const Action &action) override; + QPair executeAction(Device *device, const Action &action) override; }; diff --git a/plugins/deviceplugins/mock/devicepluginmock.cpp b/plugins/deviceplugins/mock/devicepluginmock.cpp index 06a92f6b..81b586e6 100644 --- a/plugins/deviceplugins/mock/devicepluginmock.cpp +++ b/plugins/deviceplugins/mock/devicepluginmock.cpp @@ -180,11 +180,11 @@ bool DevicePluginMock::configureAutoDevice(QList loadedDevices, Device return true; } -DeviceManager::DeviceError DevicePluginMock::executeAction(Device *device, const Action &action) +QPair DevicePluginMock::executeAction(Device *device, const Action &action) { qDebug() << "Should execute action" << action.actionTypeId(); m_daemons.value(device)->actionExecuted(action.actionTypeId()); - return DeviceManager::DeviceErrorNoError; + return report(); } void DevicePluginMock::setState(const StateTypeId &stateTypeId, const QVariant &value) diff --git a/plugins/deviceplugins/mock/devicepluginmock.h b/plugins/deviceplugins/mock/devicepluginmock.h index 9cf66b3f..c6d225dc 100644 --- a/plugins/deviceplugins/mock/devicepluginmock.h +++ b/plugins/deviceplugins/mock/devicepluginmock.h @@ -48,7 +48,7 @@ public: bool configureAutoDevice(QList loadedDevices, Device *device) const override; public slots: - DeviceManager::DeviceError executeAction(Device *device, const Action &action) override; + QPair executeAction(Device *device, const Action &action) override; private slots: void setState(const StateTypeId &stateTypeId, const QVariant &value); diff --git a/plugins/deviceplugins/openweathermap/devicepluginopenweathermap.cpp b/plugins/deviceplugins/openweathermap/devicepluginopenweathermap.cpp index 0b4a3dd6..b0455732 100644 --- a/plugins/deviceplugins/openweathermap/devicepluginopenweathermap.cpp +++ b/plugins/deviceplugins/openweathermap/devicepluginopenweathermap.cpp @@ -442,13 +442,13 @@ DeviceManager::HardwareResources DevicePluginOpenweathermap::requiredHardware() return DeviceManager::HardwareResourceTimer; } -DeviceManager::DeviceError DevicePluginOpenweathermap::executeAction(Device *device, const Action &action) +QPair DevicePluginOpenweathermap::executeAction(Device *device, const Action &action) { qDebug() << "execute action " << updateWeatherActionTypeId.toString(); if(action.actionTypeId() == updateWeatherActionTypeId){ m_openweaher->update(device->paramValue("id").toString()); } - return DeviceManager::DeviceErrorNoError; + return report(); } QString DevicePluginOpenweathermap::pluginName() const diff --git a/plugins/deviceplugins/openweathermap/devicepluginopenweathermap.h b/plugins/deviceplugins/openweathermap/devicepluginopenweathermap.h index b13bdf48..3c997fbe 100644 --- a/plugins/deviceplugins/openweathermap/devicepluginopenweathermap.h +++ b/plugins/deviceplugins/openweathermap/devicepluginopenweathermap.h @@ -41,7 +41,7 @@ public: DeviceManager::DeviceError discoverDevices(const DeviceClassId &deviceClassId, const QVariantMap ¶ms) const override; bool deviceCreated(Device *device) override; DeviceManager::HardwareResources requiredHardware() const override; - DeviceManager::DeviceError executeAction(Device *device, const Action &action) override; + QPair executeAction(Device *device, const Action &action) override; QString pluginName() const override; PluginId pluginId() const override; diff --git a/plugins/deviceplugins/wakeonlan/devicepluginwakeonlan.cpp b/plugins/deviceplugins/wakeonlan/devicepluginwakeonlan.cpp index 459da3aa..0c253cb9 100644 --- a/plugins/deviceplugins/wakeonlan/devicepluginwakeonlan.cpp +++ b/plugins/deviceplugins/wakeonlan/devicepluginwakeonlan.cpp @@ -180,13 +180,13 @@ PluginId DevicePluginWakeOnLan::pluginId() const return PluginId("b5a87848-de56-451e-84a6-edd26ad4958f"); } -DeviceManager::DeviceError DevicePluginWakeOnLan::executeAction(Device *device, const Action &action) +QPair DevicePluginWakeOnLan::executeAction(Device *device, const Action &action) { qDebug() << "execute action " << action.actionTypeId().toString(); if(action.actionTypeId() == wolActionTypeId){ wakeup(device->paramValue("mac").toString()); } - return DeviceManager::DeviceErrorNoError; + return report(); } void DevicePluginWakeOnLan::wakeup(QString mac) diff --git a/plugins/deviceplugins/wakeonlan/devicepluginwakeonlan.h b/plugins/deviceplugins/wakeonlan/devicepluginwakeonlan.h index d74c3142..fc533ae1 100644 --- a/plugins/deviceplugins/wakeonlan/devicepluginwakeonlan.h +++ b/plugins/deviceplugins/wakeonlan/devicepluginwakeonlan.h @@ -39,7 +39,7 @@ public: QString pluginName() const override; PluginId pluginId() const override; - DeviceManager::DeviceError executeAction(Device *device, const Action &action) override; + QPair executeAction(Device *device, const Action &action) override; private slots: diff --git a/server/guhcore.cpp b/server/guhcore.cpp index 0b39a049..5fac6ece 100644 --- a/server/guhcore.cpp +++ b/server/guhcore.cpp @@ -95,18 +95,18 @@ void GuhCore::gotEvent(const Event &event) { foreach (const Action &action, m_ruleEngine->evaluateEvent(event)) { qDebug() << "executing action" << action.actionTypeId(); - DeviceManager::DeviceError status = m_deviceManager->executeAction(action); - switch(status) { + QPair status = m_deviceManager->executeAction(action); + switch(status.first) { case DeviceManager::DeviceErrorNoError: break; case DeviceManager::DeviceErrorSetupFailed: - qDebug() << "Error executing action. Device setup failed."; + qDebug() << "Error executing action. Device setup failed:" << status.second; break; case DeviceManager::DeviceErrorActionParameterError: - qDebug() << "Error executing action. Invalid action parameter."; + qDebug() << "Error executing action. Invalid action parameter:" << status.second; break; default: - qDebug() << "Error executing action:" << status; + qDebug() << "Error executing action:" << status.first << status.second; } } } diff --git a/server/jsonrpc/actionhandler.cpp b/server/jsonrpc/actionhandler.cpp index df9ef565..46a7c9a7 100644 --- a/server/jsonrpc/actionhandler.cpp +++ b/server/jsonrpc/actionhandler.cpp @@ -57,27 +57,27 @@ JsonReply* ActionHandler::ExecuteAction(const QVariantMap ¶ms) QVariantMap returns; - DeviceManager::DeviceError error = GuhCore::instance()->deviceManager()->executeAction(action); + QPair error = GuhCore::instance()->deviceManager()->executeAction(action); - switch (error) { + switch (error.first) { case DeviceManager::DeviceErrorNoError: returns.insert("success", true); returns.insert("errorMessage", ""); break; case DeviceManager::DeviceErrorDeviceNotFound: - returns.insert("errorMessage", "No such device"); + returns.insert("errorMessage", QString("No such device: %1").arg(error.second)); returns.insert("success", false); break; case DeviceManager::DeviceErrorActionTypeNotFound: - returns.insert("errorMessage", "ActionType not found"); + returns.insert("errorMessage", QString("ActionType not found: %1").arg(error.second)); returns.insert("success", false); break; case DeviceManager::DeviceErrorMissingParameter: - returns.insert("errorMessage", "Missing parameter"); + returns.insert("errorMessage", QString("Missing parameter: %1").arg(error.second)); returns.insert("success", false); break; default: - returns.insert("errorMessage", QString("Unknown error %1").arg(error)); + returns.insert("errorMessage", QString("Unknown error %1 %2").arg(error.first).arg(error.second)); returns.insert("success", false); } diff --git a/server/jsonrpc/jsontypes.cpp b/server/jsonrpc/jsontypes.cpp index 48613446..22f93c77 100644 --- a/server/jsonrpc/jsontypes.cpp +++ b/server/jsonrpc/jsontypes.cpp @@ -383,13 +383,15 @@ QVariantMap JsonTypes::packRule(const Rule &rule) Param JsonTypes::unpackParam(const QVariantMap ¶mMap) { - return Param(paramMap.value("name").toString(), paramMap.value("value")); + QString key = paramMap.keys().first(); + return Param(key, paramMap.value(key)); } QList JsonTypes::unpackParams(const QVariantList ¶mList) { QList params; foreach (const QVariant ¶mVariant, paramList) { + qDebug() << "unpacking param" << paramVariant; params.append(unpackParam(paramVariant.toMap())); } return params; diff --git a/tests/scripts/executeaction.sh b/tests/scripts/executeaction.sh index f24b3c1d..bc48b27f 100755 --- a/tests/scripts/executeaction.sh +++ b/tests/scripts/executeaction.sh @@ -7,6 +7,7 @@ elif [ -z $4 ]; then elif [ -z $5 ]; then echo "usage: $0 host actionTypeId deviceId [paramname paramvalue] [paramname paramvalue]" elif [ -z $6 ]; then + echo calling.. $4 $5 (echo '{"id":1, "method":"Actions.ExecuteAction","params":{"actionTypeId": "{'$2'}", "deviceId":"{'$3'}","params":[{"'$4'":"'$5'"}]}}'; sleep 1) | nc $1 1234 elif [ -z $7 ]; then echo "usage: $0 host actionTypeId deviceId [paramname paramvalue] [paramname paramvalue]"