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.

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] Add counter invalidation events to the ICD Test Event trigger handler #33174

Merged
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
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
1 change: 1 addition & 0 deletions src/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ if (chip_build_tests) {
"${chip_root}/src/lib/support/tests",
"${chip_root}/src/lib/support/tests:tests_nltest",
"${chip_root}/src/protocols/secure_channel/tests",
"${chip_root}/src/protocols/secure_channel/tests:tests_nltest",
"${chip_root}/src/system/tests",
"${chip_root}/src/transport/tests",
]
Expand Down
14 changes: 12 additions & 2 deletions src/app/icd/server/ICDManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,10 @@
namespace {
enum class ICDTestEventTriggerEvent : uint64_t
{
kAddActiveModeReq = 0x0046'0000'00000001,
kRemoveActiveModeReq = 0x0046'0000'00000002,
kAddActiveModeReq = 0x0046'0000'00000001,
kRemoveActiveModeReq = 0x0046'0000'00000002,
kInvalidateHalfCounterValues = 0x0046'0000'00000003,
kInvalidateAllCounterValues = 0x0046'0000'00000004,
};
} // namespace

Expand Down Expand Up @@ -666,6 +668,14 @@ CHIP_ERROR ICDManager::HandleEventTrigger(uint64_t eventTrigger)
case ICDTestEventTriggerEvent::kRemoveActiveModeReq:
SetKeepActiveModeRequirements(KeepActiveFlag::kTestEventTriggerActiveMode, false);
break;
#if CHIP_CONFIG_ENABLE_ICD_CIP
Copy link
Contributor

Choose a reason for hiding this comment

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

When is this not enabled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For SIT devices that still use the TestEventTrigger but might not have the Check-In Protocol support.

case ICDTestEventTriggerEvent::kInvalidateHalfCounterValues:
err = ICDConfigurationData::GetInstance().GetICDCounter().InvalidateHalfCheckInCouterValues();
break;
case ICDTestEventTriggerEvent::kInvalidateAllCounterValues:
err = ICDConfigurationData::GetInstance().GetICDCounter().InvalidateAllCheckInCounterValues();
break;
#endif // CHIP_CONFIG_ENABLE_ICD_CIP
default:
err = CHIP_ERROR_INVALID_ARGUMENT;
break;
Expand Down
44 changes: 42 additions & 2 deletions src/app/tests/TestICDManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,10 @@ constexpr uint8_t kKeyBuffer2b[] = {
// Taken from the ICDManager Implementation
enum class ICDTestEventTriggerEvent : uint64_t
{
kAddActiveModeReq = 0x0046'0000'00000001,
kRemoveActiveModeReq = 0x0046'0000'00000002,
kAddActiveModeReq = 0x0046'0000'00000001,
kRemoveActiveModeReq = 0x0046'0000'00000002,
kInvalidateHalfCounterValues = 0x0046'0000'00000003,
kInvalidateAllCounterValues = 0x0046'0000'00000004,
};

class TestICDStateObserver : public app::ICDStateObserver
Expand Down Expand Up @@ -742,6 +744,40 @@ class TestICDManager
NL_TEST_ASSERT(aSuite, ctx->mICDManager.mOperationalState == ICDManager::OperationalState::IdleMode);
}

static void TestHandleTestEventTriggerInvalidateHalfCounterValues(nlTestSuite * aSuite, void * aContext)
{
TestContext * ctx = static_cast<TestContext *>(aContext);

constexpr uint32_t startValue = 1;
constexpr uint32_t expectedValue = 2147483648;

// Set starting value for test
ICDConfigurationData::GetInstance().GetICDCounter().SetValue(startValue);

// Trigger ICD kInvalidateHalfCounterValues event
ctx->mICDManager.HandleEventTrigger(static_cast<uint64_t>(ICDTestEventTriggerEvent::kInvalidateHalfCounterValues));

// Validate counter has the expected value
NL_TEST_ASSERT(aSuite, ICDConfigurationData::GetInstance().GetICDCounter().GetValue() == expectedValue);
}

static void TestHandleTestEventTriggerInvalidateAllCounterValues(nlTestSuite * aSuite, void * aContext)
{
TestContext * ctx = static_cast<TestContext *>(aContext);

constexpr uint32_t startValue = 105;
constexpr uint32_t expectedValue = 104;

// Set starting value for test
ICDConfigurationData::GetInstance().GetICDCounter().SetValue(startValue);

// Trigger ICD kInvalidateHalfCounterValues event
ctx->mICDManager.HandleEventTrigger(static_cast<uint64_t>(ICDTestEventTriggerEvent::kInvalidateAllCounterValues));

// Validate counter has the expected value
NL_TEST_ASSERT(aSuite, ICDConfigurationData::GetInstance().GetICDCounter().GetValue() == expectedValue);
}

/**
* @brief Test verifies when OnEnterIdleMode is called during normal operations.
* Without the ActiveMode timer being extended
Expand Down Expand Up @@ -1083,6 +1119,10 @@ static const nlTest sTests[] = {
NL_TEST_DEF("TestICDStayActive", TestICDManager::TestICDMStayActive),
NL_TEST_DEF("TestShouldCheckInMsgsBeSentAtActiveModeFunction", TestICDManager::TestShouldCheckInMsgsBeSentAtActiveModeFunction),
NL_TEST_DEF("TestHandleTestEventTriggerActiveModeReq", TestICDManager::TestHandleTestEventTriggerActiveModeReq),
NL_TEST_DEF("TestHandleTestEventTriggerInvalidateHalfCounterValues",
TestICDManager::TestHandleTestEventTriggerInvalidateHalfCounterValues),
NL_TEST_DEF("TestHandleTestEventTriggerInvalidateAllCounterValues",
TestICDManager::TestHandleTestEventTriggerInvalidateAllCounterValues),
NL_TEST_DEF("TestICDStateObserverOnEnterIdleModeActiveModeDuration",
TestICDManager::TestICDStateObserverOnEnterIdleModeActiveModeDuration),
NL_TEST_DEF("TestICDStateObserverOnEnterIdleModeActiveModeThreshold",
Expand Down
37 changes: 27 additions & 10 deletions src/lib/support/CHIPCounter.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,23 @@ class Counter
virtual ~Counter() {}

/**
* @brief
* Advance the value of the counter.
* @brief Advance the value of the counter.
*
* @return A CHIP error code if anything failed, CHIP_NO_ERROR otherwise.
*/
virtual CHIP_ERROR Advance() = 0;

/**
* @brief
* Get the current value of the counter.
* @brief Advances the current counter value by N
*
* @param value value of N
*
* @return A CHIP error code if anything failed, CHIP_NO_ERROR otherwise.
*/
virtual CHIP_ERROR AdvanceBy(T value) = 0;

/**
* @brief Get the current value of the counter.
*
* @return The current value of the counter.
*/
Expand All @@ -76,8 +83,7 @@ class MonotonicallyIncreasingCounter : public Counter<T>
~MonotonicallyIncreasingCounter() override{};

/**
* @brief
* Initialize a MonotonicallyIncreasingCounter object.
* @brief Initialize a MonotonicallyIncreasingCounter object.
*
* @param[in] aStartValue The starting value of the counter.
*
Expand All @@ -93,8 +99,7 @@ class MonotonicallyIncreasingCounter : public Counter<T>
}

/**
* @brief
* Advance the value of the counter.
* @brief Advance the value of the counter.
*
* @return A CHIP error code if something fails, CHIP_NO_ERROR otherwise
*/
Expand All @@ -108,8 +113,20 @@ class MonotonicallyIncreasingCounter : public Counter<T>
}

/**
* @brief
* Get the current value of the counter.
* @brief Advances the current counter value by N
*
* @param value value of N
*
* @return A CHIP error code if something fails, CHIP_NO_ERROR otherwise
*/
CHIP_ERROR AdvanceBy(T value) override
{
mCounterValue = static_cast<T>(mCounterValue + value);
return CHIP_NO_ERROR;
}

/**
* @brief Get the current value of the counter.
*
* @return The current value of the counter.
*/
Expand Down
34 changes: 28 additions & 6 deletions src/lib/support/PersistedCounter.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,22 @@ class PersistedCounter : public MonotonicallyIncreasingCounter<T>
}
#endif

ReturnErrorOnFailure(PersistNextEpochStart(startValue + aEpoch));
ReturnErrorOnFailure(PersistNextEpochStart(static_cast<T>(startValue + aEpoch)));

// This will set the starting value, after which we're ready.
return MonotonicallyIncreasingCounter<T>::Init(startValue);
}

CHIP_ERROR AdvanceBy(T value) override
{
VerifyOrReturnError(mStorage != nullptr, CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(mKey.IsInitialized(), CHIP_ERROR_INCORRECT_STATE);

ReturnErrorOnFailure(MonotonicallyIncreasingCounter<T>::AdvanceBy(value));

return VerifyAndPersistNextEpochStart(MonotonicallyIncreasingCounter<T>::GetValue());
}

/**
* @brief
* Increment the counter and write to persisted storage if we've completed
Expand All @@ -124,20 +134,32 @@ class PersistedCounter : public MonotonicallyIncreasingCounter<T>

ReturnErrorOnFailure(MonotonicallyIncreasingCounter<T>::Advance());

return VerifyAndPersistNextEpochStart(mNextEpoch);
}

private:
CHIP_ERROR VerifyAndPersistNextEpochStart(T refEpoch)
{

if (MonotonicallyIncreasingCounter<T>::GetValue() >= mNextEpoch)
{
// Value advanced past the previously persisted "start point".
// Ensure that a new starting point is persisted.
ReturnErrorOnFailure(PersistNextEpochStart(mNextEpoch + mEpoch));
ReturnErrorOnFailure(PersistNextEpochStart(static_cast<T>(refEpoch + mEpoch)));

// Advancing the epoch should have ensured that the current value is valid
VerifyOrReturnError(static_cast<T>(MonotonicallyIncreasingCounter<T>::GetValue() + mEpoch) == mNextEpoch,
CHIP_ERROR_INTERNAL);

// Advancing the epoch should have ensured that the current value
// is valid
VerifyOrReturnError(MonotonicallyIncreasingCounter<T>::GetValue() < mNextEpoch, CHIP_ERROR_INTERNAL);
// Previous check did not take into consideration that the counter value can be equal to the max counter value or
// rollover.
// TODO(#33175): PersistedCounter allows rollover so this check is incorrect. We need a Counter class that adequatly
// manages rollover behavior for counters that cannot rollover.
// VerifyOrReturnError(MonotonicallyIncreasingCounter<T>::GetValue() < mNextEpoch, CHIP_ERROR_INTERNAL);
}
return CHIP_NO_ERROR;
}

private:
/**
* @brief
* Write out the counter value to persistent storage.
Expand Down
4 changes: 2 additions & 2 deletions src/lib/support/tests/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,12 @@ chip_test_suite("tests") {
"TestBitMask.cpp",
"TestBufferReader.cpp",
"TestBytesToHex.cpp",
"TestCHIPCounter.cpp",
"TestDefer.cpp",
"TestFixedBufferAllocator.cpp",
"TestFold.cpp",
"TestIniEscaping.cpp",
"TestPersistedCounter.cpp",
"TestSafeInt.cpp",
"TestSafeString.cpp",
"TestScoped.cpp",
Expand Down Expand Up @@ -66,14 +68,12 @@ chip_test_suite_using_nltest("tests_nltest") {
test_sources = [
"TestBufferWriter.cpp",
"TestBytesCircularBuffer.cpp",
"TestCHIPCounter.cpp",
"TestCHIPMem.cpp",
"TestCHIPMemString.cpp",
"TestErrorStr.cpp",
"TestIntrusiveList.cpp",
"TestJsonToTlv.cpp",
"TestJsonToTlvToJson.cpp",
"TestPersistedCounter.cpp",
"TestPool.cpp",
"TestPrivateHeap.cpp",
"TestScopedBuffer.cpp",
Expand Down
95 changes: 38 additions & 57 deletions src/lib/support/tests/TestCHIPCounter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,82 +16,63 @@
* limitations under the License.
*/

#include <nlunit-test.h>

#include <gtest/gtest.h>
#include <lib/support/CHIPCounter.h>
#include <lib/support/UnitTestRegistration.h>
#include <stdint.h>

using namespace chip;

static void CheckStartWithZero(nlTestSuite * inSuite, void * inContext)
TEST(TestCHIPCounter, TestStartWithZero)
{
chip::MonotonicallyIncreasingCounter<uint64_t> counter;
NL_TEST_ASSERT(inSuite, counter.GetValue() == 0);
MonotonicallyIncreasingCounter<uint64_t> counter;
EXPECT_EQ(counter.GetValue(), 0ULL);
}

static void CheckInitialize(nlTestSuite * inSuite, void * inContext)
TEST(TestCHIPCounter, TestInitialize)
{
chip::MonotonicallyIncreasingCounter<uint64_t> counter;
MonotonicallyIncreasingCounter<uint64_t> counter;

NL_TEST_ASSERT(inSuite, counter.Init(4321) == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, counter.GetValue() == 4321);
EXPECT_EQ(counter.Init(4321), CHIP_NO_ERROR);
EXPECT_EQ(counter.GetValue(), 4321ULL);
}

static void CheckAdvance(nlTestSuite * inSuite, void * inContext)
TEST(TestCHIPCounter, TestAdvance)
{
chip::MonotonicallyIncreasingCounter<uint64_t> counter;
MonotonicallyIncreasingCounter<uint64_t> counter;

NL_TEST_ASSERT(inSuite, counter.Init(22) == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, counter.GetValue() == 22);
NL_TEST_ASSERT(inSuite, counter.Advance() == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, counter.GetValue() == 23);
NL_TEST_ASSERT(inSuite, counter.Advance() == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, counter.GetValue() == 24);
EXPECT_EQ(counter.Init(22), CHIP_NO_ERROR);
EXPECT_EQ(counter.GetValue(), 22ULL);
EXPECT_EQ(counter.Advance(), CHIP_NO_ERROR);
EXPECT_EQ(counter.GetValue(), 23ULL);
EXPECT_EQ(counter.Advance(), CHIP_NO_ERROR);
EXPECT_EQ(counter.GetValue(), 24ULL);
}

/**
* Test Suite. It lists all the test functions.
*/

// clang-format off
static const nlTest sTests[] =
TEST(TestCHIPCounter, TestAdvanceWithRollover)
{
NL_TEST_DEF("Start with zero", CheckStartWithZero),
NL_TEST_DEF("Can initialize", CheckInitialize),
NL_TEST_DEF("Can Advance", CheckAdvance),
NL_TEST_SENTINEL()
};
// clang-format on
MonotonicallyIncreasingCounter<uint64_t> counter;
EXPECT_EQ(counter.Init(UINT64_MAX), CHIP_NO_ERROR);

/**
* Set up the test suite.
*/
static int TestSetup(void * inContext)
{
return (SUCCESS);
EXPECT_EQ(counter.Advance(), CHIP_NO_ERROR);
EXPECT_EQ(counter.GetValue(), 0ULL);
}

/**
* Tear down the test suite.
*/
static int TestTeardown(void * inContext)
TEST(TestCHIPCounter, AdvanceBy)
{
return (SUCCESS);
}
MonotonicallyIncreasingCounter<uint64_t> counter;
constexpr uint64_t step = 9876;
uint64_t expectedValue = step;

int TestCHIPCounter()
{
// clang-format off
nlTestSuite theSuite = {
"chip-counter",
&sTests[0],
TestSetup,
TestTeardown
};
// clang-format on
EXPECT_EQ(counter.Init(0), CHIP_NO_ERROR);
EXPECT_EQ(counter.AdvanceBy(step), CHIP_NO_ERROR);
EXPECT_EQ(counter.GetValue(), expectedValue);

// Run test suite against one context.
nlTestRunner(&theSuite, nullptr);
expectedValue += step;
EXPECT_EQ(counter.AdvanceBy(step), CHIP_NO_ERROR);
EXPECT_EQ(counter.GetValue(), expectedValue);

return nlTestRunnerStats(&theSuite);
// Force rollover
expectedValue += UINT64_MAX;
EXPECT_EQ(counter.AdvanceBy(UINT64_MAX), CHIP_NO_ERROR);
EXPECT_EQ(counter.GetValue(), expectedValue);
}

CHIP_REGISTER_TEST_SUITE(TestCHIPCounter)
Loading
Loading