From 84de29fddcafc9e8cd23f930102d62493ab30def Mon Sep 17 00:00:00 2001 From: Sebastian Romero Date: Mon, 7 Apr 2025 17:23:17 +0200 Subject: [PATCH 1/5] Explicitly handle NAN values --- src/property/types/CloudWrapperFloat.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/property/types/CloudWrapperFloat.h b/src/property/types/CloudWrapperFloat.h index 7ce857c85..7c6bee8ef 100644 --- a/src/property/types/CloudWrapperFloat.h +++ b/src/property/types/CloudWrapperFloat.h @@ -39,7 +39,10 @@ class CloudWrapperFloat : public CloudWrapperBase { public: CloudWrapperFloat(float& v) : _primitive_value(v), _cloud_value(v), _local_value(v) {} virtual bool isDifferentFromCloud() { - return _primitive_value != _cloud_value && (abs(_primitive_value - _cloud_value) >= Property::_min_delta_property); + if (std::isnan(_primitive_value) || std::isnan(_cloud_value)) { + return std::isnan(_primitive_value) != std::isnan(_cloud_value); + } + return _primitive_value != _cloud_value && fabs(_primitive_value - _cloud_value) >= Property::_min_delta_property; } virtual void fromCloudToLocal() { _primitive_value = _cloud_value; From c383fd7641a190db3ea71d679a0cc3604f759d9a Mon Sep 17 00:00:00 2001 From: Sebastian Romero Date: Mon, 7 Apr 2025 17:35:34 +0200 Subject: [PATCH 2/5] Handle NaN values in CloudFloat comparison --- src/property/types/CloudFloat.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/property/types/CloudFloat.h b/src/property/types/CloudFloat.h index 514df7866..55c53aafc 100644 --- a/src/property/types/CloudFloat.h +++ b/src/property/types/CloudFloat.h @@ -46,7 +46,10 @@ class CloudFloat : public Property { return _value; } virtual bool isDifferentFromCloud() { - return _value != _cloud_value && (abs(_value - _cloud_value) >= Property::_min_delta_property); + if (std::isnan(_value) || std::isnan(_cloud_value)) { + return std::isnan(_value) != std::isnan(_cloud_value); + } + return _value != _cloud_value && fabs(_value - _cloud_value) >= Property::_min_delta_property; } virtual void fromCloudToLocal() { _value = _cloud_value; From 706ffdcd871a92ba831d1c95b8214f28770205c3 Mon Sep 17 00:00:00 2001 From: Sebastian Romero Date: Wed, 9 Apr 2025 15:42:25 +0200 Subject: [PATCH 3/5] Add test case for CloudFloat --- extras/test/CMakeLists.txt | 1 + extras/test/src/test_CloudFloat.cpp | 112 ++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+) create mode 100644 extras/test/src/test_CloudFloat.cpp diff --git a/extras/test/CMakeLists.txt b/extras/test/CMakeLists.txt index ae745aeaf..7ba90e644 100644 --- a/extras/test/CMakeLists.txt +++ b/extras/test/CMakeLists.txt @@ -66,6 +66,7 @@ set(TEST_SRCS src/test_addPropertyReal.cpp src/test_callback.cpp src/test_CloudColor.cpp + src/test_CloudFloat.cpp src/test_CloudLocation.cpp src/test_CloudSchedule.cpp src/test_decode.cpp diff --git a/extras/test/src/test_CloudFloat.cpp b/extras/test/src/test_CloudFloat.cpp new file mode 100644 index 000000000..7f6437c40 --- /dev/null +++ b/extras/test/src/test_CloudFloat.cpp @@ -0,0 +1,112 @@ +/* + Copyright (c) 2019 Arduino. All rights reserved. +*/ + +/************************************************************************************** + INCLUDE + **************************************************************************************/ + +#include + +#include + +#include + +/************************************************************************************** + TEST CODE + **************************************************************************************/ + +SCENARIO("Arduino Cloud Properties ", "[ArduinoCloudThing::CloudFloat]") +{ + WHEN("NAN value from cloud is received") + { + PropertyContainer property_container; + CloudFloat prop = 2.0f; + prop.writeOnDemand(); + addPropertyToContainer(property_container, prop, "test", Permission::ReadWrite); + + /* [{0: "test", 2: NaN}] = 81A200647465737402F97E00 */ + uint8_t const payload[] = { 0x81, 0xA2, 0x00, 0x64, 0x74, 0x65, 0x73, 0x74, 0x02, 0xF9, 0x7E, 0x00 }; + int const payload_length = sizeof(payload) / sizeof(uint8_t); + CBORDecoder::decode(property_container, payload, payload_length); + + REQUIRE(prop.isDifferentFromCloud() == true); + } + + WHEN("Local value is NAN") + { + PropertyContainer property_container; + CloudFloat prop = NAN; + prop.writeOnDemand(); + addPropertyToContainer(property_container, prop, "test", Permission::ReadWrite); + + /* [{0: "test", 2: 1.5}] = 81A200647465737402F93E00 */ + uint8_t const payload[] = { 0x81, 0xA2, 0x00, 0x64, 0x74, 0x65, 0x73, 0x74, 0x02, 0xF9, 0x3E, 0x00 }; + int const payload_length = sizeof(payload) / sizeof(uint8_t); + CBORDecoder::decode(property_container, payload, payload_length); + + REQUIRE(prop.isDifferentFromCloud() == true); + } + + WHEN("both, the local and the cloud values are NaN") + { + PropertyContainer property_container; + CloudFloat prop = NAN; + prop.writeOnDemand(); + addPropertyToContainer(property_container, prop, "test", Permission::ReadWrite); + + /* [{0: "test", 2: NaN}] = 81A200647465737402F97E00 */ + uint8_t const payload[] = { 0x81, 0xA2, 0x00, 0x64, 0x74, 0x65, 0x73, 0x74, 0x02, 0xF9, 0x7E, 0x00 }; + int const payload_length = sizeof(payload) / sizeof(uint8_t); + CBORDecoder::decode(property_container, payload, payload_length); + + REQUIRE(prop.isDifferentFromCloud() == false); + } + + WHEN("the local and cloud values are the same") + { + PropertyContainer property_container; + CloudFloat prop = 1.5; + prop.writeOnDemand(); + addPropertyToContainer(property_container, prop, "test", Permission::ReadWrite); + + /* [{0: "test", 2: 1.5}] = 81A200647465737402F93E00 */ + uint8_t const payload[] = { 0x81, 0xA2, 0x00, 0x64, 0x74, 0x65, 0x73, 0x74, 0x02, 0xF9, 0x3E, 0x00 }; + int const payload_length = sizeof(payload) / sizeof(uint8_t); + CBORDecoder::decode(property_container, payload, payload_length); + + REQUIRE(prop.isDifferentFromCloud() == false); + } + + WHEN("the local and the cloud values differ") + { + PropertyContainer property_container; + CloudFloat prop = 1.0f; + prop.writeOnDemand(); + addPropertyToContainer(property_container, prop, "test", Permission::ReadWrite); + + /* [{0: "test", 2: 1.5}] = 81A200647465737402F97E00 */ + uint8_t const payload[] = { 0x81, 0xA2, 0x00, 0x64, 0x74, 0x65, 0x73, 0x74, 0x02, 0xF9, 0x7E, 0x00 }; + int const payload_length = sizeof(payload) / sizeof(uint8_t); + CBORDecoder::decode(property_container, payload, payload_length); + + REQUIRE(prop.isDifferentFromCloud() == true); + } + + WHEN("the local value differs by 0.25 from the cloud value") + { + PropertyContainer property_container; + CloudFloat prop = 1.0f; + prop.writeOnDemand(); + addPropertyToContainer(property_container, prop, "test", Permission::ReadWrite); + + /* [{0: "test", 2: 1.5}] = 81A200647465737402F97E00 */ + uint8_t const payload[] = { 0x81, 0xA2, 0x00, 0x64, 0x74, 0x65, 0x73, 0x74, 0x02, 0xF9, 0x7E, 0x00 }; + int const payload_length = sizeof(payload) / sizeof(uint8_t); + CBORDecoder::decode(property_container, payload, payload_length); + prop.publishOnChange(0.25f, 0); // 0.25 min delta + + REQUIRE(prop.isDifferentFromCloud() == true); + } + +} From 9fa6a11d94dfef1e6b18cc1667d887faf1572e45 Mon Sep 17 00:00:00 2001 From: Andrea Gilardoni Date: Wed, 9 Apr 2025 16:34:08 +0200 Subject: [PATCH 4/5] Improved and generalized ieee754 is different function --- src/property/math_utils.h | 29 ++++++++++++++++++++++++++ src/property/types/CloudFloat.h | 6 ++---- src/property/types/CloudWrapperFloat.h | 6 ++---- 3 files changed, 33 insertions(+), 8 deletions(-) create mode 100644 src/property/math_utils.h diff --git a/src/property/math_utils.h b/src/property/math_utils.h new file mode 100644 index 000000000..5d5e465c0 --- /dev/null +++ b/src/property/math_utils.h @@ -0,0 +1,29 @@ +/* + This file is part of the ArduinoIoTCloud library. + + Copyright (c) 2024 Arduino SA + + This Source Code Form is subject to the terms of the Mozilla Public + License, v. 2.0. If a copy of the MPL was not distributed with this + file, You can obtain one at http://mozilla.org/MPL/2.0/. +*/ +#pragma once +#include + +namespace arduino { namespace math { + + template + inline bool ieee754_different(const T a, const T b, const T delta = 0.0) { + /* The following comparison is introduced to take into account the very peculiar + * way of handling NaN values by the standard IEEE754. + * We consider two floating point number different if their binary representation is different. + * or if those two IEEE754 values are numbers or zero(which is handled on its own) exceed + * a certain threshold + */ + return memcmp((uint8_t*)&a, (uint8_t*)&b, sizeof(b)) != 0 && ( + (std::fpclassify(a) != FP_NORMAL && std::fpclassify(a) != FP_ZERO) || + (std::fpclassify(b) != FP_NORMAL && std::fpclassify(b) != FP_ZERO) || + fabs(a - b) >= delta + ); + } +}} diff --git a/src/property/types/CloudFloat.h b/src/property/types/CloudFloat.h index 55c53aafc..1918f9669 100644 --- a/src/property/types/CloudFloat.h +++ b/src/property/types/CloudFloat.h @@ -19,6 +19,7 @@ #define CLOUDFLOAT_H_ #include +#include "../math_utils.h" /****************************************************************************** INCLUDE @@ -46,10 +47,7 @@ class CloudFloat : public Property { return _value; } virtual bool isDifferentFromCloud() { - if (std::isnan(_value) || std::isnan(_cloud_value)) { - return std::isnan(_value) != std::isnan(_cloud_value); - } - return _value != _cloud_value && fabs(_value - _cloud_value) >= Property::_min_delta_property; + return arduino::math::ieee754_different(_value, _cloud_value, Property::_min_delta_property); } virtual void fromCloudToLocal() { _value = _cloud_value; diff --git a/src/property/types/CloudWrapperFloat.h b/src/property/types/CloudWrapperFloat.h index 7c6bee8ef..4e8d97986 100644 --- a/src/property/types/CloudWrapperFloat.h +++ b/src/property/types/CloudWrapperFloat.h @@ -19,6 +19,7 @@ #define CLOUDWRAPPERFLOAT_H_ #include +#include "../math_utils.h" /****************************************************************************** INCLUDE @@ -39,10 +40,7 @@ class CloudWrapperFloat : public CloudWrapperBase { public: CloudWrapperFloat(float& v) : _primitive_value(v), _cloud_value(v), _local_value(v) {} virtual bool isDifferentFromCloud() { - if (std::isnan(_primitive_value) || std::isnan(_cloud_value)) { - return std::isnan(_primitive_value) != std::isnan(_cloud_value); - } - return _primitive_value != _cloud_value && fabs(_primitive_value - _cloud_value) >= Property::_min_delta_property; + return arduino::math::ieee754_different(_primitive_value, _cloud_value, Property::_min_delta_property); } virtual void fromCloudToLocal() { _primitive_value = _cloud_value; From 9c2aaef2b56c91dd11d4f3762143383ccb319820 Mon Sep 17 00:00:00 2001 From: Sebastian Romero Date: Fri, 11 Apr 2025 14:37:08 +0200 Subject: [PATCH 5/5] Add test for CloudWrapperFloat --- extras/test/CMakeLists.txt | 1 + extras/test/src/test_CloudWrapperFloat.cpp | 118 +++++++++++++++++++++ 2 files changed, 119 insertions(+) create mode 100644 extras/test/src/test_CloudWrapperFloat.cpp diff --git a/extras/test/CMakeLists.txt b/extras/test/CMakeLists.txt index 7ba90e644..5435dfcf1 100644 --- a/extras/test/CMakeLists.txt +++ b/extras/test/CMakeLists.txt @@ -67,6 +67,7 @@ set(TEST_SRCS src/test_callback.cpp src/test_CloudColor.cpp src/test_CloudFloat.cpp + src/test_CloudWrapperFloat.cpp src/test_CloudLocation.cpp src/test_CloudSchedule.cpp src/test_decode.cpp diff --git a/extras/test/src/test_CloudWrapperFloat.cpp b/extras/test/src/test_CloudWrapperFloat.cpp new file mode 100644 index 000000000..aa4f6a295 --- /dev/null +++ b/extras/test/src/test_CloudWrapperFloat.cpp @@ -0,0 +1,118 @@ +/* + Copyright (c) 2019 Arduino. All rights reserved. +*/ + +/************************************************************************************** + INCLUDE + **************************************************************************************/ + +#include + +#include + +#include + +/************************************************************************************** + TEST CODE + **************************************************************************************/ + +SCENARIO("Arduino Cloud Properties ", "[ArduinoCloudThing::CloudWrapperFloat]") +{ + WHEN("NAN value from cloud is received") + { + PropertyContainer property_container; + float value = 2.0f; + CloudWrapperFloat prop(value); + prop.writeOnDemand(); + addPropertyToContainer(property_container, prop, "test", Permission::ReadWrite); + + /* [{0: "test", 2: NaN}] = 81A200647465737402F97E00 */ + uint8_t const payload[] = { 0x81, 0xA2, 0x00, 0x64, 0x74, 0x65, 0x73, 0x74, 0x02, 0xF9, 0x7E, 0x00 }; + int const payload_length = sizeof(payload) / sizeof(uint8_t); + CBORDecoder::decode(property_container, payload, payload_length); + + REQUIRE(prop.isDifferentFromCloud() == true); + } + + WHEN("Local value is NAN") + { + PropertyContainer property_container; + float value = NAN; + CloudWrapperFloat prop(value); + prop.writeOnDemand(); + addPropertyToContainer(property_container, prop, "test", Permission::ReadWrite); + + /* [{0: "test", 2: 1.5}] = 81A200647465737402F93E00 */ + uint8_t const payload[] = { 0x81, 0xA2, 0x00, 0x64, 0x74, 0x65, 0x73, 0x74, 0x02, 0xF9, 0x3E, 0x00 }; + int const payload_length = sizeof(payload) / sizeof(uint8_t); + CBORDecoder::decode(property_container, payload, payload_length); + + REQUIRE(prop.isDifferentFromCloud() == true); + } + + WHEN("both, the local and the cloud values are NaN") + { + PropertyContainer property_container; + float value = NAN; + CloudWrapperFloat prop(value); + prop.writeOnDemand(); + addPropertyToContainer(property_container, prop, "test", Permission::ReadWrite); + + /* [{0: "test", 2: NaN}] = 81A200647465737402F97E00 */ + uint8_t const payload[] = { 0x81, 0xA2, 0x00, 0x64, 0x74, 0x65, 0x73, 0x74, 0x02, 0xF9, 0x7E, 0x00 }; + int const payload_length = sizeof(payload) / sizeof(uint8_t); + CBORDecoder::decode(property_container, payload, payload_length); + + REQUIRE(prop.isDifferentFromCloud() == false); + } + + WHEN("the local and cloud values are the same") + { + PropertyContainer property_container; + float value = 1.5f; + CloudWrapperFloat prop(value); + prop.writeOnDemand(); + addPropertyToContainer(property_container, prop, "test", Permission::ReadWrite); + + /* [{0: "test", 2: 1.5}] = 81A200647465737402F93E00 */ + uint8_t const payload[] = { 0x81, 0xA2, 0x00, 0x64, 0x74, 0x65, 0x73, 0x74, 0x02, 0xF9, 0x3E, 0x00 }; + int const payload_length = sizeof(payload) / sizeof(uint8_t); + CBORDecoder::decode(property_container, payload, payload_length); + + REQUIRE(prop.isDifferentFromCloud() == false); + } + + WHEN("the local and the cloud values differ") + { + PropertyContainer property_container; + float value = 1.0f; + CloudWrapperFloat prop(value); + prop.writeOnDemand(); + addPropertyToContainer(property_container, prop, "test", Permission::ReadWrite); + + /* [{0: "test", 2: 1.5}] = 81A200647465737402F97E00 */ + uint8_t const payload[] = { 0x81, 0xA2, 0x00, 0x64, 0x74, 0x65, 0x73, 0x74, 0x02, 0xF9, 0x7E, 0x00 }; + int const payload_length = sizeof(payload) / sizeof(uint8_t); + CBORDecoder::decode(property_container, payload, payload_length); + + REQUIRE(prop.isDifferentFromCloud() == true); + } + + WHEN("the local value differs by 0.25 from the cloud value") + { + PropertyContainer property_container; + float value = 1.0f; + CloudWrapperFloat prop(value); + prop.writeOnDemand(); + addPropertyToContainer(property_container, prop, "test", Permission::ReadWrite); + + /* [{0: "test", 2: 1.5}] = 81A200647465737402F97E00 */ + uint8_t const payload[] = { 0x81, 0xA2, 0x00, 0x64, 0x74, 0x65, 0x73, 0x74, 0x02, 0xF9, 0x7E, 0x00 }; + int const payload_length = sizeof(payload) / sizeof(uint8_t); + CBORDecoder::decode(property_container, payload, payload_length); + prop.publishOnChange(0.25f, 0); // 0.25 min delta + + REQUIRE(prop.isDifferentFromCloud() == true); + } + +}