[diag] Handle errors of TransmitPacket()

`TransmitPacket()` should return an error from platform Radio implementation, as for example Radio can be in incorrect state. If error occurs, increase `mSentErrorInvalidStatePackets` stat. Add wrong state case to tests and fix `diag repeat stop` called too lata. Signed-off-by: Maciej Baczmanski <[email protected]>
maciejbaczmanski · Jan 7, 2025 · c70f942 · c70f942
1 parent 3470934
commit c70f942
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 13 deletions.
diff --git a/src/core/diags/README.md b/src/core/diags/README.md
@@ -346,6 +346,7 @@ received packets: 10
 sent success packets: 10
 sent error cca packets: 0
 sent error abort packets: 0
+sent error invalid state packets: 0
 sent error others packets: 0
 first received packet: rssi=-65, lqi=101
 last received packet: rssi=-64, lqi=98

diff --git a/src/core/diags/factory_diags.cpp b/src/core/diags/factory_diags.cpp
@@ -450,7 +450,7 @@ Error Diags::ProcessSend(uint8_t aArgsLength, char *aArgs[])
     VerifyOrExit(txLength >= OT_RADIO_FRAME_MIN_SIZE, error = kErrorInvalidArgs);
     mTxLen = txLength;
 
-    TransmitPacket();
+    SuccessOrExit(error = TransmitPacket());
 
 exit:
     return error;
@@ -489,11 +489,13 @@ void Diags::OutputStats(void)
            "sent success packets: %lu\r\n"
            "sent error cca packets: %lu\r\n"
            "sent error abort packets: %lu\r\n"
+           "sent error invalid state packets: %lu\r\n"
            "sent error others packets: %lu\r\n"
            "first received packet: rssi=%d, lqi=%u\r\n"
            "last received packet: rssi=%d, lqi=%u\r\n",
            ToUlong(mStats.mReceivedPackets), ToUlong(mStats.mSentSuccessPackets), ToUlong(mStats.mSentErrorCcaPackets),
-           ToUlong(mStats.mSentErrorAbortPackets), ToUlong(mStats.mSentErrorOthersPackets), mStats.mFirstRssi,
+           ToUlong(mStats.mSentErrorAbortPackets), ToUlong(mStats.mSentErrorInvalidStatePackets),
+           ToUlong(mStats.mSentErrorOthersPackets), mStats.mFirstRssi,
            mStats.mFirstLqi, mStats.mLastRssi, mStats.mLastLqi);
 }
 
@@ -528,8 +530,9 @@ Error Diags::ProcessStop(uint8_t aArgsLength, char *aArgs[])
     return kErrorNone;
 }
 
-void Diags::TransmitPacket(void)
+Error Diags::TransmitPacket(void)
 {
+    Error error = kErrorNone;
     mTxPacket->mChannel = mChannel;
 
     if (mIsTxPacketSet)
@@ -550,7 +553,14 @@ void Diags::TransmitPacket(void)
     }
 
     mDiagSendOn = true;
-    IgnoreError(Get<Radio>().Transmit(*static_cast<Mac::TxFrame *>(mTxPacket)));
+    error = Get<Radio>().Transmit(*static_cast<Mac::TxFrame *>(mTxPacket));
+
+    if(error == kErrorInvalidState)
+    {
+        mStats.mSentErrorInvalidStatePackets++;
+    }
+
+    return error;
 }
 
 Error Diags::ParseReceiveConfigFormat(const char *aFormat, ReceiveConfig &aConfig)
@@ -747,7 +757,7 @@ void Diags::AlarmFired(void)
     {
         uint32_t now = otPlatAlarmMilliGetNow();
 
-        TransmitPacket();
+        IgnoreError(TransmitPacket());
         otPlatAlarmMilliStartAt(&GetInstance(), now, mTxPeriod);
     }
     else
@@ -856,7 +866,7 @@ void Diags::TransmitDone(Error aError)
     VerifyOrExit(mTxPackets > 1);
     mTxPackets--;
 
-    TransmitPacket();
+    IgnoreError(TransmitPacket());
 
 exit:
     return;

diff --git a/src/core/diags/factory_diags.hpp b/src/core/diags/factory_diags.hpp
@@ -138,6 +138,7 @@ class Diags : public InstanceLocator, private NonCopyable
         uint32_t mSentFailedPackets;
         uint32_t mSentErrorCcaPackets;
         uint32_t mSentErrorAbortPackets;
+        uint32_t mSentErrorInvalidStatePackets;
         uint32_t mSentErrorOthersPackets;
         int8_t   mFirstRssi;
         uint8_t  mFirstLqi;
@@ -232,10 +233,10 @@ class Diags : public InstanceLocator, private NonCopyable
     Error GetPowerSettings(uint8_t aChannel, PowerSettings &aPowerSettings);
     Error ParseReceiveConfigFormat(const char *aFormat, ReceiveConfig &aConfig);
     Error RadioReceive(void);
+    Error TransmitPacket(void);
     void  OutputReceivedFrame(const otRadioFrame *aFrame);
     bool  ShouldHandleReceivedFrame(const otRadioFrame &aFrame) const;
 
-    void TransmitPacket(void);
     void Output(const char *aFormat, ...);
     void ResetTxPacket(void);
     void OutputStats(void);

diff --git a/tests/scripts/expect/cli-diags.exp b/tests/scripts/expect/cli-diags.exp
@@ -64,6 +64,7 @@ expect "received packets: 0"
 expect "sent success packets: 10"
 expect "sent error cca packets: 0"
 expect "sent error abort packets: 0"
+expect "sent error invalid state packets: 0"
 expect "sent error others packets: 0"
 expect "first received packet: rssi=0, lqi=0"
 expect "last received packet: rssi=0, lqi=0"
@@ -76,6 +77,7 @@ expect "received packets: 10"
 expect "sent success packets: 0"
 expect "sent error cca packets: 0"
 expect "sent error abort packets: 0"
+expect "sent error invalid state packets: 0"
 expect "sent error others packets: 0"
 expect "first received packet: rssi=-20, lqi=0"
 expect "last received packet: rssi=-20, lqi=0"
@@ -99,6 +101,7 @@ expect -r {received packets: \d+}
 expect "sent success packets: 0"
 expect "sent error cca packets: 0"
 expect "sent error abort packets: 0"
+expect "sent error invalid state packets: 0"
 expect "sent error others packets: 0"
 expect "first received packet: rssi=-20, lqi=0"
 expect "last received packet: rssi=-20, lqi=0"
@@ -127,6 +130,21 @@ send_user "input odd length test\n"
 send "diag frame 123\n"
 expect "Error"
 
+send "diag radio sleep\n"
+expect_line "Done"
+
+send "diag radio disable\n"
+expect_line "Done"
+
+send "diag send 10 10\n"
+expect "Error 13: InvalidState"
+
+send "diag radio enable\n"
+expect_line "Done"
+
+send "diag radio receive\n"
+expect_line "Done"
+
 send_user "shortest frame test\n"
 send "diag frame 112233\n"
 expect "Done"
@@ -138,6 +156,9 @@ send "diag frame 112233445566778899001122334455667788990011223344556677889900112
 expect "Done"
 send "diag repeat 1\n"
 expect "Done"
+sleep 3
+send "diag repeat stop\n"
+expect "Done"
 
 send_user "send frame with security processed\n"
 send "diag frame -s 112233\n"
@@ -157,9 +178,6 @@ expect "Done"
 send "diag send 1\n"
 expect "Done"
 
-send "diag repeat stop\n"
-expect "Done"
-
 send "diag channel 11\n"
 expect_line "Done"
 

diff --git a/tests/scripts/thread-cert/test_diag.py b/tests/scripts/thread-cert/test_diag.py
@@ -58,6 +58,7 @@ def test(self):
                 'sent success packets: 0\r\n'
                 'sent error cca packets: 0\r\n'
                 'sent error abort packets: 0\r\n'
+                'sent error invalid state packets: 0\r\n'
                 'sent error others packets: 0\r\n'
                 'first received packet: rssi=0, lqi=0\r\n'
                 'last received packet: rssi=0, lqi=0\r\n',

diff --git a/tools/otci/otci/otci.py b/tools/otci/otci/otci.py
@@ -2692,13 +2692,14 @@ def diag_get_stats(self) -> Dict[str, int]:
         result['sent_success_packets'] = int(output[1].split(":")[1])
         result['sent_error_cca_packets'] = int(output[2].split(":")[1])
         result['sent_error_abort_packets'] = int(output[3].split(":")[1])
-        result['sent_error_others_packets'] = int(output[4].split(":")[1])
+        result['sent_error_invalid_state_packets'] = int(output[4].split(":")[1])
+        result['sent_error_others_packets'] = int(output[5].split(":")[1])
 
-        values = re.findall("\-?\d+", output[5])
+        values = re.findall("\-?\d+", output[6])
         result['first_received_packet_rssi'] = int(values[0])
         result['first_received_packet_lqi'] = int(values[1])
 
-        values = re.findall("\-?\d+", output[6])
+        values = re.findall("\-?\d+", output[7])
         result['last_received_packet_rssi'] = int(values[0])
         result['last_received_packet_lqi'] = int(values[1])