06/01 addressing additional stack PR reviews

Author: suveshpratapa

examples/platforms/simulation/radio.c

@@ -1168,17 +1168,23 @@ void otPlatRadioSetWakeKey(otInstance *aInstance, uint8_t aKeyIndex, const otMac
     OT_UNUSED_VARIABLE(aKeyIndex);
     OT_UNUSED_VARIABLE(aWakeKey);
 }
+#endif
+
+#if OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_INITIATOR_ENABLE
+static uint32_t sWakeFrameCounter;
 
 void otPlatRadioSetWakeFrameCounter(otInstance *aInstance, uint32_t aWakeFrameCounter)
 {
     OT_UNUSED_VARIABLE(aInstance);
-    OT_UNUSED_VARIABLE(aWakeFrameCounter);
+
+    sWakeFrameCounter = aWakeFrameCounter;
 }
 
 uint32_t otPlatRadioGetWakeFrameCounter(otInstance *aInstance)
 {
     OT_UNUSED_VARIABLE(aInstance);
-    return 0;
+
+    return sWakeFrameCounter;
 }
 #endif
 

include/openthread/thread_direct.h

@@ -116,6 +116,9 @@ typedef void (*otThreadDirectEventCallback)(otThreadDirectEvent           aEvent
  * Valid for both WI and WL roles.  A subsequent call replaces any previously
  * registered callback.  Pass NULL to clear.
  *
+ * Requires `OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_INITIATOR_ENABLE` or
+ * `OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_LISTENER_ENABLE`.
+ *
  * @param[in] aInstance  The OpenThread instance.
  * @param[in] aCallback  The callback function pointer, or NULL to clear.
  * @param[in] aContext   Application-specific context pointer passed to @p aCallback.
@@ -164,6 +167,8 @@ typedef enum
  *                                   or @p aKeyIndex is out of range.
  * @retval OT_ERROR_INVALID_STATE    A wake burst is already in progress, or @p aKeyIndex is a
  *                                   guest index for which no key has been provisioned.
+ *
+ * Requires `OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_INITIATOR_ENABLE`.
  */
 otError otThreadDirectWakeup(otInstance            *aInstance,
                              const otExtAddress    *aExtAddress,
@@ -194,14 +199,17 @@ otError otThreadDirectUnlink(otInstance *aInstance, const otExtAddress *aExtAddr
  * @param[in] aInstance  The OpenThread instance.
  * @param[in] aEnable    TRUE to enable WL listen mode; FALSE to disable.
  *
- * @retval OT_ERROR_NONE             Mode updated.
- * @retval OT_ERROR_NOT_IMPLEMENTED  Feature is not implemented.
+ * @retval OT_ERROR_NONE  Mode updated.
+ *
+ * Requires `OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_LISTENER_ENABLE`.
  */
 otError otThreadDirectWakeListenerEnable(otInstance *aInstance, bool aEnable);
 
 /**
  * Returns whether WL wake channel listening is currently active.
  *
+ * Requires `OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_LISTENER_ENABLE`.
+ *
  * @param[in] aInstance  The OpenThread instance.
  *
  * @returns TRUE if the WL is listening for TD Wake Commands, FALSE otherwise.
@@ -215,6 +223,8 @@ bool otThreadDirectIsWakeListenerEnabled(otInstance *aInstance);
  * immediately follows, during which the WI waits for a TD Link Command from
  * the WL.
  *
+ * Requires `OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_INITIATOR_ENABLE`.
+ *
  * @param[in] aInstance  The OpenThread instance.
  *
  * @returns TRUE if a wake burst or connection window is active, FALSE otherwise.
@@ -225,6 +235,9 @@ bool otThreadDirectIsWakeBurstActive(otInstance *aInstance);
  * Configures the Scheduled Listen Window (SLW) period this device advertises to
  * its peer in the SCA LTV.  Both WI and WL may call this function.
  *
+ * Requires `OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_INITIATOR_ENABLE` or
+ * `OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_LISTENER_ENABLE`.
+ *
  * Phase is a dynamic, stack-computed value (time until the next SLW window at
  * frame-build time) and is not configurable by the application.
  *
@@ -282,7 +295,8 @@ typedef struct otThreadDirectLocalSca
  *
  * @retval OT_ERROR_NONE          Parameters stored.
  * @retval OT_ERROR_INVALID_ARGS  @p aParams is NULL, @p aParams->mDuration is 0,
- *                                or @p aParams->mDuration is outside [1, 31].
+ *                                @p aParams->mDuration is outside [1, 31], or
+ *                                @p aParams->mOffsetUs is outside [-1024, 1023].
  */
 otError otThreadDirectSetRamOverride(otInstance *aInstance, const otThreadDirectRamParams *aParams);
 

src/cli/cli_td.cpp

@@ -47,12 +47,16 @@ namespace Cli {
 
 // clang-format off
 const ThreadDirect::Command ThreadDirect::sCommands[] = {
-    {"channel",    &ThreadDirect::ProcessChannel},
-    {"help",       &ThreadDirect::ProcessHelp},
-    {"link",       &ThreadDirect::ProcessLink},
-    {"unlink",     &ThreadDirect::ProcessUnlink},
-    {"wake",       &ThreadDirect::ProcessWake},
+    {"channel", &ThreadDirect::ProcessChannel},
+    {"help",    &ThreadDirect::ProcessHelp},
+    {"link",    &ThreadDirect::ProcessLink},
+    {"unlink",  &ThreadDirect::ProcessUnlink},
+#if OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_INITIATOR_ENABLE
+    {"wake", &ThreadDirect::ProcessWake},
+#endif
+#if OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_LISTENER_ENABLE
     {"wakelisten", &ThreadDirect::ProcessWakeListen},
+#endif
 };
 // clang-format on
 
@@ -128,12 +132,6 @@ otError ThreadDirect::ProcessWake(Arg aArgs[])
 exit:
     return error;
 }
-#else
-otError ThreadDirect::ProcessWake(Arg aArgs[])
-{
-    OT_UNUSED_VARIABLE(aArgs);
-    return OT_ERROR_NOT_IMPLEMENTED;
-}
 #endif // OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_INITIATOR_ENABLE
 
 void ThreadDirect::HandleDirectEvent(otThreadDirectEvent           aEvent,
@@ -175,9 +173,9 @@ void ThreadDirect::HandleDirectEvent(otThreadDirectEvent aEvent, const otThreadD
     }
 }
 
+#if OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_LISTENER_ENABLE
 otError ThreadDirect::ProcessWakeListen(Arg aArgs[])
 {
-#if OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_LISTENER_ENABLE
     otError error = OT_ERROR_NONE;
 
     if (aArgs[0].IsEmpty() || aArgs[0] == "enable" || aArgs[0] == "disable")
@@ -210,11 +208,8 @@ otError ThreadDirect::ProcessWakeListen(Arg aArgs[])
 
 exit:
     return error;
-#else
-    OT_UNUSED_VARIABLE(aArgs);
-    return OT_ERROR_NOT_IMPLEMENTED;
-#endif
 }
+#endif // OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_LISTENER_ENABLE
 
 otError ThreadDirect::ProcessLink(Arg aArgs[])
 {
@@ -315,19 +310,19 @@ otError ThreadDirect::ProcessLinkRam(Arg aArgs[])
     {
         otThreadDirectRamParams params;
         uint8_t                 duration;
-        int32_t                 offsetRaw;
+        int16_t                 offsetUs;
         uint8_t                 hexBuf[4];
         uint16_t                hexLen = sizeof(hexBuf);
 
         memset(&params, 0, sizeof(params));
 
         SuccessOrExit(error = aArgs[1].ParseAsHexString(hexLen, hexBuf));
-        SuccessOrExit(error = aArgs[2].ParseAsInt32(offsetRaw));
+        SuccessOrExit(error = aArgs[2].ParseAsInt16(offsetUs));
         SuccessOrExit(error = aArgs[3].ParseAsUint8(duration));
         VerifyOrExit(aArgs[4].IsEmpty(), error = OT_ERROR_INVALID_ARGS);
 
         memcpy(params.mBits, hexBuf, hexLen);
-        params.mOffsetUs = static_cast<int16_t>(offsetRaw);
+        params.mOffsetUs = offsetUs;
         params.mDuration = duration;
 
         error = otThreadDirectSetRamOverride(GetInstancePtr(), &params);

src/cli/cli_td.hpp

@@ -62,8 +62,12 @@ class ThreadDirect : private Utils
 
     otError ProcessHelp(Arg aArgs[]);
     otError ProcessChannel(Arg aArgs[]);
+#if OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_INITIATOR_ENABLE
     otError ProcessWake(Arg aArgs[]);
+#endif
+#if OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_LISTENER_ENABLE
     otError ProcessWakeListen(Arg aArgs[]);
+#endif
     otError ProcessLink(Arg aArgs[]);
     otError ProcessUnlink(Arg aArgs[]);
 

src/core/api/thread_direct_api.cpp

@@ -43,8 +43,11 @@
 #include "mac/mac_frame.hpp"
 #include "mac/mac_types.hpp"
 #include "mac/sub_mac.hpp"
+
+#if OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_INITIATOR_ENABLE
 #include "mac/wakeup_tx_scheduler.hpp"
 #include "meshcop/dataset_manager.hpp"
+#endif
 
 #if OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_INITIATOR_ENABLE || OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_LISTENER_ENABLE
 
@@ -55,14 +58,15 @@ void otThreadDirectSetEventCallback(otInstance *aInstance, otThreadDirectEventCa
     AsCoreType(aInstance).Get<Mac::Mac>().SetDirectEventCallback(aCallback, aContext);
 }
 
+#if OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_INITIATOR_ENABLE
+
 otError otThreadDirectWakeup(otInstance            *aInstance,
                              const otExtAddress    *aExtAddress,
                              otThreadDirectWakeType aWakeType,
                              uint16_t               aIntervalUs,
                              uint16_t               aDurationMs,
                              uint8_t                aKeyIndex)
 {
-#if OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_INITIATOR_ENABLE
     otError error = OT_ERROR_NONE;
 
     VerifyOrExit(aExtAddress != nullptr, error = OT_ERROR_INVALID_ARGS);
@@ -98,18 +102,15 @@ otError otThreadDirectWakeup(otInstance            *aInstance,
 
 exit:
     return error;
-#else
-    OT_UNUSED_VARIABLE(aInstance);
-    OT_UNUSED_VARIABLE(aExtAddress);
-    OT_UNUSED_VARIABLE(aWakeType);
-    OT_UNUSED_VARIABLE(aIntervalUs);
-    OT_UNUSED_VARIABLE(aDurationMs);
-    OT_UNUSED_VARIABLE(aKeyIndex);
+}
 
-    return OT_ERROR_NOT_IMPLEMENTED;
-#endif
+bool otThreadDirectIsWakeBurstActive(otInstance *aInstance)
+{
+    return AsCoreType(aInstance).Get<WakeupTxScheduler>().IsRunning();
 }
 
+#endif // OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_INITIATOR_ENABLE
+
 otError otThreadDirectUnlink(otInstance *aInstance, const otExtAddress *aExtAddress)
 {
     OT_UNUSED_VARIABLE(aInstance);
@@ -118,37 +119,19 @@ otError otThreadDirectUnlink(otInstance *aInstance, const otExtAddress *aExtAddr
     return OT_ERROR_NOT_IMPLEMENTED;
 }
 
+#if OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_LISTENER_ENABLE
+
 otError otThreadDirectWakeListenerEnable(otInstance *aInstance, bool aEnable)
 {
-#if OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_LISTENER_ENABLE
     return AsCoreType(aInstance).Get<Mac::Mac>().SetWakeupListenEnabled(aEnable);
-#else
-    OT_UNUSED_VARIABLE(aInstance);
-    OT_UNUSED_VARIABLE(aEnable);
-
-    return OT_ERROR_NOT_IMPLEMENTED;
-#endif
 }
 
 bool otThreadDirectIsWakeListenerEnabled(otInstance *aInstance)
 {
-#if OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_LISTENER_ENABLE
     return AsCoreType(aInstance).Get<Mac::Mac>().IsWakeupListenEnabled();
-#else
-    OT_UNUSED_VARIABLE(aInstance);
-    return false;
-#endif
 }
 
-bool otThreadDirectIsWakeBurstActive(otInstance *aInstance)
-{
-#if OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_INITIATOR_ENABLE
-    return AsCoreType(aInstance).Get<WakeupTxScheduler>().IsRunning();
-#else
-    OT_UNUSED_VARIABLE(aInstance);
-    return false;
-#endif
-}
+#endif // OPENTHREAD_CONFIG_THREAD_DIRECT_WAKE_LISTENER_ENABLE
 
 otError otThreadDirectSetSlwSchedule(otInstance *aInstance, uint16_t aSlwPeriodSlots)
 {
@@ -165,6 +148,9 @@ otError otThreadDirectSetRamOverride(otInstance *aInstance, const otThreadDirect
     // a stored state since it has no meaning outside of a per-frame codec context.
     VerifyOrExit(aParams->mDuration >= 1 && aParams->mDuration <= Mac::ScaParams::kRamDurationMax,
                  error = OT_ERROR_INVALID_ARGS);
+    VerifyOrExit(aParams->mOffsetUs >= Mac::ScaParams::kRamOffsetUsMin &&
+                     aParams->mOffsetUs <= Mac::ScaParams::kRamOffsetUsMax,
+                 error = OT_ERROR_INVALID_ARGS);
 
     params.mRamDuration = aParams->mDuration;
     params.mRamOffsetUs = aParams->mOffsetUs;

src/core/config/mac.h

@@ -43,7 +43,6 @@
  * @{
  */
 
-#include "config/thread_direct.h"
 #include "config/time_sync.h"
 
 /**

src/core/mac/direct_handler.cpp

@@ -76,6 +76,9 @@ Error DirectHandler::SetRamMask(const Mac::ScaParams &aParams)
     Error error = kErrorNone;
 
     VerifyOrExit(aParams.mRamDuration <= Mac::ScaParams::kRamDurationMax, error = kErrorInvalidArgs);
+    VerifyOrExit(aParams.mRamOffsetUs >= Mac::ScaParams::kRamOffsetUsMin &&
+                     aParams.mRamOffsetUs <= Mac::ScaParams::kRamOffsetUsMax,
+                 error = kErrorInvalidArgs);
 
     mLocalSca.mRamDuration = aParams.mRamDuration;
     mLocalSca.mRamOffsetUs = aParams.mRamOffsetUs;

src/core/mac/direct_handler.hpp

@@ -79,7 +79,8 @@ class DirectHandler : public InstanceLocator, private NonCopyable
      * @param[in] aParams  RAM parameters to store.
      *
      * @retval kErrorNone         Parameters stored.
-     * @retval kErrorInvalidArgs  @p aParams.mRamDuration is outside [0, 31].
+     * @retval kErrorInvalidArgs  @p aParams.mRamDuration is outside [0, 31] or
+     *                            @p aParams.mRamOffsetUs is outside [-1024, 1023].
      */
     Error SetRamMask(const Mac::ScaParams &aParams);
 

src/core/mac/mac_header_ie.hpp

@@ -313,6 +313,8 @@ struct ScaParams
     static constexpr uint8_t kRamDurationNoChange      = 0;  ///< RAM Duration: no change to prior RAM.
     static constexpr uint8_t kRamDurationNoConstraints = 1;  ///< RAM Duration: device has no CoEx constraints.
     static constexpr uint8_t kRamDurationMax           = 31; ///< Maximum valid RAM Duration value.
+    static constexpr int16_t kRamOffsetUsMin           = -1024;
+    static constexpr int16_t kRamOffsetUsMax           = 1023;
 
     uint16_t mSlwPeriodSlots;            ///< SLW Period in 160 us slots (0 = no SLW schedule configured).
     uint16_t mSlwPhaseSlots;             ///< SLW Phase in 160 us slots.

src/core/mac/mac_types.hpp

@@ -976,7 +976,7 @@ struct WakeupInfo
 {
     ExtAddress mExtAddress;        ///< Extended address of the Wake Initiator.
     uint32_t   mAttachDelayUs;     ///< Rendezvous Time converted to us offset before sending TD Link Command.
-    uint8_t    mRetryInterval : 2; ///< Retry Interval (RI field from Wake Frame, 4-bit wire value).
+    uint8_t    mRetryInterval : 4; ///< Retry Interval (RI field from Wake Frame, 4-bit wire value).
     uint8_t    mRetryCount : 4;    ///< Retry Count (RC field from Wake Frame, 4-bit wire value).
 };
 #endif