Skip to content

Commit 9166399

Browse files
authored
Merge pull request #500 from pennam/fix-roll
TimedAttempt fixes and tests
2 parents 12e9def + 08092f6 commit 9166399

File tree

6 files changed

+207
-7
lines changed

6 files changed

+207
-7
lines changed

extras/test/CMakeLists.txt

+3-1
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ set(TEST_SRCS
4545
src/test_writeOnly.cpp
4646
src/test_writeOnDemand.cpp
4747
src/test_writeOnChange.cpp
48+
src/test_TimedAttempt.cpp
4849
)
4950

5051
set(TEST_UTIL_SRCS
@@ -53,6 +54,7 @@ set(TEST_UTIL_SRCS
5354
)
5455

5556
set(TEST_DUT_SRCS
57+
../../src/utility/time/TimedAttempt.cpp
5658
../../src/property/Property.cpp
5759
../../src/property/PropertyContainer.cpp
5860
../../src/cbor/CBORDecoder.cpp
@@ -84,7 +86,7 @@ set(TEST_TARGET_SRCS
8486

8587
##########################################################################
8688

87-
add_compile_definitions(HOST)
89+
add_compile_definitions(HOST HAS_TCP)
8890
add_compile_options(-Wall -Wextra -Wpedantic -Werror)
8991
add_compile_options(-Wno-cast-function-type)
9092

extras/test/include/Arduino.h

+7
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,13 @@
1111

1212
#include <string>
1313

14+
/******************************************************************************
15+
DEFINES
16+
******************************************************************************/
17+
#ifndef min
18+
#define min(a,b) ((a)<(b)?(a):(b))
19+
#endif
20+
1421
/******************************************************************************
1522
TYPEDEF
1623
******************************************************************************/

extras/test/src/test_TimedAttempt.cpp

+186
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
/*
2+
Copyright (c) 2024 Arduino. All rights reserved.
3+
*/
4+
5+
/******************************************************************************
6+
INCLUDE
7+
******************************************************************************/
8+
9+
#include <catch.hpp>
10+
11+
#include <TimedAttempt.h>
12+
#include <AIoTC_Config.h>
13+
#include <Arduino.h>
14+
#include <limits.h>
15+
16+
/******************************************************************************
17+
TEST CODE
18+
******************************************************************************/
19+
20+
SCENARIO("Test broker connection retries")
21+
{
22+
TimedAttempt _connection_attempt(0,0);
23+
24+
_connection_attempt.begin(AIOT_CONFIG_RECONNECTION_RETRY_DELAY_ms,
25+
AIOT_CONFIG_MAX_RECONNECTION_RETRY_DELAY_ms);
26+
27+
/* 100000 retries are more or less 37 days without connection */
28+
while(_connection_attempt.getRetryCount() < 100000) {
29+
_connection_attempt.retry();
30+
31+
switch(_connection_attempt.getRetryCount()) {
32+
case 1:
33+
REQUIRE(_connection_attempt.getWaitTime() == 2000);
34+
break;
35+
case 2:
36+
REQUIRE(_connection_attempt.getWaitTime() == 4000);
37+
break;
38+
case 3:
39+
REQUIRE(_connection_attempt.getWaitTime() == 8000);
40+
break;
41+
case 4:
42+
REQUIRE(_connection_attempt.getWaitTime() == 16000);
43+
break;
44+
default:
45+
REQUIRE(_connection_attempt.getWaitTime() == 32000);
46+
break;
47+
}
48+
}
49+
}
50+
51+
SCENARIO("Test thing id request with no answer from the cloud")
52+
{
53+
TimedAttempt _attachAttempt(0,0);
54+
55+
_attachAttempt.begin(AIOT_CONFIG_THING_ID_REQUEST_RETRY_DELAY_ms,
56+
AIOT_CONFIG_MAX_THING_ID_REQUEST_RETRY_DELAY_ms);
57+
58+
/* 100000 retries are more or less 37 days of requests */
59+
while(_attachAttempt.getRetryCount() < 100000) {
60+
_attachAttempt.retry();
61+
62+
switch(_attachAttempt.getRetryCount()) {
63+
case 1:
64+
REQUIRE(_attachAttempt.getWaitTime() == 4000);
65+
break;
66+
case 2:
67+
REQUIRE(_attachAttempt.getWaitTime() == 8000);
68+
break;
69+
case 3:
70+
REQUIRE(_attachAttempt.getWaitTime() == 16000);
71+
break;
72+
default:
73+
REQUIRE(_attachAttempt.getWaitTime() == 32000);
74+
break;
75+
}
76+
}
77+
}
78+
79+
SCENARIO("Test thing id request of a detached device")
80+
{
81+
TimedAttempt _attachAttempt(0,0);
82+
83+
_attachAttempt.begin(AIOT_CONFIG_THING_ID_REQUEST_RETRY_DELAY_ms,
84+
AIOT_CONFIG_MAX_THING_ID_REQUEST_RETRY_DELAY_ms);
85+
86+
while(_attachAttempt.getRetryCount() < 100000) {
87+
_attachAttempt.retry();
88+
89+
switch(_attachAttempt.getRetryCount()) {
90+
case 1:
91+
REQUIRE(_attachAttempt.getWaitTime() == 4000);
92+
_attachAttempt.reconfigure(AIOT_CONFIG_THING_ID_REQUEST_RETRY_DELAY_ms *
93+
AIOT_CONFIG_DEVICE_REGISTERED_RETRY_DELAY_k,
94+
AIOT_CONFIG_MAX_THING_ID_REQUEST_RETRY_DELAY_ms *
95+
AIOT_CONFIG_MAX_DEVICE_REGISTERED_RETRY_DELAY_k);
96+
break;
97+
case 2:
98+
REQUIRE(_attachAttempt.getWaitTime() == 80000);
99+
break;
100+
case 3:
101+
REQUIRE(_attachAttempt.getWaitTime() == 160000);
102+
break;
103+
case 4:
104+
REQUIRE(_attachAttempt.getWaitTime() == 320000);
105+
break;
106+
case 5:
107+
REQUIRE(_attachAttempt.getWaitTime() == 640000);
108+
break;
109+
default:
110+
REQUIRE(_attachAttempt.getWaitTime() == 1280000);
111+
break;
112+
}
113+
}
114+
}
115+
116+
SCENARIO("Test last value request")
117+
{
118+
TimedAttempt _syncAttempt(0,0);
119+
120+
_syncAttempt.begin(AIOT_CONFIG_TIMEOUT_FOR_LASTVALUES_SYNC_ms);
121+
122+
/* 100000 retries are more or less 37 days of requests */
123+
while(_syncAttempt.getRetryCount() < 100000) {
124+
_syncAttempt.retry();
125+
126+
switch(_syncAttempt.getRetryCount()) {
127+
default:
128+
REQUIRE(_syncAttempt.getWaitTime() == 30000);
129+
break;
130+
}
131+
}
132+
}
133+
134+
SCENARIO("Test isExpired() and isRetry()")
135+
{
136+
TimedAttempt attempt(0,0);
137+
138+
attempt.begin(AIOT_CONFIG_RECONNECTION_RETRY_DELAY_ms,
139+
AIOT_CONFIG_MAX_RECONNECTION_RETRY_DELAY_ms);
140+
141+
/* Initial condition */
142+
set_millis(0);
143+
REQUIRE(attempt.isExpired() == false);
144+
REQUIRE(attempt.isRetry() == false);
145+
146+
/* Normal retry 2000ms */
147+
attempt.retry();
148+
REQUIRE(attempt.isRetry() == true);
149+
set_millis(1000);
150+
REQUIRE(attempt.isExpired() == false);
151+
set_millis(1999);
152+
REQUIRE(attempt.isExpired() == false);
153+
set_millis(2000);
154+
REQUIRE(attempt.isExpired() == false);
155+
set_millis(2001);
156+
REQUIRE(attempt.isExpired() == true);
157+
158+
/* Retry with rollover 4000ms */
159+
set_millis(ULONG_MAX - 1999);
160+
attempt.retry();
161+
REQUIRE(attempt.isRetry() == true);
162+
set_millis(0);
163+
REQUIRE(attempt.isExpired() == false);
164+
set_millis(1999);
165+
REQUIRE(attempt.isExpired() == false);
166+
set_millis(2000);
167+
REQUIRE(attempt.isExpired() == false);
168+
set_millis(2001);
169+
REQUIRE(attempt.isExpired() == true);
170+
171+
/* Normal retry 8000ms */
172+
set_millis(4000);
173+
attempt.retry();
174+
REQUIRE(attempt.isRetry() == true);
175+
set_millis(4000);
176+
REQUIRE(attempt.isExpired() == false);
177+
set_millis(11999);
178+
REQUIRE(attempt.isExpired() == false);
179+
set_millis(12000);
180+
REQUIRE(attempt.isExpired() == false);
181+
set_millis(12001);
182+
REQUIRE(attempt.isExpired() == true);
183+
184+
attempt.reset();
185+
REQUIRE(attempt.isRetry() == false);
186+
}

src/AIoTC_Config.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@
155155
#define AIOT_CONFIG_THING_ID_REQUEST_MAX_RETRY_CNT (10UL)
156156

157157
#define AIOT_CONFIG_DEVICE_REGISTERED_RETRY_DELAY_k (10UL)
158-
#define AIOT_CONFIG_MAX_DEVICE_REGISTERED_RETRY_DELAY_k (4UL)
158+
#define AIOT_CONFIG_MAX_DEVICE_REGISTERED_RETRY_DELAY_k (40UL)
159159

160160
#define AIOT_CONFIG_TIMEOUT_FOR_LASTVALUES_SYNC_ms (30000UL)
161161
#define AIOT_CONFIG_LASTVALUES_SYNC_MAX_RETRY_CNT (10UL)

src/utility/time/TimedAttempt.cpp

+9-4
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,16 @@ TimedAttempt::TimedAttempt(unsigned long minDelay, unsigned long maxDelay)
3030

3131
void TimedAttempt::begin(unsigned long delay) {
3232
_retryCount = 0;
33+
_retryDelay = 0;
34+
_retryTick = 0;
3335
_minDelay = delay;
3436
_maxDelay = delay;
3537
}
3638

3739
void TimedAttempt::begin(unsigned long minDelay, unsigned long maxDelay) {
3840
_retryCount = 0;
41+
_retryDelay = 0;
42+
_retryTick = 0;
3943
_minDelay = minDelay;
4044
_maxDelay = maxDelay;
4145
}
@@ -52,9 +56,10 @@ unsigned long TimedAttempt::retry() {
5256
}
5357

5458
unsigned long TimedAttempt::reload() {
55-
unsigned long retryDelay = (1 << _retryCount) * _minDelay;
56-
_retryDelay = min(retryDelay, _maxDelay);
57-
_nextRetryTick = millis() + _retryDelay;
59+
unsigned long shift = _retryCount > 31 ? 31 : _retryCount;
60+
unsigned long delay = (1UL << shift) * _minDelay;
61+
_retryDelay = min(delay, _maxDelay);
62+
_retryTick = millis();
5863
return _retryDelay;
5964
}
6065

@@ -67,7 +72,7 @@ bool TimedAttempt::isRetry() {
6772
}
6873

6974
bool TimedAttempt::isExpired() {
70-
return millis() > _nextRetryTick;
75+
return millis() - _retryTick > _retryDelay;
7176
}
7277

7378
unsigned int TimedAttempt::getRetryCount() {

src/utility/time/TimedAttempt.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class TimedAttempt {
3434
private:
3535
unsigned long _minDelay;
3636
unsigned long _maxDelay;
37-
unsigned long _nextRetryTick;
37+
unsigned long _retryTick;
3838
unsigned long _retryDelay;
3939
unsigned int _retryCount;
4040
};

0 commit comments

Comments
 (0)