[otns] make various enhancements to Otns (openthread#11643)

abtink · web-flow · commit a495a2284b3e · 2025-06-30T14:39:30.000-07:00
This commit contains various enhancements to the `Otns` class:

- Makes `Otns` methods non-static. This aligns their use with the
  clang-tidy `readability-static-accessed-through-instance` check,
  which disallows accessing static methods through an instance.
- Updates the `make-pretty` script to enable OTNS and include it in
  `clang-tidy` checks.
- Adds a stub implementation of `otPlatOtnsStatus()` in the simulation
  and fake platforms. This allows the OTNS feature to be enabled in
  `make-pretty` builds and covered by GitHub Action CI checks.
- Simplifies the `EmitStatus` methods to use the `String` class for
  constructing the status string.
- Adds a new helper method to construct the CoAP status string,
  removing duplicated code.
diff --git a/examples/platforms/simulation/misc.c b/examples/platforms/simulation/misc.c
@@ -122,3 +122,7 @@ void otPlatAssertFail(const char *aFilename, int aLineNumber)
     assert(false);
     exit(1);
 }
+
+#if OPENTHREAD_CONFIG_OTNS_ENABLE && !OPENTHREAD_SIMULATION_VIRTUAL_TIME
+void otPlatOtnsStatus(const char *aStatus) { OT_UNUSED_VARIABLE(aStatus); }
+#endif
diff --git a/examples/platforms/simulation/platform-config.h b/examples/platforms/simulation/platform-config.h
@@ -76,17 +76,6 @@
 #define OPENTHREAD_CONFIG_NCP_SPI_ENABLE 0
 #endif
 
-/**
- * Check OTNS configurations
- */
-#if OPENTHREAD_CONFIG_OTNS_ENABLE
-
-#if !OPENTHREAD_SIMULATION_VIRTUAL_TIME
-#error "OTNS requires virtual time simulations"
-#endif
-
-#endif // OPENTHREAD_CONFIG_OTNS_ENABLE
-
 /**
  * @def OPENTHREAD_SIMULATION_MAX_NETWORK_SIZE
  *
diff --git a/script/make-pretty b/script/make-pretty
@@ -129,6 +129,7 @@ OT_CLANG_TIDY_BUILD_OPTS=(
     '-DOT_NAT64_TRANSLATOR=ON'
     '-DOT_NETDATA_PUBLISHER=ON'
     '-DOT_NETDIAG_CLIENT=ON'
+    '-DOT_OTNS=ON'
     '-DOT_PING_SENDER=ON'
     '-DOT_REFERENCE_DEVICE=ON'
     '-DOT_SERVICE=ON'
diff --git a/src/core/utils/otns.cpp b/src/core/utils/otns.cpp
@@ -42,49 +42,49 @@ namespace Utils {
 
 RegisterLogModule("Otns");
 
-const int kMaxStatusStringLength = 128;
+void Otns::EmitShortAddress(uint16_t aShortAddress) const { EmitStatus("rloc16=%d", aShortAddress); }
 
-void Otns::EmitShortAddress(uint16_t aShortAddress) { EmitStatus("rloc16=%d", aShortAddress); }
-
-void Otns::EmitExtendedAddress(const Mac::ExtAddress &aExtAddress)
+void Otns::EmitExtendedAddress(const Mac::ExtAddress &aExtAddress) const
 {
     Mac::ExtAddress revExtAddress;
+
     revExtAddress.Set(aExtAddress.m8, Mac::ExtAddress::kReverseByteOrder);
     EmitStatus("extaddr=%s", revExtAddress.ToString().AsCString());
 }
 
 void Otns::EmitPingRequest(const Ip6::Address &aPeerAddress,
                            uint16_t            aPingLength,
                            uint32_t            aTimestamp,
-                           uint8_t             aHopLimit)
+                           uint8_t             aHopLimit) const
 {
     OT_UNUSED_VARIABLE(aHopLimit);
-    EmitStatus("ping_request=%s,%d,%lu", aPeerAddress.ToString().AsCString(), aPingLength, aTimestamp);
+    EmitStatus("ping_request=%s,%d,%lu", aPeerAddress.ToString().AsCString(), aPingLength, ToUlong(aTimestamp));
 }
 
-void Otns::EmitPingReply(const Ip6::Address &aPeerAddress, uint16_t aPingLength, uint32_t aTimestamp, uint8_t aHopLimit)
+void Otns::EmitPingReply(const Ip6::Address &aPeerAddress,
+                         uint16_t            aPingLength,
+                         uint32_t            aTimestamp,
+                         uint8_t             aHopLimit) const
 {
-    EmitStatus("ping_reply=%s,%u,%lu,%d", aPeerAddress.ToString().AsCString(), aPingLength, aTimestamp, aHopLimit);
+    EmitStatus("ping_reply=%s,%u,%lu,%d", aPeerAddress.ToString().AsCString(), aPingLength, ToUlong(aTimestamp),
+               aHopLimit);
 }
 
-void Otns::EmitStatus(const char *aFmt, ...)
+void Otns::EmitStatus(const char *aFmt, ...) const
 {
-    char statusStr[kMaxStatusStringLength + 1];
-    int  n;
-
-    va_list ap;
-    va_start(ap, aFmt);
+    StatusString string;
+    va_list      args;
 
-    n = vsnprintf(statusStr, sizeof(statusStr), aFmt, ap);
-    OT_UNUSED_VARIABLE(n);
-    OT_ASSERT(n >= 0);
+    va_start(args, aFmt);
+    string.AppendVarArgs(aFmt, args);
+    va_end(args);
 
-    va_end(ap);
-
-    otPlatOtnsStatus(statusStr);
+    EmitStatus(string);
 }
 
-void Otns::HandleNotifierEvents(Events aEvents)
+void Otns::EmitStatus(const StatusString &aString) const { otPlatOtnsStatus(aString.AsCString()); }
+
+void Otns::HandleNotifierEvents(Events aEvents) const
 {
     if (aEvents.Contains(kEventThreadRoleChanged))
     {
@@ -93,7 +93,7 @@ void Otns::HandleNotifierEvents(Events aEvents)
 
     if (aEvents.Contains(kEventThreadPartitionIdChanged))
     {
-        EmitStatus("parid=%x", Get<Mle::Mle>().GetLeaderData().GetPartitionId());
+        EmitStatus("parid=%lx", ToUlong(Get<Mle::Mle>().GetLeaderData().GetPartitionId()));
     }
 
 #if OPENTHREAD_CONFIG_JOINER_ENABLE
@@ -104,95 +104,118 @@ void Otns::HandleNotifierEvents(Events aEvents)
 #endif
 }
 
-void Otns::EmitNeighborChange(NeighborTable::Event aEvent, const Neighbor &aNeighbor)
+void Otns::EmitNeighborChange(NeighborTable::Event aEvent, const Neighbor &aNeighbor) const
 {
+    StatusString string;
+
     switch (aEvent)
     {
     case NeighborTable::kRouterAdded:
-        EmitStatus("router_added=%s", aNeighbor.GetExtAddress().ToString().AsCString());
+        string.Append("router_added");
         break;
     case NeighborTable::kRouterRemoved:
-        EmitStatus("router_removed=%s", aNeighbor.GetExtAddress().ToString().AsCString());
+        string.Append("router_removed");
         break;
     case NeighborTable::kChildAdded:
-        EmitStatus("child_added=%s", aNeighbor.GetExtAddress().ToString().AsCString());
+        string.Append("child_added");
         break;
     case NeighborTable::kChildRemoved:
-        EmitStatus("child_removed=%s", aNeighbor.GetExtAddress().ToString().AsCString());
+        string.Append("child_removed");
         break;
     case NeighborTable::kChildModeChanged:
-        break;
+        ExitNow();
     }
+
+    string.Append("=%s", aNeighbor.GetExtAddress().ToString().AsCString());
+    EmitStatus(string);
+
+exit:
+    return;
 }
 
-void Otns::EmitTransmit(const Mac::TxFrame &aFrame)
+void Otns::EmitTransmit(const Mac::TxFrame &aFrame) const
 {
+    StatusString string;
     Mac::Address dst;
-    uint16_t     frameControlField = aFrame.GetFrameControlField();
-    uint8_t      channel           = aFrame.GetChannel();
-    uint8_t      sequence          = aFrame.GetSequence();
 
     IgnoreError(aFrame.GetDstAddr(dst));
 
+    string.Append("transmit=%d,%04x,%d", aFrame.GetChannel(), aFrame.GetFrameControlField(), aFrame.GetSequence());
+
     if (dst.IsShort())
     {
-        EmitStatus("transmit=%d,%04x,%d,%04x", channel, frameControlField, sequence, dst.GetShort());
+        string.Append(",%04x", dst.GetShort());
     }
     else if (dst.IsExtended())
     {
-        EmitStatus("transmit=%d,%04x,%d,%s", channel, frameControlField, sequence, dst.ToString().AsCString());
+        string.Append(",%s", dst.ToString().AsCString());
     }
-    else
-    {
-        EmitStatus("transmit=%d,%04x,%d", channel, frameControlField, sequence);
-    }
-}
 
-void Otns::EmitDeviceMode(Mle::DeviceMode aMode)
-{
-    EmitStatus("mode=%s%s%s", aMode.IsRxOnWhenIdle() ? "r" : "", aMode.IsFullThreadDevice() ? "d" : "",
-               (aMode.GetNetworkDataType() == NetworkData::kFullSet) ? "n" : "");
+    EmitStatus(string);
 }
 
-void Otns::EmitCoapSend(const Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo)
+void Otns::EmitDeviceMode(Mle::DeviceMode aMode) const
 {
-    char  uriPath[Coap::Message::kMaxReceivedUriPath + 1];
-    Error error;
+    StatusString string;
 
-    SuccessOrExit(error = aMessage.ReadUriPathOptions(uriPath));
+    string.Append("mode=");
+
+    if (aMode.IsRxOnWhenIdle())
+    {
+        string.Append("r");
+    }
 
-    EmitStatus("coap=send,%d,%d,%d,%s,%s,%d", aMessage.GetMessageId(), aMessage.GetType(), aMessage.GetCode(), uriPath,
-               aMessageInfo.GetPeerAddr().ToString().AsCString(), aMessageInfo.GetPeerPort());
+    if (aMode.IsFullThreadDevice())
+    {
+        string.Append("d");
+    }
 
-exit:
-    LogWarnOnError(error, "EmitCoapSend");
+    if (aMode.GetNetworkDataType() == NetworkData::kFullSet)
+    {
+        string.Append("m");
+    }
+
+    EmitStatus(string);
 }
 
-void Otns::EmitCoapReceive(const Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo)
+void Otns::EmitCoapSend(const Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo) const
 {
-    char  uriPath[Coap::Message::kMaxReceivedUriPath + 1];
-    Error error = kErrorNone;
+    EmitCoapStatus("send", aMessage, aMessageInfo);
+}
 
-    SuccessOrExit(error = aMessage.ReadUriPathOptions(uriPath));
+void Otns::EmitCoapReceive(const Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo) const
+{
+    EmitCoapStatus("recv", aMessage, aMessageInfo);
+}
 
-    EmitStatus("coap=recv,%d,%d,%d,%s,%s,%d", aMessage.GetMessageId(), aMessage.GetType(), aMessage.GetCode(), uriPath,
-               aMessageInfo.GetPeerAddr().ToString().AsCString(), aMessageInfo.GetPeerPort());
-exit:
-    LogWarnOnError(error, "EmitCoapReceive");
+void Otns::EmitCoapSendFailure(Error aError, Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo) const
+{
+    EmitCoapStatus("send_error", aMessage, aMessageInfo, &aError);
 }
 
-void Otns::EmitCoapSendFailure(Error aError, Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo)
+void Otns::EmitCoapStatus(const char             *aAction,
+                          const Coap::Message    &aMessage,
+                          const Ip6::MessageInfo &aMessageInfo,
+                          Error                  *aError) const
 {
-    char  uriPath[Coap::Message::kMaxReceivedUriPath + 1];
-    Error error = kErrorNone;
+    Error        error;
+    char         uriPath[Coap::Message::kMaxReceivedUriPath + 1];
+    StatusString string;
 
     SuccessOrExit(error = aMessage.ReadUriPathOptions(uriPath));
 
-    EmitStatus("coap=send_error,%d,%d,%d,%s,%s,%d,%s", aMessage.GetMessageId(), aMessage.GetType(), aMessage.GetCode(),
-               uriPath, aMessageInfo.GetPeerAddr().ToString().AsCString(), aMessageInfo.GetPeerPort(),
-               ErrorToString(aError));
+    string.Append("coap=%s,%d,%d,%d,%s,%s,%d", aAction, aMessage.GetMessageId(), aMessage.GetType(), aMessage.GetCode(),
+                  uriPath, aMessageInfo.GetPeerAddr().ToString().AsCString(), aMessageInfo.GetPeerPort());
+
+    if (aError != nullptr)
+    {
+        string.Append(",%s", ErrorToString(*aError));
+    }
+
+    EmitStatus(string);
+
 exit:
-    LogWarnOnError(error, "EmitCoapSendFailure");
+    LogWarnOnError(error, "EmitCoapStatus");
 }
 
 } // namespace Utils
diff --git a/src/core/utils/otns.hpp b/src/core/utils/otns.hpp
@@ -46,6 +46,7 @@
 #include "common/locator.hpp"
 #include "common/non_copyable.hpp"
 #include "common/notifier.hpp"
+#include "common/string.hpp"
 #include "mac/mac_frame.hpp"
 #include "mac/mac_types.hpp"
 #include "net/ip6_address.hpp"
@@ -78,14 +79,14 @@ class Otns : public InstanceLocator, private NonCopyable
      *
      * @param[in]  aShortAddress  The new short address.
      */
-    static void EmitShortAddress(uint16_t aShortAddress);
+    void EmitShortAddress(uint16_t aShortAddress) const;
 
     /**
      * Emits radio extended address to OTNS when changed.
      *
      * @param[in]  aExtAddress  The new extended address.
      */
-    static void EmitExtendedAddress(const Mac::ExtAddress &aExtAddress);
+    void EmitExtendedAddress(const Mac::ExtAddress &aExtAddress) const;
 
     /**
      * Emits ping request information to OTNS when sending.
@@ -95,10 +96,10 @@ class Otns : public InstanceLocator, private NonCopyable
      * @param[in]  aTimestamp    The timestamp of the ping request.
      * @param[in]  aHopLimit     The hop limit of the ping request.
      */
-    static void EmitPingRequest(const Ip6::Address &aPeerAddress,
-                                uint16_t            aPingLength,
-                                uint32_t            aTimestamp,
-                                uint8_t             aHopLimit);
+    void EmitPingRequest(const Ip6::Address &aPeerAddress,
+                         uint16_t            aPingLength,
+                         uint32_t            aTimestamp,
+                         uint8_t             aHopLimit) const;
 
     /**
      * Emits ping reply information to OTNS when received.
@@ -108,40 +109,40 @@ class Otns : public InstanceLocator, private NonCopyable
      * @param[in]  aTimestamp    The timestamp of the ping reply.
      * @param[in]  aHopLimit     The hop limit of the ping reply.
      */
-    static void EmitPingReply(const Ip6::Address &aPeerAddress,
-                              uint16_t            aPingLength,
-                              uint32_t            aTimestamp,
-                              uint8_t             aHopLimit);
+    void EmitPingReply(const Ip6::Address &aPeerAddress,
+                       uint16_t            aPingLength,
+                       uint32_t            aTimestamp,
+                       uint8_t             aHopLimit) const;
 
     /**
      * Emits a neighbor table event to OTNS when a neighbor is added or removed.
      *
      * @param[in]  aEvent     The event type.
      * @param[in]  aNeighbor  The neighbor that is added or removed.
      */
-    static void EmitNeighborChange(NeighborTable::Event aEvent, const Neighbor &aNeighbor);
+    void EmitNeighborChange(NeighborTable::Event aEvent, const Neighbor &aNeighbor) const;
 
     /**
      * Emits a transmit event to OTNS.
      *
      * @param[in]  aFrame  The frame of the transmission.
      */
-    static void EmitTransmit(const Mac::TxFrame &aFrame);
+    void EmitTransmit(const Mac::TxFrame &aFrame) const;
 
     /**
      * Emits the device mode to OTNS.
      *
      * @param[in] aMode The device mode.
      */
-    static void EmitDeviceMode(Mle::DeviceMode aMode);
+    void EmitDeviceMode(Mle::DeviceMode aMode) const;
 
     /**
      * Emits the sending COAP message info to OTNS.
      *
      * @param[in] aMessage      The sending COAP message.
      * @param[in] aMessageInfo  The message info.
      */
-    static void EmitCoapSend(const Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo);
+    void EmitCoapSend(const Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo) const;
 
     /**
      * Emits the COAP message sending failure to OTNS.
@@ -150,19 +151,29 @@ class Otns : public InstanceLocator, private NonCopyable
      * @param[in] aMessage      The COAP message failed to send.
      * @param[in] aMessageInfo  The message info.
      */
-    static void EmitCoapSendFailure(Error aError, Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo);
+    void EmitCoapSendFailure(Error aError, Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo) const;
 
     /**
      * Emits the received COAP message info to OTNS.
      *
      * @param[in] aMessage      The received COAP message.
      * @param[in] aMessageInfo  The message info.
      */
-    static void EmitCoapReceive(const Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo);
+    void EmitCoapReceive(const Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo) const;
 
 private:
-    static void EmitStatus(const char *aFmt, ...);
-    void        HandleNotifierEvents(Events aEvents);
+    static constexpr uint16_t kStatusStringLength = 128;
+
+    using StatusString = String<kStatusStringLength>;
+
+    void EmitStatus(const StatusString &aString) const;
+    void EmitStatus(const char *aFmt, ...) const OT_TOOL_PRINTF_STYLE_FORMAT_ARG_CHECK(2, 3);
+    void EmitCoapStatus(const char             *aAction,
+                        const Coap::Message    &aMessage,
+                        const Ip6::MessageInfo &aMessageInfo,
+                        Error                  *aError = nullptr) const;
+
+    void HandleNotifierEvents(Events aEvents) const;
 };
 
 } // namespace Utils
diff --git a/tests/gtest/fake_platform.cpp b/tests/gtest/fake_platform.cpp
diff --git a/tests/toranj/openthread-core-toranj-config-simulation.h b/tests/toranj/openthread-core-toranj-config-simulation.h

Original file line number	Diff line number	Diff line change
`@@ -122,3 +122,7 @@ void otPlatAssertFail(const char *aFilename, int aLineNumber)`
`122`	`122`	`assert(false);`
`123`	`123`	`exit(1);`
`124`	`124`	`}`
	`125`	`+`
	`126`	`+#if OPENTHREAD_CONFIG_OTNS_ENABLE && !OPENTHREAD_SIMULATION_VIRTUAL_TIME`
	`127`	`+void otPlatOtnsStatus(const char *aStatus) { OT_UNUSED_VARIABLE(aStatus); }`
	`128`	`+#endif`