diff --git a/libnymea-core/integrations/thingmanagerimplementation.cpp b/libnymea-core/integrations/thingmanagerimplementation.cpp index 72a65ee4..47f52003 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) @@ -199,10 +199,7 @@ Thing::ThingError ThingManagerImplementation::setPluginConfig(const PluginId &pl settings.beginGroup(plugin->pluginId().toString()); foreach (const Param ¶m, pluginConfig) { - settings.beginGroup(param.paramTypeId().toString()); - settings.setValue("type", static_cast(param.value().type())); - settings.setValue("value", param.value()); - settings.endGroup(); + settings.setValue(param.paramTypeId().toString(), param.value()); } settings.endGroup(); @@ -259,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](){ @@ -396,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); @@ -463,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; } @@ -693,8 +690,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); @@ -702,6 +698,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()) { @@ -1211,14 +1208,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); @@ -1375,42 +1372,25 @@ void ThingManagerImplementation::loadPlugin(IntegrationPlugin *pluginIface, cons NymeaSettings settings(NymeaSettings::SettingsRolePlugins); settings.beginGroup("PluginConfig"); ParamList params; - if (settings.childGroups().contains(pluginIface->pluginId().toString())) { - settings.beginGroup(pluginIface->pluginId().toString()); - if (!settings.childGroups().isEmpty()) { - // Note: since nymea 0.12.2 the param type gets saved too for better data converting - foreach (const QString ¶mTypeIdString, settings.childGroups()) { - ParamTypeId paramTypeId(paramTypeIdString); - ParamType paramType = pluginIface->configurationDescription().findById(paramTypeId); - if (!paramType.isValid()) { - qCWarning(dcThingManager()) << "Not loading Param for plugin" << pluginIface->pluginName() << "because the ParamType for the saved Param" << ParamTypeId(paramTypeIdString).toString() << "could not be found."; - continue; - } - - QVariant paramValue; - settings.beginGroup(paramTypeIdString); - paramValue = settings.value("value", paramType.defaultValue()); - paramValue.convert(settings.value("type").toInt()); - params.append(Param(paramTypeId, paramValue)); - settings.endGroup(); - } - } else { - // Note: < nymea 0.12.2 - foreach (const QString ¶mTypeIdString, settings.allKeys()) { - params.append(Param(ParamTypeId(paramTypeIdString), settings.value(paramTypeIdString))); - } - } - - settings.endGroup(); - } else if (!pluginIface->configurationDescription().isEmpty()){ - // plugin requires config but none stored. Init with defaults - foreach (const ParamType ¶mType, pluginIface->configurationDescription()) { - Param param(paramType.id(), paramType.defaultValue()); - params.append(param); + settings.beginGroup(pluginIface->pluginId().toString()); + foreach (const ParamType ¶mType, pluginIface->configurationDescription()) { + QVariant value = paramType.defaultValue(); + if (settings.contains(paramType.id().toString())) { + value = settings.value(paramType.id().toString()); + } else if (settings.childGroups().contains(paramType.id().toString())) { + // 0.12.2 - 0.22 used to store it in subgroups + settings.beginGroup(paramType.id().toString()); + value = settings.value("value"); + settings.endGroup(); } + value.convert(paramType.type()); + Param param(paramType.id(), value); + params.append(param); } - settings.endGroup(); + settings.endGroup(); // pluginId + + settings.endGroup(); // PluginConfig if (params.count() > 0) { Thing::ThingError status = pluginIface->setConfiguration(params); @@ -1491,69 +1471,70 @@ void ThingManagerImplementation::loadConfiguredThings() ParamList params; settings.beginGroup("Params"); - if (!settings.childGroups().isEmpty()) { - foreach (const QString ¶mTypeIdString, settings.childGroups()) { - ParamTypeId paramTypeId(paramTypeIdString); - ParamType paramType = thingClass.paramTypes().findById(paramTypeId); - QVariant defaultValue; - if (!paramType.isValid()) { - // NOTE: We're not skipping unknown parameters to give plugins a chance to still access old values if they change their config and migrate things over. - qCWarning(dcThingManager()) << "Unknown param" << paramTypeIdString << "for" << thing << ". ParamType could not be found in device class."; - } - // Note: since nymea 0.12.2 - QVariant paramValue; - settings.beginGroup(paramTypeIdString); - paramValue = settings.value("value", paramType.defaultValue()); - paramValue.convert(settings.value("type").toInt()); - params.append(Param(paramTypeId, paramValue)); - settings.endGroup(); // ParamId - } - } else { - foreach (const QString ¶mTypeIdString, settings.allKeys()) { - params.append(Param(ParamTypeId(paramTypeIdString), settings.value(paramTypeIdString))); - } - } - // Make sure all params are around. if they aren't initialize with default values foreach (const ParamType ¶mType, thingClass.paramTypes()) { - if (!params.hasParam(paramType.id())) { - params.append(Param(paramType.id(), paramType.defaultValue())); + QVariant value = paramType.defaultValue(); + if (settings.contains(paramType.id().toString())) { + value = settings.value(paramType.id().toString()); + } else if (settings.childGroups().contains(paramType.id().toString())) { + // 0.12.2 - 0.22 used to store in subgroups + settings.beginGroup(paramType.id().toString()); + value = settings.value("value"); + settings.endGroup(); // paramTypeId + } + value.convert(paramType.type()); + Param param(paramType.id(), value); + params.append(param); + } + + // In order to give plugins a chance to migrate stuff stored in the params (to e.g. pluginStorage()) we'll load + // params that might have disappeared from the ParamTypes but still have stuff stored in the config + foreach (const QString paramTypeIdString, settings.childKeys()) { + ParamTypeId paramTypeId(paramTypeIdString); + if (!params.hasParam(paramTypeId)) { + qCDebug(dcThingManager()) << "Loading legacy param" << paramTypeIdString << "for thing" << thing->name(); + Param param(paramTypeId, settings.value(paramTypeIdString)); + params.append(param); } } - thing->setParams(params); + // 0.12.2 - 0.22 used to store in subgroups + foreach (const QString ¶mTypeIdString, settings.childGroups()) { + settings.beginGroup(paramTypeIdString); + ParamTypeId paramTypeId(paramTypeIdString); + if (!params.hasParam(paramTypeId)) { + qCDebug(dcThingManager()) << "Loading legacy param" << paramTypeIdString << "for thing" << thing->name(); + Param param(paramTypeId, settings.value("value")); + params.append(param); + } + settings.endGroup(); // paramTypeId + } + settings.endGroup(); // Params + thing->setParams(params); + ParamList thingSettings; settings.beginGroup("Settings"); - if (!settings.childGroups().isEmpty()) { - foreach (const QString ¶mTypeIdString, settings.childGroups()) { - ParamTypeId paramTypeId(paramTypeIdString); - ParamType paramType = thingClass.settingsTypes().findById(paramTypeId); - if (!paramType.isValid()) { - qCWarning(dcThingManager()) << "Not loading Setting for thing" << thing << "because the ParamType for the saved Setting" << ParamTypeId(paramTypeIdString).toString() << "could not be found."; - continue; - } - // Note: since nymea 0.12.2 - QVariant paramValue; - settings.beginGroup(paramTypeIdString); - paramValue = settings.value("value", paramType.defaultValue()); - paramValue.convert(settings.value("type").toInt()); - thingSettings.append(Param(paramTypeId, paramValue)); - settings.endGroup(); // ParamId - } - } else { - foreach (const QString ¶mTypeIdString, settings.allKeys()) { - params.append(Param(ParamTypeId(paramTypeIdString), settings.value(paramTypeIdString))); + foreach (const ParamType ¶mType, thingClass.settingsTypes()) { + QVariant value = paramType.defaultValue(); + if (settings.contains(paramType.id().toString())) { + value = settings.value(paramType.id().toString()); + } else if (settings.childGroups().contains(paramType.id().toString())) { + // 0.12.2 - 0.22 used to store in subgroups + settings.beginGroup(paramType.id().toString()); + value = settings.value("value"); + settings.endGroup(); // paramTypeId } + value.convert(paramType.type()); + Param param(paramType.id(), value); + thingSettings.append(param); } - // Fill in any missing params with defaults - thingSettings = buildParams(thingClass.settingsTypes(), thingSettings); + settings.endGroup(); // Settings thing->setSettings(thingSettings); - settings.endGroup(); // Settings settings.endGroup(); // ThingId // We always add the thing to the list in this case. If it's in the stored things @@ -1626,19 +1607,13 @@ void ThingManagerImplementation::storeConfiguredThings() settings.beginGroup("Params"); foreach (const Param ¶m, thing->params()) { - settings.beginGroup(param.paramTypeId().toString()); - settings.setValue("type", static_cast(param.value().type())); - settings.setValue("value", param.value()); - settings.endGroup(); // ParamTypeId + settings.setValue(param.paramTypeId().toString(), param.value()); } settings.endGroup(); // Params settings.beginGroup("Settings"); foreach (const Param ¶m, thing->settings()) { - settings.beginGroup(param.paramTypeId().toString()); - settings.setValue("type", static_cast(param.value().type())); - settings.setValue("value", param.value()); - settings.endGroup(); // ParamTypeId + settings.setValue(param.paramTypeId().toString(), param.value()); } settings.endGroup(); // Settings @@ -1726,19 +1701,19 @@ void ThingManagerImplementation::onAutoThingDisappeared(const ThingId &thingId) Thing *thing = m_configuredThings.value(thingId); if (!thing) { - qWarning(dcThingManager) << "Received an autoThingDisappeared signal but this thing is unknown:" << thingId; + qCWarning(dcThingManager) << "Received an autoThingDisappeared signal but this thing is unknown:" << thingId; return; } ThingClass thingClass = m_supportedThings.value(thing->thingClassId()); if (thingClass.pluginId() != plugin->pluginId()) { - qWarning(dcThingManager) << "Received a autoThingDisappeared signal but emitting plugin does not own the thing"; + qCWarning(dcThingManager) << "Received a autoThingDisappeared signal but emitting plugin does not own the thing"; return; } if (!thing->autoCreated()) { - qWarning(dcThingManager) << "Received an autoThingDisappeared signal but thing creationMethod is not CreateMothodAuto"; + qCWarning(dcThingManager) << "Received an autoThingDisappeared signal but thing creationMethod is not CreateMothodAuto"; return; } @@ -1940,17 +1915,25 @@ 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; if (first.hasParam(paramType.id())) { - finalParams.append(Param(paramType.id(), first.paramValue(paramType.id()))); + value = first.paramValue(paramType.id()); } else if (second.hasParam(paramType.id())) { - finalParams.append(Param(paramType.id(), second.paramValue(paramType.id()))); + value = second.paramValue(paramType.id()); } else if (paramType.defaultValue().isValid()){ - finalParams.append(Param(paramType.id(), paramType.defaultValue())); + value = paramType.defaultValue(); + } + if (!value.isNull()) { + bool success = value.convert(paramType.type()); + if (!success) { + qCWarning(dcThingManager()) << "Type mismatch in param" << paramType.name() << "for value" << value; + } + finalParams.append(Param(paramType.id(), value)); } } return finalParams; @@ -2004,13 +1987,6 @@ ThingSetupInfo* ThingManagerImplementation::setupThing(Thing *thing) ThingClass thingClass = findThingClass(thing->thingClassId()); IntegrationPlugin *plugin = m_integrationPlugins.value(thingClass.pluginId()); - if (!plugin) { - qCWarning(dcThingManager) << "Can't find a plugin for this thing" << thing; - ThingSetupInfo *info = new ThingSetupInfo(thing, this); - info->finish(Thing::ThingErrorPluginNotFound, tr("The plugin for this thing is not loaded.")); - return info; - } - QList states; foreach (const StateType &stateType, thingClass.stateTypes()) { State state(stateType.id(), thing->id()); @@ -2023,8 +1999,14 @@ ThingSetupInfo* ThingManagerImplementation::setupThing(Thing *thing) connect(thing, &Thing::settingChanged, this, &ThingManagerImplementation::slotThingSettingChanged); connect(thing, &Thing::nameChanged, this, &ThingManagerImplementation::slotThingNameChanged); - ThingSetupInfo *info = new ThingSetupInfo(thing, this, 30000); + + if (!plugin) { + qCWarning(dcThingManager) << "Can't find a plugin for this thing" << thing; + info->finish(Thing::ThingErrorPluginNotFound, tr("The plugin for this thing is not loaded.")); + return info; + } + plugin->setupThing(info); return info; @@ -2045,16 +2027,17 @@ void ThingManagerImplementation::loadThingStates(Thing *thing) ThingClass thingClass = m_supportedThings.value(thing->thingClassId()); foreach (const StateType &stateType, thingClass.stateTypes()) { if (stateType.cached()) { - QVariant value; - // First try to load new style - if (settings.childGroups().contains(stateType.id().toString())) { + QVariant value(stateType.defaultValue()); + + if (settings.contains(stateType.id().toString())) { + value = settings.value(stateType.id().toString()); + } else if (settings.childGroups().contains(stateType.id().toString())) { + // 0.9 - 0.22 used to store in a subgroup settings.beginGroup(stateType.id().toString()); - value = settings.value("value", stateType.defaultValue()); - value.convert(settings.value("type").toInt()); + value = settings.value("value"); settings.endGroup(); - } else { // Try to fall back to the pre 0.9.0 way of storing states - value = settings.value(stateType.id().toString(), stateType.defaultValue()); } + value.convert(stateType.type()); thing->setStateValue(stateType.id(), value); } else { thing->setStateValue(stateType.id(), stateType.defaultValue()); @@ -2125,12 +2108,7 @@ void ThingManagerImplementation::storeThingStates(Thing *thing) settings.beginGroup(thing->id().toString()); ThingClass thingClass = m_supportedThings.value(thing->thingClassId()); foreach (const StateType &stateType, thingClass.stateTypes()) { - if (stateType.cached()) { - settings.beginGroup(stateType.id().toString()); - settings.setValue("type", static_cast(thing->stateValue(stateType.id()).type())); - settings.setValue("value", thing->stateValue(stateType.id())); - settings.endGroup(); - } + settings.setValue(stateType.id().toString(), thing->stateValue(stateType.id())); } settings.endGroup(); } diff --git a/libnymea/integrations/pluginmetadata.cpp b/libnymea/integrations/pluginmetadata.cpp index 524043e2..cd71154c 100644 --- a/libnymea/integrations/pluginmetadata.cpp +++ b/libnymea/integrations/pluginmetadata.cpp @@ -359,17 +359,28 @@ void PluginMetadata::parse(const QJsonObject &jsonObject) stateType.setUnit(unitVerification.second); } - stateType.setDefaultValue(st.value("defaultValue").toVariant()); - if (st.contains("minValue")) - stateType.setMinValue(st.value("minValue").toVariant()); + QVariant defaultValue = st.value("defaultValue").toVariant(); + defaultValue.convert(stateType.type()); + stateType.setDefaultValue(defaultValue); - if (st.contains("maxValue")) - stateType.setMaxValue(st.value("maxValue").toVariant()); + if (st.contains("minValue")) { + QVariant minValue = st.value("minValue").toVariant(); + minValue.convert(stateType.type()); + stateType.setMinValue(minValue); + } + + if (st.contains("maxValue")) { + QVariant maxValue = st.value("maxValue").toVariant(); + maxValue.convert(stateType.type()); + stateType.setMaxValue(maxValue); + } if (st.contains("possibleValues")) { QVariantList possibleValues; foreach (const QJsonValue &possibleValueJson, st.value("possibleValues").toArray()) { - possibleValues.append(possibleValueJson.toVariant()); + QVariant possibleValue = possibleValueJson.toVariant(); + possibleValue.convert(stateType.type()); + possibleValues.append(possibleValue); } stateType.setPossibleValues(possibleValues); @@ -824,7 +835,13 @@ QPair PluginMetadata::parseParamTypes(const QJsonArray &array) m_validationErrors.append("Param type \"" + paramName + "\" has duplicate UUID: " + paramTypeId.toString()); hasErrors = true; } - ParamType paramType(paramTypeId, paramName, t, pt.value("defaultValue").toVariant()); + QVariant defaultValue = pt.value("defaultValue").toVariant(); + if (!defaultValue.isNull()) { + // Only convert if there actually is a value as we want it to be null if it isn't specced + // explicitly and convert() would initialize it to the variant's default value + defaultValue.convert(t); + } + ParamType paramType(paramTypeId, paramName, t, defaultValue); paramType.setDisplayName(pt.value("displayName").toString()); @@ -861,7 +878,19 @@ QPair PluginMetadata::parseParamTypes(const QJsonArray &array) paramType.setReadOnly(pt.value("readOnly").toBool()); paramType.setAllowedValues(allowedValues); - paramType.setLimits(pt.value("minValue").toVariant(), pt.value("maxValue").toVariant()); + QVariant minValue = pt.value("minValue").toVariant(); + if (!minValue.isNull()) { + // Only convert if there actually is a value as we want it to be null if it isn't specced + // explicitly and convert() would initialize it to the variant's default value + minValue.convert(t); + } + QVariant maxValue = pt.value("maxValue").toVariant(); + if (!maxValue.isNull()) { + // Only convert if there actually is a value as we want it to be null if it isn't specced + // explicitly and convert() would initialize it to the variant's default value + maxValue.convert(t); + } + paramType.setLimits(minValue, maxValue); paramType.setIndex(index++); paramTypes.append(paramType); } diff --git a/libnymea/integrations/thing.cpp b/libnymea/integrations/thing.cpp index 296f1916..9f3bdbba 100644 --- a/libnymea/integrations/thing.cpp +++ b/libnymea/integrations/thing.cpp @@ -327,23 +327,49 @@ QVariant Thing::stateValue(const StateTypeId &stateTypeId) const /*! For convenience, this finds the \l{State} matching the given \a stateTypeId in this thing and sets the current value to \a value. */ void Thing::setStateValue(const StateTypeId &stateTypeId, const QVariant &value) { + StateType stateType = m_thingClass.stateTypes().findById(stateTypeId); + if (!stateType.isValid()) { + qCWarning(dcThing()) << "No such state type" << stateTypeId.toString() << "in" << m_name << "(" + thingClass().name() + ")"; + return; + } for (int i = 0; i < m_states.count(); ++i) { if (m_states.at(i).stateTypeId() == stateTypeId) { if (m_states.at(i).value() == value) return; + QVariant newValue = value; + if (!newValue.convert(stateType.type())) { + qCWarning(dcThing()).nospace() << m_name << ": Invalid value " << value << " for state " << stateType.name() << ". Type mismatch. Expected type: " << QVariant::typeToName(stateType.type()) << " (Discarding change)"; + return; + } + if (stateType.minValue().isValid() && value < stateType.minValue()) { + qCWarning(dcThing()).nospace() << m_name << ": Invalid value " << value << " for state " << stateType.name() << ". Out of range: " << stateType.minValue() << " - " << stateType.maxValue() << " (Correcting to closest value within range)"; + newValue = stateType.minValue(); + } + if (stateType.maxValue().isValid() && value > stateType.maxValue()) { + qCWarning(dcThing()).nospace() << m_name << ": Invalid value " << value << " for state " << stateType.name() << ". Out of range: " << stateType.minValue() << " - " << stateType.maxValue() << " (Correcting to closest value within range)"; + newValue = stateType.maxValue(); + } + if (!stateType.possibleValues().isEmpty() && !stateType.possibleValues().contains(value)) { + qCWarning(dcThing()).nospace() << m_name << ": Invalid value " << value << " for state " << stateType.name() << ". Not an accepted value. Possible values: " << stateType.possibleValues() << " (Discarding change)"; + return; + } - // TODO: check min/max value + possible values - // to prevent an invalid state type from the plugin side + QVariant oldValue = m_states.at(i).value(); - State newState(stateTypeId, m_id); - newState.setValue(value); - m_states[i] = newState; + if (oldValue == newValue) { + qCDebug(dcThing()).nospace() << m_name << ": Discarding state change for " << stateType.name() << " as the value did not actually change. Old value:" << oldValue << "New value:" << newValue; + return; + } + + qCDebug(dcThing()).nospace() << m_name << ": State " << stateType.name() << " changed from " << oldValue << " to " << newValue; + m_states[i].setValue(newValue); emit stateValueChanged(stateTypeId, value); return; } } - qCWarning(dcThingManager) << "Failed setting state for" << m_name << value; + Q_ASSERT_X(false, m_name.toUtf8(), QString("Failed setting state %1 to %2").arg(stateType.name()).arg(value.toString()).toUtf8()); + qCWarning(dcThing).nospace() << m_name << ": Failed setting state " << stateType.name() << "to" << value; } /*! Returns the \l{State} with the given \a stateTypeId of this thing. */ 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/libnymea/types/statedescriptor.cpp b/libnymea/types/statedescriptor.cpp index 22e0de29..2dc1e84b 100644 --- a/libnymea/types/statedescriptor.cpp +++ b/libnymea/types/statedescriptor.cpp @@ -171,11 +171,11 @@ bool StateDescriptor::operator ==(const State &state) const if ((m_stateTypeId != state.stateTypeId()) || (m_thingId != state.thingId())) { return false; } - if (!m_stateValue.canConvert(state.value().type())) { + QVariant convertedValue = m_stateValue; + bool res = convertedValue.convert(state.value().type()); + if (!res) { return false; } - QVariant convertedValue = m_stateValue; - convertedValue.convert(state.value().type()); switch (m_operatorType) { case Types::ValueOperatorEquals: return convertedValue == state.value(); diff --git a/libnymea/types/statetype.cpp b/libnymea/types/statetype.cpp index ab505a90..2550380b 100644 --- a/libnymea/types/statetype.cpp +++ b/libnymea/types/statetype.cpp @@ -222,6 +222,12 @@ QStringList StateType::mandatoryTypeProperties() return QStringList() << "id" << "name" << "displayName" << "displayNameEvent" << "type" << "defaultValue"; } +/*! Returns true if this state type has an ID, a type and a name set. */ +bool StateType::isValid() const +{ + return !m_id.isNull() && m_type != QVariant::Invalid && !m_name.isEmpty(); +} + StateTypes::StateTypes(const QList &other) { foreach (const StateType &st, other) { diff --git a/libnymea/types/statetype.h b/libnymea/types/statetype.h index 0bfdd6ee..f87e91eb 100644 --- a/libnymea/types/statetype.h +++ b/libnymea/types/statetype.h @@ -96,6 +96,8 @@ public: static QStringList typeProperties(); static QStringList mandatoryTypeProperties(); + bool isValid() const; + private: StateTypeId m_id; QString m_name; diff --git a/plugins/mock/integrationpluginmock.json b/plugins/mock/integrationpluginmock.json index 35d4fdd6..14953bec 100644 --- a/plugins/mock/integrationpluginmock.json +++ b/plugins/mock/integrationpluginmock.json @@ -98,7 +98,7 @@ "name": "double", "displayName": "Dummy double state", "displayNameEvent": "Dummy double state changed", - "type": "int", + "type": "double", "minValue": 0, "maxValue": 100, "defaultValue": 2.7 diff --git a/tests/auto/actions/testactions.cpp b/tests/auto/actions/testactions.cpp index dc028e7b..2cc576fb 100644 --- a/tests/auto/actions/testactions.cpp +++ b/tests/auto/actions/testactions.cpp @@ -85,7 +85,6 @@ void TestActions::executeAction() params.insert("deviceId", deviceId); params.insert("params", actionParams); QVariant response = injectAndWait("Actions.ExecuteAction", params); - qDebug() << "executeActionresponse" << response; verifyError(response, "deviceError", enumValueName(error)); // Fetch action execution history from mock device 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; } diff --git a/tests/auto/integrations/testintegrations.cpp b/tests/auto/integrations/testintegrations.cpp index 1eced86d..b3c6e373 100644 --- a/tests/auto/integrations/testintegrations.cpp +++ b/tests/auto/integrations/testintegrations.cpp @@ -382,10 +382,10 @@ void TestIntegrations::addThing_data() QTest::newRow("User, JustAdd, wrong param") << mockThingClassId << invalidThingParams << true << Thing::ThingErrorInvalidParameter; thingParams.clear(); thingParams << httpportParam << fakeparam; - QTest::newRow("USer, JustAdd, additional invalid param") << mockThingClassId << thingParams << false << Thing::ThingErrorNoError; + QTest::newRow("USer, JustAdd, additional invalid param") << mockThingClassId << thingParams << false << Thing::ThingErrorInvalidParameter; thingParams.clear(); thingParams << httpportParam << fakeparam2; - QTest::newRow("USer, JustAdd, additional param, valid but unused") << mockThingClassId << thingParams << true << Thing::ThingErrorNoError; + QTest::newRow("USer, JustAdd, duplicate param") << mockThingClassId << thingParams << true << Thing::ThingErrorInvalidParameter; } diff --git a/tests/auto/rules/testrules.cpp b/tests/auto/rules/testrules.cpp index 0f5c986a..df196e67 100644 --- a/tests/auto/rules/testrules.cpp +++ b/tests/auto/rules/testrules.cpp @@ -265,13 +265,14 @@ void TestRules::setWritableStateValue(const ThingId &thingId, const StateTypeId QVariantList stateChangedVariants = checkNotifications(stateSpy, "Integrations.StateChanged"); QVERIFY2(stateChangedVariants.count() == 1, "Did not get Integrations.StateChanged notification."); + qCDebug(dcTests()) << "Notification content:" << qUtf8Printable(QJsonDocument::fromVariant(stateChangedVariants).toJson()); QVariantMap notification = stateChangedVariants.first().toMap().value("params").toMap(); QVERIFY2(notification.contains("thingId"), "Integrations.StateChanged notification does not contain thingId"); QVERIFY2(ThingId(notification.value("thingId").toString()) == thingId, "Integrations.StateChanged notification does not contain the correct thingId"); QVERIFY2(notification.contains("stateTypeId"), "Integrations.StateChanged notification does not contain stateTypeId"); QVERIFY2(StateTypeId(notification.value("stateTypeId").toString()) == stateTypeId, "Integrations.StateChanged notification does not contain the correct stateTypeId"); QVERIFY2(notification.contains("value"), "Integrations.StateChanged notification does not contain new state value"); - QVERIFY2(notification.value("value") == QVariant(value), "Integrations.StateChanged notification does not contain the new value"); + QVERIFY2(notification.value("value") == QVariant(value), QString("Integrations.StateChanged notification does not contain the new value. Got: %1, Expected: %2").arg(notification.value("value").toString()).arg(QVariant(value).toString()).toUtf8()); } } @@ -1990,11 +1991,11 @@ void TestRules::testChildEvaluator_data() QTest::addColumn("trigger"); QTest::addColumn("active"); - QTest::newRow("Unchanged | 2 | 2.5 | String value 1 | #FF0000") << testThingId << ruleMap << 2 << 2.5 << "String value 1" << "#FF0000" << false << false; - QTest::newRow("Unchanged | 60 | 2.5 | String value 2 | #FF0000") << testThingId << ruleMap << 60 << 2.5 << "String value 2" << "#FF0000" << false << false; - QTest::newRow("Unchanged | 60 | 20.5 | String value 2 | #FF0000") << testThingId << ruleMap << 60 << 20.5 << "String value 2" << "#FF0000" << false << false; - QTest::newRow("Active | 60 | 20.5 | String value 2 | #00FF00") << testThingId << ruleMap << 60 << 20.5 << "String value 2" << "#00FF00" << true << true; - QTest::newRow("Active | 60 | 20.5 | String value 2 | #00FF00") << testThingId << ruleMap << 60 << 20.5 << "String value 2" << "#00FF00" << true << true; + QTest::newRow("Unchanged | 2 | 2.5 | String value 1 | #FF0000") << testThingId << ruleMap << 2 << 2.5 << "String value 1" << "#ff0000" << false << false; + QTest::newRow("Unchanged | 60 | 2.5 | String value 2 | #FF0000") << testThingId << ruleMap << 60 << 2.5 << "String value 2" << "#ff0000" << false << false; + QTest::newRow("Unchanged | 60 | 20.5 | String value 2 | #FF0000") << testThingId << ruleMap << 60 << 20.5 << "String value 2" << "#ff0000" << false << false; + QTest::newRow("Active | 60 | 20.5 | String value 2 | #00FF00") << testThingId << ruleMap << 60 << 20.5 << "String value 2" << "#00ff00" << true << true; + QTest::newRow("Active | 60 | 20.5 | String value 2 | #00FF00") << testThingId << ruleMap << 60 << 20.5 << "String value 2" << "#00ff00" << true << true; } void TestRules::testChildEvaluator() diff --git a/tests/testlib/nymeatestbase.h b/tests/testlib/nymeatestbase.h index ef98d6d1..dc84f738 100644 --- a/tests/testlib/nymeatestbase.h +++ b/tests/testlib/nymeatestbase.h @@ -118,7 +118,7 @@ protected: // just for debugging inline void printJson(const QVariant &response) { QJsonDocument jsonDoc = QJsonDocument::fromVariant(response); - qDebug() << jsonDoc.toJson(); + qCDebug(dcTests()) << jsonDoc.toJson(); } void waitForDBSync();