Skip to content

Commit

Permalink
Added unit test for ExtensionFieldSets and applied suggestions from c…
Browse files Browse the repository at this point in the history
…omments on PR

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
  • Loading branch information
lpbeliveau-silabs and bzbarsky-apple committed Mar 24, 2023
1 parent 1a2488d commit 01550b4
Show file tree
Hide file tree
Showing 12 changed files with 566 additions and 248 deletions.
11 changes: 7 additions & 4 deletions src/app/clusters/scenes/ExtensionFieldSets.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@
namespace chip {
namespace scenes {

static constexpr uint8_t kInvalidPosition = 0xff;
static constexpr uint8_t kMaxClusterPerScenes = CHIP_CONFIG_SCENES_MAX_CLUSTERS_PER_SCENES;
static constexpr uint8_t kMaxFieldsPerCluster = CHIP_CONFIG_SCENES_MAX_EXTENSION_FIELDSET_SIZE_PER_CLUSTER;
static constexpr uint8_t kInvalidPosition = 0xff;
static constexpr uint8_t kMaxClusterPerScenes = CHIP_CONFIG_SCENES_MAX_CLUSTERS_PER_SCENE;
static constexpr uint8_t kMaxFieldBytesPerCluster = CHIP_CONFIG_SCENES_MAX_EXTENSION_FIELDSET_SIZE_PER_CLUSTER;

/// @brief class meant serialize all extension ßfield sets of a scene so it can be stored and retrieved from flash memory.
class ExtensionFieldSets
{
public:
Expand All @@ -37,7 +38,9 @@ class ExtensionFieldSets
virtual CHIP_ERROR Deserialize(TLV::TLVReader & reader) = 0;
virtual void Clear() = 0;
virtual bool IsEmpty() const = 0;
virtual uint8_t GetFieldNum() 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
virtual uint8_t GetFieldSetCount() const = 0;
};
} // namespace scenes
} // namespace chip
73 changes: 34 additions & 39 deletions src/app/clusters/scenes/ExtensionFieldSetsImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,21 @@
namespace chip {
namespace scenes {

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

CHIP_ERROR ExtensionFieldSetsImpl::Serialize(TLV::TLVWriter & writer) const
{
TLV::TLVType container;
ReturnErrorOnFailure(writer.StartContainer(TLV::ContextTag(kTagEFSArrayContainer), TLV::kTLVType_Structure, container));
ReturnErrorOnFailure(writer.Put(TLV::ContextTag(kTagEFSFieldNum), static_cast<uint8_t>(this->mFieldNum)));
if (!this->IsEmpty())
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())
{
for (uint8_t i = 0; i < this->mFieldNum; i++)
for (uint8_t i = 0; i < mFieldSetsCount; i++)
{
if (!this->mEFS[i].IsEmpty())
if (!mEFS[i].IsEmpty())
{
ReturnErrorOnFailure(this->mEFS[i].Serialize(writer));
ReturnErrorOnFailure(mEFS[i].Serialize(writer));
}
}
}
Expand All @@ -44,17 +45,17 @@ CHIP_ERROR ExtensionFieldSetsImpl::Serialize(TLV::TLVWriter & writer) const
CHIP_ERROR ExtensionFieldSetsImpl::Deserialize(TLV::TLVReader & reader)
{
TLV::TLVType container;
ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Structure, TLV::ContextTag(kTagEFSArrayContainer)));
ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Structure, TLV::ContextTag(TagEFS::kFieldSetArrayContainer)));
ReturnErrorOnFailure(reader.EnterContainer(container));

ReturnErrorOnFailure(reader.Next(TLV::ContextTag(kTagEFSFieldNum)));
ReturnErrorOnFailure(reader.Get(this->mFieldNum));
ReturnErrorOnFailure(reader.Next(TLV::ContextTag(TagEFS::kFieldSetsCount)));
ReturnErrorOnFailure(reader.Get(mFieldSetsCount));

if (!this->IsEmpty())
{
for (uint8_t i = 0; i < this->mFieldNum; i++)
for (uint8_t i = 0; i < mFieldSetsCount; i++)
{
ReturnErrorOnFailure(this->mEFS[i].Deserialize(reader));
ReturnErrorOnFailure(mEFS[i].Deserialize(reader));
}
}

Expand All @@ -65,80 +66,74 @@ void ExtensionFieldSetsImpl::Clear()
{
if (!this->IsEmpty())
{
for (uint8_t i = 0; i < kMaxClusterPerScenes; i++)
for (uint8_t i = 0; i < mFieldSetsCount; i++)
{
this->mEFS[i].Clear();
mEFS[i].Clear();
}
}
this->mFieldNum = 0;
mFieldSetsCount = 0;
}

/// @brief Inserts a field Set set into the array of extension field Set sets for a scene entry.
/// If the same ID is present in the EFS array, it will overwrite it.
/// @param fieldSet field set to be inserted
/// @return CHIP_NO_ERROR if insertion worked, CHIP_ERROR_NO_MEMORY if the array is already full
CHIP_ERROR ExtensionFieldSetsImpl::InsertFieldSet(ExtensionFieldsSet & fieldSet)
CHIP_ERROR ExtensionFieldSetsImpl::InsertFieldSet(const ExtensionFieldSet & fieldSet)
{
CHIP_ERROR err = CHIP_ERROR_NO_MEMORY;
uint8_t idPosition = kInvalidPosition;
uint8_t firstEmptyPosition = kInvalidPosition;

VerifyOrReturnError(fieldSet.mID != kInvalidClusterId, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(!fieldSet.IsEmpty(), CHIP_ERROR_INVALID_ARGUMENT);

for (uint8_t i = 0; i < kMaxClusterPerScenes; i++)
{
if (this->mEFS[i].mID == fieldSet.mID)
if (mEFS[i].mID == fieldSet.mID)
{
idPosition = i;
break;
mEFS[i] = fieldSet;
return CHIP_NO_ERROR;
}

if (this->mEFS[i].IsEmpty() && firstEmptyPosition == kInvalidPosition)
if (mEFS[i].IsEmpty() && firstEmptyPosition == kInvalidPosition)
{
firstEmptyPosition = i;
}
}

// if found, replace at found position, otherwise at insert first free position, otherwise return error
if (idPosition < kMaxClusterPerScenes)
{
this->mEFS[idPosition] = fieldSet;
err = CHIP_NO_ERROR;
}
else if (firstEmptyPosition < kMaxClusterPerScenes)
if (firstEmptyPosition < kMaxClusterPerScenes)
{
this->mEFS[firstEmptyPosition] = fieldSet;
this->mFieldNum++;
err = CHIP_NO_ERROR;
mEFS[firstEmptyPosition] = fieldSet;
mFieldSetsCount++;
return CHIP_NO_ERROR;
}

return err;
return CHIP_ERROR_NO_MEMORY;
}

CHIP_ERROR ExtensionFieldSetsImpl::GetFieldSetAtPosition(ExtensionFieldsSet & fieldSet, uint8_t position)
CHIP_ERROR ExtensionFieldSetsImpl::GetFieldSetAtPosition(ExtensionFieldSet & fieldSet, uint8_t position)
{
VerifyOrReturnError(position < this->mFieldNum, CHIP_ERROR_BUFFER_TOO_SMALL);
VerifyOrReturnError(position < mFieldSetsCount, CHIP_ERROR_BUFFER_TOO_SMALL);

fieldSet = this->mEFS[position];
fieldSet = mEFS[position];

return CHIP_NO_ERROR;
}

CHIP_ERROR ExtensionFieldSetsImpl::RemoveFieldAtPosition(uint8_t position)
{
VerifyOrReturnError(position < kMaxClusterPerScenes, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnValue(!this->IsEmpty() && !this->mEFS[position].IsEmpty(), CHIP_NO_ERROR);
VerifyOrReturnValue(!this->IsEmpty() && !mEFS[position].IsEmpty(), CHIP_NO_ERROR);

uint8_t nextPos = static_cast<uint8_t>(position + 1);
uint8_t moveNum = static_cast<uint8_t>(kMaxClusterPerScenes - nextPos);

// TODO: Implement general array management methods
// Compress array after removal
memmove(&this->mEFS[position], &this->mEFS[nextPos], sizeof(ExtensionFieldsSet) * moveNum);
memmove(&mEFS[position], &mEFS[nextPos], sizeof(ExtensionFieldSet) * moveNum);

this->mFieldNum--;
mFieldSetsCount--;
// Clear last occupied position
this->mEFS[mFieldNum].Clear(); //
mEFS[mFieldSetsCount].Clear(); //

return CHIP_NO_ERROR;
}
Expand Down
109 changes: 63 additions & 46 deletions src/app/clusters/scenes/ExtensionFieldSetsImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,49 +23,66 @@
namespace chip {
namespace scenes {

enum EFSTLVTag
/// @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
/// kClusterID: Tag for the ClusterID of a field set
/// kBufferBytes: Tag for the serialized field set data
enum class TagEFS : uint8_t
{
kTagEFSArrayContainer = 1,
kTagEFSFieldNum = 1,
kTagEFSContainer,
kTagEFSClusterID,
kTagEFS,
kFieldSetArrayContainer = 1,
kFieldSetsCount,
kIndividualContainer,
kClusterID,
kClusterFieldSetData,
};

using clusterId = chip::ClusterId;

struct ExtensionFieldsSet
/// @brief Struct to serialize and de serialize a cluster extension field set
/// mID: Cluster ID, allows to identify which cluster is serialized
/// mBytesBuffer: Field ID serialized into a byte array
/// mUsedBytes: Number of bytes in the Buffer containing data, used for serializing only those bytes.
struct ExtensionFieldSet
{
clusterId mID = kInvalidClusterId;
uint8_t mBytesBuffer[kMaxFieldsPerCluster] = { 0 };
uint8_t mUsedBytes = 0;
ClusterId mID = kInvalidClusterId;
uint8_t mBytesBuffer[kMaxFieldBytesPerCluster] = { 0 };
uint8_t mUsedBytes = 0;

ExtensionFieldsSet() = default;
ExtensionFieldsSet(clusterId cmID, const uint8_t * data, uint8_t dataSize) : mID(cmID), mUsedBytes(dataSize)
ExtensionFieldSet() = default;
ExtensionFieldSet(ClusterId cmID, const uint8_t * data, uint8_t dataSize) : mID(cmID), mUsedBytes(dataSize)
{
if (dataSize <= kMaxFieldsPerCluster)
if (dataSize <= sizeof(mBytesBuffer))
{
memcpy(mBytesBuffer, data, mUsedBytes);
}
else
{
mUsedBytes = 0;
}
}

ExtensionFieldsSet(clusterId cmID, ByteSpan bytes) : mID(cmID), mUsedBytes(static_cast<uint8_t>(bytes.size()))
ExtensionFieldSet(ClusterId cmID, ByteSpan bytes) : mID(cmID), mUsedBytes(static_cast<uint8_t>(bytes.size()))
{
if (bytes.size() <= kMaxFieldsPerCluster)
if (bytes.size() <= sizeof(mBytesBuffer))
{
memcpy(mBytesBuffer, bytes.data(), bytes.size());
}
else
{
mUsedBytes = 0;
}
}

~ExtensionFieldsSet() = default;
~ExtensionFieldSet() = default;

CHIP_ERROR Serialize(TLV::TLVWriter & writer) const
{
TLV::TLVType container;
ReturnErrorOnFailure(writer.StartContainer(TLV::ContextTag(kTagEFSContainer), TLV::kTLVType_Structure, container));
ReturnErrorOnFailure(
writer.StartContainer(TLV::ContextTag(TagEFS::kIndividualContainer), TLV::kTLVType_Structure, container));

ReturnErrorOnFailure(writer.Put(TLV::ContextTag(kTagEFSClusterID), static_cast<uint16_t>(this->mID)));
ReturnErrorOnFailure(writer.PutBytes(TLV::ContextTag(kTagEFS), mBytesBuffer, mUsedBytes));
ReturnErrorOnFailure(writer.Put(TLV::ContextTag(TagEFS::kClusterID), static_cast<uint32_t>(mID)));
ReturnErrorOnFailure(writer.PutBytes(TLV::ContextTag(TagEFS::kClusterFieldSetData), mBytesBuffer, mUsedBytes));

return writer.EndContainer(container);
}
Expand All @@ -74,58 +91,58 @@ struct ExtensionFieldsSet
{
ByteSpan buffer;
TLV::TLVType container;
ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Structure, TLV::ContextTag(kTagEFSContainer)));
ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Structure, TLV::ContextTag(TagEFS::kIndividualContainer)));
ReturnErrorOnFailure(reader.EnterContainer(container));

ReturnErrorOnFailure(reader.Next(TLV::ContextTag(kTagEFSClusterID)));
ReturnErrorOnFailure(reader.Get(this->mID));
ReturnErrorOnFailure(reader.Next(TLV::ContextTag(TagEFS::kClusterID)));
ReturnErrorOnFailure(reader.Get(mID));

ReturnErrorOnFailure(reader.Next(TLV::ContextTag(kTagEFS)));
ReturnErrorOnFailure(reader.Next(TLV::ContextTag(TagEFS::kClusterFieldSetData)));
ReturnErrorOnFailure(reader.Get(buffer));
VerifyOrReturnError(buffer.size() <= kMaxFieldsPerCluster, CHIP_ERROR_BUFFER_TOO_SMALL);
this->mUsedBytes = static_cast<uint8_t>(buffer.size());
memcpy(this->mBytesBuffer, buffer.data(), this->mUsedBytes);
VerifyOrReturnError(buffer.size() <= sizeof(mBytesBuffer), CHIP_ERROR_BUFFER_TOO_SMALL);
mUsedBytes = static_cast<decltype(mUsedBytes)>(buffer.size());
memcpy(mBytesBuffer, buffer.data(), mUsedBytes);

return reader.ExitContainer(container);
}

void Clear()
{
this->mID = kInvalidClusterId;
memset(this->mBytesBuffer, 0, kMaxFieldsPerCluster);
this->mUsedBytes = 0;
mID = kInvalidClusterId;
memset(mBytesBuffer, 0, kMaxFieldBytesPerCluster);
mUsedBytes = 0;
}

bool IsEmpty() const { return (this->mUsedBytes == 0); }

bool operator==(const ExtensionFieldsSet & other)
bool operator==(const ExtensionFieldSet & other) const
{
return (this->mID == other.mID && !memcmp(this->mBytesBuffer, other.mBytesBuffer, this->mUsedBytes) &&
this->mUsedBytes == other.mUsedBytes);
return (this->mID == other.mID && this->mUsedBytes == other.mUsedBytes &&
!memcmp(this->mBytesBuffer, other.mBytesBuffer, this->mUsedBytes));
}
};

class ExtensionFieldSetsImpl : public ExtensionFieldSets
{
public:
ExtensionFieldSetsImpl();
ExtensionFieldSetsImpl(){};
~ExtensionFieldSetsImpl() override{};

// overrides
CHIP_ERROR Serialize(TLV::TLVWriter & writer) const override;
CHIP_ERROR Deserialize(TLV::TLVReader & reader) override;
void Clear() override;
bool IsEmpty() const override { return (this->mFieldNum == 0); }
uint8_t GetFieldNum() const override { return this->mFieldNum; };
bool IsEmpty() const override { return (mFieldSetsCount == 0); }
uint8_t GetFieldSetCount() const override { return mFieldSetsCount; };

// implementation
CHIP_ERROR InsertFieldSet(const ExtensionFieldsSet & field);
CHIP_ERROR GetFieldSetAtPosition(ExtensionFieldsSet & field, uint8_t position);
CHIP_ERROR InsertFieldSet(const ExtensionFieldSet & field);
CHIP_ERROR GetFieldSetAtPosition(ExtensionFieldSet & field, uint8_t position);
CHIP_ERROR RemoveFieldAtPosition(uint8_t position);

bool operator==(const ExtensionFieldSetsImpl & other)
// implementation
bool operator==(const ExtensionFieldSetsImpl & other) const
{
for (uint8_t i = 0; i < kMaxClusterPerScenes; i++)
for (uint8_t i = 0; i < mFieldSetsCount; i++)
{
if (!(this->mEFS[i] == other.mEFS[i]))
{
Expand All @@ -137,18 +154,18 @@ class ExtensionFieldSetsImpl : public ExtensionFieldSets

ExtensionFieldSetsImpl & operator=(const ExtensionFieldSetsImpl & other)
{
for (uint8_t i = 0; i < kMaxClusterPerScenes; i++)
for (uint8_t i = 0; i < other.mFieldSetsCount; i++)
{
this->mEFS[i] = other.mEFS[i];
}
mFieldNum = other.mFieldNum;
mFieldSetsCount = other.mFieldSetsCount;

return *this;
}

protected:
ExtensionFieldsSet mEFS[kMaxClusterPerScenes];
uint8_t mFieldNum = 0;
ExtensionFieldSet mEFS[kMaxClusterPerScenes];
uint8_t mFieldSetsCount = 0;
};
} // namespace scenes
} // namespace chip
Loading

0 comments on commit 01550b4

Please sign in to comment.