From 2c7ba81b4bfa32965bdf65c16a3109bd6dc6012e Mon Sep 17 00:00:00 2001 From: pennam Date: Tue, 20 Feb 2024 15:46:30 +0100 Subject: [PATCH 1/6] Add Property write policy to manually handle local values update --- src/property/Property.cpp | 13 +++++++++++++ src/property/Property.h | 12 +++++++++++- src/property/PropertyContainer.cpp | 4 +++- 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/property/Property.cpp b/src/property/Property.cpp index b26494ed9..6297c09cb 100644 --- a/src/property/Property.cpp +++ b/src/property/Property.cpp @@ -29,6 +29,7 @@ Property::Property() , _min_delta_property{0.0f} , _min_time_between_updates_millis{DEFAULT_MIN_TIME_BETWEEN_UPDATES_MILLIS} , _permission{Permission::Read} +, _write_policy{WritePolicy::Auto} , _get_time_func{nullptr} , _update_callback_func{nullptr} , _on_sync_callback_func{nullptr} @@ -102,6 +103,18 @@ Property & Property::encodeTimestamp() return (*this); } +Property & Property::writeOnChange() +{ + _write_policy = WritePolicy::Auto; + return (*this); +} + +Property & Property::writeOnDemand() +{ + _write_policy = WritePolicy::Manual; + return (*this); +} + void Property::setTimestamp(unsigned long const timestamp) { _timestamp = timestamp; diff --git a/src/property/Property.h b/src/property/Property.h index 7393da192..71a8447c9 100644 --- a/src/property/Property.h +++ b/src/property/Property.h @@ -125,6 +125,10 @@ enum class UpdatePolicy { OnChange, TimeInterval, OnDemand }; +enum class WritePolicy { + Auto, Manual +}; + typedef void(*UpdateCallbackFunc)(void); typedef unsigned long(*GetTimeCallbackFunc)(); class Property; @@ -147,6 +151,8 @@ class Property Property & publishEvery(unsigned long const seconds); Property & publishOnDemand(); Property & encodeTimestamp(); + Property & writeOnChange(); + Property & writeOnDemand(); inline String name() const { return _name; @@ -160,6 +166,9 @@ class Property inline bool isWriteableByCloud() const { return (_permission == Permission::Write) || (_permission == Permission::ReadWrite); } + inline bool isWritableOnChange() const { + return _write_policy == WritePolicy::Auto; + } void setTimestamp(unsigned long const timestamp); bool shouldBeUpdated(); @@ -209,6 +218,7 @@ class Property private: Permission _permission; + WritePolicy _write_policy; GetTimeCallbackFunc _get_time_func; UpdateCallbackFunc _update_callback_func; OnSyncCallbackFunc _on_sync_callback_func; @@ -219,7 +229,7 @@ class Property _has_been_appended_but_not_sended; /* Variables used for UpdatePolicy::TimeInterval */ unsigned long _last_updated_millis, - _update_interval_millis; + _update_interval_millis; /* Variables used for reconnection sync*/ unsigned long _last_local_change_timestamp; unsigned long _last_cloud_change_timestamp; diff --git a/src/property/PropertyContainer.cpp b/src/property/PropertyContainer.cpp index 833250f02..77ae52a36 100644 --- a/src/property/PropertyContainer.cpp +++ b/src/property/PropertyContainer.cpp @@ -121,7 +121,9 @@ void updateProperty(PropertyContainer & prop_cont, String propertyName, unsigned if (is_sync_message) { property->execCallbackOnSync(); } else { - property->fromCloudToLocal(); + if (property->isWritableOnChange()) { + property->fromCloudToLocal(); + } property->execCallbackOnChange(); property->provideEcho(); } From af472ca354fdcbdaf1a194595ded22b5858a1b31 Mon Sep 17 00:00:00 2001 From: pennam Date: Tue, 20 Feb 2024 15:48:07 +0100 Subject: [PATCH 2/6] Remove public functions to handle thing_id discovery protocol --- src/ArduinoIoTCloud.cpp | 1 + src/ArduinoIoTCloud.h | 7 +------ src/ArduinoIoTCloudTCP.cpp | 21 ++++++++------------- 3 files changed, 10 insertions(+), 19 deletions(-) diff --git a/src/ArduinoIoTCloud.cpp b/src/ArduinoIoTCloud.cpp index 580521047..7d4676e57 100644 --- a/src/ArduinoIoTCloud.cpp +++ b/src/ArduinoIoTCloud.cpp @@ -32,6 +32,7 @@ ArduinoIoTCloudClass::ArduinoIoTCloudClass() , _tz_offset{0} , _tz_dst_until{0} , _thing_id{""} +, _thing_id_property{nullptr} , _lib_version{AIOT_CONFIG_LIB_VERSION} , _device_id{""} , _cloud_event_callback{nullptr} diff --git a/src/ArduinoIoTCloud.h b/src/ArduinoIoTCloud.h index 6154f25e9..bf1c6dc8c 100644 --- a/src/ArduinoIoTCloud.h +++ b/src/ArduinoIoTCloud.h @@ -96,12 +96,6 @@ class ArduinoIoTCloudClass inline void setDeviceId(String const device_id) { _device_id = device_id; }; inline String & getDeviceId() { return _device_id; }; - inline void setThingIdOutdatedFlag() { _thing_id_outdated = true ; } - inline void clrThingIdOutdatedFlag() { _thing_id_outdated = false ; } - inline bool getThingIdOutdatedFlag() { return _thing_id_outdated; } - - inline bool deviceNotAttached() { return _thing_id == ""; } - inline ConnectionHandler * getConnection() { return _connection; } inline unsigned long getInternalTime() { return _time_service.getTime(); } @@ -160,6 +154,7 @@ class ArduinoIoTCloudClass int _tz_offset; unsigned int _tz_dst_until; String _thing_id; + Property * _thing_id_property; String _lib_version; void execCloudEventCallback(ArduinoIoTCloudEvent const event); diff --git a/src/ArduinoIoTCloudTCP.cpp b/src/ArduinoIoTCloudTCP.cpp index a78915031..5200e1043 100644 --- a/src/ArduinoIoTCloudTCP.cpp +++ b/src/ArduinoIoTCloudTCP.cpp @@ -58,11 +58,6 @@ void updateTimezoneInfo() ArduinoCloud.updateInternalTimezoneInfo(); } -void setThingIdOutdated() -{ - ArduinoCloud.setThingIdOutdatedFlag(); -} - /****************************************************************************** CTOR/DTOR ******************************************************************************/ @@ -218,7 +213,7 @@ int ArduinoIoTCloudTCP::begin(bool const enable_watchdog, String brokerAddress, addPropertyToContainer(_device_property_container, *p, "OTA_REQ", Permission::ReadWrite, -1); #endif /* OTA_ENABLED */ p = new CloudWrapperString(_thing_id); - addPropertyToContainer(_device_property_container, *p, "thing_id", Permission::ReadWrite, -1).onUpdate(setThingIdOutdated); + _thing_id_property = &addPropertyToContainer(_device_property_container, *p, "thing_id", Permission::ReadWrite, -1).writeOnDemand(); addPropertyReal(_tz_offset, "tz_offset", Permission::ReadWrite).onSync(CLOUD_WINS).onUpdate(updateTimezoneInfo); addPropertyReal(_tz_dst_until, "tz_dst_until", Permission::ReadWrite).onSync(CLOUD_WINS).onUpdate(updateTimezoneInfo); @@ -409,7 +404,7 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_WaitDeviceConfig() return State::Disconnect; } - if (getThingIdOutdatedFlag()) + if (_thing_id_property->isDifferentFromCloud()) { return State::CheckDeviceConfig; } @@ -445,7 +440,7 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_CheckDeviceConfig() updateThingTopics(); - if (deviceNotAttached()) + if (_thing_id.length() == 0) { /* Configuration received but device not attached. Wait: 40s */ unsigned long attach_retry_delay = (1 << _last_device_attach_cnt) * AIOT_CONFIG_DEVICE_TOPIC_SUBSCRIBE_RETRY_DELAY_ms; @@ -468,7 +463,7 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeThingTopics() return State::Disconnect; } - if (getThingIdOutdatedFlag()) + if (_thing_id_property->isDifferentFromCloud()) { return State::CheckDeviceConfig; } @@ -524,7 +519,7 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_RequestLastValues() return State::Disconnect; } - if (getThingIdOutdatedFlag()) + if (_thing_id_property->isDifferentFromCloud()) { return State::CheckDeviceConfig; } @@ -567,7 +562,7 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_Connected() /* We are connected so let's to our stuff here. */ else { - if (getThingIdOutdatedFlag()) + if (_thing_id_property->isDifferentFromCloud()) { return State::CheckDeviceConfig; } @@ -762,12 +757,12 @@ int ArduinoIoTCloudTCP::write(String const topic, byte const data[], int const l void ArduinoIoTCloudTCP::updateThingTopics() { + _thing_id_property->fromCloudToLocal(); + _shadowTopicOut = getTopic_shadowout(); _shadowTopicIn = getTopic_shadowin(); _dataTopicOut = getTopic_dataout(); _dataTopicIn = getTopic_datain(); - - clrThingIdOutdatedFlag(); } /****************************************************************************** From 896249be314b3fa80e93f92ec0580b3b30dad4a5 Mon Sep 17 00:00:00 2001 From: pennam Date: Tue, 20 Feb 2024 16:14:40 +0100 Subject: [PATCH 3/6] Remove public functions to handle timezone changes --- src/ArduinoIoTCloud.cpp | 2 -- src/ArduinoIoTCloud.h | 3 --- src/ArduinoIoTCloudTCP.cpp | 28 +++++++++++++++++++--------- src/ArduinoIoTCloudTCP.h | 5 +++++ 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/ArduinoIoTCloud.cpp b/src/ArduinoIoTCloud.cpp index 7d4676e57..ce191eafd 100644 --- a/src/ArduinoIoTCloud.cpp +++ b/src/ArduinoIoTCloud.cpp @@ -29,8 +29,6 @@ ArduinoIoTCloudClass::ArduinoIoTCloudClass() : _connection{nullptr} , _last_checked_property_index{0} , _time_service(TimeService) -, _tz_offset{0} -, _tz_dst_until{0} , _thing_id{""} , _thing_id_property{nullptr} , _lib_version{AIOT_CONFIG_LIB_VERSION} diff --git a/src/ArduinoIoTCloud.h b/src/ArduinoIoTCloud.h index bf1c6dc8c..9fa9d2963 100644 --- a/src/ArduinoIoTCloud.h +++ b/src/ArduinoIoTCloud.h @@ -100,7 +100,6 @@ class ArduinoIoTCloudClass inline unsigned long getInternalTime() { return _time_service.getTime(); } inline unsigned long getLocalTime() { return _time_service.getLocalTime(); } - inline void updateInternalTimezoneInfo() { _time_service.setTimeZoneData(_tz_offset, _tz_dst_until); } void addCallback(ArduinoIoTCloudEvent const event, OnCloudEventCallback callback); @@ -151,8 +150,6 @@ class ArduinoIoTCloudClass PropertyContainer _thing_property_container; unsigned int _last_checked_property_index; TimeServiceClass & _time_service; - int _tz_offset; - unsigned int _tz_dst_until; String _thing_id; Property * _thing_id_property; String _lib_version; diff --git a/src/ArduinoIoTCloudTCP.cpp b/src/ArduinoIoTCloudTCP.cpp index 5200e1043..87b1c555e 100644 --- a/src/ArduinoIoTCloudTCP.cpp +++ b/src/ArduinoIoTCloudTCP.cpp @@ -53,17 +53,16 @@ unsigned long getTime() return ArduinoCloud.getInternalTime(); } -void updateTimezoneInfo() -{ - ArduinoCloud.updateInternalTimezoneInfo(); -} - /****************************************************************************** CTOR/DTOR ******************************************************************************/ ArduinoIoTCloudTCP::ArduinoIoTCloudTCP() : _state{State::ConnectPhy} +, _tz_offset{0} +, _tz_offset_property{nullptr} +, _tz_dst_until{0} +, _tz_dst_until_property{nullptr} , _next_connection_attempt_tick{0} , _last_connection_attempt_cnt{0} , _next_device_subscribe_attempt_tick{0} @@ -214,9 +213,10 @@ int ArduinoIoTCloudTCP::begin(bool const enable_watchdog, String brokerAddress, #endif /* OTA_ENABLED */ p = new CloudWrapperString(_thing_id); _thing_id_property = &addPropertyToContainer(_device_property_container, *p, "thing_id", Permission::ReadWrite, -1).writeOnDemand(); - - addPropertyReal(_tz_offset, "tz_offset", Permission::ReadWrite).onSync(CLOUD_WINS).onUpdate(updateTimezoneInfo); - addPropertyReal(_tz_dst_until, "tz_dst_until", Permission::ReadWrite).onSync(CLOUD_WINS).onUpdate(updateTimezoneInfo); + p = new CloudWrapperInt(_tz_offset); + _tz_offset_property = &addPropertyToContainer(_thing_property_container, *p, "tz_offset", Permission::ReadWrite, -1).writeOnDemand(); + p = new CloudWrapperUnsignedInt(_tz_dst_until); + _tz_dst_until_property = &addPropertyToContainer(_thing_property_container, *p, "tz_dst_until", Permission::ReadWrite, -1).writeOnDemand(); #if OTA_ENABLED _ota_cap = OTA::isCapable(); @@ -584,13 +584,23 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_Connected() _mqtt_data_request_retransmit = false; } + /* Configure Time service with timezone data: + * _tz_offset [offset + dst] + * _tz_dst_until [posix timestamp until _tz_offset is valid] + */ + if (_tz_offset_property->isDifferentFromCloud() || _tz_dst_until_property->isDifferentFromCloud()) { + _tz_offset_property->fromCloudToLocal(); + _tz_dst_until_property->fromCloudToLocal(); + _time_service.setTimeZoneData(_tz_offset, _tz_dst_until); + } + /* Check if any properties need encoding and send them to * the cloud if necessary. */ sendThingPropertiesToCloud(); unsigned long const internal_posix_time = _time_service.getTime(); - if(internal_posix_time < _tz_dst_until) { + if (internal_posix_time < _tz_dst_until) { return State::Connected; } else { return State::RequestLastValues; diff --git a/src/ArduinoIoTCloudTCP.h b/src/ArduinoIoTCloudTCP.h index 60b6649cc..d00bb0ca6 100644 --- a/src/ArduinoIoTCloudTCP.h +++ b/src/ArduinoIoTCloudTCP.h @@ -123,6 +123,11 @@ class ArduinoIoTCloudTCP: public ArduinoIoTCloudClass State _state; + int _tz_offset; + Property * _tz_offset_property; + unsigned int _tz_dst_until; + Property * _tz_dst_until_property; + unsigned long _next_connection_attempt_tick; unsigned int _last_connection_attempt_cnt; unsigned long _next_device_subscribe_attempt_tick; From 4cf7a3417dcf813d0423a5a45a6b5f114363d884 Mon Sep 17 00:00:00 2001 From: pennam Date: Mon, 8 Apr 2024 17:12:04 +0200 Subject: [PATCH 4/6] Add writeOnDemand testcase --- extras/test/CMakeLists.txt | 1 + extras/test/src/test_writeOnDemand.cpp | 38 ++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 extras/test/src/test_writeOnDemand.cpp diff --git a/extras/test/CMakeLists.txt b/extras/test/CMakeLists.txt index 8e5148905..c330afd10 100644 --- a/extras/test/CMakeLists.txt +++ b/extras/test/CMakeLists.txt @@ -41,6 +41,7 @@ set(TEST_SRCS src/test_publishOnChangeRateLimit.cpp src/test_readOnly.cpp src/test_writeOnly.cpp + src/test_writeOnDemand.cpp ) set(TEST_UTIL_SRCS diff --git a/extras/test/src/test_writeOnDemand.cpp b/extras/test/src/test_writeOnDemand.cpp new file mode 100644 index 000000000..5bd0c3782 --- /dev/null +++ b/extras/test/src/test_writeOnDemand.cpp @@ -0,0 +1,38 @@ +/* + Copyright (c) 2019 Arduino. All rights reserved. +*/ + +/************************************************************************************** + INCLUDE + **************************************************************************************/ + +#include + +#include + +#include +#include + +/************************************************************************************** + TEST CODE + **************************************************************************************/ + +SCENARIO("An Arduino cloud property is marked 'write on demand'", "[ArduinoCloudThing::decode]") +{ + PropertyContainer property_container; + + CloudInt test = 0; + addPropertyToContainer(property_container, test, "test", Permission::ReadWrite).writeOnDemand(); + + /* [{0: "test", 2: 7}] = 81 A2 00 64 74 65 73 74 02 07 */ + uint8_t const payload[] = {0x81, 0xA2, 0x00, 0x64, 0x74, 0x65, 0x73, 0x74, 0x02, 0x07}; + int const payload_length = sizeof(payload) / sizeof(uint8_t); + CBORDecoder::decode(property_container, payload, payload_length); + + REQUIRE(test == 0); + + Property* p = getProperty(property_container, "test"); + p->fromCloudToLocal(); + + REQUIRE(test == 7); +} From 11e77ea8b9af8bbc8d1ad2c154167d5e7a12ffc0 Mon Sep 17 00:00:00 2001 From: pennam Date: Tue, 9 Apr 2024 08:50:09 +0200 Subject: [PATCH 5/6] Add writeOnChange testcase --- extras/test/CMakeLists.txt | 1 + extras/test/src/test_writeOnChange.cpp | 33 ++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 extras/test/src/test_writeOnChange.cpp diff --git a/extras/test/CMakeLists.txt b/extras/test/CMakeLists.txt index c330afd10..2588c27c0 100644 --- a/extras/test/CMakeLists.txt +++ b/extras/test/CMakeLists.txt @@ -42,6 +42,7 @@ set(TEST_SRCS src/test_readOnly.cpp src/test_writeOnly.cpp src/test_writeOnDemand.cpp + src/test_writeOnChange.cpp ) set(TEST_UTIL_SRCS diff --git a/extras/test/src/test_writeOnChange.cpp b/extras/test/src/test_writeOnChange.cpp new file mode 100644 index 000000000..0896bbdfd --- /dev/null +++ b/extras/test/src/test_writeOnChange.cpp @@ -0,0 +1,33 @@ +/* + Copyright (c) 2019 Arduino. All rights reserved. +*/ + +/************************************************************************************** + INCLUDE + **************************************************************************************/ + +#include + +#include + +#include +#include + +/************************************************************************************** + TEST CODE + **************************************************************************************/ + +SCENARIO("An Arduino cloud property is marked 'write on change'", "[ArduinoCloudThing::decode]") +{ + PropertyContainer property_container; + + CloudInt test = 0; + addPropertyToContainer(property_container, test, "test", Permission::ReadWrite).writeOnChange(); + + /* [{0: "test", 2: 7}] = 81 A2 00 64 74 65 73 74 02 07 */ + uint8_t const payload[] = {0x81, 0xA2, 0x00, 0x64, 0x74, 0x65, 0x73, 0x74, 0x02, 0x07}; + int const payload_length = sizeof(payload) / sizeof(uint8_t); + CBORDecoder::decode(property_container, payload, payload_length); + + REQUIRE(test == 7); +} From b629c11e2d3bceb080c8101ca790759fe9d9dc4c Mon Sep 17 00:00:00 2001 From: pennam Date: Tue, 9 Apr 2024 14:49:59 +0200 Subject: [PATCH 6/6] ArduinoIoTCloudClass: Remove unused variable --- src/ArduinoIoTCloud.cpp | 1 - src/ArduinoIoTCloud.h | 1 - 2 files changed, 2 deletions(-) diff --git a/src/ArduinoIoTCloud.cpp b/src/ArduinoIoTCloud.cpp index ce191eafd..84f08f780 100644 --- a/src/ArduinoIoTCloud.cpp +++ b/src/ArduinoIoTCloud.cpp @@ -34,7 +34,6 @@ ArduinoIoTCloudClass::ArduinoIoTCloudClass() , _lib_version{AIOT_CONFIG_LIB_VERSION} , _device_id{""} , _cloud_event_callback{nullptr} -, _thing_id_outdated{false} { } diff --git a/src/ArduinoIoTCloud.h b/src/ArduinoIoTCloud.h index 9fa9d2963..7436c0baf 100644 --- a/src/ArduinoIoTCloud.h +++ b/src/ArduinoIoTCloud.h @@ -162,7 +162,6 @@ class ArduinoIoTCloudClass String _device_id; OnCloudEventCallback _cloud_event_callback[3]; - bool _thing_id_outdated; }; #ifdef HAS_TCP