diff --git a/libnymea-core/integrations/thingmanagerimplementation.cpp b/libnymea-core/integrations/thingmanagerimplementation.cpp index 73813907..2de8e413 100644 --- a/libnymea-core/integrations/thingmanagerimplementation.cpp +++ b/libnymea-core/integrations/thingmanagerimplementation.cpp @@ -185,10 +185,10 @@ Thing::ThingError ThingManagerImplementation::setPluginConfig(const PluginId &pl return Thing::ThingErrorPluginNotFound; } - ParamList params = buildParams(plugin->configurationDescription(), pluginConfig); - Thing::ThingError verify = ThingUtils::verifyParams(plugin->configurationDescription(), params); + Thing::ThingError verify = ThingUtils::verifyParams(plugin->configurationDescription(), pluginConfig); if (verify != Thing::ThingErrorNoError) return verify; + ParamList params = buildParams(plugin->configurationDescription(), pluginConfig); Thing::ThingError result = plugin->setConfiguration(params); if (result != Thing::ThingErrorNoError) @@ -256,14 +256,14 @@ ThingDiscoveryInfo* ThingManagerImplementation::discoverThings(const ThingClassI return discoveryInfo; } - ParamList effectiveParams = buildParams(thingClass.discoveryParamTypes(), params); - Thing::ThingError result = ThingUtils::verifyParams(thingClass.discoveryParamTypes(), effectiveParams); + Thing::ThingError result = ThingUtils::verifyParams(thingClass.discoveryParamTypes(), params); if (result != Thing::ThingErrorNoError) { qCWarning(dcThingManager) << "Thing discovery failed. Parameter verification failed."; ThingDiscoveryInfo *discoveryInfo = new ThingDiscoveryInfo(thingClassId, params, this); discoveryInfo->finish(result); return discoveryInfo; } + ParamList effectiveParams = buildParams(thingClass.discoveryParamTypes(), params); ThingDiscoveryInfo *discoveryInfo = new ThingDiscoveryInfo(thingClassId, effectiveParams, this, 30000); connect(discoveryInfo, &ThingDiscoveryInfo::finished, this, [this, discoveryInfo](){ @@ -393,14 +393,14 @@ ThingSetupInfo *ThingManagerImplementation::reconfigureThingInternal(Thing *thin return info; } - ParamList finalParams = buildParams(thing->thingClass().paramTypes(), params); - Thing::ThingError result = ThingUtils::verifyParams(thing->thingClass().paramTypes(), finalParams); + Thing::ThingError result = ThingUtils::verifyParams(thing->thingClass().paramTypes(), params); if (result != Thing::ThingErrorNoError) { qCWarning(dcThingManager()) << "Cannot reconfigure thing. Params failed validation."; ThingSetupInfo *info = new ThingSetupInfo(nullptr, this); info->finish(result); return info; } + ParamList finalParams = buildParams(thing->thingClass().paramTypes(), params); // first remove the thing in the plugin plugin->thingRemoved(thing); @@ -460,13 +460,13 @@ Thing::ThingError ThingManagerImplementation::setThingSettings(const ThingId &th qCWarning(dcThingManager()) << "Cannot set thing settings. Thing" << thingId.toString() << "not found"; return Thing::ThingErrorThingNotFound; } - // build a list of settings using: a) provided new settings b) previous settings and c) default values - ParamList effectiveSettings = buildParams(thing->thingClass().settingsTypes(), settings, thing->settings()); - Thing::ThingError status = ThingUtils::verifyParams(thing->thingClass().settingsTypes(), effectiveSettings); + Thing::ThingError status = ThingUtils::verifyParams(thing->thingClass().settingsTypes(), settings); if (status != Thing::ThingErrorNoError) { qCWarning(dcThingManager()) << "Error setting thing settings for" << thing->name() << thing->id().toString(); return status; } + // build a list of settings using: a) provided new settings b) previous settings and c) default values + ParamList effectiveSettings = buildParams(thing->thingClass().settingsTypes(), settings, thing->settings()); thing->setSettings(effectiveSettings); return Thing::ThingErrorNoError; } @@ -689,8 +689,7 @@ ThingSetupInfo* ThingManagerImplementation::addConfiguredThingInternal(const Thi } // set params - ParamList effectiveParams = buildParams(thingClass.paramTypes(), params); - Thing::ThingError paramsResult = ThingUtils::verifyParams(thingClass.paramTypes(), effectiveParams); + Thing::ThingError paramsResult = ThingUtils::verifyParams(thingClass.paramTypes(), params); if (paramsResult != Thing::ThingErrorNoError) { qCWarning(dcThingManager()) << "Cannot add thing. Parameter verification failed."; ThingSetupInfo *info = new ThingSetupInfo(nullptr, this); @@ -698,6 +697,7 @@ ThingSetupInfo* ThingManagerImplementation::addConfiguredThingInternal(const Thi return info; } + ParamList effectiveParams = buildParams(thingClass.paramTypes(), params); Thing *thing = new Thing(plugin->pluginId(), thingClass, thingId, this); thing->setParentId(parentId); if (name.isEmpty()) { @@ -1206,14 +1206,14 @@ ThingActionInfo *ThingManagerImplementation::executeAction(const Action &action) return info; } - ParamList finalParams = buildParams(actionType.paramTypes(), action.params()); - Thing::ThingError paramCheck = ThingUtils::verifyParams(actionType.paramTypes(), finalParams); + Thing::ThingError paramCheck = ThingUtils::verifyParams(actionType.paramTypes(), action.params()); if (paramCheck != Thing::ThingErrorNoError) { qCWarning(dcThingManager()) << "Cannot execute action. Parameter verification failed."; ThingActionInfo *info = new ThingActionInfo(thing, action, this); info->finish(paramCheck); return info; } + ParamList finalParams = buildParams(actionType.paramTypes(), action.params()); finalAction.setParams(finalParams); ThingActionInfo *info = new ThingActionInfo(thing, finalAction, this, 30000); @@ -1909,9 +1909,9 @@ void ThingManagerImplementation::slotThingNameChanged() emit thingChanged(thing); } +// Merges params from first and second. First has higher priority than second. If neither are given, the default is used - if any ParamList ThingManagerImplementation::buildParams(const ParamTypes &types, const ParamList &first, const ParamList &second) { - // Merge params from discovered descriptor and additional overrides provided on API call. User provided params have higher priority than discovery params. ParamList finalParams; foreach (const ParamType ¶mType, types) { QVariant value; @@ -1927,8 +1927,8 @@ ParamList ThingManagerImplementation::buildParams(const ParamTypes &types, const if (!success) { qCWarning(dcThingManager()) << "Type mismatch in param" << paramType.name() << "for value" << value; } + finalParams.append(Param(paramType.id(), value)); } - finalParams.append(Param(paramType.id(), value)); } return finalParams; } diff --git a/libnymea/integrations/thingutils.cpp b/libnymea/integrations/thingutils.cpp index 22522203..8a1c97bb 100644 --- a/libnymea/integrations/thingutils.cpp +++ b/libnymea/integrations/thingutils.cpp @@ -41,8 +41,7 @@ ThingUtils::ThingUtils() } -/*! Verify if the given \a params matches the given \a paramTypes. Ith \a requireAll - * is true, all \l{ParamList}{Params} has to be valid. Returns \l{Device::DeviceError} to inform about the result.*/ +/*! Verify if the given \a params matches the given \a paramTypes.*/ Thing::ThingError ThingUtils::verifyParams(const QList paramTypes, const ParamList ¶ms) { foreach (const Param ¶m, params) { @@ -52,7 +51,7 @@ Thing::ThingError ThingUtils::verifyParams(const QList paramTypes, co } } foreach (const ParamType ¶mType, paramTypes) { - bool found = false; + bool found = !paramType.defaultValue().isNull(); foreach (const Param ¶m, params) { if (paramType.id() == param.paramTypeId()) { found = true; diff --git a/tests/auto/devices/testdevices.cpp b/tests/auto/devices/testdevices.cpp index 998aed0e..bc2f5d4c 100644 --- a/tests/auto/devices/testdevices.cpp +++ b/tests/auto/devices/testdevices.cpp @@ -382,10 +382,10 @@ void TestDevices::addConfiguredDevice_data() QTest::newRow("User, JustAdd, wrong param") << mockThingClassId << invalidDeviceParams << true << Device::DeviceErrorInvalidParameter; deviceParams.clear(); deviceParams << httpportParam << fakeparam; - QTest::newRow("USer, JustAdd, additional invalid param") << mockThingClassId << deviceParams << false << Device::DeviceErrorNoError; + QTest::newRow("User, JustAdd, additional invalid param") << mockThingClassId << deviceParams << false << Device::DeviceErrorInvalidParameter; deviceParams.clear(); deviceParams << httpportParam << fakeparam2; - QTest::newRow("USer, JustAdd, additional param, valid but unused") << mockThingClassId << deviceParams << true << Device::DeviceErrorNoError; + QTest::newRow("User, JustAdd, duplicate param") << mockThingClassId << deviceParams << true << Device::DeviceErrorInvalidParameter; }