[data-poll-sender] always honor the default child timeout based poll interval (#4136)

This commit changes the `DataPollSender` to ensure that a child device
will always send a data poll within its timeout interval. This
addresses an issue where a large user-specified poll period could
cause the device to adopt a poll period longer than the default child
timeout (causing parent to kick the child out of its child table).

This commit also simplifies the selection code for a new poll period
by adding a helper `UpdateIfLarger()` method to update the period only
if it's larger than a given new value.

include/openthread/link.h

@@ -431,7 +431,7 @@ uint32_t otLinkGetPollPeriod(otInstance *aInstance);
  *       Or zero to clear user-specified poll period.
  *
  * @note User-specified value should be no more than the maximal value 0x3FFFFFF ((1 << 26) - 1) allowed,
- * otherwise it would be cilpped by the maximal value.
+ * otherwise it would be clipped by the maximal value.
  *
  * @param[in]  aInstance    A pointer to an OpenThread instance.
  * @param[in]  aPollPeriod  data poll period in milliseconds.

src/core/mac/data_poll_sender.cpp

@@ -182,15 +182,11 @@ exit:
 
 uint32_t DataPollSender::GetKeepAlivePollPeriod(void) const
 {
-    uint32_t period = 0;
+    uint32_t period = GetDefaultPollPeriod();
 
     if (mExternalPollPeriod != 0)
     {
-        period = mExternalPollPeriod;
-    }
-    else
-    {
-        period = GetDefaultPollPeriod();
+        UpdateIfLarger(period, mExternalPollPeriod);
     }
 
     return period;
@@ -430,47 +426,41 @@ void DataPollSender::ScheduleNextPoll(PollPeriodSelector aPollPeriodSelector)
     }
 }
 
+void DataPollSender::UpdateIfLarger(uint32_t &aPreiod, uint32_t aNewPeriod)
+{
+    if (aPreiod > aNewPeriod)
+    {
+        aPreiod = aNewPeriod;
+    }
+}
+
 uint32_t DataPollSender::CalculatePollPeriod(void) const
 {
-    uint32_t period = 0;
+    uint32_t period = GetDefaultPollPeriod();
 
     if (mAttachMode)
     {
-        period = kAttachDataPollPeriod;
+        UpdateIfLarger(period, kAttachDataPollPeriod);
     }
 
     if (mRetxMode)
     {
-        if ((period == 0) || (period > kRetxPollPeriod))
-        {
-            period = kRetxPollPeriod;
-        }
+        UpdateIfLarger(period, kRetxPollPeriod);
     }
 
     if (mRemainingFastPolls != 0)
     {
-        if ((period == 0) || (period > kFastPollPeriod))
-        {
-            period = kFastPollPeriod;
-        }
+        UpdateIfLarger(period, kFastPollPeriod);
     }
 
     if (mExternalPollPeriod != 0)
     {
-        if ((period == 0) || (period > mExternalPollPeriod))
-        {
-            period = mExternalPollPeriod;
-        }
+        UpdateIfLarger(period, mExternalPollPeriod);
     }
 
     if (period == 0)
     {
-        period = GetDefaultPollPeriod();
-
-        if (period == 0)
-        {
-            period = kMinPollPeriod;
-        }
+        period = kMinPollPeriod;
     }
 
     return period;

src/core/mac/data_poll_sender.hpp

@@ -105,15 +105,16 @@ public:
     /**
      * This method sets/clears a user-specified/external data poll period.
      *
+     * Value of zero for `aPeriod` clears the user-specified poll period.
+     *
      * If the user provides a non-zero poll period, the user value specifies the maximum period between data
      * request transmissions. Note that OpenThread may send data request transmissions more frequently when expecting
-     * a control-message from a parent or in case of data poll transmission failures or timeouts.
+     * a control-message from a parent or in case of data poll transmission failures or timeouts, or when the specified
+     * value is larger than the child timeout.
      *
-     * Minimal non-zero value should be `OPENTHREAD_CONFIG_MAC_MINIMUM_POLL_PERIOD` (10ms). Or zero to clear
-     * user-specified poll period.
-     *
-     * User-specified value should be no more than the maximal value 0x3FFFFFF ((1 << 26) - 1) allowed, otherwise it
-     * would be cilpped by the maximal value.
+     * A non-zero `aPeriod` should be larger than or equal to `OPENTHREAD_CONFIG_MAC_MINIMUM_POLL_PERIOD` (10ms) or
+     * this method returns `OT_ERROR_INVALID_ARGS`. If a non-zero `aPeriod` is larger than maximum value of
+     * `0x3FFFFFF ((1 << 26) - 1)`, it would be clipped to this value.
      *
      * @param[in]  aPeriod  The data poll period in milliseconds.
      *
@@ -222,11 +223,25 @@ public:
     /**
      * This method gets the maximum data polling period in use.
      *
-     * @returns the maximum data polling period in use.
+     * The maximum data poll period is determined based as the minimum of the user-specified poll interval and the
+     * default poll interval.
+     *
+     * @returns The maximum data polling period in use.
      *
      */
     uint32_t GetKeepAlivePollPeriod(void) const;
 
+    /**
+     * This method returns the default maximum poll period.
+     *
+     * The default poll period is determined based on the child timeout interval, ensuing the child would send data poll
+     * within the child's timeout.
+     *
+     * @returns The maximum default data polling interval (in msec).
+     *
+     */
+    uint32_t GetDefaultPollPeriod(void) const;
+
 private:
     enum // Poll period under different conditions (in milliseconds).
     {
@@ -254,7 +269,7 @@ private:
     void        ScheduleNextPoll(PollPeriodSelector aPollPeriodSelector);
     uint32_t    CalculatePollPeriod(void) const;
     static void HandlePollTimer(Timer &aTimer);
-    uint32_t    GetDefaultPollPeriod(void) const;
+    static void UpdateIfLarger(uint32_t &aPreiod, uint32_t aNewPeriod);
 
     uint32_t mTimerStartTime;
     uint32_t mPollPeriod;