Merge pull request #440 from arduino-libraries/callback-remove

pennam · web-flow · commit c5a59b7f21fe · 2024-05-13T10:25:09.000+02:00
Remove public functions to handle thing_id and timezone
diff --git a/extras/test/CMakeLists.txt b/extras/test/CMakeLists.txt
@@ -41,6 +41,8 @@ set(TEST_SRCS
   src/test_publishOnChangeRateLimit.cpp
   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
@@ -0,0 +1,33 @@
+/*
+   Copyright (c) 2019 Arduino.  All rights reserved.
+*/
+
+/**************************************************************************************
+   INCLUDE
+ **************************************************************************************/
+
+#include <catch.hpp>
+
+#include <util/CBORTestUtil.h>
+
+#include <CBORDecoder.h>
+#include <PropertyContainer.h>
+
+/**************************************************************************************
+   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);
+}
diff --git a/extras/test/src/test_writeOnDemand.cpp b/extras/test/src/test_writeOnDemand.cpp
@@ -0,0 +1,38 @@
+/*
+   Copyright (c) 2019 Arduino.  All rights reserved.
+*/
+
+/**************************************************************************************
+   INCLUDE
+ **************************************************************************************/
+
+#include <catch.hpp>
+
+#include <util/CBORTestUtil.h>
+
+#include <CBORDecoder.h>
+#include <PropertyContainer.h>
+
+/**************************************************************************************
+   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);
+}
diff --git a/src/ArduinoIoTCloud.cpp b/src/ArduinoIoTCloud.cpp
@@ -29,13 +29,11 @@ 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}
 , _device_id{""}
 , _cloud_event_callback{nullptr}
-, _thing_id_outdated{false}
 {
 
 }
diff --git a/src/ArduinoIoTCloud.h b/src/ArduinoIoTCloud.h
@@ -96,17 +96,10 @@ 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(); }
     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);
 
@@ -157,9 +150,8 @@ 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;
 
     void execCloudEventCallback(ArduinoIoTCloudEvent const event);
@@ -170,7 +162,6 @@ class ArduinoIoTCloudClass
 
     String _device_id;
     OnCloudEventCallback _cloud_event_callback[3];
-    bool _thing_id_outdated;
 };
 
 #ifdef HAS_TCP
diff --git a/src/ArduinoIoTCloudTCP.cpp b/src/ArduinoIoTCloudTCP.cpp
@@ -53,22 +53,16 @@ unsigned long getTime()
   return ArduinoCloud.getInternalTime();
 }
 
-void updateTimezoneInfo()
-{
-  ArduinoCloud.updateInternalTimezoneInfo();
-}
-
-void setThingIdOutdated()
-{
-  ArduinoCloud.setThingIdOutdatedFlag();
-}
-
 /******************************************************************************
    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}
@@ -218,10 +212,11 @@ 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);
-
-  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);
+  _thing_id_property = &addPropertyToContainer(_device_property_container, *p, "thing_id", Permission::ReadWrite, -1).writeOnDemand();
+  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();
@@ -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;
     }
@@ -589,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;
@@ -762,12 +767,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();
 }
 
 /******************************************************************************
diff --git 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;
diff --git 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
@@ -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
@@ -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();
     }

Original file line number	Diff line number	Diff line change
`@@ -41,6 +41,8 @@ set(TEST_SRCS`
`41`	`41`	`src/test_publishOnChangeRateLimit.cpp`
`42`	`42`	`src/test_readOnly.cpp`
`43`	`43`	`src/test_writeOnly.cpp`
	`44`	`+ src/test_writeOnDemand.cpp`
	`45`	`+ src/test_writeOnChange.cpp`
`44`	`46`	`)`
`45`	`47`
`46`	`48`	`set(TEST_UTIL_SRCS`
Original file line number	Diff line number	Diff line change
`@@ -29,13 +29,11 @@ ArduinoIoTCloudClass::ArduinoIoTCloudClass()`
`29`	`29`	`: _connection{nullptr}`
`30`	`30`	`, _last_checked_property_index{0}`
`31`	`31`	`, _time_service(TimeService)`
`32`		`-, _tz_offset{0}`
`33`		`-, _tz_dst_until{0}`
`34`	`32`	`, _thing_id{""}`
	`33`	`+, _thing_id_property{nullptr}`
`35`	`34`	`, _lib_version{AIOT_CONFIG_LIB_VERSION}`
`36`	`35`	`, _device_id{""}`
`37`	`36`	`, _cloud_event_callback{nullptr}`
`38`		`-, _thing_id_outdated{false}`
`39`	`37`	`{`
`40`	`38`
`41`	`39`	`}`