changes

cube/User/Inc/mbusComm.h

@@ -17,6 +17,7 @@ typedef struct {
   int32_t period;
   int32_t delay;
   bool waiting;
+  bool active;
 } t_mbusDevice;
 
 typedef enum {
@@ -28,6 +29,7 @@ typedef enum {
 typedef struct {
   uint32_t mbusRequestCnt;
   uint32_t mbusErrorCnt;
+  uint32_t uartOctetCnt;
   uint32_t uartOverrunCnt;
   uint32_t uartFramingErrCnt;
   uint32_t uartParityErrCnt;

cube/User/Src/cmdHandler.c

@@ -44,7 +44,7 @@ typedef struct {
 
 // clear statistics
 bool clearCmd(uint8_t argc, char **args) {
-    t_mbusCommStats zeroedStats = { .mbusRequestCnt = 0, .mbusErrorCnt = 0, .uartOverrunCnt = 0, .uartFramingErrCnt = 0, .uartParityErrCnt = 0 };
+    t_mbusCommStats zeroedStats = { .mbusRequestCnt = 0, .mbusErrorCnt = 0, .uartOctetCnt = 0, .uartOverrunCnt = 0, .uartFramingErrCnt = 0, .uartParityErrCnt = 0 };
     mbusCommSetStats(zeroedStats);
     coloredMsg(LOG_YELLOW, true, "ch cc global statistics cleared");
     return true;
@@ -57,11 +57,12 @@ bool globalStatsCmd(uint8_t argc, char **args) {
       "Global statistics\n\r" \
       "  Meterbus Requests: %ld\n\r" \
       "  Meterbus Errors:   %ld\n\r" \
+      "  UART Octets:       %ld\n\r" \
       "  UART Overruns:     %ld\n\r" \
       "  UART Framing Errs: %ld\n\r" \
       "  UART Parity Errs:  %ld\n\r",
       stats->mbusRequestCnt, stats->mbusErrorCnt,
-      stats->uartOverrunCnt, stats->uartFramingErrCnt, stats->uartParityErrCnt
+      stats->uartOctetCnt, stats->uartOverrunCnt, stats->uartFramingErrCnt, stats->uartParityErrCnt
     );
     send(CMD_SOCK, (uint8_t*)buf, strlen(buf));
     return true;

cube/User/Src/main2.c

@@ -64,7 +64,7 @@ void my_setup_2() {
 }
 
 void my_loop() {
-    show(DEBUG_2, TOGGLE);
+    // show(DEBUG_2, TOGGLE);
 
     schExec();
 

cube/User/Src/mbusComm.c

@@ -18,7 +18,6 @@
 #include <oled.h>
 #include <ringbuffer.h>
 
-
 #include <mbus/mbus-protocol.h>
 
 static const char MBUS_TOPIC[] = "IoT/MBGW3/Measurement";
@@ -46,6 +45,7 @@ typedef enum {
     MBCS_IDLE,
     MBCS_SEND,
     MBCS_SEND_CONTINUED,
+    MBCS_SENDING,
     MBCS_SENDING_DONE,
     MBCS_ENABLE_FRONTEND,
     MBCS_START1,
@@ -105,7 +105,7 @@ typedef struct {
 
 static t_mbusCommHandle mbusCommHandle = { .requestId = 0, .state = MBCS_IDLE, .retryCnt = 0, .cmd = 0, .addr = 0, .startTime = 0, .receiveCnt = 0, .waitForOctet = false };
 
-static t_mbusCommStats mbusCommStats = { .mbusRequestCnt = 0, .mbusErrorCnt = 0, .uartOverrunCnt = 0, .uartFramingErrCnt = 0, .uartParityErrCnt = 0 };
+static t_mbusCommStats mbusCommStats = { .mbusRequestCnt = 0, .mbusErrorCnt = 0, .uartOctetCnt = 0, .uartOverrunCnt = 0, .uartFramingErrCnt = 0, .uartParityErrCnt = 0 };
 
 static bool mbusCommEnabled = true;
 
@@ -157,7 +157,9 @@ static void parseAndPrintFrame() {
     int r = mbus_frame_data_parse(&reply, &frame_data);
     if (r == 0) {
         mbus_data_variable *data_var = &(frame_data.data_var);
-        coloredMsg(LOG_YELLOW, false, "mbc papf [%d] sts: %02x", mbusCommHandle.requestId, data_var->header.status);
+        if (data_var->header.status) {
+            coloredMsg(LOG_RED, true, "mbc papf [%d] sts: %02x", mbusCommHandle.requestId, data_var->header.status);
+        }
         if ((data_var->header.status & 0x01)) {
             coloredMsg(LOG_RED, true, "mbc papf [%d] sts: Application Busy", mbusCommHandle.requestId);
         }
@@ -193,9 +195,9 @@ static void parseAndPrintFrame() {
             for (uint8_t j = 0; j < MBUSDEVICE_NUM_OF_CONSIDEREDFIELDS; j++) {
                 if (mbusCommHandle.device->consideredField[j] == i) {
                     parsedVIB_t parsedVIB = parseVIB(record->drh.vib);
-                    coloredMsg(LOG_YELLOW, false, "mbc papf [%d] parsed VIB N: %s, U: %s, E: %d",
-                               mbusCommHandle.requestId,
-                               parsedVIB.name, parsedVIB.unit, parsedVIB.exponent);
+                    // coloredMsg(LOG_YELLOW, false, "mbc papf [%d] parsed VIB N: %s, U: %s, E: %d",
+                    //            mbusCommHandle.requestId,
+                    //            parsedVIB.name, parsedVIB.unit, parsedVIB.exponent);
                     if (parsedVIB.found) {
                         uint32_t value = strtol(mbus_data_record_value(record), NULL, 10);
                         float weightedValue = ((float) value) * powf(10.0, ((float) parsedVIB.exponent));
@@ -259,28 +261,51 @@ void mbusCommISR() {
     show(DEBUG_1, TOGGLE);
 
     uint32_t isrflags   = READ_REG(mbusUart.Instance->SR);
+    uint32_t cr1its     = READ_REG(mbusUart.Instance->CR1);
 
-    if (((isrflags & USART_SR_RXNE) != RESET) || ((isrflags & USART_SR_ORE) != RESET)) {
+    // RXNEIE doesn't need to be considered since it is always on and more over the
+    // RXNE flag is cleared by reading the DR, which is done in any case
+    if (((isrflags & USART_SR_RXNE) != RESET) || ((isrflags & (USART_SR_ORE | USART_SR_FE | USART_SR_PE)) != RESET)) {
+        // Error flags are only valid together with the RX flag.
+        // They will be cleared by reading SR (already done above) followed by reading DR (below).
+        bool errorFound = false;
         if ((isrflags & USART_SR_ORE) != RESET) {
+            show(DEBUG_2, TOGGLE); 
             mbusCommStats.uartOverrunCnt += 1;
+            errorFound = true;
         }
+        if ((isrflags & USART_SR_FE) != RESET) {
+            mbusCommStats.uartFramingErrCnt += 1;
+            errorFound = true;
+        }
+        if ((isrflags & USART_SR_PE) != RESET) {
+            mbusCommStats.uartParityErrCnt += 1;
+            errorFound = true;
+        }
+        mbusCommStats.uartOctetCnt += 1;
         // it is required to read the DR in any case here, not only when the buffer has space
         // otherwise the interrupt flag won't be disabled, particularly important in case of
         // ORE
         uint8_t data = (uint8_t)(mbusUart.Instance->DR & (uint8_t)0x00FF);
-        if (mbusCommHandle.receiveBuffer.writeIdx < mbusCommHandle.receiveBuffer.size) {
+        if ((! errorFound) &&
+            (mbusCommHandle.receiveBuffer.writeIdx < mbusCommHandle.receiveBuffer.size)) {
             mbusCommHandle.receiveBuffer.buffer[mbusCommHandle.receiveBuffer.writeIdx] = data;
             mbusCommHandle.receiveBuffer.writeIdx += 1;
         }
-        return;
-    } else {
-        if ((isrflags & USART_SR_FE) != RESET) {
-            mbusCommStats.uartFramingErrCnt += 1;
+    } 
+    
+    // TXEIE needs to be considered since TXE is cleared by writing the DR, which isn't done
+    // after the last octet sent
+    if (((isrflags & USART_SR_TXE) != RESET) && ((cr1its & USART_CR1_TXEIE) != RESET)) {
+        if (mbusCommHandle.sendBuffer.readIdx < mbusCommHandle.sendBuffer.writeIdx) {
+            mbusUart.Instance->DR = mbusCommHandle.sendBuffer.buffer[mbusCommHandle.sendBuffer.readIdx];
+            mbusCommHandle.sendBuffer.readIdx += 1;
+            if (mbusCommHandle.sendBuffer.readIdx == mbusCommHandle.sendBuffer.writeIdx) {
+                __HAL_UART_DISABLE_IT(&mbusUart, UART_IT_TXE);
+                mbusCommHandle.state = MBCS_SENDING_DONE;
+            }
         }
-        if ((isrflags & USART_SR_PE) != RESET) {
-            mbusCommStats.uartParityErrCnt += 1;
-        }
-    }
+    } 
 }
 
 
@@ -290,12 +315,10 @@ void mbusCommExec() {
     uint8_t receivedOctet = 0;
 
     if ((mbusCommHandle.startTime != 0) && ((mbusCommHandle.startTime + 2500) < HAL_GetTick())) {
-        coloredMsg(LOG_RED, false, "TIMEOUT!! %d %ld %ld", mbusCommHandle.state, mbusCommHandle.startTime, HAL_GetTick());
-        mbusCommHandle.startTime = 0;
         mbusCommHandle.state = MBCS_TIMEOUT;
     } else if (mbusCommHandle.waitForOctet) {
         if (mbusCommHandle.receiveBuffer.readIdx >= mbusCommHandle.receiveBuffer.writeIdx) {
-            return;
+            return; // no data available, wait
         }
         receivedOctet = mbusCommHandle.receiveBuffer.buffer[mbusCommHandle.receiveBuffer.readIdx];
         mbusCommHandle.receiveBuffer.readIdx += 1;
@@ -308,7 +331,7 @@ void mbusCommExec() {
         break;
 
         case MBCS_SEND:
-        coloredMsg(LOG_YELLOW, false, "mbc hre [%d] state SEND", mbusCommHandle.requestId);
+        // coloredMsg(LOG_YELLOW, false, "mbc hre [%d] state SEND", mbusCommHandle.requestId);
         mbusCommHandle.sendBuffer.buffer[0] = 0x10;
         mbusCommHandle.sendBuffer.buffer[1] = mbusCommHandle.cmd;
         mbusCommHandle.sendBuffer.buffer[2] = mbusCommHandle.addr;
@@ -320,31 +343,32 @@ void mbusCommExec() {
         // no break !!
 
         case MBCS_SEND_CONTINUED:
-        coloredMsg(LOG_YELLOW, false, "mbc hre [%d] state SEND_CONTINUED", mbusCommHandle.requestId);
+        // coloredMsg(LOG_YELLOW, false, "mbc hre [%d] state SEND_CONTINUED", mbusCommHandle.requestId);
         show(LED_RED, OFF);
         if (! loopActive) {
             coloredMsg(LOG_YELLOW, true, "mbc hre [%d] enabling loop, try %d", mbusCommHandle.requestId, mbusCommHandle.retryCnt);
             mbusCommHandle.retryCnt++;
             loopEnable();
-            // FIXME somehow manage to delay for about 100ms
         } else {
             mbusCommHandle.retryCnt = 0;
-            // FIXME sending possibly needs to be done in a different way
-            HAL_UART_Transmit(&mbusUart, mbusCommHandle.sendBuffer.buffer, 5, HAL_MAX_DELAY);
-            // transition from here to SENDING_DONE is initiate by mbusCommTxCpltCallback
-            // interrupt callback
-            mbusCommHandle.state = MBCS_SENDING_DONE;
+            // enable transmitter interrupt
+            //coloredMsg(LOG_YELLOW, false, "mbc hre [%d] enable transmitter interrupt", mbusCommHandle.requestId);
+            __HAL_UART_ENABLE_IT(&mbusUart, UART_IT_TXE);
+            mbusCommHandle.state = MBCS_SENDING;
         }
         break;
 
+        case MBCS_SENDING:
+        // transition from here to MBCS_SENDING_DONE is done by TX ISR
+        break;
+
         case MBCS_SENDING_DONE:
-        coloredMsg(LOG_YELLOW, false, "mbc hre [%d] state SENDING_DONE", mbusCommHandle.requestId);
+        //coloredMsg(LOG_YELLOW, false, "mbc hre [%d] state SENDING_DONE", mbusCommHandle.requestId);
         mbusCommHandle.state = MBCS_ENABLE_FRONTEND;
-        // FIXME somehow manage to delay for about 3ms
         break;
 
         case MBCS_ENABLE_FRONTEND:
-        coloredMsg(LOG_YELLOW, false, "mbc hre [%d] state ENABLE_FRONTEND", mbusCommHandle.requestId);
+        // coloredMsg(LOG_YELLOW, false, "mbc hre [%d] state ENABLE_FRONTEND", mbusCommHandle.requestId);
         frontendEnable();
         calculatedChksum = 0;
         userdataIdx = 0;
@@ -480,17 +504,20 @@ void mbusCommExec() {
         break;
 
         case MBCS_ERROR:
-        coloredMsg(LOG_RED, false, "mbc hre [%d] state ERROR", mbusCommHandle.requestId);
+        // coloredMsg(LOG_RED, false, "mbc hre [%d] state ERROR", mbusCommHandle.requestId);
+        coloredMsg(LOG_RED, true, "mbc hre [%d] error", mbusCommHandle.requestId);
         show(LED_RED, ON);
         mbusCommHandle.state = MBCS_ERROR_CONTINUED;
         // no break
 
         case MBCS_ERROR_CONTINUED:
-        // stay here until timeout occurs
+        // every error will be collected by a timeout to receive all data still on the wire
+        // to avoid leaking old data in responses for new requests
         break;
 
         case MBCS_TIMEOUT:
-        coloredMsg(LOG_RED, false, "mbc hre [%d] state TIMEOUT", mbusCommHandle.requestId);
+        // coloredMsg(LOG_RED, false, "mbc hre [%d] state TIMEOUT", mbusCommHandle.requestId);
+        coloredMsg(LOG_RED, true, "mbc hre [%d] timeout", mbusCommHandle.requestId);
         mbusCommStats.mbusErrorCnt += 1;
         mbusCommHandle.device->failures += 1;
         mbusCommHandle.startTime = 0; // disable timeout
@@ -562,7 +589,8 @@ static t_mbusDevice devices[] = {
         .failures = 0,
         .period = PERIOD,
         .delay = 0,
-        .waiting = false
+        .waiting = false,
+        .active = true
     },
     {
         .deviceName = "ComputerPower",
@@ -572,7 +600,8 @@ static t_mbusDevice devices[] = {
         .failures = 0,
         .period = PERIOD,
         .delay = 0,
-        .waiting = false
+        .waiting = false,
+        .active = true
     },
     {
         .deviceName = "DryerPower",
@@ -582,7 +611,8 @@ static t_mbusDevice devices[] = {
         .failures = 0,
         .period = PERIOD,
         .delay = 0,
-        .waiting = false
+        .waiting = false,
+        .active = true
     },
     {
         .deviceName = "LaundryPower",
@@ -592,7 +622,8 @@ static t_mbusDevice devices[] = {
         .failures = 0,
         .period = PERIOD,
         .delay = 0,
-        .waiting = false
+        .waiting = false,
+        .active = true
     },
     {
         .deviceName = "DishwasherPower",
@@ -602,7 +633,8 @@ static t_mbusDevice devices[] = {
         .failures = 0,
         .period = PERIOD,
         .delay = 0,
-        .waiting = false
+        .waiting = false,
+        .active = true
     },
     {
         .deviceName = "LightPower",
@@ -612,7 +644,8 @@ static t_mbusDevice devices[] = {
         .failures = 0,
         .period = PERIOD,
         .delay = 0,
-        .waiting = false
+        .waiting = false,
+        .active = true
     },
     {
         .deviceName = "FreezerPower",
@@ -622,7 +655,8 @@ static t_mbusDevice devices[] = {
         .failures = 0,
         .period = PERIOD,
         .delay = 0,
-        .waiting = false
+        .waiting = false,
+        .active = true
     },
     {
         .deviceName = "FridgePower",
@@ -632,7 +666,8 @@ static t_mbusDevice devices[] = {
         .failures = 0,
         .period = PERIOD,
         .delay = 0,
-        .waiting = false
+        .waiting = false,
+        .active = true
     }
 };
 
@@ -689,11 +724,13 @@ static void mbusCommScheduler(void *handle) {
                 state = 1;
             }
             for (uint8_t i = 0; i < numOfDevices; i++) {
-                devices[i].delay -= 1;
-                if (devices[i].delay <= 0) {
-                    devices[i].delay = devices[i].period;
-                    devices[i].waiting = true;
-                    coloredMsg(LOG_YELLOW, false, "mbc mcs scheduled: %s", devices[i].deviceName);
+                if (devices[i].active) {
+                    devices[i].delay -= 1;
+                    if (devices[i].delay <= 0) {
+                        devices[i].delay = devices[i].period;
+                        devices[i].waiting = true;
+                        coloredMsg(LOG_YELLOW, false, "mbc mcs scheduled: %s", devices[i].deviceName);
+                    }
                 }
             }
 
@@ -715,10 +752,11 @@ void mbusCommInit() {
     coloredMsg(LOG_GREEN, true, "mbc mci initializing Meterbus communication");
 
     // enable receive interrupts
-    __HAL_UART_ENABLE_IT(&mbusUart, UART_IT_PE);
-    __HAL_UART_ENABLE_IT(&mbusUart, UART_IT_ERR);
+//    __HAL_UART_ENABLE_IT(&mbusUart, UART_IT_PE);
+//    __HAL_UART_ENABLE_IT(&mbusUart, UART_IT_ERR);
     __HAL_UART_ENABLE_IT(&mbusUart, UART_IT_RXNE);
 
+    // init buffers
     mbusCommHandle.receiveBuffer.size = 256;
     mbusCommHandle.receiveBuffer.buffer = (uint8_t*) malloc(mbusCommHandle.receiveBuffer.size);
     mbusCommHandle.receiveBuffer.readIdx = 0;

cube/User/Src/mqttComm.c

@@ -58,9 +58,9 @@ static void mqttStatusPublisher(void *handle) {
 
     char buf[256];
     uint32_t uptime = HAL_GetTick() / 1000;
-    snprintf(buf, sizeof(buf), "{\"uptime\":\"%ld\", \"runningHours\":\"%ld\", \"powerCycles\":\"%ld\", \"tasks\":\"%d\", \"requests\":\"%ld\", \"errors\":\"%ld\", \"overrun\":\"%ld\", \"framing\":\"%ld\", \"parity\":\"%ld\"}", 
+    snprintf(buf, sizeof(buf), "{\"uptime\":\"%ld\", \"runningHours\":\"%ld\", \"powerCycles\":\"%ld\", \"tasks\":\"%d\", \"requests\":\"%ld\", \"errors\":\"%ld\", \"octets\":\"%ld\", \"overrun\":\"%ld\", \"framing\":\"%ld\", \"parity\":\"%ld\"}", 
             uptime, globalDeviceStats->totalRunningHours, globalDeviceStats->totalPowercycles, schTaskCnt(), mbusCommStats->mbusRequestCnt, mbusCommStats->mbusErrorCnt, 
-            mbusCommStats->uartOverrunCnt, mbusCommStats->uartFramingErrCnt, mbusCommStats->uartParityErrCnt);
+            mbusCommStats->uartOctetCnt, mbusCommStats->uartOverrunCnt, mbusCommStats->uartFramingErrCnt, mbusCommStats->uartParityErrCnt);
     bool res = publish(&mqttClient, StatusTopic, (const uint8_t*)buf, strlen(buf), false);
     coloredMsg(LOG_GREEN, false, "mqch, publish returned %d", res);