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
Merged

[ICD] Expose SetModeDurations via ICDManager #37887

Show file tree
Hide file tree
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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; };
Expand Down
8 changes: 0 additions & 8 deletions src/app/icd/server/tests/ICDConfigurationDataTestAccess.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
22 changes: 11 additions & 11 deletions src/app/icd/server/tests/TestICDManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}

/**
Expand All @@ -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);
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}

/**
Expand All @@ -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);
Expand All @@ -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);
}

/**
Expand All @@ -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);
Expand All @@ -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);
}

/**
Expand Down
Loading