[ICD] Make optional spec checks mandatory with the launch of LongIdle…

…Time ICDs (#35956) * 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
project-chip · Oct 8, 2024 · 9fc8386 · 9fc8386
1 parent 43e447c
commit 9fc8386
Show file tree

Hide file tree

Showing 18 changed files with 20 additions and 51 deletions.
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