From 46323971615756d1c975a6a69fc285cb0ec29e15 Mon Sep 17 00:00:00 2001 From: Eduardo Montoya Date: Wed, 2 Oct 2024 15:57:18 +0200 Subject: [PATCH] review changes --- include/openthread/link.h | 23 +++++-- src/cli/README.md | 74 ++++++++++++--------- src/cli/cli.cpp | 123 ++++++++++++++++------------------- src/core/BUILD.gn | 2 + src/core/api/link_api.cpp | 39 ++--------- src/core/mac/mac.cpp | 16 ++++- src/core/mac/mac.hpp | 17 +++-- src/core/mac/sub_mac.hpp | 9 +-- src/core/mac/sub_mac_wed.cpp | 6 +- 9 files changed, 153 insertions(+), 156 deletions(-) diff --git a/include/openthread/link.h b/include/openthread/link.h index b1c3194ffdf1..e80a35e8d791 100644 --- a/include/openthread/link.h +++ b/include/openthread/link.h @@ -1132,14 +1132,21 @@ otError otLinkSetWakeupChannel(otInstance *aInstance, uint8_t aChannel); * @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. */ -otError otLinkWedListenSetEnabled(otInstance *aInstance, bool aEnable); +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`. + * + * Requires `OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE`. * @retval TRUE If listening for wake-up frames is enabled. * @retval FALSE If listening for wake-up frames is not enabled. */ @@ -1148,6 +1155,8 @@ bool otLinkIsWedListenEnabled(otInstance *aInstance); /** * Gets the WED listen interval in microseconds. * + * Requires `OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE`. + * * @param[in] aInstance A pointer to an OpenThread instance. * * @returns The WED listen interval in microseconds. @@ -1157,20 +1166,18 @@ uint32_t otLinkGetWedListenInterval(otInstance *aInstance); /** * Sets the WED listen interval in microseconds. * - * The WED listen interval must be a multiple of `OT_LINK_CSL_PERIOD_TEN_SYMBOLS_UNIT_IN_USEC`, otherwise - * `OT_ERROR_INVALID_ARGS` is returned. + * Requires `OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE`. * * @param[in] aInstance A pointer to an OpenThread instance. * @param[in] aInterval The WED listen interval in microseconds. - * - * @retval OT_ERROR_NONE Successfully set the WED listen interval. - * @retval OT_ERROR_INVALID_ARGS Invalid WED listen interval. */ -otError otLinkSetWedListenInterval(otInstance *aInstance, uint32_t aInterval); +void otLinkSetWedListenInterval(otInstance *aInstance, uint32_t aInterval); /** * Gets the WED listen duration. * + * Requires `OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE`. + * * @param[in] aInstance A pointer to an OpenThread instance. * * @returns The WED listen duration in microseconds. @@ -1180,6 +1187,8 @@ uint16_t otLinkGetWedListenDuration(otInstance *aInstance); /** * Sets the WED listen duration in microseconds. * + * Requires `OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE`. + * * @param[in] aInstance A pointer to an OpenThread instance. * @param[in] aDuration The WED listen duration in microseconds. * diff --git a/src/cli/README.md b/src/cli/README.md index 3dadfe3a9769..aa548275dd87 100644 --- a/src/cli/README.md +++ b/src/cli/README.md @@ -133,8 +133,7 @@ Done - [vendor](#vendor-name) - [verhoeff](#verhoeff-calculate) - [version](#version) -- [wakeup](#wakeup) -- [wakeupchannel](#wakeupchannel) +- [wakeup](#wakeup-channel) ## OpenThread Command Details @@ -4394,83 +4393,94 @@ Factory Diagnostics module is enabled only when building OpenThread with `OPENTH [diag]: ../../src/core/diags/README.md -### wakeup +### wakeup channel -Get the Wake-up End Device listen configuration. +Get the wake-up channel. -`OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE` is required. +Requires `OPENTHREAD_CONFIG_WAKEUP_COORDINATOR_ENABLE` or `OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE`. ```bash -> wakeup -channel: 12 -interval: 1000000us -duration: 8000us +> wakeup channel +12 Done ``` -### wakeup enable +### wakeup channel \ -Enable the WED listening feature. +Set the wake-up channel. -`OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE` is required. +Requires `OPENTHREAD_CONFIG_WAKEUP_COORDINATOR_ENABLE` or `OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE`. ```bash -> wakeup enable +> wakeupchannel 12 Done ``` -### wakeup disable +### wakeup interval -Disable the WED listening feature. +Get the wake-up interval. -`OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE` is required. +Requires `OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE`. ```bash -> wakeup disable +> wakeup interval +1000000 Done ``` -### wakeup state +### wakeup interval \ -Shows the WED listening state, among `disabled` and `enabled`. +Set the wake-up interval. -`OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE` is required. +Requires `OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE`. ```bash -> wakeup state -enabled +> wakeup interval 1000000 Done ``` -### wakeup interval \ +### wakeup duration -Set the WED listen interval in microseconds. Disable WED listening by setting this parameter to `0`. +Get the wake-up duration. -`OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE` is required. +Requires `OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE`. ```bash -> wakeup interval 1000000 +> wakeup duration +8000 Done ``` ### wakeup duration \ -Set the WED listen duration in microseconds. +Set the wake-up duration. -`OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE` is required. +Requires `OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE`. ```bash > wakeup duration 8000 Done ``` -### wakeupchannel \ +### wakeup listen -Set the wake-up channel. +Show the state of wake-up listening feature. -Requires `OPENTHREAD_CONFIG_WAKEUP_COORDINATOR_ENABLE` or `OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE`. +`OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE` is required. ```bash -> wakeupchannel 12 +> wakeup listen +Enabled +Done +``` + +### wakeup listen \[enable|disable\] + +Enable/disable listening for wake-up frames. + +`OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE` is required. + +```bash +> wakeup listen enable Done ``` diff --git a/src/cli/cli.cpp b/src/cli/cli.cpp index c01082726dfa..e764d6e1b929 100644 --- a/src/cli/cli.cpp +++ b/src/cli/cli.cpp @@ -8199,118 +8199,109 @@ template <> otError Interpreter::Process(Arg aArgs[]) #endif // OPENTHREAD_CONFIG_VERHOEFF_CHECKSUM_ENABLE #if OPENTHREAD_CONFIG_WAKEUP_COORDINATOR_ENABLE || OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE -/** - * @cli wakeupchannel (get,set) - * @code - * wakeupchannel - * 12 - * Done - * @endcode - * @code - * wakeupchannel 12 - * Done - * @endcode - * @cparam wakeupchannel [@ca{channel}] - * Use `channel` to set the wake-up channel. - * @par - * Gets or sets the wake-up channel value. - */ -template <> otError Interpreter::Process(Arg aArgs[]) -{ - return ProcessGetSet(aArgs, otLinkGetWakeupChannel, otLinkSetWakeupChannel); -} - -#if OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE template <> otError Interpreter::Process(Arg aArgs[]) { otError error = OT_ERROR_NONE; bool enable; /** - * @cli wakeup + * @cli wakeup channel (get,set) * @code - * wakeup - * channel: 12 - * interval: 1000000us - * duration: 8000us + * wakeup channel + * 12 * Done * @endcode + * @code + * wakeup channel 12 + * Done + * @endcode + * @cparam wakeup channel [@ca{channel}] + * Use `channel` to set the wake-up channel. * @par - * Gets the wake-up listening configuration. + * Gets or sets the wake-up channel value. * @sa otLinkGetWakeupChannel - * @sa otLinkGetWedListenInterval - * @sa otLinkGetWedListenDuration + * @sa otLinkSetWakeupChannel */ - if (aArgs[0].IsEmpty()) + if (aArgs[0] == "channel") { - OutputLine("channel: %u", otLinkGetWakeupChannel(GetInstancePtr())); - OutputLine("interval: %luus", ToUlong(otLinkGetWedListenInterval(GetInstancePtr()))); - OutputLine("duration: %luus", ToUlong(otLinkGetWedListenDuration(GetInstancePtr()))); + error = ProcessGetSet(aArgs, otLinkGetWakeupChannel, otLinkSetWakeupChannel); } +#if OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE /** - * @cli wakeup (enable,disable) + * @cli wakeup interval (get,set) * @code - * wakeup enable + * wakeup interval + * 12 * Done * @endcode * @code - * wakeup disable - * Done - * @endcode - * @par api_copy - * #otLinkWedListenSetEnabled - */ - else if (ParseEnableOrDisable(aArgs[0], enable) == OT_ERROR_NONE) - { - error = otLinkWedListenSetEnabled(GetInstancePtr(), enable); - } - /** - * @cli wakeup interval - * @code * wakeup interval 1000000 * Done * @endcode * @cparam wakeup interval @ca{interval} - * @par api_copy - * #otLinkSetWedListenInterval + * @par + * Gets or sets the wake-up interval value. + * @sa otLinkGetWedListenInterval + * @sa otLinkSetWedListenInterval */ else if (aArgs[0] == "interval") { - error = ProcessSet(aArgs + 1, otLinkSetWedListenInterval); + error = ProcessGetSet(aArgs + 1, otLinkGetWedListenInterval, otLinkSetWedListenInterval); } /** - * @cli wakeup duration + * @cli wakeup duration (get,set) + * @code + * wakeup duration + * 8000 + * Done + * @endcode * @code * wakeup duration 8000 * Done * @endcode * @cparam wakeup duration @ca{duration} - * @par api_copy - * #otLinkSetWedListenDuration + * @par + * Gets or sets the wake-up duration value. + * @sa otLinkGetWedListenDuration + * @sa otLinkSetWedListenDuration */ else if (aArgs[0] == "duration") { - error = ProcessSet(aArgs + 1, otLinkSetWedListenDuration); + error = ProcessGetSet(aArgs + 1, otLinkGetWedListenDuration, otLinkSetWedListenDuration); } /** - * @cli wakeup state + * @cli wakeup listen (enable,disable) * @code - * wakeup state + * wakeup listen * disabled * Done * @endcode * @code - * wakeup state + * wakeup listen * enabled * Done * @endcode + * @code + * wakeup listen enable + * Done + * @endcode + * @code + * wakeup listen disable + * Done + * @endcode + * @cparam wakeup listen @ca{enable} * @par - * Prints current wake-up listening link state. - * #otLinkIsWedListenEnabled + * Gets or sets current wake-up listening link state. + * @sa otLinkSetWedListenEnabled + * @sa otLinkIsWedListenEnabled */ - else if (aArgs[0] == "state") + else if (aArgs[0] == "listen") { - if (otLinkIsWedListenEnabled(GetInstancePtr())) + if (ParseEnableOrDisable(aArgs[1], enable) == OT_ERROR_NONE) + { + error = otLinkSetWedListenEnabled(GetInstancePtr(), enable); + } + else if (otLinkIsWedListenEnabled(GetInstancePtr())) { OutputLine("enabled"); } @@ -8318,7 +8309,9 @@ template <> otError Interpreter::Process(Arg aArgs[]) { OutputLine("disabled"); } + // TODO: extend the output with future MLE APIs (disabled → enabled → linking → linked). } +#endif // OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE else { ExitNow(error = OT_ERROR_INVALID_ARGS); @@ -8327,7 +8320,6 @@ template <> otError Interpreter::Process(Arg aArgs[]) exit: return error; } -#endif // OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE #endif // OPENTHREAD_CONFIG_WAKEUP_COORDINATOR_ENABLE || OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE #endif // OPENTHREAD_FTD || OPENTHREAD_MTD @@ -8640,10 +8632,7 @@ otError Interpreter::ProcessCommand(Arg aArgs[]) #endif // OPENTHREAD_FTD || OPENTHREAD_MTD CmdEntry("version"), #if OPENTHREAD_FTD || OPENTHREAD_MTD -#if OPENTHREAD_CONFIG_WAKEUP_COORDINATOR_ENABLE || OPENTHREAD_CONFIG_MAC_CSL_PERIPHERAL_ENABLE CmdEntry("wakeup"), - CmdEntry("wakeupchannel"), -#endif #endif }; diff --git a/src/core/BUILD.gn b/src/core/BUILD.gn index 228a1c327dd5..962a7b923de8 100644 --- a/src/core/BUILD.gn +++ b/src/core/BUILD.gn @@ -778,6 +778,8 @@ openthread_radio_sources = [ "mac/mac_types.cpp", "mac/sub_mac.cpp", "mac/sub_mac_callbacks.cpp", + "mac/sub_mac_csl_receiver.cpp", + "mac/sub_mac_wed.cpp", "radio/radio.cpp", "radio/radio_callbacks.cpp", "radio/radio_platform.cpp", diff --git a/src/core/api/link_api.cpp b/src/core/api/link_api.cpp index 7cf7b30bda72..51d86d267519 100644 --- a/src/core/api/link_api.cpp +++ b/src/core/api/link_api.cpp @@ -506,16 +506,9 @@ otError otLinkGetRegion(otInstance *aInstance, uint16_t *aRegionCode) } #if OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE -otError otLinkWedListenSetEnabled(otInstance *aInstance, bool aEnable) +otError otLinkSetWedListenEnabled(otInstance *aInstance, bool aEnable) { - Error error = kErrorInvalidState; - - VerifyOrExit(otLinkGetWedListenInterval(aInstance) > otLinkGetWedListenDuration(aInstance)); - - error = AsCoreType(aInstance).Get().WedListenEnable(aEnable); - -exit: - return error; + return AsCoreType(aInstance).Get().WedListenEnable(aEnable); } bool otLinkIsWedListenEnabled(otInstance *aInstance) @@ -528,25 +521,9 @@ uint32_t otLinkGetWedListenInterval(otInstance *aInstance) return AsCoreType(aInstance).Get().GetWedListenInterval(); } -otError otLinkSetWedListenInterval(otInstance *aInstance, uint32_t aInterval) +void otLinkSetWedListenInterval(otInstance *aInstance, uint32_t aInterval) { - Error error = kErrorNone; - uint16_t intervalInTenSymbolsUnit; - - if (aInterval == 0) - { - intervalInTenSymbolsUnit = 0; - } - else - { - VerifyOrExit((aInterval % kUsPerTenSymbols) == 0, error = kErrorInvalidArgs); - intervalInTenSymbolsUnit = ClampToUint16(aInterval / kUsPerTenSymbols); - } - - AsCoreType(aInstance).Get().SetWedListenInterval(intervalInTenSymbolsUnit); - -exit: - return error; + return AsCoreType(aInstance).Get().SetWedListenInterval(aInterval); } uint16_t otLinkGetWedListenDuration(otInstance *aInstance) @@ -556,12 +533,6 @@ uint16_t otLinkGetWedListenDuration(otInstance *aInstance) otError otLinkSetWedListenDuration(otInstance *aInstance, uint16_t aDuration) { - Error error = kErrorNone; - - VerifyOrExit(aDuration >= kMinWedListenDuration, error = kErrorInvalidArgs); - AsCoreType(aInstance).Get().SetWedListenDuration(aDuration); - -exit: - return error; + return AsCoreType(aInstance).Get().SetWedListenDuration(aDuration); } #endif // OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE diff --git a/src/core/mac/mac.cpp b/src/core/mac/mac.cpp index 2617bad7e0b6..5dc73ba24865 100644 --- a/src/core/mac/mac.cpp +++ b/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 = BigEndian::ReadUint32(aFrame.GetKeySource()); + sequence = LittleEndian::ReadUint32(aFrame.GetKeySource()); VerifyOrExit(((sequence & 0x7f) + 1) == keyid, error = kErrorSecurity); macKey = (sequence == keyManager.GetCurrentKeySequence()) ? mLinks.GetCurrentMacKey(aFrame) @@ -2474,18 +2474,27 @@ void Mac::SetWedListenInterval(uint32_t aInterval) UpdateWakeupListening(); } -void Mac::SetWedListenDuration(uint16_t aDuration) +Error Mac::SetWedListenDuration(uint16_t aDuration) { + Error error = kErrorNone; + + VerifyOrExit(aDuration >= kMinWedListenDuration, error = kErrorInvalidArgs); + mWedListenDuration = aDuration; UpdateWakeupListening(); + +exit: + return error; } Error Mac::WedListenEnable(bool aEnable) { Error error = kErrorNone; + VerifyOrExit(GetWedListenInterval() > GetWedListenDuration()); + #if OPENTHREAD_CONFIG_MAC_CSL_RECEIVER_ENABLE - if (aEnable == true && IsCslEnabled()) + if (aEnable && IsCslEnabled()) { LogWarn("Cannot enable wake-up frame listening while CSL is enabled"); ExitNow(error = kErrorInvalidState); @@ -2561,6 +2570,7 @@ Error Mac::HandleWakeupFrame(const RxFrame &aFrame) IgnoreError(WedListenEnable(false)); // TODO: start MLE attach process with the WC + OT_UNUSED_VARIABLE(attachDelayMs); exit: return error; diff --git a/src/core/mac/mac.hpp b/src/core/mac/mac.hpp index f2787f8a2dd9..6550983638c3 100644 --- a/src/core/mac/mac.hpp +++ b/src/core/mac/mac.hpp @@ -684,17 +684,17 @@ class Mac : public InstanceLocator, private NonCopyable Error GetRegion(uint16_t &aRegionCode) const; /** - * Gets the Wake-up channel. + * Gets the wake-up channel. * - * @returns Wake-up channel. + * @returns wake-up channel. */ uint8_t GetWakeupChannel(void) const { return mWakeupChannel; } #if OPENTHREAD_CONFIG_WAKEUP_COORDINATOR_ENABLE || OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE /** - * Sets the Wake-up channel. + * Sets the wake-up channel. * - * @param[in] aChannel The Wake-up channel. + * @param[in] aChannel The wake-up channel. * * @retval kErrorNone Successfully set the wake-up channel. * @retval kErrorInvalidArgs The @p aChannel is not in the supported channel mask. @@ -720,16 +720,19 @@ class Mac : public InstanceLocator, private NonCopyable /** * Gets the WED listen duration. * - * @returns WED listen duration in us. + * @returns WED listen duration in microseconds. */ uint16_t GetWedListenDuration(void) const { return mWedListenDuration; } /** * Sets the WED listen duration. * - * @param[in] aDuration The WED listen duration in us. + * @param[in] aDuration The WED listen duration in microseconds. + * + * @retval kErrorNone Successfully set the WED listen duration. + * @retval kErrorInvalidArgs The @p aDuration is below the minimum supported. */ - void SetWedListenDuration(uint16_t aDuration); + Error SetWedListenDuration(uint16_t aDuration); /** * Enables/disables listening for wake-up frames. diff --git a/src/core/mac/sub_mac.hpp b/src/core/mac/sub_mac.hpp index a1915a4c0932..3e53b5f0b621 100644 --- a/src/core/mac/sub_mac.hpp +++ b/src/core/mac/sub_mac.hpp @@ -658,10 +658,11 @@ class SubMac : public InstanceLocator, private NonCopyable #endif #if OPENTHREAD_CONFIG_WAKEUP_END_DEVICE_ENABLE - uint32_t mWedListenInterval; // The WED listen interval, in microseconds. - 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. + uint32_t mWedListenInterval; // The WED listen interval, in microseconds. + 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. TimerMicro mWedTimer; #endif }; diff --git a/src/core/mac/sub_mac_wed.cpp b/src/core/mac/sub_mac_wed.cpp index b02790f6010f..828f14da8d9b 100644 --- a/src/core/mac/sub_mac_wed.cpp +++ b/src/core/mac/sub_mac_wed.cpp @@ -61,7 +61,8 @@ void SubMac::UpdateWakeupListening(uint32_t aInterval, uint16_t aDuration, uint8 if (mWedListenInterval > 0) { - mWedSampleTime = TimeMicro(otPlatRadioGetNow(&GetInstance())) + kCslReceiveTimeAhead; + mWedSampleTime = TimerMicro::GetNow() + kCslReceiveTimeAhead; + mWedSampleTimeRadio = TimeMicro((uint32_t)otPlatRadioGetNow(&GetInstance()) + kCslReceiveTimeAhead); HandleWedTimer(); } @@ -75,9 +76,10 @@ void SubMac::HandleWedTimer(void) { if (RadioSupportsReceiveTiming() && (mState != kStateDisabled)) { - IgnoreError(Get().ReceiveAt(mWakeupChannel, mWedSampleTime.GetValue(), mWedListenDuration)); + IgnoreError(Get().ReceiveAt(mWakeupChannel, mWedSampleTimeRadio.GetValue(), mWedListenDuration)); } mWedSampleTime += mWedListenInterval; + mWedSampleTimeRadio += mWedListenInterval; mWedTimer.FireAt(mWedSampleTime - kCslReceiveTimeAhead); }