more reviews

include/openthread/link.h

@@ -1129,11 +1129,11 @@ otError otLinkSetWakeupChannel(otInstance *aInstance, uint8_t aChannel);
 /**
  * Enables or disables listening for wake-up frames.
  *
+ * Requires `OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE`.
+ *
  * @param[in] aInstance A pointer to an OpenThread instance.
  * @param[in] aEnable true to enable listening for wake-up frames, or false otherwise.
  *
- * Requires `OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE`.
- *
  * @retval OT_ERROR_NONE Successfully enabled / disabled the listening for wake-up frames.
  * @retval OT_ERROR_INVALID_STATE Could not enable listening for wake-up frames due to bad configuration.
  */
@@ -1142,10 +1142,10 @@ otError otLinkSetWedListenEnabled(otInstance *aInstance, bool aEnable);
 /**
  * Returns whether listening for wake-up frames is enabled.
  *
- * @param[in] aInstance A pointer to an OpenThread instance.
- *
  * Requires `OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE`.
  *
+ * @param[in] aInstance A pointer to an OpenThread instance.
+ *
  * @retval TRUE If listening for wake-up frames is enabled.
  * @retval FALSE If listening for wake-up frames is not enabled.
  */

src/cli/README.md

@@ -4412,7 +4412,7 @@ Set the wake-up channel.
 Requires `OPENTHREAD_CONFIG_WAKEUP_COORDINATOR_ENABLE` or `OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE`.
 
 ```bash
-> wakeupchannel 12
+> wakeup channel 12
 Done
 ```
 

src/cli/cli.cpp

@@ -8202,7 +8202,6 @@ template <> otError Interpreter::Process<Cmd("verhoeff")>(Arg aArgs[])
 template <> otError Interpreter::Process<Cmd("wakeup")>(Arg aArgs[])
 {
  otError error = OT_ERROR_NONE;
- bool enable;
 
  /**
  * @cli wakeup channel (get,set)
@@ -8292,33 +8291,19 @@ template <> otError Interpreter::Process<Cmd("wakeup")>(Arg aArgs[])
  * @cparam wakeup listen @ca{enable}
  * @par
  * Gets or sets current wake-up listening link state.
- * @sa otLinkSetWedListenEnabled
  * @sa otLinkIsWedListenEnabled
+ * @sa otLinkSetWedListenEnabled
  */
  else if (aArgs[0] == "listen")
  {
- if (ParseEnableOrDisable(aArgs[1], enable) == OT_ERROR_NONE)
- {
- error = otLinkSetWedListenEnabled(GetInstancePtr(), enable);
- }
- else if (otLinkIsWedListenEnabled(GetInstancePtr()))
- {
- OutputLine("enabled");
- }
- else
- {
- OutputLine("disabled");
- }
- // TODO: extend the output with future MLE APIs (disabled → enabled → linking → linked).
+ error = ProcessEnableDisable(aArgs + 1, otLinkIsWedListenEnabled, otLinkSetWedListenEnabled);
  }
 #endif // OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE
  else
  {
  ExitNow(error = OT_ERROR_INVALID_ARGS);
  }
 
- OT_UNUSED_VARIABLE(enable);
-
 exit:
  return error;
 }

src/core/api/link_api.cpp

@@ -508,7 +508,7 @@ otError otLinkGetRegion(otInstance *aInstance, uint16_t *aRegionCode)
 #if OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE
 otError otLinkSetWedListenEnabled(otInstance *aInstance, bool aEnable)
 {
- return AsCoreType(aInstance).Get<Mac::Mac>().WedListenEnable(aEnable);
+ return AsCoreType(aInstance).Get<Mac::Mac>().SetWedListenEnabled(aEnable);
 }
 
 bool otLinkIsWedListenEnabled(otInstance *aInstance)

src/core/mac/mac.cpp

@@ -1606,7 +1606,7 @@ Error Mac::ProcessReceiveSecurity(RxFrame &aFrame, const Address &aSrcAddr, Neig
  // TODO: Avoid generating a new key if a wake-up frame was recently received already
 
  IgnoreError(aFrame.GetKeyId(keyid));
- sequence = LittleEndian::ReadUint32(aFrame.GetKeySource());
+ sequence = BigEndian::ReadUint32(aFrame.GetKeySource());
  VerifyOrExit(((sequence & 0x7f) + 1) == keyid, error = kErrorSecurity);
 
  macKey = (sequence == keyManager.GetCurrentKeySequence()) ? mLinks.GetCurrentMacKey(aFrame)
@@ -2353,7 +2353,7 @@ void Mac::SetCslPeriod(uint16_t aPeriod)
 #if OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE
  if (IsWedListenEnabled() && aPeriod != 0)
  {
- IgnoreError(WedListenEnable(false));
+ IgnoreError(SetWedListenEnabled(false));
  LogWarn("Disabling wake-up frame listening due to CSL period change");
  }
 #endif
@@ -2487,11 +2487,11 @@ Error Mac::SetWedListenDuration(uint16_t aDuration)
  return error;
 }
 
-Error Mac::WedListenEnable(bool aEnable)
+Error Mac::SetWedListenEnabled(bool aEnable)
 {
  Error error = kErrorNone;
 
- VerifyOrExit(GetWedListenInterval() > GetWedListenDuration());
+ VerifyOrExit(GetWedListenInterval() > GetWedListenDuration(), error = kErrorInvalidArgs);
 
 #if OPENTHREAD_CONFIG_MAC_CSL_RECEIVER_ENABLE
  if (aEnable && IsCslEnabled())
@@ -2567,7 +2567,7 @@ Error Mac::HandleWakeupFrame(const RxFrame &aFrame)
 #endif
 
  // Stop receiving more wake up frames
- IgnoreError(WedListenEnable(false));
+ IgnoreError(SetWedListenEnabled(false));
 
  // TODO: start MLE attach process with the WC
  OT_UNUSED_VARIABLE(attachDelayMs);

src/core/mac/mac.hpp

@@ -742,7 +742,7 @@ class Mac : public InstanceLocator, private NonCopyable
  * @retval kErrorNone Successfully enabled/disabled listening for wake-up frames.
  * @retval kErrorInvalidState Could not enable/disable listening for wake-up frames.
  */
- Error WedListenEnable(bool aEnable);
+ Error SetWedListenEnabled(bool aEnable);
 
  /**
  * Returns whether listening for wake-up frames is enabled.

src/core/mac/sub_mac.hpp

@@ -662,7 +662,7 @@ class SubMac : public InstanceLocator, private NonCopyable
  uint16_t mWedListenDuration; // The WED listen duration, in microseconds.
  uint8_t mWakeupChannel; // The wake-up sample channel.
  TimeMicro mWedSampleTime; // The WED sample time of the current interval in local time.
- TimeMicro mWedSampleTimeRadio; // The WED sample time of the current interval in radio time.
+ uint64_t  mWedSampleTimeRadio; // The WED sample time of the current interval in radio time.
  TimerMicro mWedTimer;
 #endif
 };

src/core/mac/sub_mac_wed.cpp

@@ -50,8 +50,7 @@ void SubMac::WedInit(void)
 
 void SubMac::UpdateWakeupListening(uint32_t aInterval, uint16_t aDuration, uint8_t aChannel)
 {
- OT_ASSERT(RadioSupportsReceiveTiming());
-
+ VerifyOrExit(RadioSupportsReceiveTiming());
  VerifyOrExit(aInterval != mWedListenInterval || aDuration != mWedListenDuration || aChannel != mWakeupChannel);
 
  mWedListenInterval = aInterval;
@@ -62,7 +61,7 @@ void SubMac::UpdateWakeupListening(uint32_t aInterval, uint16_t aDuration, uint8
  if (mWedListenInterval > 0)
  {
  mWedSampleTime = TimerMicro::GetNow() + kCslReceiveTimeAhead;
- mWedSampleTimeRadio = TimeMicro((uint32_t)otPlatRadioGetNow(&GetInstance()) + kCslReceiveTimeAhead);
+ mWedSampleTimeRadio = otPlatRadioGetNow(&GetInstance()) + kCslReceiveTimeAhead;
  HandleWedTimer();
  }
 
@@ -74,9 +73,10 @@ void SubMac::HandleWedTimer(Timer &aTimer) { aTimer.Get<SubMac>().HandleWedTimer
 
 void SubMac::HandleWedTimer(void)
 {
- if (RadioSupportsReceiveTiming() && (mState != kStateDisabled))
+ if (mState != kStateDisabled)
  {
- IgnoreError(Get<Radio>().ReceiveAt(mWakeupChannel, mWedSampleTimeRadio.GetValue(), mWedListenDuration));
+ IgnoreError(
+ Get<Radio>().ReceiveAt(mWakeupChannel, static_cast<uint32_t>(mWedSampleTimeRadio), mWedListenDuration));
  }
  mWedSampleTime += mWedListenInterval;
  mWedSampleTimeRadio += mWedListenInterval;