Skip to content

Commit

Permalink
Bugfix/scene save efs dynamical array (#27078)
Browse files Browse the repository at this point in the history
* Replaced static clusterId array from SceneSaveEFS to a dynamical one.

* Added wrapper and shim to use GetClusterCountFromEndpoint in tests with expected results from TestSceneTable

* Replaced Platform memory alloc with ScopedMemoryBuffer and removed reallocation

* Update src/app/clusters/scenes-server/SceneTableImpl.cpp

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

---------

Co-authored-by: Restyled.io <commits@restyled.io>
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
  • Loading branch information
3 people authored and pull[bot] committed Sep 29, 2023
1 parent a66d409 commit 1458942
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 6 deletions.
21 changes: 16 additions & 5 deletions src/app/clusters/scenes-server/SceneTableImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -737,11 +737,14 @@ CHIP_ERROR DefaultSceneTableImpl::SceneSaveEFS(SceneTableEntry & scene)
{
if (!HandlerListEmpty())
{
uint8_t clusterCount = 0;
clusterId cArray[kMaxClustersPerScene];
Span<clusterId> cSpan(cArray);
clusterCount = GetClustersFromEndpoint(cArray, kMaxClustersPerScene);
cSpan.reduce_size(clusterCount);
// TODO : Once zap supports the scenable quality, implement a GetSceneableClusterCountFromEndpointType function to avoid
// over-allocation
uint8_t clusterCount = GetClusterCountFromEndpoint();
chip::Platform::ScopedMemoryBuffer<clusterId> cBuffer;
VerifyOrReturnError(cBuffer.Calloc(clusterCount), CHIP_ERROR_NO_MEMORY);
clusterCount = GetClustersFromEndpoint(cBuffer.Get(), clusterCount);

Span<clusterId> cSpan(cBuffer.Get(), clusterCount);
for (clusterId cluster : cSpan)
{
ExtensionFieldSet EFS;
Expand Down Expand Up @@ -836,6 +839,14 @@ uint8_t DefaultSceneTableImpl::GetClustersFromEndpoint(ClusterId * clusterList,
return emberAfGetClustersFromEndpoint(mEndpointId, clusterList, listLen, true);
}

/// @brief wrapper function around emberAfGetClusterCountForEndpoint to allow testing enforcing a specific count, shimmed in test
/// configuration because emberAfGetClusterCountForEndpoint relies on <app/util/attribute-storage.h>, which relies on zap generated
/// files
uint8_t DefaultSceneTableImpl::GetClusterCountFromEndpoint()
{
return emberAfGetClusterCountForEndpoint(mEndpointId);
}

void DefaultSceneTableImpl::SetEndpoint(EndpointId endpoint)
{
mEndpointId = endpoint;
Expand Down
3 changes: 3 additions & 0 deletions src/app/clusters/scenes-server/SceneTableImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,9 @@ class DefaultSceneTableImpl : public SceneTable<scenes::ExtensionFieldSetsImpl>
// wrapper function around emberAfGetClustersFromEndpoint to allow override when testing
virtual uint8_t GetClustersFromEndpoint(ClusterId * clusterList, uint8_t listLen);

// wrapper function around emberAfGetClusterCountForEndpoint to allow override when testing
virtual uint8_t GetClusterCountFromEndpoint();

class SceneEntryIteratorImpl : public SceneEntryIterator
{
public:
Expand Down
2 changes: 2 additions & 0 deletions src/app/tests/TestSceneTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,8 @@ class TestSceneTableImpl : public SceneTableImpl

return 0;
}

uint8_t GetClusterCountFromEndpoint() override { return 3; }
};

// Storage
Expand Down
7 changes: 6 additions & 1 deletion src/app/util/mock/attribute-storage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ uint16_t emberAfIndexFromEndpoint(chip::EndpointId endpoint)
return UINT16_MAX;
}

uint8_t emberAfClusterCount(chip::EndpointId endpoint, bool server)
uint8_t emberAfGetClusterCountForEndpoint(chip::EndpointId endpoint)
{
for (size_t i = 0; i < ArraySize(endpoints); i++)
{
Expand All @@ -128,6 +128,11 @@ uint8_t emberAfClusterCount(chip::EndpointId endpoint, bool server)
return 0;
}

uint8_t emberAfClusterCount(chip::EndpointId endpoint, bool server)
{
return emberAfGetClusterCountForEndpoint(endpoint);
}

uint16_t emberAfGetServerAttributeCount(chip::EndpointId endpoint, chip::ClusterId cluster)
{
uint16_t endpointIndex = emberAfIndexFromEndpoint(endpoint);
Expand Down

0 comments on commit 1458942

Please sign in to comment.