From aab693f9c396088c56a5765ea493f6ad6b5178db Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Thu, 7 Nov 2019 04:45:34 -0800 Subject: [PATCH] [timer] add helper methods for common timer use patterns and simplify code (#4243) This commit adds helper methods in `Timer` class which cover commonly repeated use patterns by other modules. Method `FireAt()` allows a timer to be started with a given fire time. `FireAtIfEarlier()` (re-)schedules the timer with a given a fire time only if the timer is not running or if the new given fire time is earlier than the current fire time. Modules mle, coap, ip6-mpl, joiner-router, dns-clinet, and sntp-client are updated to use the new methods. --- src/core/coap/coap.cpp | 66 +++++++++------------------ src/core/coap/coap.hpp | 35 +-------------- src/core/common/time.hpp | 29 ++++++++++++ src/core/common/timer.cpp | 22 ++++++++- src/core/common/timer.hpp | 25 +++++++++++ src/core/meshcop/joiner_router.cpp | 5 +-- src/core/meshcop/joiner_router.hpp | 20 --------- src/core/net/dns_client.cpp | 71 +++++++++--------------------- src/core/net/dns_client.hpp | 20 --------- src/core/net/ip6_mpl.cpp | 57 +++++++----------------- src/core/net/ip6_mpl.hpp | 20 --------- src/core/net/sntp_client.cpp | 70 +++++++++-------------------- src/core/net/sntp_client.hpp | 20 --------- src/core/thread/mle.cpp | 51 +++++++-------------- src/core/thread/mle.hpp | 22 --------- tests/unit/test_timer.cpp | 10 +++++ 16 files changed, 179 insertions(+), 364 deletions(-) diff --git a/src/core/coap/coap.cpp b/src/core/coap/coap.cpp index 6eb6f8218..1fc35f609 100644 --- a/src/core/coap/coap.cpp +++ b/src/core/coap/coap.cpp @@ -259,42 +259,34 @@ void CoapBase::HandleRetransmissionTimer(Timer &aTimer) void CoapBase::HandleRetransmissionTimer(void) { - TimeMilli now = TimerMilli::GetNow(); - uint32_t nextDelta = TimeMilli::kMaxDuration; + TimeMilli now = TimerMilli::GetNow(); + TimeMilli nextTime = now.GetDistantFuture(); CoapMetadata coapMetadata; - Message * message = static_cast(mPendingRequests.GetHead()); - Message * nextMessage = NULL; + Message * message; + Message * nextMessage; Ip6::MessageInfo messageInfo; - while (message != NULL) + for (message = static_cast(mPendingRequests.GetHead()); message != NULL; message = nextMessage) { nextMessage = static_cast(message->GetNext()); + coapMetadata.ReadFrom(*message); - if (coapMetadata.IsLater(now)) + if (now >= coapMetadata.mNextTimerShot) { - uint32_t diff = coapMetadata.mNextTimerShot - now; - - // Calculate the next delay and choose the lowest. - if (diff < nextDelta) + if (!coapMetadata.mConfirmable || (coapMetadata.mRetransmissionCount >= kMaxRetransmit)) { - nextDelta = diff; + // No expected response or acknowledgment. + FinalizeCoapTransaction(*message, coapMetadata, NULL, NULL, OT_ERROR_RESPONSE_TIMEOUT); + continue; } - } - else if ((coapMetadata.mConfirmable) && (coapMetadata.mRetransmissionCount < kMaxRetransmit)) - { + // Increment retransmission counter and timer. coapMetadata.mRetransmissionCount++; coapMetadata.mRetransmissionTimeout *= 2; coapMetadata.mNextTimerShot = now + coapMetadata.mRetransmissionTimeout; coapMetadata.UpdateIn(*message); - // Check if retransmission time is lower than current lowest. - if (coapMetadata.mRetransmissionTimeout < nextDelta) - { - nextDelta = coapMetadata.mRetransmissionTimeout; - } - // Retransmit if (!coapMetadata.mAcknowledged) { @@ -305,18 +297,16 @@ void CoapBase::HandleRetransmissionTimer(void) SendCopy(*message, messageInfo); } } - else - { - // No expected response or acknowledgment. - FinalizeCoapTransaction(*message, coapMetadata, NULL, NULL, OT_ERROR_RESPONSE_TIMEOUT); - } - message = nextMessage; + if (nextTime > coapMetadata.mNextTimerShot) + { + nextTime = coapMetadata.mNextTimerShot; + } } - if (nextDelta != TimeMilli::kMaxDuration) + if (nextTime < now.GetDistantFuture()) { - mRetransmissionTimer.Start(nextDelta); + mRetransmissionTimer.FireAt(nextTime); } } @@ -369,23 +359,7 @@ Message *CoapBase::CopyAndEnqueueMessage(const Message & aMessage, // Append the copy with retransmission data. SuccessOrExit(error = aCoapMetadata.AppendTo(*messageCopy)); - // Setup the timer. - if (mRetransmissionTimer.IsRunning()) - { - TimeMilli alarmFireTime; - - // If timer is already running, check if it should be restarted with earlier fire time. - alarmFireTime = mRetransmissionTimer.GetFireTime(); - - if (aCoapMetadata.IsEarlier(alarmFireTime)) - { - mRetransmissionTimer.Start(aCoapMetadata.mRetransmissionTimeout); - } - } - else - { - mRetransmissionTimer.Start(aCoapMetadata.mRetransmissionTimeout); - } + mRetransmissionTimer.FireAtIfEarlier(aCoapMetadata.mNextTimerShot); // Enqueue the message. mPendingRequests.Enqueue(*messageCopy); @@ -835,7 +809,7 @@ void ResponsesQueue::HandleTimer(void) { enqueuedResponseHeader.ReadFrom(*message); - if (enqueuedResponseHeader.IsEarlier(TimerMilli::GetNow())) + if (TimerMilli::GetNow() >= enqueuedResponseHeader.mDequeueTime) { DequeueResponse(*message); } diff --git a/src/core/coap/coap.hpp b/src/core/coap/coap.hpp index 7994e46d5..2ea8bd733 100644 --- a/src/core/coap/coap.hpp +++ b/src/core/coap/coap.hpp @@ -158,28 +158,6 @@ public: return aMessage.Write(aMessage.GetLength() - sizeof(*this), sizeof(*this), this); } - /** - * This method checks if the message shall be sent before the given time. - * - * @param[in] aTime A time to compare. - * - * @retval TRUE If the message shall be sent before the given time. - * @retval FALSE Otherwise. - * - */ - bool IsEarlier(TimeMilli aTime) const { return aTime >= mNextTimerShot; } - - /** - * This method checks if the message shall be sent after the given time. - * - * @param[in] aTime A time to compare. - * - * @retval TRUE If the message shall be sent after the given time. - * @retval FALSE Otherwise. - * - */ - bool IsLater(TimeMilli aTime) const { return aTime < mNextTimerShot; } - private: Ip6::Address mSourceAddress; ///< IPv6 address of the message source. Ip6::Address mDestinationAddress; ///< IPv6 address of the message destination. @@ -251,6 +229,8 @@ private: */ class EnqueuedResponseHeader { + friend class ResponsesQueue; + public: /** * Default constructor creating empty object. @@ -297,17 +277,6 @@ public: OT_UNUSED_VARIABLE(length); } - /** - * This method checks if the message shall be sent before the given time. - * - * @param[in] aTime A time to compare. - * - * @retval TRUE If the message shall be sent before the given time. - * @retval FALSE Otherwise. - * - */ - bool IsEarlier(TimeMilli aTime) const { return aTime >= mDequeueTime; } - /** * This method returns number of milliseconds in which the message should be sent. * diff --git a/src/core/common/time.hpp b/src/core/common/time.hpp index 350687fa5..52ee41e03 100644 --- a/src/core/common/time.hpp +++ b/src/core/common/time.hpp @@ -217,6 +217,30 @@ public: */ bool operator>(const Time &aOther) const { return (aOther < *this); } + /** + * This method returns a new `Time` instance which is in distant future relative to current `Time` object. + * + * The returned distance future `Time` is guaranteed to be equal or after (as defined by comparison operator `<=`) + * any other `Time` which is after this `Time` object, i.e., for any `t` for which we have `*this <= t`, it is + * ensured that `t <= this->GetGetDistantFuture()`. + * + * @returns A new `Time` in distance future relative to current `Time` object. + * + */ + Time GetDistantFuture(void) const { return Time(mValue + kDistantFuture); } + + /** + * This method returns a new `Time` instance which is in distant past relative to current `Time` object. + * + * The returned distance past `Time` is guaranteed to be equal or before (as defined by comparison operator `>=`) + * any other `Time` which is before this `Time` object, i.e., for any `t` for which we have `*this >= t`, it is + * ensured that `t >= this->GetDistantPast()`. + * + * @returns A new `Time` in distance past relative to current `Time` object. + * + */ + Time GetDistantPast(void) const { return Time(mValue - kDistantFuture); } + /** * This static method converts a given number of seconds to milliseconds. * @@ -234,6 +258,11 @@ public: static uint32_t MsecToSec(uint32_t aMilliseconds) { return aMilliseconds / 1000u; } private: + enum + { + kDistantFuture = (1UL << 31), + }; + uint32_t mValue; }; diff --git a/src/core/common/timer.cpp b/src/core/common/timer.cpp index 63fe8e309..24925e0ec 100644 --- a/src/core/common/timer.cpp +++ b/src/core/common/timer.cpp @@ -76,10 +76,23 @@ void TimerMilli::Start(uint32_t aDelay) void TimerMilli::StartAt(TimeMilli aStartTime, uint32_t aDelay) { assert(aDelay <= kMaxDelay); - mFireTime = aStartTime + aDelay; + FireAt(aStartTime + aDelay); +} + +void TimerMilli::FireAt(TimeMilli aFireTime) +{ + mFireTime = aFireTime; Get().Add(*this); } +void TimerMilli::FireAtIfEarlier(TimeMilli aFireTime) +{ + if (!IsRunning() || (mFireTime > aFireTime)) + { + FireAt(aFireTime); + } +} + void TimerMilli::Stop(void) { Get().Remove(*this); @@ -220,7 +233,12 @@ void TimerMicro::Start(uint32_t aDelay) void TimerMicro::StartAt(TimeMicro aStartTime, uint32_t aDelay) { assert(aDelay <= kMaxDelay); - mFireTime = aStartTime + aDelay; + FireAt(aStartTime + aDelay); +} + +void TimerMicro::FireAt(TimeMicro aFireTime) +{ + mFireTime = aFireTime; Get().Add(*this); } diff --git a/src/core/common/timer.hpp b/src/core/common/timer.hpp index 9b6293143..c07881356 100644 --- a/src/core/common/timer.hpp +++ b/src/core/common/timer.hpp @@ -175,6 +175,23 @@ public: */ void StartAt(TimeMilli sStartTime, uint32_t aDelay); + /** + * This method schedules the timer to fire at a given fire time. + * + * @param[in] aFireTime The fire time. + * + */ + void FireAt(TimeMilli aFireTime); + + /** + * This method (re-)schedules the timer with a given a fire time only if the timer is not running or the new given + * fire time is earlier than the current fire time. + * + * @param[in] aFireTime The fire time. + * + */ + void FireAtIfEarlier(TimeMilli aFireTime); + /** * This method stops the timer. * @@ -381,6 +398,14 @@ public: */ void StartAt(TimeMicro aStartTime, uint32_t aDelay); + /** + * This method schedules the timer to fire at a given fire time. + * + * @param[in] aFireTime The fire time. + * + */ + void FireAt(TimeMicro aFireTime); + /** * This method stops the timer. * diff --git a/src/core/meshcop/joiner_router.cpp b/src/core/meshcop/joiner_router.cpp index 8454f0eff..b2aa9356e 100644 --- a/src/core/meshcop/joiner_router.cpp +++ b/src/core/meshcop/joiner_router.cpp @@ -371,7 +371,6 @@ void JoinerRouter::SendDelayedJoinerEntrust(void) { DelayedJoinEntHeader delayedJoinEnt; Coap::Message * message = static_cast(mDelayedJoinEnts.GetHead()); - TimeMilli now = TimerMilli::GetNow(); Ip6::MessageInfo messageInfo; VerifyOrExit(message != NULL); @@ -383,9 +382,9 @@ void JoinerRouter::SendDelayedJoinerEntrust(void) VerifyOrExit(!mExpectJoinEntRsp || memcmp(Get().GetKek(), delayedJoinEnt.GetKek(), KeyManager::kMaxKeyLength) == 0); - if (delayedJoinEnt.IsLater(now)) + if (TimerMilli::GetNow() < delayedJoinEnt.GetSendTime()) { - mTimer.Start(delayedJoinEnt.GetSendTime() - now); + mTimer.FireAt(delayedJoinEnt.GetSendTime()); } else { diff --git a/src/core/meshcop/joiner_router.hpp b/src/core/meshcop/joiner_router.hpp index 96446c478..c562c4255 100644 --- a/src/core/meshcop/joiner_router.hpp +++ b/src/core/meshcop/joiner_router.hpp @@ -203,26 +203,6 @@ public: */ const uint8_t *GetKek(void) const { return mKek; } - /** - * This method checks if the message shall be sent before the given time. - * - * @param[in] aTime A time to compare. - * - * @retval TRUE If the message shall be sent before the given time. - * @retval FALSE Otherwise. - */ - bool IsEarlier(TimeMilli aTime) const { return aTime > mSendTime; } - - /** - * This method checks if the message shall be sent after the given time. - * - * @param[in] aTime A time to compare. - * - * @retval TRUE If the message shall be sent after the given time. - * @retval FALSE Otherwise. - */ - bool IsLater(TimeMilli aTime) const { return aTime < mSendTime; } - private: Ip6::MessageInfo mMessageInfo; ///< Message info of the message to send. TimeMilli mSendTime; ///< Time when the message shall be sent. diff --git a/src/core/net/dns_client.cpp b/src/core/net/dns_client.cpp index a9f43ea3a..a263134ac 100644 --- a/src/core/net/dns_client.cpp +++ b/src/core/net/dns_client.cpp @@ -183,10 +183,8 @@ exit: Message *Client::CopyAndEnqueueMessage(const Message &aMessage, const QueryMetadata &aQueryMetadata) { - otError error = OT_ERROR_NONE; - TimeMilli now = TimerMilli::GetNow(); - Message * messageCopy = NULL; - TimeMilli nextTransmissionTime; + otError error = OT_ERROR_NONE; + Message *messageCopy = NULL; // Create a message copy for further retransmissions. VerifyOrExit((messageCopy = aMessage.Clone()) != NULL, error = OT_ERROR_NO_BUFS); @@ -195,21 +193,7 @@ Message *Client::CopyAndEnqueueMessage(const Message &aMessage, const QueryMetad SuccessOrExit(error = aQueryMetadata.AppendTo(*messageCopy)); mPendingQueries.Enqueue(*messageCopy); - // Setup the timer. - if (mRetransmissionTimer.IsRunning()) - { - // If timer is already running, check if it should be restarted with earlier fire time. - nextTransmissionTime = mRetransmissionTimer.GetFireTime(); - - if (aQueryMetadata.IsEarlier(nextTransmissionTime)) - { - mRetransmissionTimer.Start(aQueryMetadata.mTransmissionTime - now); - } - } - else - { - mRetransmissionTimer.Start(aQueryMetadata.mTransmissionTime - now); - } + mRetransmissionTimer.FireAtIfEarlier(aQueryMetadata.mTransmissionTime); exit: @@ -418,45 +402,34 @@ void Client::HandleRetransmissionTimer(Timer &aTimer) void Client::HandleRetransmissionTimer(void) { - TimeMilli now = TimerMilli::GetNow(); - uint32_t nextDelta = TimeMilli::kMaxDuration; + TimeMilli now = TimerMilli::GetNow(); + TimeMilli nextTime = now.GetDistantFuture(); QueryMetadata queryMetadata; - Message * message = mPendingQueries.GetHead(); - Message * nextMessage = NULL; + Message * message; + Message * nextMessage; Ip6::MessageInfo messageInfo; - while (message != NULL) + for (message = mPendingQueries.GetHead(); message != NULL; message = nextMessage) { nextMessage = message->GetNext(); + queryMetadata.ReadFrom(*message); - if (queryMetadata.IsLater(now)) + if (now >= queryMetadata.mTransmissionTime) { - uint32_t diff = queryMetadata.mTransmissionTime - TimerMilli::GetNow(); - - // Calculate the next delay and choose the lowest. - if (diff < nextDelta) + if (queryMetadata.mRetransmissionCount >= kMaxRetransmit) { - nextDelta = diff; + // No expected response. + FinalizeDnsTransaction(*message, queryMetadata, NULL, 0, OT_ERROR_RESPONSE_TIMEOUT); + + continue; } - } - else if (queryMetadata.mRetransmissionCount < kMaxRetransmit) - { - uint32_t diff; // Increment retransmission counter and timer. queryMetadata.mRetransmissionCount++; queryMetadata.mTransmissionTime = now + kResponseTimeout; queryMetadata.UpdateIn(*message); - diff = kResponseTimeout; - - // Check if retransmission time is lower than current lowest. - if (diff < nextDelta) - { - nextDelta = kResponseTimeout; - } - // Retransmit messageInfo.SetPeerAddr(queryMetadata.mDestinationAddress); messageInfo.SetPeerPort(queryMetadata.mDestinationPort); @@ -464,18 +437,16 @@ void Client::HandleRetransmissionTimer(void) SendCopy(*message, messageInfo); } - else - { - // No expected response. - FinalizeDnsTransaction(*message, queryMetadata, NULL, 0, OT_ERROR_RESPONSE_TIMEOUT); - } - message = nextMessage; + if (nextTime > queryMetadata.mTransmissionTime) + { + nextTime = queryMetadata.mTransmissionTime; + } } - if (nextDelta != TimeMilli::kMaxDuration) + if (nextTime < now.GetDistantFuture()) { - mRetransmissionTimer.Start(nextDelta); + mRetransmissionTimer.FireAt(nextTime); } } diff --git a/src/core/net/dns_client.hpp b/src/core/net/dns_client.hpp index 5e74c0e36..fe18be348 100644 --- a/src/core/net/dns_client.hpp +++ b/src/core/net/dns_client.hpp @@ -108,26 +108,6 @@ public: return aMessage.Write(aMessage.GetLength() - sizeof(*this), sizeof(*this), this); } - /** - * This method checks if the message shall be sent before the given time. - * - * @param[in] aTime A time to compare. - * - * @retval TRUE If the message shall be sent before the given time. - * @retval FALSE Otherwise. - */ - bool IsEarlier(TimeMilli aTime) const { return aTime > mTransmissionTime; } - - /** - * This method checks if the message shall be sent after the given time. - * - * @param[in] aTime A time to compare. - * - * @retval TRUE If the message shall be sent after the given time. - * @retval FALSE Otherwise. - */ - bool IsLater(TimeMilli aTime) const { return aTime < mTransmissionTime; } - private: const char * mHostname; ///< A hostname to be find. otDnsResponseHandler mResponseHandler; ///< A function pointer that is called on response reception. diff --git a/src/core/net/ip6_mpl.cpp b/src/core/net/ip6_mpl.cpp index c1875e5e7..c1073db56 100644 --- a/src/core/net/ip6_mpl.cpp +++ b/src/core/net/ip6_mpl.cpp @@ -235,11 +235,9 @@ exit: void Mpl::AddBufferedMessage(Message &aMessage, uint16_t aSeedId, uint8_t aSequence, bool aIsOutbound) { - TimeMilli now = TimerMilli::GetNow(); otError error = OT_ERROR_NONE; Message * messageCopy = NULL; MplBufferedMessageMetadata messageMetadata; - TimeMilli nextTransmissionTime; uint8_t hopLimit = 0; #if OPENTHREAD_CONFIG_MPL_DYNAMIC_INTERVAL_ENABLE @@ -262,27 +260,12 @@ void Mpl::AddBufferedMessage(Message &aMessage, uint16_t aSeedId, uint8_t aSeque messageMetadata.SetSeedId(aSeedId); messageMetadata.SetSequence(aSequence); messageMetadata.SetTransmissionCount(aIsOutbound ? 1 : 0); - messageMetadata.GenerateNextTransmissionTime(now, interval); + messageMetadata.GenerateNextTransmissionTime(TimerMilli::GetNow(), interval); - // Append the message with MplBufferedMessageMetadata and add it to the queue. SuccessOrExit(error = messageMetadata.AppendTo(*messageCopy)); mBufferedMessageSet.Enqueue(*messageCopy); - if (mRetransmissionTimer.IsRunning()) - { - // If timer is already running, check if it should be restarted with earlier fire time. - nextTransmissionTime = mRetransmissionTimer.GetFireTime(); - - if (messageMetadata.IsEarlier(nextTransmissionTime)) - { - mRetransmissionTimer.Start(messageMetadata.GetTransmissionTime() - now); - } - } - else - { - // Otherwise just set the timer. - mRetransmissionTimer.Start(messageMetadata.GetTransmissionTime() - now); - } + mRetransmissionTimer.FireAtIfEarlier(messageMetadata.GetTransmissionTime()); exit: @@ -333,26 +316,23 @@ void Mpl::HandleRetransmissionTimer(Timer &aTimer) void Mpl::HandleRetransmissionTimer(void) { - TimeMilli now = TimerMilli::GetNow(); - uint32_t nextDelta = TimeMilli::kMaxDuration; + TimeMilli now = TimerMilli::GetNow(); + TimeMilli nextTime = now.GetDistantFuture(); MplBufferedMessageMetadata messageMetadata; + Message * message; + Message * nextMessage; - Message *message = mBufferedMessageSet.GetHead(); - Message *nextMessage = NULL; - - while (message != NULL) + for (message = mBufferedMessageSet.GetHead(); message != NULL; message = nextMessage) { nextMessage = message->GetNext(); + messageMetadata.ReadFrom(*message); - if (messageMetadata.IsLater(now)) + if (now < messageMetadata.GetTransmissionTime()) { - uint32_t diff = messageMetadata.GetTransmissionTime() - now; - - // Calculate the next retransmission time and choose the lowest. - if (diff < nextDelta) + if (nextTime > messageMetadata.GetTransmissionTime()) { - nextDelta = diff; + nextTime = messageMetadata.GetTransmissionTime(); } } else @@ -362,8 +342,6 @@ void Mpl::HandleRetransmissionTimer(void) if (messageMetadata.GetTransmissionCount() < GetTimerExpirations()) { - uint32_t diff; - Message *messageCopy = message->Clone(message->GetLength() - sizeof(MplBufferedMessageMetadata)); if (messageCopy != NULL) @@ -379,12 +357,9 @@ void Mpl::HandleRetransmissionTimer(void) messageMetadata.GenerateNextTransmissionTime(now, kDataMessageInterval); messageMetadata.UpdateIn(*message); - diff = messageMetadata.GetTransmissionTime() - now; - - // Check if retransmission time is lower than the current lowest one. - if (diff < nextDelta) + if (nextTime > messageMetadata.GetTransmissionTime()) { - nextDelta = diff; + nextTime = messageMetadata.GetTransmissionTime(); } } else @@ -409,13 +384,11 @@ void Mpl::HandleRetransmissionTimer(void) } } } - - message = nextMessage; } - if (nextDelta != TimeMilli::kMaxDuration) + if (nextTime < now.GetDistantFuture()) { - mRetransmissionTimer.Start(nextDelta); + mRetransmissionTimer.FireAt(nextTime); } } diff --git a/src/core/net/ip6_mpl.hpp b/src/core/net/ip6_mpl.hpp index 0af1d2943..adb4b25bf 100644 --- a/src/core/net/ip6_mpl.hpp +++ b/src/core/net/ip6_mpl.hpp @@ -311,26 +311,6 @@ public: return aMessage.Write(aMessage.GetLength() - sizeof(*this), sizeof(*this), this); } - /** - * This method checks if the message shall be sent before the given time. - * - * @param[in] aTime A time to compare. - * - * @retval TRUE If the message shall be sent before the given time. - * @retval FALSE Otherwise. - */ - bool IsEarlier(TimeMilli aTime) const { return aTime > mTransmissionTime; } - - /** - * This method checks if the message shall be sent after the given time. - * - * @param[in] aTime A time to compare. - * - * @retval TRUE If the message shall be sent after the given time. - * @retval FALSE Otherwise. - */ - bool IsLater(TimeMilli aTime) const { return aTime < mTransmissionTime; } - /** * This method returns the MPL Seed Id value. * diff --git a/src/core/net/sntp_client.cpp b/src/core/net/sntp_client.cpp index c86b17157..d42bc96f5 100644 --- a/src/core/net/sntp_client.cpp +++ b/src/core/net/sntp_client.cpp @@ -189,10 +189,8 @@ exit: Message *Client::CopyAndEnqueueMessage(const Message &aMessage, const QueryMetadata &aQueryMetadata) { - otError error = OT_ERROR_NONE; - TimeMilli now = TimerMilli::GetNow(); - Message * messageCopy = NULL; - TimeMilli nextTransmissionTime; + otError error = OT_ERROR_NONE; + Message *messageCopy = NULL; // Create a message copy for further retransmissions. VerifyOrExit((messageCopy = aMessage.Clone()) != NULL, error = OT_ERROR_NO_BUFS); @@ -201,21 +199,7 @@ Message *Client::CopyAndEnqueueMessage(const Message &aMessage, const QueryMetad SuccessOrExit(error = aQueryMetadata.AppendTo(*messageCopy)); mPendingQueries.Enqueue(*messageCopy); - // Setup the timer. - if (mRetransmissionTimer.IsRunning()) - { - // If timer is already running, check if it should be restarted with earlier fire time. - nextTransmissionTime = mRetransmissionTimer.GetFireTime(); - - if (aQueryMetadata.IsEarlier(nextTransmissionTime)) - { - mRetransmissionTimer.Start(aQueryMetadata.mTransmissionTime - now); - } - } - else - { - mRetransmissionTimer.Start(aQueryMetadata.mTransmissionTime - now); - } + mRetransmissionTimer.FireAtIfEarlier(aQueryMetadata.mTransmissionTime); exit: @@ -311,45 +295,33 @@ void Client::HandleRetransmissionTimer(Timer &aTimer) void Client::HandleRetransmissionTimer(void) { - TimeMilli now = TimerMilli::GetNow(); - uint32_t nextDelta = TimeMilli::kMaxDuration; + TimeMilli now = TimerMilli::GetNow(); + TimeMilli nextTime = now.GetDistantFuture(); QueryMetadata queryMetadata; - Message * message = mPendingQueries.GetHead(); - Message * nextMessage = NULL; + Message * message; + Message * nextMessage; Ip6::MessageInfo messageInfo; - while (message != NULL) + for (message = mPendingQueries.GetHead(); message != NULL; message = nextMessage) { nextMessage = message->GetNext(); + queryMetadata.ReadFrom(*message); - if (queryMetadata.IsLater(now)) + if (now >= queryMetadata.mTransmissionTime) { - uint32_t diff = queryMetadata.mTransmissionTime - now; - - // Calculate the next delay and choose the lowest. - if (diff < nextDelta) + if (queryMetadata.mRetransmissionCount >= kMaxRetransmit) { - nextDelta = diff; + // No expected response. + FinalizeSntpTransaction(*message, queryMetadata, 0, OT_ERROR_RESPONSE_TIMEOUT); + continue; } - } - else if (queryMetadata.mRetransmissionCount < kMaxRetransmit) - { - uint32_t diff; // Increment retransmission counter and timer. queryMetadata.mRetransmissionCount++; queryMetadata.mTransmissionTime = now + kResponseTimeout; queryMetadata.UpdateIn(*message); - diff = kResponseTimeout; - - // Check if retransmission time is lower than current lowest. - if (diff < nextDelta) - { - nextDelta = diff; - } - // Retransmit messageInfo.SetPeerAddr(queryMetadata.mDestinationAddress); messageInfo.SetPeerPort(queryMetadata.mDestinationPort); @@ -357,18 +329,16 @@ void Client::HandleRetransmissionTimer(void) SendCopy(*message, messageInfo); } - else - { - // No expected response. - FinalizeSntpTransaction(*message, queryMetadata, 0, OT_ERROR_RESPONSE_TIMEOUT); - } - message = nextMessage; + if (nextTime > queryMetadata.mTransmissionTime) + { + nextTime = queryMetadata.mTransmissionTime; + } } - if (nextDelta != TimeMilli::kMaxDuration) + if (nextTime < now.GetDistantFuture()) { - mRetransmissionTimer.Start(nextDelta); + mRetransmissionTimer.FireAt(nextTime); } } diff --git a/src/core/net/sntp_client.hpp b/src/core/net/sntp_client.hpp index 9f56f3d68..b5ed68544 100644 --- a/src/core/net/sntp_client.hpp +++ b/src/core/net/sntp_client.hpp @@ -472,26 +472,6 @@ public: return aMessage.Write(aMessage.GetLength() - sizeof(*this), sizeof(*this), this); } - /** - * This method checks if the message shall be sent before the given time. - * - * @param[in] aTime A time to compare. - * - * @retval TRUE If the message shall be sent before the given time. - * @retval FALSE Otherwise. - */ - bool IsEarlier(TimeMilli aTime) const { return aTime > mTransmissionTime; } - - /** - * This method checks if the message shall be sent after the given time. - * - * @param[in] aTime A time to compare. - * - * @retval TRUE If the message shall be sent after the given time. - * @retval FALSE Otherwise. - */ - bool IsLater(TimeMilli aTime) const { return aTime < mTransmissionTime; } - private: uint32_t mTransmitTimestamp; ///< Time at the client when the request departed for the server. otSntpResponseHandler mResponseHandler; ///< A function pointer that is called on response reception. diff --git a/src/core/thread/mle.cpp b/src/core/thread/mle.cpp index f05dfeaf3..cab5c6211 100644 --- a/src/core/thread/mle.cpp +++ b/src/core/thread/mle.cpp @@ -1884,24 +1884,22 @@ void Mle::HandleDelayedResponseTimer(Timer &aTimer) void Mle::HandleDelayedResponseTimer(void) { DelayedResponseHeader delayedResponse; - TimeMilli now = TimerMilli::GetNow(); - uint32_t nextDelay = TimeMilli::kMaxDuration; - Message * message = mDelayedResponses.GetHead(); - Message * nextMessage = NULL; + TimeMilli now = TimerMilli::GetNow(); + TimeMilli nextSendTime = now.GetDistantFuture(); + Message * message; + Message * nextMessage; - while (message != NULL) + for (message = mDelayedResponses.GetHead(); message != NULL; message = nextMessage) { nextMessage = message->GetNext(); + delayedResponse.ReadFrom(*message); - if (delayedResponse.IsLater(now)) + if (now < delayedResponse.GetSendTime()) { - uint32_t diff = delayedResponse.GetSendTime() - now; - - // Calculate the next delay and choose the lowest. - if (diff < nextDelay) + if (nextSendTime > delayedResponse.GetSendTime()) { - nextDelay = diff; + nextSendTime = delayedResponse.GetSendTime(); } } else @@ -1917,7 +1915,7 @@ void Mle::HandleDelayedResponseTimer(void) LogMleMessage("Send delayed message", delayedResponse.GetDestination()); // Here enters fast poll mode, as for Rx-Off-when-idle device, the enqueued msg should - // be Mle Data Reqeuest. + // be Mle Data Request. // Note: Finer-grade check (e.g. message subtype) might be required when deciding whether // or not enters fast poll mode fast poll mode if there are other type of delayed message // for Rx-Off-when-idle device. @@ -1931,13 +1929,11 @@ void Mle::HandleDelayedResponseTimer(void) message->Free(); } } - - message = nextMessage; } - if (nextDelay != TimeMilli::kMaxDuration) + if (nextSendTime < now.GetDistantFuture()) { - mDelayedResponseTimer.Start(nextDelay); + mDelayedResponseTimer.FireAt(nextSendTime); } } @@ -2561,30 +2557,13 @@ exit: otError Mle::AddDelayedResponse(Message &aMessage, const Ip6::Address &aDestination, uint16_t aDelay) { - otError error = OT_ERROR_NONE; - TimeMilli alarmFireTime; - TimeMilli sendTime = TimerMilli::GetNow() + aDelay; + otError error = OT_ERROR_NONE; + DelayedResponseHeader delayedResponse(TimerMilli::GetNow() + aDelay, aDestination); - // Append the message with DelayedRespnoseHeader and add to the list. - DelayedResponseHeader delayedResponse(sendTime, aDestination); SuccessOrExit(error = delayedResponse.AppendTo(aMessage)); mDelayedResponses.Enqueue(aMessage); - if (mDelayedResponseTimer.IsRunning()) - { - // If timer is already running, check if it should be restarted with earlier fire time. - alarmFireTime = mDelayedResponseTimer.GetFireTime(); - - if (delayedResponse.IsEarlier(alarmFireTime)) - { - mDelayedResponseTimer.Start(aDelay); - } - } - else - { - // Otherwise just set the timer. - mDelayedResponseTimer.Start(aDelay); - } + mDelayedResponseTimer.FireAtIfEarlier(delayedResponse.GetSendTime()); exit: return error; diff --git a/src/core/thread/mle.hpp b/src/core/thread/mle.hpp index 02cb1d6db..f818e6aab 100644 --- a/src/core/thread/mle.hpp +++ b/src/core/thread/mle.hpp @@ -440,28 +440,6 @@ public: */ const Ip6::Address &GetDestination(void) const { return mDestination; } - /** - * This method checks if the message shall be sent before the given time. - * - * @param[in] aTime A time to compare. - * - * @retval TRUE If the message shall be sent before the given time. - * @retval FALSE Otherwise. - * - */ - bool IsEarlier(TimeMilli aTime) { return aTime > mSendTime; } - - /** - * This method checks if the message shall be sent after the given time. - * - * @param[in] aTime A time to compare. - * - * @retval TRUE If the message shall be sent after the given time. - * @retval FALSE Otherwise. - * - */ - bool IsLater(TimeMilli aTime) { return aTime < mSendTime; } - private: Ip6::Address mDestination; ///< IPv6 address of the message destination. TimeMilli mSendTime; ///< Time when the message shall be sent. diff --git a/tests/unit/test_timer.cpp b/tests/unit/test_timer.cpp index 8e4683beb..af42143fa 100644 --- a/tests/unit/test_timer.cpp +++ b/tests/unit/test_timer.cpp @@ -633,6 +633,16 @@ int TestTimerTime(void) VerifyOrQuit((t1 >= t2), "Time >= failed.\n"); VerifyOrQuit(t1 - t2 == duration, "Time difference failed\n"); + t2 = t1.GetDistantFuture(); + VerifyOrQuit((t1 < t2), "GetDistanceFuture() failed\n"); + t2 += 1; + VerifyOrQuit(!(t1 < t2), "GetDistanceFuture() failed\n"); + + t2 = t1.GetDistantPast(); + VerifyOrQuit((t1 > t2), "GetDistantPast() failed\n"); + t2 -= 1; + VerifyOrQuit(!(t1 > t2), "GetDistantPast() failed\n"); + printf("--> PASSED\n"); } }