From af2e10a7bf49297556d675a5d2c45a57f429b353 Mon Sep 17 00:00:00 2001
From: Abtin Keshavarzian <abtink@google.com>
Date: Fri, 11 Oct 2024 10:32:16 -0700
Subject: [PATCH] [mac-frame] add `mCommandId` to `TxFrame::Info` (#10697)

This commit updates `TxFrame::Info` to include `mCommandId`, used when
the frame type is `kTypeMacCmd`. This allows callers to specify the
Command ID when preparing a MAC Command `TxFrame`. With this change
`Frame::SetCommandId()` is no longer used or needed so it is
removed.
---
 src/core/mac/data_poll_sender.cpp |  3 +--
 src/core/mac/mac.cpp              |  7 +++----
 src/core/mac/mac_frame.cpp        | 15 +--------------
 src/core/mac/mac_frame.hpp        | 10 +---------
 tests/unit/test_mac_frame.cpp     |  6 ------
 5 files changed, 6 insertions(+), 35 deletions(-)

diff --git a/src/core/mac/data_poll_sender.cpp b/src/core/mac/data_poll_sender.cpp
index e6deb02e3..c21bbacfe 100644
--- a/src/core/mac/data_poll_sender.cpp
+++ b/src/core/mac/data_poll_sender.cpp
@@ -567,6 +567,7 @@ Mac::TxFrame *DataPollSender::PrepareDataRequest(Mac::TxFrames &aTxFrames)
     frameInfo.mPanIds.SetBothSourceDestination(Get<Mac::Mac>().GetPanId());
 
     frameInfo.mType          = Mac::Frame::kTypeMacCmd;
+    frameInfo.mCommandId     = Mac::Frame::kMacCmdDataRequest;
     frameInfo.mSecurityLevel = Mac::Frame::kSecurityEncMic32;
     frameInfo.mKeyIdMode     = Mac::Frame::kKeyIdMode1;
 
@@ -580,8 +581,6 @@ Mac::TxFrame *DataPollSender::PrepareDataRequest(Mac::TxFrames &aTxFrames)
     }
 #endif
 
-    IgnoreError(frame->SetCommandId(Mac::Frame::kMacCmdDataRequest));
-
 exit:
     return frame;
 }
diff --git a/src/core/mac/mac.cpp b/src/core/mac/mac.cpp
index ca9c28057..652d591b8 100644
--- a/src/core/mac/mac.cpp
+++ b/src/core/mac/mac.cpp
@@ -729,13 +729,12 @@ TxFrame *Mac::PrepareBeaconRequest(void)
     frameInfo.mAddrs.mDestination.SetShort(kShortAddrBroadcast);
     frameInfo.mPanIds.SetDestination(kShortAddrBroadcast);
 
-    frameInfo.mType    = Frame::kTypeMacCmd;
-    frameInfo.mVersion = Frame::kVersion2003;
+    frameInfo.mType      = Frame::kTypeMacCmd;
+    frameInfo.mCommandId = Frame::kMacCmdBeaconRequest;
+    frameInfo.mVersion   = Frame::kVersion2003;
 
     frameInfo.PrepareHeadersIn(frame);
 
-    IgnoreError(frame.SetCommandId(Frame::kMacCmdBeaconRequest));
-
     LogInfo("Sending Beacon Request");
 
     return &frame;
diff --git a/src/core/mac/mac_frame.cpp b/src/core/mac/mac_frame.cpp
index 87af6ecca..b8dea18a4 100644
--- a/src/core/mac/mac_frame.cpp
+++ b/src/core/mac/mac_frame.cpp
@@ -238,7 +238,7 @@ void TxFrame::Info::PrepareHeadersIn(TxFrame &aTxFrame) const
 
     if (mType == kTypeMacCmd)
     {
-        builder.Append<uint8_t>(); // Placeholder for Command ID
+        IgnoreError(builder.AppendUint8(mCommandId));
     }
 
     builder.AppendLength(micSize + aTxFrame.GetFcsSize());
@@ -825,19 +825,6 @@ exit:
     return error;
 }
 
-Error Frame::SetCommandId(uint8_t aCommandId)
-{
-    Error   error = kErrorNone;
-    uint8_t index = FindPayloadIndex();
-
-    VerifyOrExit(index != kInvalidIndex, error = kErrorParse);
-
-    mPsdu[IsVersion2015() ? index : (index - 1)] = aCommandId;
-
-exit:
-    return error;
-}
-
 bool Frame::IsDataRequestCommand(void) const
 {
     bool    isDataRequest = false;
diff --git a/src/core/mac/mac_frame.hpp b/src/core/mac/mac_frame.hpp
index a2d116314..245caa289 100644
--- a/src/core/mac/mac_frame.hpp
+++ b/src/core/mac/mac_frame.hpp
@@ -481,15 +481,6 @@ public:
      */
     Error GetCommandId(uint8_t &aCommandId) const;
 
-    /**
-     * Sets the Command ID.
-     *
-     * @param[in]  aCommandId  The Command ID.
-     *
-     * @retval kErrorNone  Successfully set the Command ID.
-     */
-    Error SetCommandId(uint8_t aCommandId);
-
     /**
      * Indicates whether the frame is a MAC Data Request command (data poll).
      *
@@ -1065,6 +1056,7 @@ public:
         PanIds        mPanIds;               ///< Source and destination PAN Ids.
         SecurityLevel mSecurityLevel;        ///< Frame security level.
         KeyIdMode     mKeyIdMode;            ///< Frame security key ID mode.
+        CommandId     mCommandId;            ///< Command ID (applicable when `mType == kTypeMacCmd`).
         bool          mSuppressSequence : 1; ///< Whether to suppress seq number.
 
 #if OPENTHREAD_CONFIG_MAC_HEADER_IE_SUPPORT
diff --git a/tests/unit/test_mac_frame.cpp b/tests/unit/test_mac_frame.cpp
index 9085ba4cb..197c5577c 100644
--- a/tests/unit/test_mac_frame.cpp
+++ b/tests/unit/test_mac_frame.cpp
@@ -640,9 +640,6 @@ void TestMacFrameApi(void)
     VerifyOrQuit(frame.GetType() == Mac::Frame::kTypeMacCmd);
     SuccessOrQuit(frame.GetCommandId(commandId));
     VerifyOrQuit(commandId == Mac::Frame::kMacCmdDataRequest);
-    SuccessOrQuit(frame.SetCommandId(Mac::Frame::kMacCmdBeaconRequest));
-    SuccessOrQuit(frame.GetCommandId(commandId));
-    VerifyOrQuit(commandId == Mac::Frame::kMacCmdBeaconRequest);
 
 #if (OPENTHREAD_CONFIG_THREAD_VERSION >= OT_THREAD_VERSION_1_2)
     // IEEE 802.15.4-2015 Mac Command
@@ -660,9 +657,6 @@ void TestMacFrameApi(void)
     SuccessOrQuit(frame.GetCommandId(commandId));
     VerifyOrQuit(commandId == Mac::Frame::kMacCmdDataRequest);
     printf("commandId:%d\n", commandId);
-    SuccessOrQuit(frame.SetCommandId(Mac::Frame::kMacCmdOrphanNotification));
-    SuccessOrQuit(frame.GetCommandId(commandId));
-    VerifyOrQuit(commandId == Mac::Frame::kMacCmdOrphanNotification);
 
 #endif
 }