Merge proddy v2a14, remove tx start-timeout, yield in sensors for uart-task

src/emsesp.cpp

@@ -124,8 +124,8 @@ void EMSESP::show_emsbus(uuid::console::Shell & shell) {
         shell.printfln(F("  #telegrams received: %d"), rxservice_.telegram_count());
         shell.printfln(F("  #read requests sent: %d"), txservice_.telegram_read_count());
         shell.printfln(F("  #write requests sent: %d"), txservice_.telegram_write_count());
-        shell.printfln(F("  #CRC errors: %d (%d%%)"), rxservice_.telegram_error_count(), success_rate);
+        shell.printfln(F("  #incomplete telegrams: %d (%d%%)"), rxservice_.telegram_error_count(), success_rate);
-        shell.printfln(F("  #Tx fails: %d"), txservice_.telegram_fail_count());
+        shell.printfln(F("  #tx fails (after 3 retries): %d"), txservice_.telegram_fail_count());
     } else {
         shell.printfln(F("EMS Bus is disconnected"));
     }
@@ -564,8 +564,10 @@ void EMSESP::incoming_telegram(uint8_t * data, const uint8_t length) {
     uint8_t first_value = data[0];
     if (((first_value & 0x7F) == txservice_.ems_bus_id()) && (length > 1)) {
         // if we ask ourself at roomcontrol for version e.g. 0B 98 02 ...
-        Roomctrl::check((data[1] ^ 0x80 ^ rxservice_.ems_mask()), data, length);
+        Roomctrl::check((data[1] ^ 0x80 ^ rxservice_.ems_mask()), data);
-        rxservice_.add(data, length); // just for logging
+#ifdef EMSESP_DEBUG
+        rxservice_.add(data, length); // just for logging, if compiled with additional debugging
+#endif
         return; // it's an echo
     }
 
@@ -606,6 +608,9 @@ void EMSESP::incoming_telegram(uint8_t * data, const uint8_t length) {
         if (!tx_successful) {
             // the telegram we got wasn't what we had requested
             // So re-send the last Tx and increment retry count
+#ifdef EMSESP_DEBUG
+            LOG_DEBUG(F("send: %s, received: %s"), txservice_.last_tx_to_string().c_str(), Helpers::data_to_hex(data, length).c_str());
+#endif
             uint8_t retries = txservice_.retry_tx(); // returns 0 if exceeded count
             if (retries) {
                 LOG_ERROR(F("Last Tx operation failed. Retrying #%d..."), retries);
@@ -630,7 +635,7 @@ void EMSESP::incoming_telegram(uint8_t * data, const uint8_t length) {
         return;
     } else {
         // check if there is a message for the roomcontroller
-        Roomctrl::check((data[1] ^ 0x80 ^ rxservice_.ems_mask()), data, length);
+        Roomctrl::check((data[1] ^ 0x80 ^ rxservice_.ems_mask()), data);
         // add to RxQueue, what ever it is.
         rxservice_.add(data, length);
     }
@@ -670,7 +675,7 @@ void EMSESP::console_commands(Shell & shell, unsigned int context) {
 
     EMSESPShell::commands->add_command(
         ShellContext::EMS,
-        CommandFlags::USER,
+        CommandFlags::ADMIN,
         flash_string_vector{F_(set), F_(bus_id)},
         flash_string_vector{F_(deviceid_mandatory)},
         [](Shell & shell, const std::vector<std::string> & arguments) {
@@ -697,7 +702,7 @@ void EMSESP::console_commands(Shell & shell, unsigned int context) {
 
     EMSESPShell::commands->add_command(
         ShellContext::EMS,
-        CommandFlags::USER,
+        CommandFlags::ADMIN,
         flash_string_vector{F_(set), F_(tx_mode)},
         flash_string_vector{F_(n_mandatory)},
         [](Shell & shell, const std::vector<std::string> & arguments) {
@@ -720,7 +725,7 @@ void EMSESP::console_commands(Shell & shell, unsigned int context) {
 
     EMSESPShell::commands->add_command(
         ShellContext::EMS,
-        CommandFlags::USER,
+        CommandFlags::ADMIN,
         flash_string_vector{F_(set), F_(read_only)},
         flash_string_vector{F_(bool_mandatory)},
         [](Shell & shell, const std::vector<std::string> & arguments) {
@@ -811,8 +816,8 @@ void EMSESP::start() {
     };
 
     system_.start();
-    console_.start();
     network_.start();
+    console_.start();
     sensors_.start();
     rxservice_.start();
     txservice_.start();

src/roomcontrol.cpp

@@ -18,15 +18,12 @@
 
 #include "roomcontrol.h"
 
-MAKE_PSTR(logger_name, "roomctrl")
-
 namespace emsesp {
 
 uint32_t rc_time_      = 0;
 uint16_t hc_           = EMS_VALUE_USHORT_NOTSET;
-int16_t remotetemp[4] = {
+int16_t  remotetemp[4] = {EMS_VALUE_SHORT_NOTSET, EMS_VALUE_SHORT_NOTSET, EMS_VALUE_SHORT_NOTSET, EMS_VALUE_SHORT_NOTSET};
-    EMS_VALUE_SHORT_NOTSET, EMS_VALUE_SHORT_NOTSET, EMS_VALUE_SHORT_NOTSET, EMS_VALUE_SHORT_NOTSET
+
-};
 /**
  * set the temperature,
  */
@@ -47,8 +44,8 @@ void Roomctrl::send(uint8_t addr) {
     if (remotetemp[hc_] == EMS_VALUE_SHORT_NOTSET) {
         return;
     }
-    if (millis() - rc_time_ > 60000) { // send every minute
+    if (uuid::get_uptime() - rc_time_ > SEND_INTERVAL) { // send every minute
-        rc_time_ = millis();
+        rc_time_ = uuid::get_uptime();                   // use EMS-ESP's millis() to prevent overhead
         temperature(addr, 0x00);                         // send to all
     } else {
         // acknowledge every poll, otherwise the master shows error A11-822
@@ -59,16 +56,19 @@ void Roomctrl::send(uint8_t addr) {
 /**
  * check if there is a message for the remote room controller
  */
-void Roomctrl::check(uint8_t addr, uint8_t * data, const uint8_t length) {
+void Roomctrl::check(uint8_t addr, uint8_t * data) {
     uint8_t hc_num = addr - ADDR;
+
     // check address, reply only on addresses 0x18..0x1B
     if (hc_num > 3) {
         return;
     }
+
     // no reply if the temperature is not set
     if (remotetemp[hc_num] == EMS_VALUE_SHORT_NOTSET) {
         return;
     }
+
     // for now we only reply to version and remote temperature
     if (data[2] == 0x02) {
         version(addr, data[0]);

src/roomcontrol.h

@@ -16,8 +16,8 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#ifndef EMSESP_ROOMCTRL_H
+#ifndef EMSESP_ROOMCONTROL_H
-#define EMSESP_ROOMCTRL_H
+#define EMSESP_ROOMCONTROL_H
 
 #include "emsesp.h"
 #include "telegram.h"
@@ -29,15 +29,16 @@ namespace emsesp {
 class Roomctrl {
   public:
     static void send(uint8_t addr);
-    static void check(uint8_t addr, uint8_t * data, const uint8_t length);
+    static void check(uint8_t addr, uint8_t * data);
     static void set_remotetemp(uint8_t hc, int16_t temp);
 
   private:
-    #define ADDR 0x18
+    static constexpr uint8_t  ADDR          = 0x18;
+    static constexpr uint32_t SEND_INTERVAL = 60000; // 1 minute
+
     static void version(uint8_t addr, uint8_t dst);
     static void unknown(uint8_t addr, uint8_t dst, uint8_t type, uint8_t offset);
     static void temperature(uint8_t addr, uint8_t dst);
-
 };
 
 } // namespace emsesp

src/sensors.cpp

@@ -60,6 +60,7 @@ void Sensors::loop() {
         if (time_now - last_activity_ >= READ_INTERVAL_MS) {
             // LOG_DEBUG(F("Read sensor temperature")); // uncomment for debug
             if (bus_.reset()) {
+                yield();
                 bus_.skip();
                 bus_.write(CMD_CONVERT_TEMP);
 
@@ -151,18 +152,17 @@ float Sensors::get_temperature_c(const uint8_t addr[]) {
         LOG_ERROR(F("Bus reset failed before reading scratchpad from %s"), Device(addr).to_string().c_str());
         return NAN;
     }
-
+    yield();
     uint8_t scratchpad[SCRATCHPAD_LEN] = {0};
-
     bus_.select(addr);
     bus_.write(CMD_READ_SCRATCHPAD);
     bus_.read_bytes(scratchpad, SCRATCHPAD_LEN);
-
+    yield();
     if (!bus_.reset()) {
         LOG_ERROR(F("Bus reset failed after reading scratchpad from %s"), Device(addr).to_string().c_str());
         return NAN;
     }
-
+    yield();
     if (bus_.crc8(scratchpad, SCRATCHPAD_LEN - 1) != scratchpad[SCRATCHPAD_LEN - 1]) {
         LOG_WARNING(F("Invalid scratchpad CRC: %02X%02X%02X%02X%02X%02X%02X%02X%02X from device %s"),
                     scratchpad[0],

src/telegram.cpp

@@ -390,7 +390,7 @@ void TxService::send() {
 
     // if there's nothing in the queue to transmit, send back a poll and quit
     if (tx_telegrams_.empty()) {
-        //send_poll();
+        send_poll();
         return;
     }
 
@@ -489,6 +489,8 @@ void TxService::send_telegram(const QueuedTxTelegram & tx_telegram) {
 
     if (status != EMS_TX_STATUS_OK) {
         LOG_ERROR(F("Failed to transmit Tx via UART. Error: %s"), status == EMS_TX_WTD_TIMEOUT ? F("Timeout") : F("BRK"));
+        tx_waiting(false); // nothing send, tx not in wait state
+        return;
     }
 
     tx_waiting(true); // tx now in a wait state

src/uart/emsuart_esp32.cpp

@@ -32,9 +32,6 @@ static intr_handle_t   uart_handle;
 static RingbufHandle_t buf_handle   = NULL;
 static bool            drop_next_rx = true;
 static uint8_t         tx_mode_     = 0xFF;
-static uint32_t        emsRxTime;
-
-#define EMS_RX_TO_TX_TIMEOUT 20
 
 /*
 * Task to handle the incoming data
@@ -72,7 +69,6 @@ void IRAM_ATTR EMSuart::emsuart_rx_intr_handler(void * para) {
         if ((!drop_next_rx) && ((length == 2) || (length > 4))) {
             int baseType = 0;
             xRingbufferSendFromISR(buf_handle, rxbuf, length - 1, &baseType);
-            emsRxTime = millis();
         }
         drop_next_rx = false;
     }
@@ -139,9 +135,6 @@ void EMSuart::send_poll(uint8_t data) {
  * returns code, 1=success
  */
 EMSUART_STATUS EMSuart::transmit(uint8_t * buf, uint8_t len) {
-    if (millis() - emsRxTime > EMS_RX_TO_TX_TIMEOUT) {
-        return EMS_TX_WTD_TIMEOUT;
-    }
     if (len > 0) {
         for (uint8_t i = 0; i < len; i++) {
             EMS_UART.fifo.rw_byte = buf[i];

src/uart/emsuart_esp8266.cpp

@@ -24,8 +24,10 @@
 
 namespace emsesp {
 
-os_event_t recvTaskQueue[EMSUART_recvTaskQueueLen]; // our Rx queue
+MAKE_PSTR(logger_name, "emsuart")
 
+uuid::log::Logger     EMSuart::logger_{F_(logger_name), uuid::log::Facility::CONSOLE};
+os_event_t            recvTaskQueue[EMSUART_recvTaskQueueLen]; // our Rx queue
 EMSuart::EMSRxBuf_t * pEMSRxBuf;
 EMSuart::EMSRxBuf_t * paEMSRxBuf[EMS_MAXBUFFERS];
 uint8_t               emsRxBufIdx  = 0;
@@ -34,8 +36,6 @@ uint8_t               tx_mode_     = 0xFF;
 bool                  drop_next_rx = true;
 uint32_t              emsRxTime;
 
-#define EMS_RX_TO_TX_TIMEOUT 20
-
 //
 // Main interrupt handler
 // Important: must not use ICACHE_FLASH_ATTR
@@ -46,7 +46,7 @@ void ICACHE_RAM_ATTR EMSuart::emsuart_rx_intr_handler(void * para) {
 
     if (USIS(EMSUART_UART) & ((1 << UIBD))) { // BREAK detection = End of EMS data block
         length = 0;
-        while ((USS(EMSUART_UART) >> USRXC) & 0xFF) { // read fifo into buffer
+        while ((USS(EMSUART_UART) >> USRXC) & 0x0FF) { // read fifo into buffer
             uint8_t rx = USF(EMSUART_UART);
             if (length < EMS_MAXBUFFERSIZE) {
                 uart_buffer[length++] = rx;
@@ -59,7 +59,7 @@ void ICACHE_RAM_ATTR EMSuart::emsuart_rx_intr_handler(void * para) {
         if (!drop_next_rx) {
             pEMSRxBuf->length = length;
             os_memcpy((void *)pEMSRxBuf->buffer, (void *)&uart_buffer, pEMSRxBuf->length); // copy data into transfer buffer, including the BRK 0x00 at the end
-            emsRxTime = millis();
+            emsRxTime = uuid::get_uptime();
         }
         drop_next_rx = false;
         system_os_post(EMSUART_recvTaskPrio, 0, 0); // call emsuart_recvTask() at next opportunity
@@ -249,9 +249,12 @@ EMSUART_STATUS ICACHE_FLASH_ATTR EMSuart::transmit(uint8_t * buf, uint8_t len) {
     if (len == 0) {
         return EMS_TX_STATUS_OK; // nothing to send
     }
-    if (millis() > (emsRxTime + EMS_RX_TO_TX_TIMEOUT)) { // send allowed within 20 ms
+#ifdef EMSESP_DEBUG
-        return EMS_TX_WTD_TIMEOUT;
+    LOG_INFO(F("UART Responsetime: %d ms"),uuid::get_uptime() - emsRxTime);
-    }
+#endif
+    // if ((uuid::get_uptime() - emsRxTime) > EMS_RX_TO_TX_TIMEOUT)) { // send allowed within 20 ms
+    //      return EMS_TX_WTD_TIMEOUT;
+    // }
 
     // new code from Michael. See https://github.com/proddy/EMS-ESP/issues/380
     if (tx_mode_ == EMS_TXMODE_NEW) {
@@ -320,12 +323,11 @@ EMSUART_STATUS ICACHE_FLASH_ATTR EMSuart::transmit(uint8_t * buf, uint8_t len) {
      * 
      */
 
-    EMSUART_STATUS result = EMS_TX_STATUS_OK;
-
     // disable rx interrupt
     // clear Rx status register, resetting the Rx FIFO and flush it
     noInterrupts();
     // ETS_UART_INTR_DISABLE();
+    // USC0(EMSUART_UART) |= (1 << UCRXRST); // reset uart rx fifo
     emsuart_flush_fifos();
 
     // send the bytes along the serial line
@@ -354,7 +356,6 @@ EMSUART_STATUS ICACHE_FLASH_ATTR EMSuart::transmit(uint8_t * buf, uint8_t len) {
     // on Rx-BRK (bus collision), we simply enable Rx and leave it
     // otherwise we send the final Tx-BRK in the loopback and re=enable Rx-INT.
     // worst case, we'll see an additional Rx-BRK...
-    if (result == EMS_TX_STATUS_OK) {
     // neither bus collision nor timeout - send terminating BRK signal
     if (!(USIS(EMSUART_UART) & (1 << UIBD))) {
         // no bus collision - send terminating BRK signal
@@ -369,12 +370,11 @@ EMSUART_STATUS ICACHE_FLASH_ATTR EMSuart::transmit(uint8_t * buf, uint8_t len) {
         USIC(EMSUART_UART) = (1 << UIBD);                     // clear BRK detect IRQ
         phantomBreak       = 1;
     }
-    }
 
     interrupts();
     // ETS_UART_INTR_ENABLE(); // open up the FIFO again to start receiving
 
-    return result; // send the Tx status back
+    return EMS_TX_STATUS_OK; // send the Tx ok status back
 }
 
 } // namespace emsesp

src/uart/emsuart_esp8266.h

@@ -22,6 +22,7 @@
 #include <Arduino.h>
 
 #include <user_interface.h>
+#include <uuid/log.h>
 
 #define EMSUART_UART 0      // UART 0
 #define EMSUART_CONFIG 0x1C // 8N1 (8 bits, no stop bits, 1 parity)
@@ -74,9 +75,10 @@ class EMSuart {
     } EMSRxBuf_t;
 
   private:
+    // static constexpr uint32_t     EMS_RX_TO_TX_TIMEOUT = 20;
+    static uuid::log::Logger      logger_;
     static void ICACHE_RAM_ATTR   emsuart_rx_intr_handler(void * para);
     static void ICACHE_FLASH_ATTR emsuart_recvTask(os_event_t * events);
-
     static void ICACHE_FLASH_ATTR emsuart_flush_fifos();
     static void ICACHE_FLASH_ATTR tx_brk();
 };

src/version.h

@@ -1 +1 @@
-#define EMSESP_APP_VERSION "2.0.0a13"
+#define EMSESP_APP_VERSION "2.0.0a14"