[ICD] Refactor ICDManager to use System::Clock types (project-chip#31881)

mkardous-silabs · web-flow · commit 83e47bff9aa4 · 2024-02-05T20:33:24.000Z
* Refactor time members

* add using namespace

* Fix ActiveModeThreshold type and cast
diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp
@@ -46,7 +46,7 @@ using Status = Protocols::InteractionModel::Status;
 uint16_t ReadHandler::GetPublisherSelectedIntervalLimit()
 {
 #if CHIP_CONFIG_ENABLE_ICD_SERVER
-    return static_cast<uint16_t>(ICDConfigurationData::GetInstance().GetIdleModeDurationSec());
+    return std::chrono::duration_cast<System::Clock::Seconds16>(ICDConfigurationData::GetInstance().GetIdleModeDuration()).count();
 #else
     return kSubscriptionMaxIntervalPublisherLimit;
 #endif
diff --git a/src/app/SubscriptionsInfoProvider.h b/src/app/SubscriptionsInfoProvider.h
@@ -42,6 +42,7 @@ class SubscriptionsInfoProvider
      *
      * @param[in] aFabricIndex fabric index of the subject
      * @param[in] subjectID NodeId of the subject
+     *            subjectID may encode a CAT in the reserved section of the NodeID.
      *
      * @return true subject has at least one active subscription with the device
      *         false subject doesn't have any active subscription with the device
@@ -54,6 +55,7 @@ class SubscriptionsInfoProvider
      *
      * @param[in] aFabricIndex fabric index of the subject
      * @param[in] subjectID NodeId of the subject
+     *            subjectID may encode a CAT in the reserved section of the NodeID
      *
      * @return true subject has at least one persisted subscription with the device
      *         false subject doesn't have any persisted subscription with the device
diff --git a/src/app/clusters/icd-management-server/icd-management-server.cpp b/src/app/clusters/icd-management-server/icd-management-server.cpp
@@ -101,17 +101,17 @@ CHIP_ERROR IcdManagementAttributeAccess::Read(const ConcreteReadAttributePath &
 
 CHIP_ERROR IcdManagementAttributeAccess::ReadIdleModeDuration(EndpointId endpoint, AttributeValueEncoder & encoder)
 {
-    return encoder.Encode(mICDConfigurationData->GetIdleModeDurationSec());
+    return encoder.Encode(mICDConfigurationData->GetIdleModeDuration().count());
 }
 
 CHIP_ERROR IcdManagementAttributeAccess::ReadActiveModeDuration(EndpointId endpoint, AttributeValueEncoder & encoder)
 {
-    return encoder.Encode(mICDConfigurationData->GetActiveModeDurationMs());
+    return encoder.Encode(mICDConfigurationData->GetActiveModeDuration().count());
 }
 
 CHIP_ERROR IcdManagementAttributeAccess::ReadActiveModeThreshold(EndpointId endpoint, AttributeValueEncoder & encoder)
 {
-    return encoder.Encode(mICDConfigurationData->GetActiveModeThresholdMs());
+    return encoder.Encode(mICDConfigurationData->GetActiveModeThreshold().count());
 }
 
 CHIP_ERROR IcdManagementAttributeAccess::ReadRegisteredClients(EndpointId endpoint, AttributeValueEncoder & encoder)
diff --git a/src/app/icd/server/ICDCheckInSender.cpp b/src/app/icd/server/ICDCheckInSender.cpp
@@ -64,7 +64,8 @@ CHIP_ERROR ICDCheckInSender::SendCheckInMsg(const Transport::PeerAddress & addr)
         size_t writtenBytes                                     = 0;
         Encoding::LittleEndian::BufferWriter writer(activeModeThresholdBuffer, sizeof(activeModeThresholdBuffer));
 
-        writer.Put16(ICDConfigurationData::GetInstance().GetActiveModeThresholdMs());
+        uint16_t activeModeThreshold_ms = ICDConfigurationData::GetInstance().GetActiveModeThreshold().count();
+        writer.Put16(activeModeThreshold_ms);
         VerifyOrReturnError(writer.Fit(writtenBytes), CHIP_ERROR_INTERNAL);
 
         ByteSpan activeModeThresholdByteSpan(writer.Buffer(), writtenBytes);
diff --git a/src/app/icd/server/ICDConfigurationData.cpp b/src/app/icd/server/ICDConfigurationData.cpp
@@ -37,20 +37,24 @@ System::Clock::Milliseconds32 ICDConfigurationData::GetSlowPollingInterval()
     return mSlowPollingInterval;
 }
 
-CHIP_ERROR ICDConfigurationData::SetModeDurations(Optional<uint32_t> activeModeDuration_ms, Optional<uint32_t> idleModeDuration_ms)
+CHIP_ERROR ICDConfigurationData::SetModeDurations(Optional<System::Clock::Milliseconds32> activeModeDuration,
+                                                  Optional<System::Clock::Milliseconds32> idleModeDuration)
 {
-    VerifyOrReturnError(activeModeDuration_ms.HasValue() || idleModeDuration_ms.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
+    VerifyOrReturnError(activeModeDuration.HasValue() || idleModeDuration.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
 
-    uint32_t tmpIdleModeDuration_s =
-        idleModeDuration_ms.HasValue() ? (idleModeDuration_ms.Value() / kMillisecondsPerSecond) : mIdleModeDuration_s;
-    uint32_t tmpActiveModeDuration_ms = activeModeDuration_ms.HasValue() ? activeModeDuration_ms.Value() : mActiveModeDuration_ms;
+    // Convert idleModeDuration to seconds for the correct precision
+    System::Clock::Seconds32 tmpIdleModeDuration = idleModeDuration.HasValue()
+        ? std::chrono::duration_cast<System::Clock::Seconds32>(idleModeDuration.Value())
+        : mIdleModeDuration;
 
-    VerifyOrReturnError(tmpActiveModeDuration_ms <= (tmpIdleModeDuration_s * kMillisecondsPerSecond), CHIP_ERROR_INVALID_ARGUMENT);
-    VerifyOrReturnError(tmpIdleModeDuration_s <= kMaxIdleModeDuration_s, CHIP_ERROR_INVALID_ARGUMENT);
-    VerifyOrReturnError(tmpIdleModeDuration_s >= kMinIdleModeDuration_s, CHIP_ERROR_INVALID_ARGUMENT);
+    System::Clock::Milliseconds32 tmpActiveModeDuration = activeModeDuration.ValueOr(mActiveModeDuration);
 
-    mIdleModeDuration_s    = tmpIdleModeDuration_s;
-    mActiveModeDuration_ms = tmpActiveModeDuration_ms;
+    VerifyOrReturnError(tmpActiveModeDuration <= tmpIdleModeDuration, CHIP_ERROR_INVALID_ARGUMENT);
+    VerifyOrReturnError(tmpIdleModeDuration <= kMaxIdleModeDuration, CHIP_ERROR_INVALID_ARGUMENT);
+    VerifyOrReturnError(tmpIdleModeDuration >= kMinIdleModeDuration, CHIP_ERROR_INVALID_ARGUMENT);
+
+    mIdleModeDuration   = tmpIdleModeDuration;
+    mActiveModeDuration = tmpActiveModeDuration;
 
     return CHIP_NO_ERROR;
 }
diff --git a/src/app/icd/server/ICDConfigurationData.h b/src/app/icd/server/ICDConfigurationData.h
@@ -54,11 +54,11 @@ class ICDConfigurationData
 
     static ICDConfigurationData & GetInstance() { return instance; };
 
-    uint32_t GetIdleModeDurationSec() { return mIdleModeDuration_s; }
+    System::Clock::Seconds32 GetIdleModeDuration() { return mIdleModeDuration; }
 
-    uint32_t GetActiveModeDurationMs() { return mActiveModeDuration_ms; }
+    System::Clock::Milliseconds32 GetActiveModeDuration() { return mActiveModeDuration; }
 
-    uint16_t GetActiveModeThresholdMs() { return mActiveThreshold_ms; }
+    System::Clock::Milliseconds16 GetActiveModeThreshold() { return mActiveThreshold; }
 
     uint32_t GetICDCounter() { return mICDCounter; }
 
@@ -68,7 +68,7 @@ class ICDConfigurationData
 
     System::Clock::Milliseconds32 GetFastPollingInterval() { return mFastPollingInterval; }
 
-    uint32_t GetMinLitActiveModeThresholdMs() { return kMinLitActiveModeThreshold_ms; }
+    System::Clock::Milliseconds16 GetMinLitActiveModeThreshold() { return kMinLitActiveModeThreshold; }
 
     /**
      * If ICD_ENFORCE_SIT_SLOW_POLL_LIMIT is set to 0, function will always return the configured Slow Polling interval
@@ -103,37 +103,39 @@ class ICDConfigurationData
     void SetSlowPollingInterval(System::Clock::Milliseconds32 slowPollInterval) { mSlowPollingInterval = slowPollInterval; };
     void SetFastPollingInterval(System::Clock::Milliseconds32 fastPollInterval) { mFastPollingInterval = fastPollInterval; };
 
-    static constexpr uint32_t kMinLitActiveModeThreshold_ms = 5000;
+    static constexpr System::Clock::Milliseconds16 kMinLitActiveModeThreshold = System::Clock::Milliseconds16(5000);
 
     /**
      * @brief Change the ActiveModeDuration or the IdleModeDuration value
      *        If only one value is provided, check will be done agaisn't the other already set value.
      *
-     * @param[in] activeModeDuration_ms new ActiveModeDuration value
-     * @param[in] idleModeDuration_ms new IdleModeDuration value
+     * @param[in] activeModeDuration new ActiveModeDuration value
+     * @param[in] idleModeDuration new IdleModeDuration value
      *                                The precision of the IdleModeDuration must be seconds.
      * @return CHIP_ERROR CHIP_ERROR_INVALID_ARGUMENT is returned if idleModeDuration_ms is smaller than activeModeDuration_ms
      *                                                is returned if idleModeDuration_ms is greater than 64800000 ms
      *                                                is returned if idleModeDuration_ms is smaller than 1000 ms
      *                                                is returned if no valid values are provided
      *                    CHIP_NO_ERROR is returned if the new intervals were set
      */
-    CHIP_ERROR SetModeDurations(Optional<uint32_t> activeModeDuration_ms, Optional<uint32_t> idleModeDuration_ms);
+    CHIP_ERROR SetModeDurations(Optional<System::Clock::Milliseconds32> activeModeDuration,
+                                Optional<System::Clock::Milliseconds32> idleModeDuration);
 
-    static constexpr uint32_t kMaxIdleModeDuration_s = (18 * kSecondsPerHour);
-    static constexpr uint32_t kMinIdleModeDuration_s = 1;
+    static constexpr System::Clock::Seconds32 kMaxIdleModeDuration = System::Clock::Seconds32(18 * kSecondsPerHour);
+    static constexpr System::Clock::Seconds32 kMinIdleModeDuration = System::Clock::Seconds32(1);
 
-    static_assert((CHIP_CONFIG_ICD_IDLE_MODE_DURATION_SEC) <= kMaxIdleModeDuration_s,
+    static_assert((CHIP_CONFIG_ICD_IDLE_MODE_DURATION_SEC) <= kMaxIdleModeDuration.count(),
                   "Spec requires the IdleModeDuration to be equal or inferior to 64800s.");
-    static_assert((CHIP_CONFIG_ICD_IDLE_MODE_DURATION_SEC) >= kMinIdleModeDuration_s,
+    static_assert((CHIP_CONFIG_ICD_IDLE_MODE_DURATION_SEC) >= kMinIdleModeDuration.count(),
                   "Spec requires the IdleModeDuration to be equal or greater to 1s.");
-    uint32_t mIdleModeDuration_s = CHIP_CONFIG_ICD_IDLE_MODE_DURATION_SEC;
+    System::Clock::Seconds32 mIdleModeDuration = System::Clock::Seconds32(CHIP_CONFIG_ICD_IDLE_MODE_DURATION_SEC);
 
-    static_assert((CHIP_CONFIG_ICD_ACTIVE_MODE_DURATION_MS) <= (CHIP_CONFIG_ICD_IDLE_MODE_DURATION_SEC * kMillisecondsPerSecond),
+    static_assert(System::Clock::Milliseconds32(CHIP_CONFIG_ICD_ACTIVE_MODE_DURATION_MS) <=
+                      System::Clock::Seconds32(CHIP_CONFIG_ICD_IDLE_MODE_DURATION_SEC),
                   "Spec requires the IdleModeDuration be equal or greater to the ActiveModeDuration.");
-    uint32_t mActiveModeDuration_ms = CHIP_CONFIG_ICD_ACTIVE_MODE_DURATION_MS;
+    System::Clock::Milliseconds32 mActiveModeDuration = System::Clock::Milliseconds32(CHIP_CONFIG_ICD_ACTIVE_MODE_DURATION_MS);
 
-    uint16_t mActiveThreshold_ms = CHIP_CONFIG_ICD_ACTIVE_MODE_THRESHOLD_MS;
+    System::Clock::Milliseconds16 mActiveThreshold = System::Clock::Milliseconds16(CHIP_CONFIG_ICD_ACTIVE_MODE_THRESHOLD_MS);
 
     uint32_t mICDCounter = 0;
 
diff --git a/src/app/icd/server/ICDManager.cpp b/src/app/icd/server/ICDManager.cpp
@@ -35,6 +35,7 @@ namespace app {
 using namespace chip::app;
 using namespace chip::app::Clusters;
 using namespace chip::app::Clusters::IcdManagement;
+using namespace System::Clock;
 
 static_assert(UINT8_MAX >= CHIP_CONFIG_MAX_EXCHANGE_CONTEXTS,
               "ICDManager::mOpenExchangeContextCount cannot hold count for the max exchange count");
@@ -55,8 +56,8 @@ void ICDManager::Init(PersistentStorageDelegate * storage, FabricTable * fabricT
                            "The CheckIn protocol feature is required for LIT support.");
         VerifyOrDieWithMsg(SupportsFeature(Feature::kUserActiveModeTrigger), AppServer,
                            "The user ActiveMode trigger feature is required for LIT support.");
-        VerifyOrDieWithMsg(ICDConfigurationData::GetInstance().GetMinLitActiveModeThresholdMs() <=
-                               ICDConfigurationData::GetInstance().GetActiveModeThresholdMs(),
+        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,
@@ -280,13 +281,12 @@ void ICDManager::UpdateOperationState(OperationalState state)
         // When the active mode interval is 0, we stay in idleMode until a notification brings the icd into active mode
         // unless the device would need to send Check-In messages
         // TODO(#30281) : Verify how persistent subscriptions affects this at ICDManager::Init
-        if (ICDConfigurationData::GetInstance().GetActiveModeDurationMs() > 0 || CheckInMessagesWouldBeSent())
+        if (ICDConfigurationData::GetInstance().GetActiveModeDuration() > kZero || CheckInMessagesWouldBeSent())
         {
-            uint32_t idleModeDuration = ICDConfigurationData::GetInstance().GetIdleModeDurationSec();
-            DeviceLayer::SystemLayer().StartTimer(System::Clock::Seconds32(idleModeDuration), OnIdleModeDone, this);
+            DeviceLayer::SystemLayer().StartTimer(ICDConfigurationData::GetInstance().GetIdleModeDuration(), OnIdleModeDone, this);
         }
 
-        System::Clock::Milliseconds32 slowPollInterval = ICDConfigurationData::GetInstance().GetSlowPollingInterval();
+        Milliseconds32 slowPollInterval = ICDConfigurationData::GetInstance().GetSlowPollingInterval();
 
         // Going back to Idle, all Check-In messages are sent
         mICDSenderPool.ReleaseAll();
@@ -305,21 +305,23 @@ void ICDManager::UpdateOperationState(OperationalState state)
             // Make sure the idle mode timer is stopped
             DeviceLayer::SystemLayer().CancelTimer(OnIdleModeDone, this);
 
-            mOperationalState           = OperationalState::ActiveMode;
-            uint32_t activeModeDuration = ICDConfigurationData::GetInstance().GetActiveModeDurationMs();
+            mOperationalState                 = OperationalState::ActiveMode;
+            Milliseconds32 activeModeDuration = ICDConfigurationData::GetInstance().GetActiveModeDuration();
 
-            if (activeModeDuration == 0 && !mKeepActiveFlags.HasAny())
+            if (activeModeDuration == kZero && !mKeepActiveFlags.HasAny())
             {
                 // A Network Activity triggered the active mode and activeModeDuration is 0.
                 // Stay active for at least Active Mode Threshold.
-                activeModeDuration = ICDConfigurationData::GetInstance().GetActiveModeThresholdMs();
+                activeModeDuration = ICDConfigurationData::GetInstance().GetActiveModeThreshold();
             }
 
-            DeviceLayer::SystemLayer().StartTimer(System::Clock::Timeout(activeModeDuration), OnActiveModeDone, this);
+            DeviceLayer::SystemLayer().StartTimer(activeModeDuration, OnActiveModeDone, this);
 
-            uint32_t activeModeJitterInterval =
-                (activeModeDuration >= ICD_ACTIVE_TIME_JITTER_MS) ? activeModeDuration - ICD_ACTIVE_TIME_JITTER_MS : 0;
-            DeviceLayer::SystemLayer().StartTimer(System::Clock::Timeout(activeModeJitterInterval), OnTransitionToIdle, this);
+            Milliseconds32 activeModeJitterInterval = Milliseconds32(ICD_ACTIVE_TIME_JITTER_MS);
+            activeModeJitterInterval =
+                (activeModeDuration >= activeModeJitterInterval) ? activeModeDuration - activeModeJitterInterval : kZero;
+
+            DeviceLayer::SystemLayer().StartTimer(activeModeJitterInterval, OnTransitionToIdle, this);
 
             CHIP_ERROR err =
                 DeviceLayer::ConnectivityMgr().SetPollingInterval(ICDConfigurationData::GetInstance().GetFastPollingInterval());
@@ -337,14 +339,16 @@ void ICDManager::UpdateOperationState(OperationalState state)
         }
         else
         {
-            uint16_t activeModeThreshold = ICDConfigurationData::GetInstance().GetActiveModeThresholdMs();
-            DeviceLayer::SystemLayer().ExtendTimerTo(System::Clock::Timeout(activeModeThreshold), OnActiveModeDone, this);
-            uint16_t activeModeJitterThreshold =
-                (activeModeThreshold >= ICD_ACTIVE_TIME_JITTER_MS) ? activeModeThreshold - ICD_ACTIVE_TIME_JITTER_MS : 0;
+            Milliseconds16 activeModeThreshold = ICDConfigurationData::GetInstance().GetActiveModeThreshold();
+            DeviceLayer::SystemLayer().ExtendTimerTo(activeModeThreshold, OnActiveModeDone, this);
+
+            Milliseconds32 activeModeJitterThreshold = Milliseconds32(ICD_ACTIVE_TIME_JITTER_MS);
+            activeModeJitterThreshold =
+                (activeModeThreshold >= activeModeJitterThreshold) ? activeModeThreshold - activeModeJitterThreshold : kZero;
+
             if (!mTransitionToIdleCalled)
             {
-                DeviceLayer::SystemLayer().ExtendTimerTo(System::Clock::Timeout(activeModeJitterThreshold), OnTransitionToIdle,
-                                                         this);
+                DeviceLayer::SystemLayer().ExtendTimerTo(activeModeJitterThreshold, OnTransitionToIdle, this);
             }
         }
     }
diff --git a/src/app/tests/TestICDManager.cpp b/src/app/tests/TestICDManager.cpp
diff --git a/src/messaging/ReliableMessageProtocolConfig.cpp b/src/messaging/ReliableMessageProtocolConfig.cpp

Original file line number	Diff line number	Diff line change
`@@ -46,7 +46,7 @@ using Status = Protocols::InteractionModel::Status;`
`46`	`46`	`uint16_t ReadHandler::GetPublisherSelectedIntervalLimit()`
`47`	`47`	`{`
`48`	`48`	`#if CHIP_CONFIG_ENABLE_ICD_SERVER`
`49`		`- return static_cast<uint16_t>(ICDConfigurationData::GetInstance().GetIdleModeDurationSec());`
	`49`	`+ return std::chrono::duration_cast<System::Clock::Seconds16>(ICDConfigurationData::GetInstance().GetIdleModeDuration()).count();`
`50`	`50`	`#else`
`51`	`51`	`return kSubscriptionMaxIntervalPublisherLimit;`
`52`	`52`	`#endif`
Original file line number	Diff line number	Diff line change
`@@ -42,6 +42,7 @@ class SubscriptionsInfoProvider`
`42`	`42`	`*`
`43`	`43`	`* @param[in] aFabricIndex fabric index of the subject`
`44`	`44`	`* @param[in] subjectID NodeId of the subject`
	`45`	`+ * subjectID may encode a CAT in the reserved section of the NodeID.`
`45`	`46`	`*`
`46`	`47`	`* @return true subject has at least one active subscription with the device`
`47`	`48`	`* false subject doesn't have any active subscription with the device`
`@@ -54,6 +55,7 @@ class SubscriptionsInfoProvider`
`54`	`55`	`*`
`55`	`56`	`* @param[in] aFabricIndex fabric index of the subject`
`56`	`57`	`* @param[in] subjectID NodeId of the subject`
	`58`	`+ * subjectID may encode a CAT in the reserved section of the NodeID`
`57`	`59`	`*`
`58`	`60`	`* @return true subject has at least one persisted subscription with the device`
`59`	`61`	`* false subject doesn't have any persisted subscription with the device`
Original file line number	Diff line number	Diff line change
`@@ -101,17 +101,17 @@ CHIP_ERROR IcdManagementAttributeAccess::Read(const ConcreteReadAttributePath &`
`101`	`101`
`102`	`102`	`CHIP_ERROR IcdManagementAttributeAccess::ReadIdleModeDuration(EndpointId endpoint, AttributeValueEncoder & encoder)`
`103`	`103`	`{`
`104`		`- return encoder.Encode(mICDConfigurationData->GetIdleModeDurationSec());`
	`104`	`+ return encoder.Encode(mICDConfigurationData->GetIdleModeDuration().count());`
`105`	`105`	`}`
`106`	`106`
`107`	`107`	`CHIP_ERROR IcdManagementAttributeAccess::ReadActiveModeDuration(EndpointId endpoint, AttributeValueEncoder & encoder)`
`108`	`108`	`{`
`109`		`- return encoder.Encode(mICDConfigurationData->GetActiveModeDurationMs());`
	`109`	`+ return encoder.Encode(mICDConfigurationData->GetActiveModeDuration().count());`
`110`	`110`	`}`
`111`	`111`
`112`	`112`	`CHIP_ERROR IcdManagementAttributeAccess::ReadActiveModeThreshold(EndpointId endpoint, AttributeValueEncoder & encoder)`
`113`	`113`	`{`
`114`		`- return encoder.Encode(mICDConfigurationData->GetActiveModeThresholdMs());`
	`114`	`+ return encoder.Encode(mICDConfigurationData->GetActiveModeThreshold().count());`
`115`	`115`	`}`
`116`	`116`
`117`	`117`	`CHIP_ERROR IcdManagementAttributeAccess::ReadRegisteredClients(EndpointId endpoint, AttributeValueEncoder & encoder)`