Removed global SceneTable functions and pointer, reworked UnregisterH…

…anlder

src/app/chip_data_model.gni

@@ -157,9 +157,9 @@ template("chip_data_model") {
       "${_app_root}/clusters/identify-server/identify-server.h",
       "${_app_root}/clusters/level-control/level-control.h",
       "${_app_root}/clusters/on-off-server/on-off-server.h",
-      "${_app_root}/clusters/scenes/ExtensionFieldsSets.h",
-      "${_app_root}/clusters/scenes/ExtensionFieldsSetsImpl.cpp",
-      "${_app_root}/clusters/scenes/ExtensionFieldsSetsImpl.h",
+      "${_app_root}/clusters/scenes/ExtensionFieldSets.h",
+      "${_app_root}/clusters/scenes/ExtensionFieldSetsImpl.cpp",
+      "${_app_root}/clusters/scenes/ExtensionFieldSetsImpl.h",
       "${_app_root}/clusters/scenes/SceneTable.h",
       "${_app_root}/clusters/scenes/SceneTableImpl.cpp",
       "${_app_root}/clusters/scenes/SceneTableImpl.h",

src/app/clusters/scenes/ExtensionFieldsSetsImpl.cpp → src/app/clusters/scenes/ExtensionFieldSetsImpl.cpp

@@ -15,14 +15,14 @@
  *    limitations under the License.
  */
 
-#include <app/clusters/scenes/ExtensionFieldsSetsImpl.h>
+#include <app/clusters/scenes/ExtensionFieldSetsImpl.h>
 
 namespace chip {
 namespace scenes {
 
-ExtensionFieldsSetsImpl::ExtensionFieldsSetsImpl() : ExtensionFieldsSets() {}
+ExtensionFieldSetsImpl::ExtensionFieldSetsImpl() : ExtensionFieldsSets() {}
 
-CHIP_ERROR ExtensionFieldsSetsImpl::Serialize(TLV::TLVWriter & writer) const
+CHIP_ERROR ExtensionFieldSetsImpl::Serialize(TLV::TLVWriter & writer) const
 {
     TLV::TLVType container;
     ReturnErrorOnFailure(writer.StartContainer(TLV::ContextTag(kTagEFSArrayContainer), TLV::kTLVType_Structure, container));
@@ -41,7 +41,7 @@ CHIP_ERROR ExtensionFieldsSetsImpl::Serialize(TLV::TLVWriter & writer) const
     return writer.EndContainer(container);
 }
 
-CHIP_ERROR ExtensionFieldsSetsImpl::Deserialize(TLV::TLVReader & reader)
+CHIP_ERROR ExtensionFieldSetsImpl::Deserialize(TLV::TLVReader & reader)
 {
     TLV::TLVType container;
     ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Structure, TLV::ContextTag(kTagEFSArrayContainer)));
@@ -61,7 +61,7 @@ CHIP_ERROR ExtensionFieldsSetsImpl::Deserialize(TLV::TLVReader & reader)
     return reader.ExitContainer(container);
 }
 
-void ExtensionFieldsSetsImpl::Clear()
+void ExtensionFieldSetsImpl::Clear()
 {
     if (!this->IsEmpty())
     {
@@ -77,7 +77,7 @@ void ExtensionFieldsSetsImpl::Clear()
 ///        If the same ID is present in the EFS array, it will overwrite it.
 /// @param fieldSet field set to be inserted
 /// @return CHIP_NO_ERROR if insertion worked, CHIP_ERROR_NO_MEMORY if the array is already full
-CHIP_ERROR ExtensionFieldsSetsImpl::InsertFieldSet(ExtensionFieldsSet & fieldSet)
+CHIP_ERROR ExtensionFieldSetsImpl::InsertFieldSet(ExtensionFieldsSet & fieldSet)
 {
     CHIP_ERROR err             = CHIP_ERROR_NO_MEMORY;
     uint8_t idPosition         = kInvalidPosition;
@@ -115,7 +115,7 @@ CHIP_ERROR ExtensionFieldsSetsImpl::InsertFieldSet(ExtensionFieldsSet & fieldSet
     return err;
 }
 
-CHIP_ERROR ExtensionFieldsSetsImpl::GetFieldSetAtPosition(ExtensionFieldsSet & fieldSet, uint8_t position)
+CHIP_ERROR ExtensionFieldSetsImpl::GetFieldSetAtPosition(ExtensionFieldsSet & fieldSet, uint8_t position)
 {
     VerifyOrReturnError(position < this->mFieldNum, CHIP_ERROR_BUFFER_TOO_SMALL);
 
@@ -124,7 +124,7 @@ CHIP_ERROR ExtensionFieldsSetsImpl::GetFieldSetAtPosition(ExtensionFieldsSet & f
     return CHIP_NO_ERROR;
 }
 
-CHIP_ERROR ExtensionFieldsSetsImpl::RemoveFieldAtPosition(uint8_t position)
+CHIP_ERROR ExtensionFieldSetsImpl::RemoveFieldAtPosition(uint8_t position)
 {
     VerifyOrReturnError(position < kMaxClusterPerScenes, CHIP_ERROR_INVALID_ARGUMENT);
     VerifyOrReturnValue(!this->IsEmpty() && !this->mEFS[position].IsEmpty(), CHIP_NO_ERROR);

src/app/clusters/scenes/ExtensionFieldsSetsImpl.h → src/app/clusters/scenes/ExtensionFieldSetsImpl.h

@@ -17,7 +17,7 @@
 
 #pragma once
 
-#include <app/clusters/scenes/ExtensionFieldsSets.h>
+#include <app/clusters/scenes/ExtensionFieldSets.h>
 #include <lib/core/DataModelTypes.h>
 
 namespace chip {
@@ -105,11 +105,11 @@ struct ExtensionFieldsSet
     }
 };
 
-class ExtensionFieldsSetsImpl : public ExtensionFieldsSets
+class ExtensionFieldSetsImpl : public ExtensionFieldsSets
 {
 public:
-    ExtensionFieldsSetsImpl();
-    ~ExtensionFieldsSetsImpl() override{};
+    ExtensionFieldSetsImpl();
+    ~ExtensionFieldSetsImpl() override{};
 
     // overrides
     CHIP_ERROR Serialize(TLV::TLVWriter & writer) const override;
@@ -123,7 +123,7 @@ class ExtensionFieldsSetsImpl : public ExtensionFieldsSets
     CHIP_ERROR GetFieldSetAtPosition(ExtensionFieldsSet & field, uint8_t position);
     CHIP_ERROR RemoveFieldAtPosition(uint8_t position);
 
-    bool operator==(const ExtensionFieldsSetsImpl & other)
+    bool operator==(const ExtensionFieldSetsImpl & other)
     {
         for (uint8_t i = 0; i < kMaxClusterPerScenes; i++)
         {
@@ -135,7 +135,7 @@ class ExtensionFieldsSetsImpl : public ExtensionFieldsSets
         return true;
     }
 
-    ExtensionFieldsSetsImpl & operator=(const ExtensionFieldsSetsImpl & other)
+    ExtensionFieldSetsImpl & operator=(const ExtensionFieldSetsImpl & other)
     {
         for (uint8_t i = 0; i < kMaxClusterPerScenes; i++)
         {

src/app/clusters/scenes/SceneTable.h

@@ -17,7 +17,7 @@
 #pragma once
 
 #include <app-common/zap-generated/cluster-objects.h>
-#include <app/clusters/scenes/ExtensionFieldsSetsImpl.h>
+#include <app/clusters/scenes/ExtensionFieldSetsImpl.h>
 #include <lib/support/CHIPMemString.h>
 #include <lib/support/CommonIterator.h>
 #include <lib/support/CommonPersistentData.h>
@@ -56,10 +56,12 @@ class SceneHandler
     SceneHandler(){};
     virtual ~SceneHandler() = default;
 
-    /// @brief Gets the list of supported clusters for an endpoint
+    /// @brief Copies the list of supported clusters for an endpoint in a Span and resizes the span to fit the actual number of
+    /// supported clusters
     /// @param endpoint target endpoint
     /// @param clusterBuffer Buffer to hold the supported cluster IDs, cannot hold more than
-    /// CHIP_CONFIG_SCENES_MAX_CLUSTERS_PER_SCENES
+    /// CHIP_CONFIG_SCENES_MAX_CLUSTERS_PER_SCENES, the function shall use the reduce_size() method in the event it is supporting
+    /// less than CHIP_CONFIG_SCENES_MAX_CLUSTERS_PER_SCENES clusters
     virtual void GetSupportedClusters(EndpointId endpoint, Span<ClusterId> & clusterBuffer) = 0;
 
     /// @brief Returns whether or not a cluster for scenes is supported on an endpoint
@@ -78,8 +80,8 @@ class SceneHandler
     virtual CHIP_ERROR SerializeAdd(EndpointId endpoint, ClusterId & cluster, MutableByteSpan & serialisedBytes,
                                     app::Clusters::Scenes::Structs::ExtensionFieldSet::DecodableType & extensionFieldSet) = 0;
 
-    /// @brief From command StoreScene, retrieves ExtensionField from nvm, it is the functions responsability to resize the mutable
-    /// span if necessary, a number of byte equal to the span will be stored in memory
+    /// @brief From command StoreScene, retrieves ExtensionField from currently active values, it is the functions responsability to
+    /// resize the mutable span if necessary, a number of byte equal to the span will be stored in memory
     /// @param endpoint Target Endpoint
     /// @param cluster Target Cluster
     /// @param serialisedBytes Output buffer, data needs to be writen in there and size adjusted if smaller than
@@ -176,7 +178,7 @@ class SceneTable
         char mName[kSceneNameMax]                = { 0 };
         size_t mNameLength                       = 0;
         SceneTransitionTime mSceneTransitionTime = 0;
-        ExtensionFieldsSetsImpl mExtensionFieldsSets;
+        ExtensionFieldSetsImpl mExtensionFieldSets;
         TransitionTime100ms mTransitionTime100ms = 0;
         CharSpan mNameSpan;
 
@@ -185,19 +187,19 @@ class SceneTable
         {
             this->SetName(sceneName);
         }
-        SceneData(ExtensionFieldsSetsImpl fields, const CharSpan & sceneName = CharSpan(), SceneTransitionTime time = 0,
+        SceneData(ExtensionFieldSetsImpl fields, const CharSpan & sceneName = CharSpan(), SceneTransitionTime time = 0,
                   TransitionTime100ms time100ms = 0) :
             mSceneTransitionTime(time),
             mTransitionTime100ms(time100ms)
         {
             this->SetName(sceneName);
-            mExtensionFieldsSets = fields;
+            mExtensionFieldSets = fields;
         }
         SceneData(const SceneData & other) :
             mSceneTransitionTime(other.mSceneTransitionTime), mTransitionTime100ms(other.mTransitionTime100ms)
         {
             this->SetName(other.mNameSpan);
-            mExtensionFieldsSets = other.mExtensionFieldsSets;
+            mExtensionFieldSets = other.mExtensionFieldSets;
         }
         ~SceneData(){};
 
@@ -217,7 +219,7 @@ class SceneTable
                 writer.Put(TLV::ContextTag(kTagSceneDTransitionTime), static_cast<uint16_t>(this->mSceneTransitionTime)));
             ReturnErrorOnFailure(
                 writer.Put(TLV::ContextTag(kTagSceneDTransitionTime100), static_cast<uint8_t>(this->mTransitionTime100ms)));
-            ReturnErrorOnFailure(this->mExtensionFieldsSets.Serialize(writer));
+            ReturnErrorOnFailure(this->mExtensionFieldSets.Serialize(writer));
 
             return writer.EndContainer(container);
         }
@@ -243,7 +245,7 @@ class SceneTable
             ReturnErrorOnFailure(reader.Get(this->mSceneTransitionTime));
             ReturnErrorOnFailure(reader.Next(TLV::ContextTag(kTagSceneDTransitionTime100)));
             ReturnErrorOnFailure(reader.Get(this->mTransitionTime100ms));
-            ReturnErrorOnFailure(this->mExtensionFieldsSets.Deserialize(reader));
+            ReturnErrorOnFailure(this->mExtensionFieldSets.Deserialize(reader));
 
             return reader.ExitContainer(container);
         }
@@ -269,20 +271,20 @@ class SceneTable
             this->SetName(CharSpan());
             mSceneTransitionTime = 0;
             mTransitionTime100ms = 0;
-            mExtensionFieldsSets.Clear();
+            mExtensionFieldSets.Clear();
         }
 
         bool operator==(const SceneData & other)
         {
             return (this->mNameSpan.data_equal(other.mNameSpan) && (this->mSceneTransitionTime == other.mSceneTransitionTime) &&
                     (this->mTransitionTime100ms == other.mTransitionTime100ms) &&
-                    (this->mExtensionFieldsSets == other.mExtensionFieldsSets));
+                    (this->mExtensionFieldSets == other.mExtensionFieldSets));
         }
 
         void operator=(const SceneData & other)
         {
             this->SetName(other.mNameSpan);
-            this->mExtensionFieldsSets = other.mExtensionFieldsSets;
+            this->mExtensionFieldSets  = other.mExtensionFieldSets;
             this->mSceneTransitionTime = other.mSceneTransitionTime;
             this->mTransitionTime100ms = other.mTransitionTime100ms;
         }
@@ -332,8 +334,9 @@ class SceneTable
     virtual CHIP_ERROR RemoveSceneTableEntryAtPosition(FabricIndex fabric_index, SceneIndex scened_idx)               = 0;
 
     // SceneHandlers
-    virtual CHIP_ERROR RegisterHandler(SceneHandler * handler) = 0;
-    virtual CHIP_ERROR UnregisterHandler(uint8_t position)     = 0;
+    virtual CHIP_ERROR RegisterHandler(SceneHandler * handler)   = 0;
+    virtual CHIP_ERROR UnregisterHandler(SceneHandler * handler) = 0;
+    virtual CHIP_ERROR UnregisterAllHandler()                    = 0;
 
     // Extension field sets operation
     virtual CHIP_ERROR SceneSaveEFS(SceneTableEntry & scene)                                    = 0;
@@ -348,33 +351,13 @@ class SceneTable
     virtual SceneEntryIterator * IterateSceneEntry(FabricIndex fabric_index) = 0;
 
     // Handlers
-    virtual bool HandlerListEmpty() { return (handlerNum == 0); }
-    virtual bool HandlerListFull() { return (handlerNum >= kMaxSceneHandlers); }
-    virtual uint8_t GetHandlerNum() { return this->handlerNum; }
+    virtual bool HandlerListEmpty() { return (mNumHandlers == 0); }
+    virtual bool HandlerListFull() { return (mNumHandlers >= kMaxSceneHandlers); }
+    virtual uint8_t GetHandlerNum() { return this->mNumHandlers; }
 
     SceneHandler * mHandlers[kMaxSceneHandlers] = { nullptr };
-    uint8_t handlerNum                          = 0;
+    uint8_t mNumHandlers                        = 0;
 };
 
-/**
- * Instance getter for the global SceneTable.
- *
- * Callers have to externally synchronize usage of this function.
- *
- * @return The global Scene Table
- */
-SceneTable * GetSceneTable();
-
-/**
- * Instance setter for the global Scene Table.
- *
- * Callers have to externally synchronize usage of this function.
- *
- * The `provider` can be set to nullptr if the owner is done with it fully.
- *
- * @param[in] provider pointer to the Scene Table global instance to use
- */
-void SetSceneTable(SceneTable * provider);
-
 } // namespace scenes
 } // namespace chip

src/app/clusters/scenes/SceneTableImpl.cpp

@@ -431,17 +431,32 @@ CHIP_ERROR DefaultSceneTableImpl::RegisterHandler(SceneHandler * handler)
     else if (fisrtEmptyPosition < kMaxSceneHandlers)
     {
         this->mHandlers[fisrtEmptyPosition] = handler;
-        this->handlerNum++;
+        this->mNumHandlers++;
         err = CHIP_NO_ERROR;
     }
 
     return err;
 }
 
-CHIP_ERROR DefaultSceneTableImpl::UnregisterHandler(uint8_t position)
+CHIP_ERROR DefaultSceneTableImpl::UnregisterHandler(SceneHandler * handler)
 {
-    VerifyOrReturnError(position < kMaxSceneHandlers, CHIP_ERROR_INVALID_ARGUMENT);
-    VerifyOrReturnValue(!this->HandlerListEmpty() && !(this->mHandlers[position] == nullptr), CHIP_NO_ERROR);
+    uint8_t position = kInvalidPosition;
+
+    // Verify list is populated and handler is not null
+    VerifyOrReturnValue(!this->HandlerListEmpty() && !(handler == nullptr), CHIP_NO_ERROR);
+
+    // Finds the position of the Handler to unregister
+    for (uint8_t i = 0; i < this->mNumHandlers; i++)
+    {
+        if (this->mHandlers[i] == handler)
+        {
+            position = i;
+            break;
+        }
+    }
+
+    // Verify Handler was found
+    VerifyOrReturnValue(position < kMaxSceneHandlers, CHIP_NO_ERROR);
 
     uint8_t nextPos = static_cast<uint8_t>(position + 1);
     uint8_t moveNum = static_cast<uint8_t>(kMaxSceneHandlers - nextPos);
@@ -450,9 +465,19 @@ CHIP_ERROR DefaultSceneTableImpl::UnregisterHandler(uint8_t position)
     // Compress array after removal
     memmove(&this->mHandlers[position], &this->mHandlers[nextPos], sizeof(this->mHandlers[position]) * moveNum);
 
-    this->handlerNum--;
+    this->mNumHandlers--;
     // Clear last occupied position
-    this->mHandlers[handlerNum] = nullptr;
+    this->mHandlers[mNumHandlers] = nullptr;
+
+    return CHIP_NO_ERROR;
+}
+
+CHIP_ERROR DefaultSceneTableImpl::UnregisterAllHandler()
+{
+    for (uint8_t i = 0; i < this->mNumHandlers; i++)
+    {
+        ReturnErrorOnFailure(this->UnregisterHandler(this->mHandlers[0]));
+    }
 
     return CHIP_NO_ERROR;
 }
@@ -461,7 +486,7 @@ CHIP_ERROR DefaultSceneTableImpl::SceneSaveEFS(SceneTableEntry & scene)
 {
     if (!this->HandlerListEmpty())
     {
-        for (uint8_t i = 0; i < this->handlerNum; i++)
+        for (uint8_t i = 0; i < this->mNumHandlers; i++)
         {
             clusterId cArray[kMaxClusterPerScenes];
             Span<clusterId> cSpan(cArray);
@@ -476,7 +501,7 @@ CHIP_ERROR DefaultSceneTableImpl::SceneSaveEFS(SceneTableEntry & scene)
                     EFS.mID = cArray[j];
                     ReturnErrorOnFailure(this->mHandlers[i]->SerializeSave(scene.mStorageId.mEndpointId, EFS.mID, EFSSpan));
                     EFS.mUsedBytes = (uint8_t) EFSSpan.size();
-                    ReturnErrorOnFailure(scene.mStorageData.mExtensionFieldsSets.InsertFieldSet(EFS));
+                    ReturnErrorOnFailure(scene.mStorageData.mExtensionFieldSets.InsertFieldSet(EFS));
                 }
             }
         }
@@ -499,17 +524,17 @@ CHIP_ERROR DefaultSceneTableImpl::SceneApplyEFS(FabricIndex fabric_index, const
 
     if (!this->HandlerListEmpty())
     {
-        for (uint8_t i = 0; i < scene.mStorageData.mExtensionFieldsSets.GetFieldNum(); i++)
+        for (uint8_t i = 0; i < scene.mStorageData.mExtensionFieldSets.GetFieldNum(); i++)
         {
-            scene.mStorageData.mExtensionFieldsSets.GetFieldSetAtPosition(EFS, i);
+            scene.mStorageData.mExtensionFieldSets.GetFieldSetAtPosition(EFS, i);
             cluster = EFS.mID;
             time    = scene.mStorageData.mSceneTransitionTime * 1000 +
                 (scene.mStorageData.mTransitionTime100ms ? scene.mStorageData.mTransitionTime100ms * 10 : 0);
             ByteSpan EFSSpan = MutableByteSpan(EFS.mBytesBuffer, EFS.mUsedBytes);
 
             if (!EFS.IsEmpty())
             {
-                for (uint8_t j = 0; j < this->handlerNum; j++)
+                for (uint8_t j = 0; j < this->mNumHandlers; j++)
                 {
                     ReturnErrorOnFailure(this->mHandlers[j]->ApplyScene(scene.mStorageId.mEndpointId, cluster, EFSSpan, time));
                 }
@@ -590,21 +615,5 @@ void DefaultSceneTableImpl::SceneEntryIteratorImpl::Release()
     mProvider.mSceneEntryIterators.ReleaseObject(this);
 }
 
-namespace {
-
-SceneTable * gSceneTable = nullptr;
-
-} // namespace
-
-SceneTable * GetSceneTable()
-{
-    return gSceneTable;
-}
-
-void SetSceneTable(SceneTable * provider)
-{
-    gSceneTable = provider;
-}
-
 } // namespace scenes
 } // namespace chip