Redesigning OTALogic in a way that allows it to be kept on the stack without being allocated on the heap with 'new'

aentinger · aentinger · commit ca5c266a2730 · 2020-05-28T10:35:04.000+02:00
diff --git a/extras/test/src/test_OTALogic.cpp b/extras/test/src/test_OTALogic.cpp
@@ -63,7 +63,8 @@ TEST_CASE("OTAStorage initialisation fails", "[OTAStorage::init() -> returns fal
 
 
   /* Perform test */
-  OTALogic ota_logic(ota_storage.get());
+  OTALogic ota_logic;
+  ota_logic.setOTAStorage(ota_storage.get());
 
   WHEN("OTALogic::update() is called")
   {
@@ -95,7 +96,8 @@ TEST_CASE("OTAStorage opening of storage file fails", "[OTAStorage::open() -> re
 
 
   /* Perform test */
-  OTALogic ota_logic(ota_storage.get());
+  OTALogic ota_logic;
+  ota_logic.setOTAStorage(ota_storage.get());
 
   WHEN("OTALogic::update() is called and some bytes have been received")
   {
@@ -131,7 +133,8 @@ TEST_CASE("OTAStorage writing to storage file fails", "[OTAStorage::write() -> f
 
 
   /* Perform test */
-  OTALogic ota_logic(ota_storage.get());
+  OTALogic ota_logic;
+  ota_logic.setOTAStorage(ota_storage.get());
 
   WHEN("OTALogic::update() is called and some bytes have been received")
   {
@@ -166,7 +169,8 @@ TEST_CASE("Data overrun due to receiving too much data", "[OTALogic - Data Overr
 
 
   /* Perform test */
-  OTALogic ota_logic(ota_storage.get());
+  OTALogic ota_logic;
+  ota_logic.setOTAStorage(ota_storage.get());
 
   WHEN("Too much data is received before OTALogic::update() is called again to process the incoming data")
   {
@@ -213,7 +217,8 @@ TEST_CASE("Valid OTA data is received ", "[OTALogic]")
 
 
   /* Perform test */
-  OTALogic ota_logic(ota_storage.get());
+  OTALogic ota_logic;
+  ota_logic.setOTAStorage(ota_storage.get());
   simulateOTABinaryReception(ota_logic, valid_ota_test_data);
 
 
@@ -259,7 +264,8 @@ TEST_CASE("Invalid OTA data is received ", "[OTALogic - CRC wrong]")
 
 
   /* Perform test */
-  OTALogic ota_logic(ota_storage.get());
+  OTALogic ota_logic;
+  ota_logic.setOTAStorage(ota_storage.get());
   simulateOTABinaryReception(ota_logic, invalid_valid_ota_test_data_crc_wrong);
 
   
diff --git a/src/ArduinoIoTCloudTCP.cpp b/src/ArduinoIoTCloudTCP.cpp
@@ -78,7 +78,6 @@ ArduinoIoTCloudTCP::ArduinoIoTCloudTCP()
 , _dataTopicIn("")
 , _ota_topic_in{""}
 #if OTA_ENABLED
-, _ota_logic{nullptr}
 , _ota_storage_type{static_cast<int>(OTAStorage::Type::NotAvailable)}
 , _ota_error{static_cast<int>(OTAError::None)}
 #endif /* OTA_ENABLED */
@@ -155,10 +154,8 @@ void ArduinoIoTCloudTCP::update()
    * 'update' method here in order to process incoming data and generally
    * to transition to the OTA logic update states.
    */
-  if (_ota_logic) {
-    OTAError const err = _ota_logic->update();
-    _ota_error = static_cast<int>(err);
-  }
+  OTAError const err = _ota_logic.update();
+  _ota_error = static_cast<int>(err);
 #endif /* OTA_ENABLED */
 
   // Check if a primitive property wrapper is locally changed
@@ -217,8 +214,7 @@ void ArduinoIoTCloudTCP::setOTAStorage(OTAStorage & ota_storage)
   _ota_storage_type = static_cast<int>(ota_storage.type());
   addPropertyReal(_ota_storage_type, "OTA_STORAGE_TYPE", Permission::Read);
   addPropertyReal(_ota_error, "OTA_ERROR", Permission::Read);
-  if(_ota_logic) delete _ota_logic;
-  _ota_logic = new OTALogic(ota_storage);
+  _ota_logic.setOTAStorage(ota_storage);
 }
 #endif /* OTA_ENABLED */
 
@@ -287,8 +283,8 @@ void ArduinoIoTCloudTCP::handleMessage(int length)
     _syncStatus = ArduinoIoTSynchronizationStatus::SYNC_STATUS_VALUES_PROCESSED;
   }
 #if OTA_ENABLED
-  if (_ota_logic && (_ota_topic_in == topic)) {
-    _ota_logic->onOTADataReceived(bytes, length);
+  if (_ota_topic_in == topic) {
+    _ota_logic.onOTADataReceived(bytes, length);
   }
 #endif /* OTA_ENABLED */
 }
diff --git a/src/ArduinoIoTCloudTCP.h b/src/ArduinoIoTCloudTCP.h
@@ -128,7 +128,7 @@ class ArduinoIoTCloudTCP: public ArduinoIoTCloudClass
     String _ota_topic_in;
 
 #if OTA_ENABLED
-    OTALogic * _ota_logic;
+    OTALogic _ota_logic;
     int _ota_storage_type;
     int _ota_error;
 #endif /* OTA_ENABLED */
diff --git a/src/ArduinoIoTCloud_Config.h b/src/ArduinoIoTCloud_Config.h
@@ -23,7 +23,7 @@
  ******************************************************************************/
 
 #ifndef OTA_STORAGE_MKRMEM
-  #define OTA_STORAGE_MKRMEM      (0)
+  #define OTA_STORAGE_MKRMEM      (1)
 #endif
 
 /******************************************************************************
diff --git a/src/utility/ota/OTALogic.cpp b/src/utility/ota/OTALogic.cpp
@@ -41,8 +41,8 @@
  * CTOR/DTOR
  ******************************************************************************/
 
-OTALogic::OTALogic(OTAStorage & ota_storage)
-: _ota_storage(ota_storage)
+OTALogic::OTALogic()
+: _ota_storage{nullptr}
 , _ota_state{OTAState::Init}
 , _ota_error{OTAError::None}
 {
@@ -105,7 +105,7 @@ void OTALogic::onOTADataReceived(uint8_t const * const data, size_t const length
 
 OTAState OTALogic::handle_Init()
 {
-  if (_ota_storage.init()) {
+  if (_ota_storage->init()) {
     return OTAState::Idle;
   } else {
     _ota_error = OTAError::StorageInitFailed;
@@ -123,7 +123,7 @@ OTAState OTALogic::handle_Idle()
 
 OTAState OTALogic::handle_StartDownload()
 {
-  if(_ota_storage.open()) {
+  if(_ota_storage->open()) {
     return OTAState::WaitForHeader;
   } else {
     _ota_error = OTAError::StorageOpenFailed;
@@ -187,7 +187,7 @@ OTAState OTALogic::handle_WaitForBinary()
 OTAState OTALogic::handle_BinaryReceived()
 {
   /* Write to OTA storage */
-  if(_ota_storage.write(_mqtt_ota_buf.buf, _mqtt_ota_buf.num_bytes) != _mqtt_ota_buf.num_bytes)
+  if(_ota_storage->write(_mqtt_ota_buf.buf, _mqtt_ota_buf.num_bytes) != _mqtt_ota_buf.num_bytes)
   {
     _ota_error = OTAError::StorageWriteFailed;
     return OTAState::Error;
@@ -201,7 +201,7 @@ OTAState OTALogic::handle_BinaryReceived()
   _mqtt_ota_buf.num_bytes = 0;
 
   if(_ota_bin_data.bytes_received >= _ota_bin_data.hdr_len) {
-    _ota_storage.close();
+    _ota_storage->close();
     _ota_bin_data.crc32 = crc_finalize(_ota_bin_data.crc32);
     return OTAState::Verify;
   }
@@ -212,10 +212,10 @@ OTAState OTALogic::handle_BinaryReceived()
 OTAState OTALogic::handle_Verify()
 {
   if(_ota_bin_data.crc32 == _ota_bin_data.hdr_crc32) {
-    _ota_storage.deinit();
+    _ota_storage->deinit();
     return OTAState::Reset;
   } else {
-    _ota_storage.remove();
+    _ota_storage->remove();
     _ota_error = OTAError::ChecksumMismatch;
     return OTAState::Error;
   }
diff --git a/src/utility/ota/OTALogic.h b/src/utility/ota/OTALogic.h
@@ -67,7 +67,10 @@ class OTALogic
 
 public:
 
-  OTALogic(OTAStorage & ota_storage);
+  OTALogic();
+
+
+  inline void setOTAStorage(OTAStorage & ota_storage) { _ota_storage = &ota_storage; }
 
 
   OTAError update();
@@ -95,7 +98,7 @@ class OTALogic
     crc_t    crc32;
   } sOTABinaryData;
 
-  OTAStorage & _ota_storage;
+  OTAStorage * _ota_storage;
   OTAState _ota_state;
   OTAError _ota_error;
   sMQTTOTABuffer _mqtt_ota_buf;
