From d8254d08500ed74625e257b7d7b60cf2f9401a08 Mon Sep 17 00:00:00 2001 From: pennam Date: Tue, 3 Oct 2023 08:58:30 +0200 Subject: [PATCH 01/17] Add missing debug print --- src/ArduinoIoTCloudTCP.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ArduinoIoTCloudTCP.cpp b/src/ArduinoIoTCloudTCP.cpp index 87b1c555e..5c89d2280 100644 --- a/src/ArduinoIoTCloudTCP.cpp +++ b/src/ArduinoIoTCloudTCP.cpp @@ -642,6 +642,7 @@ void ArduinoIoTCloudTCP::handle_OTARequest() { ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_Disconnect() { DEBUG_ERROR("ArduinoIoTCloudTCP::%s MQTT client connection lost", __FUNCTION__); + DEBUG_INFO("Disconnected from Arduino IoT Cloud"); _mqttClient.stop(); execCloudEventCallback(ArduinoIoTCloudEvent::DISCONNECT); return State::ConnectPhy; From 16ad787993e43acc3f808d98fdfd4f9f7e6ea3d5 Mon Sep 17 00:00:00 2001 From: pennam Date: Tue, 3 Oct 2023 17:50:32 +0200 Subject: [PATCH 02/17] Simplify Connect/Disconnect Attach/Detach logic --- src/ArduinoIoTCloudTCP.cpp | 40 +++++++++++++------------------------- src/ArduinoIoTCloudTCP.h | 2 -- 2 files changed, 13 insertions(+), 29 deletions(-) diff --git a/src/ArduinoIoTCloudTCP.cpp b/src/ArduinoIoTCloudTCP.cpp index 5c89d2280..0a19a3a89 100644 --- a/src/ArduinoIoTCloudTCP.cpp +++ b/src/ArduinoIoTCloudTCP.cpp @@ -87,7 +87,6 @@ ArduinoIoTCloudTCP::ArduinoIoTCloudTCP() , _shadowTopicIn("") , _dataTopicOut("") , _dataTopicIn("") -, _deviceSubscribedToThing{false} #if OTA_ENABLED , _ota_cap{false} , _ota_error{static_cast(OTAError::None)} @@ -376,16 +375,13 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeDeviceTopic() if (!_mqttClient.subscribe(_deviceTopicIn)) { DEBUG_ERROR("ArduinoIoTCloudTCP::%s could not subscribe to %s", __FUNCTION__, _deviceTopicIn.c_str()); - return State::SubscribeDeviceTopic; } if (_last_device_subscribe_cnt > AIOT_CONFIG_LASTVALUES_SYNC_MAX_RETRY_CNT) { _last_device_subscribe_cnt = 0; _next_device_subscribe_attempt_tick = 0; - _mqttClient.stop(); - execCloudEventCallback(ArduinoIoTCloudEvent::DISCONNECT); - return State::ConnectPhy; + return State::Disconnect; } /* No device configuration reply. Wait: 5s -> 10s -> 20s -> 30s */ @@ -428,16 +424,6 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_CheckDeviceConfig() return State::Disconnect; } - if(_deviceSubscribedToThing == true) - { - /* Unsubscribe from old things topics and go on with a new subscription */ - _mqttClient.unsubscribe(_shadowTopicIn); - _mqttClient.unsubscribe(_dataTopicIn); - _deviceSubscribedToThing = false; - DEBUG_INFO("Disconnected from Arduino IoT Cloud"); - execCloudEventCallback(ArduinoIoTCloudEvent::DISCONNECT); - } - updateThingTopics(); if (_thing_id.length() == 0) @@ -465,7 +451,7 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeThingTopics() if (_thing_id_property->isDifferentFromCloud()) { - return State::CheckDeviceConfig; + return State::Disconnect; } unsigned long const now = millis(); @@ -481,9 +467,7 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeThingTopics() { _last_subscribe_request_cnt = 0; _last_subscribe_request_tick = 0; - _mqttClient.stop(); - execCloudEventCallback(ArduinoIoTCloudEvent::DISCONNECT); - return State::ConnectPhy; + return State::Disconnect; } _last_subscribe_request_tick = now; @@ -506,7 +490,6 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeThingTopics() DEBUG_INFO("Connected to Arduino IoT Cloud"); DEBUG_INFO("Thing ID: %s", getThingId().c_str()); execCloudEventCallback(ArduinoIoTCloudEvent::CONNECT); - _deviceSubscribedToThing = true; /*Add retry wait time otherwise we are trying to reconnect every 250 ms...*/ return State::RequestLastValues; @@ -521,7 +504,7 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_RequestLastValues() if (_thing_id_property->isDifferentFromCloud()) { - return State::CheckDeviceConfig; + return State::Disconnect; } /* Check whether or not we need to send a new request. */ @@ -542,9 +525,7 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_RequestLastValues() { _last_sync_request_cnt = 0; _last_sync_request_tick = 0; - _mqttClient.stop(); - execCloudEventCallback(ArduinoIoTCloudEvent::DISCONNECT); - return State::ConnectPhy; + return State::Disconnect; } } @@ -564,7 +545,7 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_Connected() { if (_thing_id_property->isDifferentFromCloud()) { - return State::CheckDeviceConfig; + return State::Disconnect; } /* Check if a primitive property wrapper is locally changed. @@ -641,9 +622,14 @@ void ArduinoIoTCloudTCP::handle_OTARequest() { ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_Disconnect() { - DEBUG_ERROR("ArduinoIoTCloudTCP::%s MQTT client connection lost", __FUNCTION__); + if (!_mqttClient.connected()) { + DEBUG_ERROR("ArduinoIoTCloudTCP::%s MQTT client connection lost", __FUNCTION__); + } else { + _mqttClient.unsubscribe(_shadowTopicIn); + _mqttClient.unsubscribe(_dataTopicIn); + _mqttClient.stop(); + } DEBUG_INFO("Disconnected from Arduino IoT Cloud"); - _mqttClient.stop(); execCloudEventCallback(ArduinoIoTCloudEvent::DISCONNECT); return State::ConnectPhy; } diff --git a/src/ArduinoIoTCloudTCP.h b/src/ArduinoIoTCloudTCP.h index d00bb0ca6..0be0bfcf0 100644 --- a/src/ArduinoIoTCloudTCP.h +++ b/src/ArduinoIoTCloudTCP.h @@ -179,8 +179,6 @@ class ArduinoIoTCloudTCP: public ArduinoIoTCloudClass String _dataTopicOut; String _dataTopicIn; - bool _deviceSubscribedToThing; - #if OTA_ENABLED bool _ota_cap; int _ota_error; From 68e3d97202e8c22931adf1b8f3b66f5cec2f076c Mon Sep 17 00:00:00 2001 From: pennam Date: Wed, 4 Oct 2023 15:09:51 +0200 Subject: [PATCH 03/17] Simplify Attach/Detach timeout logic --- src/AIoTC_Config.h | 3 ++- src/ArduinoIoTCloudTCP.cpp | 32 ++++++++++++++++---------------- src/ArduinoIoTCloudTCP.h | 1 - 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/AIoTC_Config.h b/src/AIoTC_Config.h index e47a8ac2b..b9301934f 100644 --- a/src/AIoTC_Config.h +++ b/src/AIoTC_Config.h @@ -139,10 +139,11 @@ #define AIOT_CONFIG_RECONNECTION_RETRY_DELAY_ms (1000UL) #define AIOT_CONFIG_MAX_RECONNECTION_RETRY_DELAY_ms (32000UL) -#define AIOT_CONFIG_DEVICE_TOPIC_SUBSCRIBE_RETRY_DELAY_ms (5*1000UL) +#define AIOT_CONFIG_DEVICE_TOPIC_SUBSCRIBE_RETRY_DELAY_ms (2000UL) #define AIOT_CONFIG_MAX_DEVICE_TOPIC_SUBSCRIBE_RETRY_DELAY_ms (32000UL) #define AIOT_CONFIG_THING_TOPICS_SUBSCRIBE_RETRY_DELAY_ms (1000UL) #define AIOT_CONFIG_THING_TOPICS_SUBSCRIBE_MAX_RETRY_CNT (10UL) +#define AIOT_CONFIG_DEVICE_TOPIC_ATTACH_RETRY_DELAY_ms (20000UL) #define AIOT_CONFIG_MAX_DEVICE_TOPIC_ATTACH_RETRY_DELAY_ms (1280000UL) #define AIOT_CONFIG_TIMEOUT_FOR_LASTVALUES_SYNC_ms (30000UL) #define AIOT_CONFIG_LASTVALUES_SYNC_MAX_RETRY_CNT (10UL) diff --git a/src/ArduinoIoTCloudTCP.cpp b/src/ArduinoIoTCloudTCP.cpp index 0a19a3a89..d18b34e09 100644 --- a/src/ArduinoIoTCloudTCP.cpp +++ b/src/ArduinoIoTCloudTCP.cpp @@ -362,7 +362,7 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SendDeviceProperties() } sendDevicePropertiesToCloud(); - return State::SubscribeDeviceTopic; + return State::WaitDeviceConfig; } ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeDeviceTopic() @@ -384,11 +384,10 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeDeviceTopic() return State::Disconnect; } - /* No device configuration reply. Wait: 5s -> 10s -> 20s -> 30s */ + _last_device_subscribe_cnt++; unsigned long subscribe_retry_delay = (1 << _last_device_subscribe_cnt) * AIOT_CONFIG_DEVICE_TOPIC_SUBSCRIBE_RETRY_DELAY_ms; subscribe_retry_delay = min(subscribe_retry_delay, static_cast(AIOT_CONFIG_MAX_DEVICE_TOPIC_SUBSCRIBE_RETRY_DELAY_ms)); _next_device_subscribe_attempt_tick = millis() + subscribe_retry_delay; - _last_device_subscribe_cnt++; return State::WaitDeviceConfig; } @@ -400,20 +399,19 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_WaitDeviceConfig() return State::Disconnect; } - if (_thing_id_property->isDifferentFromCloud()) - { - return State::CheckDeviceConfig; - } - - if (millis() > _next_device_subscribe_attempt_tick) + bool const is_retry_attempt = (_last_device_subscribe_cnt > 0); + if (is_retry_attempt && (millis() > _next_device_subscribe_attempt_tick)) { /* Configuration not received or device not attached to a valid thing. Try to resubscribe */ if (_mqttClient.unsubscribe(_deviceTopicIn)) { DEBUG_ERROR("ArduinoIoTCloudTCP::%s device waiting for valid thing_id", __FUNCTION__); - return State::SubscribeDeviceTopic; } } + + if (!is_retry_attempt || (is_retry_attempt && (millis() > _next_device_subscribe_attempt_tick))) + return State::SubscribeDeviceTopic; + return State::WaitDeviceConfig; } @@ -428,16 +426,19 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_CheckDeviceConfig() 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; + /* Device configuration received, but invalid thing_id. Do not increase counter, but recompute delay. + * Device not attached. Wait: 40s -> 80s -> 160s -> 320s -> 640s -> 1280s -> 1280s ... + */ + unsigned long attach_retry_delay = (1 << _last_device_subscribe_cnt) * AIOT_CONFIG_DEVICE_TOPIC_ATTACH_RETRY_DELAY_ms; attach_retry_delay = min(attach_retry_delay, static_cast(AIOT_CONFIG_MAX_DEVICE_TOPIC_ATTACH_RETRY_DELAY_ms)); _next_device_subscribe_attempt_tick = millis() + attach_retry_delay; - _last_device_attach_cnt++; return State::WaitDeviceConfig; } DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s Device attached to a new valid Thing %s", __FUNCTION__, getThingId().c_str()); - _last_device_attach_cnt = 0; + + /* Received valid thing_id reset counters and go on */ + _last_device_subscribe_cnt = 0; return State::SubscribeThingTopics; } @@ -652,8 +653,7 @@ void ArduinoIoTCloudTCP::handleMessage(int length) /* Topic for OTA properties and device configuration */ if (_deviceTopicIn == topic) { CBORDecoder::decode(_device_property_container, (uint8_t*)bytes, length); - _last_device_subscribe_cnt = 0; - _next_device_subscribe_attempt_tick = 0; + _state = State::CheckDeviceConfig; } /* Topic for user input data */ diff --git a/src/ArduinoIoTCloudTCP.h b/src/ArduinoIoTCloudTCP.h index 0be0bfcf0..cbe9e6fcc 100644 --- a/src/ArduinoIoTCloudTCP.h +++ b/src/ArduinoIoTCloudTCP.h @@ -132,7 +132,6 @@ class ArduinoIoTCloudTCP: public ArduinoIoTCloudClass unsigned int _last_connection_attempt_cnt; unsigned long _next_device_subscribe_attempt_tick; unsigned int _last_device_subscribe_cnt; - unsigned int _last_device_attach_cnt; unsigned long _last_sync_request_tick; unsigned int _last_sync_request_cnt; unsigned long _last_subscribe_request_tick; From baf69e4581454d3c3f4941097d0e049caf951fdf Mon Sep 17 00:00:00 2001 From: pennam Date: Wed, 4 Oct 2023 15:11:31 +0200 Subject: [PATCH 04/17] Add comments and DEBUG --- src/ArduinoIoTCloudTCP.cpp | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/ArduinoIoTCloudTCP.cpp b/src/ArduinoIoTCloudTCP.cpp index d18b34e09..07d25af3c 100644 --- a/src/ArduinoIoTCloudTCP.cpp +++ b/src/ArduinoIoTCloudTCP.cpp @@ -344,13 +344,15 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_ConnectMqttBroker() return State::SendDeviceProperties; } + /* Can't connect to the broker. Wait: 2s -> 4s -> 8s -> 16s -> 32s -> 32s ... */ _last_connection_attempt_cnt++; unsigned long reconnection_retry_delay = (1 << _last_connection_attempt_cnt) * AIOT_CONFIG_RECONNECTION_RETRY_DELAY_ms; reconnection_retry_delay = min(reconnection_retry_delay, static_cast(AIOT_CONFIG_MAX_RECONNECTION_RETRY_DELAY_ms)); _next_connection_attempt_tick = millis() + reconnection_retry_delay; DEBUG_ERROR("ArduinoIoTCloudTCP::%s could not connect to %s:%d", __FUNCTION__, _brokerAddress.c_str(), _brokerPort); - DEBUG_ERROR("ArduinoIoTCloudTCP::%s %d connection attempt at tick time %d", __FUNCTION__, _last_connection_attempt_cnt, _next_connection_attempt_tick); + DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s %d next connection attempt in %d ms", __FUNCTION__, _last_connection_attempt_cnt, reconnection_retry_delay); + /* Go back to ConnectPhy and retry to get time from network (invalid time for SSL handshake?)*/ return State::ConnectPhy; } @@ -361,6 +363,8 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SendDeviceProperties() return State::Disconnect; } + DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s announce device to the Cloud %d", __FUNCTION__, _time_service.getTime()); + /* TODO check if write fails */ sendDevicePropertiesToCloud(); return State::WaitDeviceConfig; } @@ -372,11 +376,17 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeDeviceTopic() return State::Disconnect; } + DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s request device configuration %d", __FUNCTION__, _time_service.getTime()); + if (!_mqttClient.subscribe(_deviceTopicIn)) { + /* If device_id is wrong the board can't connect to the broker so this condition + * should never happen. + */ DEBUG_ERROR("ArduinoIoTCloudTCP::%s could not subscribe to %s", __FUNCTION__, _deviceTopicIn.c_str()); } + /* Max retry than disconnect */ if (_last_device_subscribe_cnt > AIOT_CONFIG_LASTVALUES_SYNC_MAX_RETRY_CNT) { _last_device_subscribe_cnt = 0; @@ -384,10 +394,12 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeDeviceTopic() return State::Disconnect; } + /* No device configuration received. Wait: 4s -> 8s -> 16s -> 32s -> 32s ...*/ _last_device_subscribe_cnt++; unsigned long subscribe_retry_delay = (1 << _last_device_subscribe_cnt) * AIOT_CONFIG_DEVICE_TOPIC_SUBSCRIBE_RETRY_DELAY_ms; subscribe_retry_delay = min(subscribe_retry_delay, static_cast(AIOT_CONFIG_MAX_DEVICE_TOPIC_SUBSCRIBE_RETRY_DELAY_ms)); _next_device_subscribe_attempt_tick = millis() + subscribe_retry_delay; + DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s %d next configuration request in %d ms", __FUNCTION__, _last_device_subscribe_cnt, subscribe_retry_delay); return State::WaitDeviceConfig; } @@ -405,7 +417,7 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_WaitDeviceConfig() /* Configuration not received or device not attached to a valid thing. Try to resubscribe */ if (_mqttClient.unsubscribe(_deviceTopicIn)) { - DEBUG_ERROR("ArduinoIoTCloudTCP::%s device waiting for valid thing_id", __FUNCTION__); + DEBUG_ERROR("ArduinoIoTCloudTCP::%s device waiting for valid thing_id %d", __FUNCTION__, _time_service.getTime()); } } @@ -432,10 +444,12 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_CheckDeviceConfig() unsigned long attach_retry_delay = (1 << _last_device_subscribe_cnt) * AIOT_CONFIG_DEVICE_TOPIC_ATTACH_RETRY_DELAY_ms; attach_retry_delay = min(attach_retry_delay, static_cast(AIOT_CONFIG_MAX_DEVICE_TOPIC_ATTACH_RETRY_DELAY_ms)); _next_device_subscribe_attempt_tick = millis() + attach_retry_delay; + + DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s device not attached, next configuration request in %d ms", __FUNCTION__, attach_retry_delay); return State::WaitDeviceConfig; } - DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s Device attached to a new valid Thing %s", __FUNCTION__, getThingId().c_str()); + DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s device attached to a new valid thing_id %s %d", __FUNCTION__, getThingId().c_str(), _time_service.getTime()); /* Received valid thing_id reset counters and go on */ _last_device_subscribe_cnt = 0; From 5c9db23034f7748605016084ba2d1e52f8d375e6 Mon Sep 17 00:00:00 2001 From: pennam Date: Thu, 5 Oct 2023 09:04:41 +0200 Subject: [PATCH 05/17] Reorder functions implementation --- src/ArduinoIoTCloudTCP.cpp | 46 +++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/src/ArduinoIoTCloudTCP.cpp b/src/ArduinoIoTCloudTCP.cpp index 07d25af3c..cf416ff9d 100644 --- a/src/ArduinoIoTCloudTCP.cpp +++ b/src/ArduinoIoTCloudTCP.cpp @@ -369,6 +369,29 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SendDeviceProperties() return State::WaitDeviceConfig; } +ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_WaitDeviceConfig() +{ + if (!_mqttClient.connected()) + { + return State::Disconnect; + } + + bool const is_retry_attempt = (_last_device_subscribe_cnt > 0); + if (is_retry_attempt && (millis() > _next_device_subscribe_attempt_tick)) + { + /* Configuration not received or device not attached to a valid thing. Try to resubscribe */ + if (_mqttClient.unsubscribe(_deviceTopicIn)) + { + DEBUG_ERROR("ArduinoIoTCloudTCP::%s device waiting for valid thing_id %d", __FUNCTION__, _time_service.getTime()); + } + } + + if (!is_retry_attempt || (is_retry_attempt && (millis() > _next_device_subscribe_attempt_tick))) + return State::SubscribeDeviceTopic; + + return State::WaitDeviceConfig; +} + ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeDeviceTopic() { if (!_mqttClient.connected()) @@ -404,29 +427,6 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeDeviceTopic() return State::WaitDeviceConfig; } -ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_WaitDeviceConfig() -{ - if (!_mqttClient.connected()) - { - return State::Disconnect; - } - - bool const is_retry_attempt = (_last_device_subscribe_cnt > 0); - if (is_retry_attempt && (millis() > _next_device_subscribe_attempt_tick)) - { - /* Configuration not received or device not attached to a valid thing. Try to resubscribe */ - if (_mqttClient.unsubscribe(_deviceTopicIn)) - { - DEBUG_ERROR("ArduinoIoTCloudTCP::%s device waiting for valid thing_id %d", __FUNCTION__, _time_service.getTime()); - } - } - - if (!is_retry_attempt || (is_retry_attempt && (millis() > _next_device_subscribe_attempt_tick))) - return State::SubscribeDeviceTopic; - - return State::WaitDeviceConfig; -} - ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_CheckDeviceConfig() { if (!_mqttClient.connected()) From 3aeb4abdfce0b0f3ebbd7955b7c9a038fe5b42f8 Mon Sep 17 00:00:00 2001 From: pennam Date: Thu, 5 Oct 2023 11:13:50 +0200 Subject: [PATCH 06/17] Unify naming and logic of thing susbscribe retry --- src/ArduinoIoTCloudTCP.cpp | 24 ++++++++---------------- src/ArduinoIoTCloudTCP.h | 4 ++-- 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/src/ArduinoIoTCloudTCP.cpp b/src/ArduinoIoTCloudTCP.cpp index cf416ff9d..c5e35905d 100644 --- a/src/ArduinoIoTCloudTCP.cpp +++ b/src/ArduinoIoTCloudTCP.cpp @@ -69,8 +69,8 @@ ArduinoIoTCloudTCP::ArduinoIoTCloudTCP() , _last_device_subscribe_cnt{0} , _last_sync_request_tick{0} , _last_sync_request_cnt{0} -, _last_subscribe_request_tick{0} -, _last_subscribe_request_cnt{0} +, _next_thing_subscribe_attempt_tick{0} +, _last_thing_subscribe_attempt_cnt{0} , _mqtt_data_buf{0} , _mqtt_data_len{0} , _mqtt_data_request_retransmit{false} @@ -413,7 +413,6 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeDeviceTopic() if (_last_device_subscribe_cnt > AIOT_CONFIG_LASTVALUES_SYNC_MAX_RETRY_CNT) { _last_device_subscribe_cnt = 0; - _next_device_subscribe_attempt_tick = 0; return State::Disconnect; } @@ -469,24 +468,18 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeThingTopics() return State::Disconnect; } - unsigned long const now = millis(); - bool const is_subscribe_retry_delay_expired = (now - _last_subscribe_request_tick) > AIOT_CONFIG_THING_TOPICS_SUBSCRIBE_RETRY_DELAY_ms; - bool const is_first_subscribe_request = (_last_subscribe_request_cnt == 0); - - if (!is_first_subscribe_request && !is_subscribe_retry_delay_expired) - { + bool const is_retry_attempt = (_last_thing_subscribe_attempt_cnt > 0); + if (is_retry_attempt && (millis() < _next_thing_subscribe_attempt_tick)) return State::SubscribeThingTopics; - } - if (_last_subscribe_request_cnt > AIOT_CONFIG_THING_TOPICS_SUBSCRIBE_MAX_RETRY_CNT) + if (_last_thing_subscribe_attempt_cnt > AIOT_CONFIG_THING_TOPICS_SUBSCRIBE_MAX_RETRY_CNT) { - _last_subscribe_request_cnt = 0; - _last_subscribe_request_tick = 0; + _last_thing_subscribe_attempt_cnt = 0; return State::Disconnect; } - _last_subscribe_request_tick = now; - _last_subscribe_request_cnt++; + _next_thing_subscribe_attempt_tick = millis() + AIOT_CONFIG_THING_TOPICS_SUBSCRIBE_RETRY_DELAY_ms; + _last_thing_subscribe_attempt_cnt++; if (!_mqttClient.subscribe(_dataTopicIn)) { @@ -506,7 +499,6 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeThingTopics() DEBUG_INFO("Thing ID: %s", getThingId().c_str()); execCloudEventCallback(ArduinoIoTCloudEvent::CONNECT); - /*Add retry wait time otherwise we are trying to reconnect every 250 ms...*/ return State::RequestLastValues; } diff --git a/src/ArduinoIoTCloudTCP.h b/src/ArduinoIoTCloudTCP.h index cbe9e6fcc..88efca3f7 100644 --- a/src/ArduinoIoTCloudTCP.h +++ b/src/ArduinoIoTCloudTCP.h @@ -134,8 +134,8 @@ class ArduinoIoTCloudTCP: public ArduinoIoTCloudClass unsigned int _last_device_subscribe_cnt; unsigned long _last_sync_request_tick; unsigned int _last_sync_request_cnt; - unsigned long _last_subscribe_request_tick; - unsigned int _last_subscribe_request_cnt; + unsigned long _next_thing_subscribe_attempt_tick; + unsigned int _last_thing_subscribe_attempt_cnt; String _brokerAddress; uint16_t _brokerPort; uint8_t _mqtt_data_buf[MQTT_TRANSMIT_BUFFER_SIZE]; From 0c21a29c7e3b4df2d8bff9db76eacdec798ccda2 Mon Sep 17 00:00:00 2001 From: pennam Date: Fri, 6 Oct 2023 14:01:05 +0200 Subject: [PATCH 07/17] Reorder timeout variables declaration and initialization --- src/ArduinoIoTCloudTCP.cpp | 4 ++-- src/ArduinoIoTCloudTCP.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ArduinoIoTCloudTCP.cpp b/src/ArduinoIoTCloudTCP.cpp index c5e35905d..d2e8672be 100644 --- a/src/ArduinoIoTCloudTCP.cpp +++ b/src/ArduinoIoTCloudTCP.cpp @@ -67,10 +67,10 @@ ArduinoIoTCloudTCP::ArduinoIoTCloudTCP() , _last_connection_attempt_cnt{0} , _next_device_subscribe_attempt_tick{0} , _last_device_subscribe_cnt{0} -, _last_sync_request_tick{0} -, _last_sync_request_cnt{0} , _next_thing_subscribe_attempt_tick{0} , _last_thing_subscribe_attempt_cnt{0} +, _last_sync_request_tick{0} +, _last_sync_request_cnt{0} , _mqtt_data_buf{0} , _mqtt_data_len{0} , _mqtt_data_request_retransmit{false} diff --git a/src/ArduinoIoTCloudTCP.h b/src/ArduinoIoTCloudTCP.h index 88efca3f7..26a1b2ead 100644 --- a/src/ArduinoIoTCloudTCP.h +++ b/src/ArduinoIoTCloudTCP.h @@ -132,10 +132,10 @@ class ArduinoIoTCloudTCP: public ArduinoIoTCloudClass unsigned int _last_connection_attempt_cnt; unsigned long _next_device_subscribe_attempt_tick; unsigned int _last_device_subscribe_cnt; - unsigned long _last_sync_request_tick; - unsigned int _last_sync_request_cnt; unsigned long _next_thing_subscribe_attempt_tick; unsigned int _last_thing_subscribe_attempt_cnt; + unsigned long _last_sync_request_tick; + unsigned int _last_sync_request_cnt; String _brokerAddress; uint16_t _brokerPort; uint8_t _mqtt_data_buf[MQTT_TRANSMIT_BUFFER_SIZE]; From 8d84dcd9657b877a6b3b3a2af638d7929345b4b4 Mon Sep 17 00:00:00 2001 From: pennam Date: Fri, 6 Oct 2023 15:49:49 +0200 Subject: [PATCH 08/17] Unify naming and logic of last values request retry --- src/ArduinoIoTCloudTCP.cpp | 33 +++++++++++++++------------------ src/ArduinoIoTCloudTCP.h | 4 ++-- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/ArduinoIoTCloudTCP.cpp b/src/ArduinoIoTCloudTCP.cpp index d2e8672be..409e7754f 100644 --- a/src/ArduinoIoTCloudTCP.cpp +++ b/src/ArduinoIoTCloudTCP.cpp @@ -69,8 +69,8 @@ ArduinoIoTCloudTCP::ArduinoIoTCloudTCP() , _last_device_subscribe_cnt{0} , _next_thing_subscribe_attempt_tick{0} , _last_thing_subscribe_attempt_cnt{0} -, _last_sync_request_tick{0} -, _last_sync_request_cnt{0} +, _next_sync_attempt_tick{0} +, _last_sync_attempt_cnt{0} , _mqtt_data_buf{0} , _mqtt_data_len{0} , _mqtt_data_request_retransmit{false} @@ -515,27 +515,25 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_RequestLastValues() } /* Check whether or not we need to send a new request. */ - unsigned long const now = millis(); - bool const is_sync_request_timeout = (now - _last_sync_request_tick) > AIOT_CONFIG_TIMEOUT_FOR_LASTVALUES_SYNC_ms; - bool const is_first_sync_request = (_last_sync_request_cnt == 0); - if (is_first_sync_request || is_sync_request_timeout) + bool const is_retry_attempt = (_last_sync_attempt_cnt > 0); + if (is_retry_attempt && (millis() < _next_sync_attempt_tick)) + return State::RequestLastValues; + + if (_last_sync_attempt_cnt > AIOT_CONFIG_LASTVALUES_SYNC_MAX_RETRY_CNT) { - DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s [%d] last values requested", __FUNCTION__, now); - requestLastValue(); - _last_sync_request_tick = now; /* Track the number of times a get-last-values request was sent to the cloud. * If no data is received within a certain number of retry-requests it's a better * strategy to disconnect and re-establish connection from the ground up. */ - _last_sync_request_cnt++; - if (_last_sync_request_cnt > AIOT_CONFIG_LASTVALUES_SYNC_MAX_RETRY_CNT) - { - _last_sync_request_cnt = 0; - _last_sync_request_tick = 0; - return State::Disconnect; - } + _last_sync_attempt_cnt = 0; + return State::Disconnect; } + DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s [%d] last values requested", __FUNCTION__, _time_service.getTime()); + requestLastValue(); + _next_sync_attempt_tick = millis() + AIOT_CONFIG_TIMEOUT_FOR_LASTVALUES_SYNC_ms; + _last_sync_attempt_cnt++; + return State::RequestLastValues; } @@ -674,8 +672,7 @@ void ArduinoIoTCloudTCP::handleMessage(int length) CBORDecoder::decode(_thing_property_container, (uint8_t*)bytes, length, true); _time_service.setTimeZoneData(_tz_offset, _tz_dst_until); execCloudEventCallback(ArduinoIoTCloudEvent::SYNC); - _last_sync_request_cnt = 0; - _last_sync_request_tick = 0; + _last_sync_attempt_cnt = 0; _state = State::Connected; } } diff --git a/src/ArduinoIoTCloudTCP.h b/src/ArduinoIoTCloudTCP.h index 26a1b2ead..a200f5060 100644 --- a/src/ArduinoIoTCloudTCP.h +++ b/src/ArduinoIoTCloudTCP.h @@ -134,8 +134,8 @@ class ArduinoIoTCloudTCP: public ArduinoIoTCloudClass unsigned int _last_device_subscribe_cnt; unsigned long _next_thing_subscribe_attempt_tick; unsigned int _last_thing_subscribe_attempt_cnt; - unsigned long _last_sync_request_tick; - unsigned int _last_sync_request_cnt; + unsigned long _next_sync_attempt_tick; + unsigned int _last_sync_attempt_cnt; String _brokerAddress; uint16_t _brokerPort; uint8_t _mqtt_data_buf[MQTT_TRANSMIT_BUFFER_SIZE]; From ca26344addc33bb558949ef949f2097186e8d44b Mon Sep 17 00:00:00 2001 From: pennam Date: Fri, 6 Oct 2023 17:00:17 +0200 Subject: [PATCH 09/17] Remove WaitDeviceConfig state --- src/ArduinoIoTCloudTCP.cpp | 28 +++++++++------------------- src/ArduinoIoTCloudTCP.h | 2 -- 2 files changed, 9 insertions(+), 21 deletions(-) diff --git a/src/ArduinoIoTCloudTCP.cpp b/src/ArduinoIoTCloudTCP.cpp index 409e7754f..bb2e6705e 100644 --- a/src/ArduinoIoTCloudTCP.cpp +++ b/src/ArduinoIoTCloudTCP.cpp @@ -270,7 +270,6 @@ void ArduinoIoTCloudTCP::update() case State::ConnectMqttBroker: next_state = handle_ConnectMqttBroker(); break; case State::SendDeviceProperties: next_state = handle_SendDeviceProperties(); break; case State::SubscribeDeviceTopic: next_state = handle_SubscribeDeviceTopic(); break; - case State::WaitDeviceConfig: next_state = handle_WaitDeviceConfig(); break; case State::CheckDeviceConfig: next_state = handle_CheckDeviceConfig(); break; case State::SubscribeThingTopics: next_state = handle_SubscribeThingTopics(); break; case State::RequestLastValues: next_state = handle_RequestLastValues(); break; @@ -366,10 +365,10 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SendDeviceProperties() DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s announce device to the Cloud %d", __FUNCTION__, _time_service.getTime()); /* TODO check if write fails */ sendDevicePropertiesToCloud(); - return State::WaitDeviceConfig; + return State::SubscribeDeviceTopic; } -ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_WaitDeviceConfig() +ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeDeviceTopic() { if (!_mqttClient.connected()) { @@ -377,7 +376,10 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_WaitDeviceConfig() } bool const is_retry_attempt = (_last_device_subscribe_cnt > 0); - if (is_retry_attempt && (millis() > _next_device_subscribe_attempt_tick)) + if (is_retry_attempt && (millis() < _next_device_subscribe_attempt_tick)) + return State::SubscribeDeviceTopic; + + if (is_retry_attempt) { /* Configuration not received or device not attached to a valid thing. Try to resubscribe */ if (_mqttClient.unsubscribe(_deviceTopicIn)) @@ -386,19 +388,6 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_WaitDeviceConfig() } } - if (!is_retry_attempt || (is_retry_attempt && (millis() > _next_device_subscribe_attempt_tick))) - return State::SubscribeDeviceTopic; - - return State::WaitDeviceConfig; -} - -ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeDeviceTopic() -{ - if (!_mqttClient.connected()) - { - return State::Disconnect; - } - DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s request device configuration %d", __FUNCTION__, _time_service.getTime()); if (!_mqttClient.subscribe(_deviceTopicIn)) @@ -423,7 +412,7 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeDeviceTopic() _next_device_subscribe_attempt_tick = millis() + subscribe_retry_delay; DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s %d next configuration request in %d ms", __FUNCTION__, _last_device_subscribe_cnt, subscribe_retry_delay); - return State::WaitDeviceConfig; + return State::SubscribeDeviceTopic; } ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_CheckDeviceConfig() @@ -445,7 +434,7 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_CheckDeviceConfig() _next_device_subscribe_attempt_tick = millis() + attach_retry_delay; DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s device not attached, next configuration request in %d ms", __FUNCTION__, attach_retry_delay); - return State::WaitDeviceConfig; + return State::SubscribeDeviceTopic; } DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s device attached to a new valid thing_id %s %d", __FUNCTION__, getThingId().c_str(), _time_service.getTime()); @@ -657,6 +646,7 @@ void ArduinoIoTCloudTCP::handleMessage(int length) /* Topic for OTA properties and device configuration */ if (_deviceTopicIn == topic) { CBORDecoder::decode(_device_property_container, (uint8_t*)bytes, length); + _last_device_subscribe_cnt = 0; _state = State::CheckDeviceConfig; } diff --git a/src/ArduinoIoTCloudTCP.h b/src/ArduinoIoTCloudTCP.h index a200f5060..5815c86bf 100644 --- a/src/ArduinoIoTCloudTCP.h +++ b/src/ArduinoIoTCloudTCP.h @@ -113,7 +113,6 @@ class ArduinoIoTCloudTCP: public ArduinoIoTCloudClass ConnectMqttBroker, SendDeviceProperties, SubscribeDeviceTopic, - WaitDeviceConfig, CheckDeviceConfig, SubscribeThingTopics, RequestLastValues, @@ -199,7 +198,6 @@ class ArduinoIoTCloudTCP: public ArduinoIoTCloudClass State handle_SyncTime(); State handle_ConnectMqttBroker(); State handle_SendDeviceProperties(); - State handle_WaitDeviceConfig(); State handle_CheckDeviceConfig(); State handle_SubscribeDeviceTopic(); State handle_SubscribeThingTopics(); From 30dde27aff88ec55c7a0f0b5935b9ec5417793b1 Mon Sep 17 00:00:00 2001 From: pennam Date: Fri, 6 Oct 2023 17:08:26 +0200 Subject: [PATCH 10/17] We can safely send another subscribe request without unsubscribe --- src/ArduinoIoTCloudTCP.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/ArduinoIoTCloudTCP.cpp b/src/ArduinoIoTCloudTCP.cpp index bb2e6705e..e1ec6564f 100644 --- a/src/ArduinoIoTCloudTCP.cpp +++ b/src/ArduinoIoTCloudTCP.cpp @@ -382,10 +382,7 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeDeviceTopic() if (is_retry_attempt) { /* Configuration not received or device not attached to a valid thing. Try to resubscribe */ - if (_mqttClient.unsubscribe(_deviceTopicIn)) - { - DEBUG_ERROR("ArduinoIoTCloudTCP::%s device waiting for valid thing_id %d", __FUNCTION__, _time_service.getTime()); - } + DEBUG_ERROR("ArduinoIoTCloudTCP::%s device waiting for valid thing_id %d", __FUNCTION__, _time_service.getTime()); } DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s request device configuration %d", __FUNCTION__, _time_service.getTime()); From 03e87ba69060897192d23c0746a79fd64caeaa5f Mon Sep 17 00:00:00 2001 From: pennam Date: Fri, 6 Oct 2023 17:10:04 +0200 Subject: [PATCH 11/17] Merge disconnect conditions --- src/ArduinoIoTCloudTCP.cpp | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/ArduinoIoTCloudTCP.cpp b/src/ArduinoIoTCloudTCP.cpp index e1ec6564f..a18485b25 100644 --- a/src/ArduinoIoTCloudTCP.cpp +++ b/src/ArduinoIoTCloudTCP.cpp @@ -444,12 +444,7 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_CheckDeviceConfig() ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeThingTopics() { - if (!_mqttClient.connected()) - { - return State::Disconnect; - } - - if (_thing_id_property->isDifferentFromCloud()) + if (!_mqttClient.connected() || _thing_id_property->isDifferentFromCloud()) { return State::Disconnect; } @@ -490,12 +485,7 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeThingTopics() ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_RequestLastValues() { - if (!_mqttClient.connected()) - { - return State::Disconnect; - } - - if (_thing_id_property->isDifferentFromCloud()) + if (!_mqttClient.connected() || _thing_id_property->isDifferentFromCloud()) { return State::Disconnect; } From a7f05599c7075133ae6bcf76d2f353b90afbda47 Mon Sep 17 00:00:00 2001 From: pennam Date: Mon, 9 Oct 2023 10:12:51 +0200 Subject: [PATCH 12/17] Reorder timeout and retry defines --- src/AIoTC_Config.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/AIoTC_Config.h b/src/AIoTC_Config.h index b9301934f..063f1af9b 100644 --- a/src/AIoTC_Config.h +++ b/src/AIoTC_Config.h @@ -141,10 +141,10 @@ #define AIOT_CONFIG_MAX_RECONNECTION_RETRY_DELAY_ms (32000UL) #define AIOT_CONFIG_DEVICE_TOPIC_SUBSCRIBE_RETRY_DELAY_ms (2000UL) #define AIOT_CONFIG_MAX_DEVICE_TOPIC_SUBSCRIBE_RETRY_DELAY_ms (32000UL) -#define AIOT_CONFIG_THING_TOPICS_SUBSCRIBE_RETRY_DELAY_ms (1000UL) -#define AIOT_CONFIG_THING_TOPICS_SUBSCRIBE_MAX_RETRY_CNT (10UL) #define AIOT_CONFIG_DEVICE_TOPIC_ATTACH_RETRY_DELAY_ms (20000UL) #define AIOT_CONFIG_MAX_DEVICE_TOPIC_ATTACH_RETRY_DELAY_ms (1280000UL) +#define AIOT_CONFIG_THING_TOPICS_SUBSCRIBE_RETRY_DELAY_ms (1000UL) +#define AIOT_CONFIG_THING_TOPICS_SUBSCRIBE_MAX_RETRY_CNT (10UL) #define AIOT_CONFIG_TIMEOUT_FOR_LASTVALUES_SYNC_ms (30000UL) #define AIOT_CONFIG_LASTVALUES_SYNC_MAX_RETRY_CNT (10UL) From c9d808b7a0fb1321b014dc762abc5f85ef5d06b2 Mon Sep 17 00:00:00 2001 From: pennam Date: Mon, 9 Oct 2023 10:14:10 +0200 Subject: [PATCH 13/17] Add specific define for max number of retry connecting to device topic --- src/AIoTC_Config.h | 1 + src/ArduinoIoTCloudTCP.cpp | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/AIoTC_Config.h b/src/AIoTC_Config.h index 063f1af9b..f4fbd2a6a 100644 --- a/src/AIoTC_Config.h +++ b/src/AIoTC_Config.h @@ -141,6 +141,7 @@ #define AIOT_CONFIG_MAX_RECONNECTION_RETRY_DELAY_ms (32000UL) #define AIOT_CONFIG_DEVICE_TOPIC_SUBSCRIBE_RETRY_DELAY_ms (2000UL) #define AIOT_CONFIG_MAX_DEVICE_TOPIC_SUBSCRIBE_RETRY_DELAY_ms (32000UL) +#define AIOT_CONFIG_DEVICE_TOPIC_MAX_RETRY_CNT (10UL) #define AIOT_CONFIG_DEVICE_TOPIC_ATTACH_RETRY_DELAY_ms (20000UL) #define AIOT_CONFIG_MAX_DEVICE_TOPIC_ATTACH_RETRY_DELAY_ms (1280000UL) #define AIOT_CONFIG_THING_TOPICS_SUBSCRIBE_RETRY_DELAY_ms (1000UL) diff --git a/src/ArduinoIoTCloudTCP.cpp b/src/ArduinoIoTCloudTCP.cpp index a18485b25..d70dcfe88 100644 --- a/src/ArduinoIoTCloudTCP.cpp +++ b/src/ArduinoIoTCloudTCP.cpp @@ -396,7 +396,7 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeDeviceTopic() } /* Max retry than disconnect */ - if (_last_device_subscribe_cnt > AIOT_CONFIG_LASTVALUES_SYNC_MAX_RETRY_CNT) + if (_last_device_subscribe_cnt > AIOT_CONFIG_DEVICE_TOPIC_MAX_RETRY_CNT) { _last_device_subscribe_cnt = 0; return State::Disconnect; From 5d9f39d6780e973423955a346328c8b6f859d01e Mon Sep 17 00:00:00 2001 From: pennam Date: Mon, 9 Oct 2023 15:13:55 +0200 Subject: [PATCH 14/17] Add Class to handle retries and state transitions --- src/ArduinoIoTCloudTCP.cpp | 103 ++++++++++++++---------------- src/ArduinoIoTCloudTCP.h | 10 +-- src/utility/time/TimedAttempt.cpp | 75 ++++++++++++++++++++++ src/utility/time/TimedAttempt.h | 40 ++++++++++++ 4 files changed, 166 insertions(+), 62 deletions(-) create mode 100644 src/utility/time/TimedAttempt.cpp create mode 100644 src/utility/time/TimedAttempt.h diff --git a/src/ArduinoIoTCloudTCP.cpp b/src/ArduinoIoTCloudTCP.cpp index d70dcfe88..2fe03df53 100644 --- a/src/ArduinoIoTCloudTCP.cpp +++ b/src/ArduinoIoTCloudTCP.cpp @@ -59,18 +59,11 @@ unsigned long getTime() ArduinoIoTCloudTCP::ArduinoIoTCloudTCP() : _state{State::ConnectPhy} +, _connection_attempt(0,0) , _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} -, _last_device_subscribe_cnt{0} -, _next_thing_subscribe_attempt_tick{0} -, _last_thing_subscribe_attempt_cnt{0} -, _next_sync_attempt_tick{0} -, _last_sync_attempt_cnt{0} , _mqtt_data_buf{0} , _mqtt_data_len{0} , _mqtt_data_request_retransmit{false} @@ -113,7 +106,12 @@ int ArduinoIoTCloudTCP::begin(ConnectionHandler & connection, bool const enable_ #else _brokerPort = brokerPort; #endif + + /* Setup TimeService */ _time_service.begin(&connection); + + /* Setup retry timers */ + _connection_attempt.begin(AIOT_CONFIG_RECONNECTION_RETRY_DELAY_ms, AIOT_CONFIG_MAX_RECONNECTION_RETRY_DELAY_ms); return begin(enable_watchdog, _brokerAddress, _brokerPort); } @@ -132,15 +130,16 @@ int ArduinoIoTCloudTCP::begin(bool const enable_watchdog, String brokerAddress, if(!_password.length()) { #endif + #if defined(BOARD_HAS_SECURE_ELEMENT) if (!_selement.begin()) { DEBUG_ERROR("ArduinoIoTCloudTCP::%s could not initialize secure element.", __FUNCTION__); -#if defined(ARDUINO_UNOWIFIR4) + #if defined(ARDUINO_UNOWIFIR4) if (String(WiFi.firmwareVersion()) < String("0.4.1")) { DEBUG_ERROR("ArduinoIoTCloudTCP::%s In order to read device certificate, WiFi firmware needs to be >= 0.4.1, current %s", __FUNCTION__, WiFi.firmwareVersion()); } -#endif + #endif return 0; } if (!SElementArduinoCloudDeviceId::read(_selement, getDeviceId(), SElementArduinoCloudSlot::DeviceId)) @@ -317,8 +316,7 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_ConnectPhy() { if (_connection->check() == NetworkConnectionState::CONNECTED) { - bool const is_retry_attempt = (_last_connection_attempt_cnt > 0); - if (!is_retry_attempt || (is_retry_attempt && (millis() > _next_connection_attempt_tick))) + if (!_connection_attempt.isRetry() || (_connection_attempt.isRetry() && _connection_attempt.isExpired())) return State::SyncTime; } @@ -339,18 +337,20 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_ConnectMqttBroker() { if (_mqttClient.connect(_brokerAddress.c_str(), _brokerPort)) { - _last_connection_attempt_cnt = 0; + DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s connected to %s:%d", __FUNCTION__, _brokerAddress.c_str(), _brokerPort); + /* Reconfigure timers for next state */ + _connection_attempt.begin(AIOT_CONFIG_DEVICE_TOPIC_SUBSCRIBE_RETRY_DELAY_ms, AIOT_CONFIG_MAX_DEVICE_TOPIC_SUBSCRIBE_RETRY_DELAY_ms); return State::SendDeviceProperties; } /* Can't connect to the broker. Wait: 2s -> 4s -> 8s -> 16s -> 32s -> 32s ... */ - _last_connection_attempt_cnt++; - unsigned long reconnection_retry_delay = (1 << _last_connection_attempt_cnt) * AIOT_CONFIG_RECONNECTION_RETRY_DELAY_ms; - reconnection_retry_delay = min(reconnection_retry_delay, static_cast(AIOT_CONFIG_MAX_RECONNECTION_RETRY_DELAY_ms)); - _next_connection_attempt_tick = millis() + reconnection_retry_delay; +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wunused-variable" + unsigned long const reconnection_retry_delay = _connection_attempt.retry(); +#pragma GCC diagnostic pop DEBUG_ERROR("ArduinoIoTCloudTCP::%s could not connect to %s:%d", __FUNCTION__, _brokerAddress.c_str(), _brokerPort); - DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s %d next connection attempt in %d ms", __FUNCTION__, _last_connection_attempt_cnt, reconnection_retry_delay); + DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s %d next connection attempt in %d ms", __FUNCTION__, _connection_attempt.getRetryCount(), reconnection_retry_delay); /* Go back to ConnectPhy and retry to get time from network (invalid time for SSL handshake?)*/ return State::ConnectPhy; } @@ -375,11 +375,10 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeDeviceTopic() return State::Disconnect; } - bool const is_retry_attempt = (_last_device_subscribe_cnt > 0); - if (is_retry_attempt && (millis() < _next_device_subscribe_attempt_tick)) + if (_connection_attempt.isRetry() && !_connection_attempt.isExpired()) return State::SubscribeDeviceTopic; - if (is_retry_attempt) + if (_connection_attempt.isRetry()) { /* Configuration not received or device not attached to a valid thing. Try to resubscribe */ DEBUG_ERROR("ArduinoIoTCloudTCP::%s device waiting for valid thing_id %d", __FUNCTION__, _time_service.getTime()); @@ -396,18 +395,17 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeDeviceTopic() } /* Max retry than disconnect */ - if (_last_device_subscribe_cnt > AIOT_CONFIG_DEVICE_TOPIC_MAX_RETRY_CNT) + if (_connection_attempt.getRetryCount() > AIOT_CONFIG_DEVICE_TOPIC_MAX_RETRY_CNT) { - _last_device_subscribe_cnt = 0; return State::Disconnect; } /* No device configuration received. Wait: 4s -> 8s -> 16s -> 32s -> 32s ...*/ - _last_device_subscribe_cnt++; - unsigned long subscribe_retry_delay = (1 << _last_device_subscribe_cnt) * AIOT_CONFIG_DEVICE_TOPIC_SUBSCRIBE_RETRY_DELAY_ms; - subscribe_retry_delay = min(subscribe_retry_delay, static_cast(AIOT_CONFIG_MAX_DEVICE_TOPIC_SUBSCRIBE_RETRY_DELAY_ms)); - _next_device_subscribe_attempt_tick = millis() + subscribe_retry_delay; - DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s %d next configuration request in %d ms", __FUNCTION__, _last_device_subscribe_cnt, subscribe_retry_delay); +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wunused-variable" + unsigned long const subscribe_retry_delay = _connection_attempt.retry(); +#pragma GCC diagnostic pop + DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s %d next configuration request in %d ms", __FUNCTION__, _connection_attempt.getRetryCount(), subscribe_retry_delay); return State::SubscribeDeviceTopic; } @@ -426,18 +424,18 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_CheckDeviceConfig() /* Device configuration received, but invalid thing_id. Do not increase counter, but recompute delay. * Device not attached. Wait: 40s -> 80s -> 160s -> 320s -> 640s -> 1280s -> 1280s ... */ - unsigned long attach_retry_delay = (1 << _last_device_subscribe_cnt) * AIOT_CONFIG_DEVICE_TOPIC_ATTACH_RETRY_DELAY_ms; - attach_retry_delay = min(attach_retry_delay, static_cast(AIOT_CONFIG_MAX_DEVICE_TOPIC_ATTACH_RETRY_DELAY_ms)); - _next_device_subscribe_attempt_tick = millis() + attach_retry_delay; - +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wunused-variable" + unsigned long const attach_retry_delay = _connection_attempt.reconfigure(AIOT_CONFIG_DEVICE_TOPIC_ATTACH_RETRY_DELAY_ms, AIOT_CONFIG_MAX_DEVICE_TOPIC_ATTACH_RETRY_DELAY_ms); +#pragma GCC diagnostic pop DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s device not attached, next configuration request in %d ms", __FUNCTION__, attach_retry_delay); return State::SubscribeDeviceTopic; } DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s device attached to a new valid thing_id %s %d", __FUNCTION__, getThingId().c_str(), _time_service.getTime()); - /* Received valid thing_id reset counters and go on */ - _last_device_subscribe_cnt = 0; + /* Received valid thing_id, reconfigure timers for next state and go on */ + _connection_attempt.begin(AIOT_CONFIG_THING_TOPICS_SUBSCRIBE_RETRY_DELAY_ms); return State::SubscribeThingTopics; } @@ -449,18 +447,15 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeThingTopics() return State::Disconnect; } - bool const is_retry_attempt = (_last_thing_subscribe_attempt_cnt > 0); - if (is_retry_attempt && (millis() < _next_thing_subscribe_attempt_tick)) + if (_connection_attempt.isRetry() && !_connection_attempt.isExpired()) return State::SubscribeThingTopics; - if (_last_thing_subscribe_attempt_cnt > AIOT_CONFIG_THING_TOPICS_SUBSCRIBE_MAX_RETRY_CNT) + if (_connection_attempt.getRetryCount() > AIOT_CONFIG_THING_TOPICS_SUBSCRIBE_MAX_RETRY_CNT) { - _last_thing_subscribe_attempt_cnt = 0; return State::Disconnect; } - _next_thing_subscribe_attempt_tick = millis() + AIOT_CONFIG_THING_TOPICS_SUBSCRIBE_RETRY_DELAY_ms; - _last_thing_subscribe_attempt_cnt++; + _connection_attempt.retry(); if (!_mqttClient.subscribe(_dataTopicIn)) { @@ -480,6 +475,8 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeThingTopics() DEBUG_INFO("Thing ID: %s", getThingId().c_str()); execCloudEventCallback(ArduinoIoTCloudEvent::CONNECT); + /* Successfully subscribed to thing topics, reconfigure timers for next state and go on */ + _connection_attempt.begin(AIOT_CONFIG_TIMEOUT_FOR_LASTVALUES_SYNC_ms); return State::RequestLastValues; } @@ -491,25 +488,21 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_RequestLastValues() } /* Check whether or not we need to send a new request. */ - bool const is_retry_attempt = (_last_sync_attempt_cnt > 0); - if (is_retry_attempt && (millis() < _next_sync_attempt_tick)) + if (_connection_attempt.isRetry() && !_connection_attempt.isExpired()) return State::RequestLastValues; - if (_last_sync_attempt_cnt > AIOT_CONFIG_LASTVALUES_SYNC_MAX_RETRY_CNT) + /* Track the number of times a get-last-values request was sent to the cloud. + * If no data is received within a certain number of retry-requests it's a better + * strategy to disconnect and re-establish connection from the ground up. + */ + if (_connection_attempt.getRetryCount() > AIOT_CONFIG_LASTVALUES_SYNC_MAX_RETRY_CNT) { - /* Track the number of times a get-last-values request was sent to the cloud. - * If no data is received within a certain number of retry-requests it's a better - * strategy to disconnect and re-establish connection from the ground up. - */ - _last_sync_attempt_cnt = 0; return State::Disconnect; } - DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s [%d] last values requested", __FUNCTION__, _time_service.getTime()); + _connection_attempt.retry(); requestLastValue(); - _next_sync_attempt_tick = millis() + AIOT_CONFIG_TIMEOUT_FOR_LASTVALUES_SYNC_ms; - _last_sync_attempt_cnt++; - + DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s [%d] last values requested", __FUNCTION__, _time_service.getTime()); return State::RequestLastValues; } @@ -608,10 +601,14 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_Disconnect() } else { _mqttClient.unsubscribe(_shadowTopicIn); _mqttClient.unsubscribe(_dataTopicIn); + /* TODO add device topic */ _mqttClient.stop(); } DEBUG_INFO("Disconnected from Arduino IoT Cloud"); execCloudEventCallback(ArduinoIoTCloudEvent::DISCONNECT); + + /* Setup timer for broker connection and restart */ + _connection_attempt.begin(AIOT_CONFIG_RECONNECTION_RETRY_DELAY_ms, AIOT_CONFIG_MAX_RECONNECTION_RETRY_DELAY_ms); return State::ConnectPhy; } @@ -633,7 +630,6 @@ void ArduinoIoTCloudTCP::handleMessage(int length) /* Topic for OTA properties and device configuration */ if (_deviceTopicIn == topic) { CBORDecoder::decode(_device_property_container, (uint8_t*)bytes, length); - _last_device_subscribe_cnt = 0; _state = State::CheckDeviceConfig; } @@ -649,7 +645,6 @@ void ArduinoIoTCloudTCP::handleMessage(int length) CBORDecoder::decode(_thing_property_container, (uint8_t*)bytes, length, true); _time_service.setTimeZoneData(_tz_offset, _tz_dst_until); execCloudEventCallback(ArduinoIoTCloudEvent::SYNC); - _last_sync_attempt_cnt = 0; _state = State::Connected; } } diff --git a/src/ArduinoIoTCloudTCP.h b/src/ArduinoIoTCloudTCP.h index 5815c86bf..9b5ffa6c0 100644 --- a/src/ArduinoIoTCloudTCP.h +++ b/src/ArduinoIoTCloudTCP.h @@ -25,6 +25,7 @@ #include #include #include +#include #if defined(BOARD_HAS_SECURE_ELEMENT) #include @@ -121,20 +122,13 @@ class ArduinoIoTCloudTCP: public ArduinoIoTCloudClass }; State _state; + TimedAttempt _connection_attempt; 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; - unsigned int _last_device_subscribe_cnt; - unsigned long _next_thing_subscribe_attempt_tick; - unsigned int _last_thing_subscribe_attempt_cnt; - unsigned long _next_sync_attempt_tick; - unsigned int _last_sync_attempt_cnt; String _brokerAddress; uint16_t _brokerPort; uint8_t _mqtt_data_buf[MQTT_TRANSMIT_BUFFER_SIZE]; diff --git a/src/utility/time/TimedAttempt.cpp b/src/utility/time/TimedAttempt.cpp new file mode 100644 index 000000000..97e39a833 --- /dev/null +++ b/src/utility/time/TimedAttempt.cpp @@ -0,0 +1,75 @@ +/* + This file is part of the Arduino_SecureElement library. + + Copyright (c) 2024 Arduino SA + + This Source Code Form is subject to the terms of the Mozilla Public + License, v. 2.0. If a copy of the MPL was not distributed with this + file, You can obtain one at http://mozilla.org/MPL/2.0/. +*/ + +/****************************************************************************** + * INCLUDE + ******************************************************************************/ + +#include +#include "TimedAttempt.h" + +/****************************************************************************** + * CTOR/DTOR + ******************************************************************************/ + +TimedAttempt::TimedAttempt(unsigned long minDelay, unsigned long maxDelay) +: _minDelay(minDelay) +, _maxDelay(maxDelay) { +} + +/****************************************************************************** + * PUBLIC MEMBER FUNCTIONS + ******************************************************************************/ + +void TimedAttempt::begin(unsigned long delay) { + _retryCount = 0; + _minDelay = delay; + _maxDelay = delay; +} + +void TimedAttempt::begin(unsigned long minDelay, unsigned long maxDelay) { + _retryCount = 0; + _minDelay = minDelay; + _maxDelay = maxDelay; +} + +unsigned long TimedAttempt::reconfigure(unsigned long minDelay, unsigned long maxDelay) { + _minDelay = minDelay; + _maxDelay = maxDelay; + return reload(); +} + +unsigned long TimedAttempt::retry() { + _retryCount++; + return reload(); +} + +unsigned long TimedAttempt::reload() { + unsigned long retryDelay = (1 << _retryCount) * _minDelay; + retryDelay = min(retryDelay, _maxDelay); + _nextRetryTick = millis() + retryDelay; + return retryDelay; +} + +void TimedAttempt::reset() { + _retryCount = 0; +} + +bool TimedAttempt::isRetry() { + return _retryCount > 0; +} + +bool TimedAttempt::isExpired() { + return millis() > _nextRetryTick; +} + +unsigned int TimedAttempt::getRetryCount() { + return _retryCount; +} diff --git a/src/utility/time/TimedAttempt.h b/src/utility/time/TimedAttempt.h new file mode 100644 index 000000000..68cad47cf --- /dev/null +++ b/src/utility/time/TimedAttempt.h @@ -0,0 +1,40 @@ +/* + This file is part of the Arduino_SecureElement library. + + Copyright (c) 2024 Arduino SA + + This Source Code Form is subject to the terms of the Mozilla Public + License, v. 2.0. If a copy of the MPL was not distributed with this + file, You can obtain one at http://mozilla.org/MPL/2.0/. +*/ + +#ifndef TIMED_ATTEMPT_H +#define TIMED_ATTEMPT_H + +/****************************************************************************** + * CLASS DECLARATION + ******************************************************************************/ + +class TimedAttempt { + +public: + TimedAttempt(unsigned long minDelay, unsigned long maxDelay); + + void begin(unsigned long delay); + void begin(unsigned long minDelay, unsigned long maxDelay); + unsigned long reconfigure(unsigned long minDelay, unsigned long maxDelay); + unsigned long retry(); + unsigned long reload(); + void reset(); + bool isRetry(); + bool isExpired(); + unsigned int getRetryCount(); + +private: + unsigned long _minDelay; + unsigned long _maxDelay; + unsigned long _nextRetryTick; + unsigned int _retryCount; +}; + +#endif /* TIMED_ATTEMPT_H */ From 2f12cb9c802cff07fbbfdd1c4215a1d7ef76b0d8 Mon Sep 17 00:00:00 2001 From: pennam Date: Thu, 14 Mar 2024 13:33:19 +0100 Subject: [PATCH 15/17] TimedAttempt: add variable to store next attempt wait time --- src/utility/time/TimedAttempt.cpp | 2 +- src/utility/time/TimedAttempt.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/utility/time/TimedAttempt.cpp b/src/utility/time/TimedAttempt.cpp index 97e39a833..b3b63df56 100644 --- a/src/utility/time/TimedAttempt.cpp +++ b/src/utility/time/TimedAttempt.cpp @@ -53,7 +53,7 @@ unsigned long TimedAttempt::retry() { unsigned long TimedAttempt::reload() { unsigned long retryDelay = (1 << _retryCount) * _minDelay; - retryDelay = min(retryDelay, _maxDelay); + _retryDelay = min(retryDelay, _maxDelay); _nextRetryTick = millis() + retryDelay; return retryDelay; } diff --git a/src/utility/time/TimedAttempt.h b/src/utility/time/TimedAttempt.h index 68cad47cf..aed030a7f 100644 --- a/src/utility/time/TimedAttempt.h +++ b/src/utility/time/TimedAttempt.h @@ -34,6 +34,7 @@ class TimedAttempt { unsigned long _minDelay; unsigned long _maxDelay; unsigned long _nextRetryTick; + unsigned long _retryDelay; unsigned int _retryCount; }; From a7c28d7f4ee25da23f1dda9eab50e35606a63c22 Mon Sep 17 00:00:00 2001 From: pennam Date: Thu, 14 Mar 2024 13:34:12 +0100 Subject: [PATCH 16/17] TimedAttempt: add getWaitTime() --- src/utility/time/TimedAttempt.cpp | 4 ++++ src/utility/time/TimedAttempt.h | 1 + 2 files changed, 5 insertions(+) diff --git a/src/utility/time/TimedAttempt.cpp b/src/utility/time/TimedAttempt.cpp index b3b63df56..01da506d7 100644 --- a/src/utility/time/TimedAttempt.cpp +++ b/src/utility/time/TimedAttempt.cpp @@ -73,3 +73,7 @@ bool TimedAttempt::isExpired() { unsigned int TimedAttempt::getRetryCount() { return _retryCount; } + +unsigned int TimedAttempt::getWaitTime() { + return _retryDelay; +} diff --git a/src/utility/time/TimedAttempt.h b/src/utility/time/TimedAttempt.h index aed030a7f..67a1931c0 100644 --- a/src/utility/time/TimedAttempt.h +++ b/src/utility/time/TimedAttempt.h @@ -29,6 +29,7 @@ class TimedAttempt { bool isRetry(); bool isExpired(); unsigned int getRetryCount(); + unsigned int getWaitTime(); private: unsigned long _minDelay; From 51710f377d2d49cc6d269aeafd1a0655449d3a29 Mon Sep 17 00:00:00 2001 From: pennam Date: Thu, 14 Mar 2024 13:37:02 +0100 Subject: [PATCH 17/17] ArduinoIoTCloudTCP: use getWaitTime() for broker connection retry --- src/ArduinoIoTCloudTCP.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/ArduinoIoTCloudTCP.cpp b/src/ArduinoIoTCloudTCP.cpp index 2fe03df53..794be824c 100644 --- a/src/ArduinoIoTCloudTCP.cpp +++ b/src/ArduinoIoTCloudTCP.cpp @@ -344,13 +344,10 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_ConnectMqttBroker() } /* Can't connect to the broker. Wait: 2s -> 4s -> 8s -> 16s -> 32s -> 32s ... */ -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wunused-variable" - unsigned long const reconnection_retry_delay = _connection_attempt.retry(); -#pragma GCC diagnostic pop + _connection_attempt.retry(); DEBUG_ERROR("ArduinoIoTCloudTCP::%s could not connect to %s:%d", __FUNCTION__, _brokerAddress.c_str(), _brokerPort); - DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s %d next connection attempt in %d ms", __FUNCTION__, _connection_attempt.getRetryCount(), reconnection_retry_delay); + DEBUG_VERBOSE("ArduinoIoTCloudTCP::%s %d next connection attempt in %d ms", __FUNCTION__, _connection_attempt.getRetryCount(), _connection_attempt.getWaitTime()); /* Go back to ConnectPhy and retry to get time from network (invalid time for SSL handshake?)*/ return State::ConnectPhy; }