Revert "[ICD]Add needed elements to the ICD Manager to handle LIT mode (

project-chip#27916)"

This reverts commit e6f610b.

examples/all-clusters-app/esp32/main/CMakeLists.txt

@@ -34,7 +34,6 @@ set(SRC_DIRS_LIST
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/examples/platform/esp32/lock"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/examples/platform/esp32/mode-support"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/server"
-                      "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/icd"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/util"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/reporting"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/administrator-commissioning-server"

examples/all-clusters-minimal-app/esp32/main/CMakeLists.txt

@@ -32,7 +32,6 @@ set(SRC_DIRS_LIST
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/examples/platform/esp32/shell_extension"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/examples/providers"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/server"
-                      "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/icd"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/util"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/reporting"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/administrator-commissioning-server"

src/app/chip_data_model.cmake

@@ -143,7 +143,7 @@ function(chip_configure_data_model APP_TARGET)
         ${CHIP_APP_BASE_DIR}/util/attribute-storage.cpp
         ${CHIP_APP_BASE_DIR}/util/attribute-table.cpp
         ${CHIP_APP_BASE_DIR}/util/binding-table.cpp
-        ${CHIP_APP_BASE_DIR}/icd/IcdMonitoringTable.cpp
+        ${CHIP_APP_BASE_DIR}/util/IcdMonitoringTable.cpp
         ${CHIP_APP_BASE_DIR}/util/DataModelHandler.cpp
         ${CHIP_APP_BASE_DIR}/util/ember-compatibility-functions.cpp
         ${CHIP_APP_BASE_DIR}/util/error-mapping.cpp

src/app/chip_data_model.gni

@@ -165,6 +165,8 @@ template("chip_data_model") {
       "${_app_root}/clusters/scenes-server/SceneTableImpl.h",
       "${_app_root}/clusters/scenes-server/scenes-server.h",
       "${_app_root}/util/DataModelHandler.cpp",
+      "${_app_root}/util/IcdMonitoringTable.cpp",
+      "${_app_root}/util/IcdMonitoringTable.h",
       "${_app_root}/util/attribute-size-util.cpp",
       "${_app_root}/util/attribute-storage.cpp",
       "${_app_root}/util/attribute-table.cpp",
@@ -195,10 +197,6 @@ template("chip_data_model") {
                                      [ invoker.zap_file ])
     }
 
-    if (!defined(deps)) {
-      deps = []
-    }
-
     foreach(cluster, _cluster_sources) {
       if (cluster == "door-lock-server") {
         sources += [
@@ -258,10 +256,6 @@ template("chip_data_model") {
           "${_app_root}/clusters/${cluster}/${cluster}.h",
           "${_app_root}/clusters/${cluster}/operational-state-delegate.h",
         ]
-      } else if (cluster == "icd-management-server") {
-        sources += [ "${_app_root}/clusters/${cluster}/${cluster}.cpp" ]
-
-        deps += [ "${chip_root}/src/app/icd:monitoring-table" ]
       } else {
         sources += [ "${_app_root}/clusters/${cluster}/${cluster}.cpp" ]
       }

src/app/clusters/icd-management-server/icd-management-server.cpp

@@ -26,7 +26,7 @@
 #include <app/AttributeAccessInterface.h>
 #include <app/CommandHandler.h>
 #include <app/ConcreteAttributePath.h>
-#include <app/icd/IcdMonitoringTable.h>
+#include <app/util/IcdMonitoringTable.h>
 #include <app/util/af.h>
 #include <app/util/attribute-storage.h>
 

src/app/icd/BUILD.gn

@@ -25,16 +25,6 @@ source_set("manager-srcs") {
     "ICDManager.h",
   ]
 
-  deps = [ ":monitoring-table" ]
-  public_deps = [ "${chip_root}/src/credentials:credentials" ]
-}
-
-source_set("monitoring-table") {
-  sources = [
-    "IcdMonitoringTable.cpp",
-    "IcdMonitoringTable.h",
-  ]
-
   public_deps = [
     "${chip_root}/src/lib/core",
     "${chip_root}/src/platform:platform",

src/app/icd/ICDManager.cpp

@@ -19,13 +19,11 @@
 #include <app-common/zap-generated/ids/Attributes.h>
 #include <app-common/zap-generated/ids/Clusters.h>
 #include <app/icd/ICDManager.h>
-#include <app/icd/IcdMonitoringTable.h>
 #include <lib/support/CodeUtils.h>
 #include <lib/support/logging/CHIPLogging.h>
 #include <platform/ConnectivityManager.h>
 #include <platform/LockTracker.h>
 #include <platform/internal/CHIPDeviceLayerInternal.h>
-#include <stdlib.h>
 
 namespace chip {
 namespace app {
@@ -34,13 +32,8 @@ using namespace chip::app;
 using namespace chip::app::Clusters;
 using namespace chip::app::Clusters::IcdManagement;
 
-void ICDManager::ICDManager::Init(PersistentStorageDelegate * storage, FabricTable * fabricTable)
+void ICDManager::ICDManager::Init()
 {
-    VerifyOrDie(storage != nullptr);
-    VerifyOrDie(fabricTable != nullptr);
-    mStorage     = storage;
-    mFabricTable = fabricTable;
-
     uint32_t activeModeInterval;
     if (Attributes::ActiveModeInterval::Get(kRootEndpointId, &activeModeInterval) != EMBER_ZCL_STATUS_SUCCESS)
     {
@@ -56,17 +49,16 @@ void ICDManager::ICDManager::Shutdown()
     // cancel any running timer of the icd
     DeviceLayer::SystemLayer().CancelTimer(OnIdleModeDone, this);
     DeviceLayer::SystemLayer().CancelTimer(OnActiveModeDone, this);
-    mICDMode          = ICDMode::SIT;
+    mIcdMode          = ICDMode::SIT;
     mOperationalState = OperationalState::IdleMode;
-    mStorage          = nullptr;
-    mFabricTable      = nullptr;
 }
 
 bool ICDManager::SupportsCheckInProtocol()
 {
     bool success;
     uint32_t featureMap;
     success = (Attributes::FeatureMap::Get(kRootEndpointId, &featureMap) == EMBER_ZCL_STATUS_SUCCESS);
+
     return success ? ((featureMap & to_underlying(Feature::kCheckInProtocolSupport)) != 0) : false;
 }
 
@@ -76,25 +68,25 @@ void ICDManager::UpdateIcdMode()
 
     ICDMode tempMode = ICDMode::SIT;
 
+    // TODO ICD LIT FIX DEPENDENCY ISSUE with app/util/IcdMonitoringTable.h and app/server:server
     // The Check In Protocol Feature is required and the slow polling interval shall also be greater than 15 seconds
     // to run an ICD in LIT mode.
-    if (GetSlowPollingInterval() > GetSITPollingThreshold() && SupportsCheckInProtocol())
-    {
-        VerifyOrDie(mStorage != nullptr);
-        VerifyOrDie(mFabricTable != nullptr);
-        // We can only get to LIT Mode, if at least one client is registered with the ICD device
-        for (const auto & fabricInfo : *mFabricTable)
-        {
-            // We only need 1 valid entry to ensure LIT compliance
-            IcdMonitoringTable table(*mStorage, fabricInfo.GetFabricIndex(), 1 /*Table entry limit*/);
-            if (!table.IsEmpty())
-            {
-                tempMode = ICDMode::LIT;
-                break;
-            }
-        }
-    }
-    mICDMode = tempMode;
+    // if (kSlowPollingInterval > kICDSitModePollingThreashold && SupportsCheckInProtocol())
+    // {
+    //     // We can only get to LIT Mode, if at least one client is registered to the ICD device
+    //     const auto & fabricTable = Server::GetInstance().GetFabricTable();
+    //     for (const auto & fabricInfo : fabricTable)
+    //     {
+    //         PersistentStorageDelegate & storage = Server::GetInstance().GetPersistentStorage();
+    //         IcdMonitoringTable table(storage, fabricInfo.GetFabricIndex(), 1);
+    //         if (!table.IsEmpty())
+    //         {
+    //             tempMode = ICDMode::LIT;
+    //             break;
+    //         }
+    //     }
+    // }
+    mIcdMode = tempMode;
 }
 
 void ICDManager::UpdateOperationState(OperationalState state)
@@ -113,17 +105,7 @@ void ICDManager::UpdateOperationState(OperationalState state)
         }
         DeviceLayer::SystemLayer().StartTimer(System::Clock::Timeout(idleModeInterval), OnIdleModeDone, this);
 
-        System::Clock::Milliseconds32 slowPollInterval = GetSlowPollingInterval();
-        // When in SIT mode, the slow poll interval SHOULDN'T be greater than the SIT mode polling threshold, per spec.
-        if (mICDMode == ICDMode::SIT && slowPollInterval > GetSITPollingThreshold())
-        {
-            ChipLogDetail(AppServer, "The Slow Polling Interval of an ICD in SIT mode should be <= %" PRIu32,
-                          (GetSITPollingThreshold().count() / 1000));
-            // TODO Spec to define this conformance as a SHALL
-            // slowPollInterval = GetSITPollingThreshold();
-        }
-
-        CHIP_ERROR err = DeviceLayer::ConnectivityMgr().SetPollingInterval(slowPollInterval);
+        CHIP_ERROR err = DeviceLayer::ConnectivityMgr().SetPollingInterval(GetSlowPollingInterval());
         if (err != CHIP_NO_ERROR)
         {
             ChipLogError(AppServer, "Failed to set Polling Interval: err %" CHIP_ERROR_FORMAT, err.Format());

src/app/icd/ICDManager.h

@@ -16,7 +16,6 @@
  */
 #pragma once
 
-#include <credentials/FabricTable.h>
 #include <lib/support/BitFlags.h>
 #include <platform/CHIPDeviceConfig.h>
 #include <platform/internal/CHIPDeviceLayerInternal.h>
@@ -51,16 +50,15 @@ class ICDManager
     };
 
     ICDManager() {}
-    void Init(PersistentStorageDelegate * storage, FabricTable * fabricTable);
+    void Init();
     void Shutdown();
     void UpdateIcdMode();
     void UpdateOperationState(OperationalState state);
     void SetKeepActiveModeRequirements(KeepActiveFlags flag, bool state);
     bool IsKeepActive() { return mKeepActiveFlags.HasAny(); }
-    ICDMode GetICDMode() { return mICDMode; }
+    ICDMode GetIcdMode() { return mIcdMode; }
     OperationalState GetOperationalState() { return mOperationalState; }
 
-    static System::Clock::Milliseconds32 GetSITPollingThreshold() { return kSITPollingThreshold; }
     static System::Clock::Milliseconds32 GetSlowPollingInterval() { return kSlowPollingInterval; }
     static System::Clock::Milliseconds32 GetFastPollingInterval() { return kFastPollingInterval; }
 
@@ -69,10 +67,9 @@ class ICDManager
     static void OnActiveModeDone(System::Layer * aLayer, void * appState);
 
 private:
-    // SIT ICDs should have a SlowPollingThreshold shorter than or equal to 15s (spec 9.16.1.5)
-    static constexpr System::Clock::Milliseconds32 kSITPollingThreshold = System::Clock::Milliseconds32(15000);
-    static constexpr System::Clock::Milliseconds32 kSlowPollingInterval = CHIP_DEVICE_CONFIG_ICD_SLOW_POLL_INTERVAL;
-    static constexpr System::Clock::Milliseconds32 kFastPollingInterval = CHIP_DEVICE_CONFIG_ICD_FAST_POLL_INTERVAL;
+    static constexpr System::Clock::Milliseconds32 kICDSitModePollingThreashold = System::Clock::Milliseconds32(15000);
+    static constexpr System::Clock::Milliseconds32 kSlowPollingInterval         = CHIP_DEVICE_CONFIG_ICD_SLOW_POLL_INTERVAL;
+    static constexpr System::Clock::Milliseconds32 kFastPollingInterval         = CHIP_DEVICE_CONFIG_ICD_FAST_POLL_INTERVAL;
 
     // Minimal constraint value of the the ICD attributes.
     static constexpr uint32_t kMinIdleModeInterval    = 500;
@@ -82,10 +79,8 @@ class ICDManager
     bool SupportsCheckInProtocol();
 
     BitFlags<KeepActiveFlags> mKeepActiveFlags{ 0 };
-    OperationalState mOperationalState   = OperationalState::IdleMode;
-    ICDMode mICDMode                     = ICDMode::SIT;
-    PersistentStorageDelegate * mStorage = nullptr;
-    FabricTable * mFabricTable           = nullptr;
+    OperationalState mOperationalState = OperationalState::IdleMode;
+    ICDMode mIcdMode                   = ICDMode::SIT;
 };
 
 } // namespace app

src/app/server/Server.cpp

@@ -249,7 +249,7 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
 #endif // CHIP_CONFIG_ENABLE_SERVER_IM_EVENT
 
 #if CHIP_CONFIG_ENABLE_ICD_SERVER
-    mICDManager.Init(mDeviceStorage, &GetFabricTable());
+    mICDManager.Init();
     mICDEventManager.Init(&mICDManager);
 #endif // CHIP_CONFIG_ENABLE_ICD_SERVER
 

src/app/tests/BUILD.gn

@@ -54,6 +54,18 @@ source_set("binding-test-srcs") {
   ]
 }
 
+source_set("icd-management-test-srcs") {
+  sources = [
+    "${chip_root}/src/app/util/IcdMonitoringTable.cpp",
+    "${chip_root}/src/app/util/IcdMonitoringTable.h",
+  ]
+
+  public_deps = [
+    "${chip_root}/src/app/common:cluster-objects",
+    "${chip_root}/src/lib/core",
+  ]
+}
+
 source_set("ota-requestor-test-srcs") {
   sources = [
     "${chip_root}/src/app/clusters/ota-requestor/DefaultOTARequestorStorage.cpp",
@@ -170,14 +182,14 @@ chip_test_suite("tests") {
 
   public_deps = [
     ":binding-test-srcs",
+    ":icd-management-test-srcs",
     ":operational-state-test-srcs",
     ":ota-requestor-test-srcs",
     ":scenes-table-test-srcs",
     ":time-sync-data-provider-test-srcs",
     "${chip_root}/src/app",
     "${chip_root}/src/app/common:cluster-objects",
     "${chip_root}/src/app/icd:manager-srcs",
-    "${chip_root}/src/app/icd:monitoring-table",
     "${chip_root}/src/app/tests:helpers",
     "${chip_root}/src/app/util/mock:mock_ember",
     "${chip_root}/src/lib/core",

src/app/tests/TestIcdMonitoringTable.cpp

@@ -15,7 +15,7 @@
  *    limitations under the License.
  */
 
-#include <app/icd/IcdMonitoringTable.h>
+#include <app/util/IcdMonitoringTable.h>
 #include <lib/core/CHIPError.h>
 #include <lib/support/DefaultStorageKeyAllocator.h>
 #include <lib/support/TestPersistentStorageDelegate.h>