Skip to content

Commit

Permalink
Addressed comments, fixed TLV structures, removed redundant checks
Browse files Browse the repository at this point in the history
  • Loading branch information
@lpbeliveau-silabs @pull
lpbeliveau-silabs authored and pull[bot] committed Nov 27, 2023
1 parent fbbd882 commit 88b0677
Show file tree
Hide file tree
Showing 9 changed files with 171 additions and 111 deletions.
12 changes: 7 additions & 5 deletions src/app/clusters/scenes/ExtensionFieldSets.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,14 @@ class ExtensionFieldSets
ExtensionFieldSets(){};
virtual ~ExtensionFieldSets() = default;

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;
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;
/// @brief Gets a count of how many initialized fields sets are in the object
/// @return The number of initialized field sets 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
/// Clusters)
virtual uint8_t GetFieldSetCount() const = 0;
};
} // namespace scenes
Expand Down
39 changes: 18 additions & 21 deletions src/app/clusters/scenes/ExtensionFieldSetsImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,43 +22,40 @@ namespace scenes {

// ExtensionFieldSetsImpl::ExtensionFieldSetsImpl() : ExtensionFieldSets() {}

CHIP_ERROR ExtensionFieldSetsImpl::Serialize(TLV::TLVWriter & writer) const
CHIP_ERROR ExtensionFieldSetsImpl::Serialize(TLV::TLVWriter & writer, TLV::Tag structTag) const
{
TLV::TLVType container;
ReturnErrorOnFailure(
writer.StartContainer(TLV::ContextTag(TagEFS::kFieldSetArrayContainer), TLV::kTLVType_Structure, container));
ReturnErrorOnFailure(writer.Put(TLV::ContextTag(TagEFS::kFieldSetsCount), static_cast<uint8_t>(mFieldSetsCount)));
if (!IsEmpty())
ReturnErrorOnFailure(writer.StartContainer(structTag, TLV::kTLVType_Structure, container));
// ReturnErrorOnFailure(writer.Put(TLV::ContextTag(TagEFS::kFieldSetsCount), static_cast<uint8_t>(mFieldSetsCount)));
ReturnErrorOnFailure(writer.StartContainer(TLV::ContextTag(TagEFS::kFieldSetArrayContainer), TLV::kTLVType_Array, container));
for (uint8_t i = 0; i < mFieldSetsCount; i++)
{
for (uint8_t i = 0; i < mFieldSetsCount; i++)
{
if (!mFieldSets[i].IsEmpty())
{
ReturnErrorOnFailure(mFieldSets[i].Serialize(writer));
}
}
ReturnErrorOnFailure(mFieldSets[i].Serialize(writer));
}

return writer.EndContainer(container);
return writer.EndContainer(container);
}

CHIP_ERROR ExtensionFieldSetsImpl::Deserialize(TLV::TLVReader & reader)
CHIP_ERROR ExtensionFieldSetsImpl::Deserialize(TLV::TLVReader & reader, TLV::Tag structTag)
{
TLV::TLVType container;
ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Structure, TLV::ContextTag(TagEFS::kFieldSetArrayContainer)));
ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Structure, structTag));
ReturnErrorOnFailure(reader.EnterContainer(container));

ReturnErrorOnFailure(reader.Next(TLV::ContextTag(TagEFS::kFieldSetsCount)));
ReturnErrorOnFailure(reader.Get(mFieldSetsCount));
ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Array, TLV::ContextTag(TagEFS::kFieldSetArrayContainer)));
ReturnErrorOnFailure(reader.EnterContainer(container));

if (!this->IsEmpty())
uint8_t i = 0;
CHIP_ERROR err;
while ((err = reader.Next(TLV::AnonymousTag())) == CHIP_NO_ERROR && i < kMaxClustersPerScene)
{
for (uint8_t i = 0; i < mFieldSetsCount; i++)
{
ReturnErrorOnFailure(mFieldSets[i].Deserialize(reader));
}
ReturnErrorOnFailure(mFieldSets[i].Deserialize(reader));
i++;
}
mFieldSetsCount = i;

VerifyOrReturnError(err == CHIP_END_OF_TLV, err);
return reader.ExitContainer(container);
}

Expand Down
26 changes: 15 additions & 11 deletions src/app/clusters/scenes/ExtensionFieldSetsImpl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,12 @@ namespace chip {
namespace scenes {

/// @brief Tags Used to serialize Extension Field Sets struct as well as individual field sets.
/// kArrayContainer: Tag for the container of the Struct with the EFS array
/// kFieldSetsCount: Tag representing the number of individual field sets
/// kIndividualContainer: Tag for the container of single EFS struct
/// kFieldSetArrayContainer: Tag for the container of the EFS array
/// kClusterID: Tag for the ClusterID of a field set
/// kBufferBytes: Tag for the serialized field set data
/// kClusterFieldSetData: Tag for the serialized field set data
enum class TagEFS : uint8_t
{
kFieldSetArrayContainer = 1,
kFieldSetsCount,
kIndividualContainer,
kClusterID,
kClusterFieldSetData,
};
Expand Down Expand Up @@ -78,8 +74,7 @@ struct ExtensionFieldSet
CHIP_ERROR Serialize(TLV::TLVWriter & writer) const
{
TLV::TLVType container;
ReturnErrorOnFailure(
writer.StartContainer(TLV::ContextTag(TagEFS::kIndividualContainer), TLV::kTLVType_Structure, container));
ReturnErrorOnFailure(writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Structure, container));

ReturnErrorOnFailure(writer.Put(TLV::ContextTag(TagEFS::kClusterID), mID));
ReturnErrorOnFailure(writer.PutBytes(TLV::ContextTag(TagEFS::kClusterFieldSetData), mBytesBuffer, mUsedBytes));
Expand All @@ -91,7 +86,6 @@ struct ExtensionFieldSet
{
ByteSpan buffer;
TLV::TLVType container;
ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Structure, TLV::ContextTag(TagEFS::kIndividualContainer)));
ReturnErrorOnFailure(reader.EnterContainer(container));

ReturnErrorOnFailure(reader.Next(TLV::ContextTag(TagEFS::kClusterID)));
Expand Down Expand Up @@ -128,8 +122,8 @@ class ExtensionFieldSetsImpl : public ExtensionFieldSets
~ExtensionFieldSetsImpl() override{};

// overrides
CHIP_ERROR Serialize(TLV::TLVWriter & writer) const override;
CHIP_ERROR Deserialize(TLV::TLVReader & reader) override;
CHIP_ERROR Serialize(TLV::TLVWriter & writer, TLV::Tag structTag) const override;
CHIP_ERROR Deserialize(TLV::TLVReader & reader, TLV::Tag structTag) override;
void Clear() override;
bool IsEmpty() const override { return (mFieldSetsCount == 0); }
uint8_t GetFieldSetCount() const override { return mFieldSetsCount; };
Expand All @@ -141,6 +135,11 @@ class ExtensionFieldSetsImpl : public ExtensionFieldSets
// implementation
bool operator==(const ExtensionFieldSetsImpl & other) const
{
if (this->mFieldSetsCount != other.mFieldSetsCount)
{
return false;
}

for (uint8_t i = 0; i < mFieldSetsCount; i++)
{
if (!(this->mFieldSets[i] == other.mFieldSets[i]))
Expand All @@ -159,6 +158,11 @@ class ExtensionFieldSetsImpl : public ExtensionFieldSets
}
mFieldSetsCount = other.mFieldSetsCount;

for (uint8_t i = mFieldSetsCount; i < kMaxClustersPerScene; i++)
{
this->mFieldSets[i].Clear();
}

return *this;
}

Expand Down
8 changes: 7 additions & 1 deletion src/app/clusters/scenes/SceneTable.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ class SceneHandler : public IntrusiveListNodeBase<>
/// @param cluster[out] Cluster in the Extension field set, filled by the function
/// @param serialisedBytes[out] Buffer to fill from the ExtensionFieldSet in command
/// @return CHIP_NO_ERROR if successful, CHIP_ERROR value otherwise
/// @note Only gets called after the scene-cluster has previously verified that the endpoint,cluster valuer pair is supported by
/// the handler. It is therefore the implementation's reponsibility to also implement the SupportsCluster method.
virtual CHIP_ERROR SerializeAdd(EndpointId endpoint,
const app::Clusters::Scenes::Structs::ExtensionFieldSet::DecodableType & extensionFieldSet,
ClusterId & cluster, MutableByteSpan & serialisedBytes) = 0;
Expand All @@ -103,11 +105,13 @@ class SceneHandler : public IntrusiveListNodeBase<>
/// @brief Deserialize an ExtensionFieldSet into a cluster object (e.g. when handling ViewScene).
///
/// @param endpoint[in] Endpoint ID
/// @param cluster[in] Cluster ID to save
/// @param cluster[in] Cluster ID
/// @param serializedBytes[in] ExtensionFieldSet stored in NVM
///
/// @param extensionFieldSet[out] ExtensionFieldSet in command format
/// @return CHIP_NO_ERROR if successful, CHIP_ERROR value otherwise
/// @note Only gets called after the scene-cluster has previously verified that the endpoint,cluster valuer pair is supported by
/// the handler. It is therefore the implementation's reponsibility to also implement the SupportsCluster method.
virtual CHIP_ERROR Deserialize(EndpointId endpoint, ClusterId cluster, const ByteSpan & serializedBytes,

app::Clusters::Scenes::Structs::ExtensionFieldSet::Type & extensionFieldSet) = 0;
Expand Down Expand Up @@ -149,6 +153,8 @@ class SceneTable
mSceneId = kUndefinedSceneId;
}

bool IsValid() { return (mEndpointId != kInvalidEndpointId) && (mSceneId != kUndefinedSceneId); }

bool operator==(const SceneStorageId & other)
{
return (mEndpointId == other.mEndpointId && mGroupId == other.mGroupId && mSceneId == other.mSceneId);
Expand Down
70 changes: 46 additions & 24 deletions src/app/clusters/scenes/SceneTableImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,24 @@
namespace chip {
namespace scenes {

/// @brief Tags Used to serialize Scene specific Fabric data
/// kSceneCount: Tag for the number of scenes in Fabric
/// kGroupID: Tag for the next Fabric
enum class TagSceneImpl : uint8_t
/// @brief Tags Used to serialize Scenes so they can be stored in flash memory.
/// kSceneCount: Number of scene in a Fabric
/// kStorageIDArray: Array of StorageID struct
/// kEndpointID: Tag for the Endpoint ID to which this scene applies to
/// kGroupID: Tag for GroupID if the Scene is a Group Scene
/// kSceneID: Tag for the scene ID together with the two previous tag, forms the SceneStorageID
/// kName: Tag for the name of the scene
/// kTransitionTime: Tag for the transition time of the scene in miliseconds
enum class TagScene : uint8_t
{
kSceneCount = 1,
kStorageIDArray,
kEndpointID,
kGroupID,
kSceneID,
kName,
kTransitionTimeMs,
kExtensionFieldSetsContainer,
};

using SceneTableEntry = DefaultSceneTableImpl::SceneTableEntry;
Expand Down Expand Up @@ -68,12 +80,12 @@ struct SceneTableData : public SceneTableEntry, PersistentData<kPersistentSceneB
{
CharSpan nameSpan(mStorageData.mName, mStorageData.mNameLength);
TLV::TLVType container;
ReturnErrorOnFailure(writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Structure, container));
LogErrorOnFailure(writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Structure, container));

// Scene ID
ReturnErrorOnFailure(writer.Put(TLV::ContextTag(TagScene::kEndpointID), mStorageId.mEndpointId));
ReturnErrorOnFailure(writer.Put(TLV::ContextTag(TagScene::kGroupID), mStorageId.mGroupId));
ReturnErrorOnFailure(writer.Put(TLV::ContextTag(TagScene::kID), mStorageId.mSceneId));
ReturnErrorOnFailure(writer.Put(TLV::ContextTag(TagScene::kSceneID), mStorageId.mSceneId));

// Scene Data
// A length of 0 means the name wasn't used so it won't get stored
Expand All @@ -83,18 +95,15 @@ struct SceneTableData : public SceneTableEntry, PersistentData<kPersistentSceneB
}

ReturnErrorOnFailure(writer.Put(TLV::ContextTag(TagScene::kTransitionTimeMs), mStorageData.mSceneTransitionTimeMs));
ReturnErrorOnFailure(mStorageData.mExtensionFieldSets.Serialize(writer));
ReturnErrorOnFailure(
mStorageData.mExtensionFieldSets.Serialize(writer, TLV::ContextTag(TagScene::kExtensionFieldSetsContainer)));

return writer.EndContainer(container);
}

CHIP_ERROR Deserialize(TLV::TLVReader & reader) override
{
char nameBuffer[kSceneNameMaxLength] = { 0 };
CharSpan nameSpan(nameBuffer);

ReturnErrorOnFailure(reader.Next(TLV::AnonymousTag()));
VerifyOrReturnError(TLV::kTLVType_Structure == reader.GetType(), CHIP_ERROR_INTERNAL);
ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Structure, TLV::AnonymousTag()));

TLV::TLVType container;
ReturnErrorOnFailure(reader.EnterContainer(container));
Expand All @@ -104,7 +113,7 @@ struct SceneTableData : public SceneTableEntry, PersistentData<kPersistentSceneB
ReturnErrorOnFailure(reader.Get(mStorageId.mEndpointId));
ReturnErrorOnFailure(reader.Next(TLV::ContextTag(TagScene::kGroupID)));
ReturnErrorOnFailure(reader.Get(mStorageId.mGroupId));
ReturnErrorOnFailure(reader.Next(TLV::ContextTag(TagScene::kID)));
ReturnErrorOnFailure(reader.Next(TLV::ContextTag(TagScene::kSceneID)));
ReturnErrorOnFailure(reader.Get(mStorageId.mSceneId));

// Scene Data
Expand All @@ -113,16 +122,19 @@ struct SceneTableData : public SceneTableEntry, PersistentData<kPersistentSceneB
VerifyOrReturnError(TLV::ContextTag(TagScene::kName) == currTag || TLV::ContextTag(TagScene::kTransitionTimeMs) == currTag,
CHIP_ERROR_WRONG_TLV_TYPE);

CharSpan nameSpan;
// A name may or may not have been stored. Check whether it was.
if (currTag == TLV::ContextTag(TagScene::kName))
{
ReturnErrorOnFailure(reader.Get(nameSpan));
mStorageData.SetName(nameSpan);
ReturnErrorOnFailure(reader.Next(TLV::ContextTag(TagScene::kTransitionTimeMs)));
}
// Empty name will be initialized if the name wasn't stored
mStorageData.SetName(nameSpan);

ReturnErrorOnFailure(reader.Get(mStorageData.mSceneTransitionTimeMs));
ReturnErrorOnFailure(mStorageData.mExtensionFieldSets.Deserialize(reader));
ReturnErrorOnFailure(
mStorageData.mExtensionFieldSets.Deserialize(reader, TLV::ContextTag(TagScene::kExtensionFieldSetsContainer)));

return reader.ExitContainer(container);
}
Expand Down Expand Up @@ -166,16 +178,19 @@ struct FabricSceneData : public PersistentData<kPersistentFabricBufferMax>
{
TLV::TLVType container;
ReturnErrorOnFailure(writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Structure, container));
ReturnErrorOnFailure(writer.Put(TLV::ContextTag(TagScene::kSceneCount), scene_count));
ReturnErrorOnFailure(writer.StartContainer(TLV::ContextTag(TagScene::kStorageIDArray), TLV::kTLVType_Array, container));

ReturnErrorOnFailure(writer.Put(TLV::ContextTag(TagSceneImpl::kSceneCount), scene_count));
// Storing the scene map
for (uint8_t i = 0; i < kMaxScenesPerFabric; i++)
{
ReturnErrorOnFailure(writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Structure, container));
ReturnErrorOnFailure(writer.Put(TLV::ContextTag(TagScene::kEndpointID), (scene_map[i].mEndpointId)));
ReturnErrorOnFailure(writer.Put(TLV::ContextTag(TagScene::kGroupID), (scene_map[i].mGroupId)));
ReturnErrorOnFailure(writer.Put(TLV::ContextTag(TagScene::kID), (scene_map[i].mSceneId)));
ReturnErrorOnFailure(writer.Put(TLV::ContextTag(TagScene::kSceneID), (scene_map[i].mSceneId)));
ReturnErrorOnFailure(writer.EndContainer(container));
}

ReturnErrorOnFailure(writer.EndContainer(container));
return writer.EndContainer(container);
}

Expand All @@ -186,21 +201,29 @@ struct FabricSceneData : public PersistentData<kPersistentFabricBufferMax>
TLV::TLVType container;
ReturnErrorOnFailure(reader.EnterContainer(container));

ReturnErrorOnFailure(reader.Next(TLV::ContextTag(TagSceneImpl::kSceneCount)));
ReturnErrorOnFailure(reader.Next(TLV::ContextTag(TagScene::kSceneCount)));
ReturnErrorOnFailure(reader.Get(scene_count));
ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Array, TLV::ContextTag(TagScene::kStorageIDArray)));
ReturnErrorOnFailure(reader.EnterContainer(container));

uint8_t i = 0;
while (reader.Next(TLV::ContextTag(TagScene::kEndpointID)) != CHIP_END_OF_TLV)
CHIP_ERROR err;
while ((err = reader.Next(TLV::AnonymousTag())) == CHIP_NO_ERROR && i < kMaxScenesPerFabric)
{
ReturnErrorOnFailure(reader.EnterContainer(container));
ReturnErrorOnFailure(reader.Next(TLV::ContextTag(TagScene::kEndpointID)));
ReturnErrorOnFailure(reader.Get(scene_map[i].mEndpointId));
ReturnErrorOnFailure(reader.Next(TLV::ContextTag(TagScene::kGroupID)));
ReturnErrorOnFailure(reader.Get(scene_map[i].mGroupId));
ReturnErrorOnFailure(reader.Next(TLV::ContextTag(TagScene::kID)));
ReturnErrorOnFailure(reader.Next(TLV::ContextTag(TagScene::kSceneID)));
ReturnErrorOnFailure(reader.Get(scene_map[i].mSceneId));
ReturnErrorOnFailure(reader.ExitContainer(container));

i++;
}
VerifyOrReturnError(err == CHIP_END_OF_TLV, err);

ReturnErrorOnFailure(reader.ExitContainer(container));
return reader.ExitContainer(container);
}

Expand All @@ -222,7 +245,7 @@ struct FabricSceneData : public PersistentData<kPersistentFabricBufferMax>
idx = index;
return CHIP_NO_ERROR; // return scene at current index if scene found
}
if (scene_map[index].mEndpointId == kInvalidEndpointId && firstFreeIdx == kUndefinedSceneIndex)
if (!scene_map[index].IsValid() && firstFreeIdx == kUndefinedSceneIndex)
{
firstFreeIdx = index;
}
Expand Down Expand Up @@ -454,7 +477,6 @@ CHIP_ERROR DefaultSceneTableImpl::SceneApplyEFS(const SceneTableEntry & scene)

if (!EFS.IsEmpty())
{
IntrusiveList<SceneHandler>::Iterator iter = mHandlerList.begin();
for (auto & handler : mHandlerList)
{
if (handler.SupportsCluster(scene.mStorageId.mEndpointId, cluster))
Expand Down Expand Up @@ -525,7 +547,7 @@ bool DefaultSceneTableImpl::SceneEntryIteratorImpl::Next(SceneTableEntry & outpu
// looks for next available scene
while (mSceneIndex < kMaxScenesPerFabric)
{
if (fabric.scene_map[mSceneIndex].mEndpointId != kInvalidEndpointId)
if (fabric.scene_map[mSceneIndex].IsValid())
{
scene.index = mSceneIndex;
VerifyOrReturnError(scene.Load(mProvider.mStorage) == CHIP_NO_ERROR, false);
Expand Down
Loading

0 comments on commit 88b0677

Please sign in to comment.