From 25571539e4c485cb07ed375606f5d1cad2defe46 Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs <112982107+lpbeliveau-silabs@users.noreply.github.com> Date: Thu, 23 Mar 2023 17:00:32 -0400 Subject: [PATCH] Apply suggestions from code review Co-authored-by: Boris Zbarsky --- .../clusters/scenes/ExtensionFieldSetsImpl.cpp | 3 +-- src/app/clusters/scenes/SceneTable.h | 17 ++++++++--------- src/app/clusters/scenes/SceneTableImpl.cpp | 2 +- src/app/clusters/scenes/SceneTableImpl.h | 6 +++--- 4 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/app/clusters/scenes/ExtensionFieldSetsImpl.cpp b/src/app/clusters/scenes/ExtensionFieldSetsImpl.cpp index 8c9c2870a7ed3c..d45f0bdd2eb3ed 100644 --- a/src/app/clusters/scenes/ExtensionFieldSetsImpl.cpp +++ b/src/app/clusters/scenes/ExtensionFieldSetsImpl.cpp @@ -26,7 +26,6 @@ CHIP_ERROR ExtensionFieldSetsImpl::Serialize(TLV::TLVWriter & writer, TLV::Tag s { TLV::TLVType container; ReturnErrorOnFailure(writer.StartContainer(structTag, TLV::kTLVType_Structure, container)); - // ReturnErrorOnFailure(writer.Put(TLV::ContextTag(TagEFS::kFieldSetsCount), static_cast(mFieldSetsCount))); ReturnErrorOnFailure(writer.StartContainer(TLV::ContextTag(TagEFS::kFieldSetArrayContainer), TLV::kTLVType_Array, container)); for (uint8_t i = 0; i < mFieldSetsCount; i++) { @@ -132,7 +131,7 @@ CHIP_ERROR ExtensionFieldSetsImpl::RemoveFieldAtPosition(uint8_t position) mFieldSetsCount--; // Clear last occupied position - mFieldSets[mFieldSetsCount].Clear(); // + mFieldSets[mFieldSetsCount].Clear(); return CHIP_NO_ERROR; } diff --git a/src/app/clusters/scenes/SceneTable.h b/src/app/clusters/scenes/SceneTable.h index 8333416b86f286..5736077c69e471 100644 --- a/src/app/clusters/scenes/SceneTable.h +++ b/src/app/clusters/scenes/SceneTable.h @@ -50,7 +50,7 @@ static constexpr size_t kSceneNameMaxLength = CHIP_CONFIG_SCENES_CLUSTER_MAXIMUM /// /// A SceneHandler can handle a single pair, or many such pairs. /// -/// @note If more than one handler claims to handl a specific pair, only one of +/// @note If more than one handler claims to handle a specific pair, only one of /// those handlers will get called when executing actions related to extension field sets on the scene /// table. It is not defined which handler will be selected. @@ -110,8 +110,7 @@ class SceneHandler : public IntrusiveListNodeBase<> /// /// @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. + /// @note Only gets called for handlers for which SupportsCluster() is true for the given endpoint and cluster. virtual CHIP_ERROR Deserialize(EndpointId endpoint, ClusterId cluster, const ByteSpan & serializedBytes, app::Clusters::Scenes::Structs::ExtensionFieldSet::Type & extensionFieldSet) = 0; @@ -220,16 +219,16 @@ class SceneTable bool operator==(const SceneData & other) { - return (this->mNameLength == other.mNameLength && !memcmp(this->mName, other.mName, this->mNameLength) && - (this->mSceneTransitionTimeMs == other.mSceneTransitionTimeMs) && - (this->mExtensionFieldSets == other.mExtensionFieldSets)); + return (mNameLength == other.mNameLength && !memcmp(mName, other.mName,mNameLength) && + (mSceneTransitionTimeMs == other.mSceneTransitionTimeMs) && + (mExtensionFieldSets == other.mExtensionFieldSets)); } void operator=(const SceneData & other) { - this->SetName(CharSpan(other.mName, other.mNameLength)); - this->mExtensionFieldSets = other.mExtensionFieldSets; - this->mSceneTransitionTimeMs = other.mSceneTransitionTimeMs; + SetName(CharSpan(other.mName, other.mNameLength)); + mExtensionFieldSets = other.mExtensionFieldSets; + mSceneTransitionTimeMs = other.mSceneTransitionTimeMs; } }; diff --git a/src/app/clusters/scenes/SceneTableImpl.cpp b/src/app/clusters/scenes/SceneTableImpl.cpp index e439f5b9721374..6e183a93cf93c5 100644 --- a/src/app/clusters/scenes/SceneTableImpl.cpp +++ b/src/app/clusters/scenes/SceneTableImpl.cpp @@ -24,7 +24,7 @@ namespace chip { namespace scenes { /// @brief Tags Used to serialize Scenes so they can be stored in flash memory. -/// kSceneCount: Number of scene in a Fabric +/// kSceneCount: Number of scenes 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 diff --git a/src/app/clusters/scenes/SceneTableImpl.h b/src/app/clusters/scenes/SceneTableImpl.h index f10b89953e4375..c566dcf5a05294 100644 --- a/src/app/clusters/scenes/SceneTableImpl.h +++ b/src/app/clusters/scenes/SceneTableImpl.h @@ -50,7 +50,7 @@ class DefaultSceneHandlerImpl : public scenes::SceneHandler /// @return CHIP_NO_ERROR if successful, CHIP_ERROR_INVALID_ARGUMENT if the cluster is not supported, CHIP_ERROR value otherwise virtual CHIP_ERROR SerializeAdd(EndpointId endpoint, const app::Clusters::Scenes::Structs::ExtensionFieldSet::DecodableType & extensionFieldSet, - ClusterId & cluster, MutableByteSpan & serialisedBytes) override + ClusterId & cluster, MutableByteSpan & serializedBytes) override { app::Clusters::Scenes::Structs::AttributeValuePair::DecodableType aVPair; TLV::TLVWriter writer; @@ -93,7 +93,7 @@ class DefaultSceneHandlerImpl : public scenes::SceneHandler 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)), + writer, TLV::ContextTag(app::Clusters::Scenes::Structs::ExtensionFieldSet::Fields::kAttributeValueList), attributeValueList)); ReturnErrorOnFailure(writer.EndContainer(outer)); @@ -125,7 +125,7 @@ class DefaultSceneHandlerImpl : public scenes::SceneHandler ReturnErrorOnFailure(reader.EnterContainer(outer)); ReturnErrorOnFailure(reader.Next( TLV::kTLVType_Array, - TLV::ContextTag(to_underlying(app::Clusters::Scenes::Structs::ExtensionFieldSet::Fields::kAttributeValueList)))); + TLV::ContextTag(app::Clusters::Scenes::Structs::ExtensionFieldSet::Fields::kAttributeValueList))); attributeValueList.Decode(reader); auto pair_iterator = attributeValueList.begin();