From 1089650299f920eb76550b75024795caef2b4171 Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Tue, 31 Jan 2023 16:18:20 -0500 Subject: [PATCH] Refactored sceneData to support logging without name, added test functionnal with test unit --- src/app/clusters/scenes/SceneTable.h | 52 ++++++++++-------- src/app/clusters/scenes/SceneTableImpl.cpp | 7 ++- src/app/tests/BUILD.gn | 1 + src/app/tests/TestSceneTable.cpp | 61 +++++++++++++++------- 4 files changed, 77 insertions(+), 44 deletions(-) diff --git a/src/app/clusters/scenes/SceneTable.h b/src/app/clusters/scenes/SceneTable.h index 7121cea2abbdc1..ddc96f69036c95 100644 --- a/src/app/clusters/scenes/SceneTable.h +++ b/src/app/clusters/scenes/SceneTable.h @@ -21,6 +21,7 @@ #include #include #include +#include namespace chip { namespace scenes { @@ -108,27 +109,30 @@ class SceneTable static constexpr TLV::Tag TagSceneTransitionTime100() { return TLV::ContextTag(3); } char name[kSceneNameMax] = { 0 }; + size_t nameLength = 0; SceneTransitionTime sceneTransitionTime = 0; ExtensionFieldsSets extentsionFieldsSets; TransitionTime100ms transitionTime100 = 0; + CharSpan nameSpan; - SceneData(const char * sceneName = nullptr, SceneTransitionTime time = 0, TransitionTime100ms time100ms = 0) : + SceneData(const CharSpan & sceneName = CharSpan(), SceneTransitionTime time = 0, TransitionTime100ms time100ms = 0) : sceneTransitionTime(time), transitionTime100(time100ms) { - SetName(sceneName); + this->SetName(sceneName); } - SceneData(ExtensionFieldsSets fields, const char * sceneName = nullptr, SceneTransitionTime time = 0, + SceneData(ExtensionFieldsSets fields, const CharSpan & sceneName = CharSpan(), SceneTransitionTime time = 0, TransitionTime100ms time100ms = 0) : sceneTransitionTime(time), transitionTime100(time100ms) { - SetName(sceneName); + this->SetName(sceneName); extentsionFieldsSets = fields; } - SceneData(const SceneData & data) : sceneTransitionTime(data.sceneTransitionTime), transitionTime100(data.transitionTime100) + SceneData(const SceneData & other) : + sceneTransitionTime(other.sceneTransitionTime), transitionTime100(other.transitionTime100) { - SetName(data.name); - extentsionFieldsSets = data.extentsionFieldsSets; + this->SetName(other.nameSpan); + extentsionFieldsSets = other.extentsionFieldsSets; } CHIP_ERROR Serialize(TLV::TLVWriter & writer) const @@ -136,11 +140,10 @@ class SceneTable TLV::TLVType container; ReturnErrorOnFailure(writer.StartContainer(TLV::ContextTag(1), TLV::kTLVType_Structure, container)); - // assumes a 0 size means the name wasn't used so it doesn't get stored - if (this->name[0] != 0) + // A 0 size means the name wasn't used so it won't get stored + if (!this->nameSpan.empty()) { - size_t name_size = strnlen(this->name, kSceneNameMax); - ReturnErrorOnFailure(writer.PutString(TagSceneName(), this->name, static_cast(name_size))); + ReturnErrorOnFailure(writer.PutString(TagSceneName(), this->nameSpan)); } ReturnErrorOnFailure(writer.Put(TagSceneTransitionTime(), static_cast(this->sceneTransitionTime))); @@ -162,14 +165,13 @@ class SceneTable // If there was no error, a name is expected from the storage, if there was an unexpectec TLV element, if (currTag == TagSceneName()) { - size_t name_size = reader.GetLength(); - VerifyOrReturnError(name_size <= (kSceneNameMax - 1), CHIP_ERROR_BUFFER_TOO_SMALL); - ReturnErrorOnFailure(reader.GetString(this->name, name_size)); - this->name[name_size] = 0; + ReturnErrorOnFailure(reader.Get(this->nameSpan)); + this->SetName(this->nameSpan); + + // Putting a null terminator + ReturnErrorOnFailure(reader.Next(TagSceneTransitionTime())); } - // Putting a null terminator - ReturnErrorOnFailure(reader.Next(TagSceneTransitionTime())); ReturnErrorOnFailure(reader.Get(this->sceneTransitionTime)); ReturnErrorOnFailure(reader.Next(TagSceneTransitionTime100())); ReturnErrorOnFailure(reader.Get(this->transitionTime100)); @@ -177,21 +179,25 @@ class SceneTable return reader.ExitContainer(container); } - void SetName(const char * sceneName) + + void SetName(const CharSpan & sceneName) { - if (nullptr == sceneName) + if (nullptr == sceneName.data()) { - name[0] = 0; + name[0] = 0; + nameLength = 0; } else { Platform::CopyString(name, sceneName); + nameLength = sceneName.size(); } + nameSpan = CharSpan(name, nameLength); } void Clear() { - this->SetName(nullptr); + this->SetName(CharSpan()); sceneTransitionTime = 0; transitionTime100 = 0; extentsionFieldsSets.Clear(); @@ -199,14 +205,14 @@ class SceneTable bool operator==(const SceneData & other) { - return (!strncmp(this->name, other.name, kSceneNameMax) && (this->sceneTransitionTime == other.sceneTransitionTime) && + return (this->nameSpan.data_equal(other.nameSpan) && (this->sceneTransitionTime == other.sceneTransitionTime) && (this->transitionTime100 == other.transitionTime100) && (this->extentsionFieldsSets == other.extentsionFieldsSets)); } void operator=(const SceneData & other) { - this->SetName(other.name); + this->SetName(other.nameSpan); this->extentsionFieldsSets = other.extentsionFieldsSets; this->sceneTransitionTime = other.sceneTransitionTime; this->transitionTime100 = other.transitionTime100; diff --git a/src/app/clusters/scenes/SceneTableImpl.cpp b/src/app/clusters/scenes/SceneTableImpl.cpp index 0ef66965c42c0f..c3c20d0c749dfe 100644 --- a/src/app/clusters/scenes/SceneTableImpl.cpp +++ b/src/app/clusters/scenes/SceneTableImpl.cpp @@ -273,6 +273,7 @@ struct SceneTableData : public SceneTableEntry, PersistentDataSetSceneTableEntry(kFabric1, scene9)); // Not Found - NL_TEST_ASSERT(aSuite, CHIP_ERROR_NOT_FOUND == sceneTable->SetSceneTableEntry(kFabric1, scene9)); + NL_TEST_ASSERT(aSuite, CHIP_ERROR_NOT_FOUND == sceneTable->GetSceneTableEntry(kFabric1, sceneId9, scene)); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == sceneTable->GetSceneTableEntry(kFabric1, sceneId1, scene)); NL_TEST_ASSERT(aSuite, scene == scene1); @@ -194,7 +195,7 @@ void TestRemoveScenes(nlTestSuite * aSuite, void * aContext) auto * iterator = sceneTable->IterateSceneEntry(kFabric1); NL_TEST_ASSERT(aSuite, iterator->Count() == 7); NL_TEST_ASSERT(aSuite, iterator->Next(scene)); - NL_TEST_ASSERT(aSuite, scene == scene1); + NL_TEST_ASSERT(aSuite, scene == scene10); iterator->Release(); // Remove first @@ -206,6 +207,10 @@ void TestRemoveScenes(nlTestSuite * aSuite, void * aContext) // Remove Next NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == sceneTable->RemoveSceneTableEntry(kFabric1, scene3.storageId)); + iterator = sceneTable->IterateSceneEntry(kFabric1); + NL_TEST_ASSERT(aSuite, iterator->Count() == 5); + NL_TEST_ASSERT(aSuite, iterator->Next(scene)); + NL_TEST_ASSERT(aSuite, scene == scene2); NL_TEST_ASSERT(aSuite, iterator->Next(scene)); NL_TEST_ASSERT(aSuite, scene == scene4); @@ -229,9 +234,9 @@ void TestRemoveScenes(nlTestSuite * aSuite, void * aContext) NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == sceneTable->RemoveSceneTableEntry(kFabric1, scene7.storageId)); iterator = sceneTable->IterateSceneEntry(kFabric1); - NL_TEST_ASSERT(aSuite, iterator->Count() == 2); + NL_TEST_ASSERT(aSuite, iterator->Count() == 1); NL_TEST_ASSERT(aSuite, iterator->Next(scene)); - NL_TEST_ASSERT(aSuite, scene == scene8); + NL_TEST_ASSERT(aSuite, scene == scene12); // Remove last NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == sceneTable->RemoveSceneTableEntry(kFabric1, scene8.storageId)); @@ -243,7 +248,6 @@ void TestRemoveScenes(nlTestSuite * aSuite, void * aContext) iterator = sceneTable->IterateSceneEntry(kFabric1); NL_TEST_ASSERT(aSuite, iterator->Count() == 0); - auto * iterator = sceneTable->IterateSceneEntry(kFabric1); } void TestFabricScenes(nlTestSuite * aSuite, void * aContext) @@ -304,14 +308,32 @@ void TestFabricScenes(nlTestSuite * aSuite, void * aContext) } // namespace +/** + * Tear down the test suite. + */ int TestSetup(void * inContext) { VerifyOrReturnError(CHIP_NO_ERROR == chip::Platform::MemoryInit(), FAILURE); VerifyOrReturnError(CHIP_NO_ERROR == sSceneTable.Init(&testStorage), FAILURE); SetSceneTable(&sSceneTable); + + return SUCCESS; } +/** + * Tear down the test suite. + */ +int TestTeardown(void * inContext) +{ + SceneTableImpl * sceneTable = chip::scenes::GetSceneTable(); + if (nullptr != sceneTable) + { + sceneTable->Finish(); + } + chip::Platform::MemoryShutdown(); + return SUCCESS; +} int TestSceneTable() { static nlTest sTests[] = { @@ -323,9 +345,10 @@ int TestSceneTable() nlTestSuite theSuite = { "SceneTable", &sTests[0], - nullptr, - nullptr, + TestSetup, + TestTeardown, }; + nlTestRunner(&theSuite, nullptr); return (nlTestRunnerStats(&theSuite)); }