Skip to content

Commit 40bfd31

Browse files
pennampennam
authored
Merge pull request #293 from pennam/avoid_multi_split
Avoid multivalue properties split
2 parents bcccca8 + 78b3158 commit 40bfd31

21 files changed

+365
-77
lines changed

‎extras/test/src/test_encode.cpp

+77-11
Large diffs are not rendered by default.

‎extras/test/src/util/CBORTestUtil.cpp

+3-2
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,10 @@ namespace cbor
2727
std::vector<uint8_t> encode(PropertyContainer & property_container, bool lightPayload)
2828
{
2929
int bytes_encoded = 0;
30-
uint8_t buf[200] = {0};
30+
unsigned int starting_property_index = 0;
31+
uint8_t buf[256] = {0};
3132

32-
if (CBOREncoder::encode(property_container, buf, 200, bytes_encoded, lightPayload) == CborNoError)
33+
if (CBOREncoder::encode(property_container, buf, 256, bytes_encoded, starting_property_index, lightPayload) == CborNoError)
3334
return std::vector<uint8_t>(buf, buf + bytes_encoded);
3435
else
3536
return std::vector<uint8_t>();

‎src/ArduinoIoTCloud.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
ArduinoIoTCloudClass::ArduinoIoTCloudClass()
2929
: _connection{nullptr}
30+
, _last_checked_property_index{0}
3031
, _time_service(ArduinoIoTCloudTimeService())
3132
, _tz_offset{0}
3233
, _tz_dst_until{0}

‎src/ArduinoIoTCloud.h

+1
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ class ArduinoIoTCloudClass
148148

149149
ConnectionHandler * _connection;
150150
PropertyContainer _property_container;
151+
unsigned int _last_checked_property_index;
151152
TimeService & _time_service;
152153
int _tz_offset;
153154
unsigned int _tz_dst_until;

‎src/ArduinoIoTCloudLPWAN.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ void ArduinoIoTCloudLPWAN::sendPropertiesToCloud()
150150
int bytes_encoded = 0;
151151
uint8_t data[CBOR_LORA_MSG_MAX_SIZE];
152152

153-
if (CBOREncoder::encode(_property_container, data, sizeof(data), bytes_encoded, true) == CborNoError)
153+
if (CBOREncoder::encode(_property_container, data, sizeof(data), bytes_encoded, _last_checked_property_index, true) == CborNoError)
154154
if (bytes_encoded > 0)
155155
writeProperties(data, bytes_encoded);
156156
}

‎src/ArduinoIoTCloudTCP.cpp

+5-4
Original file line numberDiff line numberDiff line change
@@ -602,12 +602,12 @@ void ArduinoIoTCloudTCP::handleMessage(int length)
602602
}
603603
}
604604

605-
void ArduinoIoTCloudTCP::sendPropertyContainerToCloud(PropertyContainer & property_container)
605+
void ArduinoIoTCloudTCP::sendPropertyContainerToCloud(PropertyContainer & property_container, unsigned int & current_property_index)
606606
{
607607
int bytes_encoded = 0;
608608
uint8_t data[MQTT_TRANSMIT_BUFFER_SIZE];
609609

610-
if (CBOREncoder::encode(property_container, data, sizeof(data), bytes_encoded, false) == CborNoError)
610+
if (CBOREncoder::encode(property_container, data, sizeof(data), bytes_encoded, current_property_index, false) == CborNoError)
611611
if (bytes_encoded > 0)
612612
{
613613
/* If properties have been encoded store them in the back-up buffer
@@ -622,13 +622,14 @@ void ArduinoIoTCloudTCP::sendPropertyContainerToCloud(PropertyContainer & proper
622622

623623
void ArduinoIoTCloudTCP::sendPropertiesToCloud()
624624
{
625-
sendPropertyContainerToCloud(_property_container);
625+
sendPropertyContainerToCloud(_property_container, _last_checked_property_index);
626626
}
627627

628628
#if OTA_ENABLED
629629
void ArduinoIoTCloudTCP::sendOTAPropertiesToCloud()
630630
{
631631
PropertyContainer ota_property_container;
632+
unsigned int last_ota_property_index = 0;
632633

633634
std::list<String> const ota_property_list {"OTA_CAP", "OTA_ERROR", "OTA_SHA256", "OTA_URL", "OTA_REQ"};
634635
std::for_each(ota_property_list.cbegin(),
@@ -641,7 +642,7 @@ void ArduinoIoTCloudTCP::sendOTAPropertiesToCloud()
641642
}
642643
);
643644

644-
sendPropertyContainerToCloud(ota_property_container);
645+
sendPropertyContainerToCloud(ota_property_container, last_ota_property_index);
645646
}
646647
#endif
647648

‎src/ArduinoIoTCloudTCP.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ class ArduinoIoTCloudTCP: public ArduinoIoTCloudClass
166166

167167
static void onMessage(int length);
168168
void handleMessage(int length);
169-
void sendPropertyContainerToCloud(PropertyContainer & property_container);
169+
void sendPropertyContainerToCloud(PropertyContainer & property_container, unsigned int & current_property_index);
170170
void sendPropertiesToCloud();
171171
void requestLastValue();
172172
int write(String const topic, byte const data[], int const length);

‎src/cbor/CBOREncoder.cpp

+164-30
Original file line numberDiff line numberDiff line change
@@ -24,50 +24,184 @@
2424
#undef max
2525
#undef min
2626
#include <algorithm>
27+
#include <iterator>
2728

2829
#include "lib/tinycbor/cbor-lib.h"
2930

3031
/******************************************************************************
3132
* PUBLIC MEMBER FUNCTIONS
3233
******************************************************************************/
3334

34-
CborError CBOREncoder::encode(PropertyContainer & property_container, uint8_t * data, size_t const size, int & bytes_encoded, bool lightPayload)
35+
CborError CBOREncoder::encode(PropertyContainer & property_container, uint8_t * data, size_t const size, int & bytes_encoded, unsigned int & current_property_index, bool lightPayload)
3536
{
36-
CborEncoder encoder, arrayEncoder;
37+
EncoderState current_state = EncoderState::InitPropertyEncoder,
38+
next_state = EncoderState::InitPropertyEncoder;
3739

38-
cbor_encoder_init(&encoder, data, size, 0);
40+
PropertyContainerEncoder propertyEncoder(property_container, current_property_index);
3941

40-
CHECK_CBOR(cbor_encoder_create_array(&encoder, &arrayEncoder, CborIndefiniteLength));
42+
while (current_state != EncoderState::SendMessage) {
4143

42-
/* Check if backing storage and cloud has diverged
43-
* time interval may be elapsed or property may be changed
44+
switch (current_state) {
45+
case EncoderState::InitPropertyEncoder : next_state = handle_InitPropertyEncoder(propertyEncoder); break;
46+
case EncoderState::OpenCBORContainer : next_state = handle_OpenCBORContainer(propertyEncoder, data, size); break;
47+
case EncoderState::TryAppend : next_state = handle_TryAppend(propertyEncoder, lightPayload); break;
48+
case EncoderState::OutOfMemory : next_state = handle_OutOfMemory(propertyEncoder); break;
49+
case EncoderState::SkipProperty : next_state = handle_SkipProperty(propertyEncoder); break;
50+
case EncoderState::TrimAppend : next_state = handle_TrimAppend(propertyEncoder); break;
51+
case EncoderState::CloseCBORContainer : next_state = handle_CloseCBORContainer(propertyEncoder); break;
52+
case EncoderState::TrimClose : next_state = handle_TrimClose(propertyEncoder); break;
53+
case EncoderState::FinishAppend : next_state = handle_FinishAppend(propertyEncoder); break;
54+
case EncoderState::SendMessage : /* Nothing to do */ break;
55+
case EncoderState::Error : return CborErrorInternalError; break;
56+
}
57+
58+
current_state = next_state;
59+
}
60+
61+
if (propertyEncoder.encoded_property_count > 0)
62+
bytes_encoded = cbor_encoder_get_buffer_size(&propertyEncoder.encoder, data);
63+
else
64+
bytes_encoded = 0;
65+
66+
return CborNoError;
67+
}
68+
69+
/******************************************************************************
70+
PRIVATE MEMBER FUNCTIONS
71+
******************************************************************************/
72+
73+
CBOREncoder::EncoderState CBOREncoder::handle_InitPropertyEncoder(PropertyContainerEncoder & propertyEncoder)
74+
{
75+
propertyEncoder.encoded_property_count = 0;
76+
propertyEncoder.checked_property_count = 0;
77+
propertyEncoder.encoded_property_limit = 0;
78+
propertyEncoder.property_limit_active = false;
79+
return EncoderState::OpenCBORContainer;
80+
}
81+
82+
CBOREncoder::EncoderState CBOREncoder::handle_OpenCBORContainer(PropertyContainerEncoder & propertyEncoder, uint8_t * data, size_t const size)
83+
{
84+
propertyEncoder.encoded_property_count = 0;
85+
propertyEncoder.checked_property_count = 0;
86+
cbor_encoder_init(&propertyEncoder.encoder, data, size, 0);
87+
cbor_encoder_create_array(&propertyEncoder.encoder, &propertyEncoder.arrayEncoder, CborIndefiniteLength);
88+
return EncoderState::TryAppend;
89+
}
90+
91+
CBOREncoder::EncoderState CBOREncoder::handle_TryAppend(PropertyContainerEncoder & propertyEncoder, bool & lightPayload)
92+
{
93+
/* Check if backing storage and cloud has diverged. Time interval may be elapsed or property may be changed
4494
* and if that's the case encode the property into the CBOR.
4595
*/
4696
CborError error = CborNoError;
47-
int num_encoded_properties = 0;
48-
std::for_each(property_container.begin(),
49-
property_container.end(),
50-
[lightPayload, &arrayEncoder, &error, &num_encoded_properties](Property * p)
51-
{
52-
if (p->shouldBeUpdated() && p->isReadableByCloud())
53-
{
54-
error = p->append(&arrayEncoder, lightPayload);
55-
if(error == CborNoError)
56-
num_encoded_properties++;
57-
else
58-
return;
59-
}
60-
});
61-
if ((CborNoError != error) &&
62-
(CborErrorOutOfMemory != error))
63-
return error;
64-
65-
CHECK_CBOR(cbor_encoder_close_container(&encoder, &arrayEncoder));
66-
67-
if (num_encoded_properties > 0)
68-
bytes_encoded = cbor_encoder_get_buffer_size(&encoder, data);
97+
PropertyContainer::iterator iter = propertyEncoder.property_container.begin();
98+
std::advance(iter, propertyEncoder.current_property_index);
99+
100+
for(; iter != propertyEncoder.property_container.end(); iter++)
101+
{
102+
Property * p = * iter;
103+
104+
if (p->shouldBeUpdated() && p->isReadableByCloud())
105+
{
106+
error = p->append(&propertyEncoder.arrayEncoder, lightPayload);
107+
if(error == CborNoError)
108+
propertyEncoder.encoded_property_count++;
109+
}
110+
if(error == CborNoError)
111+
propertyEncoder.checked_property_count++;
112+
113+
bool const maximum_number_of_properties_reached = (propertyEncoder.encoded_property_count >= propertyEncoder.encoded_property_limit) && (propertyEncoder.property_limit_active == true);
114+
bool const cbor_encoder_error = (error != CborNoError);
115+
116+
if (maximum_number_of_properties_reached || cbor_encoder_error)
117+
break;
118+
}
119+
120+
if (CborErrorOutOfMemory == error)
121+
return EncoderState::OutOfMemory;
122+
else if (CborNoError == error)
123+
return EncoderState::CloseCBORContainer;
124+
else if (CborErrorSplitItems == error)
125+
return EncoderState::TrimAppend;
69126
else
70-
bytes_encoded = 0;
127+
return EncoderState::Error;
128+
}
71129

72-
return CborNoError;
130+
CBOREncoder::EncoderState CBOREncoder::handle_OutOfMemory(PropertyContainerEncoder & propertyEncoder)
131+
{
132+
if(propertyEncoder.encoded_property_count > 0)
133+
return EncoderState::CloseCBORContainer;
134+
else
135+
return EncoderState::SkipProperty;
136+
}
137+
138+
CBOREncoder::EncoderState CBOREncoder::handle_SkipProperty(PropertyContainerEncoder & propertyEncoder)
139+
{
140+
/* Better to skip this property otherwise we will stay blocked here. This happens only with a message property
141+
* that not fits into the CBOR buffer
142+
*/
143+
propertyEncoder.current_property_index++;
144+
if(propertyEncoder.current_property_index >= propertyEncoder.property_container.size())
145+
propertyEncoder.current_property_index = 0;
146+
return EncoderState::Error;
147+
}
148+
149+
CBOREncoder::EncoderState CBOREncoder::handle_TrimAppend(PropertyContainerEncoder & propertyEncoder)
150+
{
151+
/* Trim the number of properties to be included in the next message to avoid multivalue property split */
152+
propertyEncoder.encoded_property_limit = propertyEncoder.encoded_property_count;
153+
propertyEncoder.property_limit_active = true;
154+
if(propertyEncoder.encoded_property_limit > 0)
155+
return EncoderState::OpenCBORContainer;
156+
else
157+
return EncoderState::Error;
158+
}
159+
160+
CBOREncoder::EncoderState CBOREncoder::handle_CloseCBORContainer(PropertyContainerEncoder & propertyEncoder)
161+
{
162+
CborError error = cbor_encoder_close_container(&propertyEncoder.encoder, &propertyEncoder.arrayEncoder);
163+
if (CborNoError != error)
164+
return EncoderState::TrimClose;
165+
else
166+
return EncoderState::FinishAppend;
167+
}
168+
169+
CBOREncoder::EncoderState CBOREncoder::handle_TrimClose(PropertyContainerEncoder & propertyEncoder)
170+
{
171+
/* Trim the number of properties to be included in the next message to avoid error closing container */
172+
propertyEncoder.encoded_property_limit = propertyEncoder.encoded_property_count - 1;
173+
propertyEncoder.property_limit_active = true;
174+
if(propertyEncoder.encoded_property_limit > 0)
175+
return EncoderState::OpenCBORContainer;
176+
else
177+
return EncoderState::Error;
178+
}
179+
180+
CBOREncoder::EncoderState CBOREncoder::handle_FinishAppend(PropertyContainerEncoder & propertyEncoder)
181+
{
182+
/* Restore property message limit to CBOR_ENCODER_NO_PROPERTIES_LIMIT */
183+
propertyEncoder.property_limit_active = false;
184+
185+
/* 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 */
186+
PropertyContainer::iterator iter = propertyEncoder.property_container.begin();
187+
std::advance(iter, propertyEncoder.current_property_index);
188+
int num_appended_properties = 0;
189+
190+
for(; iter != propertyEncoder.property_container.end(); iter++)
191+
{
192+
Property * p = * iter;
193+
if (num_appended_properties >= propertyEncoder.checked_property_count)
194+
break;
195+
196+
p->appendCompleted();
197+
num_appended_properties++;
198+
}
199+
200+
/* Advance property index for the nex message */
201+
propertyEncoder.current_property_index += propertyEncoder.checked_property_count;
202+
203+
if(propertyEncoder.current_property_index >= propertyEncoder.property_container.size())
204+
propertyEncoder.current_property_index = 0;
205+
206+
return EncoderState::SendMessage;
73207
}

‎src/cbor/CBOREncoder.h

+40-2
Original file line numberDiff line numberDiff line change
@@ -42,16 +42,54 @@ class CBOREncoder
4242
{
4343

4444
public:
45-
4645
/* encode return > 0 if a property has changed and encodes the changed properties in CBOR format into the provided buffer */
4746
/* 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*/
48-
static CborError encode(PropertyContainer & property_container, uint8_t * data, size_t const size, int & bytes_encoded, bool lightPayload = false);
47+
static CborError encode(PropertyContainer & property_container, uint8_t * data, size_t const size, int & bytes_encoded, unsigned int & current_property_index, bool lightPayload = false);
4948

5049
private:
5150

5251
CBOREncoder() { }
5352
CBOREncoder(CborEncoder const &) { }
5453

54+
enum class EncoderState
55+
{
56+
InitPropertyEncoder,
57+
OpenCBORContainer,
58+
TryAppend,
59+
OutOfMemory,
60+
SkipProperty,
61+
TrimAppend,
62+
CloseCBORContainer,
63+
TrimClose,
64+
FinishAppend,
65+
SendMessage,
66+
Error
67+
};
68+
69+
struct PropertyContainerEncoder
70+
{
71+
PropertyContainerEncoder(PropertyContainer & _property_container, unsigned int & _current_property_index): property_container(_property_container), current_property_index(_current_property_index) { }
72+
PropertyContainer & property_container;
73+
unsigned int & current_property_index;
74+
int encoded_property_count;
75+
int checked_property_count;
76+
int encoded_property_limit;
77+
bool property_limit_active;
78+
CborEncoder encoder;
79+
CborEncoder arrayEncoder;
80+
};
81+
82+
static EncoderState handle_InitPropertyEncoder(PropertyContainerEncoder & propertyEncoder);
83+
static EncoderState handle_OpenCBORContainer(PropertyContainerEncoder & propertyEncoder, uint8_t * data, size_t const size);
84+
static EncoderState handle_TryAppend(PropertyContainerEncoder & propertyEncoder, bool & lightPayload);
85+
static EncoderState handle_OutOfMemory(PropertyContainerEncoder & propertyEncoder);
86+
static EncoderState handle_SkipProperty(PropertyContainerEncoder & propertyEncoder);
87+
static EncoderState handle_TrimAppend(PropertyContainerEncoder & propertyEncoder);
88+
static EncoderState handle_CloseCBORContainer(PropertyContainerEncoder & propertyEncoder);
89+
static EncoderState handle_TrimClose(PropertyContainerEncoder & propertyEncoder);
90+
static EncoderState handle_FinishAppend(PropertyContainerEncoder & propertyEncoder);
91+
static EncoderState handle_AdvancePropertyContainer(PropertyContainerEncoder & propertyEncoder);
92+
5593
};
5694

5795
#endif /* ARDUINO_CBOR_CBOR_ENCODER_H_ */

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

+12
Original file line numberDiff line numberDiff line change
@@ -21,4 +21,16 @@
2121
} while(0);
2222
#endif /* CHECK_CBOR */
2323

24+
#ifndef CHECK_CBOR_MULTI
25+
#define CHECK_CBOR_MULTI(expr) \
26+
do { \
27+
CborError error = CborNoError; \
28+
error = (expr); \
29+
if (CborErrorOutOfMemory == error) \
30+
return CborErrorSplitItems; \
31+
if (CborNoError != error) \
32+
return error; \
33+
} while(0);
34+
#endif /* CHECK_CBOR_MULTI */
35+
2436
#endif

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

+1
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ typedef enum CborError {
189189
/* encoder errors */
190190
CborErrorTooManyItems = 768,
191191
CborErrorTooFewItems,
192+
CborErrorSplitItems,
192193

193194
/* internal implementation errors */
194195
CborErrorDataTooLarge = 1024,

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

+3
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,9 @@ const char *cbor_error_string(CborError error)
163163
case CborErrorTooFewItems:
164164
return _("too few items added to encoder");
165165

166+
case CborErrorSplitItems:
167+
return _("splitted item added to encoder");
168+
166169
case CborErrorDataTooLarge:
167170
return _("internal error: data too large");
168171

0 commit comments

Comments
 (0)