Finish refactor of the Sub logic when interacting with the ICDManager

Author: mkardous-silabs

src/app/BUILD.gn

@@ -201,6 +201,7 @@ static_library("interaction-model") {
     ":subscription-info-provider",
     "${chip_root}/src/app/MessageDef",
     "${chip_root}/src/app/icd/server:icd-server-config",
+    "${chip_root}/src/app/icd/server:manager",
     "${chip_root}/src/app/icd/server:observer",
     "${chip_root}/src/lib/address_resolve",
     "${chip_root}/src/lib/support",

src/app/InteractionModelEngine.cpp

@@ -364,8 +364,10 @@ bool InteractionModelEngine::SubjectHasPersistedSubscription(FabricIndex aFabric
 
 #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
     auto * iterator = mpSubscriptionResumptionStorage->IterateSubscriptions();
-    SubscriptionResumptionStorage::SubscriptionInfo subscriptionInfo;
+    // Verify that we were able t0 allocate an iterator. If not, assume we have a persisted subscription.
+    VerifyOrReturnValue(iterator, true);
 
+    SubscriptionResumptionStorage::SubscriptionInfo subscriptionInfo;
     while (iterator->Next(subscriptionInfo))
     {
         // TODO(#31873): Persistent subscription only stores the NodeID for now. We cannot check if the CAT matches
@@ -1914,22 +1916,22 @@ CHIP_ERROR InteractionModelEngine::ResumeSubscriptions()
     // future improvements: https://github.com/project-chip/connectedhomeip/issues/25439
 
     SubscriptionResumptionStorage::SubscriptionInfo subscriptionInfo;
-    auto * iterator           = mpSubscriptionResumptionStorage->IterateSubscriptions();
-    int subscriptionsToResume = 0;
-    uint16_t minInterval      = 0;
+    auto * iterator            = mpSubscriptionResumptionStorage->IterateSubscriptions();
+    mNumOfSubscriptionToResume = 0;
+    uint16_t minInterval       = 0;
     while (iterator->Next(subscriptionInfo))
     {
-        subscriptionsToResume++;
+        mNumOfSubscriptionToResume++;
         minInterval = std::max(minInterval, subscriptionInfo.mMinInterval);
     }
     iterator->Release();
 
-    if (subscriptionsToResume)
+    if (mNumOfSubscriptionToResume)
     {
 #if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
         mSubscriptionResumptionScheduled = true;
 #endif
-        ChipLogProgress(InteractionModel, "Resuming %d subscriptions in %u seconds", subscriptionsToResume, minInterval);
+        ChipLogProgress(InteractionModel, "Resuming %d subscriptions in %u seconds", mNumOfSubscriptionToResume, minInterval);
         ReturnErrorOnFailure(mpExchangeMgr->GetSessionManager()->SystemLayer()->StartTimer(System::Clock::Seconds16(minInterval),
                                                                                            ResumeSubscriptionsTimerCallback, this));
     }
@@ -2051,5 +2053,24 @@ bool InteractionModelEngine::HasSubscriptionsToResume()
 }
 #endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
 
+#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
+void InteractionModelEngine::DecrementNumSubscriptionToResume()
+{
+    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
+
+    mNumOfSubscriptionToResume--;
+
+#if CHIP_CONFIG_ENABLE_ICD_CIP && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
+    if (!mNumOfSubscriptionToResume)
+    {
+        mICDManager->SetBootUpResumeSubscriptionExecuted();
+    }
+#endif // CHIP_CONFIG_ENABLE_ICD_CIP && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
+}
+#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
+
 } // namespace app
 } // namespace chip

src/app/InteractionModelEngine.h

@@ -49,6 +49,7 @@
 #include <app/TimedHandler.h>
 #include <app/WriteClient.h>
 #include <app/WriteHandler.h>
+#include <app/icd/server/ICDServerConfig.h>
 #include <app/reporting/Engine.h>
 #include <app/reporting/ReportScheduler.h>
 #include <app/util/attribute-metadata.h>
@@ -68,6 +69,10 @@
 
 #include <app/CASESessionManager.h>
 
+#if CHIP_CONFIG_ENABLE_ICD_SERVER
+#include <app/icd/server/ICDManager.h> // nogncheck
+#endif                                 // CHIP_CONFIG_ENABLE_ICD_SERVER
+
 namespace chip {
 namespace app {
 
@@ -128,6 +133,10 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
 
     void Shutdown();
 
+#if CHIP_CONFIG_ENABLE_ICD_SERVER
+    void SetICDManager(ICDManager * manager) { mICDManager = manager; };
+#endif // CHIP_CONFIG_ENABLE_ICD_SERVER
+
     Messaging::ExchangeManager * GetExchangeManager(void) const { return mpExchangeMgr; }
 
     /**
@@ -318,6 +327,10 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
 
     bool SubjectHasPersistedSubscription(FabricIndex aFabricIndex, NodeId subjectID) override;
 
+#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
+    void DecrementNumSubscriptionToResume();
+#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
+
 #if CONFIG_BUILD_FOR_HOST_UNIT_TEST
     //
     // Get direct access to the underlying read handler pool
@@ -602,6 +615,10 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
 
     CommandHandlerInterface * mCommandHandlerList = nullptr;
 
+#if CHIP_CONFIG_ENABLE_ICD_SERVER
+    ICDManager * mICDManager = nullptr;
+#endif // CHIP_CONFIG_ENABLE_ICD_SERVER
+
     ObjectPool<CommandHandler, CHIP_IM_MAX_NUM_COMMAND_HANDLER> mCommandHandlerObjs;
     ObjectPool<TimedHandler, CHIP_IM_MAX_NUM_TIMED_HANDLER> mTimedHandlers;
     WriteHandler mWriteHandlers[CHIP_IM_MAX_NUM_WRITE_HANDLER];
@@ -660,12 +677,15 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
 #endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
 #endif // CONFIG_BUILD_FOR_HOST_UNIT_TEST
 
-#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
+#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
+    int8_t mNumOfSubscriptionToResume = 0;
+#if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
     bool HasSubscriptionsToResume();
     uint32_t ComputeTimeSecondsTillNextSubscriptionResumption();
     uint32_t mNumSubscriptionResumptionRetries = 0;
     bool mSubscriptionResumptionScheduled      = false;
-#endif
+#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
+#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
 
     FabricTable * mpFabricTable;
 

src/app/SubscriptionResumptionSessionEstablisher.cpp

@@ -90,15 +90,21 @@ void SubscriptionResumptionSessionEstablisher::HandleDeviceConnected(void * cont
     AutoDeleteEstablisher establisher(static_cast<SubscriptionResumptionSessionEstablisher *>(context));
     SubscriptionResumptionStorage::SubscriptionInfo & subscriptionInfo = establisher->mSubscriptionInfo;
     InteractionModelEngine * imEngine                                  = InteractionModelEngine::GetInstance();
+
+    // Decrement the number of subscriptions to resume
+    imEngine->DecrementNumSubscriptionToResume();
+
     if (!imEngine->EnsureResourceForSubscription(subscriptionInfo.mFabricIndex, subscriptionInfo.mAttributePaths.AllocatedSize(),
                                                  subscriptionInfo.mEventPaths.AllocatedSize()))
     {
+        // TODO - Should we keep the subscription here?
         ChipLogProgress(InteractionModel, "no resource for subscription resumption");
         return;
     }
     ReadHandler * readHandler = imEngine->mReadHandlers.CreateObject(*imEngine, imEngine->GetReportScheduler());
     if (readHandler == nullptr)
     {
+        // TODO - Should we keep the subscription here?
         ChipLogProgress(InteractionModel, "no resource for ReadHandler creation");
         return;
     }
@@ -118,10 +124,15 @@ void SubscriptionResumptionSessionEstablisher::HandleDeviceConnectionFailure(voi
                                                                              CHIP_ERROR error)
 {
     AutoDeleteEstablisher establisher(static_cast<SubscriptionResumptionSessionEstablisher *>(context));
+    InteractionModelEngine * imEngine                                  = InteractionModelEngine::GetInstance();
     SubscriptionResumptionStorage::SubscriptionInfo & subscriptionInfo = establisher->mSubscriptionInfo;
     ChipLogError(DataManagement, "Failed to establish CASE for subscription-resumption with error '%" CHIP_ERROR_FORMAT "'",
                  error.Format());
-    auto * subscriptionResumptionStorage = InteractionModelEngine::GetInstance()->GetSubscriptionResumptionStorage();
+
+    // Decrement the number of subscriptions to resume
+    imEngine->DecrementNumSubscriptionToResume();
+
+    auto * subscriptionResumptionStorage = imEngine->GetSubscriptionResumptionStorage();
     if (!subscriptionResumptionStorage)
     {
         ChipLogError(DataManagement, "Failed to get subscription resumption storage");

src/app/icd/server/BUILD.gn

@@ -22,6 +22,11 @@ buildconfig_header("icd-server-buildconfig") {
   header = "ICDServerBuildConfig.h"
   header_dir = "app/icd/server"
 
+  if (chip_enable_icd_lit || chip_enable_icd_checkin ||
+      chip_enable_icd_user_active_mode_trigger) {
+    assert(chip_enable_icd_lit)
+  }
+
   if (chip_enable_icd_lit) {
     assert(chip_enable_icd_checkin && chip_enable_icd_user_active_mode_trigger)
   }

src/app/icd/server/ICDManager.cpp

@@ -105,6 +105,10 @@ void ICDManager::Shutdown()
     mFabricTable     = nullptr;
     mSubInfoProvider = nullptr;
     mICDSenderPool.ReleaseAll();
+
+#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
+    mIsBootUpResumeSubscriptionExecuted = false;
+#endif // !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
 #endif // CHIP_CONFIG_ENABLE_ICD_CIP
 }
 
@@ -152,6 +156,7 @@ void ICDManager::SendCheckInMsgs()
 
         ICDMonitoringTable table(*mStorage, fabricInfo.GetFabricIndex(), supported_clients /*Table entry limit*/,
                                  mSymmetricKeystore);
+
         if (table.IsEmpty())
         {
             continue;
@@ -173,8 +178,7 @@ void ICDManager::SendCheckInMsgs()
                 continue;
             }
 
-            bool active = mSubInfoProvider->SubjectHasActiveSubscription(entry.fabricIndex, entry.monitoredSubject);
-            if (active)
+            if (!CheckInWouldBeSentAtActiveModeVerifier(entry.fabricIndex, entry.monitoredSubject))
             {
                 continue;
             }
@@ -244,17 +248,77 @@ bool ICDManager::CheckInMessagesWouldBeSent(std::function<RegistrationVerificati
     return false;
 }
 
+/**
+ * CheckInWouldBeSentAtActiveModeVerifier 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.
+ */
+#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.
+ *
+ * @param aFabricIndex
+ * @param subjectID subjectID to check. Can be an opperationnal 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.
+ */
+bool ICDManager::CheckInWouldBeSentAtActiveModeVerifier(FabricIndex aFabricIndex, NodeId subjectID)
+{
+    return !(mSubInfoProvider->SubjectHasActiveSubscription(aFabricIndex, subjectID) ||
+             mSubInfoProvider->SubjectHasPersistedSubscription(aFabricIndex, subjectID));
+}
+#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.
+ *
+ * @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 we have tried to resubscribe.
+ *
+ * @param aFabricIndex
+ * @param subjectID subjectID to check. Can be an opperationnal node id or a CAT
+ *
+ * @return true Returns true if the fabricIndex and subjectId combination does not have an active subscription.
+ *              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.
+ */
 bool ICDManager::CheckInWouldBeSentAtActiveModeVerifier(FabricIndex aFabricIndex, NodeId subjectID)
 {
-    VerifyOrReturnValue(mSubInfoProvider->SubjectHasActiveSubscription(aFabricIndex, subjectID), true);
+    bool wouldSendCheckIn = !(mSubInfoProvider->SubjectHasActiveSubscription(aFabricIndex, subjectID));
 
-#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
-    // If at least one registration has a persisted entry, do not send Check-In message.
-    // The resumption of the persisted subscription will serve the same function a check-in would have served.
-    VerifyOrReturnValue(mSubInfoProvider->SubjectHasPersistedSubscription(aFabricIndex, subjectID), true);
-#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
-    return false;
+    if (!mIsBootUpResumeSubscriptionExecuted)
+    {
+        wouldSendCheckIn = wouldSendCheckIn && !mSubInfoProvider->SubjectHasPersistedSubscription(aFabricIndex, subjectID);
+    }
+
+    return wouldSendCheckIn;
+}
+#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.
+ *
+ * @param aFabricIndex
+ * @param subjectID subjectID to check. Can be an opperationnal node id or a CAT
+ *
+ * @return true Returns true if the fabricIndex and subjectId combination does not have an active subscription.
+ * @return false Returns false if the fabricIndex and subjectId combination has an active subscription.
+ */
+bool ICDManager::CheckInWouldBeSentAtActiveModeVerifier(FabricIndex aFabricIndex, NodeId subjectID)
+{
+    return !(mSubInfoProvider->SubjectHasActiveSubscription(aFabricIndex, subjectID));
 }
+#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
 
 void ICDManager::TriggerCheckInMessages(const std::function<RegistrationVerificationFunction> & verifier)
 {

src/app/icd/server/ICDManager.h

@@ -138,10 +138,20 @@ class ICDManager : public ICDListener
      * @param[in] algo Verifier function to use to determine if we need to send check-in messages
      */
     void TriggerCheckInMessages(const std::function<RegistrationVerificationFunction> & verifier);
+
+#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
+    /**
+     * @brief Set mSubCheckInBootCheckExecuted to true
+     *        Function allows the InteractionModelEngine to notify the ICDManager that the boot up subscription resumption has been
+     *        completed.
+     */
+    void SetBootUpResumeSubscriptionExecuted() { mIsBootUpResumeSubscriptionExecuted = true; };
+#endif // !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION && CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
 #endif // CHIP_CONFIG_ENABLE_ICD_CIP
 
 #if CONFIG_BUILD_FOR_HOST_UNIT_TEST
     void SetTestFeatureMapValue(uint32_t featureMap) { mFeatureMap = featureMap; };
+    bool GetIsBootUpResumeSubscriptionExecuted() { return mIsBootUpResumeSubscriptionExecuted; };
 #endif
 
     // Implementation of ICDListener functions.
@@ -212,6 +222,9 @@ class ICDManager : public ICDListener
     ObjectPool<ObserverPointer, CHIP_CONFIG_ICD_OBSERVERS_POOL_SIZE> mStateObserverPool;
 
 #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
     PersistentStorageDelegate * mStorage           = nullptr;
     FabricTable * mFabricTable                     = nullptr;
     Messaging::ExchangeManager * mExchangeManager  = nullptr;

src/app/server/Server.cpp

@@ -332,6 +332,10 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
                                                                  &mCASESessionManager, mSubscriptionResumptionStorage);
     SuccessOrExit(err);
 
+#if CHIP_CONFIG_ENABLE_ICD_SERVER
+    chip::app::InteractionModelEngine::GetInstance()->SetICDManager(&mICDManager);
+#endif // CHIP_CONFIG_ENABLE_ICD_SERVER
+
     // ICD Init needs to be after data model init and InteractionModel Init
 #if CHIP_CONFIG_ENABLE_ICD_SERVER
 
@@ -452,7 +456,7 @@ void Server::OnPlatformEvent(const DeviceLayer::ChipDeviceEvent & event)
 #endif // CHIP_CONFIG_ENABLE_ICD_SERVER && CHIP_CONFIG_ENABLE_ICD_CIP
 #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
         ResumeSubscriptions();
-#endif
+#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
         break;
 #if CHIP_SYSTEM_CONFIG_USE_OPEN_THREAD_ENDPOINT
     case DeviceEventType::kThreadConnectivityChange:
@@ -524,9 +528,9 @@ void Server::RejoinExistingMulticastGroups()
 bool Server::CheckInWouldBeSentAtBootVerifier(FabricIndex aFabricIndex, NodeId subjectID)
 {
 #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
-    // At least one registration has a persisted entry. Do not send Check-In message.
-    // This is to cover the use-case where the subscription resumption feature is used with the Check-In message.
-    VerifyOrReturnValue(!chip::app::InteractionModelEngine::GetInstance()->SubjectHasPersistedSubscription(aFabricIndex, subjectID),
+    // If at least one registration has a persisted entry, do not send Check-In message.
+    // The resumption of the persisted subscription will serve the same function a check-in would have served.
+    VerifyOrReturnValue(!app::InteractionModelEngine::GetInstance()->SubjectHasPersistedSubscription(aFabricIndex, subjectID),
                         false);
 #endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
 
@@ -576,6 +580,9 @@ void Server::Shutdown()
 
     chip::Dnssd::Resolver::Instance().Shutdown();
     chip::app::InteractionModelEngine::GetInstance()->Shutdown();
+#if CHIP_CONFIG_ENABLE_ICD_SERVER
+    chip::app::InteractionModelEngine::GetInstance()->SetICDManager(nullptr);
+#endif // CHIP_CONFIG_ENABLE_ICD_SERVER
     mCommissioningWindowManager.Shutdown();
     mMessageCounterManager.Shutdown();
     mExchangeMgr.Shutdown();