[low-power] fix csl channel (#5501)

If CSL Channel is never specified, the actual CSL channel should
always be the phy channel that the device is using. If CSL Channel
hasn't been specified and radio channel changes, the actual CSL
channel should also change. In current implementation, CSL Channel is
by default a valid value (not specified by upper layer) and wouldn't
change when radio channel changes even if it's never specified.

This commit fixes this issue. On CSL receiver, a bit is extracted from
mCslChannel to be used as a flag mIsCslChannelSpecified. If it is
false, mCslChannel would keep synchronized to the value of
mRadioChannel in Mac and Csl Channel TLV wouldn't be appended in Child
Update Request to its parent. On CSL transmitter, CSL channel info in
child is allowed to be 0. If it's 0, the channel info in frame would
be set using Mac::GetPanChannel.

include/openthread/link.h

@@ -1019,7 +1019,8 @@ uint8_t otLinkCslGetChannel(otInstance *aInstance);
  * This function sets the CSL channel.
  *
  * @param[in]  aInstance      A pointer to an OpenThread instance.
- * @param[in]  aChannel       The CSL sample channel.
+ * @param[in]  aChannel       The CSL sample channel. Channel value should be `0` (Set CSL Channel unspecified) or
+ *                            within the range [1, 10] (if 915-MHz supported) and [11, 26] (if 2.4 GHz supported).
  *
  * @retval OT_ERROR_NONE           Successfully set the CSL parameters.
  * @retval OT_ERROR_INVALID_ARGS   Invalid @p aChannel.

src/core/api/link_api.cpp

@@ -493,7 +493,7 @@ otError otLinkCslSetChannel(otInstance *aInstance, uint8_t aChannel)
     otError   error    = OT_ERROR_NONE;
     Instance &instance = *static_cast<Instance *>(aInstance);
 
-    VerifyOrExit((Radio::kChannelMin <= aChannel) && (aChannel <= Radio::kChannelMax), error = OT_ERROR_INVALID_ARGS);
+    VerifyOrExit(Radio::IsCslChannelValid(aChannel), error = OT_ERROR_INVALID_ARGS);
 
     instance.Get<Mac::Mac>().SetCslChannel(aChannel);
 

src/core/mac/mac.cpp

@@ -658,7 +658,7 @@ void Mac::UpdateIdleMode(void)
 #if OPENTHREAD_CONFIG_MAC_CSL_RECEIVER_ENABLE
         if (IsCslEnabled())
         {
-            IgnoreError(mSubMac.CslSample());
+            IgnoreError(mSubMac.CslSample(mRadioChannel));
             ExitNow();
         }
 #endif
@@ -2269,6 +2269,7 @@ void Mac::SetCslChannel(uint8_t aChannel)
     VerifyOrExit(GetCslChannel() != aChannel, OT_NOOP);
 
     mSubMac.SetCslChannel(aChannel);
+    mSubMac.SetCslChannelSpecified(aChannel != 0 ? true : false);
 
     if (IsCslEnabled())
     {

src/core/mac/mac.hpp

@@ -693,6 +693,14 @@ public:
      */
     void SetCslChannel(uint8_t aChannel);
 
+    /**
+     * This method indicates if CSL channel has been explicitly specified by the upper layer.
+     *
+     * @returns If CSL channel has been specified.
+     *
+     */
+    bool IsCslChannelSpecified(void) const { return mSubMac.IsCslChannelSpecified(); }
+
     /**
      * This method gets the CSL period.
      *

src/core/mac/sub_mac.cpp

@@ -68,7 +68,8 @@ SubMac::SubMac(Instance &aInstance)
 #if OPENTHREAD_CONFIG_MAC_CSL_RECEIVER_ENABLE
     , mCslTimeout(OPENTHREAD_CONFIG_CSL_TIMEOUT)
     , mCslPeriod(0)
-    , mCslChannel(OPENTHREAD_CONFIG_DEFAULT_CHANNEL)
+    , mCslChannel(0)
+    , mIsCslChannelSpecified(false)
     , mCslState(kCslIdle)
     , mCslTimer(aInstance, SubMac::HandleCslTimer, this)
 #endif
@@ -211,10 +212,15 @@ exit:
 }
 
 #if OPENTHREAD_CONFIG_MAC_CSL_RECEIVER_ENABLE
-otError SubMac::CslSample(void)
+otError SubMac::CslSample(uint8_t aPanChannel)
 {
     otError error = OT_ERROR_NONE;
 
+    if (!IsCslChannelSpecified())
+    {
+        mCslChannel = aPanChannel;
+    }
+
     switch (mCslState)
     {
     case kCslSample:

src/core/mac/sub_mac.hpp

@@ -305,12 +305,15 @@ public:
      * started, `mState` will become `kStateCslSample`. But it could be doing `Sleep` or `Receive` at this moment
      * (depending on `mCslState`).
      *
+     * @param[in]  aPanChannel  The current phy channel used by the device. This param will only take effect when CSL
+     *                          channel hasn't been explicitly specified.
+     *
      * @retval OT_ERROR_NONE          Successfully entered CSL operation (sleep or receive according to CSL timer).
      * @retval OT_ERROR_BUSY          The radio was transmitting.
      * @retval OT_ERROR_INVALID_STATE The radio was disabled.
      *
      */
-    otError CslSample(void);
+    otError CslSample(uint8_t aPanChannel);
 #endif
 
     /**
@@ -384,11 +387,25 @@ public:
     /**
      * This method sets the CSL channel.
      *
-     * @param[in]  aChannel  The CSL channel.
+     * @param[in]  aChannel  The CSL channel. `0` to set CSL Channel unspecified.
      *
      */
     void SetCslChannel(uint8_t aChannel);
 
+    /**
+     * This method indicates if CSL channel has been explicitly specified by the upper layer.
+     *
+     * @returns If CSL channel has been specified.
+     *
+     */
+    bool IsCslChannelSpecified(void) const { return mIsCslChannelSpecified; }
+
+    /**
+     * This method sets the flag representing if CSL channel has been specified.
+     *
+     */
+    void SetCslChannelSpecified(bool aIsSpecified) { mIsCslChannelSpecified = aIsSpecified; }
+
     /**
      * This method gets the CSL period.
      *
@@ -604,10 +621,13 @@ private:
 #endif
 
 #if OPENTHREAD_CONFIG_MAC_CSL_RECEIVER_ENABLE
-    uint32_t  mCslTimeout;    ///< The CSL synchronized timeout in seconds.
-    TimeMicro mCslSampleTime; ///< The CSL sample time of the current period.
-    uint16_t  mCslPeriod;     ///< The CSL sample period, in units of 10 symbols (160 microseconds).
-    uint8_t   mCslChannel;    ///< The CSL sample channel.
+    uint32_t  mCslTimeout;     ///< The CSL synchronized timeout in seconds.
+    TimeMicro mCslSampleTime;  ///< The CSL sample time of the current period.
+    uint16_t  mCslPeriod;      ///< The CSL sample period, in units of 10 symbols (160 microseconds).
+    uint8_t   mCslChannel : 7; ///< The actually CSL sample channel. If `mIsCslChannelSpecified` is 0, this should be
+                               ///< equal to the Pan channel of `Mac`.
+    uint8_t mIsCslChannelSpecified : 1; ///< Indicates whether or not the CSL channel was explicitly specified by
+                                        ///< the user.
 
     CslState mCslState;
 

src/core/radio/radio.hpp

@@ -594,6 +594,18 @@ public:
      */
     uint32_t GetPreferredChannelMask(void) { return otPlatRadioGetPreferredChannelMask(GetInstance()); }
 
+    /**
+     * This method checks if a given channel is valid as a CSL channel.
+     *
+     * @retval true   The channel is valid.
+     * @retval false  The channel is invalid.
+     *
+     */
+    static bool IsCslChannelValid(uint8_t aCslChannel)
+    {
+        return (aCslChannel == 0) || ((kChannelMin <= aCslChannel) && (aCslChannel <= kChannelMax));
+    }
+
 private:
     otInstance *GetInstance(void) { return reinterpret_cast<otInstance *>(&InstanceLocator::GetInstance()); }
 

src/core/thread/csl_tx_scheduler.cpp

@@ -200,7 +200,8 @@ otError CslTxScheduler::HandleFrameRequest(Mac::TxFrame &aFrame)
         aFrame.SetIsARetransmission(false);
     }
 
-    aFrame.SetChannel(mCslTxChild->GetCslChannel());
+    aFrame.SetChannel(mCslTxChild->GetCslChannel() == 0 ? Get<Mac::Mac>().GetPanChannel()
+                                                        : mCslTxChild->GetCslChannel());
     aFrame.SetTxPhase(mCslTxChild->GetCslPhase());
     aFrame.SetTxPeriod(mCslTxChild->GetCslPeriod());
 

src/core/thread/mle.cpp

@@ -1396,7 +1396,11 @@ otError Mle::AppendCslChannel(Message &aMessage)
     otError       error = OT_ERROR_NONE;
     CslChannelTlv cslChannel;
 
-    VerifyOrExit(Get<Mac::Mac>().GetPanChannel() != Get<Mac::Mac>().GetCslChannel(), OT_NOOP);
+    // In current implementation, it's allowed to set CSL Channel unspecified. As `0` is not valid for Channel value
+    // in CSL Channel TLV, if CSL channel is not specified, we don't append CSL Channel TLV.
+    // And on transmitter side, it would also set CSL Channel for the child to `0` if it doesn't find a CSL Channel
+    // TLV.
+    VerifyOrExit(Get<Mac::Mac>().IsCslChannelSpecified(), OT_NOOP);
 
     cslChannel.Init();
     cslChannel.SetChannelPage(0);

src/core/thread/mle_router.cpp

@@ -2509,9 +2509,10 @@ void MleRouter::HandleChildUpdateRequest(const Message &         aMessage,
         {
             child->SetCslChannel(static_cast<uint8_t>(cslChannel.GetChannel()));
         }
-        else if (child->GetCslChannel() == 0)
+        else
         {
-            child->SetCslChannel(Get<Mac::Mac>().GetPanChannel());
+            // Set CSL Channel unspecified.
+            child->SetCslChannel(0);
         }
     }
 #endif // OPENTHREAD_CONFIG_MAC_CSL_TRANSMITTER_ENABLE

tests/scripts/thread-cert/config.py

@@ -229,6 +229,7 @@ def create_default_mle_tlvs_factories():
         mle.TlvType.PANID: mle.PanIdFactory(),
         mle.TlvType.ACTIVE_TIMESTAMP: mle.ActiveTimestampFactory(),
         mle.TlvType.PENDING_TIMESTAMP: mle.PendingTimestampFactory(),
+        mle.TlvType.CSL_CHANNEL: mle.CslChannelFactory(),
         mle.TlvType.CSL_SYNCHRONIZED_TIMEOUT: mle.CslSynchronizedTimeoutFactory(),
         mle.TlvType.ACTIVE_OPERATIONAL_DATASET: mle.ActiveOperationalDatasetFactory(),
         mle.TlvType.PENDING_OPERATIONAL_DATASET: mle.PendingOperationalDatasetFactory(),

tests/scripts/thread-cert/mle.py

@@ -87,6 +87,7 @@ class TlvType(IntEnum):
     ACTIVE_OPERATIONAL_DATASET = 24
     PENDING_OPERATIONAL_DATASET = 25
     THREAD_DISCOVERY = 26
+    CSL_CHANNEL = 80
     CSL_SYNCHRONIZED_TIMEOUT = 85
     TIME_REQUEST = 252
     TIME_PARAMETER = 253
@@ -1048,6 +1049,20 @@ class ThreadDiscoveryFactory:
         return ThreadDiscovery(tlvs)
 
 
+class CslChannel:
+    # TODO: Not implemented yet
+
+    def __init__(self):
+        print("CslChannel is not implemented yet.")
+
+
+class CslChannelFactory:
+    # TODO: Not implemented yet
+
+    def parse(self, data, message_info):
+        return CslChannel()
+
+
 class CslSynchronizedTimeout:
     # TODO: Not implemented yet
 

tests/scripts/thread-cert/v1_2_test_csl_transmission.py

@@ -29,6 +29,7 @@
 
 import unittest
 
+import mle
 import thread_cert
 
 LEADER = 1
@@ -55,7 +56,6 @@ class SSED_CslTransmission(thread_cert.TestCase):
 
         self.nodes[SSED_1].set_csl_period(CSL_PERIOD)
         self.nodes[SSED_1].set_csl_timeout(CSL_TIMEOUT)
-        self.nodes[SSED_1].set_csl_channel(CSL_CHANNEL)
 
         self.nodes[SSED_1].get_csl_info()
 
@@ -71,6 +71,27 @@ class SSED_CslTransmission(thread_cert.TestCase):
         self.assertTrue(self.nodes[LEADER].ping(self.nodes[SSED_1].get_rloc()))
         self.simulator.go(5)
 
+        ssed_messages = self.simulator.get_messages_sent_by(SSED_1)
+        msg = ssed_messages.next_mle_message(mle.CommandType.CHILD_UPDATE_REQUEST)
+        msg.assertMleMessageDoesNotContainTlv(mle.CslChannel)
+
+        self.nodes[SSED_1].set_csl_channel(CSL_CHANNEL)
+        self.simulator.go(1)
+        ssed_messages = self.simulator.get_messages_sent_by(SSED_1)
+        msg = ssed_messages.next_mle_message(mle.CommandType.CHILD_UPDATE_REQUEST)
+        msg.assertMleMessageContainsTlv(mle.CslChannel)
+        self.assertTrue(self.nodes[LEADER].ping(self.nodes[SSED_1].get_rloc()))
+        self.simulator.go(5)
+
+        self.nodes[SSED_1].set_csl_channel(0)
+        self.simulator.go(1)
+        self.assertTrue(self.nodes[LEADER].ping(self.nodes[SSED_1].get_rloc()))
+        self.simulator.go(5)
+
+        ssed_messages = self.simulator.get_messages_sent_by(SSED_1)
+        msg = ssed_messages.next_mle_message(mle.CommandType.CHILD_UPDATE_REQUEST)
+        msg.assertMleMessageDoesNotContainTlv(mle.CslChannel)
+
         self.nodes[SSED_1].set_csl_period(0)
         self.assertFalse(self.nodes[LEADER].ping(self.nodes[SSED_1].get_rloc()))
         self.simulator.go(2)