[ICD] Change IdleModeInterval Type from milliseconds to seconds (#28804)

* change idle mode interval type to seconds * generated files * Update silabs configurations * Fix unit tests with type change * Address review comments * restyle
project-chip · Nov 15, 2023 · 5b4024f · 5b4024f
1 parent 528f3d8
commit 5b4024f
Show file tree

Hide file tree

Showing 17 changed files with 137 additions and 396 deletions.
diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml
@@ -418,6 +418,7 @@ jobs:
                   scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --app out/linux-x64-all-clusters-ipv6only-no-ble-no-wifi-tsan-clang-test/chip-all-clusters-app  --factoryreset --app-args "--discriminator 1234 --KVS kvs1 --trace-to json:out/trace_data/app-{SCRIPT_BASE_NAME}.json" --script "src/python_testing/TC_ACE_1_3.py" --script-args "--storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --trace-to json:out/trace_data/test-{SCRIPT_BASE_NAME}.json --trace-to perfetto:out/trace_data/test-{SCRIPT_BASE_NAME}.perfetto"'
                   scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --app out/linux-x64-all-clusters-ipv6only-no-ble-no-wifi-tsan-clang-test/chip-all-clusters-app  --factoryreset --app-args "--discriminator 1234 --KVS kvs1 --trace-to json:out/trace_data/app-{SCRIPT_BASE_NAME}.json" --script "src/python_testing/TC_CGEN_2_4.py" --script-args "--storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --trace-to json:out/trace_data/test-{SCRIPT_BASE_NAME}.json --trace-to perfetto:out/trace_data/test-{SCRIPT_BASE_NAME}.perfetto"'
                   scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --app out/linux-x64-all-clusters-ipv6only-no-ble-no-wifi-tsan-clang-test/chip-all-clusters-app  --factoryreset --app-args "--discriminator 1234 --KVS kvs1 --trace-to json:out/trace_data/app-{SCRIPT_BASE_NAME}.json" --script "src/python_testing/TC_DA_1_2.py" --script-args "--storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --PICS src/app/tests/suites/certification/ci-pics-values --trace-to json:out/trace_data/test-{SCRIPT_BASE_NAME}.json --trace-to perfetto:out/trace_data/test-{SCRIPT_BASE_NAME}.perfetto"'
+                  scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --app out/linux-x64-all-clusters-ipv6only-no-ble-no-wifi-tsan-clang-test/chip-all-clusters-app  --factoryreset --app-args "--discriminator 1234 --KVS kvs1 --trace-to json:out/trace_data/app-{SCRIPT_BASE_NAME}.json" --script "src/python_testing/TC_ICDM_2_1.py" --script-args "--storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --PICS src/app/tests/suites/certification/ci-pics-values --trace-to json:out/trace_data/test-{SCRIPT_BASE_NAME}.json --trace-to perfetto:out/trace_data/test-{SCRIPT_BASE_NAME}.perfetto"'
                   scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --app out/linux-x64-all-clusters-ipv6only-no-ble-no-wifi-tsan-clang-test/chip-all-clusters-app  --factoryreset --app-args "--discriminator 1234 --KVS kvs1 --trace-to json:out/trace_data/app-{SCRIPT_BASE_NAME}.json" --script "src/python_testing/TC_TIMESYNC_3_1.py" --script-args "--storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --trace-to json:out/trace_data/test-{SCRIPT_BASE_NAME}.json --trace-to perfetto:out/trace_data/test-{SCRIPT_BASE_NAME}.perfetto"'
                   scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --app out/linux-x64-all-clusters-ipv6only-no-ble-no-wifi-tsan-clang-test/chip-all-clusters-app  --factoryreset --app-args "--discriminator 1234 --KVS kvs1 --trace-to json:out/trace_data/app-{SCRIPT_BASE_NAME}.json" --script "src/python_testing/TC_DA_1_5.py" --script-args "--storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --PICS src/app/tests/suites/certification/ci-pics-values --trace-to json:out/trace_data/test-{SCRIPT_BASE_NAME}.json --trace-to perfetto:out/trace_data/test-{SCRIPT_BASE_NAME}.perfetto"'
                   scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --app out/linux-x64-all-clusters-ipv6only-no-ble-no-wifi-tsan-clang-test/chip-all-clusters-app  --factoryreset --app-args "--discriminator 1234 --KVS kvs1 --trace-to json:out/trace_data/app-{SCRIPT_BASE_NAME}.json" --script "src/python_testing/TC_IDM_1_2.py" --script-args "--storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --trace-to json:out/trace_data/test-{SCRIPT_BASE_NAME}.json --trace-to perfetto:out/trace_data/test-{SCRIPT_BASE_NAME}.perfetto"'

diff --git a/examples/light-switch-app/silabs/openthread.gni b/examples/light-switch-app/silabs/openthread.gni
@@ -35,6 +35,6 @@ sl_ot_idle_interval_ms = 30000  # 30s Idle Intervals
 sl_ot_active_interval_ms = 500  # 500ms Active Intervals
 
 # ICD Matter Configuration flags
-sl_idle_mode_interval_ms = 3600000  # 60min Idle Mode Interval
+sl_idle_mode_interval_s = 3600  # 60min Idle Mode Interval
 sl_active_mode_interval_ms = 60000  # 60s Active Mode Interval
 sl_active_mode_threshold_ms = 1000  # 1s Active Mode Threshold
diff --git a/examples/lock-app/silabs/openthread.gni b/examples/lock-app/silabs/openthread.gni
@@ -35,6 +35,6 @@ sl_ot_idle_interval_ms = 5000  # 5s Idle Intervals
 sl_ot_active_interval_ms = 500  # 500ms Active Intervals
 
 # ICD Matter Configuration flags
-sl_idle_mode_interval_ms = 600000  # 10min Idle Mode Interval
+sl_idle_mode_interval_s = 600  # 10min Idle Mode Interval
 sl_active_mode_interval_ms = 10000  # 10s Active Mode Interval
 sl_active_mode_threshold_ms = 1000  # 1s Active Mode Threshold
diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp
@@ -45,7 +45,7 @@ using Status = Protocols::InteractionModel::Status;
 uint16_t ReadHandler::GetPublisherSelectedIntervalLimit()
 {
 #if CHIP_CONFIG_ENABLE_ICD_SERVER
-    return static_cast<uint16_t>(IcdManagementServer::GetInstance().GetIdleModeInterval() / 1000);
+    return static_cast<uint16_t>(IcdManagementServer::GetInstance().GetIdleModeInterval());
 #else
     return kSubscriptionMaxIntervalPublisherLimit;
 #endif

diff --git a/src/app/icd/ICDManager.cpp b/src/app/icd/ICDManager.cpp
@@ -129,7 +129,7 @@ void ICDManager::UpdateOperationState(OperationalState state)
     {
         mOperationalState         = OperationalState::IdleMode;
         uint32_t idleModeInterval = IcdManagementServer::GetInstance().GetIdleModeInterval();
-        DeviceLayer::SystemLayer().StartTimer(System::Clock::Timeout(idleModeInterval), OnIdleModeDone, this);
+        DeviceLayer::SystemLayer().StartTimer(System::Clock::Seconds32(idleModeInterval), OnIdleModeDone, this);
 
         System::Clock::Milliseconds32 slowPollInterval = GetSlowPollingInterval();
 

diff --git a/src/app/icd/IcdManagementServer.h b/src/app/icd/IcdManagementServer.h
@@ -66,10 +66,26 @@ class IcdManagementServer
     IcdManagementServer() = default;
 
     static IcdManagementServer mInstance;
-    uint32_t mIdleInterval           = CHIP_CONFIG_ICD_IDLE_MODE_INTERVAL;
-    uint32_t mActiveInterval         = CHIP_CONFIG_ICD_ACTIVE_MODE_INTERVAL;
-    uint16_t mActiveThreshold        = CHIP_CONFIG_ICD_ACTIVE_MODE_THRESHOLD;
-    uint32_t mIcdCounter             = 0;
+
+    static_assert((CHIP_CONFIG_ICD_IDLE_MODE_INTERVAL) <= 64800,
+                  "Spec requires the IdleModeInterval to be equal or inferior to 64800s.");
+    static_assert((CHIP_CONFIG_ICD_IDLE_MODE_INTERVAL) >= 1, "Spec requires the IdleModeInterval to be equal or greater to 1s.");
+    uint32_t mIdleInterval = CHIP_CONFIG_ICD_IDLE_MODE_INTERVAL; // in seconds.
+
+    static_assert((CHIP_CONFIG_ICD_ACTIVE_MODE_INTERVAL) <= (CHIP_CONFIG_ICD_IDLE_MODE_INTERVAL * 1000),
+                  "Spec requires the IdleModeInterval be equal or greater to the ActiveModeInterval.");
+    static_assert((CHIP_CONFIG_ICD_ACTIVE_MODE_INTERVAL) >= 300,
+                  "Spec requires the ActiveModeThreshold to be equal or greater to 300ms");
+    uint32_t mActiveInterval = CHIP_CONFIG_ICD_ACTIVE_MODE_INTERVAL; // in milliseconds
+
+    static_assert((CHIP_CONFIG_ICD_ACTIVE_MODE_THRESHOLD) >= 300,
+                  "Spec requires the ActiveModeThreshold to be equal or greater to 300ms.");
+    uint16_t mActiveThreshold = CHIP_CONFIG_ICD_ACTIVE_MODE_THRESHOLD; // in milliseconds
+
+    uint32_t mIcdCounter = 0;
+
+    static_assert((CHIP_CONFIG_ICD_CLIENTS_SUPPORTED_PER_FABRIC) >= 1,
+                  "Spec requires the minimum of supported clients per fabric be equal or greater to 1.");
     uint16_t mFabricClientsSupported = CHIP_CONFIG_ICD_CLIENTS_SUPPORTED_PER_FABRIC;
 };
 

diff --git a/src/app/tests/TestICDManager.cpp b/src/app/tests/TestICDManager.cpp
@@ -117,7 +117,7 @@ class TestICDManager
         AdvanceClockAndRunEventLoop(ctx, IcdManagementServer::GetInstance().GetActiveModeInterval() + 1);
         // Active mode interval expired, ICDManager transitioned to the IdleMode.
         NL_TEST_ASSERT(aSuite, ctx->mICDManager.mOperationalState == ICDManager::OperationalState::IdleMode);
-        AdvanceClockAndRunEventLoop(ctx, IcdManagementServer::GetInstance().GetIdleModeInterval() + 1);
+        AdvanceClockAndRunEventLoop(ctx, (IcdManagementServer::GetInstance().GetIdleModeInterval() * 1000) + 1);
         // Idle mode interval expired, ICDManager transitioned to the ActiveMode.
         NL_TEST_ASSERT(aSuite, ctx->mICDManager.mOperationalState == ICDManager::OperationalState::ActiveMode);
 

diff --git a/src/app/tests/suites/TestIcdManagementCluster.yaml b/src/app/tests/suites/TestIcdManagementCluster.yaml
@@ -32,7 +32,7 @@ tests:
       command: "readAttribute"
       attribute: "IdleModeInterval"
       response:
-          value: 2000
+          value: 2
 
     - label: "Read ActiveModeInterval"
       command: "readAttribute"

diff --git a/src/app/tests/suites/certification/Test_TC_ICDM_2_1.yaml b/src/app/tests/suites/certification/Test_TC_ICDM_2_1.yaml
diff --git a/src/app/tests/suites/certification/Test_TC_IDM_4_2.yaml b/src/app/tests/suites/certification/Test_TC_IDM_4_2.yaml
@@ -49,15 +49,15 @@ tests:
       verification: |
           TH reads from the DUT the IdleModeInterval attribute by sending below mentioned command
 
-           icdmanagement read idle-mode-interval 1
+           icdmanagement read idle-mode-interval 1 0
           Please note down the value of IdleModeInterval as SUBSCRIPTION_MAX_INTERVAL_PUBLISHER_LIMIT for use in the next step validations.
 
 
           [1690475024.398278][5675:5677] CHIP:DMG:         SuppressResponse = true,
           [1690475024.398282][5675:5677] CHIP:DMG:         InteractionModelRevision = 1
           [1690475024.398285][5675:5677] CHIP:DMG: }
           [1690475024.398735][5675:5677] CHIP:TOO: Endpoint: 0 Cluster: 0x0000_0046 Attribute 0x0000_0000 DataVersion: 4216223957
-          [1690475024.398921][5675:5677] CHIP:TOO:   IdleModeInterval: 500
+          [1690475024.398921][5675:5677] CHIP:TOO:   IdleModeInterval: 1
           [1690475024.399202][5675:5677] CHIP:EM: <<< [E:28320i S:44238 M:28503816 (Ack:266918637)] (S) Msg TX to 1:0000000000000001 [3C19] --- Type 0000:10 (SecureChannel:StandaloneAck)
           [1690475024.399208][5675:5677] CHIP:IN: (S) Sending msg 28503816 on secure session with LSID: 44238
           [1690475024.399217][5675:5677] CHIP:EM: Flushed pending ack for MessageCounter:266918637 on exchange 28320i

diff --git a/src/app/tests/suites/certification/ci-pics-values b/src/app/tests/suites/certification/ci-pics-values
@@ -2098,7 +2098,6 @@ BRBINFO.C.E02=1
 BRBINFO.C.E03=1
 
 # Interaction Data Model
-ICDM.S=1
 MCORE.IDM.C=1
 MCORE.IDM.C.InvokeRequest=1
 MCORE.IDM.C.ReadRequest=1
@@ -2592,6 +2591,9 @@ FAN.S.C00.Rsp=1
 
 #ICD Management
 
+#Server
+ICDM.S=1
+
 #Server Attribute
 ICDM.S.A0000=1
 ICDM.S.A0001=1
@@ -2715,4 +2717,4 @@ REFALM.S.A0002=1
 REFALM.S.A0003=1
 REFALM.S.E00=1
 REFALM.S.C00.Rsp=1
-REFALM.S.C01.Rsp=1
+REFALM.S.C01.Rsp=1
diff --git a/src/app/tests/suites/ciTests.json b/src/app/tests/suites/ciTests.json
@@ -74,7 +74,7 @@
     "GeneralCommissioning": ["Test_TC_CGEN_1_1", "Test_TC_CGEN_2_1"],
     "GeneralDiagnostics": ["Test_TC_DGGEN_1_1", "Test_TC_DGGEN_2_1"],
     "GroupKeyManagement": ["Test_TC_GRPKEY_1_1"],
-    "IcdManagement": ["Test_TC_ICDM_1_1", "Test_TC_ICDM_2_1"],
+    "IcdManagement": ["Test_TC_ICDM_1_1"],
     "Identify": [
         "Test_TC_I_1_1",
         "Test_TC_I_2_1",

diff --git a/src/app/zap-templates/zcl/data-model/chip/icd-management-cluster.xml b/src/app/zap-templates/zcl/data-model/chip/icd-management-cluster.xml
@@ -42,7 +42,7 @@ limitations under the License.
         <!-- Current cluster version -->
         <globalAttribute side="either" code="0xFFFD" value="1"/>
 
-        <attribute side="server" code="0x00" define="IDLE_MODE_INTERVAL" type="INT32U" min="500" max="64800000" default="500" writable="false" optional="false" isNullable="false">IdleModeInterval</attribute>
+        <attribute side="server" code="0x00" define="IDLE_MODE_INTERVAL" type="INT32U" min="1" max="64800" default="1" writable="false" optional="false" isNullable="false">IdleModeInterval</attribute>
         <attribute side="server" code="0x01" define="ACTIVE_MODE_INTERVAL" type="INT32U" min="300" default="300" writable="false" optional="false" isNullable="false">ActiveModeInterval</attribute>
         <attribute side="server" code="0x02" define="ACTIVE_MODE_THRESHOLD" type="INT16U" min="300" default="300" writable="false" optional="false" isNullable="false">ActiveModeThreshold</attribute>
         <attribute side="server" code="0x03" define="REGISTERED_CLIENTS" type="ARRAY" entryType="MonitoringRegistrationStruct" writable="false" optional="true" isNullable="false">

diff --git a/src/lib/core/CHIPConfig.h b/src/lib/core/CHIPConfig.h
@@ -1476,10 +1476,10 @@ extern const char CHIP_NON_PRODUCTION_MARKER[];
 /**
  * @def CHIP_CONFIG_ICD_IDLE_MODE_INTERVAL
  *
- * @brief Default value for the ICD Management cluster IdleModeInterval attribute, in milliseconds
+ * @brief Default value for the ICD Management cluster IdleModeInterval attribute, in seconds
  */
 #ifndef CHIP_CONFIG_ICD_IDLE_MODE_INTERVAL
-#define CHIP_CONFIG_ICD_IDLE_MODE_INTERVAL 2000
+#define CHIP_CONFIG_ICD_IDLE_MODE_INTERVAL 2
 #endif
 
 /**