[ICD] Make optional spec checks mandatory with the launch of LongIdleTime ICDs (#35956) (#35960)

mkardous-silabs · web-flow · commit 8e0b710c7804 · 2024-10-08T12:58:22.000Z
* Remove optional spec check

* Add static_assert for the slow poll config for SIT ICDs

* Add missing include

* fix restyler error

* fix spacing

* Remove client build flag since it is not used
diff --git a/config/esp32/components/chip/CMakeLists.txt b/config/esp32/components/chip/CMakeLists.txt
@@ -143,9 +143,6 @@ endif()
 
 if(CONFIG_ENABLE_ICD_SERVER)
     chip_gn_arg_append("chip_enable_icd_server"               "true")
-    if(CONFIG_ICD_ENFORCE_SIT_SLOW_POLL_LIMIT)
-        chip_gn_arg_append("icd_enforce_sit_slow_poll_limit"               "true")
-    endif()
     if(CONFIG_ICD_REPORT_ON_ACTIVE_MODE)
         chip_gn_arg_append("chip_icd_report_on_active_mode"               "true")
     endif()
diff --git a/config/esp32/components/chip/Kconfig b/config/esp32/components/chip/Kconfig
@@ -32,7 +32,7 @@ menu "CHIP Core"
             default 8
             help
                 The maximum number of simultaneously active CHIP exchange contexts.
-
+                                           
                 An exchange context object is used to track the state of an ongoing CHIP message
                 exchange (conversation) with a peer, e.g. a cloud service, a mobile application, or
                 another device.
@@ -410,13 +410,6 @@ menu "CHIP Device Layer"
             help
                 Enables or Disables ICD server
 
-        config ICD_ENFORCE_SIT_SLOW_POLL_LIMIT
-            bool "Enforce SIT Slow Polling Max value to 15 seconds"
-            depends on ENABLE_ICD_SERVER
-            default n
-            help
-                Set to true to enforce SIT Slow Polling Max value to 15seconds
-
         config ICD_REPORT_ON_ACTIVE_MODE
             bool "Emit a report on entering active mode"
             depends on ENABLE_ICD_SERVER
diff --git a/config/nrfconnect/chip-module/CMakeLists.txt b/config/nrfconnect/chip-module/CMakeLists.txt
@@ -156,7 +156,6 @@ if (CONFIG_CHIP_ENABLE_ICD_SUPPORT)
     matter_add_gn_arg_bool  ("chip_enable_icd_checkin"                   CONFIG_CHIP_ICD_CHECK_IN_SUPPORT)
     matter_add_gn_arg_bool  ("chip_enable_icd_user_active_mode_trigger"  CONFIG_CHIP_ICD_UAT_SUPPORT)
     matter_add_gn_arg_bool  ("chip_enable_icd_dsls"                      CONFIG_CHIP_ICD_DSLS_SUPPORT)
-    matter_add_gn_arg_bool  ("icd_enforce_sit_slow_poll_limit"           TRUE)
 endif()
 
 if (CONFIG_CHIP_FACTORY_DATA OR CONFIG_CHIP_FACTORY_DATA_CUSTOM_BACKEND)
diff --git a/docs/guides/ti/matter-users-guide/enabling_icd_on_ti_devices.md b/docs/guides/ti/matter-users-guide/enabling_icd_on_ti_devices.md
@@ -24,9 +24,6 @@ Trigger Support, set the following parameter to true:
 chip_enable_icd_lit = true
 ```
 
-TI examples have only been tested with the ICD Server configuration. To enable
-the client configuration, set `chip_enable_icd_client` to true.
-
 Persistent subscriptions allow devices to attempt resuming existing
 subscriptions following a device reset. To enable persistent subscriptions, set
 the following parameter to true:
diff --git a/examples/contact-sensor-app/nxp/k32w0/args.gni b/examples/contact-sensor-app/nxp/k32w0/args.gni
@@ -28,7 +28,6 @@ chip_generate_link_map_file = true
 
 chip_enable_icd_server = true
 chip_enable_icd_lit = false
-icd_enforce_sit_slow_poll_limit = true
 chip_persist_subscriptions = true
 chip_subscription_timeout_resumption = true
 
diff --git a/examples/contact-sensor-app/nxp/k32w1/args.gni b/examples/contact-sensor-app/nxp/k32w1/args.gni
@@ -32,7 +32,6 @@ chip_with_lwip = false
 chip_enable_icd_server = true
 chip_enable_icd_lit = false
 chip_enable_icd_dsls = false
-icd_enforce_sit_slow_poll_limit = true
 chip_persist_subscriptions = true
 chip_subscription_timeout_resumption = true
 
diff --git a/examples/contact-sensor-app/nxp/mcxw71/args.gni b/examples/contact-sensor-app/nxp/mcxw71/args.gni
@@ -30,7 +30,6 @@ chip_with_lwip = false
 
 chip_enable_icd_server = true
 chip_enable_icd_lit = false
-icd_enforce_sit_slow_poll_limit = true
 chip_persist_subscriptions = true
 chip_subscription_timeout_resumption = true
 
diff --git a/examples/lit-icd-app/silabs/build_for_wifi_args.gni b/examples/lit-icd-app/silabs/build_for_wifi_args.gni
@@ -29,7 +29,6 @@ sl_enable_test_event_trigger = true
 chip_enable_icd_server = true
 chip_subscription_timeout_resumption = false
 sl_use_subscription_syncing = true
-icd_enforce_sit_slow_poll_limit = true
 chip_enable_icd_lit = true
 
 # ICD Matter Configuration flags
diff --git a/examples/lit-icd-app/silabs/openthread.gni b/examples/lit-icd-app/silabs/openthread.gni
@@ -32,7 +32,6 @@ sl_enable_test_event_trigger = true
 chip_enable_icd_server = true
 chip_subscription_timeout_resumption = false
 sl_use_subscription_syncing = true
-icd_enforce_sit_slow_poll_limit = true
 chip_icd_report_on_active_mode = true
 chip_enable_icd_lit = true
 
diff --git a/examples/lock-app/nxp/k32w1/args.gni b/examples/lock-app/nxp/k32w1/args.gni
@@ -30,7 +30,6 @@ chip_with_lwip = false
 
 chip_enable_icd_server = true
 chip_enable_icd_lit = false
-icd_enforce_sit_slow_poll_limit = true
 chip_persist_subscriptions = true
 chip_subscription_timeout_resumption = true
 
diff --git a/examples/lock-app/nxp/mcxw71/args.gni b/examples/lock-app/nxp/mcxw71/args.gni
@@ -30,7 +30,6 @@ chip_with_lwip = false
 
 chip_enable_icd_server = true
 chip_enable_icd_lit = false
-icd_enforce_sit_slow_poll_limit = true
 chip_persist_subscriptions = true
 chip_subscription_timeout_resumption = true
 
diff --git a/examples/smoke-co-alarm-app/silabs/build_for_wifi_args.gni b/examples/smoke-co-alarm-app/silabs/build_for_wifi_args.gni
@@ -28,7 +28,6 @@ sl_enable_test_event_trigger = true
 chip_enable_icd_server = true
 chip_subscription_timeout_resumption = false
 sl_use_subscription_syncing = true
-icd_enforce_sit_slow_poll_limit = true
 chip_enable_icd_lit = true
 
 # ICD Matter Configuration flags
diff --git a/examples/smoke-co-alarm-app/silabs/openthread.gni b/examples/smoke-co-alarm-app/silabs/openthread.gni
@@ -32,7 +32,6 @@ sl_enable_test_event_trigger = true
 chip_enable_icd_server = true
 chip_subscription_timeout_resumption = false
 sl_use_subscription_syncing = true
-icd_enforce_sit_slow_poll_limit = true
 chip_icd_report_on_active_mode = true
 chip_enable_icd_lit = true
 
diff --git a/src/app/icd/icd.gni b/src/app/icd/icd.gni
@@ -14,23 +14,15 @@
 
 declare_args() {
   # Matter SDK Configuration flag to enable ICD server functionality
-  # TODO - Add Specifics when the design is refined
   chip_enable_icd_server = false
 
   chip_enable_icd_lit = false
 
-  # Matter SDK Configuration flag to enable ICD client functionality
-  # TODO - Add Specifics when the design is refined
-  chip_enable_icd_client = false
-
   # Matter SDK Configuration flag to make the ICD manager emit a report on entering active mode
   chip_icd_report_on_active_mode = false
 
   icd_max_notification_subscribers = 1
 
-  # Set to true to enforce SIT Slow Polling Max value to 15seconds (spec 9.16.1.5)
-  icd_enforce_sit_slow_poll_limit = false
-
   # Set to true if device supports dynamic switching from SIT to LIT operating modes (DSLS)
   chip_enable_icd_dsls = false
 }
diff --git a/src/app/icd/server/BUILD.gn b/src/app/icd/server/BUILD.gn
@@ -39,7 +39,6 @@ buildconfig_header("icd-server-buildconfig") {
     "CHIP_CONFIG_ENABLE_ICD_DSLS=${chip_enable_icd_dsls}",
     "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}",
   ]
 
   visibility = [ ":icd-server-config" ]
diff --git a/src/app/icd/server/ICDConfigurationData.cpp b/src/app/icd/server/ICDConfigurationData.cpp
@@ -16,7 +16,6 @@
  */
 
 #include "ICDConfigurationData.h"
-#include <app/icd/server/ICDServerConfig.h>
 #include <lib/support/CodeUtils.h>
 
 namespace chip {
@@ -25,15 +24,16 @@ ICDConfigurationData ICDConfigurationData::instance;
 
 System::Clock::Milliseconds32 ICDConfigurationData::GetSlowPollingInterval()
 {
-#if ICD_ENFORCE_SIT_SLOW_POLL_LIMIT
-    // When in SIT mode, the slow poll interval SHOULDN'T be greater than the SIT mode polling threshold, per spec.
+#if CHIP_CONFIG_ENABLE_ICD_LIT
+    // When in SIT mode, the slow poll interval SHALL NOT be greater than the SIT mode polling threshold, per spec.
     // This is important for ICD device configured for LIT operation but currently operating as a SIT
     // due to a lack of client registration
     if (mICDMode == ICDMode::SIT && mSlowPollingInterval > kSITPollingThreshold)
     {
         return kSITPollingThreshold;
     }
-#endif
+#endif // CHIP_CONFIG_ENABLE_ICD_LIT
+
     return mSlowPollingInterval;
 }
 
diff --git a/src/app/icd/server/ICDConfigurationData.h b/src/app/icd/server/ICDConfigurationData.h
@@ -17,6 +17,7 @@
 
 #pragma once
 
+#include <app/icd/server/ICDServerConfig.h>
 #include <lib/core/Optional.h>
 #include <lib/support/TimeUtils.h>
 #include <platform/CHIPDeviceConfig.h>
@@ -77,14 +78,11 @@ class ICDConfigurationData
     System::Clock::Seconds32 GetMaximumCheckInBackoff() { return mMaximumCheckInBackOff; }
 
     /**
-     * If ICD_ENFORCE_SIT_SLOW_POLL_LIMIT is set to 0, function will always return the configured Slow Polling interval
-     * (CHIP_DEVICE_CONFIG_ICD_SLOW_POLL_INTERVAL).
-     *
-     * If ICD_ENFORCE_SIT_SLOW_POLL_LIMIT is set to 1, the returned value will depend on the devices operating mode.
+     * The returned value will depend on the devices operating mode.
      * If ICDMode == SIT && the configured slow poll interval is superior to the maximum threshold (15s), the function will return
-     * the threshold (15s). If ICDMode == SIT but the configured slow poll interval is equal or inferior to the threshold, the
-     * function will the return the configured slow poll interval. If ICDMode == LIT, the function will return the configured slow
-     * poll interval.
+     * the threshold kSITPollingThreshold (<= 15s). If ICDMode == SIT but the configured slow poll interval is equal or inferior to
+     * the threshold, the function will the return the configured slow poll interval. If ICDMode == LIT, the function will return
+     * the configured slow poll interval.
      *
      * @return System::Clock::Milliseconds32
      */
@@ -158,12 +156,18 @@ class ICDConfigurationData
                   "Spec requires the MaximumCheckInBackOff to be equal or superior to the IdleModeDuration");
     System::Clock::Seconds32 mMaximumCheckInBackOff = System::Clock::Seconds32(CHIP_CONFIG_ICD_MAXIMUM_CHECK_IN_BACKOFF_SEC);
 
-    // SIT ICDs should have a SlowPollingThreshold shorter than or equal to 15s (spec 9.16.1.5)
-    static_assert((CHIP_DEVICE_CONFIG_ICD_SIT_SLOW_POLL_LIMIT).count() <= 15000,
+    // SIT ICDs SHALL have a SlowPollingThreshold shorter than or equal to 15s (spec 9.16.1.5)
+    static constexpr System::Clock::Milliseconds32 kSitIcdSlowPollMaximum = System::Clock::Milliseconds32(15000);
+    static_assert((CHIP_DEVICE_CONFIG_ICD_SIT_SLOW_POLL_LIMIT).count() <= kSitIcdSlowPollMaximum.count(),
                   "Spec requires the maximum slow poll interval for the SIT device to be smaller or equal than 15 s.");
     static constexpr System::Clock::Milliseconds32 kSITPollingThreshold = CHIP_DEVICE_CONFIG_ICD_SIT_SLOW_POLL_LIMIT;
-    System::Clock::Milliseconds32 mSlowPollingInterval                  = CHIP_DEVICE_CONFIG_ICD_SLOW_POLL_INTERVAL;
-    System::Clock::Milliseconds32 mFastPollingInterval                  = CHIP_DEVICE_CONFIG_ICD_FAST_POLL_INTERVAL;
+
+#if CHIP_CONFIG_ENABLE_ICD_LIT == 0
+    static_assert((CHIP_DEVICE_CONFIG_ICD_SLOW_POLL_INTERVAL <= kSitIcdSlowPollMaximum),
+                  "LIT support is required for slow polling intervals superior to 15 seconds");
+#endif
+    System::Clock::Milliseconds32 mSlowPollingInterval = CHIP_DEVICE_CONFIG_ICD_SLOW_POLL_INTERVAL;
+    System::Clock::Milliseconds32 mFastPollingInterval = CHIP_DEVICE_CONFIG_ICD_FAST_POLL_INTERVAL;
 
     ICDMode mICDMode = ICDMode::SIT;
 };
diff --git a/src/app/icd/server/ICDManager.cpp b/src/app/icd/server/ICDManager.cpp
@@ -80,9 +80,6 @@ void ICDManager::Init()
         VerifyOrDieWithMsg(ICDConfigurationData::GetInstance().GetMinLitActiveModeThreshold() <=
                                ICDConfigurationData::GetInstance().GetActiveModeThreshold(),
                            AppServer, "The minimum ActiveModeThreshold value for a LIT ICD is 5 seconds.");
-        // Disabling check until LIT support is compelte
-        // VerifyOrDieWithMsg((GetSlowPollingInterval() <= GetSITPollingThreshold()) , AppServer,
-        //                    "LIT support is required for slow polling intervals superior to 15 seconds");
     }
 #endif // CHIP_CONFIG_ENABLE_ICD_LIT
 

Original file line number	Diff line number	Diff line change
`@@ -39,7 +39,6 @@ buildconfig_header("icd-server-buildconfig") {`
`39`	`39`	`"CHIP_CONFIG_ENABLE_ICD_DSLS=${chip_enable_icd_dsls}",`
`40`	`40`	`"ICD_REPORT_ON_ENTER_ACTIVE_MODE=${chip_icd_report_on_active_mode}",`
`41`	`41`	`"ICD_MAX_NOTIFICATION_SUBSCRIBERS=${icd_max_notification_subscribers}",`
`42`		`- "ICD_ENFORCE_SIT_SLOW_POLL_LIMIT=${icd_enforce_sit_slow_poll_limit}",`
`43`	`42`	`]`
`44`	`43`
`45`	`44`	`visibility = [ ":icd-server-config" ]`
Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,6 @@`
`16`	`16`	`*/`
`17`	`17`
`18`	`18`	`#include "ICDConfigurationData.h"`
`19`		`-#include <app/icd/server/ICDServerConfig.h>`
`20`	`19`	`#include <lib/support/CodeUtils.h>`
`21`	`20`
`22`	`21`	`namespace chip {`
`@@ -25,15 +24,16 @@ ICDConfigurationData ICDConfigurationData::instance;`
`25`	`24`
`26`	`25`	`System::Clock::Milliseconds32 ICDConfigurationData::GetSlowPollingInterval()`
`27`	`26`	`{`
`28`		`-#if ICD_ENFORCE_SIT_SLOW_POLL_LIMIT`
`29`		`- // When in SIT mode, the slow poll interval SHOULDN'T be greater than the SIT mode polling threshold, per spec.`
	`27`	`+#if CHIP_CONFIG_ENABLE_ICD_LIT`
	`28`	`+ // When in SIT mode, the slow poll interval SHALL NOT be greater than the SIT mode polling threshold, per spec.`
`30`	`29`	`// This is important for ICD device configured for LIT operation but currently operating as a SIT`
`31`	`30`	`// due to a lack of client registration`
`32`	`31`	`if (mICDMode == ICDMode::SIT && mSlowPollingInterval > kSITPollingThreshold)`
`33`	`32`	`{`
`34`	`33`	`return kSITPollingThreshold;`
`35`	`34`	`}`
`36`		`-#endif`
	`35`	`+#endif // CHIP_CONFIG_ENABLE_ICD_LIT`
	`36`	`+`
`37`	`37`	`return mSlowPollingInterval;`
`38`	`38`	`}`
`39`	`39`
Original file line number	Diff line number	Diff line change
`@@ -80,9 +80,6 @@ void ICDManager::Init()`
`80`	`80`	`VerifyOrDieWithMsg(ICDConfigurationData::GetInstance().GetMinLitActiveModeThreshold() <=`
`81`	`81`	`ICDConfigurationData::GetInstance().GetActiveModeThreshold(),`
`82`	`82`	`AppServer, "The minimum ActiveModeThreshold value for a LIT ICD is 5 seconds.");`
`83`		`- // Disabling check until LIT support is compelte`
`84`		`- // VerifyOrDieWithMsg((GetSlowPollingInterval() <= GetSITPollingThreshold()) , AppServer,`
`85`		`- // "LIT support is required for slow polling intervals superior to 15 seconds");`
`86`	`83`	`}`
`87`	`84`	`#endif // CHIP_CONFIG_ENABLE_ICD_LIT`
`88`	`85`