Address review comments

project-chip · Jul 22, 2023 · f915189 · f915189
1 parent d06f949
commit f915189
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 15 deletions.
diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp
@@ -329,9 +329,9 @@ void InteractionModelEngine::OnDone(ReadHandler & apReadObj)
 #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
     if (!mSubscriptionResumptionScheduled && HasSubscriptionsToResume())
     {
-        mSubscriptionResumptionScheduled  = true;
-        auto timeTillNextResubscriptionMs = ComputeTimeTillNextSubscriptionResumption();
-        mpExchangeMgr->GetSessionManager()->SystemLayer()->StartTimer(System::Clock::Seconds32(timeTillNextResubscriptionMs),
+        mSubscriptionResumptionScheduled    = true;
+        auto timeTillNextResubscriptionSecs = ComputeTimeSecondsTillNextSubscriptionResumption();
+        mpExchangeMgr->GetSessionManager()->SystemLayer()->StartTimer(System::Clock::Seconds32(timeTillNextResubscriptionSecs),
                                                                       ResumeSubscriptionsTimerCallback, this);
         mNumSubscriptionResumptionRetries++;
     }
@@ -1873,22 +1873,18 @@ void InteractionModelEngine::ResumeSubscriptionsTimerCallback(System::Layer * ap
 }
 
 #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
-uint32_t InteractionModelEngine::ComputeTimeTillNextSubscriptionResumption()
+uint32_t InteractionModelEngine::ComputeTimeSecondsTillNextSubscriptionResumption()
 {
-    uint32_t waitTimeInSeconds = 0;
-
     if (mNumSubscriptionResumptionRetries <= CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_FIBONACCI_STEP_INDEX)
     {
-        waitTimeInSeconds = CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MIN_RETRY_INTERVAL_SECS +
+        return CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MIN_RETRY_INTERVAL_SECS +
             GetFibonacciForIndex(mNumSubscriptionResumptionRetries) *
-                CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_WAIT_TIME_MULTIPLIER_SECS;
+            CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_WAIT_TIME_MULTIPLIER_SECS;
     }
     else
     {
-        waitTimeInSeconds = CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_RETRY_INTERVAL_SECS;
+        return CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_RETRY_INTERVAL_SECS;
     }
-
-    return waitTimeInSeconds;
 }
 
 bool InteractionModelEngine::HasSubscriptionsToResume()

diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h
@@ -616,7 +616,7 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
 
 #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
     bool HasSubscriptionsToResume();
-    uint32_t ComputeTimeTillNextSubscriptionResumption();
+    uint32_t ComputeTimeSecondsTillNextSubscriptionResumption();
     uint32_t mNumSubscriptionResumptionRetries = 0;
     bool mSubscriptionResumptionScheduled      = false;
 #endif

diff --git a/src/app/tests/TestInteractionModelEngine.cpp b/src/app/tests/TestInteractionModelEngine.cpp
@@ -236,7 +236,7 @@ void TestInteractionModelEngine::TestSubscriptionResumptionTimer(nlTestSuite * a
 
     uint32_t timeTillNextResubscriptionMs;
     InteractionModelEngine::GetInstance()->mNumSubscriptionResumptionRetries = 0;
-    timeTillNextResubscriptionMs = InteractionModelEngine::GetInstance()->ComputeTimeTillNextSubscriptionResumption();
+    timeTillNextResubscriptionMs = InteractionModelEngine::GetInstance()->ComputeTimeSecondsTillNextSubscriptionResumption();
     NL_TEST_ASSERT(apSuite, timeTillNextResubscriptionMs == CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MIN_RETRY_INTERVAL_SECS);
 
     uint32_t lastTimeTillNextResubscriptionMs = timeTillNextResubscriptionMs;
@@ -245,14 +245,14 @@ void TestInteractionModelEngine::TestSubscriptionResumptionTimer(nlTestSuite * a
          CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_FIBONACCI_STEP_INDEX;
          InteractionModelEngine::GetInstance()->mNumSubscriptionResumptionRetries++)
     {
-        timeTillNextResubscriptionMs = InteractionModelEngine::GetInstance()->ComputeTimeTillNextSubscriptionResumption();
+        timeTillNextResubscriptionMs = InteractionModelEngine::GetInstance()->ComputeTimeSecondsTillNextSubscriptionResumption();
         NL_TEST_ASSERT(apSuite, timeTillNextResubscriptionMs >= lastTimeTillNextResubscriptionMs);
         NL_TEST_ASSERT(apSuite, timeTillNextResubscriptionMs < CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_RETRY_INTERVAL_SECS);
         lastTimeTillNextResubscriptionMs = timeTillNextResubscriptionMs;
     }
 
     InteractionModelEngine::GetInstance()->mNumSubscriptionResumptionRetries = 2000;
-    timeTillNextResubscriptionMs = InteractionModelEngine::GetInstance()->ComputeTimeTillNextSubscriptionResumption();
+    timeTillNextResubscriptionMs = InteractionModelEngine::GetInstance()->ComputeTimeSecondsTillNextSubscriptionResumption();
     NL_TEST_ASSERT(apSuite, timeTillNextResubscriptionMs == CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_RETRY_INTERVAL_SECS);
 }
 #endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION