Make AttributeAccessInterface and CommandHandlerInterface registries consistent as objects with singletons

docs/cluster_and_device_type_dev/cluster_and_device_type_dev.md

@@ -107,9 +107,9 @@ ending in the cluster initialization code.
 EmberAfInitializeAttributes - ember attribute storage - for all attributes
 marked as “RAM” in the zap, sets defaults in the storage
 Matter<Cluster>PluginServerCallback - .h is a generated file, .cpp impl is done
-in the server cluster code. Use this to setup the cluster and do attribute
-overrides registerAttributeAccessOverride - use this if you want to handle
-attribute reads and writes externally
+in the server cluster code. Use this to setup the cluster and setup overrides in
+chip::app::AttributeAccessInterfaceRegistry::Instance().Register - use this if
+you want to handle attribute reads and writes externally
 
 Blue sections can be overridden.
 

docs/upgrading.md

@@ -69,10 +69,26 @@ independent of the InteractionModelEngine class.
 The following replacements exist:
 
 -   `chip::app::InteractionModelEngine::RegisterCommandHandler` replaced by
-    `chip::app::CommandHandlerInterfaceRegistry::RegisterCommandHandler`
+    `chip::app::CommandHandlerInterfaceRegistry::Instance().RegisterCommandHandler`
 -   `chip::app::InteractionModelEngine::UnregisterCommandHandler` replaced by
-    `chip::app::CommandHandlerInterfaceRegistry::UnregisterCommandHandler`
+    `chip::app::CommandHandlerInterfaceRegistry::Instance().UnregisterCommandHandler`
 -   `chip::app::InteractionModelEngine::FindCommandHandler` replaced by
-    `chip::app::CommandHandlerInterfaceRegistry::GetCommandHandler`
+    `chip::app::CommandHandlerInterfaceRegistry::Instance().GetCommandHandler`
 -   `chip::app::InteractionModelEngine::UnregisterCommandHandlers` replaced by
-    `chip::app::CommandHandlerInterfaceRegistry::UnregisterAllCommandHandlersForEndpoint`
+    `chip::app::CommandHandlerInterfaceRegistry::Instance().UnregisterAllCommandHandlersForEndpoint`
+
+### AttributeAccessInterface registration and removal
+
+A new object exists for the attribute access interface registry, accessible as
+`chip::app::AttributeHandlerInterfaceRegistry::Instance()`
+
+Replacements for methods are:
+
+-   `registerAttributeAccessOverride` replaced by
+    `chip::app::AttributeAccessInterfaceRegistry::Instance().Register`
+-   `unregisterAttributeAccessOverride` replaced by
+    `chip::app::AttributeAccessInterfaceRegistry::Instance().Unregister`
+-   `unregisterAllAttributeAccessOverridesForEndpoint` replaced by
+    `chip::app::AttributeAccessInterfaceRegistry::Instance().UnregisterAllForEndpoint`
+-   `chip::app::GetAttributeAccessOverride` replaced by
+    `chip::app::AttributeAccessInterfaceRegistry::Instance().Get`

examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp

@@ -98,5 +98,5 @@ CHIP_ERROR ActionsAttrAccess::Read(const ConcreteReadAttributePath & aPath, Attr
 
 void MatterActionsPluginServerInitCallback()
 {
-    registerAttributeAccessOverride(&gAttrAccess);
+    AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess);
 }

examples/all-clusters-app/all-clusters-common/src/fan-stub.cpp

@@ -173,6 +173,6 @@ void emberAfFanControlClusterInitCallback(EndpointId endpoint)
 {
     VerifyOrDie(mFanControlManager == nullptr);
     mFanControlManager = new FanControlManager(endpoint);
-    registerAttributeAccessOverride(mFanControlManager);
+    AttributeAccessInterfaceRegistry::Instance().Register(mFanControlManager);
     FanControl::SetDefaultDelegate(endpoint, mFanControlManager);
 }

examples/bridge-app/asr/src/bridged-actions-stub.cpp

@@ -97,5 +97,5 @@ CHIP_ERROR ActionsAttrAccess::Read(const ConcreteReadAttributePath & aPath, Attr
 
 void MatterActionsPluginServerInitCallback(void)
 {
-    registerAttributeAccessOverride(&gAttrAccess);
+    AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess);
 }

examples/bridge-app/esp32/main/DeviceCallbacks.cpp

@@ -113,5 +113,5 @@ CHIP_ERROR ActionsAttrAccess::Read(const ConcreteReadAttributePath & aPath, Attr
 
 void MatterActionsPluginServerInitCallback(void)
 {
-    registerAttributeAccessOverride(&gAttrAccess);
+    AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess);
 }

examples/bridge-app/linux/bridged-actions-stub.cpp

@@ -133,5 +133,5 @@ CHIP_ERROR ActionsAttrAccess::Read(const ConcreteReadAttributePath & aPath, Attr
 
 void MatterActionsPluginServerInitCallback()
 {
-    registerAttributeAccessOverride(&gAttrAccess);
+    AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess);
 }

examples/bridge-app/linux/main.cpp

@@ -999,7 +999,7 @@ void ApplicationInit()
         }
     }
 
-    registerAttributeAccessOverride(&gPowerAttrAccess);
+    AttributeAccessInterfaceRegistry::Instance().Register(&gPowerAttrAccess);
 }
 
 void ApplicationShutdown() {}

examples/bridge-app/telink/src/DeviceCallbacks.cpp

@@ -101,5 +101,5 @@ CHIP_ERROR ActionsAttrAccess::Read(const ConcreteReadAttributePath & aPath, Attr
 
 void MatterActionsPluginServerInitCallback(void)
 {
-    registerAttributeAccessOverride(&gAttrAccess);
+    AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess);
 }

examples/chef/common/chef-fan-control-manager.cpp

@@ -141,6 +141,6 @@ void emberAfFanControlClusterInitCallback(EndpointId endpoint)
 {
     VerifyOrDie(!mFanControlManager);
     mFanControlManager = std::make_unique<ChefFanControlManager>(endpoint);
-    registerAttributeAccessOverride(mFanControlManager.get());
+    AttributeAccessInterfaceRegistry::Instance().Register(mFanControlManager.get());
     FanControl::SetDefaultDelegate(endpoint, mFanControlManager.get());
 }

examples/fabric-bridge-app/linux/main.cpp

@@ -174,7 +174,7 @@ void ApplicationInit()
 {
     ChipLogDetail(NotSpecified, "Fabric-Bridge: ApplicationInit()");
 
-    CommandHandlerInterfaceRegistry::RegisterCommandHandler(&gAdministratorCommissioningCommandHandler);
+    CommandHandlerInterfaceRegistry::Instance().RegisterCommandHandler(&gAdministratorCommissioningCommandHandler);
 
 #if defined(PW_RPC_FABRIC_BRIDGE_SERVICE) && PW_RPC_FABRIC_BRIDGE_SERVICE
     InitRpcServer(kFabricBridgeServerPort);

examples/log-source-app/linux/main.cpp

@@ -110,7 +110,7 @@ int main(int argc, char * argv[])
     // Initialize device attestation config
     SetDeviceAttestationCredentialsProvider(chip::Credentials::Examples::GetExampleDACProvider());
 
-    CommandHandlerInterfaceRegistry::RegisterCommandHandler(&GetLogProvider());
+    CommandHandlerInterfaceRegistry::Instance().RegisterCommandHandler(&GetLogProvider());
 
     chip::DeviceLayer::PlatformMgr().RunEventLoop();
 

examples/placeholder/linux/src/bridged-actions-stub.cpp

@@ -98,5 +98,5 @@ CHIP_ERROR ActionsAttrAccess::Read(const ConcreteReadAttributePath & aPath, Attr
 
 void MatterActionsPluginServerInitCallback(void)
 {
-    registerAttributeAccessOverride(&gAttrAccess);
+    chip::app::AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess);
 }

examples/tv-app/android/java/AppImpl.cpp

@@ -572,7 +572,7 @@ CHIP_ERROR InitVideoPlayerPlatform(jobject contentAppEndpointManager)
     {
         ContentAppCommandDelegate * delegate =
             new ContentAppCommandDelegate(contentAppEndpointManager, contentAppClusters[i].clusterId);
-        chip::app::CommandHandlerInterfaceRegistry::RegisterCommandHandler(delegate);
+        app::CommandHandlerInterfaceRegistry::Instance().RegisterCommandHandler(delegate);
         ChipLogProgress(AppServer, "Registered command handler delegate for cluster %d", contentAppClusters[i].clusterId);
     }
 

scripts/py_matter_idl/matter_idl/test_idl_generator.py

@@ -55,8 +55,8 @@ def ReadMatterIdl(repo_path: str) -> str:
         return stream.read()
 
 
-def ParseMatterIdl(repo_path: str, skip_meta: bool) -> Idl:
-    return CreateParser(skip_meta=skip_meta).parse(ReadMatterIdl(repo_path))
+def ParseMatterIdl(repo_path: str, skip_meta: bool, merge_globals: bool) -> Idl:
+    return CreateParser(skip_meta=skip_meta, merge_globals=merge_globals).parse(ReadMatterIdl(repo_path))
 
 
 def RenderAsIdlTxt(idl: Idl) -> str:
@@ -106,8 +106,11 @@ def test_client_clusters(self):
         # Files MUST be identical except the header comments which are different
         original = SkipLeadingComments(ReadMatterIdl(path), also_strip=[
                                        " // NOTE: Default/not specifically set"])
+
+        # Do not merge globals: zap generators do not generate IDLs with
+        # globals inside structures (i.e. no comments about referenced globals)
         generated = SkipLeadingComments(RenderAsIdlTxt(
-            ParseMatterIdl(path, skip_meta=False)))
+            ParseMatterIdl(path, skip_meta=False, merge_globals=False)))
 
         self.assertTextEqual(original, generated)
 
@@ -126,9 +129,9 @@ def test_app_rendering(self):
         ]
 
         for path in test_paths:
-            idl = ParseMatterIdl(path, skip_meta=True)
+            idl = ParseMatterIdl(path, skip_meta=True, merge_globals=True)
             txt = RenderAsIdlTxt(idl)
-            idl2 = CreateParser(skip_meta=True).parse(txt)
+            idl2 = CreateParser(skip_meta=True, merge_globals=True).parse(txt)
 
             # checks that data types and content is the same
             self.assertEqual(idl, idl2)

src/app/AttributeAccessInterface.h

@@ -30,8 +30,8 @@
  * endpoint or for all endpoints.
  *
  * Instances of AttributeAccessInterface that are registered via
- * registerAttributeAccessOverride will be consulted before taking the normal
- * attribute access codepath and can use that codepath as a fallback if desired.
+ * AttributeAccessInterfaceRegistry::Instance().Register will be consulted before taking the
+ * normal attribute access codepath and can use that codepath as a fallback if desired.
  */
 namespace chip {
 namespace app {

src/app/AttributeAccessInterfaceRegistry.cpp

@@ -13,24 +13,22 @@
  *    See the License for the specific language governing permissions and
  *    limitations under the License.
  */
+#include "app/AttributeAccessInterface.h"
 #include <app/AttributeAccessInterfaceRegistry.h>
 
 #include <app/AttributeAccessInterfaceCache.h>
 
-using namespace chip::app;
-
 namespace {
 
-AttributeAccessInterface * gAttributeAccessOverrides = nullptr;
-AttributeAccessInterfaceCache gAttributeAccessInterfaceCache;
+using chip::app::AttributeAccessInterface;
 
 // shouldUnregister returns true if the given AttributeAccessInterface should be
 // unregistered.
 template <typename F>
-void UnregisterMatchingAttributeAccessInterfaces(F shouldUnregister)
+void UnregisterMatchingAttributeAccessInterfaces(F shouldUnregister, AttributeAccessInterface *& list_head)
 {
     AttributeAccessInterface * prev = nullptr;
-    AttributeAccessInterface * cur  = gAttributeAccessOverrides;
+    AttributeAccessInterface * cur  = list_head;
     while (cur)
     {
         AttributeAccessInterface * next = cur->GetNext();
@@ -43,7 +41,7 @@ void UnregisterMatchingAttributeAccessInterfaces(F shouldUnregister)
             }
             else
             {
-                gAttributeAccessOverrides = next;
+                list_head = next;
             }
 
             cur->SetNext(nullptr);
@@ -60,43 +58,51 @@ void UnregisterMatchingAttributeAccessInterfaces(F shouldUnregister)
 
 } // namespace
 
-void unregisterAttributeAccessOverride(AttributeAccessInterface * attrOverride)
+namespace chip {
+namespace app {
+
+AttributeAccessInterfaceRegistry & AttributeAccessInterfaceRegistry::Instance()
+{
+    static AttributeAccessInterfaceRegistry instance;
+    return instance;
+}
+
+void AttributeAccessInterfaceRegistry::Unregister(AttributeAccessInterface * attrOverride)
 {
-    gAttributeAccessInterfaceCache.Invalidate();
-    UnregisterMatchingAttributeAccessInterfaces([attrOverride](AttributeAccessInterface * entry) { return entry == attrOverride; });
+    mAttributeAccessInterfaceCache.Invalidate();
+    UnregisterMatchingAttributeAccessInterfaces([attrOverride](AttributeAccessInterface * entry) { return entry == attrOverride; },
+                                                mAttributeAccessOverrides);
 }
 
-void unregisterAllAttributeAccessOverridesForEndpoint(EmberAfDefinedEndpoint * definedEndpoint)
+void AttributeAccessInterfaceRegistry::UnregisterAllForEndpoint(EndpointId endpointId)
 {
+    mAttributeAccessInterfaceCache.Invalidate();
     UnregisterMatchingAttributeAccessInterfaces(
-        [endpoint = definedEndpoint->endpoint](AttributeAccessInterface * entry) { return entry->MatchesEndpoint(endpoint); });
+        [endpointId](AttributeAccessInterface * entry) { return entry->MatchesEndpoint(endpointId); }, mAttributeAccessOverrides);
 }
 
-bool registerAttributeAccessOverride(AttributeAccessInterface * attrOverride)
+bool AttributeAccessInterfaceRegistry::Register(AttributeAccessInterface * attrOverride)
 {
-    gAttributeAccessInterfaceCache.Invalidate();
-    for (auto * cur = gAttributeAccessOverrides; cur; cur = cur->GetNext())
+    mAttributeAccessInterfaceCache.Invalidate();
+    for (auto * cur = mAttributeAccessOverrides; cur; cur = cur->GetNext())
     {
         if (cur->Matches(*attrOverride))
         {
             ChipLogError(InteractionModel, "Duplicate attribute override registration failed");
             return false;
         }
     }
-    attrOverride->SetNext(gAttributeAccessOverrides);
-    gAttributeAccessOverrides = attrOverride;
+    attrOverride->SetNext(mAttributeAccessOverrides);
+    mAttributeAccessOverrides = attrOverride;
     return true;
 }
 
-namespace chip {
-namespace app {
-
-app::AttributeAccessInterface * GetAttributeAccessOverride(EndpointId endpointId, ClusterId clusterId)
+AttributeAccessInterface * AttributeAccessInterfaceRegistry::Get(EndpointId endpointId, ClusterId clusterId)
 {
     using CacheResult = AttributeAccessInterfaceCache::CacheResult;
 
     AttributeAccessInterface * cached = nullptr;
-    CacheResult result                = gAttributeAccessInterfaceCache.Get(endpointId, clusterId, &cached);
+    CacheResult result                = mAttributeAccessInterfaceCache.Get(endpointId, clusterId, &cached);
     switch (result)
     {
     case CacheResult::kDefinitelyUnused:
@@ -106,17 +112,17 @@ app::AttributeAccessInterface * GetAttributeAccessOverride(EndpointId endpointId
     case CacheResult::kCacheMiss:
     default:
         // Did not cache yet, search set of AAI registered, and cache if found.
-        for (app::AttributeAccessInterface * cur = gAttributeAccessOverrides; cur; cur = cur->GetNext())
+        for (app::AttributeAccessInterface * cur = mAttributeAccessOverrides; cur; cur = cur->GetNext())
         {
             if (cur->Matches(endpointId, clusterId))
             {
-                gAttributeAccessInterfaceCache.MarkUsed(endpointId, clusterId, cur);
+                mAttributeAccessInterfaceCache.MarkUsed(endpointId, clusterId, cur);
                 return cur;
             }
         }
 
         // Did not find AAI registered: mark as definitely not using.
-        gAttributeAccessInterfaceCache.MarkUnused(endpointId, clusterId);
+        mAttributeAccessInterfaceCache.MarkUnused(endpointId, clusterId);
     }
 
     return nullptr;

src/app/AttributeAccessInterfaceRegistry.h

@@ -16,39 +16,49 @@
 #pragma once
 
 #include <app/AttributeAccessInterface.h>
-#include <app/util/af-types.h>
+#include <app/AttributeAccessInterfaceCache.h>
 
-/**
- * Register an attribute access override.  It will remain registered until the
- * endpoint it's registered for is disabled (or until shutdown if it's
- * registered for all endpoints) or until it is explicitly unregistered.
- * Registration will fail if there is an already-registered override for the
- * same set of attributes.
- *
- * @return false if there is an existing override that the new one would
- *               conflict with.  In this case the override is not registered.
- * @return true if registration was successful.
- */
-bool registerAttributeAccessOverride(chip::app::AttributeAccessInterface * attrOverride);
+namespace chip {
+namespace app {
 
-/**
- * Unregister an attribute access override (for example if the object
- * implementing AttributeAccessInterface is being destroyed).
- */
-void unregisterAttributeAccessOverride(chip::app::AttributeAccessInterface * attrOverride);
+class AttributeAccessInterfaceRegistry
+{
+public:
+    /**
+     * Register an attribute access override.  It will remain registered until the
+     * endpoint it's registered for is disabled (or until shutdown if it's
+     * registered for all endpoints) or until it is explicitly unregistered.
+     * Registration will fail if there is an already-registered override for the
+     * same set of attributes.
+     *
+     * @return false if there is an existing override that the new one would
+     *               conflict with.  In this case the override is not registered.
+     * @return true if registration was successful.
+     */
+    bool Register(AttributeAccessInterface * attrOverride);
 
-/**
- * Unregister all attribute access interfaces that match this given endpoint.
- */
-void unregisterAllAttributeAccessOverridesForEndpoint(EmberAfDefinedEndpoint * definedEndpoint);
+    /**
+     * Unregister an attribute access override (for example if the object
+     * implementing AttributeAccessInterface is being destroyed).
+     */
+    void Unregister(AttributeAccessInterface * attrOverride);
 
-namespace chip {
-namespace app {
+    /**
+     * Unregister all attribute access interfaces that match this given endpoint.
+     */
+    void UnregisterAllForEndpoint(EndpointId endpointId);
 
-/**
- *  Get the registered attribute access override. nullptr when attribute access override is not found.
- */
-AttributeAccessInterface * GetAttributeAccessOverride(EndpointId aEndpointId, ClusterId aClusterId);
+    /**
+     *  Get the registered attribute access override. nullptr when attribute access override is not found.
+     */
+    AttributeAccessInterface * Get(EndpointId aEndpointId, ClusterId aClusterId);
+
+    static AttributeAccessInterfaceRegistry & Instance();
+
+private:
+    AttributeAccessInterface * mAttributeAccessOverrides = nullptr;
+    AttributeAccessInterfaceCache mAttributeAccessInterfaceCache;
+};
 
 } // namespace app
 } // namespace chip