[mle] smaller enhancements (#8812)

This commit contains smaller enhancements in `MleRouter` class,
defining `kChallengeTimeout` constant, using ternary `? :` replacing
`if/else` checks, combining `#if` blocks, etc.

src/core/thread/mle.cpp

@@ -4940,10 +4940,7 @@ Error Mle::RxMessage::ReadChallengeOrResponse(uint8_t aTlvType, Challenge &aBuff
     SuccessOrExit(error = Tlv::FindTlvValueOffset(*this, aTlvType, offset, length));
     VerifyOrExit(length >= kMinChallengeSize, error = kErrorParse);
 
-    if (length > kMaxChallengeSize)
-    {
-        length = kMaxChallengeSize;
-    }
+    length = Min(length, kMaxChallengeSize);
 
     ReadBytes(offset, aBuffer.mBuffer, length);
     aBuffer.mLength = static_cast<uint8_t>(length);

src/core/thread/mle_router.cpp

@@ -184,20 +184,6 @@ exit:
     return error;
 }
 
-// If the router was a leader or had more than 5 children prior to reset,
-// the multicast link request is retransmitted as a critical message.
-void MleRouter::SetLinkRequestTransmissionCounter(void)
-{
-    uint16_t numOfChildren = mChildTable.GetNumChildren(Child::kInStateValidOrRestoring);
-
-    mLinkRequestAttempts = kMaxTransmissionCount;
-
-    if (mWasLeader || numOfChildren >= kMinCriticalChildrenCount)
-    {
-        mLinkRequestAttempts = kMaxCriticalTransmissionCount;
-    }
-}
-
 Error MleRouter::BecomeRouter(ThreadStatusTlv::Status aStatus)
 {
     Error error = kErrorNone;
@@ -215,7 +201,14 @@ Error MleRouter::BecomeRouter(ThreadStatusTlv::Status aStatus)
     switch (mRole)
     {
     case kRoleDetached:
-        SetLinkRequestTransmissionCounter();
+        // If router had more than `kMinCriticalChildrenCount` children
+        // or was a leader prior to reset we treat the multicast Link
+        // Request as a critical message.
+        mLinkRequestAttempts =
+            (mWasLeader || mChildTable.GetNumChildren(Child::kInStateValidOrRestoring) >= kMinCriticalChildrenCount)
+                ? kMaxCriticalTransmissionCount
+                : kMaxTransmissionCount;
+
         SuccessOrExit(error = SendLinkRequest(nullptr));
         mLinkRequestAttempts--;
         ScheduleMessageTransmissionTimer();
@@ -240,10 +233,6 @@ Error MleRouter::BecomeLeader(void)
     Router  *router;
     uint32_t partitionId;
     uint8_t  leaderId;
-#if OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE
-    uint8_t minRouterId;
-    uint8_t maxRouterId;
-#endif
 
     VerifyOrExit(!Get<MeshCoP::ActiveDatasetManager>().IsPartiallyComplete(), error = kErrorInvalidState);
     VerifyOrExit(!IsDisabled(), error = kErrorInvalidState);
@@ -253,22 +242,18 @@ Error MleRouter::BecomeLeader(void)
     mRouterTable.Clear();
 
 #if OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE
-    partitionId = mPreferredLeaderPartitionId ? mPreferredLeaderPartitionId : Random::NonCrypto::GetUint32();
+    {
+        uint8_t minId;
+        uint8_t maxId;
+
+        mRouterTable.GetRouterIdRange(minId, maxId);
+        partitionId = mPreferredLeaderPartitionId ? mPreferredLeaderPartitionId : Random::NonCrypto::GetUint32();
+        leaderId    = (IsRouterIdValid(mPreviousRouterId) && minId <= mPreviousRouterId && mPreviousRouterId <= maxId)
+                          ? mPreviousRouterId
+                          : Random::NonCrypto::GetUint8InRange(minId, maxId + 1);
+    }
 #else
     partitionId = Random::NonCrypto::GetUint32();
-#endif
-
-#if OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE
-    mRouterTable.GetRouterIdRange(minRouterId, maxRouterId);
-    if (IsRouterIdValid(mPreviousRouterId) && minRouterId <= mPreviousRouterId && mPreviousRouterId <= maxRouterId)
-    {
-        leaderId = mPreviousRouterId;
-    }
-    else
-    {
-        leaderId = Random::NonCrypto::GetUint8InRange(minRouterId, maxRouterId + 1);
-    }
-#else
     leaderId    = IsRouterIdValid(mPreviousRouterId) ? mPreviousRouterId
                                                      : Random::NonCrypto::GetUint8InRange(0, kMaxRouterId + 1);
 #endif
@@ -327,13 +312,11 @@ void MleRouter::HandleChildStart(AttachMode aMode)
     case kDowngradeToReed:
         SendAddressRelease();
 
-        // reset children info if any
         if (HasChildren())
         {
             RemoveChildren();
         }
 
-        // reset routerId info
         SetRouterId(kInvalidRouterId);
         break;
 
@@ -594,7 +577,7 @@ Error MleRouter::SendLinkRequest(Neighbor *aNeighbor)
     if (aNeighbor == nullptr)
     {
         mChallenge.GenerateRandom();
-        mChallengeTimeout = (((2 * kMaxResponseDelay) + kStateUpdatePeriod - 1) / kStateUpdatePeriod);
+        mChallengeTimeout = kChallengeTimeout;
 
         SuccessOrExit(error = message->AppendChallengeTlv(mChallenge));
         destination.SetToLinkLocalAllRoutersMulticast();
@@ -1771,24 +1754,15 @@ void MleRouter::SendParentResponse(Child *aChild, const Challenge &aChallenge, b
 #endif
 
     aChild->GenerateChallenge();
-
     SuccessOrExit(error = message->AppendChallengeTlv(aChild->GetChallenge(), aChild->GetChallengeSize()));
-    error = message->AppendLinkMarginTlv(aChild->GetLinkInfo().GetLinkMargin());
-    SuccessOrExit(error);
-
+    SuccessOrExit(error = message->AppendLinkMarginTlv(aChild->GetLinkInfo().GetLinkMargin()));
     SuccessOrExit(error = message->AppendConnectivityTlv());
     SuccessOrExit(error = message->AppendVersionTlv());
 
     destination.SetToLinkLocalAddress(aChild->GetExtAddress());
 
-    if (aRoutersOnlyRequest)
-    {
-        delay = 1 + Random::NonCrypto::GetUint16InRange(0, kParentResponseMaxDelayRouters);
-    }
-    else
-    {
-        delay = 1 + Random::NonCrypto::GetUint16InRange(0, kParentResponseMaxDelayAll);
-    }
+    delay = 1 + Random::NonCrypto::GetUint16InRange(0, aRoutersOnlyRequest ? kParentResponseMaxDelayRouters
+                                                                           : kParentResponseMaxDelayAll);
 
     SuccessOrExit(error = message->SendAfterDelay(destination, delay));
 

src/core/thread/mle_router.hpp

@@ -542,9 +542,9 @@ public:
 #endif
 
 private:
-    static constexpr uint16_t kDiscoveryMaxJitter            = 250;  // Max jitter delay Discovery Responses (in msec).
-    static constexpr uint32_t kStateUpdatePeriod             = 1000; // State update period (in msec).
-    static constexpr uint16_t kUnsolicitedDataResponseJitter = 500;  // Max delay for unsol Data Response (in msec).
+    static constexpr uint16_t kDiscoveryMaxJitter            = 250; // Max jitter delay Discovery Responses (in msec).
+    static constexpr uint16_t kChallengeTimeout              = 2;   // Challenge timeout (in sec).
+    static constexpr uint16_t kUnsolicitedDataResponseJitter = 500; // Max delay for unsol Data Response (in msec).
 
     // Threshold to accept a router upgrade request with reason
     // `kBorderRouterRequest` (number of BRs acting as router in
@@ -629,8 +629,6 @@ private:
     void        HandleAdvertiseTrickleTimer(void);
     void        HandleTimeTick(void);
 
-    void SetLinkRequestTransmissionCounter(void);
-
     TrickleTimer mAdvertiseTrickleTimer;
 
     ChildTable  mChildTable;

src/core/thread/mle_types.hpp

@@ -127,8 +127,8 @@ constexpr uint16_t kMaxChildId = 511; ///< Maximum Child ID
 constexpr uint8_t kRouterIdOffset   = 10; ///< Bit offset of Router ID in RLOC16
 constexpr uint8_t kRlocPrefixLength = 14; ///< Prefix length of RLOC in bytes
 
-constexpr uint8_t kMinChallengeSize = 4; ///< Minimum Challenge size in bytes.
-constexpr uint8_t kMaxChallengeSize = 8; ///< Maximum Challenge size in bytes.
+constexpr uint16_t kMinChallengeSize = 4; ///< Minimum Challenge size in bytes.
+constexpr uint16_t kMaxChallengeSize = 8; ///< Maximum Challenge size in bytes.
 
 /*
  * Routing Protocol Constants