Skip to content

Commit

Permalink
Refactored sceneData to support logging without name, added test func…
Browse files Browse the repository at this point in the history 
…tionnal with test unit
  • Loading branch information
@lpbeliveau-silabs @pull
lpbeliveau-silabs authored and pull[bot] committed Jul 24, 2023
1 parent 3f94333 commit 1089650
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 44 deletions.
52 changes: 29 additions & 23 deletions src/app/clusters/scenes/SceneTable.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <lib/support/CHIPMemString.h>
#include <lib/support/CommonIterator.h>
#include <lib/support/CommonPersistentData.h>
#include <lib/support/Span.h>

namespace chip {
namespace scenes {
Expand Down Expand Up @@ -108,39 +109,41 @@ 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
{
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<uint32_t>(name_size)));
ReturnErrorOnFailure(writer.PutString(TagSceneName(), this->nameSpan));
}

ReturnErrorOnFailure(writer.Put(TagSceneTransitionTime(), static_cast<uint16_t>(this->sceneTransitionTime)));
Expand All @@ -162,51 +165,54 @@ 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));
ReturnErrorOnFailure(this->extentsionFieldsSets.Deserialize(reader));

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();
}

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;
Expand Down
7 changes: 5 additions & 2 deletions src/app/clusters/scenes/SceneTableImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,15 +273,18 @@ struct SceneTableData : public SceneTableEntry, PersistentData<kPersistentBuffer
bool Find(PersistentStorageDelegate * storage, const FabricSceneData & fabric,
DefaultSceneTableImpl::SceneStorageId target_scene)
{
CHIP_ERROR err;
fabric_index = fabric.fabric_index;
storageId = fabric.first_scene;
index = 0;
first = true;

while (index < fabric.scene_count)
{
if (CHIP_NO_ERROR != Load(storage))
err = Load(storage);
if (err != CHIP_NO_ERROR)
{
LogErrorOnFailure(err);
break;
}
if (storageId == target_scene)
Expand Down Expand Up @@ -351,7 +354,7 @@ CHIP_ERROR DefaultSceneTableImpl::SetSceneTableEntry(FabricIndex fabric_index, c
ReturnErrorOnFailure(prev.Save(mStorage));
}

ReturnErrorOnFailure(fabric.Save(mStorage));
LogErrorOnFailure(fabric.Save(mStorage));

return scene.Save(mStorage);
}
Expand Down
1 change: 1 addition & 0 deletions src/app/tests/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ source_set("scenes-table-test-srcs") {
"${chip_root}/src/app/common:cluster-objects",
"${chip_root}/src/lib/core",
]
defines = [ "SCENES_TESTING_CONFIGURATION=1" ]
}

chip_test_suite("tests") {
Expand Down
61 changes: 42 additions & 19 deletions src/app/tests/TestSceneTable.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ using SceneTableEntry = chip::scenes::DefaultSceneTableImpl::SceneTableEntry;
using SceneTableImpl = chip::scenes::DefaultSceneTableImpl;
using SceneStorageId = chip::scenes::DefaultSceneTableImpl::SceneStorageId;
using SceneData = chip::scenes::DefaultSceneTableImpl::SceneData;
using CharSpan = chip::CharSpan;

namespace {

Expand All @@ -48,18 +49,18 @@ static const SceneStorageId sceneId8(1, 0xEE, 0x101);
static const SceneStorageId sceneId9(1, 0xAB, 0x101);

// Scene data
static const SceneData sceneData1("Scene #1");
static const SceneData sceneData2("Scene #2", 2, 5);
static const SceneData sceneData3("Scene #3", 25);
static const SceneData sceneData4("Scene num4", 5);
static const SceneData sceneData5(nullptr);
static const SceneData sceneData6("Scene #6", 3, 15);
static const SceneData sceneData7("Scene #7", 20, 5);
static const SceneData sceneData8("Scene num8", 10);
static const SceneData sceneData9("NAME TOO LOOONNG", 30, 15);
static const SceneData sceneData10("Scene #10", 10, 1);
static const SceneData sceneData11("Scene #11", 20, 10);
static const SceneData sceneData12("Scene #12", 30, 5);
static const SceneData sceneData1(CharSpan("Scene #1", sizeof("Scene #1")));
static const SceneData sceneData2(CharSpan("Scene #2", sizeof("Scene #2")), 2, 5);
static const SceneData sceneData3(CharSpan("Scene #3", sizeof("Scene #3")), 25);
static const SceneData sceneData4(CharSpan("Scene num4", sizeof("Scene num4")), 5);
static const SceneData sceneData5(CharSpan(), 10);
static const SceneData sceneData6(CharSpan("Scene #6", sizeof("Scene #6")), 3, 15);
static const SceneData sceneData7(CharSpan("Scene #7", sizeof("Scene #7")), 20, 5);
static const SceneData sceneData8(CharSpan("NAME TOO LOOONNG", sizeof("Scene num4")), 10);
static const SceneData sceneData9(CharSpan("Scene #9", sizeof("Scene #9")), 30, 15);
static const SceneData sceneData10(CharSpan("Scene #10", sizeof("Scene #10")), 10, 1);
static const SceneData sceneData11(CharSpan("Scene #11", sizeof("Scene #11")), 20, 10);
static const SceneData sceneData12(CharSpan("Scene #12", sizeof("Scene #12")), 30, 5);

// Scenes
SceneTableEntry scene1(sceneId1, sceneData1);
Expand Down Expand Up @@ -105,7 +106,7 @@ void TestStoreScenes(nlTestSuite * aSuite, void * aContext)
NL_TEST_ASSERT(aSuite, CHIP_ERROR_INVALID_LIST_LENGTH == sceneTable->SetSceneTableEntry(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);
Expand Down Expand Up @@ -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
Expand All @@ -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);

Expand All @@ -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));
Expand All @@ -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)
Expand Down Expand Up @@ -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[] = {
Expand All @@ -323,9 +345,10 @@ int TestSceneTable()
nlTestSuite theSuite = {
"SceneTable",
&sTests[0],
nullptr,
nullptr,
TestSetup,
TestTeardown,
};

nlTestRunner(&theSuite, nullptr);
return (nlTestRunnerStats(&theSuite));
}
Expand Down

0 comments on commit 1089650

Please sign in to comment.