Incremental improvements to controller dependencies (project-chip#36567)

* Remove unused reference to CHIP_CONFIG_ENABLE_READ_CLIENT * ExamplePersistentStorage does not need CHIPDeviceController * Tidy up some Linux app-main dependencies * Reduce dependency of data model on controller * Fix log placerholders to use ChipLogFormatX64 in fabric sync example
mkardous-silabs · Nov 22, 2024 · 7d8c30b · 7d8c30b
1 parent 7df9b14
commit 7d8c30b
Show file tree

Hide file tree

Showing 27 changed files with 102 additions and 113 deletions.
diff --git a/examples/fabric-admin/device_manager/DeviceManager.cpp b/examples/fabric-admin/device_manager/DeviceManager.cpp
@@ -227,8 +227,9 @@ void DeviceManager::SubscribeRemoteFabricBridge()
 
     if (error != CHIP_NO_ERROR)
     {
-        ChipLogError(NotSpecified, "Failed to subscribe to the remote bridge (NodeId: %lu). Error: %" CHIP_ERROR_FORMAT,
-                     mRemoteBridgeNodeId, error.Format());
+        ChipLogError(NotSpecified,
+                     "Failed to subscribe to the remote bridge (NodeId: " ChipLogFormatX64 "). Error: %" CHIP_ERROR_FORMAT,
+                     ChipLogValueX64(mRemoteBridgeNodeId), error.Format());
         return;
     }
 }
@@ -251,8 +252,9 @@ void DeviceManager::ReadSupportedDeviceCategories()
     if (error != CHIP_NO_ERROR)
     {
         ChipLogError(NotSpecified,
-                     "Failed to read SupportedDeviceCategories from the remote bridge (NodeId: %lu). Error: %" CHIP_ERROR_FORMAT,
-                     mRemoteBridgeNodeId, error.Format());
+                     "Failed to read SupportedDeviceCategories from the remote bridge (NodeId: " ChipLogFormatX64
+                     "). Error: %" CHIP_ERROR_FORMAT,
+                     ChipLogValueX64(mRemoteBridgeNodeId), error.Format());
     }
 }
 
@@ -293,8 +295,9 @@ void DeviceManager::RequestCommissioningApproval()
     if (error != CHIP_NO_ERROR)
     {
         ChipLogError(NotSpecified,
-                     "Failed to request commissioning-approval to the remote bridge (NodeId: %lu). Error: %" CHIP_ERROR_FORMAT,
-                     mRemoteBridgeNodeId, error.Format());
+                     "Failed to request commissioning-approval to the remote bridge (NodeId: " ChipLogFormatX64
+                     "). Error: %" CHIP_ERROR_FORMAT,
+                     ChipLogValueX64(mRemoteBridgeNodeId), error.Format());
         return;
     }
 
@@ -417,8 +420,9 @@ void DeviceManager::SendCommissionNodeRequest(uint64_t requestId, uint16_t respo
     if (error != CHIP_NO_ERROR)
     {
         ChipLogError(NotSpecified,
-                     "Failed to send CommissionNode command to the remote bridge (NodeId: %lu). Error: %" CHIP_ERROR_FORMAT,
-                     mRemoteBridgeNodeId, error.Format());
+                     "Failed to send CommissionNode command to the remote bridge (NodeId: " ChipLogFormatX64
+                     "). Error: %" CHIP_ERROR_FORMAT,
+                     ChipLogValueX64(mRemoteBridgeNodeId), error.Format());
         return;
     }
 }

diff --git a/examples/fabric-sync/admin/DeviceManager.cpp b/examples/fabric-sync/admin/DeviceManager.cpp
@@ -221,8 +221,9 @@ void DeviceManager::SubscribeRemoteFabricBridge()
 
     if (error != CHIP_NO_ERROR)
     {
-        ChipLogError(NotSpecified, "Failed to subscribe to the remote bridge (NodeId: %lu). Error: %" CHIP_ERROR_FORMAT,
-                     mRemoteBridgeNodeId, error.Format());
+        ChipLogError(NotSpecified,
+                     "Failed to subscribe to the remote bridge (NodeId: " ChipLogFormatX64 "). Error: %" CHIP_ERROR_FORMAT,
+                     ChipLogValueX64(mRemoteBridgeNodeId), error.Format());
         return;
     }
 }
@@ -245,8 +246,9 @@ void DeviceManager::ReadSupportedDeviceCategories()
     if (error != CHIP_NO_ERROR)
     {
         ChipLogError(NotSpecified,
-                     "Failed to read SupportedDeviceCategories from the remote bridge (NodeId: %lu). Error: %" CHIP_ERROR_FORMAT,
-                     mRemoteBridgeNodeId, error.Format());
+                     "Failed to read SupportedDeviceCategories from the remote bridge (NodeId: " ChipLogFormatX64
+                     "). Error: %" CHIP_ERROR_FORMAT,
+                     ChipLogValueX64(mRemoteBridgeNodeId), error.Format());
     }
 }
 
@@ -287,8 +289,9 @@ void DeviceManager::RequestCommissioningApproval()
     if (error != CHIP_NO_ERROR)
     {
         ChipLogError(NotSpecified,
-                     "Failed to request commissioning-approval to the remote bridge (NodeId: %lu). Error: %" CHIP_ERROR_FORMAT,
-                     mRemoteBridgeNodeId, error.Format());
+                     "Failed to request commissioning-approval to the remote bridge (NodeId: " ChipLogFormatX64
+                     "). Error: %" CHIP_ERROR_FORMAT,
+                     ChipLogValueX64(mRemoteBridgeNodeId), error.Format());
         return;
     }
 
@@ -411,8 +414,9 @@ void DeviceManager::SendCommissionNodeRequest(uint64_t requestId, uint16_t respo
     if (error != CHIP_NO_ERROR)
     {
         ChipLogError(NotSpecified,
-                     "Failed to send CommissionNode command to the remote bridge (NodeId: %lu). Error: %" CHIP_ERROR_FORMAT,
-                     mRemoteBridgeNodeId, error.Format());
+                     "Failed to send CommissionNode command to the remote bridge (NodeId: " ChipLogFormatX64
+                     "). Error: %" CHIP_ERROR_FORMAT,
+                     ChipLogValueX64(mRemoteBridgeNodeId), error.Format());
         return;
     }
 }

diff --git a/examples/fabric-sync/admin/DeviceSynchronization.cpp b/examples/fabric-sync/admin/DeviceSynchronization.cpp
@@ -262,7 +262,7 @@ void DeviceSynchronizer::GetUniqueId()
     }
     else
     {
-        ChipLogDetail(NotSpecified, "Failed to get UniqueId from remote Fabric Sync Aggregator")
+        ChipLogDetail(NotSpecified, "Failed to get UniqueId from remote Fabric Sync Aggregator");
     }
 }
 

diff --git a/examples/fabric-sync/admin/FabricAdmin.cpp b/examples/fabric-sync/admin/FabricAdmin.cpp
@@ -17,6 +17,7 @@
 
 #include "FabricAdmin.h"
 #include <AppMain.h>
+#include <CommissionerMain.h>
 #include <bridge/include/FabricBridge.h>
 #include <controller/CHIPDeviceControllerFactory.h>
 

diff --git a/examples/fabric-sync/main.cpp b/examples/fabric-sync/main.cpp
@@ -16,6 +16,7 @@
  */
 
 #include <AppMain.h>
+#include <CommissionerMain.h>
 #include <admin/FabricAdmin.h>
 #include <admin/PairingManager.h>
 #include <bridge/include/Bridge.h>

diff --git a/examples/fabric-sync/shell/AddBridgeCommand.cpp b/examples/fabric-sync/shell/AddBridgeCommand.cpp
@@ -85,8 +85,9 @@ CHIP_ERROR AddBridgeCommand::RunCommand()
 
     admin::PairingManager::Instance().SetPairingDelegate(this);
 
-    ChipLogProgress(NotSpecified, "Running AddBridgeCommand with Node ID: %lu, PIN Code: %u, Address: %s, Port: %u", mBridgeNodeId,
-                    mSetupPINCode, mRemoteAddr, mRemotePort);
+    ChipLogProgress(NotSpecified,
+                    "Running AddBridgeCommand with Node ID: " ChipLogFormatX64 ", PIN Code: %u, Address: %s, Port: %u",
+                    ChipLogValueX64(mBridgeNodeId), mSetupPINCode, mRemoteAddr, mRemotePort);
 
     return admin::PairingManager::Instance().PairDevice(mBridgeNodeId, mSetupPINCode, mRemoteAddr, mRemotePort);
 }

diff --git a/examples/fabric-sync/shell/AddDeviceCommand.cpp b/examples/fabric-sync/shell/AddDeviceCommand.cpp
@@ -71,8 +71,9 @@ CHIP_ERROR AddDeviceCommand::RunCommand()
         return CHIP_ERROR_INVALID_ARGUMENT;
     }
 
-    ChipLogProgress(NotSpecified, "Running AddDeviceCommand with Node ID: %lu, PIN Code: %u, Address: %s, Port: %u", mNodeId,
-                    mSetupPINCode, mRemoteAddr, mRemotePort);
+    ChipLogProgress(NotSpecified,
+                    "Running AddDeviceCommand with Node ID: " ChipLogFormatX64 ", PIN Code: %u, Address: %s, Port: %u",
+                    ChipLogValueX64(mNodeId), mSetupPINCode, mRemoteAddr, mRemotePort);
 
     admin::PairingManager::Instance().SetPairingDelegate(this);
 

diff --git a/examples/fabric-sync/shell/PairDeviceCommand.cpp b/examples/fabric-sync/shell/PairDeviceCommand.cpp
@@ -69,7 +69,8 @@ CHIP_ERROR PairDeviceCommand::RunCommand()
         return CHIP_ERROR_INVALID_ARGUMENT;
     }
 
-    ChipLogProgress(NotSpecified, "Running PairDeviceCommand with Node ID: %lu, Code: %s", mNodeId, mPayload);
+    ChipLogProgress(NotSpecified, "Running PairDeviceCommand with Node ID: " ChipLogFormatX64 ", Code: %s",
+                    ChipLogValueX64(mNodeId), mPayload);
 
     admin::PairingManager::Instance().SetPairingDelegate(this);
 

diff --git a/examples/fabric-sync/shell/RemoveDeviceCommand.cpp b/examples/fabric-sync/shell/RemoveDeviceCommand.cpp
@@ -61,7 +61,7 @@ CHIP_ERROR RemoveDeviceCommand::RunCommand()
 
     admin::PairingManager::Instance().SetPairingDelegate(this);
 
-    ChipLogProgress(NotSpecified, "Running RemoveDeviceCommand with Node ID: %lu", mNodeId);
+    ChipLogProgress(NotSpecified, "Running RemoveDeviceCommand with Node ID: " ChipLogFormatX64, ChipLogValueX64(mNodeId));
 
     return admin::PairingManager::Instance().UnpairDevice(mNodeId);
 }

diff --git a/examples/platform/linux/AppMain.cpp b/examples/platform/linux/AppMain.cpp
@@ -33,8 +33,6 @@
 #include <lib/support/logging/CHIPLogging.h>
 
 #include <credentials/DeviceAttestationCredsProvider.h>
-#include <credentials/attestation_verifier/DefaultDeviceAttestationVerifier.h>
-#include <credentials/attestation_verifier/DeviceAttestationVerifier.h>
 
 #include <lib/support/CHIPMem.h>
 #include <lib/support/ScopedBuffer.h>
@@ -51,10 +49,6 @@
 #if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE
 #include "CommissionerMain.h"
 #include <ControllerShellCommands.h>
-#include <controller/CHIPDeviceControllerFactory.h>
-#include <controller/ExampleOperationalCredentialsIssuer.h>
-#include <lib/core/CHIPPersistentStorageDelegate.h>
-#include <platform/KeyValueStoreManager.h>
 #endif // CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE
 
 #if defined(ENABLE_CHIP_SHELL)

diff --git a/examples/platform/linux/AppMain.h b/examples/platform/linux/AppMain.h
@@ -19,7 +19,6 @@
 #pragma once
 
 #include <app/server/Server.h>
-#include <controller/CHIPDeviceController.h>
 #include <controller/CommissionerDiscoveryController.h>
 #include <crypto/RawKeySessionKeystore.h>
 #include <lib/core/CHIPError.h>
@@ -89,23 +88,6 @@ class DefaultAppMainLoopImplementation : public AppMainLoopImplementation
  */
 void ChipLinuxAppMainLoop(AppMainLoopImplementation * impl = nullptr);
 
-#if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE
-
-using chip::PersistentStorageDelegate;
-using chip::Controller::DeviceCommissioner;
-using chip::Crypto::SessionKeystore;
-using chip::Transport::PeerAddress;
-
-CHIP_ERROR CommissionerPairOnNetwork(uint32_t pincode, uint16_t disc, PeerAddress address);
-CHIP_ERROR CommissionerPairUDC(uint32_t pincode, size_t index);
-
-DeviceCommissioner * GetDeviceCommissioner();
-CommissionerDiscoveryController * GetCommissionerDiscoveryController();
-SessionKeystore * GetSessionKeystore();
-PersistentStorageDelegate * GetPersistentStorageDelegate();
-
-#endif // CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE
-
 // For extra init calls, the function will be called right before running Matter main loop.
 void ApplicationInit();
 

diff --git a/examples/platform/linux/BUILD.gn b/examples/platform/linux/BUILD.gn
@@ -96,8 +96,6 @@ source_set("app-main") {
     ":smco-test-event-trigger",
     ":water-heater-management-test-event-trigger",
     "${chip_root}/src/app/codegen-data-model-provider:instance-header",
-    "${chip_root}/src/controller:controller",
-    "${chip_root}/src/controller:gen_check_chip_controller_headers",
     "${chip_root}/src/lib",
     "${chip_root}/src/platform/logging:default",
   ]
@@ -164,7 +162,6 @@ source_set("commissioner-main") {
 
   public_deps = [
     "${chip_root}/src/controller:controller",
-    "${chip_root}/src/controller:gen_check_chip_controller_headers",
     "${chip_root}/src/lib",
   ]
   deps = [ "${chip_root}/src/app/server" ]

diff --git a/examples/platform/linux/ControllerShellCommands.cpp b/examples/platform/linux/ControllerShellCommands.cpp
@@ -19,9 +19,9 @@
  * @file Contains shell commands for for performing discovery (eg. of commissionable nodes) related to commissioning.
  */
 
-#include <AppMain.h>
-#include <ControllerShellCommands.h>
-#include <inttypes.h>
+#include "ControllerShellCommands.h"
+#include "CommissionerMain.h"
+
 #include <lib/core/CHIPCore.h>
 #include <lib/shell/Commands.h>
 #include <lib/shell/Engine.h>
@@ -33,6 +33,8 @@
 #include <protocols/secure_channel/RendezvousParameters.h>
 #include <protocols/user_directed_commissioning/UserDirectedCommissioning.h>
 
+#include <inttypes.h>
+
 namespace chip {
 namespace Shell {
 

diff --git a/src/app/AttributePathParams.h b/src/app/AttributePathParams.h
@@ -25,9 +25,7 @@
 
 namespace chip {
 namespace app {
-#if CHIP_CONFIG_ENABLE_READ_CLIENT
-class ReadClient;
-#endif // CHIP_CONFIG_ENABLE_READ_CLIENT
+
 struct AttributePathParams
 {
     AttributePathParams() = default;

diff --git a/src/app/chip_data_model.gni b/src/app/chip_data_model.gni
@@ -235,6 +235,10 @@ template("chip_data_model") {
       deps = []
     }
 
+    if (!defined(public_deps)) {
+      public_deps = []
+    }
+
     if (!defined(cflags)) {
       cflags = []
     }
@@ -269,9 +273,12 @@ template("chip_data_model") {
       } else if (cluster == "application-launcher-server") {
         sources += [
           "${_app_root}/app-platform/ContentApp.cpp",
+          "${_app_root}/app-platform/ContentApp.h",
           "${_app_root}/app-platform/ContentAppPlatform.cpp",
+          "${_app_root}/app-platform/ContentAppPlatform.h",
           "${_app_root}/clusters/${cluster}/${cluster}.cpp",
         ]
+        deps += [ "${chip_root}/src/controller" ]
       } else if (cluster == "ota-requestor") {
         sources += [
           # TODO - align name of folder ?
@@ -288,6 +295,10 @@ template("chip_data_model") {
           "${_app_root}/clusters/${cluster}/OTATestEventTriggerHandler.cpp",
           "${_app_root}/clusters/${cluster}/OTATestEventTriggerHandler.h",
         ]
+
+        # TODO: DefaultOTARequestor depends on controller, however the cluster server itself does not.
+        # Maybe DefaultOTARequestor and related code should have its own source set.
+        deps += [ "${chip_root}/src/controller:interactions" ]
       } else if (cluster == "bindings") {
         sources += [
           "${_app_root}/clusters/${cluster}/${cluster}.cpp",
@@ -446,10 +457,6 @@ template("chip_data_model") {
       }
     }
 
-    if (!defined(public_deps)) {
-      public_deps = []
-    }
-
     public_deps += [
       ":${_data_model_name}_codegen",
       ":${_data_model_name}_zapgen",
@@ -459,12 +466,16 @@ template("chip_data_model") {
       "${chip_root}/src/app/common:attribute-type",
       "${chip_root}/src/app/common:cluster-objects",
       "${chip_root}/src/app/common:enums",
+      "${chip_root}/src/app/server",
       "${chip_root}/src/app/util:types",
       "${chip_root}/src/app/util/persistence",
-      "${chip_root}/src/controller",
       "${chip_root}/src/lib/core",
       "${chip_root}/src/lib/support",
       "${chip_root}/src/protocols/secure_channel",
+
+      # TODO: Embedded example apps currently build with chip_build_controller = false, and so get a libCHIP without controller support,
+      # but nevertheless expect to have access to some of the "controller" code to implement bindings and related functionality.
+      "${chip_root}/src/controller:interactions",
     ]
     public_deps += codegen_data_model_PUBLIC_DEPS
 

diff --git a/src/app/clusters/content-app-observer/content-app-observer.cpp b/src/app/clusters/content-app-observer/content-app-observer.cpp
@@ -33,7 +33,8 @@
 #include <platform/CHIPDeviceConfig.h>
 
 #if CHIP_DEVICE_CONFIG_APP_PLATFORM_ENABLED
-#include <app/app-platform/ContentAppPlatform.h>
+#include <app/app-platform/ContentAppPlatform.h> // nogncheck
+
 #endif // CHIP_DEVICE_CONFIG_APP_PLATFORM_ENABLED
 
 using namespace chip;

diff --git a/src/app/clusters/content-control-server/content-control-server.cpp b/src/app/clusters/content-control-server/content-control-server.cpp
@@ -33,7 +33,8 @@
 #include <platform/CHIPDeviceConfig.h>
 
 #if CHIP_DEVICE_CONFIG_APP_PLATFORM_ENABLED
-#include <app/app-platform/ContentAppPlatform.h>
+#include <app/app-platform/ContentAppPlatform.h> // nogncheck
+
 #endif // CHIP_DEVICE_CONFIG_APP_PLATFORM_ENABLED
 
 using namespace chip;

diff --git a/src/app/clusters/content-launch-server/content-launch-server.cpp b/src/app/clusters/content-launch-server/content-launch-server.cpp
@@ -29,7 +29,8 @@
 #include <platform/CHIPDeviceConfig.h>
 
 #if CHIP_DEVICE_CONFIG_APP_PLATFORM_ENABLED
-#include <app/app-platform/ContentAppPlatform.h>
+#include <app/app-platform/ContentAppPlatform.h> // nogncheck
+
 #endif // CHIP_DEVICE_CONFIG_APP_PLATFORM_ENABLED
 
 #include <list>

diff --git a/src/app/clusters/keypad-input-server/keypad-input-server.cpp b/src/app/clusters/keypad-input-server/keypad-input-server.cpp
@@ -35,7 +35,8 @@
 #include <platform/CHIPDeviceConfig.h>
 
 #if CHIP_DEVICE_CONFIG_APP_PLATFORM_ENABLED
-#include <app/app-platform/ContentAppPlatform.h>
+#include <app/app-platform/ContentAppPlatform.h> // nogncheck
+
 #endif // CHIP_DEVICE_CONFIG_APP_PLATFORM_ENABLED
 
 using namespace chip;

diff --git a/src/app/clusters/media-playback-server/media-playback-server.cpp b/src/app/clusters/media-playback-server/media-playback-server.cpp
@@ -37,7 +37,8 @@
 #include <platform/CHIPDeviceConfig.h>
 
 #if CHIP_DEVICE_CONFIG_APP_PLATFORM_ENABLED
-#include <app/app-platform/ContentAppPlatform.h>
+#include <app/app-platform/ContentAppPlatform.h> // nogncheck
+
 #endif // CHIP_DEVICE_CONFIG_APP_PLATFORM_ENABLED
 
 using namespace chip;

diff --git a/src/app/clusters/target-navigator-server/target-navigator-server.cpp b/src/app/clusters/target-navigator-server/target-navigator-server.cpp
@@ -36,7 +36,8 @@
 #include <platform/CHIPDeviceConfig.h>
 
 #if CHIP_DEVICE_CONFIG_APP_PLATFORM_ENABLED
-#include <app/app-platform/ContentAppPlatform.h>
+#include <app/app-platform/ContentAppPlatform.h> // nogncheck
+
 #endif // CHIP_DEVICE_CONFIG_APP_PLATFORM_ENABLED
 
 using namespace chip;
-Original file line number
+Diff line change
@@ Expand Up / @@ -262,7 +262,7 @@ void DeviceSynchronizer::GetUniqueId() @@
         }
         else
         {
-            ChipLogDetail(NotSpecified, "Failed to get UniqueId from remote Fabric Sync Aggregator")
+            ChipLogDetail(NotSpecified, "Failed to get UniqueId from remote Fabric Sync Aggregator");
         }
     }
@@ Expand Down @@