Apply suggestions from code review

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

Author: 2 people

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<uint8_t>(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;
 }

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 <endpoint, cluster> pair, or many such pairs.
 ///
-/// @note If more than one handler claims to handl a specific <endpoint, cluster> pair, only one of
+/// @note If more than one handler claims to handle a specific <endpoint, cluster> 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;
         }
     };
 

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

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();