From 145894256440a8663c91b2fac4958eb0654ae6c0 Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs <112982107+lpbeliveau-silabs@users.noreply.github.com> Date: Mon, 5 Jun 2023 19:33:58 -0400 Subject: [PATCH] Bugfix/scene save efs dynamical array (#27078) * 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 --------- Co-authored-by: Restyled.io Co-authored-by: Boris Zbarsky --- .../clusters/scenes-server/SceneTableImpl.cpp | 21 ++++++++++++++----- .../clusters/scenes-server/SceneTableImpl.h | 3 +++ src/app/tests/TestSceneTable.cpp | 2 ++ src/app/util/mock/attribute-storage.cpp | 7 ++++++- 4 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/app/clusters/scenes-server/SceneTableImpl.cpp b/src/app/clusters/scenes-server/SceneTableImpl.cpp index f0b5131f033776..f4e3de33d94634 100644 --- a/src/app/clusters/scenes-server/SceneTableImpl.cpp +++ b/src/app/clusters/scenes-server/SceneTableImpl.cpp @@ -737,11 +737,14 @@ CHIP_ERROR DefaultSceneTableImpl::SceneSaveEFS(SceneTableEntry & scene) { if (!HandlerListEmpty()) { - uint8_t clusterCount = 0; - clusterId cArray[kMaxClustersPerScene]; - Span 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 cBuffer; + VerifyOrReturnError(cBuffer.Calloc(clusterCount), CHIP_ERROR_NO_MEMORY); + clusterCount = GetClustersFromEndpoint(cBuffer.Get(), clusterCount); + + Span cSpan(cBuffer.Get(), clusterCount); for (clusterId cluster : cSpan) { ExtensionFieldSet EFS; @@ -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 , which relies on zap generated +/// files +uint8_t DefaultSceneTableImpl::GetClusterCountFromEndpoint() +{ + return emberAfGetClusterCountForEndpoint(mEndpointId); +} + void DefaultSceneTableImpl::SetEndpoint(EndpointId endpoint) { mEndpointId = endpoint; diff --git a/src/app/clusters/scenes-server/SceneTableImpl.h b/src/app/clusters/scenes-server/SceneTableImpl.h index 7298f60a310e79..b70a50ba1e8155 100644 --- a/src/app/clusters/scenes-server/SceneTableImpl.h +++ b/src/app/clusters/scenes-server/SceneTableImpl.h @@ -204,6 +204,9 @@ class DefaultSceneTableImpl : public SceneTable // 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: diff --git a/src/app/tests/TestSceneTable.cpp b/src/app/tests/TestSceneTable.cpp index 25808686828d16..8bbbdb350fb900 100644 --- a/src/app/tests/TestSceneTable.cpp +++ b/src/app/tests/TestSceneTable.cpp @@ -405,6 +405,8 @@ class TestSceneTableImpl : public SceneTableImpl return 0; } + + uint8_t GetClusterCountFromEndpoint() override { return 3; } }; // Storage diff --git a/src/app/util/mock/attribute-storage.cpp b/src/app/util/mock/attribute-storage.cpp index d763f5646a8cfa..208ddec7b523e4 100644 --- a/src/app/util/mock/attribute-storage.cpp +++ b/src/app/util/mock/attribute-storage.cpp @@ -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++) { @@ -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);