From eef26053d0b947ef9de784c7f225f2357fcbaede Mon Sep 17 00:00:00 2001 From: pennam Date: Thu, 13 Jan 2022 10:48:26 +0100 Subject: [PATCH 1/2] Add retry delay on topic subscription failures --- src/AIoTC_Config.h | 2 ++ src/ArduinoIoTCloudTCP.cpp | 62 +++++++++++++++++++++++++++----------- src/ArduinoIoTCloudTCP.h | 2 ++ 3 files changed, 48 insertions(+), 18 deletions(-) diff --git a/src/AIoTC_Config.h b/src/AIoTC_Config.h index 26be67d51..6949524d6 100644 --- a/src/AIoTC_Config.h +++ b/src/AIoTC_Config.h @@ -142,6 +142,8 @@ #define AIOT_CONFIG_RECONNECTION_RETRY_DELAY_ms (1000UL) #define AIOT_CONFIG_MAX_RECONNECTION_RETRY_DELAY_ms (32000UL) +#define AIOT_CONFIG_SUBSCRIBE_RETRY_DELAY_ms (1000UL) +#define AIOT_CONFIG_SUBSCRIBE_MAX_RETRY_CNT (10UL) #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 8bd632ad8..b1d61a7d6 100644 --- a/src/ArduinoIoTCloudTCP.cpp +++ b/src/ArduinoIoTCloudTCP.cpp @@ -79,6 +79,8 @@ ArduinoIoTCloudTCP::ArduinoIoTCloudTCP() , _last_connection_attempt_cnt{0} , _last_sync_request_tick{0} , _last_sync_request_cnt{0} +, _last_subscribe_request_tick{0} +, _last_subscribe_request_cnt{0} , _mqtt_data_buf{0} , _mqtt_data_len{0} , _mqtt_data_request_retransmit{false} @@ -404,34 +406,58 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeMqttTopics() return State::ConnectPhy; } - if (!_mqttClient.subscribe(_dataTopicIn)) + unsigned long const now = millis(); + bool const is_subscribe_retry_delay_expired = (now - _last_subscribe_request_tick) > AIOT_CONFIG_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) { - DEBUG_ERROR("ArduinoIoTCloudTCP::%s could not subscribe to %s", __FUNCTION__, _dataTopicIn.c_str()); -#if !defined(__AVR__) - DEBUG_ERROR("Check your thing configuration, and press the reset button on your board."); -#endif - return State::SubscribeMqttTopics; - } + if (_last_subscribe_request_cnt > AIOT_CONFIG_SUBSCRIBE_MAX_RETRY_CNT) + { + _last_subscribe_request_cnt = 0; + _last_subscribe_request_tick = 0; + _mqttClient.stop(); + execCloudEventCallback(ArduinoIoTCloudEvent::DISCONNECT); + return State::ConnectPhy; + } - if (_shadowTopicIn != "") - { - if (!_mqttClient.subscribe(_shadowTopicIn)) + _last_subscribe_request_tick = now; + _last_subscribe_request_cnt++; + + if (!_mqttClient.subscribe(_dataTopicIn)) { - DEBUG_ERROR("ArduinoIoTCloudTCP::%s could not subscribe to %s", __FUNCTION__, _shadowTopicIn.c_str()); + DEBUG_ERROR("ArduinoIoTCloudTCP::%s could not subscribe to %s", __FUNCTION__, _dataTopicIn.c_str()); #if !defined(__AVR__) DEBUG_ERROR("Check your thing configuration, and press the reset button on your board."); #endif return State::SubscribeMqttTopics; } - } - DEBUG_INFO("Connected to Arduino IoT Cloud"); - execCloudEventCallback(ArduinoIoTCloudEvent::CONNECT); + if (_shadowTopicIn != "") + { + if (!_mqttClient.subscribe(_shadowTopicIn)) + { + DEBUG_ERROR("ArduinoIoTCloudTCP::%s could not subscribe to %s", __FUNCTION__, _shadowTopicIn.c_str()); +#if !defined(__AVR__) + DEBUG_ERROR("Check your thing configuration, and press the reset button on your board."); +#endif + return State::SubscribeMqttTopics; + } + } - if (_shadowTopicIn != "") - return State::RequestLastValues; - else - return State::Connected; + _last_subscribe_request_cnt = 0; + _last_subscribe_request_tick = 0; + + DEBUG_INFO("Connected to Arduino IoT Cloud"); + execCloudEventCallback(ArduinoIoTCloudEvent::CONNECT); + + if (_shadowTopicIn != "") + return State::RequestLastValues; + else + return State::Connected; + + } + + return State::SubscribeMqttTopics; } ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_RequestLastValues() diff --git a/src/ArduinoIoTCloudTCP.h b/src/ArduinoIoTCloudTCP.h index c36fae4d8..7d91d86d3 100644 --- a/src/ArduinoIoTCloudTCP.h +++ b/src/ArduinoIoTCloudTCP.h @@ -116,6 +116,8 @@ class ArduinoIoTCloudTCP: public ArduinoIoTCloudClass unsigned int _last_connection_attempt_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; String _brokerAddress; uint16_t _brokerPort; uint8_t _mqtt_data_buf[MQTT_TRANSMIT_BUFFER_SIZE]; From 696b48eb557a9884557ebd57566d366700efb231 Mon Sep 17 00:00:00 2001 From: pennam Date: Tue, 18 Jan 2022 09:53:05 +0100 Subject: [PATCH 2/2] Avoid big nested 'if' when checking for subscribe_retry_delay --- src/ArduinoIoTCloudTCP.cpp | 72 +++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/src/ArduinoIoTCloudTCP.cpp b/src/ArduinoIoTCloudTCP.cpp index b1d61a7d6..33c87f32c 100644 --- a/src/ArduinoIoTCloudTCP.cpp +++ b/src/ArduinoIoTCloudTCP.cpp @@ -409,55 +409,55 @@ ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_SubscribeMqttTopics() unsigned long const now = millis(); bool const is_subscribe_retry_delay_expired = (now - _last_subscribe_request_tick) > AIOT_CONFIG_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) + + if (!is_first_subscribe_request && !is_subscribe_retry_delay_expired) { - if (_last_subscribe_request_cnt > AIOT_CONFIG_SUBSCRIBE_MAX_RETRY_CNT) - { - _last_subscribe_request_cnt = 0; - _last_subscribe_request_tick = 0; - _mqttClient.stop(); - execCloudEventCallback(ArduinoIoTCloudEvent::DISCONNECT); - return State::ConnectPhy; - } + return State::SubscribeMqttTopics; + } + + if (_last_subscribe_request_cnt > AIOT_CONFIG_SUBSCRIBE_MAX_RETRY_CNT) + { + _last_subscribe_request_cnt = 0; + _last_subscribe_request_tick = 0; + _mqttClient.stop(); + execCloudEventCallback(ArduinoIoTCloudEvent::DISCONNECT); + return State::ConnectPhy; + } - _last_subscribe_request_tick = now; - _last_subscribe_request_cnt++; + _last_subscribe_request_tick = now; + _last_subscribe_request_cnt++; - if (!_mqttClient.subscribe(_dataTopicIn)) - { - DEBUG_ERROR("ArduinoIoTCloudTCP::%s could not subscribe to %s", __FUNCTION__, _dataTopicIn.c_str()); + if (!_mqttClient.subscribe(_dataTopicIn)) + { + DEBUG_ERROR("ArduinoIoTCloudTCP::%s could not subscribe to %s", __FUNCTION__, _dataTopicIn.c_str()); #if !defined(__AVR__) - DEBUG_ERROR("Check your thing configuration, and press the reset button on your board."); + DEBUG_ERROR("Check your thing configuration, and press the reset button on your board."); #endif - return State::SubscribeMqttTopics; - } + return State::SubscribeMqttTopics; + } - if (_shadowTopicIn != "") + if (_shadowTopicIn != "") + { + if (!_mqttClient.subscribe(_shadowTopicIn)) { - if (!_mqttClient.subscribe(_shadowTopicIn)) - { - DEBUG_ERROR("ArduinoIoTCloudTCP::%s could not subscribe to %s", __FUNCTION__, _shadowTopicIn.c_str()); + DEBUG_ERROR("ArduinoIoTCloudTCP::%s could not subscribe to %s", __FUNCTION__, _shadowTopicIn.c_str()); #if !defined(__AVR__) - DEBUG_ERROR("Check your thing configuration, and press the reset button on your board."); + DEBUG_ERROR("Check your thing configuration, and press the reset button on your board."); #endif - return State::SubscribeMqttTopics; - } + return State::SubscribeMqttTopics; } + } - _last_subscribe_request_cnt = 0; - _last_subscribe_request_tick = 0; - - DEBUG_INFO("Connected to Arduino IoT Cloud"); - execCloudEventCallback(ArduinoIoTCloudEvent::CONNECT); + DEBUG_INFO("Connected to Arduino IoT Cloud"); + execCloudEventCallback(ArduinoIoTCloudEvent::CONNECT); + _last_subscribe_request_cnt = 0; + _last_subscribe_request_tick = 0; - if (_shadowTopicIn != "") - return State::RequestLastValues; - else - return State::Connected; - - } + if (_shadowTopicIn != "") + return State::RequestLastValues; + else + return State::Connected; - return State::SubscribeMqttTopics; } ArduinoIoTCloudTCP::State ArduinoIoTCloudTCP::handle_RequestLastValues()