[diag] do not allow running diag send and diag repeat concurrently (#11184)

The current diag module allows users running the 'diag send' and 'diag
repeat' concurrently. The command run later will change the settings
of the command run earlier, which will cause unexpected test results.

This commit does not allow running 'diag send' and 'diag repeat'
concurrently.

src/core/diags/factory_diags.cpp

@@ -198,9 +198,9 @@ Diags::Diags(Instance &aInstance)
     , mChannel(20)
     , mTxPower(0)
     , mTxLen(0)
+    , mCurTxCmd(kTxCmdNone)
     , mIsTxPacketSet(false)
     , mIsAsyncSend(false)
-    , mRepeatActive(false)
     , mDiagSendOn(false)
     , mOutputCallback(nullptr)
     , mOutputContext(nullptr)
@@ -387,7 +387,7 @@ Error Diags::ProcessRepeat(uint8_t aArgsLength, char *aArgs[])
     if (StringMatch(aArgs[0], "stop"))
     {
         otPlatAlarmMilliStop(&GetInstance());
-        mRepeatActive = false;
+        mCurTxCmd = kTxCmdNone;
     }
     else
     {
@@ -395,6 +395,7 @@ Error Diags::ProcessRepeat(uint8_t aArgsLength, char *aArgs[])
         uint8_t  txLength;
 
         VerifyOrExit(aArgsLength >= 1, error = kErrorInvalidArgs);
+        VerifyOrExit(mCurTxCmd == kTxCmdNone, error = kErrorInvalidState);
 
         SuccessOrExit(error = Utils::CmdLineParser::ParseAsUint32(aArgs[0], txPeriod));
         mTxPeriod = txPeriod;
@@ -415,11 +416,10 @@ Error Diags::ProcessRepeat(uint8_t aArgsLength, char *aArgs[])
 
         VerifyOrExit((txLength >= OT_RADIO_FRAME_MIN_SIZE) && (txLength <= OT_RADIO_FRAME_MAX_SIZE),
                      error = kErrorInvalidArgs);
-        mTxLen = txLength;
 
-        mRepeatActive = true;
-        uint32_t now  = otPlatAlarmMilliGetNow();
-        otPlatAlarmMilliStartAt(&GetInstance(), now, mTxPeriod);
+        mTxLen    = txLength;
+        mCurTxCmd = kTxCmdRepeat;
+        otPlatAlarmMilliStartAt(&GetInstance(), otPlatAlarmMilliGetNow(), mTxPeriod);
     }
 
 exit:
@@ -433,6 +433,7 @@ Error Diags::ProcessSend(uint8_t aArgsLength, char *aArgs[])
     uint8_t  txLength;
 
     VerifyOrExit(aArgsLength >= 1, error = kErrorInvalidArgs);
+    VerifyOrExit(mCurTxCmd == kTxCmdNone, error = kErrorInvalidState);
 
     if (StringMatch(aArgs[0], "async"))
     {
@@ -468,6 +469,7 @@ Error Diags::ProcessSend(uint8_t aArgsLength, char *aArgs[])
     mTxLen = txLength;
 
     SuccessOrExit(error = TransmitPacket());
+    mCurTxCmd = kTxCmdSend;
 
     if (!mIsAsyncSend)
     {
@@ -574,12 +576,14 @@ Error Diags::TransmitPacket(void)
         }
     }
 
-    mDiagSendOn = true;
-    error       = Get<Radio>().Transmit(*static_cast<Mac::TxFrame *>(mTxPacket));
-
-    if (error == kErrorInvalidState)
+    error = Get<Radio>().Transmit(*static_cast<Mac::TxFrame *>(mTxPacket));
+    if (error == kErrorNone)
     {
-        mStats.mSentErrorInvalidStatePackets++;
+        mDiagSendOn = true;
+    }
+    else
+    {
+        UpdateTxStats(error);
     }
 
     return error;
@@ -777,7 +781,7 @@ extern "C" void otPlatDiagAlarmFired(otInstance *aInstance) { AsCoreType(aInstan
 
 void Diags::AlarmFired(void)
 {
-    if (mRepeatActive)
+    if (mCurTxCmd == kTxCmdRepeat)
     {
         uint32_t now = otPlatAlarmMilliGetNow();
 
@@ -872,26 +876,8 @@ void Diags::TransmitDone(Error aError)
         IgnoreError(Get<Radio>().Sleep());
     }
 
-    switch (aError)
-    {
-    case kErrorNone:
-        mStats.mSentSuccessPackets++;
-        break;
-
-    case kErrorChannelAccessFailure:
-        mStats.mSentErrorCcaPackets++;
-        break;
-
-    case kErrorAbort:
-        mStats.mSentErrorAbortPackets++;
-        break;
-
-    default:
-        mStats.mSentErrorOthersPackets++;
-        break;
-    }
-
-    VerifyOrExit(!mRepeatActive && (mTxPackets > 0));
+    UpdateTxStats(aError);
+    VerifyOrExit((mCurTxCmd == kTxCmdSend) && (mTxPackets > 0));
 
     if (mTxPackets > 1)
     {
@@ -901,6 +887,7 @@ void Diags::TransmitDone(Error aError)
     else
     {
         mTxPackets = 0;
+        mCurTxCmd  = kTxCmdNone;
 
         if (!mIsAsyncSend)
         {
@@ -926,6 +913,32 @@ exit:
     return ret;
 }
 
+void Diags::UpdateTxStats(Error aError)
+{
+    switch (aError)
+    {
+    case kErrorNone:
+        mStats.mSentSuccessPackets++;
+        break;
+
+    case kErrorChannelAccessFailure:
+        mStats.mSentErrorCcaPackets++;
+        break;
+
+    case kErrorAbort:
+        mStats.mSentErrorAbortPackets++;
+        break;
+
+    case kErrorInvalidState:
+        mStats.mSentErrorInvalidStatePackets++;
+        break;
+
+    default:
+        mStats.mSentErrorOthersPackets++;
+        break;
+    }
+}
+
 #endif // OPENTHREAD_RADIO
 
 Error Diags::ProcessContinuousWave(uint8_t aArgsLength, char *aArgs[])

src/core/diags/factory_diags.hpp

@@ -240,12 +240,20 @@ private:
     void Output(const char *aFormat, ...);
     void ResetTxPacket(void);
     void OutputStats(void);
+    void UpdateTxStats(Error aError);
 
     static bool IsChannelValid(uint8_t aChannel);
 
     static const struct Command sCommands[];
 
 #if OPENTHREAD_FTD || OPENTHREAD_MTD || (OPENTHREAD_RADIO && OPENTHREAD_RADIO_CLI)
+    enum TxCmd : uint8_t
+    {
+        kTxCmdNone,
+        kTxCmdRepeat,
+        kTxCmdSend,
+    };
+
     Stats mStats;
 
     otRadioFrame *mTxPacket;
@@ -254,10 +262,10 @@ private:
     uint8_t       mChannel;
     int8_t        mTxPower;
     uint8_t       mTxLen;
+    TxCmd         mCurTxCmd;
     bool          mIsHeaderUpdated : 1;
     bool          mIsTxPacketSet : 1;
     bool          mIsAsyncSend : 1;
-    bool          mRepeatActive : 1;
     bool          mDiagSendOn : 1;
     bool          mIsSleepOn : 1;
 #endif

tests/scripts/expect/cli-diags.exp

@@ -158,6 +158,18 @@ expect_line "Done"
 send "diag radio receive\n"
 expect_line "Done"
 
+send "diag repeat 100 100\n"
+expect "Done"
+
+send "diag send 1 10\n"
+expect "Error 13: InvalidState"
+
+send "diag repeat stop\n"
+expect "Done"
+
+send "diag send 1 10\n"
+expect "Done"
+
 send_user "shortest frame test\n"
 send "diag frame 112233\n"
 expect "Done"