From 89627809ff06c467b9f794599dac659affa6faae Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Wed, 23 Sep 2026 21:26:11 -0700 Subject: [PATCH] [radio] add Radio::IsChannelValid() helper (#13637) This commit introduces `Radio::IsChannelValid(uint16_t aChannel)` to check if a channel number falls within `[Radio::kChannelMin, Radio::kChannelMax]`. - Replaces `Radio::IsCslChannelValid()` with `Radio::IsChannelValid()`, handling the special case of CSL channel 0 (disabled) explicitly at call sites in `otLinkSetCslChannel()` and `ChannelManager`. - Eliminates private duplicate channel validation helpers from `PowerCalibration` and `Diags`. - Consolidates duplicated channel range checks across `ChannelTlvValue`, `ChannelMonitor`, `PowerCalibration`, `Diags`, and `RadioSpinel`. --- src/core/api/link_api.cpp | 5 ++++- src/core/diags/factory_diags.cpp | 15 +++++---------- src/core/diags/factory_diags.hpp | 1 - src/core/radio/radio.cpp | 6 ------ src/core/radio/radio.hpp | 14 ++++++++++---- src/core/thread/mle_tlvs.cpp | 13 +------------ src/core/utils/channel_manager.cpp | 5 ++++- src/core/utils/channel_monitor.cpp | 2 +- src/core/utils/power_calibration.cpp | 7 ++++--- src/core/utils/power_calibration.hpp | 4 ---- src/lib/spinel/radio_spinel.cpp | 5 +++-- 11 files changed, 32 insertions(+), 45 deletions(-) diff --git a/src/core/api/link_api.cpp b/src/core/api/link_api.cpp index 3308ec094..6dba9c7aa 100644 --- a/src/core/api/link_api.cpp +++ b/src/core/api/link_api.cpp @@ -415,7 +415,10 @@ otError otLinkSetCslChannel(otInstance *aInstance, uint8_t aChannel) { Error error = kErrorNone; - VerifyOrExit(Radio::IsCslChannelValid(aChannel), error = kErrorInvalidArgs); + if (aChannel != 0) + { + VerifyOrExit(Radio::IsChannelValid(aChannel), error = kErrorInvalidArgs); + } AsCoreType(aInstance).Get().SetCslChannel(aChannel); diff --git a/src/core/diags/factory_diags.cpp b/src/core/diags/factory_diags.cpp index 9bd3627a4..a5776c7ca 100644 --- a/src/core/diags/factory_diags.cpp +++ b/src/core/diags/factory_diags.cpp @@ -88,7 +88,7 @@ Error Diags::ProcessChannel(uint8_t aArgsLength, char *aArgs[]) VerifyOrExit(aArgsLength == 1, error = kErrorInvalidArgs); SuccessOrExit(error = Utils::CmdLineParser::ParseAsUint8(aArgs[0], channel)); - VerifyOrExit(IsChannelValid(channel), error = kErrorInvalidArgs); + VerifyOrExit(Radio::IsChannelValid(channel), error = kErrorInvalidArgs); otPlatDiagChannelSet(channel); @@ -262,7 +262,7 @@ Error Diags::ProcessFrame(uint8_t aArgsLength, char *aArgs[]) VerifyOrExit(aArgsLength > 1, error = kErrorInvalidArgs); SuccessOrExit(error = Utils::CmdLineParser::ParseAsUint8(aArgs[0], rxChannelAfterTxDone)); - VerifyOrExit(IsChannelValid(rxChannelAfterTxDone), error = kErrorInvalidArgs); + VerifyOrExit(Radio::IsChannelValid(rxChannelAfterTxDone), error = kErrorInvalidArgs); } else if (StringMatch(aArgs[0], "-d")) { @@ -342,7 +342,7 @@ Error Diags::ProcessChannel(uint8_t aArgsLength, char *aArgs[]) uint8_t channel; SuccessOrExit(error = Utils::CmdLineParser::ParseAsUint8(aArgs[0], channel)); - VerifyOrExit(IsChannelValid(channel), error = kErrorInvalidArgs); + VerifyOrExit(Radio::IsChannelValid(channel), error = kErrorInvalidArgs); mChannel = channel; otPlatDiagChannelSet(mChannel); @@ -928,7 +928,7 @@ void Diags::TransmitDone(Error aError) if (mCurTxCmd == kTxCmdSweep) { - if (IsChannelValid(mChannel + 1)) + if (Radio::IsChannelValid(mChannel + 1)) { mChannel += 1; otPlatDiagChannelSet(mChannel); @@ -1091,7 +1091,7 @@ Error Diags::ProcessPowerSettings(uint8_t aArgsLength, char *aArgs[]) else if (aArgsLength == 1) { SuccessOrExit(error = Utils::CmdLineParser::ParseAsUint8(aArgs[0], channel)); - VerifyOrExit(IsChannelValid(channel), error = kErrorInvalidArgs); + VerifyOrExit(Radio::IsChannelValid(channel), error = kErrorInvalidArgs); SuccessOrExit(error = GetPowerSettings(channel, powerSettings)); Output("TargetPower(0.01dBm): %d\r\nActualPower(0.01dBm): %d\r\nRawPowerSetting: %s\r\n", @@ -1187,11 +1187,6 @@ exit: return error; } -bool Diags::IsChannelValid(uint8_t aChannel) -{ - return (aChannel >= Radio::kChannelMin && aChannel <= Radio::kChannelMax); -} - bool Diags::IsFrameLengthValid(uint16_t aLength) { return (aLength >= Radio::kFrameMinSize) && (aLength <= Radio::kFrameMaxSize); diff --git a/src/core/diags/factory_diags.hpp b/src/core/diags/factory_diags.hpp index 4f8347f38..f1f98ae5c 100644 --- a/src/core/diags/factory_diags.hpp +++ b/src/core/diags/factory_diags.hpp @@ -243,7 +243,6 @@ private: void OutputStats(void); void UpdateTxStats(Error aError); - static bool IsChannelValid(uint8_t aChannel); static bool IsFrameLengthValid(uint16_t aLength); static const struct Command sCommands[]; diff --git a/src/core/radio/radio.cpp b/src/core/radio/radio.cpp index 7c11b4d45..48b0f5e83 100644 --- a/src/core/radio/radio.cpp +++ b/src/core/radio/radio.cpp @@ -72,12 +72,6 @@ uint32_t ChannelMaskForPage(uint8_t aChannelPage) return mask; } -bool IsCslChannelValid(uint8_t aCslChannel) -{ - return ((aCslChannel == 0) || - ((kChannelMin == aCslChannel) || ((kChannelMin < aCslChannel) && (aCslChannel <= kChannelMax)))); -} - //--------------------------------------------------------------------------------------------------------------------- #if OPENTHREAD_RADIO diff --git a/src/core/radio/radio.hpp b/src/core/radio/radio.hpp index e23841280..79a99a0db 100644 --- a/src/core/radio/radio.hpp +++ b/src/core/radio/radio.hpp @@ -43,6 +43,7 @@ #include "common/locator.hpp" #include "common/non_copyable.hpp" +#include "common/num_utils.hpp" #include "common/numeric_limits.hpp" #include "common/time.hpp" #include "mac/mac_frame.hpp" @@ -175,12 +176,17 @@ constexpr bool SupportsChannelPage(uint8_t aChannelPage) uint32_t ChannelMaskForPage(uint8_t aChannelPage); /** - * Checks if a given channel is valid as a CSL channel. + * Checks if a given channel is a valid channel number (within `[kChannelMin, kChannelMax]`). * - * @retval true The channel is valid. - * @retval false The channel is invalid. + * @param[in] aChannel The channel number to check. + * + * @retval TRUE The channel is within `[kChannelMin, kChannelMax]`. + * @retval FALSE The channel is outside `[kChannelMin, kChannelMax]`. */ -bool IsCslChannelValid(uint8_t aCslChannel); +constexpr bool IsChannelValid(uint16_t aChannel) +{ + return IsValueInRange(aChannel, kChannelMin, kChannelMax); +} class Radio; diff --git a/src/core/thread/mle_tlvs.cpp b/src/core/thread/mle_tlvs.cpp index 3a7b2dca9..1cd174160 100644 --- a/src/core/thread/mle_tlvs.cpp +++ b/src/core/thread/mle_tlvs.cpp @@ -278,18 +278,7 @@ void ChannelTlvValue::SetChannelAndPage(uint16_t aChannel) bool ChannelTlvValue::IsValid(void) const { - bool isValid = false; - uint16_t channel; - - VerifyOrExit(Radio::SupportsChannelPage(mChannelPage)); - - channel = GetChannel(); - VerifyOrExit((Radio::kChannelMin <= channel) && (channel <= Radio::kChannelMax)); - - isValid = true; - -exit: - return isValid; + return Radio::SupportsChannelPage(mChannelPage) && Radio::IsChannelValid(GetChannel()); } //--------------------------------------------------------------------------------------------------------------------- diff --git a/src/core/utils/channel_manager.cpp b/src/core/utils/channel_manager.cpp index d46ab3f8a..93733d3e7 100644 --- a/src/core/utils/channel_manager.cpp +++ b/src/core/utils/channel_manager.cpp @@ -124,7 +124,10 @@ void ChannelManager::ChangeCslChannel(uint8_t aChannel) ExitNow(); } - VerifyOrExit(Radio::IsCslChannelValid(aChannel)); + if (aChannel != 0) + { + VerifyOrExit(Radio::IsChannelValid(aChannel)); + } LogInfo("Change to Csl channel %d now.", aChannel); diff --git a/src/core/utils/channel_monitor.cpp b/src/core/utils/channel_monitor.cpp index 4ebc6c699..28894dbda 100644 --- a/src/core/utils/channel_monitor.cpp +++ b/src/core/utils/channel_monitor.cpp @@ -104,7 +104,7 @@ uint16_t ChannelMonitor::GetChannelOccupancy(uint8_t aChannel) const { uint16_t occupancy = 0; - VerifyOrExit((Radio::kChannelMin <= aChannel) && (aChannel <= Radio::kChannelMax)); + VerifyOrExit(Radio::IsChannelValid(aChannel)); occupancy = mChannelOccupancy[aChannel - Radio::kChannelMin]; exit: diff --git a/src/core/utils/power_calibration.cpp b/src/core/utils/power_calibration.cpp index 6164aa22c..82c4c2814 100644 --- a/src/core/utils/power_calibration.cpp +++ b/src/core/utils/power_calibration.cpp @@ -85,7 +85,8 @@ Error PowerCalibration::AddCalibratedPower(uint8_t aChannel, uint8_t chIndex; AssertPointerIsNotNull(aRawPowerSetting); - VerifyOrExit(IsChannelValid(aChannel) && aRawPowerSettingLength <= CalibratedPowerEntry::kMaxRawPowerSettingSize, + VerifyOrExit(Radio::IsChannelValid(aChannel) && + aRawPowerSettingLength <= CalibratedPowerEntry::kMaxRawPowerSettingSize, error = kErrorInvalidArgs); chIndex = aChannel - Radio::kChannelMin; @@ -118,7 +119,7 @@ Error PowerCalibration::SetChannelTargetPower(uint8_t aChannel, int16_t aTargetP { Error error = kErrorNone; - VerifyOrExit(IsChannelValid(aChannel), error = kErrorInvalidArgs); + VerifyOrExit(Radio::IsChannelValid(aChannel), error = kErrorInvalidArgs); mTargetPowerTable[aChannel - Radio::kChannelMin] = aTargetPower; if (aChannel == mLastChannel) @@ -145,7 +146,7 @@ Error PowerCalibration::GetPowerSettings(uint8_t aChannel, int16_t minPower = NumericLimits::kMax; uint8_t minPowerIndex = kInvalidIndex; - VerifyOrExit(IsChannelValid(aChannel), error = kErrorInvalidArgs); + VerifyOrExit(Radio::IsChannelValid(aChannel), error = kErrorInvalidArgs); VerifyOrExit((mLastChannel != aChannel) || IsPowerUpdated()); chIndex = aChannel - Radio::kChannelMin; diff --git a/src/core/utils/power_calibration.hpp b/src/core/utils/power_calibration.hpp index f607b66c6..8c64b264e 100644 --- a/src/core/utils/power_calibration.hpp +++ b/src/core/utils/power_calibration.hpp @@ -143,10 +143,6 @@ private: }; bool IsPowerUpdated(void) const { return mCalibratedPowerIndex == kInvalidIndex; } - bool IsChannelValid(uint8_t aChannel) const - { - return ((aChannel >= Radio::kChannelMin) && (aChannel <= Radio::kChannelMax)); - } static constexpr uint8_t kInvalidIndex = NumericLimits::kMax; static constexpr uint16_t kInvalidPower = NumericLimits::kMax; diff --git a/src/lib/spinel/radio_spinel.cpp b/src/lib/spinel/radio_spinel.cpp index 695932d7f..fe82197f4 100644 --- a/src/lib/spinel/radio_spinel.cpp +++ b/src/lib/spinel/radio_spinel.cpp @@ -2362,7 +2362,8 @@ exit: otError RadioSpinel::SetChannelMaxTransmitPower(uint8_t aChannel, int8_t aMaxPower) { otError error = OT_ERROR_NONE; - VerifyOrExit(aChannel >= Radio::kChannelMin && aChannel <= Radio::kChannelMax, error = OT_ERROR_INVALID_ARGS); + + VerifyOrExit(Radio::IsChannelValid(aChannel), error = OT_ERROR_INVALID_ARGS); mMaxPowerTable.SetTransmitPower(aChannel, aMaxPower); SuccessOrExit(error = Set(SPINEL_PROP_PHY_CHAN_MAX_POWER, SPINEL_DATATYPE_UINT8_S SPINEL_DATATYPE_INT8_S, aChannel, aMaxPower)); @@ -2486,7 +2487,7 @@ otError RadioSpinel::ClearCalibratedPowers(void) { return Set(SPINEL_PROP_PHY_CA otError RadioSpinel::SetChannelTargetPower(uint8_t aChannel, int16_t aTargetPower) { otError error = OT_ERROR_NONE; - VerifyOrExit(aChannel >= Radio::kChannelMin && aChannel <= Radio::kChannelMax, error = OT_ERROR_INVALID_ARGS); + VerifyOrExit(Radio::IsChannelValid(aChannel), error = OT_ERROR_INVALID_ARGS); error = Set(SPINEL_PROP_PHY_CHAN_TARGET_POWER, SPINEL_DATATYPE_UINT8_S SPINEL_DATATYPE_INT16_S, aChannel, aTargetPower);