From ee13a024092170a83bcfe49f5c39470b803fc08e Mon Sep 17 00:00:00 2001 From: proddy Date: Thu, 18 Jun 2020 12:19:21 +0200 Subject: [PATCH] cleaner handling of Tx retries for both READ and WRITE operations --- src/emsesp.cpp | 31 +++++++++++++++++-------------- src/telegram.cpp | 30 +++++++++++++++++------------- src/telegram.h | 43 ++++++++++++++++++++++++++----------------- 3 files changed, 60 insertions(+), 44 deletions(-) diff --git a/src/emsesp.cpp b/src/emsesp.cpp index 9d0814a27..4af4a1fa9 100644 --- a/src/emsesp.cpp +++ b/src/emsesp.cpp @@ -571,6 +571,7 @@ void EMSESP::incoming_telegram(uint8_t * data, const uint8_t length) { if (((first_value & 0x7F) == txservice_.ems_bus_id()) && (length > 1)) { // if we ask ourself at roomcontrol for version e.g. 0B 98 02 00 20 Roomctrl::check((data[1] ^ 0x80 ^ rxservice_.ems_mask()), data); + #ifdef EMSESP_DEBUG // get_uptime is only updated once per loop, does not give the right time LOG_DEBUG(F("[DEBUG] Echo after %d ms: %s"), ::millis() - tx_time_, Helpers::data_to_hex(data, length).c_str()); @@ -579,27 +580,27 @@ void EMSESP::incoming_telegram(uint8_t * data, const uint8_t length) { } // are we waiting for a response from a recent Tx Read or Write? - bool tx_successful = false; - if (EMSbus::tx_waiting()) { - EMSbus::tx_waiting(false); // reset Tx wait state + uint8_t op = EMSbus::tx_waiting(); + if (op != Telegram::Operation::NONE) { + bool tx_successful = false; + EMSbus::tx_waiting(Telegram::Operation::NONE); // reset Tx wait state - // if it's a single byte 1 or 4 then its maybe a response from the last write action - if (length == 1) { + // if we're waiting on a Write operation, we want a single byte 1 or 4 + if ((op == Telegram::Operation::TX_WRITE) && (length == 1)) { if (first_value == TxService::TX_WRITE_SUCCESS) { - LOG_DEBUG(F("Last Tx write successful. Sending read request.")); + LOG_DEBUG(F("Last Tx write successful")); txservice_.increment_telegram_write_count(); // last tx/write was confirmed ok txservice_.send_poll(); // close the bus txservice_.post_send_query(); // follow up with any post-read - tx_successful = true; txservice_.reset_retry_count(); + tx_successful = true; } else if (first_value == TxService::TX_WRITE_FAIL) { LOG_ERROR(F("Last Tx write rejected by host")); txservice_.send_poll(); // close the bus txservice_.reset_retry_count(); } - } else { - // got a telegram with data in it. See if the src/dest matches that from the last one we sent - // and continue to process it + } else if (op == Telegram::Operation::TX_READ) { + // got a telegram with data in it. See if the src/dest matches that from the last one we sent and continue to process it uint8_t src = data[0]; uint8_t dest = data[1]; if (txservice_.is_last_tx(src, dest)) { @@ -613,17 +614,19 @@ void EMSESP::incoming_telegram(uint8_t * data, const uint8_t length) { // if Tx wasn't successful, retry or give up if (!tx_successful) { - // the telegram we got wasn't what we had requested - // So re-send the last Tx and increment retry count + // the telegram we got wasn't what we had requested, so re-send the last Tx and increment retry count uint8_t retries = txservice_.retry_tx(); // returns 0 if exceeded count #ifdef EMSESP_DEBUG - LOG_DEBUG(F("[DEBUG] Last Tx operation failed. Retry #%d. Sent: %s, received: %s"), + LOG_DEBUG(F("[DEBUG] Last Tx %s operation failed. Retry #%d. Sent: %s, received: %s"), + (op == Telegram::Operation::TX_WRITE) ? F("Write") : F("Read"), retries, txservice_.last_tx_to_string().c_str(), Helpers::data_to_hex(data, length).c_str()); #endif if (!retries) { - LOG_ERROR(F("Last Tx operation failed after %d retries. Ignoring request."), txservice_.MAXIMUM_TX_RETRIES); + LOG_ERROR(F("Last Tx %s operation failed after %d retries. Ignoring request."), + (op == Telegram::Operation::TX_WRITE) ? F("Write") : F("Read"), + txservice_.MAXIMUM_TX_RETRIES); } return; diff --git a/src/telegram.cpp b/src/telegram.cpp index d6d91cb34..9e7db3e88 100644 --- a/src/telegram.cpp +++ b/src/telegram.cpp @@ -42,7 +42,7 @@ uint32_t EMSbus::last_bus_activity_ = 0; // timestamp of last time bool EMSbus::bus_connected_ = false; // start assuming the bus hasn't been connected uint8_t EMSbus::ems_mask_ = EMS_MASK_UNSET; // unset so its triggered when booting, the its 0x00=buderus, 0x80=junker/ht3 uint8_t EMSbus::ems_bus_id_ = EMSESP_DEFAULT_BUS_ID; -bool EMSbus::tx_waiting_ = false; +uint8_t EMSbus::tx_waiting_ = Telegram::Operation::NONE; bool EMSbus::tx_active_ = false; uuid::log::Logger EMSbus::logger_{F_(logger_name), uuid::log::Facility::CONSOLE}; @@ -348,7 +348,7 @@ void TxService::loop() { #ifndef EMSESP_STANDALONE if ((uuid::get_uptime() - last_tx_check_) > TX_LOOP_WAIT) { last_tx_check_ = uuid::get_uptime(); - if (!tx_active_ && (EMSbus::bus_connected())) { + if (!tx_active() && (EMSbus::bus_connected())) { LOG_ERROR(F("Tx is not active. Please check connection.")); } } @@ -443,7 +443,7 @@ void TxService::send_telegram(const QueuedTxTelegram & tx_telegram) { uint8_t length = message_p; - remember_tx(telegram_raw, length); // make a copy of it in case we want to re-send it, without the CRC + remember_tx(telegram->operation, telegram_raw, length); // make a copy of it in case we want to re-send it, without the CRC telegram_raw[length] = calculate_crc(telegram_raw, length); // generate and append CRC to the end @@ -473,12 +473,12 @@ void TxService::send_telegram(const QueuedTxTelegram & tx_telegram) { if (status == EMS_TX_STATUS_ERR) { LOG_ERROR(F("Failed to transmit Tx via UART.")); - increment_telegram_fail_count(); // another Tx fail - tx_waiting(false); // nothing send, tx not in wait state + increment_telegram_fail_count(); // another Tx fail + tx_waiting(Telegram::Operation::NONE); // nothing send, tx not in wait state return; } - tx_waiting(true); // tx now in a wait state + tx_waiting(telegram->operation); // tx now in a wait state } // send an array of bytes as a telegram @@ -492,7 +492,7 @@ void TxService::send_telegram(const uint8_t * data, const uint8_t length) { } telegram_raw[length] = calculate_crc(telegram_raw, length); // apppend CRC - tx_waiting(false); // no post validation + tx_waiting(Telegram::Operation::NONE); // no post validation needed // send the telegram to the UART Tx uint16_t status = EMSuart::transmit(telegram_raw, length); @@ -519,11 +519,12 @@ void TxService::add(const uint8_t operation, const uint8_t dest, const uint16_t tx_telegrams_.emplace_back(tx_telegram_id_++, std::move(telegram)); } -// builds a Tx telegram and adds to queue, using only raw data +// builds a Tx telegram and adds to queue +// this is used by the retry() function to put the last failed Tx back into the queue // format is EMS 1.0 (src, dest, type_id, offset, data) // length is the length of the whole telegram data // front = true if adding to the front of queue, e.g. with an Tx retry. Default is false. -void TxService::add(uint8_t * data, const uint8_t length, bool front) { +void TxService::add(uint8_t operation, uint8_t * data, const uint8_t length, bool front) { if (length < 5) { LOG_ERROR(F("Tx telegram too short (telegram length is %d)"), length); return; @@ -538,7 +539,7 @@ void TxService::add(uint8_t * data, const uint8_t length, bool front) { uint8_t offset = data[3]; uint8_t * message_data = data + 4; - auto telegram = std::make_shared(Telegram::Operation::TX_RAW, src, dest, type_id, offset, message_data, message_length); + auto telegram = std::make_shared(operation, src, dest, type_id, offset, message_data, message_length); // operation is TX_WRITE or TX_READ // if the queue is full, make room but removing the last one if (tx_telegrams_.size() >= MAX_TX_TELEGRAMS) { @@ -599,15 +600,18 @@ void TxService::send_raw(const char * telegram_data) { return; // nothing to send } - add(data, count + 1); // add to Tx queue + add(Telegram::Operation::TX_RAW, data, count + 1); // add to Tx queue } // save the last Tx sent, only the telegram, excluding the CRC -void TxService::remember_tx(const uint8_t * data, const uint8_t length) { +void TxService::remember_tx(const uint8_t operation, const uint8_t * data, const uint8_t length) { for (uint8_t i = 0; i < length; i++) { telegram_last_[i] = data[i]; } + telegram_last_length_ = length; + telegram_last_op_ = operation; + if (ems_mask() != EMS_MASK_UNSET) { telegram_last_[0] ^= ems_mask(); } @@ -622,7 +626,7 @@ uint8_t TxService::retry_tx() { return 0; } - add(telegram_last_, telegram_last_length_, true); // add the last Tx telegram to the front of the tx queue, at the top + add(telegram_last_op_, telegram_last_, telegram_last_length_, true); // add the last Tx telegram to the front of the tx queue, at the top return retry_count_; } diff --git a/src/telegram.h b/src/telegram.h index b0d7192e9..5f2d3550a 100644 --- a/src/telegram.h +++ b/src/telegram.h @@ -76,6 +76,7 @@ class Telegram { std::string to_string(const uint8_t * telegram, uint8_t length) const; enum Operation : uint8_t { + NONE = 0, RX, TX_RAW, TX_READ, @@ -144,17 +145,23 @@ class EMSbus { bus_connected_ = true; } - static bool tx_waiting() { - return tx_waiting_; + + + static bool tx_active() { + return tx_active_; + } + static void tx_active(bool tx_active) { + tx_active_ = tx_active; } - static bool tx_active_; // whether Tx is active or not - - static void tx_waiting(bool tx_waiting) { + static uint8_t tx_waiting() { + return tx_waiting_; + } + static void tx_waiting(uint8_t tx_waiting) { tx_waiting_ = tx_waiting; - // if false, then it's been reset which means we have an active Tx - if (!tx_waiting) { + // if NONE, then it's been reset which means we have an active Tx + if ((tx_waiting == Telegram::Operation::NONE) && !(tx_active_)) { tx_active_ = true; } } @@ -168,7 +175,8 @@ class EMSbus { static bool bus_connected_; // start assuming the bus hasn't been connected static uint8_t ems_mask_; // unset 0x00 buderus 0x80 junkers/ht3 static uint8_t ems_bus_id_; // the bus id, which configurable and stored in settings - static bool tx_waiting_; // state of the Tx queue (idle, waiting for ack) + static uint8_t tx_waiting_; // state of the Tx line (NONE or waiting on a TX_READ or TX_WRITE) + static bool tx_active_; // is true is we have a working Tx connection }; class RxService : public EMSbus { @@ -240,7 +248,7 @@ class TxService : public EMSbus { void send(); void add(const uint8_t operation, const uint8_t dest, const uint16_t type_id, const uint8_t offset, uint8_t * message_data, const uint8_t message_length); - void add(uint8_t * data, const uint8_t length, bool front = false); + void add(const uint8_t operation, uint8_t * data, const uint8_t length, bool front = false); void read_request(const uint16_t type_id, const uint8_t dest, const uint8_t offset = 0); @@ -250,7 +258,7 @@ class TxService : public EMSbus { void flush_tx_queue(); - void remember_tx(const uint8_t * data, const uint8_t length); + void remember_tx(const uint8_t operation, const uint8_t * data, const uint8_t length); uint8_t retry_tx(); @@ -319,16 +327,17 @@ class TxService : public EMSbus { std::deque tx_telegrams_; + uint16_t telegram_read_count_ = 0; // # Tx successful reads + uint16_t telegram_write_count_ = 0; // # Tx successful writes + uint16_t telegram_fail_count_ = 0; // # Tx unsuccessful transmits + + const std::shared_ptr telegram_last; + uint8_t telegram_last_[EMS_MAX_TELEGRAM_LENGTH]; // copy of last telegram uint8_t telegram_last_length_; // and its length uint16_t telegram_last_post_send_query_; // which type ID to query after a successful send, to read back the values just written - - const std::shared_ptr telegram_last; // copy of last telegram - uint16_t telegram_read_count_ = 0; // # Tx successful reads - uint16_t telegram_write_count_ = 0; // # Tx successful writes - uint16_t telegram_fail_count_ = 0; // # Tx unsuccessful transmits - - uint8_t retry_count_ = 0; // count for # Tx retries + uint8_t telegram_last_op_; // TX_WRITE or TX_READ + uint8_t retry_count_ = 0; // count for # Tx retries void send_telegram(const QueuedTxTelegram & tx_telegram); void send_telegram(const uint8_t * data, const uint8_t length);