From 6cf060aa936f10f2171ca8d7e623b96499110060 Mon Sep 17 00:00:00 2001 From: Michael Zanetti Date: Wed, 18 Jul 2018 01:41:46 +0200 Subject: [PATCH] fix an issue where state values in rules could be casted wrong from string to double --- libnymea-core/stateevaluator.cpp | 14 +++++--------- libnymea/types/statedescriptor.cpp | 19 +++++++++++-------- plugins/mock/devicepluginmock.json | 12 +++++++++++- plugins/mock/httpdaemon.cpp | 6 +++++- tests/auto/devices/testdevices.cpp | 6 +++--- tests/auto/nymeatestbase.cpp | 1 + tests/auto/nymeatestbase.h | 1 + tests/auto/rules/testrules.cpp | 4 ++++ 8 files changed, 41 insertions(+), 22 deletions(-) diff --git a/libnymea-core/stateevaluator.cpp b/libnymea-core/stateevaluator.cpp index aa9a3321..ea61c597 100644 --- a/libnymea-core/stateevaluator.cpp +++ b/libnymea-core/stateevaluator.cpp @@ -285,27 +285,23 @@ bool StateEvaluator::isValid() const foreach (const StateType &stateType, deviceClass.stateTypes()) { if (stateType.id() == m_stateDescriptor.stateTypeId()) { - if (!m_stateDescriptor.stateValue().canConvert(stateType.type())) { - qCWarning(dcRuleEngine) << "Wrong state value for state descriptor" << m_stateDescriptor.stateTypeId() << " Got:" << m_stateDescriptor.stateValue() << " Expected:" << QVariant::typeToName(stateType.type()); - return false; - } - - if (!m_stateDescriptor.stateValue().convert(stateType.type())) { + QVariant stateValue = m_stateDescriptor.stateValue(); + if (!stateValue.convert(stateType.type())) { qCWarning(dcRuleEngine) << "Could not convert value of state descriptor" << m_stateDescriptor.stateTypeId() << " to:" << QVariant::typeToName(stateType.type()) << " Got:" << m_stateDescriptor.stateValue(); return false; } - if (stateType.maxValue().isValid() && m_stateDescriptor.stateValue() > stateType.maxValue()) { + if (stateType.maxValue().isValid() && stateValue > stateType.maxValue()) { qCWarning(dcRuleEngine) << "Value out of range for state descriptor" << m_stateDescriptor.stateTypeId() << " Got:" << m_stateDescriptor.stateValue() << " Max:" << stateType.maxValue(); return false; } - if (stateType.minValue().isValid() && m_stateDescriptor.stateValue() < stateType.minValue()) { + if (stateType.minValue().isValid() && stateValue < stateType.minValue()) { qCWarning(dcRuleEngine) << "Value out of range for state descriptor" << m_stateDescriptor.stateTypeId() << " Got:" << m_stateDescriptor.stateValue() << " Min:" << stateType.minValue(); return false; } - if (!stateType.possibleValues().isEmpty() && !stateType.possibleValues().contains(m_stateDescriptor.stateValue())) { + if (!stateType.possibleValues().isEmpty() && !stateType.possibleValues().contains(stateValue)) { QStringList possibleValues; foreach (const QVariant &value, stateType.possibleValues()) { possibleValues.append(value.toString()); diff --git a/libnymea/types/statedescriptor.cpp b/libnymea/types/statedescriptor.cpp index 2c7cdb30..29b70aac 100644 --- a/libnymea/types/statedescriptor.cpp +++ b/libnymea/types/statedescriptor.cpp @@ -134,21 +134,24 @@ bool StateDescriptor::operator ==(const State &state) const if ((m_stateTypeId != state.stateTypeId()) || (m_deviceId != state.deviceId())) { return false; } - QVariant convertedValue = state.value(); - convertedValue.convert(m_stateValue.type()); + if (!m_stateValue.canConvert(state.value().type())) { + return false; + } + QVariant convertedValue = m_stateValue; + convertedValue.convert(state.value().type()); switch (m_operatorType) { case Types::ValueOperatorEquals: - return m_stateValue == convertedValue; + return convertedValue == state.value(); case Types::ValueOperatorGreater: - return convertedValue > m_stateValue; + return state.value() > convertedValue; case Types::ValueOperatorGreaterOrEqual: - return convertedValue >= m_stateValue; + return state.value() >= convertedValue; case Types::ValueOperatorLess: - return convertedValue < m_stateValue; + return state.value() < convertedValue; case Types::ValueOperatorLessOrEqual: - return convertedValue <= m_stateValue; + return state.value() <= convertedValue; case Types::ValueOperatorNotEquals: - return m_stateValue != convertedValue; + return convertedValue != state.value(); } return false; } diff --git a/plugins/mock/devicepluginmock.json b/plugins/mock/devicepluginmock.json index cbad53ff..348e8cd5 100644 --- a/plugins/mock/devicepluginmock.json +++ b/plugins/mock/devicepluginmock.json @@ -85,13 +85,23 @@ }, { "id": "9dd6a97c-dfd1-43dc-acbd-367932742310", - "name": "boolValue", + "name": "bool", "displayName": "Dummy bool state", "displayNameEvent": "Dummy bool state changed", "defaultValue": false, "type": "bool", "cached": false }, + { + "id": "7cac53ee-7048-4dc9-b000-7b585390f34c", + "name": "double", + "displayName": "Dummy double state", + "displayNameEvent": "Dummy double state changed", + "type": "int", + "minValue": 0, + "maxValue": 100, + "defaultValue": 2.7 + }, { "id": "6c8ab9a6-0164-4795-b829-f4394fe4edc4", "name": "batteryLevel", diff --git a/plugins/mock/httpdaemon.cpp b/plugins/mock/httpdaemon.cpp index 6bada84c..d3c1e8ef 100644 --- a/plugins/mock/httpdaemon.cpp +++ b/plugins/mock/httpdaemon.cpp @@ -85,8 +85,12 @@ void HttpDaemon::readClient() if (url.path() == "/setstate") { StateTypeId stateTypeId = StateTypeId(query.queryItems().first().first); QVariant stateValue = query.queryItems().first().second; - if (stateTypeId == mockBoolValueStateTypeId || stateTypeId == mockBatteryCriticalStateTypeId) { + if (stateTypeId == mockBoolStateTypeId || stateTypeId == mockBatteryCriticalStateTypeId) { stateValue.convert(QVariant::Bool); + } else if (stateTypeId == mockIntStateTypeId) { + stateValue.convert(QVariant::Int); + } else if (stateTypeId == mockDoubleStateTypeId) { + stateValue.convert(QVariant::Double); } qCDebug(dcMockDevice) << "Set state value" << stateValue; emit setState(stateTypeId, stateValue); diff --git a/tests/auto/devices/testdevices.cpp b/tests/auto/devices/testdevices.cpp index ed1f355b..b681913b 100644 --- a/tests/auto/devices/testdevices.cpp +++ b/tests/auto/devices/testdevices.cpp @@ -671,7 +671,7 @@ void TestDevices::getEventTypes_data() QTest::addColumn("deviceClassId"); QTest::addColumn("resultCount"); - QTest::newRow("valid deviceclass") << mockDeviceClassId << 7; + QTest::newRow("valid deviceclass") << mockDeviceClassId << 8; QTest::newRow("invalid deviceclass") << DeviceClassId("094f8024-5caa-48c1-ab6a-de486a92088f") << 0; } @@ -696,7 +696,7 @@ void TestDevices::getStateTypes_data() QTest::addColumn("deviceClassId"); QTest::addColumn("resultCount"); - QTest::newRow("valid deviceclass") << mockDeviceClassId << 5; + QTest::newRow("valid deviceclass") << mockDeviceClassId << 6; QTest::newRow("invalid deviceclass") << DeviceClassId("094f8024-5caa-48c1-ab6a-de486a92088f") << 0; } @@ -797,7 +797,7 @@ void TestDevices::getStateValues() QCOMPARE(response.toMap().value("params").toMap().value("deviceError").toString(), JsonTypes::deviceErrorToString(statusCode)); if (statusCode == DeviceManager::DeviceErrorNoError) { QVariantList values = response.toMap().value("params").toMap().value("values").toList(); - QCOMPARE(values.count(), 5); // Mock device has two states... + QCOMPARE(values.count(), 6); // Mock device has 6 states... } } diff --git a/tests/auto/nymeatestbase.cpp b/tests/auto/nymeatestbase.cpp index 21b869e3..7eb653a3 100644 --- a/tests/auto/nymeatestbase.cpp +++ b/tests/auto/nymeatestbase.cpp @@ -56,6 +56,7 @@ EventTypeId mockEvent1Id = EventTypeId("45bf3752-0fc6-46b9-89fd-ffd878b5b22b"); EventTypeId mockEvent2Id = EventTypeId("863d5920-b1cf-4eb9-88bd-8f7b8583b1cf"); StateTypeId mockIntStateId = StateTypeId("80baec19-54de-4948-ac46-31eabfaceb83"); StateTypeId mockBoolStateId = StateTypeId("9dd6a97c-dfd1-43dc-acbd-367932742310"); +StateTypeId mockDoubleStateId = StateTypeId("7cac53ee-7048-4dc9-b000-7b585390f34c"); StateTypeId mockBatteryCriticalStateId = StateTypeId("580bc611-1a55-41f3-996f-8d3ccf543db3"); ActionTypeId mockActionIdPower = ActionTypeId("064aed0d-da4c-49d4-b236-60f97e98ff84"); ActionTypeId mockActionIdWithParams = ActionTypeId("dea0f4e1-65e3-4981-8eaa-2701c53a9185"); diff --git a/tests/auto/nymeatestbase.h b/tests/auto/nymeatestbase.h index 4f9336b0..8e2f9d34 100644 --- a/tests/auto/nymeatestbase.h +++ b/tests/auto/nymeatestbase.h @@ -56,6 +56,7 @@ extern ActionTypeId mockActionIdAsyncFailing; extern EventTypeId mockEvent1Id; extern EventTypeId mockEvent2Id; extern StateTypeId mockIntStateId; +extern StateTypeId mockDoubleStateId; extern StateTypeId mockBatteryCriticalStateId; extern StateTypeId mockBoolStateId; diff --git a/tests/auto/rules/testrules.cpp b/tests/auto/rules/testrules.cpp index 00d5536a..f9a9ef45 100644 --- a/tests/auto/rules/testrules.cpp +++ b/tests/auto/rules/testrules.cpp @@ -1712,6 +1712,10 @@ void TestRules::testStateEvaluator_data() QTest::newRow("LessOrEqual, not matching") << m_mockDeviceId << mockIntStateId << QVariant(2) << Types::ValueOperatorLessOrEqual << false; QTest::newRow("LessOrEqual, matching (less)") << m_mockDeviceId << mockIntStateId << QVariant(777) << Types::ValueOperatorLessOrEqual << true; QTest::newRow("LessOrEqual, matching (equals)") << m_mockDeviceId << mockIntStateId << QVariant(10) << Types::ValueOperatorLessOrEqual << true; + QTest::newRow("Less, not matching, double") << m_mockDeviceId << mockDoubleStateId << QVariant(2.1) << Types::ValueOperatorLess << false; + QTest::newRow("Less, not matching, double as string") << m_mockDeviceId << mockDoubleStateId << QVariant("2.1") << Types::ValueOperatorLess << false; + QTest::newRow("Less, matching, double") << m_mockDeviceId << mockDoubleStateId << QVariant(4.2) << Types::ValueOperatorLess << true; + QTest::newRow("Less, matching, double as string") << m_mockDeviceId << mockDoubleStateId << QVariant("4.2") << Types::ValueOperatorLess << true; } void TestRules::testStateEvaluator()