Add value clamping for Param and State values

libnymea/integrations/thing.cpp

@@ -125,33 +125,29 @@
 */
 
 #include "thing.h"
-#include "types/event.h"
 #include "loggingcategories.h"
 #include "statevaluefilters/statevaluefilteradaptive.h"
 #include "thingutils.h"
+#include "types/event.h"
 
-#include <QJsonDocument>
 #include <QDebug>
+#include <QJsonDocument>
 
 /*! Construct a Thing with the given \a pluginId, \a id, \a thingClassId and \a parent. */
-Thing::Thing(const PluginId &pluginId, const ThingClass &thingClass, const ThingId &id, QObject *parent):
+Thing::Thing(const PluginId &pluginId, const ThingClass &thingClass, const ThingId &id, QObject *parent)
-    QObject(parent),
+    : QObject(parent)
-    m_thingClass(thingClass),
+    , m_thingClass(thingClass)
-    m_pluginId(pluginId),
+    , m_pluginId(pluginId)
-    m_id(id)
+    , m_id(id)
-{
+{}
-
-}
 
 /*! Construct a Thing with the given \a pluginId, \a thingClassId and \a parent. A new ThingId will be created for this Thing. */
-Thing::Thing(const PluginId &pluginId, const ThingClass &thingClass, QObject *parent):
+Thing::Thing(const PluginId &pluginId, const ThingClass &thingClass, QObject *parent)
-    QObject(parent),
+    : QObject(parent)
-    m_thingClass(thingClass),
+    , m_thingClass(thingClass)
-    m_pluginId(pluginId),
+    , m_pluginId(pluginId)
-    m_id(ThingId::createThingId())
+    , m_id(ThingId::createThingId())
-{
+{}
-
-}
 
 Thing::~Thing()
 {
@@ -376,23 +372,99 @@ void Thing::setStateValue(const StateTypeId &stateTypeId, const QVariant &value)
         if (m_states.at(i).stateTypeId() == stateTypeId) {
             QVariant newValue = value;
             if (!newValue.convert(stateType.type())) {
-                qCWarning(dcThing()).nospace() << this << ": Invalid value " << value << " for state " << stateType.name() << ". Type mismatch. Expected type: " << QVariant::typeToName(stateType.type()) << " (Discarding change)";
+                qCWarning(dcThing()).nospace()
+                    << this
+                    << ": Invalid value "
+                    << value
+                    << " for state "
+                    << stateType.name()
+                    << ". Type mismatch. Expected type: "
+                    << QVariant::typeToName(stateType.type())
+                    << " (Discarding change)";
                 return;
             }
             State state = m_states.at(i);
             if (state.minValue().isValid() && ThingUtils::variantLessThan(value, state.minValue())) {
-                qCWarning(dcThing()).nospace() << this << ": Invalid value " << value << " for state " << stateType.name() << ". Out of range: " << state.minValue() << " - " << state.maxValue() << " (Correcting to closest value within range)";
+                qCWarning(dcThing()).nospace()
+                    << this
+                    << ": Invalid value "
+                    << value
+                    << " for state "
+                    << stateType.name()
+                    << ". Out of range: "
+                    << state.minValue()
+                    << " - "
+                    << state.maxValue()
+                    << " (Correcting to closest value within range)";
                 newValue = state.minValue();
             }
             if (state.maxValue().isValid() && ThingUtils::variantGreaterThan(value, state.maxValue())) {
-                qCWarning(dcThing()).nospace() << this << ": Invalid value " << value << " for state " << stateType.name() << ". Out of range: " << state.minValue() << " - " << state.maxValue() << " (Correcting to closest value within range)";
+                qCWarning(dcThing()).nospace()
+                    << this
+                    << ": Invalid value "
+                    << value
+                    << " for state "
+                    << stateType.name()
+                    << ". Out of range: "
+                    << state.minValue()
+                    << " - "
+                    << state.maxValue()
+                    << " (Correcting to closest value within range)";
                 newValue = state.maxValue();
             }
             if (!stateType.possibleValues().isEmpty() && !stateType.possibleValues().contains(value)) {
-                qCWarning(dcThing()).nospace() << this << ": Invalid value " << value << " for state " << stateType.name() << ". Not an accepted value. Possible values: " << stateType.possibleValues() << " (Discarding change)";
+                qCWarning(dcThing()).nospace()
+                    << this
+                    << ": Invalid value "
+                    << value
+                    << " for state "
+                    << stateType.name()
+                    << ". Not an accepted value. Possible values: "
+                    << stateType.possibleValues()
+                    << " (Discarding change)";
                 return;
             }
 
+            QVariant clampedValue = ThingUtils::ensureValueClamping(newValue, stateType.type(), state.minValue(), state.maxValue(), stateType.stepSize());
+            if (stateType.stepSize() != 0) {
+                const double stepEpsilon = qMax(qAbs(stateType.stepSize()) * 1e-9, 1e-12);
+                bool stepAdjusted = false;
+                switch (stateType.type()) {
+                case QMetaType::Double:
+                    stepAdjusted = qAbs(newValue.toDouble() - clampedValue.toDouble()) > stepEpsilon;
+                    break;
+                case QMetaType::Float:
+                    stepAdjusted = qAbs(newValue.toFloat() - clampedValue.toFloat()) > stepEpsilon;
+                    break;
+                case QMetaType::Int:
+                case QMetaType::UInt:
+                case QMetaType::LongLong:
+                case QMetaType::ULongLong:
+                case QMetaType::Short:
+                case QMetaType::ULong:
+                case QMetaType::UShort:
+                    stepAdjusted = (newValue != clampedValue);
+                    break;
+                default:
+                    break;
+                }
+
+                if (stepAdjusted) {
+                    newValue = clampedValue;
+                    qCWarning(dcThing()).nospace()
+                        << this
+                        << ": Invalid value "
+                        << value
+                        << " for state "
+                        << stateType.name()
+                        << ". Step size: "
+                        << stateType.stepSize()
+                        << " (Correcting to closest value within step size)";
+                }
+            } else {
+                newValue = clampedValue;
+            }
+
             StateValueFilter *filter = m_stateValueFilters.value(stateTypeId);
             if (filter) {
                 filter->addValue(newValue);
@@ -402,7 +474,14 @@ void Thing::setStateValue(const StateTypeId &stateTypeId, const QVariant &value)
 
             QVariant oldValue = m_states.at(i).value();
             if (oldValue == newValue) {
-                qCDebug(dcThing()).nospace() << this << ": Discarding state change for " << stateType.name() << " as the value did not actually change. Old value:" << oldValue << "New value:" << newValue;
+                qCDebug(dcThing()).nospace()
+                    << this
+                    << ": Discarding state change for "
+                    << stateType.name()
+                    << " as the value did not actually change. Old value:"
+                    << oldValue
+                    << "New value:"
+                    << newValue;
                 return;
             }
 
@@ -448,7 +527,14 @@ void Thing::setStateMinValue(const StateTypeId &stateTypeId, const QVariant &min
 
             // Sanity check for max >= min
             if (ThingUtils::variantLessThan(m_states.at(i).maxValue(), newMin)) {
-                qCWarning(dcThing()).nospace() << this << ": Adjusting state maximum value for " << stateType.name() << " from " << m_states.at(i).maxValue() << " to new minimum value of " << newMin;
+                qCWarning(dcThing()).nospace()
+                    << this
+                    << ": Adjusting state maximum value for "
+                    << stateType.name()
+                    << " from "
+                    << m_states.at(i).maxValue()
+                    << " to new minimum value of "
+                    << newMin;
                 m_states[i].setMaxValue(newMin);
             }
             if (ThingUtils::variantLessThan(m_states.at(i).value(), newMin)) {
@@ -492,12 +578,26 @@ void Thing::setStateMaxValue(const StateTypeId &stateTypeId, const QVariant &max
             if (newMax.isValid()) {
                 // Sanity check for min <= max
                 if (ThingUtils::variantGreaterThan(m_states.at(i).minValue(), newMax)) {
-                    qCWarning(dcThing()).nospace() << this << ": Adjusting minimum state value for " << stateType.name() << " from " << m_states.at(i).minValue() << " to new maximum value of " << newMax;
+                    qCWarning(dcThing()).nospace()
+                        << this
+                        << ": Adjusting minimum state value for "
+                        << stateType.name()
+                        << " from "
+                        << m_states.at(i).minValue()
+                        << " to new maximum value of "
+                        << newMax;
                     m_states[i].setMinValue(newMax);
                 }
 
                 if (ThingUtils::variantGreaterThan(m_states.at(i).value(), newMax)) {
-                    qCInfo(dcThing()).nospace() << this << ": Adjusting state value for " << stateType.name() << " from " << m_states.at(i).value() << " to new maximum value of " << newMax;
+                    qCInfo(dcThing()).nospace()
+                        << this
+                        << ": Adjusting state value for "
+                        << stateType.name()
+                        << " from "
+                        << m_states.at(i).value()
+                        << " to new maximum value of "
+                        << newMax;
                     m_states[i].setValue(maxValue);
                 }
             }
@@ -539,16 +639,37 @@ void Thing::setStateMinMaxValues(const StateTypeId &stateTypeId, const QVariant
             if (newMax.isValid() || newMax.isValid()) {
                 // Sanity check for min <= max
                 if (ThingUtils::variantGreaterThan(newMin, newMax)) {
-                    qCWarning(dcThing()).nospace() << this << ": Adjusting maximum state value for " << stateType.name() << " from " << m_states.at(i).maxValue() << " to new minimum value of " << newMax;
+                    qCWarning(dcThing()).nospace()
+                        << this
+                        << ": Adjusting maximum state value for "
+                        << stateType.name()
+                        << " from "
+                        << m_states.at(i).maxValue()
+                        << " to new minimum value of "
+                        << newMax;
                     m_states[i].setMaxValue(newMin);
                 }
 
                 if (ThingUtils::variantLessThan(m_states.at(i).value(), m_states.at(i).minValue())) {
-                    qCInfo(dcThing()).nospace() << this << ": Adjusting state value for " << stateType.name() << " from " << m_states.at(i).value() << " to new minimum value of " << m_states.at(i).minValue();
+                    qCInfo(dcThing()).nospace()
+                        << this
+                        << ": Adjusting state value for "
+                        << stateType.name()
+                        << " from "
+                        << m_states.at(i).value()
+                        << " to new minimum value of "
+                        << m_states.at(i).minValue();
                     m_states[i].setValue(m_states.at(i).minValue());
                 }
                 if (ThingUtils::variantGreaterThan(m_states.at(i).value(), m_states.at(i).maxValue())) {
-                    qCInfo(dcThing()).nospace() << this << ": Adjusting state value for " << stateType.name() << " from " << m_states.at(i).value() << " to new maximum value of " << m_states.at(i).maxValue();
+                    qCInfo(dcThing()).nospace()
+                        << this
+                        << ": Adjusting state value for "
+                        << stateType.name()
+                        << " from "
+                        << m_states.at(i).value()
+                        << " to new maximum value of "
+                        << m_states.at(i).maxValue();
                     m_states[i].setValue(m_states.at(i).maxValue());
                 }
             }
@@ -559,7 +680,6 @@ void Thing::setStateMinMaxValues(const StateTypeId &stateTypeId, const QVariant
     }
     Q_ASSERT_X(false, m_name.toUtf8(), QString("Failed setting maximum state value %1 to %2").arg(stateType.name()).arg(maxValue.toString()).toUtf8());
     qCWarning(dcThing()).nospace() << this << ": Failed setting maximum state value " << stateType.name() << " to " << maxValue;
-
 }
 
 void Thing::setStateMinMaxValues(const QString &stateName, const QVariant &minValue, const QVariant &maxValue)
@@ -585,10 +705,24 @@ void Thing::setStatePossibleValues(const StateTypeId &stateTypeId, const QVarian
 
             if (!values.contains(m_states.value(i).value())) {
                 if (values.contains(stateType.defaultValue())) {
-                    qCInfo(dcThing()).nospace() << this << ": Adjusting state value for " << stateType.name() << " from " << m_states.at(i).value() << " to default value of " << stateType.defaultValue();
+                    qCInfo(dcThing()).nospace()
+                        << this
+                        << ": Adjusting state value for "
+                        << stateType.name()
+                        << " from "
+                        << m_states.at(i).value()
+                        << " to default value of "
+                        << stateType.defaultValue();
                     m_states[i].setValue(stateType.defaultValue());
                 } else if (!values.isEmpty()) {
-                    qCInfo(dcThing()).nospace() << this << ": Adjusting state value for " << stateType.name() << " from " << m_states.at(i).value() << " to new value of " << values.first();
+                    qCInfo(dcThing()).nospace()
+                        << this
+                        << ": Adjusting state value for "
+                        << stateType.name()
+                        << " from "
+                        << m_states.at(i).value()
+                        << " to new value of "
+                        << values.first();
                     m_states[i].setValue(values.first());
                 }
             }
@@ -597,8 +731,9 @@ void Thing::setStatePossibleValues(const StateTypeId &stateTypeId, const QVarian
         }
     }
     qCWarning(dcThing()).nospace() << this << ": Failed setting maximum state value " << stateType.name() << " to " << values;
-    Q_ASSERT_X(false, m_name.toUtf8(), QString("Failed setting possible state values for %1 to %2").arg(stateType.name()).arg(QString(QJsonDocument::fromVariant(values).toJson())).toUtf8());
+    Q_ASSERT_X(false,
-
+               m_name.toUtf8(),
+               QString("Failed setting possible state values for %1 to %2").arg(stateType.name()).arg(QString(QJsonDocument::fromVariant(values).toJson())).toUtf8());
 }
 
 /*! Returns the \l{State} with the given \a stateTypeId of this thing. */
@@ -731,9 +866,9 @@ void Thing::setStateValueFilter(const StateTypeId &stateTypeId, Types::StateValu
     }
 }
 
-Things::Things(const QList<Thing*> &other)
+Things::Things(const QList<Thing *> &other)
 {
-    foreach (Thing* thing, other) {
+    foreach (Thing *thing, other) {
         this->append(thing);
     }
 }
@@ -783,7 +918,7 @@ QDebug operator<<(QDebug debug, Thing *thing)
 Things Things::filterByParam(const ParamTypeId &paramTypeId, const QVariant &value)
 {
     Things ret;
-    foreach (Thing* thing, *this) {
+    foreach (Thing *thing, *this) {
         if (!thing->thingClass().paramTypes().findById(paramTypeId).isValid()) {
             continue;
         }
@@ -798,7 +933,7 @@ Things Things::filterByParam(const ParamTypeId &paramTypeId, const QVariant &val
 Things Things::filterByThingClassId(const ThingClassId &thingClassId)
 {
     Things ret;
-    foreach (Thing* thing, *this) {
+    foreach (Thing *thing, *this) {
         if (thing->thingClassId() == thingClassId) {
             ret << thing;
         }
@@ -835,5 +970,5 @@ QVariant Things::get(int index) const
 
 void Things::put(const QVariant &variant)
 {
-    append(variant.value<Thing*>());
+    append(variant.value<Thing *>());
 }

libnymea/integrations/thingutils.cpp

@@ -29,6 +29,29 @@
 #include <QFileInfo>
 #include <QJsonParseError>
 #include <QMetaEnum>
+#include <qmath.h>
+
+namespace {
+
+bool isStepSizeType(QMetaType::Type type)
+{
+    switch (type) {
+    case QMetaType::Int:
+    case QMetaType::UInt:
+    case QMetaType::LongLong:
+    case QMetaType::ULongLong:
+    case QMetaType::Double:
+    case QMetaType::Float:
+    case QMetaType::Short:
+    case QMetaType::ULong:
+    case QMetaType::UShort:
+        return true;
+    default:
+        return false;
+    }
+}
+
+}
 
 ThingUtils::ThingUtils()
 {
@@ -134,6 +157,39 @@ Thing::ThingError ThingUtils::verifyParam(const ParamType &paramType, const Para
         }
     }
 
+    if (paramType.stepSize() != 0) {
+        QVariant paramValue = param.value();
+        paramValue.convert(static_cast<int>(paramType.type()));
+        QVariant clampedValue = ThingUtils::ensureValueClamping(paramValue, paramType.type(), paramType.minValue(), paramType.maxValue(), paramType.stepSize());
+        const double stepEpsilon = qMax(qAbs(paramType.stepSize()) * 1e-9, 1e-12);
+        bool stepAdjusted = false;
+        switch (paramType.type()) {
+        case QMetaType::Double:
+            stepAdjusted = qAbs(paramValue.toDouble() - clampedValue.toDouble()) > stepEpsilon;
+            break;
+        case QMetaType::Float:
+            stepAdjusted = qAbs(paramValue.toFloat() - clampedValue.toFloat()) > stepEpsilon;
+            break;
+        case QMetaType::Int:
+        case QMetaType::UInt:
+        case QMetaType::LongLong:
+        case QMetaType::ULongLong:
+        case QMetaType::Short:
+        case QMetaType::ULong:
+        case QMetaType::UShort:
+            stepAdjusted = (paramValue != clampedValue);
+            break;
+        default:
+            break;
+        }
+
+        if (stepAdjusted) {
+            qCWarning(dcThing()) << "Value not matching step size for param" << param.paramTypeId().toString()
+                                 << " Got:" << param.value() << " Step size:" << paramType.stepSize();
+            return Thing::ThingErrorInvalidParameter;
+        }
+    }
+
     if (!paramType.allowedValues().isEmpty() && !paramType.allowedValues().contains(param.value())) {
         QStringList allowedValues;
         foreach (const QVariant &value, paramType.allowedValues()) {
@@ -400,6 +456,73 @@ QStringList ThingUtils::generateInterfaceParentList(const QString &interface)
     return ret;
 }
 
+QVariant ThingUtils::ensureValueClamping(const QVariant value, QMetaType::Type type, const QVariant &minValue, const QVariant &maxValue, double stepSize)
+{
+    QVariant adjustedValue = value;
+    if (!adjustedValue.canConvert(static_cast<int>(type)) || !adjustedValue.convert(static_cast<int>(type))) {
+        return value;
+    }
+
+    if (stepSize == 0 || !isStepSizeType(type)) {
+        if (minValue.isValid() && ThingUtils::variantLessThan(adjustedValue, minValue)) {
+            return minValue;
+        }
+        if (maxValue.isValid() && ThingUtils::variantGreaterThan(adjustedValue, maxValue)) {
+            return maxValue;
+        }
+        return adjustedValue;
+    }
+
+    const double step = qAbs(stepSize);
+    const bool hasMinValue = minValue.isValid();
+    const bool hasMaxValue = maxValue.isValid();
+    const double baseValue = hasMinValue ? minValue.toDouble() : 0.0;
+    const double currentValue = adjustedValue.toDouble();
+    const qint64 roundedSteps = qRound64((currentValue - baseValue) / step);
+    double steppedValue = baseValue + roundedSteps * step;
+
+    if (hasMinValue) {
+        const double min = minValue.toDouble();
+        if (steppedValue < min) {
+            steppedValue = min;
+        }
+    }
+
+    if (hasMaxValue) {
+        const double max = maxValue.toDouble();
+        if (steppedValue > max) {
+            const qint64 maxSteps = static_cast<qint64>(qFloor((max - baseValue) / step));
+            steppedValue = baseValue + maxSteps * step;
+            if (hasMinValue && steppedValue < minValue.toDouble()) {
+                steppedValue = minValue.toDouble();
+            }
+        }
+    }
+
+    switch (type) {
+    case QMetaType::Int:
+        return QVariant(static_cast<int>(qRound64(steppedValue)));
+    case QMetaType::UInt:
+        return QVariant(static_cast<uint>(qRound64(steppedValue)));
+    case QMetaType::LongLong:
+        return QVariant(static_cast<qint64>(qRound64(steppedValue)));
+    case QMetaType::ULongLong:
+        return QVariant(static_cast<quint64>(qRound64(steppedValue)));
+    case QMetaType::Double:
+        return QVariant(steppedValue);
+    case QMetaType::Float:
+        return QVariant(static_cast<float>(steppedValue));
+    case QMetaType::Short:
+        return QVariant::fromValue(static_cast<short>(qRound64(steppedValue)));
+    case QMetaType::ULong:
+        return QVariant::fromValue(static_cast<ulong>(qRound64(steppedValue)));
+    case QMetaType::UShort:
+        return QVariant::fromValue(static_cast<ushort>(qRound64(steppedValue)));
+    default:
+        return adjustedValue;
+    }
+}
+
 bool ThingUtils::variantLessThan(const QVariant &leftHandSide, const QVariant &rightHandSide)
 {
     // Note: https://www.mail-archive.com/development@qt-project.org/msg39450.html

libnymea/integrations/thingutils.h

@@ -26,7 +26,6 @@
 #define THINGUTILS_H
 
 #include "thing.h"
-#include "pluginmetadata.h"
 
 #include "types/paramtype.h"
 #include "types/interface.h"
@@ -45,6 +44,8 @@ public:
     static Interface mergeInterfaces(const Interface &iface1, const Interface &iface2);
     static QStringList generateInterfaceParentList(const QString &interface);
 
+    static QVariant ensureValueClamping(const QVariant value, QMetaType::Type type, const QVariant &minValue, const QVariant &maxValue, double stepSize);
+
     static bool variantLessThan(const QVariant &leftHandSide, const QVariant &rightHandSide);
     static bool variantGreaterThan(const QVariant &leftHandSide, const QVariant &rightHandSide);
 };

nymea.pro

@@ -11,7 +11,7 @@ isEmpty(NYMEA_VERSION) {
 
 # define protocol versions
 JSON_PROTOCOL_VERSION_MAJOR=8
-JSON_PROTOCOL_VERSION_MINOR=5
+JSON_PROTOCOL_VERSION_MINOR=3
 JSON_PROTOCOL_VERSION="$${JSON_PROTOCOL_VERSION_MAJOR}.$${JSON_PROTOCOL_VERSION_MINOR}"
 LIBNYMEA_API_VERSION_MAJOR=9
 LIBNYMEA_API_VERSION_MINOR=0

tests/auto/api.json

@@ -1,4 +1,4 @@
-8.5
+8.3
 {
     "enums": {
         "BasicType": [