Remove dependency of libChipController on app-specific codegen. (#24555)

* Remove dependency of libChipController on app-specific codegen. * Remove the unused cluster id member from ClusterBase. * Make ClusterBase constructor public. * Use ClusterBase instead of generated cluster structs in src/controller. * Remove the CHIPClusters.h includes and codegen dependency. * Revert the non-workflow changes from PR ##24539, since those are not needed anymore. * Regenerate generated files.
project-chip · Jan 20, 2023 · 7fe4abc · 7fe4abc
1 parent 7e3d712
commit 7fe4abc
Show file tree

Hide file tree

Showing 30 changed files with 125 additions and 751 deletions.
diff --git a/examples/tv-casting-app/tv-casting-common/include/MediaBase.h b/examples/tv-casting-app/tv-casting-common/include/MediaBase.h
@@ -46,8 +46,8 @@ class MediaBase
     {
     public:
         MediaClusterBase(chip::Messaging::ExchangeManager & exchangeManager, const chip::SessionHandle & session,
-                         chip::ClusterId cluster, chip::EndpointId endpoint) :
-            ClusterBase(exchangeManager, session, cluster, endpoint)
+                         chip::EndpointId endpoint) :
+            ClusterBase(exchangeManager, session, endpoint)
         {}
     };
 

diff --git a/examples/tv-casting-app/tv-casting-common/include/MediaCommandBase.h b/examples/tv-casting-app/tv-casting-common/include/MediaCommandBase.h
@@ -36,8 +36,7 @@ class MediaCommandBase : public MediaBase
 
         sResponseCallback = responseCallback;
 
-        MediaClusterBase cluster(*deviceProxy->GetExchangeManager(), deviceProxy->GetSecureSession().Value(), mClusterId,
-                                 mTvEndpoint);
+        MediaClusterBase cluster(*deviceProxy->GetExchangeManager(), deviceProxy->GetSecureSession().Value(), mTvEndpoint);
         return cluster.InvokeCommand(request, nullptr, OnSuccess, OnFailure);
     }
 

diff --git a/examples/tv-casting-app/tv-casting-common/include/MediaReadBase.h b/examples/tv-casting-app/tv-casting-common/include/MediaReadBase.h
@@ -34,8 +34,7 @@ class MediaReadBase : public MediaBase
         auto deviceProxy = mTargetVideoPlayerInfo->GetOperationalDeviceProxy();
         ReturnErrorCodeIf(deviceProxy == nullptr || !deviceProxy->ConnectionReady(), CHIP_ERROR_PEER_NODE_NOT_FOUND);
 
-        MediaClusterBase cluster(*deviceProxy->GetExchangeManager(), deviceProxy->GetSecureSession().Value(), mClusterId,
-                                 mTvEndpoint);
+        MediaClusterBase cluster(*deviceProxy->GetExchangeManager(), deviceProxy->GetSecureSession().Value(), mTvEndpoint);
 
         return cluster.template ReadAttribute<TypeInfo>(context, successFn, failureFn);
     }

diff --git a/examples/tv-casting-app/tv-casting-common/include/MediaSubscriptionBase.h b/examples/tv-casting-app/tv-casting-common/include/MediaSubscriptionBase.h
@@ -35,8 +35,7 @@ class MediaSubscriptionBase : public MediaBase
         auto deviceProxy = mTargetVideoPlayerInfo->GetOperationalDeviceProxy();
         ReturnErrorCodeIf(deviceProxy == nullptr || !deviceProxy->ConnectionReady(), CHIP_ERROR_PEER_NODE_NOT_FOUND);
 
-        MediaClusterBase cluster(*deviceProxy->GetExchangeManager(), deviceProxy->GetSecureSession().Value(), mClusterId,
-                                 mTvEndpoint);
+        MediaClusterBase cluster(*deviceProxy->GetExchangeManager(), deviceProxy->GetSecureSession().Value(), mTvEndpoint);
 
         return cluster.template SubscribeAttribute<TypeInfo>(context, successFn, failureFn, minInterval, maxInterval,
                                                              onSubscriptionEstablished, nullptr /* resubscribeCb */,

diff --git a/scripts/codepregen.py b/scripts/codepregen.py
@@ -74,7 +74,7 @@ def _ParallelGenerateOne(arg):
 @click.option(
     '--generator',
     default='all',
-    type=click.Choice(['all', 'zap', 'codegen', 'codegen-cpp-only']),
+    type=click.Choice(['all', 'zap', 'codegen']),
     help='To what code generator to restrict the generation.')
 @click.option(
     '--input-glob',
@@ -119,12 +119,9 @@ def main(log_level, parallel, dry_run, generator, input_glob, sdk_root, output_d
 
     if generator == 'zap':
         filter.file_type = IdlFileType.ZAP
-    elif generator == 'codegen' or generator == 'codegen-cpp-only':
+    elif generator == 'codegen':
         filter.file_type = IdlFileType.MATTER
 
-    if generator == 'codegen-cpp-only':
-        filter.cpp_only = True
-
     targets = FindPregenerationTargets(sdk_root, filter, runner)
 
     runner.ensure_directory_exists(output_dir)

diff --git a/scripts/pregenerate/__init__.py b/scripts/pregenerate/__init__.py
@@ -55,8 +55,6 @@ class TargetFilter:
     # If non-empty only the given paths will be code-generated
     path_glob: List[str] = field(default_factory=list)
 
-    cpp_only: bool = False
-
 
 # TODO: the build GlobMatcher is more complete by supporting `{}` grouping
 #       For now this limited glob seems sufficient.
@@ -100,5 +98,5 @@ def FindPregenerationTargets(sdk_root: str, filter: TargetFilter, runner):
                 continue
 
         for generator in generators:
-            if generator.Accept(idl, filter.cpp_only):
+            if generator.Accept(idl):
                 yield generator.CreateTarget(idl, runner=runner)
diff --git a/scripts/pregenerate/using_codegen.py b/scripts/pregenerate/using_codegen.py
@@ -64,7 +64,7 @@ class CodegenBridgePregenerator:
     def __init__(self, sdk_root):
         self.sdk_root = sdk_root
 
-    def Accept(self, idl: InputIdlFile, cpp_only: bool):
+    def Accept(self, idl: InputIdlFile):
         # Bridge is highly specific, a single path is acceptable for dynamic
         # bridge codegen
         return idl.relative_path == "examples/dynamic-bridge-app/bridge-common/bridge-app.matter"
@@ -79,12 +79,9 @@ class CodegenJavaPregenerator:
     def __init__(self, sdk_root):
         self.sdk_root = sdk_root
 
-    def Accept(self, idl: InputIdlFile, cpp_only: bool):
-        if cpp_only:
-            return False
-
-        # Java is highly specific, a single path is acceptable for java
-        # codegen
+    def Accept(self, idl: InputIdlFile):
+        # Java is highly specific, a single path is acceptable for dynamic
+        # bridge codegen
         return idl.relative_path == "src/controller/data_model/controller-clusters.matter"
 
     def CreateTarget(self, idl: InputIdlFile, runner):
@@ -97,7 +94,7 @@ class CodegenCppAppPregenerator:
     def __init__(self, sdk_root):
         self.sdk_root = sdk_root
 
-    def Accept(self, idl: InputIdlFile, cpp_only: bool):
+    def Accept(self, idl: InputIdlFile):
         if idl.file_type != IdlFileType.MATTER:
             return False
 

diff --git a/scripts/pregenerate/using_zap.py b/scripts/pregenerate/using_zap.py
@@ -77,7 +77,7 @@ class ZapApplicationPregenerator:
     def __init__(self, sdk_root):
         self.sdk_root = sdk_root
 
-    def Accept(self, idl: InputIdlFile, cpp_only: bool):
+    def Accept(self, idl: InputIdlFile):
         if idl.file_type != IdlFileType.ZAP:
             return False
 

diff --git a/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/CHIPClusters.h b/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/CHIPClusters.h
@@ -35,7 +35,7 @@ class DLL_EXPORT OtaSoftwareUpdateProviderCluster : public ClusterBase
 public:
     OtaSoftwareUpdateProviderCluster(Messaging::ExchangeManager & exchangeManager, const SessionHandle & session,
                                      EndpointId endpoint) :
-        ClusterBase(exchangeManager, session, app::Clusters::OtaSoftwareUpdateProvider::Id, endpoint)
+        ClusterBase(exchangeManager, session, endpoint)
     {}
     ~OtaSoftwareUpdateProviderCluster() {}
 };

diff --git a/scripts/tools/zap/tests/outputs/lighting-app/app-templates/CHIPClusters.h b/scripts/tools/zap/tests/outputs/lighting-app/app-templates/CHIPClusters.h
@@ -35,7 +35,7 @@ class DLL_EXPORT OtaSoftwareUpdateProviderCluster : public ClusterBase
 public:
     OtaSoftwareUpdateProviderCluster(Messaging::ExchangeManager & exchangeManager, const SessionHandle & session,
                                      EndpointId endpoint) :
-        ClusterBase(exchangeManager, session, app::Clusters::OtaSoftwareUpdateProvider::Id, endpoint)
+        ClusterBase(exchangeManager, session, endpoint)
     {}
     ~OtaSoftwareUpdateProviderCluster() {}
 };

diff --git a/scripts/tools/zap_regen_all.py b/scripts/tools/zap_regen_all.py
@@ -113,41 +113,6 @@ def generate(self) -> TargetRunStats:
         )
 
 
-class ZAPPregenerateTarget:
-    def __init__(self, input_glob, output_dir, generator=None):
-        self.input_glob = input_glob
-        self.output_dir = output_dir
-        self.script = "./scripts/codepregen.py"
-        self.command = [self.script, "--input-glob", input_glob]
-        self.generator = generator
-
-        if generator is not None:
-            self.command.extend(["--generator", generator])
-        self.command.append(output_dir)
-
-    def distinct_output(self):
-        input_template = self.input_glob
-        if self.generator is not None:
-            input_template += " " + self.generator
-        return ZapDistinctOutput(input_template=input_template, output_directory=self.output_dir)
-
-    def log_command(self):
-        logging.info("  %s" % " ".join(self.command))
-
-    def generate(self) -> TargetRunStats:
-        logging.info("Generating target: %s" % " ".join(self.command))
-
-        generate_start = time.time()
-        subprocess.check_call(self.command)
-        generate_end = time.time()
-
-        return TargetRunStats(
-            generate_time=generate_end - generate_start,
-            config=self.script,
-            template=self.script,
-        )
-
-
 def checkPythonVersion():
     if sys.version_info[0] < 3:
         print('Must use Python 3. Current version is ' +
@@ -235,10 +200,10 @@ def getGlobalTemplatesTargets():
     #
     # TODO: These files can be code generated at compile time, we should figure
     #       out a path for this codegen to not be required.
-    targets.append(ZAPPregenerateTarget(
-        "*controller-clusters*", "zzz_generated/darwin", generator="zap"))
-    targets.append(ZAPPregenerateTarget(
-        "*controller-clusters*", "zzz_generated/darwin", generator="codegen-cpp-only"))
+    targets.append(ZAPGenerateTarget(
+        'src/controller/data_model/controller-clusters.zap',
+        template="src/app/zap-templates/app-templates.json",
+        output_dir=os.path.join('zzz_generated/darwin/controller-clusters/zap-generated')))
 
     return targets
 

diff --git a/src/app/zap-templates/templates/app/CHIPClusters.zapt b/src/app/zap-templates/templates/app/CHIPClusters.zapt
@@ -18,7 +18,7 @@ namespace Controller {
 class DLL_EXPORT {{asUpperCamelCase name}}Cluster : public ClusterBase
 {
 public:
-    {{asUpperCamelCase name}}Cluster(Messaging::ExchangeManager & exchangeManager, const SessionHandle & session, EndpointId endpoint) : ClusterBase(exchangeManager, session, app::Clusters::{{asUpperCamelCase name}}::Id, endpoint) {}
+    {{asUpperCamelCase name}}Cluster(Messaging::ExchangeManager & exchangeManager, const SessionHandle & session, EndpointId endpoint) : ClusterBase(exchangeManager, session, endpoint) {}
     ~{{asUpperCamelCase name}}Cluster() {}
 };
 

diff --git a/src/controller/AutoCommissioner.cpp b/src/controller/AutoCommissioner.cpp
@@ -22,7 +22,6 @@
 #include <controller/CHIPDeviceController.h>
 #include <credentials/CHIPCert.h>
 #include <lib/support/SafeInt.h>
-#include <zap-generated/CHIPClusters.h>
 
 namespace chip {
 namespace Controller {

diff --git a/src/controller/BUILD.gn b/src/controller/BUILD.gn
@@ -67,18 +67,7 @@ static_library("controller") {
     "${chip_root}/src/transport",
   ]
 
-  deps = [
-    # NOTE:
-    #   awkward zapgen_zapgen dependency to get access to generated headers
-    #   but NOT to include generated CPP files (IMCommandHandler.cpp)
-    "${chip_root}/src/controller/data_model:data_model_zapgen_files",
-    "${chip_root}/src/lib/address_resolve",
-  ]
-
-  configs += [
-    # Get access to zzz_generated/CHIPClusters.h
-    "${chip_root}/src/controller/data_model:data_model_zapgen_config",
-  ]
+  deps = [ "${chip_root}/src/lib/address_resolve" ]
 
   defines = []
 }
diff --git a/src/controller/CHIPCluster.h b/src/controller/CHIPCluster.h
@@ -57,6 +57,10 @@ using SubscriptionOnDoneCallback      = std::function<void(void)>;
 class DLL_EXPORT ClusterBase
 {
 public:
+    ClusterBase(Messaging::ExchangeManager & exchangeManager, const SessionHandle & session, EndpointId endpoint) :
+        mExchangeManager(exchangeManager), mSession(session), mEndpoint(endpoint)
+    {}
+
     virtual ~ClusterBase() {}
 
     // Temporary function to set command timeout before we move over to InvokeCommand
@@ -69,8 +73,6 @@ class DLL_EXPORT ClusterBase
      */
     Optional<System::Clock::Timeout> GetCommandTimeout() { return mTimeout; }
 
-    ClusterId GetClusterId() const { return mClusterId; }
-
     /*
      * This function permits sending an invoke request using cluster objects that represent the request and response data payloads.
      *
@@ -397,20 +399,13 @@ class DLL_EXPORT ClusterBase
     }
 
 protected:
-    ClusterBase(Messaging::ExchangeManager & exchangeManager, const SessionHandle & session, ClusterId cluster,
-                EndpointId endpoint) :
-        mExchangeManager(exchangeManager),
-        mSession(session), mClusterId(cluster), mEndpoint(endpoint)
-    {}
-
     Messaging::ExchangeManager & mExchangeManager;
 
     // Since cluster object is ephemeral, the session shall be valid during the entire lifespan, so we do not need to check the
     // session existence when using it. For java and objective-c binding, the cluster object is allocated in the heap, such that we
     // can't use SessionHandle here, in such case, the cluster object must be freed when the session is released.
     SessionHolder mSession;
 
-    const ClusterId mClusterId;
     EndpointId mEndpoint;
     Optional<System::Clock::Timeout> mTimeout;
 };