Skip to content

Commit

Permalink
Fixed suspicious usage of sizeof, rewrote function comments, implemen…
Browse files Browse the repository at this point in the history 
…ted test for sceneHandler registration
  • Loading branch information
@lpbeliveau-silabs @pull
lpbeliveau-silabs authored and pull[bot] committed Nov 8, 2023
1 parent 6a81b90 commit 4002331
Show file tree
Hide file tree
Showing 6 changed files with 240 additions and 143 deletions.
2 changes: 1 addition & 1 deletion src/app/clusters/scenes/ExtensionFieldsSetsImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ CHIP_ERROR ExtensionFieldsSetsImpl::RemoveFieldAtPosition(uint8_t position)
VerifyOrReturnError(position < kMaxClusterPerScenes, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnValue(!this->IsEmpty() && !this->mEFS[position].IsEmpty(), CHIP_NO_ERROR);

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

// TODO: Implement general array management methods
Expand Down
87 changes: 64 additions & 23 deletions src/app/clusters/scenes/SceneTable.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,40 +56,52 @@ class SceneHandler
SceneHandler(){};
virtual ~SceneHandler() = default;

/// @brief Gets the list of supported clusters for an endpoint
/// @param endpoint target endpoint
/// @param clusterBuffer Buffer to hold the supported cluster IDs, cannot hold more than
/// CHIP_CONFIG_SCENES_MAX_CLUSTERS_PER_SCENES
virtual void GetSupportedClusters(EndpointId endpoint, Span<ClusterId> & clusterBuffer) = 0;

/// @brief Returns whether or not a cluster is supported on an endpoint
/// @param endpoint Target Endpoint ID
/// @param cluster Target Cluster ID
/// @return true if supported, false if not supported
virtual bool SupportsCluster(EndpointId endpoint, ClusterId cluster) = 0;

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

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

/// @brief From stored scene (e.g. ViewScene), deserialize ExtensionFieldSet into a command object
/// @param endpoint Endpoint ID
/// @param cluster Cluster ID to set in command
/// @param serialysedBytes ExtensionFieldSet stored in NVM
/// @param cluster Cluster ID to save
/// @param serialisedBytes ExtensionFieldSet stored in NVM
/// @param extensionFieldSet ExtensionFieldSet in command format
/// @return
virtual CHIP_ERROR Deserialize(EndpointId endpoint, ClusterId cluster, ByteSpan & serialysedBytes,
/// @return CHIP_NO_ERROR if successful, CHIP_ERROR value otherwise
virtual CHIP_ERROR Deserialize(EndpointId endpoint, ClusterId cluster, ByteSpan & serialisedBytes,
app::Clusters::Scenes::Structs::ExtensionFieldSet::Type & extensionFieldSet) = 0;

/// @brief From stored scene (e.g RecallScene), applies EFS values to cluster at transition time
/// @param endpoint Endpoint ID
/// @param cluster Cluster ID
/// @param serialysedBytes ExtensionFieldSet stored in NVM
/// @param serialisedBytes ExtensionFieldSet stored in NVM
/// @param timeMs Transition time in ms to apply the scene
/// @return
virtual CHIP_ERROR ApplyScene(EndpointId endpoint, ClusterId cluster, ByteSpan & serialysedBytes, TransitionTimeMs timeMs) = 0;
/// @return CHIP_NO_ERROR if successful, CHIP_ERROR value otherwise
virtual CHIP_ERROR ApplyScene(EndpointId endpoint, ClusterId cluster, ByteSpan & serialisedBytes, TransitionTimeMs timeMs) = 0;
};

class SceneTable
Expand Down Expand Up @@ -318,21 +330,50 @@ class SceneTable
virtual CHIP_ERROR RemoveSceneTableEntry(FabricIndex fabric_index, SceneStorageId scene_id) = 0;
virtual CHIP_ERROR RemoveSceneTableEntryAtPosition(FabricIndex fabric_index, SceneIndex scened_idx) = 0;

// Iterators
using SceneEntryIterator = CommonIterator<SceneTableEntry>;
// SceneHandlers
virtual CHIP_ERROR RegisterHandler(SceneHandler * handler) = 0;
virtual CHIP_ERROR UnregisterHandler(uint8_t position) = 0;

virtual SceneEntryIterator * IterateSceneEntry(FabricIndex fabric_index) = 0;
// Extension field sets operation
virtual CHIP_ERROR SceneSaveEFS(SceneTableEntry & scene) = 0;
virtual CHIP_ERROR SceneApplyEFS(FabricIndex fabric_index, const SceneStorageId & scene_id) = 0;

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

// Iterators
using SceneEntryIterator = CommonIterator<SceneTableEntry>;

virtual SceneEntryIterator * IterateSceneEntry(FabricIndex fabric_index) = 0;

// Handlers
SceneHandler * mHandlers[CHIP_CONFIG_SCENES_MAX_CLUSTERS_PER_SCENES] = { nullptr };
uint8_t handlerNum = 0;
virtual bool HandlerListEmpty() { return (handlerNum == 0); }
virtual bool HandlerListFull() { return (handlerNum >= kMaxSceneHandlers); }
virtual uint8_t GetHandlerNum() { return this->handlerNum; }

protected:
const uint8_t mMaxScenePerFabric = kMaxScenePerFabric;
SceneHandler * mHandlers[kMaxSceneHandlers] = { nullptr };
uint8_t handlerNum = 0;
};

/**
* Instance getter for the global SceneTable.
*
* Callers have to externally synchronize usage of this function.
*
* @return The global Scene Table
*/
SceneTable * GetSceneTable();

/**
* Instance setter for the global Scene Table.
*
* Callers have to externally synchronize usage of this function.
*
* The `provider` can be set to nullptr if the owner is done with it fully.
*
* @param[in] provider pointer to the Scene Table global instance to use
*/
void SetSceneTable(SceneTable * provider);

} // namespace scenes
} // namespace chip
31 changes: 19 additions & 12 deletions src/app/clusters/scenes/SceneTableImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -443,12 +443,12 @@ CHIP_ERROR DefaultSceneTableImpl::UnregisterHandler(uint8_t position)
VerifyOrReturnError(position < kMaxSceneHandlers, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnValue(!this->HandlerListEmpty() && !(this->mHandlers[position] == nullptr), CHIP_NO_ERROR);

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

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

this->handlerNum--;
// Clear last occupied position
Expand All @@ -457,20 +457,27 @@ CHIP_ERROR DefaultSceneTableImpl::UnregisterHandler(uint8_t position)
return CHIP_NO_ERROR;
}

CHIP_ERROR DefaultSceneTableImpl::SceneSaveEFS(SceneTableEntry & scene, clusterId cluster)
CHIP_ERROR DefaultSceneTableImpl::SceneSaveEFS(SceneTableEntry & scene)
{
ExtensionFieldsSet EFS;
MutableByteSpan EFSSpan = MutableByteSpan(EFS.mBytesBuffer, kMaxFieldsPerCluster);
if (!this->HandlerListEmpty())
{
for (uint8_t i = 0; i < this->handlerNum; i++)
{
clusterId cArray[kMaxClusterPerScenes];
Span<clusterId> cSpan(cArray);
if (this->mHandlers[i] != nullptr)
{
EFS.mID = cluster;
ReturnErrorOnFailure(this->mHandlers[i]->SerializeSave(scene.mStorageId.mEndpointId, cluster, EFSSpan));
EFS.mUsedBytes = (uint8_t) EFSSpan.size();
ReturnErrorOnFailure(scene.mStorageData.mExtensionFieldsSets.InsertFieldSet(EFS));
this->mHandlers[i]->GetSupportedClusters(scene.mStorageId.mEndpointId, cSpan);
for (uint8_t j = 0; j < cSpan.size(); j++)
{
ExtensionFieldsSet EFS;
MutableByteSpan EFSSpan = MutableByteSpan(EFS.mBytesBuffer, kMaxFieldsPerCluster);

EFS.mID = cArray[j];
ReturnErrorOnFailure(this->mHandlers[i]->SerializeSave(scene.mStorageId.mEndpointId, EFS.mID, EFSSpan));
EFS.mUsedBytes = (uint8_t) EFSSpan.size();
ReturnErrorOnFailure(scene.mStorageData.mExtensionFieldsSets.InsertFieldSet(EFS));
}
}
}
}
Expand Down Expand Up @@ -585,16 +592,16 @@ void DefaultSceneTableImpl::SceneEntryIteratorImpl::Release()

namespace {

DefaultSceneTableImpl * gSceneTable = nullptr;
SceneTable * gSceneTable = nullptr;

} // namespace

DefaultSceneTableImpl * GetSceneTable()
SceneTable * GetSceneTable()
{
return gSceneTable;
}

void SetSceneTable(DefaultSceneTableImpl * provider)
void SetSceneTable(SceneTable * provider)
{
gSceneTable = provider;
}
Expand Down
43 changes: 10 additions & 33 deletions src/app/clusters/scenes/SceneTableImpl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ class DefaultSceneHandlerImpl : public scenes::SceneHandler
/// @brief Function to serialize data from an add scene command, assume the incoming extensionFieldSet is initialized
/// @param endpoint Target Endpoint
/// @param cluster Cluster in the Extension field set, filled by the function
/// @param serialyzedBytes Mutable Byte span to hold EFS data from command
/// @param serialisedBytes Mutable Byte span to hold EFS data from command
/// @param extensionFieldSet Extension field set from commmand, pre initialized
/// @return CHIP_NO_ERROR if success, specific CHIP_ERROR otherwise
virtual CHIP_ERROR SerializeAdd(EndpointId endpoint, ClusterId & cluster, MutableByteSpan & serialyzedBytes,
virtual CHIP_ERROR SerializeAdd(EndpointId endpoint, ClusterId & cluster, MutableByteSpan & serialisedBytes,
app::Clusters::Scenes::Structs::ExtensionFieldSet::DecodableType & extensionFieldSet) override
{
app::DataModel::List<app::Clusters::Scenes::Structs::AttributeValuePair::Type> attributeValueList;
Expand Down Expand Up @@ -88,7 +88,7 @@ class DefaultSceneHandlerImpl : public scenes::SceneHandler
attributeValueList = mAVPairs;
attributeValueList.reduce_size(pairCount);

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

extensionFieldSet.clusterID = cluster;
reader.Init(serialyzedBytes);
reader.Init(serialisedBytes);
ReturnErrorOnFailure(reader.Next());
ReturnErrorOnFailure(reader.EnterContainer(outer));
ReturnErrorOnFailure(reader.Next());
Expand Down Expand Up @@ -183,23 +183,19 @@ class DefaultSceneTableImpl : public SceneTable
CHIP_ERROR RemoveSceneTableEntryAtPosition(FabricIndex fabric_index, SceneIndex scened_idx) override;

// SceneHandlers
CHIP_ERROR RegisterHandler(SceneHandler * handler);
CHIP_ERROR UnregisterHandler(uint8_t position);
CHIP_ERROR RegisterHandler(SceneHandler * handler) override;
CHIP_ERROR UnregisterHandler(uint8_t position) override;

// Extension field sets operation
CHIP_ERROR SceneSaveEFS(SceneTableEntry & scene, clusterId cluster);
CHIP_ERROR SceneApplyEFS(FabricIndex fabric_index, const SceneStorageId & scene_id);
CHIP_ERROR SceneSaveEFS(SceneTableEntry & scene) override;
CHIP_ERROR SceneApplyEFS(FabricIndex fabric_index, const SceneStorageId & scene_id) override;

// Fabrics
CHIP_ERROR RemoveFabric(FabricIndex fabric_index) override;

// Iterators
SceneEntryIterator * IterateSceneEntry(FabricIndex fabric_index) override;

bool HandlerListEmpty() { return (handlerNum == 0); }
bool HandlerListFull() { return (handlerNum >= CHIP_CONFIG_SCENES_MAX_CLUSTERS_PER_SCENES); }
uint8_t GetHandlerNum() { return this->handlerNum; }

protected:
class SceneEntryIteratorImpl : public SceneEntryIterator
{
Expand All @@ -222,24 +218,5 @@ class DefaultSceneTableImpl : public SceneTable
ObjectPool<SceneEntryIteratorImpl, kIteratorsMax> mSceneEntryIterators;
}; // class DefaultSceneTableImpl

/**
* Instance getter for the global SceneTable.
*
* Callers have to externally synchronize usage of this function.
*
* @return The global Scene Table
*/
DefaultSceneTableImpl * GetSceneTable();

/**
* Instance setter for the global Scene Table.
*
* Callers have to externally synchronize usage of this function.
*
* The `provider` can be set to nullptr if the owner is done with it fully.
*
* @param[in] provider pointer to the Scene Table global instance to use
*/
void SetSceneTable(DefaultSceneTableImpl * provider);
} // namespace scenes
} // namespace chip

0 comments on commit 4002331

Please sign in to comment.