Avoid multivalue properties split

extras/test/src/util/CBORTestUtil.cpp

@@ -27,9 +27,10 @@ namespace cbor
 std::vector<uint8_t> encode(PropertyContainer & property_container, bool lightPayload)
 {
   int bytes_encoded = 0;
-  uint8_t buf[200] = {0};
+  unsigned int starting_property_index = 0;
+  uint8_t buf[256] = {0};
 
-  if (CBOREncoder::encode(property_container, buf, 200, bytes_encoded, lightPayload) == CborNoError)
+  if (CBOREncoder::encode(property_container, buf, 256, bytes_encoded, starting_property_index, lightPayload) == CborNoError)
     return std::vector<uint8_t>(buf, buf + bytes_encoded);
   else
     return std::vector<uint8_t>();

src/ArduinoIoTCloud.cpp

@@ -27,6 +27,7 @@
 
 ArduinoIoTCloudClass::ArduinoIoTCloudClass()
 : _connection{nullptr}
+, _last_checked_property_index{0}
 , _time_service(ArduinoIoTCloudTimeService())
 , _tz_offset{0}
 , _tz_dst_until{0}

src/ArduinoIoTCloud.h

@@ -148,6 +148,7 @@ class ArduinoIoTCloudClass
 
     ConnectionHandler * _connection;
     PropertyContainer _property_container;
+    unsigned int _last_checked_property_index;
     TimeService & _time_service;
     int _tz_offset;
     unsigned int _tz_dst_until;

src/ArduinoIoTCloudLPWAN.cpp

@@ -150,7 +150,7 @@ void ArduinoIoTCloudLPWAN::sendPropertiesToCloud()
   int bytes_encoded = 0;
   uint8_t data[CBOR_LORA_MSG_MAX_SIZE];
 
-  if (CBOREncoder::encode(_property_container, data, sizeof(data), bytes_encoded, true) == CborNoError)
+  if (CBOREncoder::encode(_property_container, data, sizeof(data), bytes_encoded, _last_checked_property_index, true) == CborNoError)
     if (bytes_encoded > 0)
       writeProperties(data, bytes_encoded);
 }

src/ArduinoIoTCloudTCP.cpp

@@ -576,12 +576,12 @@ void ArduinoIoTCloudTCP::handleMessage(int length)
   }
 }
 
-void ArduinoIoTCloudTCP::sendPropertyContainerToCloud(PropertyContainer & property_container)
+void ArduinoIoTCloudTCP::sendPropertyContainerToCloud(PropertyContainer & property_container, unsigned int & current_property_index)
 {
   int bytes_encoded = 0;
   uint8_t data[MQTT_TRANSMIT_BUFFER_SIZE];
 
-  if (CBOREncoder::encode(property_container, data, sizeof(data), bytes_encoded, false) == CborNoError)
+  if (CBOREncoder::encode(property_container, data, sizeof(data), bytes_encoded, current_property_index, false) == CborNoError)
     if (bytes_encoded > 0)
     {
       /* If properties have been encoded store them in the back-up buffer
@@ -596,13 +596,14 @@ void ArduinoIoTCloudTCP::sendPropertyContainerToCloud(PropertyContainer & proper
 
 void ArduinoIoTCloudTCP::sendPropertiesToCloud()
 {
-  sendPropertyContainerToCloud(_property_container);
+  sendPropertyContainerToCloud(_property_container, _last_checked_property_index);
 }
 
 #if OTA_ENABLED
 void ArduinoIoTCloudTCP::sendOTAPropertiesToCloud()
 {
   PropertyContainer ota_property_container;
+  unsigned int last_ota_property_index = 0;
 
   std::list<String> const ota_property_list {"OTA_CAP", "OTA_ERROR", "OTA_SHA256", "OTA_URL", "OTA_REQ"};
   std::for_each(ota_property_list.cbegin(),
@@ -615,7 +616,7 @@ void ArduinoIoTCloudTCP::sendOTAPropertiesToCloud()
                 }
                 );
 
-  sendPropertyContainerToCloud(ota_property_container);
+  sendPropertyContainerToCloud(ota_property_container, last_ota_property_index);
 }
 #endif
 

src/ArduinoIoTCloudTCP.h

@@ -164,7 +164,7 @@ class ArduinoIoTCloudTCP: public ArduinoIoTCloudClass
 
     static void onMessage(int length);
     void handleMessage(int length);
-    void sendPropertyContainerToCloud(PropertyContainer & property_container);
+    void sendPropertyContainerToCloud(PropertyContainer & property_container, unsigned int & current_property_index);
     void sendPropertiesToCloud();
     void requestLastValue();
     int write(String const topic, byte const data[], int const length);

src/cbor/CBOREncoder.cpp

@@ -24,50 +24,184 @@
 #undef max
 #undef min
 #include <algorithm>
+#include <iterator>
 
 #include "lib/tinycbor/cbor-lib.h"
 
 /******************************************************************************
  * PUBLIC MEMBER FUNCTIONS
  ******************************************************************************/
 
-CborError CBOREncoder::encode(PropertyContainer & property_container, uint8_t * data, size_t const size, int & bytes_encoded, bool lightPayload)
+CborError CBOREncoder::encode(PropertyContainer & property_container, uint8_t * data, size_t const size, int & bytes_encoded, unsigned int & current_property_index, bool lightPayload)
 {
-  CborEncoder encoder, arrayEncoder;
+  EncoderState current_state = EncoderState::InitPropertyEncoder,
+               next_state = EncoderState::InitPropertyEncoder;
 
-  cbor_encoder_init(&encoder, data, size, 0);
+  PropertyContainerEncoder propertyEncoder(property_container, current_property_index);
 
-  CHECK_CBOR(cbor_encoder_create_array(&encoder, &arrayEncoder, CborIndefiniteLength));
+  while (current_state != EncoderState::SendMessage) {
 
-  /* Check if backing storage and cloud has diverged
-   * time interval may be elapsed or property may be changed
+    switch (current_state) {
+      case EncoderState::InitPropertyEncoder      : next_state = handle_InitPropertyEncoder(propertyEncoder); break;
+      case EncoderState::OpenCBORContainer        : next_state = handle_OpenCBORContainer(propertyEncoder, data, size); break;
+      case EncoderState::TryAppend                : next_state = handle_TryAppend(propertyEncoder, lightPayload); break;
+      case EncoderState::OutOfMemory              : next_state = handle_OutOfMemory(propertyEncoder); break;
+      case EncoderState::SkipProperty             : next_state = handle_SkipProperty(propertyEncoder); break;
+      case EncoderState::TrimAppend               : next_state = handle_TrimAppend(propertyEncoder); break;
+      case EncoderState::CloseCBORContainer       : next_state = handle_CloseCBORContainer(propertyEncoder); break;
+      case EncoderState::TrimClose                : next_state = handle_TrimClose(propertyEncoder); break;
+      case EncoderState::FinishAppend             : next_state = handle_FinishAppend(propertyEncoder); break;
+      case EncoderState::SendMessage              : /* Nothing to do */ break;
+      case EncoderState::Error                    : return CborErrorInternalError; break;
+    }
+
+    current_state = next_state;
+  }
+
+  if (propertyEncoder.encoded_property_count > 0)
+    bytes_encoded = cbor_encoder_get_buffer_size(&propertyEncoder.encoder, data);
+  else
+    bytes_encoded = 0;
+
+  return CborNoError;
+}
+
+/******************************************************************************
+   PRIVATE MEMBER FUNCTIONS
+ ******************************************************************************/
+
+CBOREncoder::EncoderState CBOREncoder::handle_InitPropertyEncoder(PropertyContainerEncoder & propertyEncoder)
+{
+  propertyEncoder.encoded_property_count = 0;
+  propertyEncoder.checked_property_count = 0;
+  propertyEncoder.encoded_property_limit = 0;
+  propertyEncoder.property_limit_active  = false;
+  return EncoderState::OpenCBORContainer;
+}
+
+CBOREncoder::EncoderState CBOREncoder::handle_OpenCBORContainer(PropertyContainerEncoder & propertyEncoder, uint8_t * data, size_t const size)
+{
+  propertyEncoder.encoded_property_count = 0;
+  propertyEncoder.checked_property_count = 0;
+  cbor_encoder_init(&propertyEncoder.encoder, data, size, 0);
+  cbor_encoder_create_array(&propertyEncoder.encoder, &propertyEncoder.arrayEncoder, CborIndefiniteLength);
+  return EncoderState::TryAppend;
+}
+
+CBOREncoder::EncoderState CBOREncoder::handle_TryAppend(PropertyContainerEncoder & propertyEncoder, bool  & lightPayload)
+{
+  /* Check if backing storage and cloud has diverged. Time interval may be elapsed or property may be changed
    * and if that's the case encode the property into the CBOR.
    */
   CborError error = CborNoError;
-  int num_encoded_properties = 0;
-  std::for_each(property_container.begin(),
-                property_container.end(),
-                [lightPayload, &arrayEncoder, &error, &num_encoded_properties](Property * p)
-                {
-                  if (p->shouldBeUpdated() && p->isReadableByCloud())
-                  {
-                    error = p->append(&arrayEncoder, lightPayload);
-                    if(error == CborNoError)
-                      num_encoded_properties++;
-                    else
-                      return;
-                  }
-                });
-  if ((CborNoError != error) && 
-      (CborErrorOutOfMemory != error))
-    return error;
-
-  CHECK_CBOR(cbor_encoder_close_container(&encoder, &arrayEncoder));
-
-  if (num_encoded_properties > 0)
-    bytes_encoded = cbor_encoder_get_buffer_size(&encoder, data);
+  PropertyContainer::iterator iter = propertyEncoder.property_container.begin();
+  std::advance(iter, propertyEncoder.current_property_index);
+
+  for(; iter != propertyEncoder.property_container.end(); iter++)
+  {
+    Property * p = * iter;
+
+    if (p->shouldBeUpdated() && p->isReadableByCloud())
+    {
+      error = p->append(&propertyEncoder.arrayEncoder, lightPayload);
+      if(error == CborNoError)
+        propertyEncoder.encoded_property_count++;
+    }
+    if(error == CborNoError)
+      propertyEncoder.checked_property_count++;
+
+    bool const maximum_number_of_properties_reached = (propertyEncoder.encoded_property_count >= propertyEncoder.encoded_property_limit) && (propertyEncoder.property_limit_active == true);
+    bool const cbor_encoder_error = (error != CborNoError);
+
+    if (maximum_number_of_properties_reached || cbor_encoder_error)
+      break;
+  }
+
+  if (CborErrorOutOfMemory == error)
+    return EncoderState::OutOfMemory;
+  else if (CborNoError == error)
+    return EncoderState::CloseCBORContainer;
+  else if (CborErrorSplitItems == error)
+    return EncoderState::TrimAppend;
   else
-    bytes_encoded = 0;
+    return EncoderState::Error;
+}
 
-  return CborNoError;
+CBOREncoder::EncoderState CBOREncoder::handle_OutOfMemory(PropertyContainerEncoder & propertyEncoder)
+{
+  if(propertyEncoder.encoded_property_count > 0)
+    return EncoderState::CloseCBORContainer;
+  else
+    return EncoderState::SkipProperty;
+}
+
+CBOREncoder::EncoderState CBOREncoder::handle_SkipProperty(PropertyContainerEncoder & propertyEncoder)
+{
+  /* Better to skip this property otherwise we will stay blocked here. This happens only with a message property 
+   * that not fits into the CBOR buffer
+   */
+  propertyEncoder.current_property_index++;
+  if(propertyEncoder.current_property_index >= propertyEncoder.property_container.size())
+    propertyEncoder.current_property_index = 0;
+  return EncoderState::Error;
+}
+
+CBOREncoder::EncoderState CBOREncoder::handle_TrimAppend(PropertyContainerEncoder & propertyEncoder)
+{
+  /* Trim the number of properties to be included in the next message to avoid multivalue property split */
+  propertyEncoder.encoded_property_limit = propertyEncoder.encoded_property_count;
+  propertyEncoder.property_limit_active = true;
+  if(propertyEncoder.encoded_property_limit > 0)
+    return EncoderState::OpenCBORContainer;
+  else
+    return EncoderState::Error;
+}
+
+CBOREncoder::EncoderState CBOREncoder::handle_CloseCBORContainer(PropertyContainerEncoder & propertyEncoder)
+{
+  CborError error = cbor_encoder_close_container(&propertyEncoder.encoder, &propertyEncoder.arrayEncoder);
+  if (CborNoError != error)
+    return EncoderState::TrimClose;
+  else
+    return EncoderState::FinishAppend;
+}
+
+CBOREncoder::EncoderState CBOREncoder::handle_TrimClose(PropertyContainerEncoder & propertyEncoder)
+{
+  /* Trim the number of properties to be included in the next message to avoid error closing container */
+  propertyEncoder.encoded_property_limit = propertyEncoder.encoded_property_count - 1;
+  propertyEncoder.property_limit_active = true;
+  if(propertyEncoder.encoded_property_limit > 0)
+    return EncoderState::OpenCBORContainer;
+  else
+    return EncoderState::Error;
+}
+
+CBOREncoder::EncoderState CBOREncoder::handle_FinishAppend(PropertyContainerEncoder & propertyEncoder)
+{
+  /* Restore property message limit to CBOR_ENCODER_NO_PROPERTIES_LIMIT */
+  propertyEncoder.property_limit_active = false;
+
+  /* The append process has been successful, so we don't need to terty to send this properties set. Cleanup _has_been_appended_but_not_sended flag */
+  PropertyContainer::iterator iter = propertyEncoder.property_container.begin();
+  std::advance(iter, propertyEncoder.current_property_index);
+  int num_appended_properties = 0;
+
+  for(; iter != propertyEncoder.property_container.end(); iter++)
+  {
+    Property * p = * iter;
+    if (num_appended_properties >= propertyEncoder.checked_property_count)
+      break;
+
+    p->appendCompleted();
+    num_appended_properties++;
+  }
+
+  /* Advance property index for the nex message */
+  propertyEncoder.current_property_index += propertyEncoder.checked_property_count;
+
+  if(propertyEncoder.current_property_index >= propertyEncoder.property_container.size())
+    propertyEncoder.current_property_index = 0;
+
+  return EncoderState::SendMessage;
 }

src/cbor/CBOREncoder.h

@@ -42,16 +42,54 @@ class CBOREncoder
 {
 
 public:
-
     /* encode return > 0 if a property has changed and encodes the changed properties in CBOR format into the provided buffer */
     /* if lightPayload is true the integer identifier of the property will be encoded in the message instead of the property name in order to reduce the size of the message payload*/
-    static CborError encode(PropertyContainer & property_container, uint8_t * data, size_t const size, int & bytes_encoded, bool lightPayload = false);
+    static CborError encode(PropertyContainer & property_container, uint8_t * data, size_t const size, int & bytes_encoded, unsigned int & current_property_index, bool lightPayload = false);
 
 private:
 
   CBOREncoder() { }
   CBOREncoder(CborEncoder const &) { }
 
+  enum class EncoderState
+  {
+    InitPropertyEncoder,
+    OpenCBORContainer,
+    TryAppend,
+    OutOfMemory,
+    SkipProperty,
+    TrimAppend,
+    CloseCBORContainer,
+    TrimClose,
+    FinishAppend,
+    SendMessage,
+    Error
+  };
+
+  struct PropertyContainerEncoder
+  {
+    PropertyContainerEncoder(PropertyContainer & _property_container, unsigned int & _current_property_index): property_container(_property_container), current_property_index(_current_property_index) { }
+    PropertyContainer & property_container;
+    unsigned int & current_property_index;
+    int encoded_property_count;
+    int checked_property_count;
+    int encoded_property_limit;
+    bool property_limit_active;
+    CborEncoder encoder;
+    CborEncoder arrayEncoder;
+  };
+
+  static EncoderState handle_InitPropertyEncoder(PropertyContainerEncoder & propertyEncoder);
+  static EncoderState handle_OpenCBORContainer(PropertyContainerEncoder & propertyEncoder, uint8_t * data, size_t const size);
+  static EncoderState handle_TryAppend(PropertyContainerEncoder & propertyEncoder, bool  & lightPayload);
+  static EncoderState handle_OutOfMemory(PropertyContainerEncoder & propertyEncoder);
+  static EncoderState handle_SkipProperty(PropertyContainerEncoder & propertyEncoder);
+  static EncoderState handle_TrimAppend(PropertyContainerEncoder & propertyEncoder);
+  static EncoderState handle_CloseCBORContainer(PropertyContainerEncoder & propertyEncoder);
+  static EncoderState handle_TrimClose(PropertyContainerEncoder & propertyEncoder);
+  static EncoderState handle_FinishAppend(PropertyContainerEncoder & propertyEncoder);
+  static EncoderState handle_AdvancePropertyContainer(PropertyContainerEncoder & propertyEncoder);
+
 };
 
 #endif /* ARDUINO_CBOR_CBOR_ENCODER_H_ */

src/cbor/lib/tinycbor/cbor-lib.h

@@ -21,4 +21,16 @@
     } while(0);
 #endif /* CHECK_CBOR */
 
+#ifndef CHECK_CBOR_MULTI
+  #define CHECK_CBOR_MULTI(expr)          \
+    do {                                  \
+      CborError error = CborNoError;      \
+      error = (expr);                     \
+      if (CborErrorOutOfMemory == error)  \
+        return CborErrorSplitItems;     \
+      if (CborNoError != error)           \
+        return error;                     \
+    } while(0);
+#endif /* CHECK_CBOR_MULTI */
+
 #endif

src/cbor/lib/tinycbor/src/cbor.h

@@ -189,6 +189,7 @@ typedef enum CborError {
     /* encoder errors */
     CborErrorTooManyItems = 768,
     CborErrorTooFewItems,
+    CborErrorSplitItems,
 
     /* internal implementation errors */
     CborErrorDataTooLarge = 1024,

src/cbor/lib/tinycbor/src/cborerrorstrings.c

@@ -163,6 +163,9 @@ const char *cbor_error_string(CborError error)
     case CborErrorTooFewItems:
         return _("too few items added to encoder");
 
+    case CborErrorSplitItems:
+        return _("splitted item added to encoder");
+
     case CborErrorDataTooLarge:
         return _("internal error: data too large");