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 <bzbarsky@apple.com> --------- Co-authored-by: Restyled.io <commits@restyled.io> Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
project-chip · Mar 21, 2024 · 4045187 · 4045187
1 parent 867e5c6
commit 4045187
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 6 deletions.
diff --git 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<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;
@@ -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;

diff --git a/src/app/clusters/scenes-server/SceneTableImpl.h b/src/app/clusters/scenes-server/SceneTableImpl.h
@@ -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:

diff --git 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
@@ -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);