Clean up comments & code

Author: mkardous-silabs

src/app/InteractionModelEngine.cpp

@@ -2054,12 +2054,12 @@ bool InteractionModelEngine::HasSubscriptionsToResume()
 #endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
 
 #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
-void InteractionModelEngine::DecrementNumSubscriptionToResume()
+void InteractionModelEngine::DecrementNumSubscriptionsToResume()
 {
     VerifyOrReturn(mNumOfSubscriptionToResume > 0);
 #if CHIP_CONFIG_ENABLE_ICD_CIP && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
     VerifyOrDie(mICDManager);
-#endif // CHIP_CONFIG_ENABLE_ICD_CIP && CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
+#endif // CHIP_CONFIG_ENABLE_ICD_CIP && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
 
     mNumOfSubscriptionToResume--;
 

src/app/InteractionModelEngine.h

@@ -328,7 +328,7 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
     bool SubjectHasPersistedSubscription(FabricIndex aFabricIndex, NodeId subjectID) override;
 
 #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
-    void DecrementNumSubscriptionToResume();
+    void DecrementNumSubscriptionsToResume();
 #endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
 
 #if CONFIG_BUILD_FOR_HOST_UNIT_TEST

src/app/SubscriptionResumptionSessionEstablisher.cpp

@@ -92,7 +92,7 @@ void SubscriptionResumptionSessionEstablisher::HandleDeviceConnected(void * cont
     InteractionModelEngine * imEngine                                  = InteractionModelEngine::GetInstance();
 
     // Decrement the number of subscriptions to resume
-    imEngine->DecrementNumSubscriptionToResume();
+    imEngine->DecrementNumSubscriptionsToResume();
 
     if (!imEngine->EnsureResourceForSubscription(subscriptionInfo.mFabricIndex, subscriptionInfo.mAttributePaths.AllocatedSize(),
                                                  subscriptionInfo.mEventPaths.AllocatedSize()))
@@ -130,7 +130,7 @@ void SubscriptionResumptionSessionEstablisher::HandleDeviceConnectionFailure(voi
                  error.Format());
 
     // Decrement the number of subscriptions to resume
-    imEngine->DecrementNumSubscriptionToResume();
+    imEngine->DecrementNumSubscriptionsToResume();
 
     auto * subscriptionResumptionStorage = imEngine->GetSubscriptionResumptionStorage();
     if (!subscriptionResumptionStorage)

src/app/icd/server/BUILD.gn

@@ -24,7 +24,7 @@ buildconfig_header("icd-server-buildconfig") {
 
   if (chip_enable_icd_lit || chip_enable_icd_checkin ||
       chip_enable_icd_user_active_mode_trigger) {
-    assert(chip_enable_icd_lit)
+    assert(chip_enable_icd_server)
   }
 
   if (chip_enable_icd_lit) {

src/app/icd/server/ICDManager.cpp

@@ -108,7 +108,7 @@ void ICDManager::Shutdown()
 
 #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
     mIsBootUpResumeSubscriptionExecuted = false;
-#endif // !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
+#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
 #endif // CHIP_CONFIG_ENABLE_ICD_CIP
 }
 
@@ -208,7 +208,7 @@ void ICDManager::SendCheckInMsgs()
 #endif // CONFIG_BUILD_FOR_HOST_UNIT_TEST
 }
 
-bool ICDManager::CheckInMessagesWouldBeSent(const std::function<ShouldCheckInMsgsBeSentFunction> shouldCheckInMsgsBeSentFunction &)
+bool ICDManager::CheckInMessagesWouldBeSent(const std::function<ShouldCheckInMsgsBeSentFunction> & shouldCheckInMsgsBeSentFunction)
 {
     VerifyOrReturnValue(shouldCheckInMsgsBeSentFunction, false);
 
@@ -250,18 +250,18 @@ bool ICDManager::CheckInMessagesWouldBeSent(const std::function<ShouldCheckInMsg
 
 /**
  * ShouldCheckInMsgsBeSentAtActiveModeFunction is used to determine if a Check-In message is required for a given registration.
- * Due to how the ICD Check-In use-case interacts with the persistent subscription and subscription timeout resumption,
- * having a single implementation of the function renders the implematention very difficult to understand and maintain.
- * Because of this, each valid feature combination has its own implementation of the verifier.
+ * Due to how the ICD Check-In use-case interacts with the persistent subscription and subscription timeout resumption features,
+ * having a single implementation of the function renders the implementation very difficult to understand and maintain.
+ * Because of this, each valid feature combination has its own implementation of the function.
  */
 #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
 #if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
 /**
  * @brief Implementation for when the persistant subscription and subscription timeout resumption feature are present.
- *        Verifier checks that there no active or persisted subscriptions for a given fabricIndex or subjectID.
+ *        Function checks that there no active or persisted subscriptions for a given fabricIndex or subjectID.
  *
  * @param aFabricIndex
- * @param subjectID subjectID to check. Can be an opperationnal node id or a CAT
+ * @param subjectID subjectID to check. Can be an operational node id or a CAT
  *
  * @return true Returns true if the fabricIndex and subjectId combination does not have an active or a persisted subscription.
  * @return false Returns false if the fabricIndex and subjectId combination has an active or persisted subscription.
@@ -273,13 +273,13 @@ bool ICDManager::ShouldCheckInMsgsBeSentAtActiveModeFunction(FabricIndex aFabric
 }
 #else
 /**
- * @brief Implementation for when the persistant subscription is present without the  subscription timeout resumption feature.
- *        Verifier checks that there no active subscriptions. If the boot up subscription resumption has been completed,
- *        verifier also checks if there are persisted subscriptions.
+ * @brief Implementation for when the persistant subscription feature is present without the subscription timeout resumption
+ * feature. Function checks that there no active subscriptions. If the boot up subscription resumption has not been completed,
+ *        function also checks if there are persisted subscriptions.
  *
  * @note The persistent subscriptions feature tries to resume subscriptions at the highest min interval
  *       of all the persisted subscriptions. As such, it is possible for the ICD to return to Idle Mode
- *       until the timer elaspses. We do not when to send Check-In message to clients with persisted subscriptions
+ *       until the timer elaspses. We do not want to send Check-In messages to clients with persisted subscriptions
  *       until we have tried to resubscribe.
  *
  * @param aFabricIndex
@@ -289,7 +289,7 @@ bool ICDManager::ShouldCheckInMsgsBeSentAtActiveModeFunction(FabricIndex aFabric
  *              If the boot up susbscription has not been completed, there must not be a persisted subscription either.
  * @return false Returns false if the fabricIndex and subjectId combination has an active subscription.
  *               If the boot up susbscription has not been completed,
- *               if the fabricIndex and subjectId combination has a persisted subscription.
+ *               returns false if the fabricIndex and subjectId combination has a persisted subscription.
  */
 bool ICDManager::ShouldCheckInMsgsBeSentAtActiveModeFunction(FabricIndex aFabricIndex, NodeId subjectID)
 {
@@ -305,8 +305,8 @@ bool ICDManager::ShouldCheckInMsgsBeSentAtActiveModeFunction(FabricIndex aFabric
 #endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
 #else
 /**
- * @brief Implementation for when neither the persistant subscription and subscription timeout resumption feature are present.
- *        Verifier checks that there no active sbuscriptions for a given fabricIndex and subjectId combination.
+ * @brief Implementation for when neither the persistant subscription and subscription timeout resumption features are present.
+ *        Function checks that there no active sbuscriptions for a given fabricIndex and subjectId combination.
  *
  * @param aFabricIndex
  * @param subjectID subjectID to check. Can be an opperationnal node id or a CAT
@@ -328,6 +328,7 @@ void ICDManager::TriggerCheckInMessages(const std::function<ShouldCheckInMsgsBeS
     // If we are already in ActiveMode, Check-In messages have already been sent.
     VerifyOrReturn(mOperationalState == OperationalState::IdleMode);
 
+    // If we don't have any Check-In messages to send, do nothing
     VerifyOrReturn(CheckInMessagesWouldBeSent(verifier));
     UpdateOperationState(OperationalState::ActiveMode);
 }
@@ -390,15 +391,15 @@ void ICDManager::UpdateOperationState(OperationalState state)
         mOperationalState = OperationalState::IdleMode;
 
 #if CHIP_CONFIG_ENABLE_ICD_CIP
-        std::function<ShouldCheckInMsgsBeSentFunction> verifier =
+        std::function<ShouldCheckInMsgsBeSentFunction> function =
             std::bind(&ICDManager::ShouldCheckInMsgsBeSentAtActiveModeFunction, this, std::placeholders::_1, std::placeholders::_2);
 #endif // CHIP_CONFIG_ENABLE_ICD_CIP
 
         // When the active mode interval is 0, we stay in idleMode until a notification brings the icd into active mode
         // unless the device would need to send Check-In messages
         if (ICDConfigurationData::GetInstance().GetActiveModeDuration() > kZero
 #if CHIP_CONFIG_ENABLE_ICD_CIP
-            || CheckInMessagesWouldBeSent(verifier)
+            || CheckInMessagesWouldBeSent(function)
 #endif // CHIP_CONFIG_ENABLE_ICD_CIP
         )
         {

src/app/icd/server/ICDManager.h

@@ -135,9 +135,9 @@ class ICDManager : public ICDListener
     /**
      * @brief Trigger the ICDManager to send Check-In message if necessary
      *
-     * @param[in] algo Verifier function to use to determine if we need to send check-in messages
+     * @param[in] function to use to determine if we need to send check-in messages
      */
-    void TriggerCheckInMessages(const std::function<ShouldCheckInMsgsBeSentFunction> & verifier);
+    void TriggerCheckInMessages(const std::function<ShouldCheckInMsgsBeSentFunction> & function);
 
 #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
     /**
@@ -151,7 +151,9 @@ class ICDManager : public ICDListener
 
 #if CONFIG_BUILD_FOR_HOST_UNIT_TEST
     void SetTestFeatureMapValue(uint32_t featureMap) { mFeatureMap = featureMap; };
+#if !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION && CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
     bool GetIsBootUpResumeSubscriptionExecuted() { return mIsBootUpResumeSubscriptionExecuted; };
+#endif // !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION && CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
 #endif
 
     // Implementation of ICDListener functions.
@@ -201,7 +203,7 @@ class ICDManager : public ICDListener
      * @return false None of the registration would require a Check-In message either because there are no registration or
      * because they all have associated subscriptions.
      */
-    bool CheckInMessagesWouldBeSent(std::function<ShouldCheckInMsgsBeSentFunction> function);
+    bool CheckInMessagesWouldBeSent(const std::function<ShouldCheckInMsgsBeSentFunction> & function);
 #endif // CHIP_CONFIG_ENABLE_ICD_CIP
 
     KeepActiveFlags mKeepActiveFlags{ 0 };
@@ -214,7 +216,7 @@ class ICDManager : public ICDListener
 #if CHIP_CONFIG_ENABLE_ICD_CIP
 #if !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION && CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
     bool mIsBootUpResumeSubscriptionExecuted = false;
-#endif // !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
+#endif // !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION && CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
     PersistentStorageDelegate * mStorage           = nullptr;
     FabricTable * mFabricTable                     = nullptr;
     Messaging::ExchangeManager * mExchangeManager  = nullptr;

src/app/server/Server.cpp

@@ -449,9 +449,9 @@ void Server::OnPlatformEvent(const DeviceLayer::ChipDeviceEvent & event)
         // We trigger Check-In messages before resuming subscriptions to avoid doing both.
         if (!mFailSafeContext.IsFailSafeArmed())
         {
-            std::function<chip::app::ICDManager::ShouldCheckInMsgsBeSentFunction> verifier =
-                std::bind(&Server::CheckInWouldBeSentAtBootVerifier, this, std::placeholders::_1, std::placeholders::_2);
-            mICDManager.TriggerCheckInMessages(verifier);
+            std::function<chip::app::ICDManager::ShouldCheckInMsgsBeSentFunction> function =
+                std::bind(&Server::ShouldCheckInMsgsBeSentAtBootFunction, this, std::placeholders::_1, std::placeholders::_2);
+            mICDManager.TriggerCheckInMessages(function);
         }
 #endif // CHIP_CONFIG_ENABLE_ICD_SERVER && CHIP_CONFIG_ENABLE_ICD_CIP
 #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS

src/app/server/Server.h

@@ -372,14 +372,14 @@ class Server
 
 #if CHIP_CONFIG_ENABLE_ICD_CIP
     /**
-     * @brief Verifier to determine if a Check-In message would be sent at Boot up
+     * @brief Function to determine if a Check-In message would be sent at Boot up
      *
      * @param aFabricIndex client fabric index
      * @param subjectID client subject ID
      * @return true Check-In message would be sent on boot up.
      * @return false Device has a persisted subscription with the client. See CHIP_CONFIG_PERSIST_SUBSCRIPTIONS.
      */
-    bool CheckInWouldBeSentAtBootVerifier(FabricIndex aFabricIndex, NodeId subjectID);
+    bool ShouldCheckInMsgsBeSentAtBootFunction(FabricIndex aFabricIndex, NodeId subjectID);
 #endif // CHIP_CONFIG_ENABLE_ICD_CIP
 #endif // CHIP_CONFIG_ENABLE_ICD_SERVER
 

src/app/tests/TestInteractionModelEngine.cpp

@@ -84,7 +84,7 @@ class TestInteractionModelEngine
     static void TestSubjectHasActiveSubscriptionMultipleSubsSingleEntry(nlTestSuite * apSuite, void * apContext);
     static void TestSubjectHasActiveSubscriptionMultipleSubsMultipleEntries(nlTestSuite * apSuite, void * apContext);
     static void TestSubjectHasActiveSubscriptionSubWithCAT(nlTestSuite * apSuite, void * apContext);
-    static void TestDecrementNumSubscriptionToResume(nlTestSuite * apSuite, void * apContext);
+    static void TestDecrementNumSubscriptionsToResume(nlTestSuite * apSuite, void * apContext);
 };
 
 int TestInteractionModelEngine::GetAttributePathListLength(SingleLinkedListNode<AttributePathParams> * apAttributePathParamsList)
@@ -683,9 +683,8 @@ void TestInteractionModelEngine::TestSubscriptionResumptionTimer(nlTestSuite * a
 }
 
 #endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
-#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
 
-void TestInteractionModelEngine::TestDecrementNumSubscriptionToResume(nlTestSuite * apSuite, void * apContext)
+void TestInteractionModelEngine::TestDecrementNumSubscriptionsToResume(nlTestSuite * apSuite, void * apContext)
 {
     TestContext & ctx                       = *static_cast<TestContext *>(apContext);
     CHIP_ERROR err                          = CHIP_NO_ERROR;
@@ -696,51 +695,52 @@ void TestInteractionModelEngine::TestDecrementNumSubscriptionToResume(nlTestSuit
     err = engine->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable(), app::reporting::GetDefaultReportScheduler());
     NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
 
-#if CHIP_CONFIG_ENABLE_ICD_CIP && CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
+#if CHIP_CONFIG_ENABLE_ICD_CIP && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
     ICDManager manager;
     engine->SetICDManager(&manager);
 #endif // CHIP_CONFIG_ENABLE_ICD_CIP && CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
 
     // Set number of subs
     engine->mNumOfSubscriptionToResume = kNumberOfSubsToResume;
 
-#if CHIP_CONFIG_ENABLE_ICD_CIP && CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
+#if CHIP_CONFIG_ENABLE_ICD_CIP && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
     // Verify mIsBootUpResumeSubscriptionExecuted has not been set
     NL_TEST_ASSERT(apSuite, !manager.GetIsBootUpResumeSubscriptionExecuted());
 #endif // CHIP_CONFIG_ENABLE_ICD_CIP && CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
 
     // Decrease number of subs by 1
     numberOfSubsRemaining--;
-    engine->DecrementNumSubscriptionToResume();
+    engine->DecrementNumSubscriptionsToResume();
     NL_TEST_ASSERT_EQUALS(apSuite, numberOfSubsRemaining, engine->mNumOfSubscriptionToResume);
 
     // Decrease to 0 subs remaining
     while (numberOfSubsRemaining > 0)
     {
-#if CHIP_CONFIG_ENABLE_ICD_CIP && CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
+#if CHIP_CONFIG_ENABLE_ICD_CIP && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
         // Verify mIsBootUpResumeSubscriptionExecuted has not been set
         NL_TEST_ASSERT(apSuite, !manager.GetIsBootUpResumeSubscriptionExecuted());
 #endif // CHIP_CONFIG_ENABLE_ICD_CIP && CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
 
         numberOfSubsRemaining--;
-        engine->DecrementNumSubscriptionToResume();
+        engine->DecrementNumSubscriptionsToResume();
         NL_TEST_ASSERT_EQUALS(apSuite, numberOfSubsRemaining, engine->mNumOfSubscriptionToResume);
     }
 
-#if CHIP_CONFIG_ENABLE_ICD_CIP && CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
+#if CHIP_CONFIG_ENABLE_ICD_CIP && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
     // Verify mIsBootUpResumeSubscriptionExecuted has been set
     NL_TEST_ASSERT(apSuite, manager.GetIsBootUpResumeSubscriptionExecuted());
 #endif // CHIP_CONFIG_ENABLE_ICD_CIP && CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
 
     // Make sure we don't rollover / go negative
-    engine->DecrementNumSubscriptionToResume();
+    engine->DecrementNumSubscriptionsToResume();
     NL_TEST_ASSERT_EQUALS(apSuite, numberOfSubsRemaining, engine->mNumOfSubscriptionToResume);
 
     // Clean up
-#if CHIP_CONFIG_ENABLE_ICD_CIP && CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
+#if CHIP_CONFIG_ENABLE_ICD_CIP && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
     engine->SetICDManager(nullptr);
 #endif // CHIP_CONFIG_ENABLE_ICD_CIP && CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
 }
+#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
 
 } // namespace app
 } // namespace chip
@@ -754,6 +754,7 @@ const nlTest sTests[] =
                 NL_TEST_DEF("TestRemoveDuplicateConcreteAttribute", chip::app::TestInteractionModelEngine::TestRemoveDuplicateConcreteAttribute),
 #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
                 NL_TEST_DEF("TestSubjectHasPersistedSubscription", chip::app::TestInteractionModelEngine::TestSubjectHasPersistedSubscription),
+                NL_TEST_DEF("TestDecrementNumSubscriptionsToResume", chip::app::TestInteractionModelEngine::TestDecrementNumSubscriptionsToResume),
 #if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
                 NL_TEST_DEF("TestSubscriptionResumptionTimer", chip::app::TestInteractionModelEngine::TestSubscriptionResumptionTimer),
 #endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
@@ -763,7 +764,6 @@ const nlTest sTests[] =
                 NL_TEST_DEF("TestSubjectHasActiveSubscriptionMultipleSubsSingleEntry", chip::app::TestInteractionModelEngine::TestSubjectHasActiveSubscriptionMultipleSubsSingleEntry),
                 NL_TEST_DEF("TestSubjectHasActiveSubscriptionMultipleSubsMultipleEntries", chip::app::TestInteractionModelEngine::TestSubjectHasActiveSubscriptionMultipleSubsMultipleEntries),
                 NL_TEST_DEF("TestSubjectHasActiveSubscriptionSubWithCAT", chip::app::TestInteractionModelEngine::TestSubjectHasActiveSubscriptionSubWithCAT),
-                NL_TEST_DEF("TestDecrementNumSubscriptionToResume", chip::app::TestInteractionModelEngine::TestDecrementNumSubscriptionToResume),
                 NL_TEST_SENTINEL()
         };
 // clang-format on