Simplify implementation of TLayer4 buffer state handling

selfbus · Mar 11, 2024 · 60221e8 · 60221e8
1 parent fafe6db
commit 60221e8
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 37 deletions.
diff --git a/sblib/inc/sblib/eib/knx_tlayer4.h b/sblib/inc/sblib/eib/knx_tlayer4.h
@@ -314,17 +314,10 @@ class TLayer4
         TELEGRAM_ACQUIRED,
         TELEGRAM_SENDING
     };
-    enum SendConnectedTelegramBufferState
-    {
-        CONNECTED_TELEGRAM_FREE,
-        CONNECTED_TELEGRAM_WAIT_T_ACK_SENT,
-        CONNECTED_TELEGRAM_WAIT_LOOP,
-        CONNECTED_TELEGRAM_SENDING
-    };
 
     volatile SendTelegramBufferState sendTelegramBufferState;
-    volatile SendConnectedTelegramBufferState sendConnectedTelegramBufferState;
-    volatile SendConnectedTelegramBufferState sendConnectedTelegramBuffer2State;
+    volatile SendTelegramBufferState sendConnectedTelegramBufferState;
+    volatile SendTelegramBufferState sendConnectedTelegramBuffer2State;
 };
 
 inline bool TLayer4::directConnection()

diff --git a/sblib/src/eib/knx_tlayer4.cpp b/sblib/src/eib/knx_tlayer4.cpp
@@ -250,8 +250,8 @@ void TLayer4::_begin()
 #endif
     state = TLayer4::CLOSED;
     sendTelegramBufferState = TELEGRAM_FREE;
-    sendConnectedTelegramBufferState = CONNECTED_TELEGRAM_FREE;
-    sendConnectedTelegramBuffer2State = CONNECTED_TELEGRAM_FREE;
+    sendConnectedTelegramBufferState = TELEGRAM_FREE;
+    sendConnectedTelegramBuffer2State = TELEGRAM_FREE;
     connectedAddr = 0;
     seqNoSend = -1;
     seqNoRcv = -1;
@@ -616,18 +616,6 @@ void TLayer4::finishedSendingTelegram(bool successful)
             // Connection control telegrams are so high priority that we retry sending them right away.
             sendPreparedTelegram();
         }
-        else if (tpci == T_ACK_PDU)
-        {
-            // Successfully sent a T_ACK. Continue with the next direct telegram.
-            if (sendConnectedTelegramBufferState == CONNECTED_TELEGRAM_WAIT_T_ACK_SENT)
-            {
-                sendConnectedTelegramBufferState = CONNECTED_TELEGRAM_WAIT_LOOP;
-            }
-            else if (sendConnectedTelegramBuffer2State == CONNECTED_TELEGRAM_WAIT_T_ACK_SENT)
-            {
-                sendConnectedTelegramBuffer2State = CONNECTED_TELEGRAM_WAIT_LOOP;
-            }
-        }
     }
 }
 
@@ -767,8 +755,8 @@ void TLayer4::actionA02sendAckPduAndProcessApci(ApciCommand apciCmd, const int8_
     connectedTime = millis(); // "restart the connection timeout timer"
 
     byte * sendBuffer;
-    volatile SendConnectedTelegramBufferState * sendBufferState;
-    if (sendConnectedTelegramBufferState == CONNECTED_TELEGRAM_FREE)
+    volatile SendTelegramBufferState * sendBufferState;
+    if (sendConnectedTelegramBufferState == TELEGRAM_FREE)
     {
         sendBuffer = sendConnectedTelegram;
         sendBufferState = &sendConnectedTelegramBufferState;
@@ -783,7 +771,7 @@ void TLayer4::actionA02sendAckPduAndProcessApci(ApciCommand apciCmd, const int8_
     // This ensures finishedSendingTelegram() can move the buffer to the next state and thus prevents
     // messages lingering in the buffer. The message will not be sent prematurely as the sending is
     // triggered in loop(), which can only execute after this function returns.
-    *sendBufferState = CONNECTED_TELEGRAM_WAIT_T_ACK_SENT;
+    *sendBufferState = TELEGRAM_ACQUIRED;
     auto sendResponse = processApci(apciCmd, telegram, telLength, sendBuffer);
     if (sendResponse)
     {
@@ -795,7 +783,7 @@ void TLayer4::actionA02sendAckPduAndProcessApci(ApciCommand apciCmd, const int8_
     }
     else
     {
-        *sendBufferState = CONNECTED_TELEGRAM_FREE;
+        *sendBufferState = TELEGRAM_FREE;
     }
 }
 
@@ -845,7 +833,7 @@ void TLayer4::actionA07SendDirectTelegram()
         return;
     }
 
-    if (sendConnectedTelegramBufferState != CONNECTED_TELEGRAM_WAIT_LOOP)
+    if (sendConnectedTelegramBufferState != TELEGRAM_ACQUIRED)
     {
         dump2(
               serial.print("ERROR A07SendTelegram Nothing to send");
@@ -864,7 +852,7 @@ void TLayer4::actionA07SendDirectTelegram()
     );
 
     setTL4State(TLayer4::OPEN_WAIT);
-    sendConnectedTelegramBufferState = CONNECTED_TELEGRAM_SENDING;
+    sendConnectedTelegramBufferState = TELEGRAM_SENDING;
 
     dump2(
         dumpTelegramBytes(true, sendConnectedTelegram, telegramSize(sendConnectedTelegram), true);
@@ -881,13 +869,13 @@ void TLayer4::actionA08IncrementSequenceNumber()
     seqNoSend++;
     seqNoSend &= 0x0F;
     connectedTime = millis(); // "restart the connection timeout timer"
-    sendConnectedTelegramBufferState = CONNECTED_TELEGRAM_FREE;
+    sendConnectedTelegramBufferState = TELEGRAM_FREE;
 
-    if (sendConnectedTelegramBuffer2State != CONNECTED_TELEGRAM_FREE)
+    if (sendConnectedTelegramBuffer2State != TELEGRAM_FREE)
     {
         memcpy(sendConnectedTelegram, sendConnectedTelegram2, telegramSize(sendConnectedTelegram2));
         sendConnectedTelegramBufferState = sendConnectedTelegramBuffer2State;
-        sendConnectedTelegramBuffer2State = CONNECTED_TELEGRAM_FREE;
+        sendConnectedTelegramBuffer2State = TELEGRAM_FREE;
     }
 }
 
@@ -918,6 +906,11 @@ void TLayer4::loop()
     if (!enabled)
         return;
 
+    // Rest of function can potentially send telegrams. Although we would busy wait until
+    // the send buffer is free, it makes more sense to keep the main loop running.
+    if (sendTelegramBufferState != TELEGRAM_FREE)
+        return;
+
     // Send a disconnect after TL4_CONNECTION_TIMEOUT_MS milliseconds inactivity
     if ((state != TLayer4::CLOSED) && (elapsed(connectedTime) >= TL4_CONNECTION_TIMEOUT_MS))
     {
@@ -927,7 +920,7 @@ void TLayer4::loop()
     }
 
     // Send a potential response message
-    if ((state == TLayer4::OPEN_IDLE) && (sendConnectedTelegramBufferState == CONNECTED_TELEGRAM_WAIT_LOOP))
+    if ((state == TLayer4::OPEN_IDLE) && (sendConnectedTelegramBufferState == TELEGRAM_ACQUIRED))
     {
         // event E15
         actionA07SendDirectTelegram();
@@ -965,8 +958,8 @@ void TLayer4::resetConnection()
           serial.print(LOG_SEP)
     );
 
-    sendConnectedTelegramBufferState = CONNECTED_TELEGRAM_FREE;
-    sendConnectedTelegramBuffer2State = CONNECTED_TELEGRAM_FREE;
+    sendConnectedTelegramBufferState = TELEGRAM_FREE;
+    sendConnectedTelegramBuffer2State = TELEGRAM_FREE;
 }
 
 bool TLayer4::setTL4State(TLayer4::TL4State newState)

diff --git a/test/lib-test-cases/src/tc_tlayer4_stepfunction.h b/test/lib-test-cases/src/tc_tlayer4_stepfunction.h
@@ -31,7 +31,7 @@ typedef struct
     int connectedAddrNew;
     byte seqNoSend;
     byte seqNoRcv;
-    TLayer4::SendConnectedTelegramBufferState sendConnectedTelegramBufferState;
+    TLayer4::SendTelegramBufferState sendConnectedTelegramBufferState;
 } ProtocolTestState;
 
 static ProtocolTestState protoState[2];
@@ -44,7 +44,7 @@ static void connect(void * state, unsigned int param)
     VaS(state)->machineState = TLayer4::OPEN_IDLE;
     VaS(state)->seqNoSend = 0;
     VaS(state)->seqNoRcv = 0;
-    VaS(state)->sendConnectedTelegramBufferState = TLayer4::CONNECTED_TELEGRAM_FREE;
+    VaS(state)->sendConnectedTelegramBufferState = TLayer4::TELEGRAM_FREE;
 }
 
 static void connectedOpenIdle(void * state, unsigned int param)