Skip to content

Commit

Permalink
Refactor of variable, members and functions according to suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
@lpbeliveau-silabs @pull
lpbeliveau-silabs authored and pull[bot] committed Aug 29, 2023
1 parent e9a79b6 commit 3735667
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 88 deletions.
28 changes: 14 additions & 14 deletions src/app/clusters/scenes/ExtensionFieldSetsImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ CHIP_ERROR ExtensionFieldSetsImpl::Serialize(TLV::TLVWriter & writer) const
{
for (uint8_t i = 0; i < mFieldSetsCount; i++)
{
if (!mEFS[i].IsEmpty())
if (!mFieldSets[i].IsEmpty())
{
ReturnErrorOnFailure(mEFS[i].Serialize(writer));
ReturnErrorOnFailure(mFieldSets[i].Serialize(writer));
}
}
}
Expand All @@ -55,7 +55,7 @@ CHIP_ERROR ExtensionFieldSetsImpl::Deserialize(TLV::TLVReader & reader)
{
for (uint8_t i = 0; i < mFieldSetsCount; i++)
{
ReturnErrorOnFailure(mEFS[i].Deserialize(reader));
ReturnErrorOnFailure(mFieldSets[i].Deserialize(reader));
}
}

Expand All @@ -68,7 +68,7 @@ void ExtensionFieldSetsImpl::Clear()
{
for (uint8_t i = 0; i < mFieldSetsCount; i++)
{
mEFS[i].Clear();
mFieldSets[i].Clear();
}
}
mFieldSetsCount = 0;
Expand All @@ -85,24 +85,24 @@ CHIP_ERROR ExtensionFieldSetsImpl::InsertFieldSet(const ExtensionFieldSet & fiel
VerifyOrReturnError(fieldSet.mID != kInvalidClusterId, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(!fieldSet.IsEmpty(), CHIP_ERROR_INVALID_ARGUMENT);

for (uint8_t i = 0; i < kMaxClusterPerScenes; i++)
for (uint8_t i = 0; i < kMaxClustersPerScene; i++)
{
if (mEFS[i].mID == fieldSet.mID)
if (mFieldSets[i].mID == fieldSet.mID)
{
mEFS[i] = fieldSet;
mFieldSets[i] = fieldSet;
return CHIP_NO_ERROR;
}

if (mEFS[i].IsEmpty() && firstEmptyPosition == kInvalidPosition)
if (mFieldSets[i].IsEmpty() && firstEmptyPosition == kInvalidPosition)
{
firstEmptyPosition = i;
}
}

// if found, replace at found position, otherwise insert at first free position, otherwise return error
if (firstEmptyPosition < kMaxClusterPerScenes)
if (firstEmptyPosition < kMaxClustersPerScene)
{
mEFS[firstEmptyPosition] = fieldSet;
mFieldSets[firstEmptyPosition] = fieldSet;
mFieldSetsCount++;
return CHIP_NO_ERROR;
}
Expand All @@ -114,7 +114,7 @@ CHIP_ERROR ExtensionFieldSetsImpl::GetFieldSetAtPosition(ExtensionFieldSet & fie
{
VerifyOrReturnError(position < mFieldSetsCount, CHIP_ERROR_BUFFER_TOO_SMALL);

fieldSet = mEFS[position];
fieldSet = mFieldSets[position];

return CHIP_NO_ERROR;
}
Expand All @@ -124,18 +124,18 @@ CHIP_ERROR ExtensionFieldSetsImpl::RemoveFieldAtPosition(uint8_t position)
VerifyOrReturnValue(position < mFieldSetsCount, CHIP_NO_ERROR);

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

// TODO: Implement general array management methods
// Compress array after removal, if the removed position is not the last
if (moveNum)
{
memmove(&mEFS[position], &mEFS[nextPos], sizeof(ExtensionFieldSet) * moveNum);
memmove(&mFieldSets[position], &mFieldSets[nextPos], sizeof(ExtensionFieldSet) * moveNum);
}

mFieldSetsCount--;
// Clear last occupied position
mEFS[mFieldSetsCount].Clear(); //
mFieldSets[mFieldSetsCount].Clear(); //

return CHIP_NO_ERROR;
}
Expand Down
9 changes: 4 additions & 5 deletions src/app/clusters/scenes/ExtensionFieldSetsImpl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,7 @@ struct ExtensionFieldSet

bool operator==(const ExtensionFieldSet & other) const
{
return (mID == other.mID && mUsedBytes == other.mUsedBytes &&
!memcmp(mBytesBuffer, other.mBytesBuffer, mUsedBytes));
return (mID == other.mID && mUsedBytes == other.mUsedBytes && !memcmp(mBytesBuffer, other.mBytesBuffer, mUsedBytes));
}
};

Expand All @@ -144,7 +143,7 @@ class ExtensionFieldSetsImpl : public ExtensionFieldSets
{
for (uint8_t i = 0; i < mFieldSetsCount; i++)
{
if (!(this->mEFS[i] == other.mEFS[i]))
if (!(this->mFieldSets[i] == other.mFieldSets[i]))
{
return false;
}
Expand All @@ -156,15 +155,15 @@ class ExtensionFieldSetsImpl : public ExtensionFieldSets
{
for (uint8_t i = 0; i < other.mFieldSetsCount; i++)
{
this->mEFS[i] = other.mEFS[i];
this->mFieldSets[i] = other.mFieldSets[i];
}
mFieldSetsCount = other.mFieldSetsCount;

return *this;
}

protected:
ExtensionFieldSet mFieldSets[kMaxClusterPerScenes];
ExtensionFieldSet mFieldSets[kMaxClustersPerScene];
uint8_t mFieldSetsCount = 0;
};
} // namespace scenes
Expand Down
39 changes: 16 additions & 23 deletions src/app/clusters/scenes/SceneTable.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,15 @@ typedef uint8_t SceneIndex;
typedef uint32_t TransitionTimeMs;
typedef uint32_t SceneTransitionTime;

constexpr GroupId kGlobalGroupSceneId = 0x0000;
constexpr SceneIndex kUndefinedSceneIndex = 0xff;
constexpr SceneId kUndefinedSceneId = 0xff;
constexpr GroupId kGlobalGroupSceneId = 0x0000;
constexpr SceneIndex kUndefinedSceneIndex = 0xff;
constexpr SceneId kUndefinedSceneId = 0xff;
static constexpr uint8_t kMaxScenesPerFabric = CHIP_CONFIG_SCENES_MAX_PER_FABRIC;

static constexpr size_t kIteratorsMax = CHIP_CONFIG_MAX_SCENES_CONCURRENT_ITERATORS;
static constexpr size_t kSceneNameMaxLength = CHIP_CONFIG_SCENES_CLUSTER_MAXIMUM_NAME_LENGTH;


/// @brief SceneHandlers are meant as interface between various clusters and the Scene table.
/// @brief SceneHandlers are meant as interface between various clusters and the Scene table.
/// When a scene command involving extension field sets is received, the Scene Table will go through
/// the list of handlers to either retrieve, populate or apply those extension field sets.
///
Expand All @@ -51,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 handl 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 All @@ -66,20 +65,20 @@ class SceneHandler : public IntrusiveListNodeBase<>
/// @param endpoint target endpoint
/// @param clusterBuffer Buffer to hold the supported cluster IDs, cannot hold more than
/// CHIP_CONFIG_SCENES_MAX_CLUSTERS_PER_SCENE. The function shall use the reduce_size() method in the event it is supporting

/// less than CHIP_CONFIG_SCENES_MAX_CLUSTERS_PER_SCENE clusters.

virtual void GetSupportedClusters(EndpointId endpoint, Span<ClusterId> & clusterBuffer) = 0;

/// @brief Returns whether or not a cluster for scenes 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 Called when handling AddScene. Allows the handler to filter through the clusters in the command to serialize only the supported ones.

/// @brief Called when handling AddScene. Allows the handler to filter through the clusters in the command to serialize only
/// the supported ones.
///
/// @param endpoint[in] Endpoint ID
/// @param extensionFieldSet[in] ExtensionFieldSets provided by the AddScene Command, pre initialized
/// @param cluster[out] Cluster in the Extension field set, filled by the function
Expand All @@ -92,8 +91,7 @@ class SceneHandler : public IntrusiveListNodeBase<>
/// @brief Called when handling StoreScene, and only if the handler supports the given endpoint and cluster.
///
/// The implementation must write the actual scene data to store to serializedBytes as described below.


///
/// @param endpoint[in] Target Endpoint
/// @param cluster[in] Target Cluster
/// @param serializedBytes[out] Output buffer, data needs to be writen in there and size adjusted to the size of the data
Expand All @@ -103,23 +101,23 @@ class SceneHandler : public IntrusiveListNodeBase<>
virtual CHIP_ERROR SerializeSave(EndpointId endpoint, ClusterId cluster, MutableByteSpan & serializedBytes) = 0;

/// @brief Deserialize an ExtensionFieldSet into a cluster object (e.g. when handling ViewScene).

///
/// @param endpoint[in] Endpoint ID
/// @param cluster[in] Cluster ID to save
/// @param serializedBytes[in] ExtensionFieldSet stored in NVM

///
/// @param extensionFieldSet[out] ExtensionFieldSet in command format
/// @return CHIP_NO_ERROR if successful, CHIP_ERROR value otherwise
virtual CHIP_ERROR Deserialize(EndpointId endpoint, ClusterId cluster, const ByteSpan & serializedBytes,

app::Clusters::Scenes::Structs::ExtensionFieldSet::Type & extensionFieldSet) = 0;

/// @brief Restore a stored scene for the given cluster instance, over timeMs milliseconds (e.g. when handling RecallScene)

///
/// @param endpoint[in] Endpoint ID
/// @param cluster[in] Cluster ID
/// @param serializedBytes[in] ExtensionFieldSet stored in NVM

///
/// @param timeMs[in] Transition time in ms to apply the scene
/// @return CHIP_NO_ERROR if successful, CHIP_ERROR value otherwise
virtual CHIP_ERROR ApplyScene(EndpointId endpoint, ClusterId cluster, const ByteSpan & serializedBytes,
Expand Down Expand Up @@ -154,7 +152,6 @@ class SceneTable
bool operator==(const SceneStorageId & other)
{
return (mEndpointId == other.mEndpointId && mGroupId == other.mGroupId && mSceneId == other.mSceneId);

}
};

Expand All @@ -168,15 +165,14 @@ class SceneTable
/// mSceneTransitionTimeSeconds in enhanced scene commands
struct SceneData
{
char mName[kSceneNameMax] = { 0 };
char mName[kSceneNameMaxLength] = { 0 };
size_t mNameLength = 0;
SceneTransitionTime mSceneTransitionTimeMs = 0;
EFStype mExtensionFieldSets;

SceneData(const CharSpan & sceneName = CharSpan(), SceneTransitionTime time = 0) : mSceneTransitionTimeMs(time)
{
SetName(sceneName);

}
SceneData(EFStype fields, const CharSpan & sceneName = CharSpan(), SceneTransitionTime time = 0) :
mSceneTransitionTimeMs(time)
Expand All @@ -202,7 +198,7 @@ class SceneTable
}
else
{
size_t maxChars = std::min(sceneName.size(), kSceneNameMax);
size_t maxChars = std::min(sceneName.size(), kSceneNameMaxLength);
memcpy(mName, sceneName.data(), maxChars);
mNameLength = maxChars;
}
Expand Down Expand Up @@ -247,14 +243,12 @@ class SceneTable
bool operator==(const SceneTableEntry & other)
{
return (mStorageId == other.mStorageId && mStorageData == other.mStorageData);

}

void operator=(const SceneTableEntry & other)
{
mStorageId = other.mStorageId;
mStorageData = other.mStorageData;

}
};

Expand Down Expand Up @@ -296,7 +290,6 @@ class SceneTable
// Handlers
virtual bool HandlerListEmpty() { return mHandlerList.Empty(); }


// SceneHandler * mHandlers[kMaxSceneHandlers] = { nullptr };
IntrusiveList<SceneHandler> mHandlerList;
};
Expand Down
Loading

0 comments on commit 3735667

Please sign in to comment.