diff --git a/libnymea-core/integrations/thingmanagerimplementation.cpp b/libnymea-core/integrations/thingmanagerimplementation.cpp index 0c858a83..73813907 100644 --- a/libnymea-core/integrations/thingmanagerimplementation.cpp +++ b/libnymea-core/integrations/thingmanagerimplementation.cpp @@ -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(); @@ -1373,42 +1370,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); @@ -1489,69 +1469,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 @@ -1622,19 +1603,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 @@ -1939,13 +1914,21 @@ ParamList ThingManagerImplementation::buildParams(const ParamTypes &types, const // 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; } @@ -1998,13 +1981,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()); @@ -2017,8 +1993,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; @@ -2039,16 +2021,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()); @@ -2119,12 +2102,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 0ec62516..65f040b3 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/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/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