Skip to content

Commit 4002331

Browse files
lpbeliveau-silabspull[bot]
lpbeliveau-silabs
authored and
pull[bot]
committed
Fixed suspicious usage of sizeof, rewrote function comments, implemented test for sceneHandler registration
1 parent 6a81b90 commit 4002331

File tree

6 files changed

+240
-143
lines changed

6 files changed

+240
-143
lines changed

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ CHIP_ERROR ExtensionFieldsSetsImpl::RemoveFieldAtPosition(uint8_t position)
129129
VerifyOrReturnError(position < kMaxClusterPerScenes, CHIP_ERROR_INVALID_ARGUMENT);
130130
VerifyOrReturnValue(!this->IsEmpty() && !this->mEFS[position].IsEmpty(), CHIP_NO_ERROR);
131131

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

135135
// TODO: Implement general array management methods

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

+64-23
Original file line numberDiff line numberDiff line change
@@ -56,40 +56,52 @@ class SceneHandler
5656
SceneHandler(){};
5757
virtual ~SceneHandler() = default;
5858

59+
/// @brief Gets the list of supported clusters for an endpoint
60+
/// @param endpoint target endpoint
61+
/// @param clusterBuffer Buffer to hold the supported cluster IDs, cannot hold more than
62+
/// CHIP_CONFIG_SCENES_MAX_CLUSTERS_PER_SCENES
63+
virtual void GetSupportedClusters(EndpointId endpoint, Span<ClusterId> & clusterBuffer) = 0;
64+
65+
/// @brief Returns whether or not a cluster is supported on an endpoint
66+
/// @param endpoint Target Endpoint ID
67+
/// @param cluster Target Cluster ID
68+
/// @return true if supported, false if not supported
5969
virtual bool SupportsCluster(EndpointId endpoint, ClusterId cluster) = 0;
6070

6171
/// @brief From command AddScene, allows handler to filter through clusters in command to serialize only the supported ones.
6272
/// @param endpoint Endpoint ID
6373
/// @param cluster Cluster ID to fetch from command
64-
/// @param serialysedBytes Buffer for ExtensionFieldSet in command
74+
/// @param serialisedBytes Buffer for ExtensionFieldSet in command
6575
/// @param extensionFieldSet ExtensionFieldSets provided by the AddScene Command
66-
/// @return
67-
virtual CHIP_ERROR SerializeAdd(EndpointId endpoint, ClusterId & cluster, MutableByteSpan & serialysedBytes,
76+
/// @return CHIP_NO_ERROR if successful, CHIP_ERROR value otherwise
77+
virtual CHIP_ERROR SerializeAdd(EndpointId endpoint, ClusterId & cluster, MutableByteSpan & serialisedBytes,
6878
app::Clusters::Scenes::Structs::ExtensionFieldSet::DecodableType & extensionFieldSet) = 0;
6979

70-
/// @brief From command SaveScene, retrieves ExtensionField from nvm
71-
/// @param endpoint
72-
/// @param cluster
73-
/// @param serialysedBytes
74-
/// @return
75-
virtual CHIP_ERROR SerializeSave(EndpointId endpoint, ClusterId cluster, MutableByteSpan & serialysedBytes) = 0;
80+
/// @brief From command StoreScene, retrieves ExtensionField from nvm, it is the functions responsability to resize the mutable
81+
/// span if necessary, a number of byte equal to the span will be stored in memory
82+
/// @param endpoint Target Endpoint
83+
/// @param cluster Target Cluster
84+
/// @param serialisedBytes Output buffer, data needs to be writen in there and size adjusted if smaller than
85+
/// kMaxFieldsPerCluster
86+
/// @return CHIP_NO_ERROR if successful, CHIP_ERROR value otherwise
87+
virtual CHIP_ERROR SerializeSave(EndpointId endpoint, ClusterId cluster, MutableByteSpan & serialisedBytes) = 0;
7688

7789
/// @brief From stored scene (e.g. ViewScene), deserialize ExtensionFieldSet into a command object
7890
/// @param endpoint Endpoint ID
79-
/// @param cluster Cluster ID to set in command
80-
/// @param serialysedBytes ExtensionFieldSet stored in NVM
91+
/// @param cluster Cluster ID to save
92+
/// @param serialisedBytes ExtensionFieldSet stored in NVM
8193
/// @param extensionFieldSet ExtensionFieldSet in command format
82-
/// @return
83-
virtual CHIP_ERROR Deserialize(EndpointId endpoint, ClusterId cluster, ByteSpan & serialysedBytes,
94+
/// @return CHIP_NO_ERROR if successful, CHIP_ERROR value otherwise
95+
virtual CHIP_ERROR Deserialize(EndpointId endpoint, ClusterId cluster, ByteSpan & serialisedBytes,
8496
app::Clusters::Scenes::Structs::ExtensionFieldSet::Type & extensionFieldSet) = 0;
8597

8698
/// @brief From stored scene (e.g RecallScene), applies EFS values to cluster at transition time
8799
/// @param endpoint Endpoint ID
88100
/// @param cluster Cluster ID
89-
/// @param serialysedBytes ExtensionFieldSet stored in NVM
101+
/// @param serialisedBytes ExtensionFieldSet stored in NVM
90102
/// @param timeMs Transition time in ms to apply the scene
91-
/// @return
92-
virtual CHIP_ERROR ApplyScene(EndpointId endpoint, ClusterId cluster, ByteSpan & serialysedBytes, TransitionTimeMs timeMs) = 0;
103+
/// @return CHIP_NO_ERROR if successful, CHIP_ERROR value otherwise
104+
virtual CHIP_ERROR ApplyScene(EndpointId endpoint, ClusterId cluster, ByteSpan & serialisedBytes, TransitionTimeMs timeMs) = 0;
93105
};
94106

95107
class SceneTable
@@ -318,21 +330,50 @@ class SceneTable
318330
virtual CHIP_ERROR RemoveSceneTableEntry(FabricIndex fabric_index, SceneStorageId scene_id) = 0;
319331
virtual CHIP_ERROR RemoveSceneTableEntryAtPosition(FabricIndex fabric_index, SceneIndex scened_idx) = 0;
320332

321-
// Iterators
322-
using SceneEntryIterator = CommonIterator<SceneTableEntry>;
333+
// SceneHandlers
334+
virtual CHIP_ERROR RegisterHandler(SceneHandler * handler) = 0;
335+
virtual CHIP_ERROR UnregisterHandler(uint8_t position) = 0;
323336

324-
virtual SceneEntryIterator * IterateSceneEntry(FabricIndex fabric_index) = 0;
337+
// Extension field sets operation
338+
virtual CHIP_ERROR SceneSaveEFS(SceneTableEntry & scene) = 0;
339+
virtual CHIP_ERROR SceneApplyEFS(FabricIndex fabric_index, const SceneStorageId & scene_id) = 0;
325340

326341
// Fabrics
327342
virtual CHIP_ERROR RemoveFabric(FabricIndex fabric_index) = 0;
328343

344+
// Iterators
345+
using SceneEntryIterator = CommonIterator<SceneTableEntry>;
346+
347+
virtual SceneEntryIterator * IterateSceneEntry(FabricIndex fabric_index) = 0;
348+
329349
// Handlers
330-
SceneHandler * mHandlers[CHIP_CONFIG_SCENES_MAX_CLUSTERS_PER_SCENES] = { nullptr };
331-
uint8_t handlerNum = 0;
350+
virtual bool HandlerListEmpty() { return (handlerNum == 0); }
351+
virtual bool HandlerListFull() { return (handlerNum >= kMaxSceneHandlers); }
352+
virtual uint8_t GetHandlerNum() { return this->handlerNum; }
332353

333-
protected:
334-
const uint8_t mMaxScenePerFabric = kMaxScenePerFabric;
354+
SceneHandler * mHandlers[kMaxSceneHandlers] = { nullptr };
355+
uint8_t handlerNum = 0;
335356
};
336357

358+
/**
359+
* Instance getter for the global SceneTable.
360+
*
361+
* Callers have to externally synchronize usage of this function.
362+
*
363+
* @return The global Scene Table
364+
*/
365+
SceneTable * GetSceneTable();
366+
367+
/**
368+
* Instance setter for the global Scene Table.
369+
*
370+
* Callers have to externally synchronize usage of this function.
371+
*
372+
* The `provider` can be set to nullptr if the owner is done with it fully.
373+
*
374+
* @param[in] provider pointer to the Scene Table global instance to use
375+
*/
376+
void SetSceneTable(SceneTable * provider);
377+
337378
} // namespace scenes
338379
} // namespace chip

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

+19-12
Original file line numberDiff line numberDiff line change
@@ -443,12 +443,12 @@ CHIP_ERROR DefaultSceneTableImpl::UnregisterHandler(uint8_t position)
443443
VerifyOrReturnError(position < kMaxSceneHandlers, CHIP_ERROR_INVALID_ARGUMENT);
444444
VerifyOrReturnValue(!this->HandlerListEmpty() && !(this->mHandlers[position] == nullptr), CHIP_NO_ERROR);
445445

446-
uint8_t nextPos = position++;
446+
uint8_t nextPos = static_cast<uint8_t>(position + 1);
447447
uint8_t moveNum = static_cast<uint8_t>(kMaxSceneHandlers - nextPos);
448448

449449
// TODO: Implement general array management methods
450450
// Compress array after removal
451-
memmove(&this->mHandlers[position], &this->mHandlers[nextPos], sizeof(SceneHandler *) * moveNum);
451+
memmove(&this->mHandlers[position], &this->mHandlers[nextPos], sizeof(this->mHandlers[position]) * moveNum);
452452

453453
this->handlerNum--;
454454
// Clear last occupied position
@@ -457,20 +457,27 @@ CHIP_ERROR DefaultSceneTableImpl::UnregisterHandler(uint8_t position)
457457
return CHIP_NO_ERROR;
458458
}
459459

460-
CHIP_ERROR DefaultSceneTableImpl::SceneSaveEFS(SceneTableEntry & scene, clusterId cluster)
460+
CHIP_ERROR DefaultSceneTableImpl::SceneSaveEFS(SceneTableEntry & scene)
461461
{
462-
ExtensionFieldsSet EFS;
463-
MutableByteSpan EFSSpan = MutableByteSpan(EFS.mBytesBuffer, kMaxFieldsPerCluster);
464462
if (!this->HandlerListEmpty())
465463
{
466464
for (uint8_t i = 0; i < this->handlerNum; i++)
467465
{
466+
clusterId cArray[kMaxClusterPerScenes];
467+
Span<clusterId> cSpan(cArray);
468468
if (this->mHandlers[i] != nullptr)
469469
{
470-
EFS.mID = cluster;
471-
ReturnErrorOnFailure(this->mHandlers[i]->SerializeSave(scene.mStorageId.mEndpointId, cluster, EFSSpan));
472-
EFS.mUsedBytes = (uint8_t) EFSSpan.size();
473-
ReturnErrorOnFailure(scene.mStorageData.mExtensionFieldsSets.InsertFieldSet(EFS));
470+
this->mHandlers[i]->GetSupportedClusters(scene.mStorageId.mEndpointId, cSpan);
471+
for (uint8_t j = 0; j < cSpan.size(); j++)
472+
{
473+
ExtensionFieldsSet EFS;
474+
MutableByteSpan EFSSpan = MutableByteSpan(EFS.mBytesBuffer, kMaxFieldsPerCluster);
475+
476+
EFS.mID = cArray[j];
477+
ReturnErrorOnFailure(this->mHandlers[i]->SerializeSave(scene.mStorageId.mEndpointId, EFS.mID, EFSSpan));
478+
EFS.mUsedBytes = (uint8_t) EFSSpan.size();
479+
ReturnErrorOnFailure(scene.mStorageData.mExtensionFieldsSets.InsertFieldSet(EFS));
480+
}
474481
}
475482
}
476483
}
@@ -585,16 +592,16 @@ void DefaultSceneTableImpl::SceneEntryIteratorImpl::Release()
585592

586593
namespace {
587594

588-
DefaultSceneTableImpl * gSceneTable = nullptr;
595+
SceneTable * gSceneTable = nullptr;
589596

590597
} // namespace
591598

592-
DefaultSceneTableImpl * GetSceneTable()
599+
SceneTable * GetSceneTable()
593600
{
594601
return gSceneTable;
595602
}
596603

597-
void SetSceneTable(DefaultSceneTableImpl * provider)
604+
void SetSceneTable(SceneTable * provider)
598605
{
599606
gSceneTable = provider;
600607
}

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

+10-33
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,10 @@ class DefaultSceneHandlerImpl : public scenes::SceneHandler
4444
/// @brief Function to serialize data from an add scene command, assume the incoming extensionFieldSet is initialized
4545
/// @param endpoint Target Endpoint
4646
/// @param cluster Cluster in the Extension field set, filled by the function
47-
/// @param serialyzedBytes Mutable Byte span to hold EFS data from command
47+
/// @param serialisedBytes Mutable Byte span to hold EFS data from command
4848
/// @param extensionFieldSet Extension field set from commmand, pre initialized
4949
/// @return CHIP_NO_ERROR if success, specific CHIP_ERROR otherwise
50-
virtual CHIP_ERROR SerializeAdd(EndpointId endpoint, ClusterId & cluster, MutableByteSpan & serialyzedBytes,
50+
virtual CHIP_ERROR SerializeAdd(EndpointId endpoint, ClusterId & cluster, MutableByteSpan & serialisedBytes,
5151
app::Clusters::Scenes::Structs::ExtensionFieldSet::DecodableType & extensionFieldSet) override
5252
{
5353
app::DataModel::List<app::Clusters::Scenes::Structs::AttributeValuePair::Type> attributeValueList;
@@ -88,7 +88,7 @@ class DefaultSceneHandlerImpl : public scenes::SceneHandler
8888
attributeValueList = mAVPairs;
8989
attributeValueList.reduce_size(pairCount);
9090

91-
writer.Init(serialyzedBytes);
91+
writer.Init(serialisedBytes);
9292
ReturnErrorOnFailure(writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Structure, outer));
9393
ReturnErrorOnFailure(app::DataModel::Encode(
9494
writer, TLV::ContextTag(to_underlying(app::Clusters::Scenes::Structs::ExtensionFieldSet::Fields::kAttributeValueList)),
@@ -101,9 +101,9 @@ class DefaultSceneHandlerImpl : public scenes::SceneHandler
101101
/// @brief Simulates taking data from nvm and loading it in a command object if the cluster is supported by the endpoint
102102
/// @param endpoint target endpoint
103103
/// @param cluster target cluster
104-
/// @param serialyzedBytes data to deserialize into EFS
104+
/// @param serialisedBytes data to deserialize into EFS
105105
/// @return CHIP_NO_ERROR if Extension Field Set was successfully populated, specific CHIP_ERROR otherwise
106-
virtual CHIP_ERROR Deserialize(EndpointId endpoint, ClusterId cluster, ByteSpan & serialyzedBytes,
106+
virtual CHIP_ERROR Deserialize(EndpointId endpoint, ClusterId cluster, ByteSpan & serialisedBytes,
107107
app::Clusters::Scenes::Structs::ExtensionFieldSet::Type & extensionFieldSet) override
108108
{
109109
app::DataModel::DecodableList<app::Clusters::Scenes::Structs::AttributeValuePair::DecodableType> attributeValueList;
@@ -117,7 +117,7 @@ class DefaultSceneHandlerImpl : public scenes::SceneHandler
117117
VerifyOrReturnError(SupportsCluster(endpoint, cluster), CHIP_ERROR_INVALID_ARGUMENT);
118118

119119
extensionFieldSet.clusterID = cluster;
120-
reader.Init(serialyzedBytes);
120+
reader.Init(serialisedBytes);
121121
ReturnErrorOnFailure(reader.Next());
122122
ReturnErrorOnFailure(reader.EnterContainer(outer));
123123
ReturnErrorOnFailure(reader.Next());
@@ -183,23 +183,19 @@ class DefaultSceneTableImpl : public SceneTable
183183
CHIP_ERROR RemoveSceneTableEntryAtPosition(FabricIndex fabric_index, SceneIndex scened_idx) override;
184184

185185
// SceneHandlers
186-
CHIP_ERROR RegisterHandler(SceneHandler * handler);
187-
CHIP_ERROR UnregisterHandler(uint8_t position);
186+
CHIP_ERROR RegisterHandler(SceneHandler * handler) override;
187+
CHIP_ERROR UnregisterHandler(uint8_t position) override;
188188

189189
// Extension field sets operation
190-
CHIP_ERROR SceneSaveEFS(SceneTableEntry & scene, clusterId cluster);
191-
CHIP_ERROR SceneApplyEFS(FabricIndex fabric_index, const SceneStorageId & scene_id);
190+
CHIP_ERROR SceneSaveEFS(SceneTableEntry & scene) override;
191+
CHIP_ERROR SceneApplyEFS(FabricIndex fabric_index, const SceneStorageId & scene_id) override;
192192

193193
// Fabrics
194194
CHIP_ERROR RemoveFabric(FabricIndex fabric_index) override;
195195

196196
// Iterators
197197
SceneEntryIterator * IterateSceneEntry(FabricIndex fabric_index) override;
198198

199-
bool HandlerListEmpty() { return (handlerNum == 0); }
200-
bool HandlerListFull() { return (handlerNum >= CHIP_CONFIG_SCENES_MAX_CLUSTERS_PER_SCENES); }
201-
uint8_t GetHandlerNum() { return this->handlerNum; }
202-
203199
protected:
204200
class SceneEntryIteratorImpl : public SceneEntryIterator
205201
{
@@ -222,24 +218,5 @@ class DefaultSceneTableImpl : public SceneTable
222218
ObjectPool<SceneEntryIteratorImpl, kIteratorsMax> mSceneEntryIterators;
223219
}; // class DefaultSceneTableImpl
224220

225-
/**
226-
* Instance getter for the global SceneTable.
227-
*
228-
* Callers have to externally synchronize usage of this function.
229-
*
230-
* @return The global Scene Table
231-
*/
232-
DefaultSceneTableImpl * GetSceneTable();
233-
234-
/**
235-
* Instance setter for the global Scene Table.
236-
*
237-
* Callers have to externally synchronize usage of this function.
238-
*
239-
* The `provider` can be set to nullptr if the owner is done with it fully.
240-
*
241-
* @param[in] provider pointer to the Scene Table global instance to use
242-
*/
243-
void SetSceneTable(DefaultSceneTableImpl * provider);
244221
} // namespace scenes
245222
} // namespace chip

0 commit comments

Comments
 (0)