From a7cbd5b0c32f2653ad2399f0362ac808a1f122ff Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Mon, 27 Nov 2023 12:27:22 -0500 Subject: [PATCH 1/5] Removed un-necessaery containter in extension field set and changed comment about max size tested given past optimisation --- .../scenes-server/ExtensionFieldSetsImpl.cpp | 12 ++---------- src/app/clusters/scenes-server/SceneTableImpl.cpp | 2 +- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/src/app/clusters/scenes-server/ExtensionFieldSetsImpl.cpp b/src/app/clusters/scenes-server/ExtensionFieldSetsImpl.cpp index 489e6788be77f9..08eff27fe54c65 100644 --- a/src/app/clusters/scenes-server/ExtensionFieldSetsImpl.cpp +++ b/src/app/clusters/scenes-server/ExtensionFieldSetsImpl.cpp @@ -24,8 +24,6 @@ namespace scenes { CHIP_ERROR ExtensionFieldSetsImpl::Serialize(TLV::TLVWriter & writer, TLV::Tag structTag) const { - TLV::TLVType structureContainer; - ReturnErrorOnFailure(writer.StartContainer(structTag, TLV::kTLVType_Structure, structureContainer)); TLV::TLVType arrayContainer; ReturnErrorOnFailure( writer.StartContainer(TLV::ContextTag(TagEFS::kFieldSetArrayContainer), TLV::kTLVType_Array, arrayContainer)); @@ -34,16 +32,11 @@ CHIP_ERROR ExtensionFieldSetsImpl::Serialize(TLV::TLVWriter & writer, TLV::Tag s ReturnErrorOnFailure(mFieldSets[i].Serialize(writer)); } - ReturnErrorOnFailure(writer.EndContainer(arrayContainer)); - return writer.EndContainer(structureContainer); + return writer.EndContainer(arrayContainer); } CHIP_ERROR ExtensionFieldSetsImpl::Deserialize(TLV::TLVReader & reader, TLV::Tag structTag) { - TLV::TLVType structureContainer; - ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Structure, structTag)); - ReturnErrorOnFailure(reader.EnterContainer(structureContainer)); - TLV::TLVType arrayContainer; ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Array, TLV::ContextTag(TagEFS::kFieldSetArrayContainer))); ReturnErrorOnFailure(reader.EnterContainer(arrayContainer)); @@ -69,8 +62,7 @@ CHIP_ERROR ExtensionFieldSetsImpl::Deserialize(TLV::TLVReader & reader, TLV::Tag return err; } - ReturnErrorOnFailure(reader.ExitContainer(arrayContainer)); - return reader.ExitContainer(structureContainer); + return reader.ExitContainer(arrayContainer); } void ExtensionFieldSetsImpl::Clear() diff --git a/src/app/clusters/scenes-server/SceneTableImpl.cpp b/src/app/clusters/scenes-server/SceneTableImpl.cpp index 19dd0dc79d1b18..352a2035d15fef 100644 --- a/src/app/clusters/scenes-server/SceneTableImpl.cpp +++ b/src/app/clusters/scenes-server/SceneTableImpl.cpp @@ -99,7 +99,7 @@ struct EndpointSceneCount : public PersistentData Date: Mon, 27 Nov 2023 16:01:16 -0500 Subject: [PATCH 2/5] Removed the tag from ExtensionFieldSetsImpl.Serialize() signature --- src/app/clusters/scenes-server/ExtensionFieldSets.h | 8 ++++---- src/app/clusters/scenes-server/ExtensionFieldSetsImpl.cpp | 4 ++-- src/app/clusters/scenes-server/ExtensionFieldSetsImpl.h | 4 ++-- src/app/clusters/scenes-server/SceneTableImpl.cpp | 7 ++----- src/app/tests/TestExtensionFieldSets.cpp | 4 ++-- 5 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/app/clusters/scenes-server/ExtensionFieldSets.h b/src/app/clusters/scenes-server/ExtensionFieldSets.h index e05525e0586f02..d926ce3bbcd9c2 100644 --- a/src/app/clusters/scenes-server/ExtensionFieldSets.h +++ b/src/app/clusters/scenes-server/ExtensionFieldSets.h @@ -34,10 +34,10 @@ class ExtensionFieldSets ExtensionFieldSets(){}; virtual ~ExtensionFieldSets() = default; - virtual CHIP_ERROR Serialize(TLV::TLVWriter & writer, TLV::Tag structTa) const = 0; - virtual CHIP_ERROR Deserialize(TLV::TLVReader & reader, TLV::Tag structTa) = 0; - virtual void Clear() = 0; - virtual bool IsEmpty() const = 0; + virtual CHIP_ERROR Serialize(TLV::TLVWriter & writer) const = 0; + virtual CHIP_ERROR Deserialize(TLV::TLVReader & reader) = 0; + virtual void Clear() = 0; + virtual bool IsEmpty() const = 0; /// @brief Gets a count of how many initialized fields sets are in the object /// @return The number of initialized field sets the object /// @note Field set refers to extension field sets, from the scene cluster (see 1.4.6.2 ExtensionFieldSet in Matter Application diff --git a/src/app/clusters/scenes-server/ExtensionFieldSetsImpl.cpp b/src/app/clusters/scenes-server/ExtensionFieldSetsImpl.cpp index 08eff27fe54c65..6e9170e1fd061b 100644 --- a/src/app/clusters/scenes-server/ExtensionFieldSetsImpl.cpp +++ b/src/app/clusters/scenes-server/ExtensionFieldSetsImpl.cpp @@ -22,7 +22,7 @@ namespace scenes { // ExtensionFieldSetsImpl::ExtensionFieldSetsImpl() : ExtensionFieldSets() {} -CHIP_ERROR ExtensionFieldSetsImpl::Serialize(TLV::TLVWriter & writer, TLV::Tag structTag) const +CHIP_ERROR ExtensionFieldSetsImpl::Serialize(TLV::TLVWriter & writer) const { TLV::TLVType arrayContainer; ReturnErrorOnFailure( @@ -35,7 +35,7 @@ CHIP_ERROR ExtensionFieldSetsImpl::Serialize(TLV::TLVWriter & writer, TLV::Tag s return writer.EndContainer(arrayContainer); } -CHIP_ERROR ExtensionFieldSetsImpl::Deserialize(TLV::TLVReader & reader, TLV::Tag structTag) +CHIP_ERROR ExtensionFieldSetsImpl::Deserialize(TLV::TLVReader & reader) { TLV::TLVType arrayContainer; ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Array, TLV::ContextTag(TagEFS::kFieldSetArrayContainer))); diff --git a/src/app/clusters/scenes-server/ExtensionFieldSetsImpl.h b/src/app/clusters/scenes-server/ExtensionFieldSetsImpl.h index 381c592941b079..50b92878daf1ed 100644 --- a/src/app/clusters/scenes-server/ExtensionFieldSetsImpl.h +++ b/src/app/clusters/scenes-server/ExtensionFieldSetsImpl.h @@ -122,8 +122,8 @@ class ExtensionFieldSetsImpl : public ExtensionFieldSets ~ExtensionFieldSetsImpl() override{}; // overrides - CHIP_ERROR Serialize(TLV::TLVWriter & writer, TLV::Tag structTag) const override; - CHIP_ERROR Deserialize(TLV::TLVReader & reader, TLV::Tag structTag) override; + CHIP_ERROR Serialize(TLV::TLVWriter & writer) const override; + CHIP_ERROR Deserialize(TLV::TLVReader & reader) override; void Clear() override; bool IsEmpty() const override { return (mFieldSetsCount == 0); } uint8_t GetFieldSetCount() const override { return mFieldSetsCount; }; diff --git a/src/app/clusters/scenes-server/SceneTableImpl.cpp b/src/app/clusters/scenes-server/SceneTableImpl.cpp index 352a2035d15fef..477e2db5fda067 100644 --- a/src/app/clusters/scenes-server/SceneTableImpl.cpp +++ b/src/app/clusters/scenes-server/SceneTableImpl.cpp @@ -39,7 +39,6 @@ enum class TagScene : uint8_t kSceneID, kName, kTransitionTimeMs, - kExtensionFieldSetsContainer, }; using SceneTableEntry = DefaultSceneTableImpl::SceneTableEntry; @@ -150,8 +149,7 @@ struct SceneTableData : public SceneTableEntry, PersistentDataSerialize(writer, TLV::ContextTag(TagTestEFS::kEFS))); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == EFS->Serialize(writer)); writer.EndContainer(outer); sceneEFS_serialized_length = writer.GetLengthWritten(); NL_TEST_ASSERT(aSuite, sceneEFS_serialized_length <= kPersistentSceneBufferMax); @@ -243,7 +243,7 @@ void TestSerializeDerializeExtensionFieldSet(nlTestSuite * aSuite, void * aConte reader.Init(sceneEFSBuffer); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == reader.Next()); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == reader.EnterContainer(outerRead)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == testSceneEFS.Deserialize(reader, TLV::ContextTag(TagTestEFS::kEFS))); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == testSceneEFS.Deserialize(reader)); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == reader.ExitContainer(outerRead)); NL_TEST_ASSERT(aSuite, *EFS == testSceneEFS); From 6a634b768843c8f54d8721c034cb5f9f1fa72807 Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Tue, 28 Nov 2023 12:21:06 -0500 Subject: [PATCH 3/5] Removed logging of empty efs array --- .../scenes-server/ExtensionFieldSetsImpl.cpp | 2 -- .../clusters/scenes-server/SceneTableImpl.cpp | 18 +++++++++++++----- src/lib/core/CHIPConfig.h | 16 ++++++++-------- 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/app/clusters/scenes-server/ExtensionFieldSetsImpl.cpp b/src/app/clusters/scenes-server/ExtensionFieldSetsImpl.cpp index 6e9170e1fd061b..4ad9618158177b 100644 --- a/src/app/clusters/scenes-server/ExtensionFieldSetsImpl.cpp +++ b/src/app/clusters/scenes-server/ExtensionFieldSetsImpl.cpp @@ -20,8 +20,6 @@ namespace chip { namespace scenes { -// ExtensionFieldSetsImpl::ExtensionFieldSetsImpl() : ExtensionFieldSets() {} - CHIP_ERROR ExtensionFieldSetsImpl::Serialize(TLV::TLVWriter & writer) const { TLV::TLVType arrayContainer; diff --git a/src/app/clusters/scenes-server/SceneTableImpl.cpp b/src/app/clusters/scenes-server/SceneTableImpl.cpp index 477e2db5fda067..24619eccc8428b 100644 --- a/src/app/clusters/scenes-server/SceneTableImpl.cpp +++ b/src/app/clusters/scenes-server/SceneTableImpl.cpp @@ -98,7 +98,7 @@ struct EndpointSceneCount : public PersistentData Date: Tue, 28 Nov 2023 15:01:53 -0500 Subject: [PATCH 4/5] Update src/lib/core/CHIPConfig.h Co-authored-by: Boris Zbarsky --- src/lib/core/CHIPConfig.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/core/CHIPConfig.h b/src/lib/core/CHIPConfig.h index f4833279c5b88d..fc1a67b603bb31 100644 --- a/src/lib/core/CHIPConfig.h +++ b/src/lib/core/CHIPConfig.h @@ -1473,7 +1473,7 @@ extern const char CHIP_NON_PRODUCTION_MARKER[]; * } * * Including all the TLV fields, the following values can help estimate the needed size for a scenes given a number of clusters: - * Empty EFS Scene Max name size: 34bytes + * Empty EFS Scene Max name size: 34 bytes * Scene Max name size + OnOff : 55 bytes * Scene Max name size + LevelControl : 64 bytes * Scene Max name size + ColorControl : 130 bytes From 60146afbbdc8749206c3c506490345a73eac5359 Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Tue, 28 Nov 2023 15:47:38 -0500 Subject: [PATCH 5/5] Remove the conditionnal storage of EFS --- src/app/clusters/scenes-server/SceneTableImpl.cpp | 12 ++---------- src/lib/core/CHIPConfig.h | 2 +- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/src/app/clusters/scenes-server/SceneTableImpl.cpp b/src/app/clusters/scenes-server/SceneTableImpl.cpp index 24619eccc8428b..a0cc24a8e4516e 100644 --- a/src/app/clusters/scenes-server/SceneTableImpl.cpp +++ b/src/app/clusters/scenes-server/SceneTableImpl.cpp @@ -149,11 +149,8 @@ struct SceneTableData : public SceneTableEntry, PersistentData