optimize Tx to handle phantom brk

src/ems.cpp

@@ -682,25 +682,11 @@ void _createValidate() {
 
 /**
  * Entry point triggered by an interrupt in emsuart.cpp
- * length is size of all the telegram bytes including the CRC, excluding the BRK at the end
+ * length is the number of all the telegram bytes up to and including the CRC at the end
  * Read commands are asynchronous as they're handled by the interrupt
  * When a telegram is processed we forcefully erase it from the stack to prevent overflow
  */
 void ems_parseTelegram(uint8_t * telegram, uint8_t length) {
-    if ((length != 0) && (telegram[0] != 0x00)) {
-        _ems_readTelegram(telegram, length);
-    }
-
-    // clear the Rx buffer just be safe and prevent duplicates
-    memset(telegram, 0, EMS_MAXBUFFERSIZE);
-}
-
-/**
- * the main logic that parses the telegram message
- * When we receive a Poll Request we need to send any Tx packages quickly within a 200ms window
- * length is total number of bytes of the telegram including the CRC byte at the end (if it exists)
- */
-void _ems_readTelegram(uint8_t * telegram, uint8_t length) {
     // create the Rx package
     static _EMS_RxTelegram EMS_RxTelegram;
     static uint32_t        _last_emsPollFrequency = 0;
@@ -980,7 +966,7 @@ void _removeTxQueue() {
 void _processType(_EMS_RxTelegram * EMS_RxTelegram) {
     uint8_t * telegram = EMS_RxTelegram->telegram;
 
-    // if its an echo of ourselves from the master UBA, ignore
+    // if its an echo of ourselves from the master UBA, ignore. This should never happen mind you
     if (EMS_RxTelegram->src == EMS_ID_ME) {
         // _debugPrintTelegram("echo:", EMS_RxTelegram, COLOR_WHITE);
         return;
@@ -1674,11 +1660,7 @@ void _process_Version(_EMS_RxTelegram * EMS_RxTelegram) {
     }
 
     if (typeFound) {
-        myDebug_P(PSTR("Device found: %s with DeviceID 0x%02X, ProductID %d, Version %s"),
+        myDebug_P(PSTR("Device found: %s (DeviceID:0x%02X ProductID:%d Version:%s)"), Other_Types[i].model_string, Other_Types[i].device_id, product_id, version);
-                  Other_Types[i].model_string,
-                  Other_Types[i].device_id,
-                  product_id,
-                  version);
 
         // add to list
         _addDevice(product_id, Other_Types[i].device_id, version, Other_Types[i].model_string);
@@ -1700,7 +1682,7 @@ void _process_Version(_EMS_RxTelegram * EMS_RxTelegram) {
         return;
 
     } else {
-        myDebug_P(PSTR("Unrecognized device found: DeviceID 0x%02X, ProductID %d, Version %s"), EMS_RxTelegram->src, product_id, version);
+        myDebug_P(PSTR("Unrecognized device found: %s (DeviceID:0x%02X ProductID:%d Version:%s)"), EMS_RxTelegram->src, product_id, version);
 
         // add to list
         _addDevice(product_id, EMS_RxTelegram->src, version, "unknown?");
@@ -1767,6 +1749,8 @@ void ems_printTxQueue() {
             strlcpy(sType, "read", sizeof(sType));
         } else if (EMS_TxTelegram.action == EMS_TX_TELEGRAM_VALIDATE) {
             strlcpy(sType, "validate", sizeof(sType));
+        } else if (EMS_TxTelegram.action == EMS_TX_TELEGRAM_RAW) {
+            strlcpy(sType, "raw", sizeof(sType));
         } else {
             strlcpy(sType, "?", sizeof(sType));
         }
@@ -2501,7 +2485,7 @@ void ems_testTelegram(uint8_t test_num) {
     myDebug_P(PSTR("[TEST %d] Injecting telegram %s"), test_num, TEST_DATA[test_num - 1]);
 
     // go an parse it
-    _ems_readTelegram(telegram, length + 1); // include CRC in length
+    ems_parseTelegram(telegram, length + 1); // include CRC in length
 #else
     myDebug_P(PSTR("Firmware not compiled with test data set"));
 #endif

src/ems.h

@@ -298,6 +298,14 @@ extern void ems_parseTelegram(uint8_t * telegram, uint8_t len);
 void        ems_init();
 void        ems_doReadCommand(uint16_t type, uint8_t dest, bool forceRefresh = false);
 void        ems_sendRawTelegram(char * telegram);
+void        ems_scanDevices();
+void        ems_printAllDevices();
+void        ems_printDevices();
+void        ems_printTxQueue();
+void        ems_testTelegram(uint8_t test_num);
+void        ems_startupTelegrams();
+bool        ems_checkEMSBUSAlive();
+void        ems_clearDeviceList();
 
 void ems_setThermostatTemp(float temperature, uint8_t temptype = 0);
 void ems_setThermostatMode(uint8_t mode);
@@ -311,11 +319,11 @@ void ems_setTxMode(uint8_t mode);
 void ems_setLogging(_EMS_SYS_LOGGING loglevel);
 void ems_setEmsRefreshed(bool b);
 void ems_setWarmWaterModeComfort(uint8_t comfort);
-bool ems_checkEMSBUSAlive();
 void ems_setModels();
 void ems_setTxDisabled(bool b);
-void ems_clearDeviceList();
 
+char *           ems_getThermostatDescription(char * buffer);
+char *           ems_getBoilerDescription(char * buffer);
 void             ems_getThermostatValues();
 void             ems_getBoilerValues();
 void             ems_getOtherValues();
@@ -332,15 +340,6 @@ void             ems_discoverModels();
 bool             ems_getTxCapable();
 uint32_t         ems_getPollFrequency();
 
-void   ems_scanDevices();
-void   ems_printAllDevices();
-void   ems_printDevices();
-char * ems_getThermostatDescription(char * buffer);
-void   ems_printTxQueue();
-char * ems_getBoilerDescription(char * buffer);
-void   ems_testTelegram(uint8_t test_num);
-void   ems_startupTelegrams();
-
 // private functions
 uint8_t _crcCalculator(uint8_t * data, uint8_t len);
 void    _processType(_EMS_RxTelegram * EMS_RxTelegram);
@@ -349,7 +348,6 @@ void    _ems_clearTxData();
 int     _ems_findBoilerModel(uint8_t model_id);
 bool    _ems_setModel(uint8_t model_id);
 void    _removeTxQueue();
-void    _ems_readTelegram(uint8_t * telegram, uint8_t length);
 
 // global so can referenced in other classes
 extern _EMS_Sys_Status EMS_Sys_Status;

src/emsuart.cpp

@@ -7,16 +7,12 @@
 
 #include "emsuart.h"
 #include "ems.h"
-#include "ets_sys.h"
-#include "osapi.h"
-
 #include <Arduino.h>
 #include <user_interface.h>
 
 _EMSRxBuf * pEMSRxBuf;
 _EMSRxBuf * paEMSRxBuf[EMS_MAXBUFFERS];
 uint8_t     emsRxBufIdx = 0;
-uint8_t     phantomBrk  = 0; // tells Rx about upcoming phantom break from Tx
 
 os_event_t recvTaskQueue[EMSUART_recvTaskQueueLen]; // our Rx queue
 
@@ -35,39 +31,28 @@ static void emsuart_rx_intr_handler(void * para) {
     }
 
     // fill IRQ buffer, by emptying Rx FIFO
-    if (U0IS & ((1 << UIFF) | (1 << UITO) | (1 << UIBD))) {
+    if (USIS(EMSUART_UART) & ((1 << UIFF) | (1 << UITO) | (1 << UIBD))) {
         while ((USS(EMSUART_UART) >> USRXC) & 0xFF) {
             uart_buffer[length++] = USF(EMSUART_UART);
         }
 
         // clear Rx FIFO full and Rx FIFO timeout interrupts
-        U0IC = (1 << UIFF) | (1 << UITO);
+        USIC(EMSUART_UART) = (1 << UIFF) | (1 << UITO);
     }
 
     // BREAK detection = End of EMS data block
     if (USIS(EMSUART_UART) & ((1 << UIBD))) {
         ETS_UART_INTR_DISABLE(); // disable all interrupts and clear them
 
-        U0IC = (1 << UIBD); // INT clear the BREAK detect interrupt
+        USIC(EMSUART_UART) = (1 << UIBD); // INT clear the BREAK detect interrupt
 
-        // we've send a BRK with loopback enabled, so we must compensate the phantom break
+        pEMSRxBuf->length = length;
-        if (phantomBrk) {
+        os_memcpy((void *)pEMSRxBuf->buffer, (void *)&uart_buffer, length); // copy data into transfer buffer, including the BRK 0x00 at the end
-            length--;       // compensate buffer length
+        EMS_Sys_Status.emsRxStatus = EMS_RX_STATUS_IDLE;                    // set the status flag stating BRK has been received and we can start a new package
-            phantomBrk = 0; // clear flag
-        }
 
-        // copy data into transfer buffer
+        system_os_post(EMSUART_recvTaskPrio, 0, 0); // call emsuart_recvTask() at next opportunity
-        pEMSRxBuf->writePtr = length;
-        os_memcpy((void *)pEMSRxBuf->buffer, (void *)&uart_buffer, length);
 
-        // set the status flag stating BRK has been received and we can start a new package
+        ETS_UART_INTR_ENABLE(); // re-enable UART interrupts
-        EMS_Sys_Status.emsRxStatus = EMS_RX_STATUS_IDLE;
-
-        // call emsuart_recvTask() at next opportunity
-        system_os_post(EMSUART_recvTaskPrio, 0, 0);
-
-        // re-enable UART interrupts
-        ETS_UART_INTR_ENABLE();
     }
 }
 
@@ -78,8 +63,21 @@ static void emsuart_rx_intr_handler(void * para) {
  */
 static void ICACHE_FLASH_ATTR emsuart_recvTask(os_event_t * events) {
     _EMSRxBuf * pCurrent = pEMSRxBuf;
-    ems_parseTelegram((uint8_t *)pCurrent->buffer, (pCurrent->writePtr) - 1); // transmit EMS buffer, excluding the BRK
+    uint8_t     length   = pCurrent->length; // number of bytes including the BRK at the end
-    pEMSRxBuf = paEMSRxBuf[++emsRxBufIdx % EMS_MAXBUFFERS];                   // next free EMS Receive buffer
+
+    // validate and transmit the EMS buffer, excluding the BRK
+    if (length == 2) {
+        // it's a poll or status code, single byte
+        ems_parseTelegram((uint8_t *)pCurrent->buffer, 1);
+    } else if ((length > 4) && (pCurrent->buffer[length - 2] != 0x00)) {
+        // ignore double BRK at the end, possibly from the Tx loopback
+        // also telegrams with no data value
+        ems_parseTelegram((uint8_t *)pCurrent->buffer, length - 1); // transmit EMS buffer, excluding the BRK
+    }
+
+    memset(pCurrent->buffer, 0x00, EMS_MAXBUFFERSIZE); // wipe memory just to be safe
+
+    pEMSRxBuf = paEMSRxBuf[++emsRxBufIdx % EMS_MAXBUFFERS]; // next free EMS Receive buffer
 }
 
 /*
@@ -119,8 +117,8 @@ void ICACHE_FLASH_ATTR emsuart_init() {
 
     // conf1 params
     // UCTOE = RX TimeOut enable (default is 1)
-    // UCTOT = RX TimeOut Threshold (7bit) = want this when no more data after 2 characters. (default is 2)
+    // UCTOT = RX TimeOut Threshold (7 bit) = want this when no more data after 2 characters (default is 2)
-    // UCFFT = RX FIFO Full Threshold (7 bit) = want this to be 31 for 32 bytes of buffer. (default was 127).
+    // UCFFT = RX FIFO Full Threshold (7 bit) = want this to be 31 for 32 bytes of buffer (default was 127)
     // see https://www.espressif.com/sites/default/files/documentation/esp8266-technical_reference_en.pdf
     USC1(EMSUART_UART) = 0;                                                                   // reset config first
     USC1(EMSUART_UART) = (EMS_MAX_TELEGRAM_LENGTH << UCFFT) | (0x02 << UCTOT) | (1 << UCTOE); // enable interupts
@@ -130,14 +128,14 @@ void ICACHE_FLASH_ATTR emsuart_init() {
     USIE(EMSUART_UART) = 0;      // disable all interrupts
 
     // enable rx break, fifo full and timeout.
-    // not frame error UIFR (because they are too frequent) or overflow UIOF because our buffer is only max 32 bytes
+    // but not frame error UIFR (because they are too frequent) or overflow UIOF because our buffer is only max 32 bytes
     USIE(EMSUART_UART) = (1 << UIBD) | (1 << UIFF) | (1 << UITO);
 
     // set up interrupt callbacks for Rx
     system_os_task(emsuart_recvTask, EMSUART_recvTaskPrio, recvTaskQueue, EMSUART_recvTaskQueueLen);
 
-    // disable esp debug which will go to Tx and mess up the line
+    // disable esp debug which will go to Tx and mess up the line - see https://github.com/espruino/Espruino/issues/655
-    system_set_os_print(0); // https://github.com/espruino/Espruino/issues/655
+    system_set_os_print(0);
 
 // swap Rx and Tx pins to use GPIO13 (D7) and GPIO15 (D8) respectively
 #ifndef NO_UART_SWAP
@@ -206,6 +204,9 @@ static inline void ICACHE_FLASH_ATTR emsuart_loopback(bool enable) {
  * Send to Tx, ending with a <BRK>
  */
 void ICACHE_FLASH_ATTR emsuart_tx_buffer(uint8_t * buf, uint8_t len) {
+    if (len == 0)
+        return;
+
     if (EMS_Sys_Status.emsTxMode == 0) { // classic mode logic
         for (uint8_t i = 0; i < len; i++) {
             USF(EMSUART_UART) = buf[i];
@@ -258,8 +259,6 @@ void ICACHE_FLASH_ATTR emsuart_tx_buffer(uint8_t * buf, uint8_t len) {
         // 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 (!(USIS(EMSUART_UART) & (1 << UIBD))) {
-            phantomBrk = 1; // tell Rx to expect a phantom BRK
-
             // no bus collision - send terminating BRK signal
             emsuart_loopback(true);
             USC0(EMSUART_UART) |= (1 << UCBRK); // set <BRK>
@@ -268,13 +267,13 @@ void ICACHE_FLASH_ATTR emsuart_tx_buffer(uint8_t * buf, uint8_t len) {
             while (!(USIS(EMSUART_UART) & (1 << UIBD))) {
                 delayMicroseconds(EMSUART_BIT_TIME);
             }
+
             USC0(EMSUART_UART) &= ~(1 << UCBRK); // clear <BRK>
 
             USIC(EMSUART_UART) = (1 << UIBD); // clear BRK detect IRQ
             emsuart_loopback(false);          // disable loopback mode
         }
 
-        // w/o flushing the Rx FIFO we will get a trailing \0 from our loopback BRK
         ETS_UART_INTR_ENABLE(); // receive anything from FIFO...
     }
 }

src/emsuart.h

@@ -27,7 +27,7 @@
 #define EMSUART_recvTaskQueueLen 64
 
 typedef struct {
-    uint8_t writePtr;
+    uint8_t length;
     uint8_t buffer[EMS_MAXBUFFERSIZE];
 } _EMSRxBuf;