Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
  • Loading branch information
2 people authored and pull[bot] committed Jun 24, 2023
1 parent c23116f commit 2557153
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 15 deletions.
3 changes: 1 addition & 2 deletions src/app/clusters/scenes/ExtensionFieldSetsImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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++)
{
Expand Down Expand Up @@ -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;
}
Expand Down
17 changes: 8 additions & 9 deletions src/app/clusters/scenes/SceneTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
};

Expand Down
2 changes: 1 addition & 1 deletion src/app/clusters/scenes/SceneTableImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions src/app/clusters/scenes/SceneTableImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));

Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit 2557153

Please sign in to comment.