[ICD] Fix ordering issue when triggering a report on Active mode

examples/lit-icd-app/linux/args.gni

@@ -29,3 +29,4 @@ matter_enable_tracing_support = true
 # ICD configurations
 chip_enable_icd_server = true
 chip_subscription_timeout_resumption = false
+chip_icd_report_on_active_mode = true

examples/lit-icd-app/silabs/openthread.gni

@@ -31,6 +31,7 @@ chip_enable_icd_server = true
 chip_subscription_timeout_resumption = false
 sl_use_subscription_synching = true
 icd_enforce_sit_slow_poll_limit = true
+chip_icd_report_on_active_mode = true
 
 # Openthread Configuration flags
 sl_ot_idle_interval_ms = 3600000  # 60mins Idle Polling Interval

src/app/icd/BUILD.gn

@@ -24,7 +24,7 @@ buildconfig_header("icd_buildconfig") {
 
   defines = [
     "CHIP_CONFIG_ENABLE_ICD_SERVER=${chip_enable_icd_server}",
-    "ICD_REPORT_ON_ENTER_ACTIVE_MODE=${chip_report_on_active_mode}",
+    "ICD_REPORT_ON_ENTER_ACTIVE_MODE=${chip_icd_report_on_active_mode}",
     "ICD_MAX_NOTIFICATION_SUBSCRIBERS=${icd_max_notification_subscribers}",
     "ICD_ENFORCE_SIT_SLOW_POLL_LIMIT=${icd_enforce_sit_slow_poll_limit}",
   ]

src/app/icd/ICDManager.cpp

@@ -488,6 +488,15 @@ void ICDManager::OnICDManagementServerEvent(ICDManagementEvents event)
     }
 }
 
+void ICDManager::OnSubscriptionReport()
+{
+    // If the device is already in ActiveMode, that means that all active subscriptions have already been marked dirty.
+    // Since we only mark them dirty when we enter ActiveMode, it is not necessary to update the operational state a second time.
+    // Doing so will only add an ActiveModeThreshold to the active time which we don't want to do here.
+    VerifyOrReturn(mOperationalState == OperationalState::IdleMode);
+    this->UpdateOperationState(OperationalState::ActiveMode);
+}
+
 ICDManager::ObserverPointer * ICDManager::RegisterObserver(ICDStateObserver * observer)
 {
     return mStateObserverPool.CreateObject(observer);

src/app/icd/ICDManager.h

@@ -105,6 +105,7 @@ class ICDManager : public ICDListener
     void OnKeepActiveRequest(KeepActiveFlags request) override;
     void OnActiveRequestWithdrawal(KeepActiveFlags request) override;
     void OnICDManagementServerEvent(ICDManagementEvents event) override;
+    void OnSubscriptionReport() override;
 
 protected:
     friend class TestICDManager;

src/app/icd/ICDNotifier.cpp

@@ -100,5 +100,16 @@ void ICDNotifier::BroadcastICDManagementEvent(ICDListener::ICDManagementEvents e
     }
 }
 
+void ICDNotifier::BroadcastSubscriptionReport()
+{
+    for (auto subscriber : mSubscribers)
+    {
+        if (subscriber != nullptr)
+        {
+            subscriber->OnSubscriptionReport();
+        }
+    }
+}
+
 } // namespace app
 } // namespace chip

src/app/icd/ICDNotifier.h

@@ -58,34 +58,40 @@ class ICDListener
     virtual ~ICDListener() {}
 
     /**
-     * @brief This function is called for all subscribers of the ICDNotifier when it calls BroadcastNetworkActivityNotification
+     * @brief This function is called for all subscribers of the ICDNotifier when it calls BroadcastNetworkActivityNotification.
      * It notifies the subscriber that a NetworkActivity occurred. For example, a message sent or received.
      */
     virtual void OnNetworkActivity() = 0;
 
     /**
-     * @brief This function is called for all subscribers of the ICDNotifier when it calls BroadcastActiveRequestNotification
+     * @brief This function is called for all subscribers of the ICDNotifier when it calls BroadcastActiveRequestNotification.
      * It informs the subscriber that there is a need to place and keep the ICD in its Active Mode.
      *
      * @param request : Identity the request source
      */
     virtual void OnKeepActiveRequest(KeepActiveFlags request) = 0;
 
     /**
-     * @brief This function is called for all subscribers of the ICDNotifier when it calls BroadcastActiveRequestWithdrawal
+     * @brief This function is called for all subscribers of the ICDNotifier when it calls BroadcastActiveRequestWithdrawal.
      * It informs the subscriber that a previous request no longer needs ICD to maintain its Active Mode.
      *
      * @param request : The request source
      */
     virtual void OnActiveRequestWithdrawal(KeepActiveFlags request) = 0;
 
     /**
-     * @brief This function is called for all subscribers of the ICDNotifier when it calls BroadcastICDManagementEvent
+     * @brief This function is called for all subscribers of the ICDNotifier when it calls BroadcastICDManagementEvent.
      * It informs the subscriber that an ICD Management action has happened and needs to be processed
      *
      * @param event : The event type
      */
-    virtual void OnICDManagementServerEvent(ICDManagementEvents event){};
+    virtual void OnICDManagementServerEvent(ICDManagementEvents event) = 0;
+
+    /**
+     * @brief This function is called for all subscribers of the ICDNoitifier when it calls BroadcastSubscriptionReport.
+     * It informs the subscriber that a subscription report data is being sent.
+     */
+    virtual void OnSubscriptionReport() = 0;
 };
 
 class ICDNotifier
@@ -104,6 +110,7 @@ class ICDNotifier
     void BroadcastActiveRequestNotification(ICDListener::KeepActiveFlags request);
     void BroadcastActiveRequestWithdrawal(ICDListener::KeepActiveFlags request);
     void BroadcastICDManagementEvent(ICDListener::ICDManagementEvents event);
+    void BroadcastSubscriptionReport();
 
     inline void BroadcastActiveRequest(ICDListener::KeepActiveFlags request, bool notify)
     {

src/app/icd/icd.gni

@@ -22,7 +22,7 @@ declare_args() {
   chip_enable_icd_client = false
 
   # Matter SDK Configuration flag to make the ICD manager emit a report on entering active mode
-  chip_report_on_active_mode = false
+  chip_icd_report_on_active_mode = false
 
   icd_max_notification_subscribers = 1
 

src/app/reporting/Engine.cpp

@@ -23,6 +23,10 @@
  *
  */
 
+#include <app/icd/ICDConfig.h>
+#if CHIP_CONFIG_ENABLE_ICD_SERVER
+#include <app/icd/ICDNotifier.h> // nogncheck
+#endif
 #include <app/AppConfig.h>
 #include <app/InteractionModelEngine.h>
 #include <app/RequiredPrivilege.h>
@@ -509,6 +513,12 @@ CHIP_ERROR Engine::BuildAndSendSingleReportData(ReadHandler * apReadHandler)
 
     if (apReadHandler->IsType(ReadHandler::InteractionType::Subscribe))
     {
+#if CHIP_CONFIG_ENABLE_ICD_SERVER
+        // Notify the ICDManager that we are about to send a subscription report before we prepare the Report payload.
+        // This allows the ICDManager to trigger any necessary updates and have the information in the report about to be sent.
+        app::ICDNotifier::GetInstance().BroadcastSubscriptionReport();
+#endif // CHIP_CONFIG_ENABLE_ICD_SERVER
+
         SubscriptionId subscriptionId = 0;
         apReadHandler->GetSubscriptionId(subscriptionId);
         reportDataBuilder.SubscriptionId(subscriptionId);
@@ -642,6 +652,7 @@ void Engine::Run()
 
         if (readHandler->ShouldReportUnscheduled() || imEngine->GetReportScheduler()->IsReportableNow(readHandler))
         {
+
             mRunningReadHandler = readHandler;
             CHIP_ERROR err      = BuildAndSendSingleReportData(readHandler);
             mRunningReadHandler = nullptr;

src/app/reporting/ReportSchedulerImpl.cpp

@@ -44,7 +44,7 @@ void ReportSchedulerImpl::OnEnterActiveMode()
 {
 #if ICD_REPORT_ON_ENTER_ACTIVE_MODE
     Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp();
-    mNodesPool.ForEachActiveObject([now](ReadHandlerNode * node) {
+    mNodesPool.ForEachActiveObject([this, now](ReadHandlerNode * node) {
         if (now >= node->GetMinTimestamp())
         {
             this->HandlerForceDirtyState(node->GetReadHandler());

src/app/tests/TestICDManager.cpp

@@ -321,6 +321,28 @@ class TestICDManager
         uint32_t counter2 = ICDConfigurationData::GetInstance().GetICDCounter();
         NL_TEST_ASSERT(aSuite, (counter + 1) == counter2);
     }
+
+    static void TestOnSubscriptionReport(nlTestSuite * aSuite, void * aContext)
+    {
+        TestContext * ctx    = static_cast<TestContext *>(aContext);
+        ICDNotifier notifier = ICDNotifier::GetInstance();
+
+        // After the init we should be in Idle mode
+        NL_TEST_ASSERT(aSuite, ctx->mICDManager.mOperationalState == ICDManager::OperationalState::IdleMode);
+
+        // Trigger a subscription report
+        notifier.BroadcastSubscriptionReport();
+        NL_TEST_ASSERT(aSuite, ctx->mICDManager.mOperationalState == ICDManager::OperationalState::ActiveMode);
+
+        // Trigger another subscription report - active time should not be increased
+        notifier.BroadcastSubscriptionReport();
+
+        // Advance time so active mode interval expires.
+        AdvanceClockAndRunEventLoop(ctx, ICDConfigurationData::GetInstance().GetActiveModeDurationMs() + 1);
+
+        // After the init we should be in Idle mode
+        NL_TEST_ASSERT(aSuite, ctx->mICDManager.mOperationalState == ICDManager::OperationalState::IdleMode);
+    }
 };
 
 } // namespace app
@@ -329,6 +351,7 @@ class TestICDManager
 namespace {
 static const nlTest sTests[] = {
     NL_TEST_DEF("TestICDModeDurations", TestICDManager::TestICDModeDurations),
+    NL_TEST_DEF("TestOnSubscriptionReport", TestICDManager::TestOnSubscriptionReport),
     NL_TEST_DEF("TestKeepActivemodeRequests", TestICDManager::TestKeepActivemodeRequests),
     NL_TEST_DEF("TestICDMRegisterUnregisterEvents", TestICDManager::TestICDMRegisterUnregisterEvents),
     NL_TEST_DEF("TestICDCounter", TestICDManager::TestICDCounter),