Split JNI read implementation .cpp into separate files (#14264)

* Split java reader codegen into per-cluster iterms.

Since GN needs to know in advance the clusters in the IDL,
used the code generator to pre-populate idl_content.gni.

* Remove the need of idl_content.gni - simpler code

* Restyle

* Add multi-cluster unit tests

* Add more unit tests, including gni generator

* Restyle

* Remove need of gni generator - seems cleaner

scripts/codegen.py

@@ -26,7 +26,7 @@
     sys.path.append(os.path.abspath(os.path.dirname(__file__)))
     from idl.matter_idl_parser import CreateParser
 
-from idl.generators import FileSystemGeneratorStorage
+from idl.generators import FileSystemGeneratorStorage, GeneratorStorage
 from idl.generators.java import JavaGenerator
 
 
@@ -40,6 +40,18 @@ def CreateGenerator(self, *args, **kargs):
             raise Error("Unknown code generator type")
 
 
+class ListGeneratedFilesStorage(GeneratorStorage):
+    """
+    Output a list of files to be generated
+    """
+
+    def get_existing_data(self, relative_path: str):
+        return None  # stdout has no pre-existing data
+
+    def write_new_data(self, relative_path: str, content: str):
+        print(relative_path)
+
+
 # Supported log levels, mapping string values required for argument
 # parsing into logging constants
 __LOG_LEVELS__ = {
@@ -50,7 +62,7 @@ def CreateGenerator(self, *args, **kargs):
 }
 
 __GENERATORS__ = {
-    'java': CodeGeneratorTypes.JAVA
+    'java': CodeGeneratorTypes.JAVA,
 }
 
 
@@ -75,10 +87,15 @@ def CreateGenerator(self, *args, **kargs):
     default=False,
     is_flag=True,
     help='If to actually generate')
+@click.option(
+    '--name-only',
+    default=False,
+    is_flag=True,
+    help='Output just a list of file names that would be generated')
 @click.argument(
     'idl_path',
     type=click.Path(exists=True))
-def main(log_level, generator, output_dir, dry_run, idl_path):
+def main(log_level, generator, output_dir, dry_run, name_only, idl_path):
     """
     Parses MATTER IDL files (.matter) and performs SDK code generation
     as set up by the program arguments.
@@ -88,9 +105,14 @@ def main(log_level, generator, output_dir, dry_run, idl_path):
     logging.info("Parsing idl from %s" % idl_path)
     idl_tree = CreateParser().parse(open(idl_path, "rt").read())
 
+    if name_only:
+        storage = ListGeneratedFilesStorage()
+    else:
+        storage = FileSystemGeneratorStorage(output_dir)
+
     logging.info("Running code generator %s" % generator)
-    generator = __GENERATORS__[generator].CreateGenerator(
-        storage=FileSystemGeneratorStorage(output_dir), idl=idl_tree)
+    generator = __GENERATORS__[
+        generator].CreateGenerator(storage, idl=idl_tree)
     generator.render(dry_run)
     logging.info("Done")
 

scripts/idl/BUILD.gn

@@ -31,10 +31,14 @@ pw_python_package("idl") {
     "tests/available_tests.yaml",
     "tests/inputs/cluster_struct_attribute.matter",
     "tests/inputs/global_struct_attribute.matter",
+    "tests/inputs/several_clusters.matter",
     "tests/inputs/simple_attribute.matter",
-    "tests/outputs/cluster_struct_attribute/jni/CHIPClustersRead.cpp",
-    "tests/outputs/global_struct_attribute/jni/CHIPClustersRead.cpp",
-    "tests/outputs/simple_attribute/jni/CHIPClustersRead.cpp",
+    "tests/outputs/cluster_struct_attribute/jni/DemoClusterClient-ReadImpl.cpp",
+    "tests/outputs/global_struct_attribute/jni/DemoClusterClient-ReadImpl.cpp",
+    "tests/outputs/several_clusters/jni/FirstClient-ReadImpl.cpp",
+    "tests/outputs/several_clusters/jni/SecondClient-ReadImpl.cpp",
+    "tests/outputs/several_clusters/jni/ThirdClient-ReadImpl.cpp",
+    "tests/outputs/simple_attribute/jni/MyClusterClient-ReadImpl.cpp",
   ]
 
   sources = [

scripts/idl/generators/java/ChipClustersRead.jinja

@@ -26,9 +26,7 @@
 #define JNI_METHOD(RETURN, CLASS_NAME, METHOD_NAME)                                                                                \
     extern "C" JNIEXPORT RETURN JNICALL Java_chip_devicecontroller_ChipClusters_00024##CLASS_NAME##_##METHOD_NAME
 
-{%- for cluster in clusters | clientClustersOnly -%}
 {%-   for attr in cluster.attributes | attributesWithCallback(known_enums) %}
-
 JNI_METHOD(void, {{cluster.name | capitalcase}}Cluster, read{{attr.definition.name | capitalcase}}Attribute)(JNIEnv * env, jobject self, jlong clusterPtr, jobject callback)
 {
     chip::DeviceLayer::StackLock lock;
@@ -54,4 +52,3 @@ JNI_METHOD(void, {{cluster.name | capitalcase}}Cluster, read{{attr.definition.na
 }
 
 {%   endfor %}
-{% endfor %}

scripts/idl/generators/java/__init__.py

@@ -89,12 +89,6 @@ def attributesWithSupportedCallback(attrs, known_enum_types: List[matter_idl_typ
         yield attr
 
 
-def ClientClustersOnly(clusters: List[Cluster]):
-    for cluster in clusters:
-        if cluster.side == ClusterSide.CLIENT:
-            yield cluster
-
-
 class JavaGenerator(CodeGenerator):
     """
     Generation of java code for matter.
@@ -105,20 +99,23 @@ def __init__(self, storage: GeneratorStorage, idl: Idl):
 
         self.jinja_env.filters['attributesWithCallback'] = attributesWithSupportedCallback
         self.jinja_env.filters['callbackName'] = CallbackName
-        self.jinja_env.filters['clientClustersOnly'] = ClientClustersOnly
 
     def internal_render_all(self):
-        # Renter a complete file currently
-        # this is to prove compatibility with zap generated code
         known_enums = self.idl.enums[:]
         for cluster in self.idl.clusters:
             known_enums.extend(cluster.enums)
 
-        self.internal_render_one_output(
-            template_path="java/ChipClustersRead.jinja",
-            output_file_name="jni/CHIPClustersRead.cpp",
-            vars={
-                'clusters': self.idl.clusters,
-                'known_enums': known_enums,
-            }
-        )
+        # Every cluster has its own impl, to avoid
+        # very large compilations (running out of RAM)
+        for cluster in self.idl.clusters:
+            if cluster.side != ClusterSide.CLIENT:
+                continue
+
+            self.internal_render_one_output(
+                template_path="java/ChipClustersRead.jinja",
+                output_file_name="jni/%sClient-ReadImpl.cpp" % cluster.name,
+                vars={
+                    'cluster': cluster,
+                    'known_enums': known_enums,
+                }
+            )

scripts/idl/test_generators.py

@@ -70,7 +70,7 @@ def get_existing_data(self, relative_path: str):
                     return golden.read()
 
         # This will attempt a new write, causing a unit test failure
-        logging.warning("Expected output %s not found" % relative_path)
+        self.checker.fail("Expected output %s not found" % relative_path)
         return None
 
     def write_new_data(self, relative_path: str, content: str):
@@ -100,6 +100,9 @@ def add_test_cases(self, yaml_test_case_dict):
     def _create_generator(self, storage: GeneratorStorage, idl: Idl):
         if self.generator_name.lower() == 'java':
             return JavaGenerator(storage, idl)
+        else:
+            raise Exception("Unknown generator for testing: %s",
+                            self.generator_name.lower())
 
     def run_test_cases(self, checker: unittest.TestCase):
         for test in self.test_cases:

scripts/idl/tests/available_tests.yaml

@@ -12,10 +12,15 @@
 #     and the expected content for those output files.
 java:
     inputs/simple_attribute.matter:
-        jni/CHIPClustersRead.cpp: outputs/simple_attribute/jni/CHIPClustersRead.cpp
+        jni/MyClusterClient-ReadImpl.cpp: outputs/simple_attribute/jni/MyClusterClient-ReadImpl.cpp
 
     inputs/global_struct_attribute.matter:
-        jni/CHIPClustersRead.cpp: outputs/global_struct_attribute/jni/CHIPClustersRead.cpp
+        jni/DemoClusterClient-ReadImpl.cpp: outputs/global_struct_attribute/jni/DemoClusterClient-ReadImpl.cpp
 
     inputs/cluster_struct_attribute.matter:
-        jni/CHIPClustersRead.cpp: outputs/cluster_struct_attribute/jni/CHIPClustersRead.cpp
+        jni/DemoClusterClient-ReadImpl.cpp: outputs/cluster_struct_attribute/jni/DemoClusterClient-ReadImpl.cpp
+
+    inputs/several_clusters.matter:
+        jni/FirstClient-ReadImpl.cpp: outputs/several_clusters/jni/FirstClient-ReadImpl.cpp
+        jni/SecondClient-ReadImpl.cpp: outputs/several_clusters/jni/SecondClient-ReadImpl.cpp
+        jni/ThirdClient-ReadImpl.cpp: outputs/several_clusters/jni/ThirdClient-ReadImpl.cpp

scripts/idl/tests/inputs/several_clusters.matter

@@ -0,0 +1,16 @@
+client cluster First = 1 {
+    attribute int16u someInteger = 1;
+}
+
+client cluster Second = 2 {
+    readonly attribute octet_string<32> someBytes = 123;
+}
+
+client cluster Third = 3 {
+    enum MyEnum : enum8 {
+      kUnknown = 0;
+      kKnown = 100;
+    }
+
+    attribute MyEnum someEnum = 10;
+}

scripts/idl/tests/outputs/cluster_struct_attribute/jni/CHIPClustersRead.cpp → scripts/idl/tests/outputs/cluster_struct_attribute/jni/DemoClusterClient-ReadImpl.cpp

@@ -12,7 +12,6 @@
 
 #define JNI_METHOD(RETURN, CLASS_NAME, METHOD_NAME)                                                                                \
     extern "C" JNIEXPORT RETURN JNICALL Java_chip_devicecontroller_ChipClusters_00024##CLASS_NAME##_##METHOD_NAME
-
 JNI_METHOD(void, DemoClusterCluster, readArmFailsafesAttribute)(JNIEnv * env, jobject self, jlong clusterPtr, jobject callback)
 {
     chip::DeviceLayer::StackLock lock;
@@ -36,4 +35,3 @@ JNI_METHOD(void, DemoClusterCluster, readArmFailsafesAttribute)(JNIEnv * env, jo
     onFailure.release();
 }
 
-

scripts/idl/tests/outputs/global_struct_attribute/jni/CHIPClustersRead.cpp → scripts/idl/tests/outputs/global_struct_attribute/jni/DemoClusterClient-ReadImpl.cpp

@@ -12,7 +12,6 @@
 
 #define JNI_METHOD(RETURN, CLASS_NAME, METHOD_NAME)                                                                                \
     extern "C" JNIEXPORT RETURN JNICALL Java_chip_devicecontroller_ChipClusters_00024##CLASS_NAME##_##METHOD_NAME
-
 JNI_METHOD(void, DemoClusterCluster, readSomeLabelsAttribute)(JNIEnv * env, jobject self, jlong clusterPtr, jobject callback)
 {
     chip::DeviceLayer::StackLock lock;
@@ -36,4 +35,3 @@ JNI_METHOD(void, DemoClusterCluster, readSomeLabelsAttribute)(JNIEnv * env, jobj
     onFailure.release();
 }
 
-

scripts/idl/tests/outputs/several_clusters/jni/FirstClient-ReadImpl.cpp

@@ -0,0 +1,37 @@
+
+#include <controller/java/zap-generated/CHIPReadCallbacks.h>
+
+#include <app-common/zap-generated/cluster-objects.h>
+#include <zap-generated/CHIPClusters.h>
+
+#include <controller/java/AndroidClusterExceptions.h>
+#include <controller/java/CHIPDefaultCallbacks.h>
+#include <jni.h>
+#include <lib/support/CodeUtils.h>
+#include <platform/PlatformManager.h>
+
+#define JNI_METHOD(RETURN, CLASS_NAME, METHOD_NAME)                                                                                \
+    extern "C" JNIEXPORT RETURN JNICALL Java_chip_devicecontroller_ChipClusters_00024##CLASS_NAME##_##METHOD_NAME
+JNI_METHOD(void, FirstCluster, readSomeIntegerAttribute)(JNIEnv * env, jobject self, jlong clusterPtr, jobject callback)
+{
+    chip::DeviceLayer::StackLock lock;
+    using TypeInfo = chip::app::Clusters::First::Attributes::SomeInteger::TypeInfo;
+    std::unique_ptr<CHIPInt16uAttributeCallback, void (*)(CHIPInt16uAttributeCallback *)> onSuccess(chip::Platform::New<CHIPInt16uAttributeCallback>(callback, false), chip::Platform::Delete<CHIPInt16uAttributeCallback>);
+    VerifyOrReturn(onSuccess.get() != nullptr, chip::AndroidClusterExceptions::GetInstance().ReturnIllegalStateException(env, callback, "Error creating native success callback", CHIP_ERROR_NO_MEMORY));
+
+    std::unique_ptr<chip::CHIPDefaultFailureCallback, void (*)(chip::CHIPDefaultFailureCallback *)> onFailure(chip::Platform::New<chip::CHIPDefaultFailureCallback>(callback), chip::Platform::Delete<chip::CHIPDefaultFailureCallback>);
+    VerifyOrReturn(onFailure.get() != nullptr, chip::AndroidClusterExceptions::GetInstance().ReturnIllegalStateException(env, callback, "Error creating native failure callback", CHIP_ERROR_NO_MEMORY));
+
+    CHIP_ERROR err = CHIP_NO_ERROR;
+    chip::Controller::FirstCluster * cppCluster = reinterpret_cast<chip::Controller::FirstCluster *>(clusterPtr);
+    VerifyOrReturn(cppCluster != nullptr, chip::AndroidClusterExceptions::GetInstance().ReturnIllegalStateException(env, callback, "Could not get native cluster", CHIP_ERROR_INCORRECT_STATE));
+
+    auto successFn = chip::Callback::Callback<CHIPFirstClusterSomeIntegerAttributeCallbackType>::FromCancelable(onSuccess->Cancel());
+    auto failureFn = chip::Callback::Callback<CHIPDefaultFailureCallbackType>::FromCancelable(onFailure->Cancel());
+    err = cppCluster->ReadAttribute<TypeInfo>(onSuccess->mContext, successFn->mCall, failureFn->mCall);
+    VerifyOrReturn(err == CHIP_NO_ERROR, chip::AndroidClusterExceptions::GetInstance().ReturnIllegalStateException(env, callback, "Error reading attribute", err));
+
+    onSuccess.release();
+    onFailure.release();
+}
+

scripts/idl/tests/outputs/several_clusters/jni/SecondClient-ReadImpl.cpp

@@ -0,0 +1,37 @@
+
+#include <controller/java/zap-generated/CHIPReadCallbacks.h>
+
+#include <app-common/zap-generated/cluster-objects.h>
+#include <zap-generated/CHIPClusters.h>
+
+#include <controller/java/AndroidClusterExceptions.h>
+#include <controller/java/CHIPDefaultCallbacks.h>
+#include <jni.h>
+#include <lib/support/CodeUtils.h>
+#include <platform/PlatformManager.h>
+
+#define JNI_METHOD(RETURN, CLASS_NAME, METHOD_NAME)                                                                                \
+    extern "C" JNIEXPORT RETURN JNICALL Java_chip_devicecontroller_ChipClusters_00024##CLASS_NAME##_##METHOD_NAME
+JNI_METHOD(void, SecondCluster, readSomeBytesAttribute)(JNIEnv * env, jobject self, jlong clusterPtr, jobject callback)
+{
+    chip::DeviceLayer::StackLock lock;
+    using TypeInfo = chip::app::Clusters::Second::Attributes::SomeBytes::TypeInfo;
+    std::unique_ptr<CHIPOctetStringAttributeCallback, void (*)(CHIPOctetStringAttributeCallback *)> onSuccess(chip::Platform::New<CHIPOctetStringAttributeCallback>(callback, false), chip::Platform::Delete<CHIPOctetStringAttributeCallback>);
+    VerifyOrReturn(onSuccess.get() != nullptr, chip::AndroidClusterExceptions::GetInstance().ReturnIllegalStateException(env, callback, "Error creating native success callback", CHIP_ERROR_NO_MEMORY));
+
+    std::unique_ptr<chip::CHIPDefaultFailureCallback, void (*)(chip::CHIPDefaultFailureCallback *)> onFailure(chip::Platform::New<chip::CHIPDefaultFailureCallback>(callback), chip::Platform::Delete<chip::CHIPDefaultFailureCallback>);
+    VerifyOrReturn(onFailure.get() != nullptr, chip::AndroidClusterExceptions::GetInstance().ReturnIllegalStateException(env, callback, "Error creating native failure callback", CHIP_ERROR_NO_MEMORY));
+
+    CHIP_ERROR err = CHIP_NO_ERROR;
+    chip::Controller::SecondCluster * cppCluster = reinterpret_cast<chip::Controller::SecondCluster *>(clusterPtr);
+    VerifyOrReturn(cppCluster != nullptr, chip::AndroidClusterExceptions::GetInstance().ReturnIllegalStateException(env, callback, "Could not get native cluster", CHIP_ERROR_INCORRECT_STATE));
+
+    auto successFn = chip::Callback::Callback<CHIPSecondClusterSomeBytesAttributeCallbackType>::FromCancelable(onSuccess->Cancel());
+    auto failureFn = chip::Callback::Callback<CHIPDefaultFailureCallbackType>::FromCancelable(onFailure->Cancel());
+    err = cppCluster->ReadAttribute<TypeInfo>(onSuccess->mContext, successFn->mCall, failureFn->mCall);
+    VerifyOrReturn(err == CHIP_NO_ERROR, chip::AndroidClusterExceptions::GetInstance().ReturnIllegalStateException(env, callback, "Error reading attribute", err));
+
+    onSuccess.release();
+    onFailure.release();
+}
+

scripts/idl/tests/outputs/several_clusters/jni/ThirdClient-ReadImpl.cpp

@@ -0,0 +1,37 @@
+
+#include <controller/java/zap-generated/CHIPReadCallbacks.h>
+
+#include <app-common/zap-generated/cluster-objects.h>
+#include <zap-generated/CHIPClusters.h>
+
+#include <controller/java/AndroidClusterExceptions.h>
+#include <controller/java/CHIPDefaultCallbacks.h>
+#include <jni.h>
+#include <lib/support/CodeUtils.h>
+#include <platform/PlatformManager.h>
+
+#define JNI_METHOD(RETURN, CLASS_NAME, METHOD_NAME)                                                                                \
+    extern "C" JNIEXPORT RETURN JNICALL Java_chip_devicecontroller_ChipClusters_00024##CLASS_NAME##_##METHOD_NAME
+JNI_METHOD(void, ThirdCluster, readSomeEnumAttribute)(JNIEnv * env, jobject self, jlong clusterPtr, jobject callback)
+{
+    chip::DeviceLayer::StackLock lock;
+    using TypeInfo = chip::app::Clusters::Third::Attributes::SomeEnum::TypeInfo;
+    std::unique_ptr<CHIPInt8uAttributeCallback, void (*)(CHIPInt8uAttributeCallback *)> onSuccess(chip::Platform::New<CHIPInt8uAttributeCallback>(callback, false), chip::Platform::Delete<CHIPInt8uAttributeCallback>);
+    VerifyOrReturn(onSuccess.get() != nullptr, chip::AndroidClusterExceptions::GetInstance().ReturnIllegalStateException(env, callback, "Error creating native success callback", CHIP_ERROR_NO_MEMORY));
+
+    std::unique_ptr<chip::CHIPDefaultFailureCallback, void (*)(chip::CHIPDefaultFailureCallback *)> onFailure(chip::Platform::New<chip::CHIPDefaultFailureCallback>(callback), chip::Platform::Delete<chip::CHIPDefaultFailureCallback>);
+    VerifyOrReturn(onFailure.get() != nullptr, chip::AndroidClusterExceptions::GetInstance().ReturnIllegalStateException(env, callback, "Error creating native failure callback", CHIP_ERROR_NO_MEMORY));
+
+    CHIP_ERROR err = CHIP_NO_ERROR;
+    chip::Controller::ThirdCluster * cppCluster = reinterpret_cast<chip::Controller::ThirdCluster *>(clusterPtr);
+    VerifyOrReturn(cppCluster != nullptr, chip::AndroidClusterExceptions::GetInstance().ReturnIllegalStateException(env, callback, "Could not get native cluster", CHIP_ERROR_INCORRECT_STATE));
+
+    auto successFn = chip::Callback::Callback<CHIPThirdClusterSomeEnumAttributeCallbackType>::FromCancelable(onSuccess->Cancel());
+    auto failureFn = chip::Callback::Callback<CHIPDefaultFailureCallbackType>::FromCancelable(onFailure->Cancel());
+    err = cppCluster->ReadAttribute<TypeInfo>(onSuccess->mContext, successFn->mCall, failureFn->mCall);
+    VerifyOrReturn(err == CHIP_NO_ERROR, chip::AndroidClusterExceptions::GetInstance().ReturnIllegalStateException(env, callback, "Error reading attribute", err));
+
+    onSuccess.release();
+    onFailure.release();
+}
+

scripts/idl/tests/outputs/simple_attribute/jni/CHIPClustersRead.cpp → scripts/idl/tests/outputs/simple_attribute/jni/MyClusterClient-ReadImpl.cpp

@@ -12,7 +12,6 @@
 
 #define JNI_METHOD(RETURN, CLASS_NAME, METHOD_NAME)                                                                                \
     extern "C" JNIEXPORT RETURN JNICALL Java_chip_devicecontroller_ChipClusters_00024##CLASS_NAME##_##METHOD_NAME
-
 JNI_METHOD(void, MyClusterCluster, readClusterAttrAttribute)(JNIEnv * env, jobject self, jlong clusterPtr, jobject callback)
 {
     chip::DeviceLayer::StackLock lock;
@@ -36,4 +35,3 @@ JNI_METHOD(void, MyClusterCluster, readClusterAttrAttribute)(JNIEnv * env, jobje
     onFailure.release();
 }
 
-

src/controller/data_model/BUILD.gn

@@ -35,6 +35,17 @@ if (current_os == "android") {
     script = "${chip_root}/scripts/codegen.py"
 
     _idl_file = "controller-clusters.matter"
+    _output_files = exec_script("${chip_root}/scripts/codegen.py",
+                                [
+                                  "--generator",
+                                  "java",
+                                  "--log-level",
+                                  "fatal",
+                                  "--name-only",
+                                  rebase_path("controller-clusters.matter"),
+                                ],
+                                "list lines",
+                                [ "controller-clusters.matter" ])
 
     args = [
       "--generator",
@@ -48,7 +59,11 @@ if (current_os == "android") {
 
     inputs = [ _idl_file ]
     sources = [ _idl_file ]
-    outputs = [ "$target_gen_dir/jni/CHIPClustersRead.cpp" ]
+    outputs = []
+
+    foreach(name, _output_files) {
+      outputs += [ "$target_gen_dir/${name}" ]
+    }
   }
 
   source_set("java-jni-sources") {