Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ICD] Expose SetModeDurations via ICDManager #37887

Merged
merged 2 commits into from
Mar 6, 2025
+18 −19
Merged

[ICD] Expose SetModeDurations via ICDManager #37887

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6873d0b
[ICD] Expose SetModeDurations via ICDManager
yunhanw-google Mar 6, 2025
9dab068
Restyled by clang-format
restyled-commits Mar 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/app/icd/server/ICDManager.h
View file
Original file line number Diff line number Diff line change
@@ -174,6 +174,13 @@ class ICDManager : public ICDListener, public TestEventTriggerHandler
*/
bool SupportsFeature(Clusters::IcdManagement::Feature feature);

// See ICDConfigurationData::SetModeDurations
CHIP_ERROR SetModeDurations(Optional<System::Clock::Milliseconds32> activeModeDuration,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should these be const & instead of passed by value?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a comment that this API doesn't currently garantee that spec compliance is respected if called at run time.

Optional<System::Clock::Milliseconds32> idleModeDuration)
{
return ICDConfigurationData::GetInstance().SetModeDurations(activeModeDuration, idleModeDuration);
};

ICDConfigurationData::ICDMode GetICDMode() { return ICDConfigurationData::GetInstance().GetICDMode(); };

OperationalState GetOperaionalState() { return mOperationalState; };
8 changes: 0 additions & 8 deletions src/app/icd/server/tests/ICDConfigurationDataTestAccess.h
View file
Original file line number Diff line number Diff line change
@@ -26,21 +26,13 @@ namespace Test {
/**
* @brief Class acts as an accessor to private methods of the ICDConfigurationData class without needing to give friend access to
* each individual test.
* This design is necessary because certain tests validate specific edge cases around specific configurations.
* See ICDConfigurationData.h for more details why SetModeDurations cannot be a public API.
*/
class ICDConfigurationDataTestAccess
{
public:
ICDConfigurationDataTestAccess() = delete;
ICDConfigurationDataTestAccess(ICDConfigurationData * data) : mData(data) {}

CHIP_ERROR SetModeDurations(Optional<System::Clock::Milliseconds32> activeModeDuration,
Optional<System::Clock::Milliseconds32> idleModeDuration)
{
return mData->SetModeDurations(activeModeDuration, idleModeDuration);
}

void SetFeatureMap(BitFlags<app::Clusters::IcdManagement::Feature> featureMap) { mData->SetFeatureMap(featureMap); }

private:
22 changes: 11 additions & 11 deletions src/app/icd/server/tests/TestICDManager.cpp
View file
Original file line number Diff line number Diff line change
@@ -273,7 +273,7 @@ TEST_F(TestICDManager, TestICDModeDurationsWith0ActiveModeDurationWithoutActiveS

// Set New durations for test case
Milliseconds32 oldActiveModeDuration = icdConfigData.GetActiveModeDuration();
privateIcdConfigData.SetModeDurations(MakeOptional<Milliseconds32>(0), NullOptional);
mICDManager.SetModeDurations(MakeOptional<Milliseconds32>(0), NullOptional);

// Verify That ICDManager starts in Idle
EXPECT_EQ(mICDManager.GetOperaionalState(), ICDManager::OperationalState::IdleMode);
@@ -331,7 +331,7 @@ TEST_F(TestICDManager, TestICDModeDurationsWith0ActiveModeDurationWithoutActiveS
EXPECT_EQ(mICDManager.GetOperaionalState(), ICDManager::OperationalState::IdleMode);

// Reset Old durations
privateIcdConfigData.SetModeDurations(MakeOptional(oldActiveModeDuration), NullOptional);
mICDManager.SetModeDurations(MakeOptional(oldActiveModeDuration), NullOptional);
}

/**
@@ -355,7 +355,7 @@ TEST_F(TestICDManager, TestICDModeDurationsWith0ActiveModeDurationWithActiveSub)

// Set New durations for test case
Milliseconds32 oldActiveModeDuration = icdConfigData.GetActiveModeDuration();
privateIcdConfigData.SetModeDurations(MakeOptional<Milliseconds32>(0), NullOptional);
mICDManager.SetModeDurations(MakeOptional<Milliseconds32>(0), NullOptional);

// Verify That ICDManager starts in Idle
EXPECT_EQ(mICDManager.GetOperaionalState(), ICDManager::OperationalState::IdleMode);
@@ -424,7 +424,7 @@ TEST_F(TestICDManager, TestICDModeDurationsWith0ActiveModeDurationWithActiveSub)
EXPECT_EQ(mICDManager.GetOperaionalState(), ICDManager::OperationalState::IdleMode);

// Reset Old durations
privateIcdConfigData.SetModeDurations(MakeOptional<Milliseconds32>(oldActiveModeDuration), NullOptional);
mICDManager.SetModeDurations(MakeOptional<Milliseconds32>(oldActiveModeDuration), NullOptional);
}
#endif // CHIP_CONFIG_ENABLE_ICD_CIP

@@ -1100,8 +1100,8 @@ TEST_F(TestICDManager, TestICDStateObserverOnTransitionToIdleModeGreaterActiveMo

// Set New durations for test case - ActiveModeDuration must be longuer than ICD_ACTIVE_TIME_JITTER_MS
Milliseconds32 oldActiveModeDuration = ICDConfigurationData::GetInstance().GetActiveModeDuration();
privateIcdConfigData.SetModeDurations(
MakeOptional<Milliseconds32>(Milliseconds32(200) + Milliseconds32(ICD_ACTIVE_TIME_JITTER_MS)), NullOptional);
mICDManager.SetModeDurations(MakeOptional<Milliseconds32>(Milliseconds32(200) + Milliseconds32(ICD_ACTIVE_TIME_JITTER_MS)),
NullOptional);

// Advance clock just before IdleMode timer expires
AdvanceClockAndRunEventLoop(ICDConfigurationData::GetInstance().GetIdleModeDuration() - 1_s);
@@ -1138,7 +1138,7 @@ TEST_F(TestICDManager, TestICDStateObserverOnTransitionToIdleModeGreaterActiveMo
EXPECT_FALSE(mICDStateObserver.mOnTransitionToIdleCalled);

// Reset Old durations
privateIcdConfigData.SetModeDurations(MakeOptional(oldActiveModeDuration), NullOptional);
mICDManager.SetModeDurations(MakeOptional(oldActiveModeDuration), NullOptional);
}

/**
@@ -1151,7 +1151,7 @@ TEST_F(TestICDManager, TestICDStateObserverOnTransitionToIdleModeEqualActiveMode

// Set New durations for test case - ActiveModeDuration must be equal to ICD_ACTIVE_TIME_JITTER_MS
Milliseconds32 oldActiveModeDuration = ICDConfigurationData::GetInstance().GetActiveModeDuration();
privateIcdConfigData.SetModeDurations(MakeOptional<Milliseconds32>(Milliseconds32(ICD_ACTIVE_TIME_JITTER_MS)), NullOptional);
mICDManager.SetModeDurations(MakeOptional<Milliseconds32>(Milliseconds32(ICD_ACTIVE_TIME_JITTER_MS)), NullOptional);

// Advance clock just before IdleMode timer expires
AdvanceClockAndRunEventLoop(ICDConfigurationData::GetInstance().GetIdleModeDuration() - 1_s);
@@ -1171,7 +1171,7 @@ TEST_F(TestICDManager, TestICDStateObserverOnTransitionToIdleModeEqualActiveMode
EXPECT_TRUE(mICDStateObserver.mOnTransitionToIdleCalled);

// Reset Old durations
privateIcdConfigData.SetModeDurations(MakeOptional(oldActiveModeDuration), NullOptional);
mICDManager.SetModeDurations(MakeOptional(oldActiveModeDuration), NullOptional);
}

/**
@@ -1183,7 +1183,7 @@ TEST_F(TestICDManager, TestICDStateObserverOnTransitionToIdleMode0ActiveModeDura

// Set New durations for test case - ActiveModeDuration equal 0
Milliseconds32 oldActiveModeDuration = ICDConfigurationData::GetInstance().GetActiveModeDuration();
privateIcdConfigData.SetModeDurations(MakeOptional<Milliseconds32>(0), NullOptional);
mICDManager.SetModeDurations(MakeOptional<Milliseconds32>(0), NullOptional);

// Advance clock just before IdleMode timer expires
AdvanceClockAndRunEventLoop(ICDConfigurationData::GetInstance().GetIdleModeDuration() - 1_s);
@@ -1203,7 +1203,7 @@ TEST_F(TestICDManager, TestICDStateObserverOnTransitionToIdleMode0ActiveModeDura
EXPECT_TRUE(mICDStateObserver.mOnTransitionToIdleCalled);

// Reset Old durations
privateIcdConfigData.SetModeDurations(MakeOptional(oldActiveModeDuration), NullOptional);
mICDManager.SetModeDurations(MakeOptional(oldActiveModeDuration), NullOptional);
}

/**