From 8ca851154cec8e4489789644b98704a39cbe38ab Mon Sep 17 00:00:00 2001 From: Pascal Brunot Date: Fri, 16 Aug 2024 15:47:39 +0200 Subject: [PATCH] FabBackend: handle failure mode (backend down, broker up) --- include/BoardInfo.hpp | 12 ++++++---- include/FabBackend.hpp | 5 ++-- src/BoardLogic.cpp | 4 ++-- src/FabBackend.cpp | 44 +++++++++++++++++++++++++----------- src/LCDWrapper.cpp | 26 +++++++++++++++++++-- test/test_mqtt/test_mqtt.cpp | 1 + 6 files changed, 68 insertions(+), 24 deletions(-) diff --git a/include/BoardInfo.hpp b/include/BoardInfo.hpp index 4998365a..da8bbc61 100644 --- a/include/BoardInfo.hpp +++ b/include/BoardInfo.hpp @@ -8,15 +8,17 @@ namespace fabomatic /// @brief Helper struct representing the display state struct BoardInfo { - bool server_connected; - Machine::PowerState power_state; - bool power_warning; + bool mqtt_connected{false}; + Machine::PowerState power_state{Machine::PowerState::Unknown}; + bool power_warning{false}; + bool unresponsive{true}; [[nodiscard]] auto operator==(const BoardInfo &other) const -> bool { - return server_connected == other.server_connected && + return mqtt_connected == other.mqtt_connected && power_state == other.power_state && - power_warning == other.power_warning; + power_warning == other.power_warning && + unresponsive == other.unresponsive; }; }; } // namespace fabomatic diff --git a/include/FabBackend.hpp b/include/FabBackend.hpp index 33bbf397..99e9a9a0 100644 --- a/include/FabBackend.hpp +++ b/include/FabBackend.hpp @@ -50,14 +50,14 @@ namespace fabomatic std::string last_reply{""}; std::string last_request{""}; - bool online{false}; + bool mqtt_connected{false}; bool answer_pending{false}; int16_t channel{-1}; Buffer buffer; + std::optional last_unresponsive{std::nullopt}; auto messageReceived(String &topic, String &payload) -> void; - auto requestReceived(String &topic, String &payload) -> void; template [[nodiscard]] auto publish(const QueryT &payload) -> PublishResult; @@ -85,6 +85,7 @@ namespace fabomatic [[nodiscard]] auto alive() -> bool; [[nodiscard]] auto publish(String topic, String payload, bool waitForAnswer) -> bool; [[nodiscard]] auto isOnline() const -> bool; + [[nodiscard]] auto isResponsive() const -> bool; [[nodiscard]] auto hasBufferedMsg() const -> bool; [[nodiscard]] auto transmitBuffer() -> bool; [[nodiscard]] auto saveBuffer() -> bool; diff --git a/src/BoardLogic.cpp b/src/BoardLogic.cpp index 1b4a6951..d86ae3a4 100644 --- a/src/BoardLogic.cpp +++ b/src/BoardLogic.cpp @@ -122,7 +122,7 @@ namespace fabomatic { constexpr auto STEPS_COUNT = 6; constexpr milliseconds delay_per_step = std::chrono::duration_cast(conf::machine::LONG_TAP_DURATION) / STEPS_COUNT; - const BoardInfo bi = {server.isOnline(), machine.getPowerState(), machine.isShutdownImminent()}; + const BoardInfo bi = {server.isOnline(), machine.getPowerState(), machine.isShutdownImminent(), !server.isResponsive()}; for (auto step = 0; step < STEPS_COUNT; step++) { @@ -425,7 +425,7 @@ namespace fabomatic lcd.setRow(1, buffer.str()); break; } - BoardInfo bi = {server.isOnline(), machine.getPowerState(), machine.isShutdownImminent()}; + BoardInfo bi = {server.isOnline(), machine.getPowerState(), machine.isShutdownImminent(), !server.isResponsive()}; lcd.update(bi, false); } diff --git a/src/FabBackend.cpp b/src/FabBackend.cpp index 8c0fe326..0902b599 100644 --- a/src/FabBackend.cpp +++ b/src/FabBackend.cpp @@ -32,7 +32,7 @@ namespace fabomatic #else channel = -1; #endif - online = false; + mqtt_connected = false; std::stringstream ss_topic_name, ss_client_name; ss_topic_name << conf::mqtt::topic << "/" << config.machine_id; @@ -83,6 +83,7 @@ namespace fabomatic if (waitForAnswer(conf::mqtt::TIMEOUT_REPLY_SERVER)) { ESP_LOGD(TAG, "MQTT Client: received answer: %s", last_reply.data()); + last_unresponsive = std::nullopt; return PublishResult::PublishedWithAnswer; } @@ -101,6 +102,7 @@ namespace fabomatic if (published) { + last_unresponsive = Tasks::arduinoNow(); return PublishResult::PublishedWithoutAnswer; } @@ -112,10 +114,10 @@ namespace fabomatic * * @param mqtt_topic The topic to publish to. * @param mqtt_payload The payload to publish. - * @param waitForAnswer Whether to wait for an answer. + * @param answerNeeded Whether to wait for an answer. * @return true if the message was published successfully, false otherwise. */ - bool FabBackend::publish(String mqtt_topic, String mqtt_payload, bool waitForAnswer) + bool FabBackend::publish(String mqtt_topic, String mqtt_payload, bool answerNeeded) { if (mqtt_payload.length() + mqtt_topic.length() > FabBackend::MAX_MSG_SIZE - 8) { @@ -123,7 +125,7 @@ namespace fabomatic return false; } - answer_pending = waitForAnswer; + answer_pending = answerNeeded; last_query.assign(mqtt_payload.c_str()); ESP_LOGI(TAG, "MQTT Client: sending message %s on topic %s", mqtt_payload.c_str(), mqtt_topic.c_str()); @@ -185,11 +187,11 @@ namespace fabomatic { if (!client.loop()) { - if (online) + if (mqtt_connected) { ESP_LOGI(TAG, "MQTT Client: connection lost"); } - online = false; + mqtt_connected = false; return false; } return true; @@ -224,17 +226,28 @@ namespace fabomatic } while (Tasks::arduinoNow() < (start_time + max_duration)); ESP_LOGE(TAG, "Failure, no answer from MQTT server (timeout:%lld ms)", max_duration.count()); + return false; } /** - * @brief Checks if the client is online. + * @brief Indicates if the MQTT client is connected to the broker and backend is responsive. * * @return true if the client is online, false otherwise. */ bool FabBackend::isOnline() const { - return online; + return mqtt_connected && isResponsive(); + } + + /** + * @brief Indicates if backend answers are received + * + * @return true if the backend is responding, false otherwise. + */ + bool FabBackend::isResponsive() const + { + return !last_unresponsive; } /** @@ -310,7 +323,7 @@ namespace fabomatic // Check if WiFi network is available, and if not, try to connect if (status != WL_CONNECTED) { - online = false; + mqtt_connected = false; if (client.connected()) { @@ -367,7 +380,7 @@ namespace fabomatic else { ESP_LOGD(TAG, "MQTT Client: subscribed to reply topic %s", response_topic.c_str()); - online = true; + mqtt_connected = true; } std::stringstream ss_req{}; @@ -381,7 +394,7 @@ namespace fabomatic else { ESP_LOGD(TAG, "MQTT Client: subscribed to requests topic %s", request_topic.c_str()); - online = true; + mqtt_connected = true; } // Announce the board to the server @@ -402,10 +415,15 @@ namespace fabomatic if (!client.connected()) { - online = false; + mqtt_connected = false; + last_unresponsive = Tasks::arduinoNow(); + } + else + { + last_unresponsive = std::nullopt; } - return online; + return mqtt_connected; } /** diff --git a/src/LCDWrapper.cpp b/src/LCDWrapper.cpp index 3c2e2cae..46a0e8f1 100644 --- a/src/LCDWrapper.cpp +++ b/src/LCDWrapper.cpp @@ -85,7 +85,18 @@ namespace fabomatic { lcd.setCursor(conf::lcd::COLS - 2, 0); lcd.write(CHAR_ANTENNA); - lcd.write(info.server_connected ? CHAR_CONNECTION : CHAR_NO_CONNECTION); + if (info.mqtt_connected) + { + lcd.write(CHAR_CONNECTION); + } + else if (info.unresponsive) + { + lcd.write('?'); + } + else + { + lcd.write(CHAR_NO_CONNECTION); + } } if (show_power_status) @@ -183,7 +194,18 @@ namespace fabomatic // Add symbols constexpr auto symbols_per_line = conf::lcd::COLS + 3; - str[symbols_per_line * 2 - 2] = bi.server_connected ? 'Y' : 'x'; + if (bi.mqtt_connected) + { + str[symbols_per_line * 2 - 2] = 'Y'; + } + else if (bi.unresponsive) + { + str[symbols_per_line * 2 - 2] = '?'; + } + else + { + str[symbols_per_line * 2 - 2] = 'x'; + } if (bi.power_state == Machine::PowerState::PoweredOn) str[symbols_per_line * 3 - 1] = 'Y'; diff --git a/test/test_mqtt/test_mqtt.cpp b/test/test_mqtt/test_mqtt.cpp index d0e1b602..f50d6dac 100644 --- a/test/test_mqtt/test_mqtt.cpp +++ b/test/test_mqtt/test_mqtt.cpp @@ -364,6 +364,7 @@ namespace fabomatic::tests if (server.isOnline()) { TEST_ASSERT_TRUE_MESSAGE(server.loop(), "test_taskMQTTAlive: Server loop failed"); + TEST_ASSERT_TRUE_MESSAGE(server.isResponsive(), "Server is not returning answers!"); } }