[ICD] Add default ICD Behavior to readhandler when establishing a sub…

…scription (#28375) * add default ICD behavior to readhandler * Add comments * Update src/app/ReadHandler.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * move check to top * Add ICD unit tests for ReadHandler * check to avoid division by 0 * fix dependency chain * restyle * remove deps for manager srcs --------- Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
project-chip · Oct 27, 2023 · 1255400 · 1255400
1 parent a9e8fc5
commit 1255400
Show file tree

Hide file tree

Showing 8 changed files with 468 additions and 8 deletions.
diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp
@@ -713,6 +713,57 @@ CHIP_ERROR ReadHandler::ProcessSubscribeRequest(System::PacketBufferHandle && aP
     ReturnErrorOnFailure(subscribeRequestParser.GetMaxIntervalCeilingSeconds(&mMaxInterval));
     VerifyOrReturnError(mMinIntervalFloorSeconds <= mMaxInterval, CHIP_ERROR_INVALID_ARGUMENT);
 
+#if CHIP_CONFIG_ENABLE_ICD_SERVER
+
+    // Default behavior for ICDs where the wanted MaxInterval for a subscription is the IdleModeInterval
+    // defined in the ICD Management Cluster.
+    // Behavior can be changed with the OnSubscriptionRequested function defined in the application callbacks
+
+    // Default Behavior Steps :
+    // If MinInterval > IdleModeInterval, try to set the MaxInterval to the first interval of IdleModeIntervals above the
+    // MinInterval.
+    // If the next interval is greater than the MaxIntervalCeiling, use the MaxIntervalCeiling.
+    // Otherwise, use IdleModeInterval as MaxInterval
+
+    // GetPublisherSelectedIntervalLimit() returns the IdleModeInterval if the device is an ICD
+    uint32_t decidedMaxInterval = GetPublisherSelectedIntervalLimit();
+
+    // Check if the PublisherSelectedIntervalLimit is 0. If so, set decidedMaxInterval to MaxIntervalCeiling
+    if (decidedMaxInterval == 0)
+    {
+        decidedMaxInterval = mMaxInterval;
+    }
+
+    // If requestedMinInterval is greater than the IdleTimeInterval, select next active up time as max interval
+    if (mMinIntervalFloorSeconds > decidedMaxInterval)
+    {
+        uint16_t ratio = mMinIntervalFloorSeconds / static_cast<uint16_t>(decidedMaxInterval);
+        if (mMinIntervalFloorSeconds % decidedMaxInterval)
+        {
+            ratio++;
+        }
+
+        decidedMaxInterval *= ratio;
+    }
+
+    // Verify that decidedMaxInterval is an acceptable value (overflow)
+    if (decidedMaxInterval > System::Clock::Seconds16::max().count())
+    {
+        decidedMaxInterval = System::Clock::Seconds16::max().count();
+    }
+
+    // Verify that the decidedMaxInterval respects MAX(GetPublisherSelectedIntervalLimit(), MaxIntervalCeiling)
+    uint16_t maximumMaxInterval = std::max(GetPublisherSelectedIntervalLimit(), mMaxInterval);
+    if (decidedMaxInterval > maximumMaxInterval)
+    {
+        decidedMaxInterval = maximumMaxInterval;
+    }
+
+    // Set max interval of the subscription
+    mMaxInterval = static_cast<uint16_t>(decidedMaxInterval);
+
+#endif // CHIP_CONFIG_ENABLE_ICD_SERVER
+
     //
     // Notify the application (if requested) of the impending subscription and check whether we should still proceed to set it up.
     // This also provides the application an opportunity to modify the negotiated min/max intervals set above.

diff --git a/src/app/icd/BUILD.gn b/src/app/icd/BUILD.gn
@@ -29,11 +29,11 @@ source_set("manager-srcs") {
     "ICDManager.h",
   ]
 
-  deps = [
+  public_deps = [
     ":cluster-srcs",
     ":observer-srcs",
+    "${chip_root}/src/credentials:credentials",
   ]
-  public_deps = [ "${chip_root}/src/credentials:credentials" ]
 }
 
 # ICD management cluster source-set is broken out of the main source-set to enable unit tests

diff --git a/src/app/tests/BUILD.gn b/src/app/tests/BUILD.gn
@@ -17,6 +17,7 @@ import("//build_overrides/chip.gni")
 import("//build_overrides/nlunit_test.gni")
 
 import("${chip_root}/build/chip/chip_test_suite.gni")
+import("${chip_root}/src/app/icd/icd.gni")
 import("${chip_root}/src/platform/device.gni")
 
 static_library("helpers") {
@@ -186,7 +187,6 @@ chip_test_suite("tests") {
     ":time-sync-data-provider-test-srcs",
     "${chip_root}/src/app",
     "${chip_root}/src/app/common:cluster-objects",
-    "${chip_root}/src/app/icd:cluster-srcs",
     "${chip_root}/src/app/icd:manager-srcs",
     "${chip_root}/src/app/tests:helpers",
     "${chip_root}/src/app/util/mock:mock_ember",
@@ -195,7 +195,10 @@ chip_test_suite("tests") {
     "${nlunit_test_root}:nlunit-test",
   ]
 
-  if (chip_config_network_layer_ble &&
+  # Do not run TestCommissionManager when running ICD specific unit tests.
+  # ICDManager has a dependency on the Accessors.h file which causes a link error
+  # when building the TestCommissionManager
+  if (!chip_enable_icd_server && chip_config_network_layer_ble &&
       (chip_device_platform == "linux" || chip_device_platform == "darwin")) {
     test_sources += [ "TestCommissionManager.cpp" ]
     public_deps += [ "${chip_root}/src/app/server" ]