Decouple Network commissioning and general commissioning via a breadcrumb tracker (#41724)

* Decouple Network commissioning and general commissioning via a
breadcrumb tracker.

* Comment update.

* Cleanup some includes.

* Fix includes.

* Update src/app/clusters/network-commissioning/tests/TestNetworkCommissioningCluster.cpp

Co-authored-by: Copilot <[email protected]>

* Make the breadcrumb tracker a reference.

* Remove the global instance for general commissioning.

* Typo fix.

* Update src/app/clusters/network-commissioning/CodegenInstance.h

Co-authored-by: Copilot <[email protected]>

* Add include dep since that makes things compile clearer.

* Look to be able to compile emulator tests as well.

* Update src/app/clusters/general-commissioning-server/CodegenIntegration.cpp

Co-authored-by: Zaid Omer <[email protected]>

* Mark the overrides in general commissioning as final, to slightly reduce
flash overhed.

* Fix comment.

* Add message on failure to set breadcrumb.

---------

Co-authored-by: Copilot <[email protected]>
Co-authored-by: Zaid Omer <[email protected]>
Co-authored-by: Andrei Litvin <[email protected]>

Author: 4 people

src/app/clusters/general-commissioning-server/BUILD.gn

@@ -16,13 +16,20 @@ import("//build_overrides/chip.gni")
 
 import("${chip_root}/src/app/common_flags.gni")
 
+source_set("bread-crumb-tracker") {
+  sources = [ "BreadCrumbTracker.h" ]
+
+  public_configs = [ "${chip_root}/src:includes" ]
+}
+
 source_set("general-commissioning-server") {
   sources = [
     "general-commissioning-cluster.cpp",
     "general-commissioning-cluster.h",
   ]
 
   public_deps = [
+    ":bread-crumb-tracker",
     "${chip_root}/src/app:app_config",
     "${chip_root}/src/app/data-model-provider",
     "${chip_root}/src/app/server",

src/app/clusters/general-commissioning-server/BreadCrumbTracker.h

@@ -0,0 +1,35 @@
+/*
+ *    Copyright (c) 2025 Project CHIP Authors
+ *    All rights reserved.
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License");
+ *    you may not use this file except in compliance with the License.
+ *    You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing, software
+ *    distributed under the License is distributed on an "AS IS" BASIS,
+ *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *    See the License for the specific language governing permissions and
+ *    limitations under the License.
+ */
+#pragma once
+#include <cstdint>
+
+namespace chip::app::Clusters {
+
+/// Defines an interface for tracking a breadcrumb value.
+///
+/// The Breadcrumb attribute allows commissioners and administrators
+/// to save a small payload that can be subsequently read, to keep
+/// track of progress (see 11.10.6.1 of the Matter spec related
+/// to Breadcrumb Attribute in the General Commissioning Cluster).
+class BreadCrumbTracker
+{
+public:
+    virtual ~BreadCrumbTracker()               = default;
+    virtual void SetBreadCrumb(uint64_t value) = 0;
+};
+
+} // namespace chip::app::Clusters

src/app/clusters/general-commissioning-server/CodegenIntegration.cpp

@@ -15,9 +15,13 @@
  *    See the License for the specific language governing permissions and
  *    limitations under the License.
  */
+#include <app/clusters/general-commissioning-server/CodegenIntegration.h>
+
 #include <app/clusters/general-commissioning-server/general-commissioning-cluster.h>
+#include <app/server-cluster/ServerClusterInterfaceRegistry.h>
 #include <app/static-cluster-config/GeneralCommissioning.h>
 #include <data-model-providers/codegen/ClusterIntegration.h>
+#include <lib/support/CodeUtils.h>
 
 using namespace chip;
 using namespace chip::app;
@@ -42,7 +46,7 @@ static_assert((kGeneralCommissioningFixedClusterCount == 0) ||
                    GeneralCommissioning::StaticApplicationConfig::kFixedClusterConfig[0].endpointNumber == kRootEndpointId),
               "General Commissioning cluster MUST be on endpoint 0");
 
-ServerClusterRegistration gRegistration(GeneralCommissioningCluster::Instance());
+RegisteredServerCluster<GeneralCommissioningCluster> gRegistration;
 
 class IntegrationDelegate : public CodegenClusterIntegration::Delegate
 {
@@ -51,23 +55,30 @@ class IntegrationDelegate : public CodegenClusterIntegration::Delegate
                                                    uint32_t optionalAttributeBits, uint32_t featureMap) override
     {
         // Configure optional attributes based on fetched bits
-        GeneralCommissioningCluster::Instance().GetOptionalAttributes() =
-            GeneralCommissioningCluster::OptionalAttributes(optionalAttributeBits);
+        gRegistration.Cluster().GetOptionalAttributes() = GeneralCommissioningCluster::OptionalAttributes(optionalAttributeBits);
 
-        return gRegistration;
+        return gRegistration.Registration();
     }
 
-    ServerClusterInterface * FindRegistration(unsigned clusterInstanceIndex) override
-    {
-        return &GeneralCommissioningCluster::Instance();
-    }
+    ServerClusterInterface * FindRegistration(unsigned clusterInstanceIndex) override { return &gRegistration.Cluster(); }
 
-    // Nothing to destroy: separate singleton class without constructor/destructor is used
+    // Nothing to destroy: gRegistration is a global static that handles destruction
     void ReleaseRegistration(unsigned clusterInstanceIndex) override {}
 };
 
 } // namespace
 
+namespace chip::app::Clusters::GeneralCommissioning {
+
+GeneralCommissioningCluster * Instance()
+{
+    // we ALWAYS return this for now, however in the future this may be instantiated
+    // at runtime (i.e only after server is initialized.)
+    return &gRegistration.Cluster();
+}
+
+} // namespace chip::app::Clusters::GeneralCommissioning
+
 void MatterGeneralCommissioningClusterInitCallback(EndpointId endpointId)
 {
     VerifyOrReturn(endpointId == kRootEndpointId);

src/app/clusters/general-commissioning-server/CodegenIntegration.h

@@ -0,0 +1,34 @@
+/*
+ *    Copyright (c) 2025 Project CHIP Authors
+ *    All rights reserved.
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License");
+ *    you may not use this file except in compliance with the License.
+ *    You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing, software
+ *    distributed under the License is distributed on an "AS IS" BASIS,
+ *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *    See the License for the specific language governing permissions and
+ *    limitations under the License.
+ */
+#pragma once
+
+#include <app/clusters/general-commissioning-server/general-commissioning-cluster.h>
+
+namespace chip {
+namespace app {
+namespace Clusters {
+namespace GeneralCommissioning {
+
+/// Returns the singleton instance of the general commissioning cluster, if enabled.
+///
+/// This is a temporary integration point for ember-based apps.
+GeneralCommissioningCluster * Instance();
+
+} // namespace GeneralCommissioning
+} // namespace Clusters
+} // namespace app
+} // namespace chip

src/app/clusters/general-commissioning-server/app_config_dependent_sources.gni

@@ -11,4 +11,7 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-app_config_dependent_sources = [ "CodegenIntegration.cpp" ]
+app_config_dependent_sources = [
+  "CodegenIntegration.cpp",
+  "CodegenIntegration.h",
+]

src/app/clusters/general-commissioning-server/general-commissioning-cluster.cpp

@@ -139,12 +139,12 @@ void NotifyTermsAndConditionsAttributeChangeIfRequired(const TermsAndConditionsS
 }
 #endif // CHIP_CONFIG_TERMS_AND_CONDITIONS_REQUIRED
 
-void OnPlatformEventHandler(const DeviceLayer::ChipDeviceEvent * event, intptr_t /* arg */)
+void OnPlatformEventHandler(const DeviceLayer::ChipDeviceEvent * event, intptr_t self)
 {
     if (event->Type == DeviceLayer::DeviceEventType::kFailSafeTimerExpired)
     {
         // Spec says to reset Breadcrumb attribute to 0.
-        GeneralCommissioningCluster::Instance().SetBreadCrumb(0);
+        reinterpret_cast<GeneralCommissioningCluster *>(self)->SetBreadCrumb(0);
 
 #if CHIP_CONFIG_TERMS_AND_CONDITIONS_REQUIRED
         if (event->FailSafeTimerExpired.updateTermsAndConditionsHasBeenInvoked)
@@ -161,16 +161,10 @@ void OnPlatformEventHandler(const DeviceLayer::ChipDeviceEvent * event, intptr_t
     }
 }
 
-GeneralCommissioningCluster gInstance;
 } // anonymous namespace
 
 namespace chip::app::Clusters {
 
-GeneralCommissioningCluster & GeneralCommissioningCluster::Instance()
-{
-    return gInstance;
-}
-
 DataModel::ActionReturnStatus GeneralCommissioningCluster::ReadAttribute(const DataModel::ReadAttributeRequest & request,
                                                                          AttributeValueEncoder & encoder)
 {
@@ -403,14 +397,14 @@ void GeneralCommissioningCluster::SetBreadCrumb(uint64_t value)
 CHIP_ERROR GeneralCommissioningCluster::Startup(ServerClusterContext & context)
 {
     ReturnErrorOnFailure(DefaultServerCluster::Startup(context));
-    PlatformMgrImpl().AddEventHandler(OnPlatformEventHandler, 0);
+    PlatformMgrImpl().AddEventHandler(OnPlatformEventHandler, reinterpret_cast<intptr_t>(this));
     Server::GetInstance().GetFabricTable().AddFabricDelegate(this);
     return CHIP_NO_ERROR;
 }
 
 void GeneralCommissioningCluster::Shutdown()
 {
-    PlatformMgrImpl().RemoveEventHandler(OnPlatformEventHandler, 0);
+    PlatformMgrImpl().RemoveEventHandler(OnPlatformEventHandler, reinterpret_cast<intptr_t>(this));
     Server::GetInstance().GetFabricTable().RemoveFabricDelegate(this);
     DefaultServerCluster::Shutdown();
 }

src/app/clusters/general-commissioning-server/general-commissioning-cluster.h

@@ -17,6 +17,7 @@
 #pragma once
 
 #include <app/AppConfig.h>
+#include <app/clusters/general-commissioning-server/BreadCrumbTracker.h>
 #include <app/server-cluster/DefaultServerCluster.h>
 #include <app/server-cluster/OptionalAttributeSet.h>
 #include <clusters/GeneralCommissioning/AttributeIds.h>
@@ -28,7 +29,7 @@
 
 namespace chip::app::Clusters {
 
-class GeneralCommissioningCluster : public DefaultServerCluster, chip::FabricTable::Delegate
+class GeneralCommissioningCluster : public DefaultServerCluster, chip::FabricTable::Delegate, public BreadCrumbTracker
 {
 public:
     using OptionalAttributes = OptionalAttributeSet<GeneralCommissioning::Attributes::IsCommissioningWithoutPower::Id>;
@@ -37,7 +38,7 @@ class GeneralCommissioningCluster : public DefaultServerCluster, chip::FabricTab
 
     OptionalAttributes & GetOptionalAttributes() { return mOptionalAttributes; }
 
-    void SetBreadCrumb(uint64_t value);
+    void SetBreadCrumb(uint64_t value) final;
 
     // Server cluster implementation
     CHIP_ERROR Startup(ServerClusterContext & context) override;
@@ -55,10 +56,7 @@ class GeneralCommissioningCluster : public DefaultServerCluster, chip::FabricTab
     CHIP_ERROR Attributes(const ConcreteClusterPath & path, ReadOnlyBufferBuilder<DataModel::AttributeEntry> & builder) override;
 
     // Fabric delegate
-    void OnFabricRemoved(const FabricTable & fabricTable, FabricIndex fabricIndex) override;
-
-    // GeneralCommissioning is a singleton cluster that exists only on the root endpoint.
-    static GeneralCommissioningCluster & Instance();
+    void OnFabricRemoved(const FabricTable & fabricTable, FabricIndex fabricIndex) final;
 
     // Feature map constant based on compile-time defines. This ensures feature map
     // is in sync with the actual supported features determined at build time.

src/app/clusters/general-commissioning-server/tests/TestGeneralCommissioningCluster.cpp

@@ -51,7 +51,7 @@ TEST_F(TestGeneralCommissioningCluster, TestAttributes)
 {
     // test without optional attributes
     {
-        auto & cluster            = GeneralCommissioningCluster::Instance();
+        GeneralCommissioningCluster cluster;
         auto & optionalAttributes = cluster.GetOptionalAttributes();
         optionalAttributes        = GeneralCommissioningCluster::OptionalAttributes(0);
 
@@ -87,7 +87,7 @@ TEST_F(TestGeneralCommissioningCluster, TestAttributes)
 
     // test with optional attributes
     {
-        auto & cluster            = GeneralCommissioningCluster::Instance();
+        GeneralCommissioningCluster cluster;
         auto & optionalAttributes = cluster.GetOptionalAttributes();
         optionalAttributes.Set<IsCommissioningWithoutPower::Id>();
 
@@ -132,8 +132,8 @@ TEST_F(TestGeneralCommissioningCluster, TestAcceptedCommands)
 {
     {
         ReadOnlyBufferBuilder<AcceptedCommandEntry> builder;
-        ASSERT_EQ(GeneralCommissioningCluster::Instance().AcceptedCommands({ kRootEndpointId, GeneralCommissioning::Id }, builder),
-                  CHIP_NO_ERROR);
+        GeneralCommissioningCluster cluster;
+        ASSERT_EQ(cluster.AcceptedCommands({ kRootEndpointId, GeneralCommissioning::Id }, builder), CHIP_NO_ERROR);
 
         ReadOnlyBufferBuilder<AcceptedCommandEntry> expectedBuilder;
         ASSERT_EQ(expectedBuilder.AppendElements({

src/app/clusters/network-commissioning/BUILD.gn

@@ -59,7 +59,7 @@ source_set("network-commissioning") {
     ":thread-response-encoding",
     ":wifi-response-encoding",
     "${chip_root}/src/app:interaction-model",
-    "${chip_root}/src/app/clusters/general-commissioning-server:general-commissioning-server",
+    "${chip_root}/src/app/clusters/general-commissioning-server:bread-crumb-tracker",
     "${chip_root}/src/app/data-model-provider",
     "${chip_root}/src/app/server",
     "${chip_root}/src/app/server-cluster",

src/app/clusters/network-commissioning/CodegenInstance.cpp

@@ -16,13 +16,27 @@
  */
 #include "CodegenInstance.h"
 
+#include <app/clusters/general-commissioning-server/CodegenIntegration.h>
+#include <app/clusters/general-commissioning-server/general-commissioning-cluster.h>
+#include <app/clusters/network-commissioning/NetworkCommissioningCluster.h>
 #include <data-model-providers/codegen/CodegenDataModelProvider.h>
 
 namespace chip {
 namespace app {
 namespace Clusters {
 namespace NetworkCommissioning {
 
+void Instance::CodegenGeneralCommissioningBreadcrumbTracker::SetBreadCrumb(uint64_t value)
+{
+    GeneralCommissioningCluster * cluster = GeneralCommissioning::Instance();
+    if (cluster == nullptr)
+    {
+        ChipLogError(Zcl, "General commissioning cluster not available. Failed to set breadcrumb.");
+        return;
+    }
+    cluster->SetBreadCrumb(value);
+}
+
 CHIP_ERROR Instance::Init()
 {
     ReturnErrorOnFailure(mCluster.Cluster().Init());