Skip to content

Commit 1207759

Browse files
lpbeliveau-silabsbzbarsky-apple
authored and
pull[bot]
committed
Added unit test for ExtensionFieldSets and applied suggestions from comments on PR
Co-authored-by: Boris Zbarsky <[email protected]>
1 parent 5fab19b commit 1207759

12 files changed

+566
-248
lines changed

‎src/app/clusters/scenes/ExtensionFieldSets.h

+7-4
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,11 @@
2323
namespace chip {
2424
namespace scenes {
2525

26-
static constexpr uint8_t kInvalidPosition = 0xff;
27-
static constexpr uint8_t kMaxClusterPerScenes = CHIP_CONFIG_SCENES_MAX_CLUSTERS_PER_SCENES;
28-
static constexpr uint8_t kMaxFieldsPerCluster = CHIP_CONFIG_SCENES_MAX_EXTENSION_FIELDSET_SIZE_PER_CLUSTER;
26+
static constexpr uint8_t kInvalidPosition = 0xff;
27+
static constexpr uint8_t kMaxClusterPerScenes = CHIP_CONFIG_SCENES_MAX_CLUSTERS_PER_SCENE;
28+
static constexpr uint8_t kMaxFieldBytesPerCluster = CHIP_CONFIG_SCENES_MAX_EXTENSION_FIELDSET_SIZE_PER_CLUSTER;
2929

30+
/// @brief class meant serialize all extension ßfield sets of a scene so it can be stored and retrieved from flash memory.
3031
class ExtensionFieldSets
3132
{
3233
public:
@@ -37,7 +38,9 @@ class ExtensionFieldSets
3738
virtual CHIP_ERROR Deserialize(TLV::TLVReader & reader) = 0;
3839
virtual void Clear() = 0;
3940
virtual bool IsEmpty() const = 0;
40-
virtual uint8_t GetFieldNum() const = 0;
41+
/// @brief Gets a count of how many initialized fields sets are in the object
42+
/// @return The number of initialized field sets in the object
43+
virtual uint8_t GetFieldSetCount() const = 0;
4144
};
4245
} // namespace scenes
4346
} // namespace chip

‎src/app/clusters/scenes/ExtensionFieldSetsImpl.cpp

+34-39
Original file line numberDiff line numberDiff line change
@@ -20,20 +20,21 @@
2020
namespace chip {
2121
namespace scenes {
2222

23-
ExtensionFieldSetsImpl::ExtensionFieldSetsImpl() : ExtensionFieldSets() {}
23+
// ExtensionFieldSetsImpl::ExtensionFieldSetsImpl() : ExtensionFieldSets() {}
2424

2525
CHIP_ERROR ExtensionFieldSetsImpl::Serialize(TLV::TLVWriter & writer) const
2626
{
2727
TLV::TLVType container;
28-
ReturnErrorOnFailure(writer.StartContainer(TLV::ContextTag(kTagEFSArrayContainer), TLV::kTLVType_Structure, container));
29-
ReturnErrorOnFailure(writer.Put(TLV::ContextTag(kTagEFSFieldNum), static_cast<uint8_t>(this->mFieldNum)));
30-
if (!this->IsEmpty())
28+
ReturnErrorOnFailure(
29+
writer.StartContainer(TLV::ContextTag(TagEFS::kFieldSetArrayContainer), TLV::kTLVType_Structure, container));
30+
ReturnErrorOnFailure(writer.Put(TLV::ContextTag(TagEFS::kFieldSetsCount), static_cast<uint8_t>(mFieldSetsCount)));
31+
if (!IsEmpty())
3132
{
32-
for (uint8_t i = 0; i < this->mFieldNum; i++)
33+
for (uint8_t i = 0; i < mFieldSetsCount; i++)
3334
{
34-
if (!this->mEFS[i].IsEmpty())
35+
if (!mEFS[i].IsEmpty())
3536
{
36-
ReturnErrorOnFailure(this->mEFS[i].Serialize(writer));
37+
ReturnErrorOnFailure(mEFS[i].Serialize(writer));
3738
}
3839
}
3940
}
@@ -44,17 +45,17 @@ CHIP_ERROR ExtensionFieldSetsImpl::Serialize(TLV::TLVWriter & writer) const
4445
CHIP_ERROR ExtensionFieldSetsImpl::Deserialize(TLV::TLVReader & reader)
4546
{
4647
TLV::TLVType container;
47-
ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Structure, TLV::ContextTag(kTagEFSArrayContainer)));
48+
ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Structure, TLV::ContextTag(TagEFS::kFieldSetArrayContainer)));
4849
ReturnErrorOnFailure(reader.EnterContainer(container));
4950

50-
ReturnErrorOnFailure(reader.Next(TLV::ContextTag(kTagEFSFieldNum)));
51-
ReturnErrorOnFailure(reader.Get(this->mFieldNum));
51+
ReturnErrorOnFailure(reader.Next(TLV::ContextTag(TagEFS::kFieldSetsCount)));
52+
ReturnErrorOnFailure(reader.Get(mFieldSetsCount));
5253

5354
if (!this->IsEmpty())
5455
{
55-
for (uint8_t i = 0; i < this->mFieldNum; i++)
56+
for (uint8_t i = 0; i < mFieldSetsCount; i++)
5657
{
57-
ReturnErrorOnFailure(this->mEFS[i].Deserialize(reader));
58+
ReturnErrorOnFailure(mEFS[i].Deserialize(reader));
5859
}
5960
}
6061

@@ -65,80 +66,74 @@ void ExtensionFieldSetsImpl::Clear()
6566
{
6667
if (!this->IsEmpty())
6768
{
68-
for (uint8_t i = 0; i < kMaxClusterPerScenes; i++)
69+
for (uint8_t i = 0; i < mFieldSetsCount; i++)
6970
{
70-
this->mEFS[i].Clear();
71+
mEFS[i].Clear();
7172
}
7273
}
73-
this->mFieldNum = 0;
74+
mFieldSetsCount = 0;
7475
}
7576

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

8685
VerifyOrReturnError(fieldSet.mID != kInvalidClusterId, CHIP_ERROR_INVALID_ARGUMENT);
86+
VerifyOrReturnError(!fieldSet.IsEmpty(), CHIP_ERROR_INVALID_ARGUMENT);
8787

8888
for (uint8_t i = 0; i < kMaxClusterPerScenes; i++)
8989
{
90-
if (this->mEFS[i].mID == fieldSet.mID)
90+
if (mEFS[i].mID == fieldSet.mID)
9191
{
92-
idPosition = i;
93-
break;
92+
mEFS[i] = fieldSet;
93+
return CHIP_NO_ERROR;
9494
}
9595

96-
if (this->mEFS[i].IsEmpty() && firstEmptyPosition == kInvalidPosition)
96+
if (mEFS[i].IsEmpty() && firstEmptyPosition == kInvalidPosition)
9797
{
9898
firstEmptyPosition = i;
9999
}
100100
}
101101

102102
// if found, replace at found position, otherwise at insert first free position, otherwise return error
103-
if (idPosition < kMaxClusterPerScenes)
104-
{
105-
this->mEFS[idPosition] = fieldSet;
106-
err = CHIP_NO_ERROR;
107-
}
108-
else if (firstEmptyPosition < kMaxClusterPerScenes)
103+
if (firstEmptyPosition < kMaxClusterPerScenes)
109104
{
110-
this->mEFS[firstEmptyPosition] = fieldSet;
111-
this->mFieldNum++;
112-
err = CHIP_NO_ERROR;
105+
mEFS[firstEmptyPosition] = fieldSet;
106+
mFieldSetsCount++;
107+
return CHIP_NO_ERROR;
113108
}
114109

115-
return err;
110+
return CHIP_ERROR_NO_MEMORY;
116111
}
117112

118-
CHIP_ERROR ExtensionFieldSetsImpl::GetFieldSetAtPosition(ExtensionFieldsSet & fieldSet, uint8_t position)
113+
CHIP_ERROR ExtensionFieldSetsImpl::GetFieldSetAtPosition(ExtensionFieldSet & fieldSet, uint8_t position)
119114
{
120-
VerifyOrReturnError(position < this->mFieldNum, CHIP_ERROR_BUFFER_TOO_SMALL);
115+
VerifyOrReturnError(position < mFieldSetsCount, CHIP_ERROR_BUFFER_TOO_SMALL);
121116

122-
fieldSet = this->mEFS[position];
117+
fieldSet = mEFS[position];
123118

124119
return CHIP_NO_ERROR;
125120
}
126121

127122
CHIP_ERROR ExtensionFieldSetsImpl::RemoveFieldAtPosition(uint8_t position)
128123
{
129124
VerifyOrReturnError(position < kMaxClusterPerScenes, CHIP_ERROR_INVALID_ARGUMENT);
130-
VerifyOrReturnValue(!this->IsEmpty() && !this->mEFS[position].IsEmpty(), CHIP_NO_ERROR);
125+
VerifyOrReturnValue(!this->IsEmpty() && !mEFS[position].IsEmpty(), CHIP_NO_ERROR);
131126

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

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

139-
this->mFieldNum--;
134+
mFieldSetsCount--;
140135
// Clear last occupied position
141-
this->mEFS[mFieldNum].Clear(); //
136+
mEFS[mFieldSetsCount].Clear(); //
142137

143138
return CHIP_NO_ERROR;
144139
}

‎src/app/clusters/scenes/ExtensionFieldSetsImpl.h

+63-46
Original file line numberDiff line numberDiff line change
@@ -23,49 +23,66 @@
2323
namespace chip {
2424
namespace scenes {
2525

26-
enum EFSTLVTag
26+
/// @brief Tags Used to serialize Extension Field Sets struct as well as individual field sets.
27+
/// kArrayContainer: Tag for the container of the Struct with the EFS array
28+
/// kFieldSetsCount: Tag representing the number of individual field sets
29+
/// kIndividualContainer: Tag for the container of single EFS struct
30+
/// kClusterID: Tag for the ClusterID of a field set
31+
/// kBufferBytes: Tag for the serialized field set data
32+
enum class TagEFS : uint8_t
2733
{
28-
kTagEFSArrayContainer = 1,
29-
kTagEFSFieldNum = 1,
30-
kTagEFSContainer,
31-
kTagEFSClusterID,
32-
kTagEFS,
34+
kFieldSetArrayContainer = 1,
35+
kFieldSetsCount,
36+
kIndividualContainer,
37+
kClusterID,
38+
kClusterFieldSetData,
3339
};
3440

35-
using clusterId = chip::ClusterId;
36-
37-
struct ExtensionFieldsSet
41+
/// @brief Struct to serialize and de serialize a cluster extension field set
42+
/// mID: Cluster ID, allows to identify which cluster is serialized
43+
/// mBytesBuffer: Field ID serialized into a byte array
44+
/// mUsedBytes: Number of bytes in the Buffer containing data, used for serializing only those bytes.
45+
struct ExtensionFieldSet
3846
{
39-
clusterId mID = kInvalidClusterId;
40-
uint8_t mBytesBuffer[kMaxFieldsPerCluster] = { 0 };
41-
uint8_t mUsedBytes = 0;
47+
ClusterId mID = kInvalidClusterId;
48+
uint8_t mBytesBuffer[kMaxFieldBytesPerCluster] = { 0 };
49+
uint8_t mUsedBytes = 0;
4250

43-
ExtensionFieldsSet() = default;
44-
ExtensionFieldsSet(clusterId cmID, const uint8_t * data, uint8_t dataSize) : mID(cmID), mUsedBytes(dataSize)
51+
ExtensionFieldSet() = default;
52+
ExtensionFieldSet(ClusterId cmID, const uint8_t * data, uint8_t dataSize) : mID(cmID), mUsedBytes(dataSize)
4553
{
46-
if (dataSize <= kMaxFieldsPerCluster)
54+
if (dataSize <= sizeof(mBytesBuffer))
4755
{
4856
memcpy(mBytesBuffer, data, mUsedBytes);
4957
}
58+
else
59+
{
60+
mUsedBytes = 0;
61+
}
5062
}
5163

52-
ExtensionFieldsSet(clusterId cmID, ByteSpan bytes) : mID(cmID), mUsedBytes(static_cast<uint8_t>(bytes.size()))
64+
ExtensionFieldSet(ClusterId cmID, ByteSpan bytes) : mID(cmID), mUsedBytes(static_cast<uint8_t>(bytes.size()))
5365
{
54-
if (bytes.size() <= kMaxFieldsPerCluster)
66+
if (bytes.size() <= sizeof(mBytesBuffer))
5567
{
5668
memcpy(mBytesBuffer, bytes.data(), bytes.size());
5769
}
70+
else
71+
{
72+
mUsedBytes = 0;
73+
}
5874
}
5975

60-
~ExtensionFieldsSet() = default;
76+
~ExtensionFieldSet() = default;
6177

6278
CHIP_ERROR Serialize(TLV::TLVWriter & writer) const
6379
{
6480
TLV::TLVType container;
65-
ReturnErrorOnFailure(writer.StartContainer(TLV::ContextTag(kTagEFSContainer), TLV::kTLVType_Structure, container));
81+
ReturnErrorOnFailure(
82+
writer.StartContainer(TLV::ContextTag(TagEFS::kIndividualContainer), TLV::kTLVType_Structure, container));
6683

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

7087
return writer.EndContainer(container);
7188
}
@@ -74,58 +91,58 @@ struct ExtensionFieldsSet
7491
{
7592
ByteSpan buffer;
7693
TLV::TLVType container;
77-
ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Structure, TLV::ContextTag(kTagEFSContainer)));
94+
ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Structure, TLV::ContextTag(TagEFS::kIndividualContainer)));
7895
ReturnErrorOnFailure(reader.EnterContainer(container));
7996

80-
ReturnErrorOnFailure(reader.Next(TLV::ContextTag(kTagEFSClusterID)));
81-
ReturnErrorOnFailure(reader.Get(this->mID));
97+
ReturnErrorOnFailure(reader.Next(TLV::ContextTag(TagEFS::kClusterID)));
98+
ReturnErrorOnFailure(reader.Get(mID));
8299

83-
ReturnErrorOnFailure(reader.Next(TLV::ContextTag(kTagEFS)));
100+
ReturnErrorOnFailure(reader.Next(TLV::ContextTag(TagEFS::kClusterFieldSetData)));
84101
ReturnErrorOnFailure(reader.Get(buffer));
85-
VerifyOrReturnError(buffer.size() <= kMaxFieldsPerCluster, CHIP_ERROR_BUFFER_TOO_SMALL);
86-
this->mUsedBytes = static_cast<uint8_t>(buffer.size());
87-
memcpy(this->mBytesBuffer, buffer.data(), this->mUsedBytes);
102+
VerifyOrReturnError(buffer.size() <= sizeof(mBytesBuffer), CHIP_ERROR_BUFFER_TOO_SMALL);
103+
mUsedBytes = static_cast<decltype(mUsedBytes)>(buffer.size());
104+
memcpy(mBytesBuffer, buffer.data(), mUsedBytes);
88105

89106
return reader.ExitContainer(container);
90107
}
91108

92109
void Clear()
93110
{
94-
this->mID = kInvalidClusterId;
95-
memset(this->mBytesBuffer, 0, kMaxFieldsPerCluster);
96-
this->mUsedBytes = 0;
111+
mID = kInvalidClusterId;
112+
memset(mBytesBuffer, 0, kMaxFieldBytesPerCluster);
113+
mUsedBytes = 0;
97114
}
98115

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

101-
bool operator==(const ExtensionFieldsSet & other)
118+
bool operator==(const ExtensionFieldSet & other) const
102119
{
103-
return (this->mID == other.mID && !memcmp(this->mBytesBuffer, other.mBytesBuffer, this->mUsedBytes) &&
104-
this->mUsedBytes == other.mUsedBytes);
120+
return (this->mID == other.mID && this->mUsedBytes == other.mUsedBytes &&
121+
!memcmp(this->mBytesBuffer, other.mBytesBuffer, this->mUsedBytes));
105122
}
106123
};
107124

108125
class ExtensionFieldSetsImpl : public ExtensionFieldSets
109126
{
110127
public:
111-
ExtensionFieldSetsImpl();
128+
ExtensionFieldSetsImpl(){};
112129
~ExtensionFieldSetsImpl() override{};
113130

114131
// overrides
115132
CHIP_ERROR Serialize(TLV::TLVWriter & writer) const override;
116133
CHIP_ERROR Deserialize(TLV::TLVReader & reader) override;
117134
void Clear() override;
118-
bool IsEmpty() const override { return (this->mFieldNum == 0); }
119-
uint8_t GetFieldNum() const override { return this->mFieldNum; };
135+
bool IsEmpty() const override { return (mFieldSetsCount == 0); }
136+
uint8_t GetFieldSetCount() const override { return mFieldSetsCount; };
120137

121-
// implementation
122-
CHIP_ERROR InsertFieldSet(const ExtensionFieldsSet & field);
123-
CHIP_ERROR GetFieldSetAtPosition(ExtensionFieldsSet & field, uint8_t position);
138+
CHIP_ERROR InsertFieldSet(const ExtensionFieldSet & field);
139+
CHIP_ERROR GetFieldSetAtPosition(ExtensionFieldSet & field, uint8_t position);
124140
CHIP_ERROR RemoveFieldAtPosition(uint8_t position);
125141

126-
bool operator==(const ExtensionFieldSetsImpl & other)
142+
// implementation
143+
bool operator==(const ExtensionFieldSetsImpl & other) const
127144
{
128-
for (uint8_t i = 0; i < kMaxClusterPerScenes; i++)
145+
for (uint8_t i = 0; i < mFieldSetsCount; i++)
129146
{
130147
if (!(this->mEFS[i] == other.mEFS[i]))
131148
{
@@ -137,18 +154,18 @@ class ExtensionFieldSetsImpl : public ExtensionFieldSets
137154

138155
ExtensionFieldSetsImpl & operator=(const ExtensionFieldSetsImpl & other)
139156
{
140-
for (uint8_t i = 0; i < kMaxClusterPerScenes; i++)
157+
for (uint8_t i = 0; i < other.mFieldSetsCount; i++)
141158
{
142159
this->mEFS[i] = other.mEFS[i];
143160
}
144-
mFieldNum = other.mFieldNum;
161+
mFieldSetsCount = other.mFieldSetsCount;
145162

146163
return *this;
147164
}
148165

149166
protected:
150-
ExtensionFieldsSet mEFS[kMaxClusterPerScenes];
151-
uint8_t mFieldNum = 0;
167+
ExtensionFieldSet mEFS[kMaxClusterPerScenes];
168+
uint8_t mFieldSetsCount = 0;
152169
};
153170
} // namespace scenes
154171
} // namespace chip

0 commit comments

Comments
 (0)