Switch yaml tests to use ClusterBase::WriteAttribute for attribute wr…

…ite. (#11460) The changes to test_cluster.zapt are the actual change we care about. The change to WriteClient.h is to fix a pre-existing bug: it calls Encode() without including all the headers that declare various signatures of Encode(). The change to ClusterTestGeneration.js fixes a bug with the signature of the success callback for writes: it had an extra arg with the type of the attribute, whereas it should have no arg at all. The change to CHIPClusters-src.zapt is to instantiate WriteAttribute for all the non-writable attributes when compiling tests, because we actually issue writes for those non-writable attributes (and test that they fail). The change to controller/data_model/BUILD.gn is because the Android compiler in CI blows up trying to compile CHIPClustersTestWrite.cpp.
project-chip · Jan 28, 2022 · b648482 · b648482
1 parent e55fbd9
commit b648482
Show file tree

Hide file tree

Showing 10 changed files with 2,696 additions and 1,560 deletions.
diff --git a/examples/chip-tool/templates/partials/test_cluster.zapt b/examples/chip-tool/templates/partials/test_cluster.zapt
@@ -60,21 +60,31 @@ class {{filename}}: public TestCommand
     {{#unless (isTestOnlyCluster cluster)}}
     {{#unless isWait}}
     {{#unless isCommand}}
+    {{#unless isWriteAttribute}}
     chip::Callback::Callback<void (*) ({{>failureArguments}})> {{>failureCallback}} { {{>failureResponse}}, this };
     chip::Callback::Callback<void (*) ({{>successArguments}})> {{>successCallback}} { {{>successResponse}}, this };
     {{/unless}}
     {{/unless}}
     {{/unless}}
+    {{/unless}}
     {{/chip_tests_items}}
 
     {{#chip_tests_items}}
     {{#unless (isTestOnlyCluster cluster)}}
     {{#unless isWait}}
     {{#unless isCommand}}
+    {{#if isWriteAttribute}}
+    static void {{>failureResponse}}(void * context, EmberAfStatus status)
+    {
+        (static_cast<{{filename}} *>(context))->OnFailureResponse_{{index}}(chip::to_underlying(status));
+    }
+    {{else}}
     static void {{>failureResponse}}({{> failureArguments}})
     {
         (static_cast<{{filename}} *>(context))->OnFailureResponse_{{index}}(status);
     }
+    {{/if}}
+
 
     static void {{>successResponse}}({{> successArguments}})
     {
@@ -146,8 +156,15 @@ class {{filename}}: public TestCommand
         {{>commandValue ns=parent.cluster ignore=true container=(concat (asLowerCamelCase name) "Argument") definedValue=definedValue}}
         {{/chip_tests_item_parameters}}
 
-        {{~#*inline "commandName"}}{{asUpperCamelCase commandName}}{{#if isAttribute}}Attribute{{asUpperCamelCase attribute}}{{/if}}{{/inline}}
-        {{#if async}}ReturnErrorOnFailure({{else}}return {{/if}}cluster.{{>commandName}}({{>successCallback}}.Cancel(){{#unless isWaitForReport}}, {{>failureCallback}}.Cancel(){{/unless}}{{#chip_tests_item_parameters}}, {{asLowerCamelCase name}}Argument{{/chip_tests_item_parameters}}){{#if async}}){{/if}};
+        {{#if isWriteAttribute}}
+          {{#*inline "failureResponse"}}OnFailureCallback_{{index}}{{/inline}}
+          {{#*inline "successResponse"}}OnSuccessCallback_{{index}}{{/inline}}
+          {{#if async}}ReturnErrorOnFailure({{else}}return {{/if}}cluster.WriteAttribute<chip::app::Clusters::{{asUpperCamelCase cluster}}::Attributes::{{asUpperCamelCase attribute}}::TypeInfo>({{#chip_tests_item_parameters}}{{asLowerCamelCase name}}Argument, {{/chip_tests_item_parameters}}this, {{>successResponse}}, {{>failureResponse}}){{#if async}}){{/if}};
+        {{else}}
+          {{~#*inline "commandName"}}{{asUpperCamelCase commandName}}{{#if isAttribute}}Attribute{{asUpperCamelCase attribute}}{{/if}}{{/inline}}
+          {{#if async}}ReturnErrorOnFailure({{else}}return {{/if}}cluster.{{>commandName}}({{>successCallback}}.Cancel(){{#unless isWaitForReport}}, {{>failureCallback}}.Cancel(){{/unless}}{{#chip_tests_item_parameters}}, {{asLowerCamelCase name}}Argument{{/chip_tests_item_parameters}}){{#if async}}){{/if}};
+        {{/if}}
+
         {{/if}}
         {{#if async}}return WaitForMs(0);{{/if}}
     }

diff --git a/src/app/WriteClient.h b/src/app/WriteClient.h
@@ -25,6 +25,7 @@
 #include <app/MessageDef/AttributeStatusIB.h>
 #include <app/MessageDef/WriteRequestMessage.h>
 #include <app/data-model/Encode.h>
+#include <app/data-model/List.h>
 #include <lib/core/CHIPCore.h>
 #include <lib/core/CHIPTLVDebug.hpp>
 #include <lib/support/CodeUtils.h>

diff --git a/src/app/chip_data_model.gni b/src/app/chip_data_model.gni
@@ -178,6 +178,7 @@ template("chip_data_model") {
         sources += [
           "${invoker.zap_pregenerated_dir}/tests/CHIPClustersTest.cpp",
           "${invoker.zap_pregenerated_dir}/tests/CHIPClustersTest.h",
+          "${invoker.zap_pregenerated_dir}/tests/CHIPClustersTestWrite.cpp",
         ]
       }
     }

diff --git a/src/app/tests/suites/templates/templates.json b/src/app/tests/suites/templates/templates.json
@@ -23,6 +23,11 @@
             "path": "../../../zap-templates/templates/app/tests/CHIPClusters-src.zapt",
             "name": "Tests C++ API",
             "output": "tests/CHIPClustersTest.cpp"
+        },
+        {
+            "path": "../../../zap-templates/templates/app/tests/CHIPClustersWrite-src.zapt",
+            "name": "Tests WriteAttribute instantiations",
+            "output": "tests/CHIPClustersTestWrite.cpp"
         }
     ]
 }
diff --git a/src/app/zap-templates/common/ClusterTestGeneration.js b/src/app/zap-templates/common/ClusterTestGeneration.js
@@ -545,6 +545,9 @@ function chip_tests_item_response_parameters(options)
   const responseValues = this.response.values.slice();
 
   const promise = assertCommandOrAttribute(this).then(item => {
+    if (this.isWriteAttribute) {
+      return [];
+    }
     const responseArgs = item.response.arguments;
 
     const responses = responseArgs.map(responseArg => {

diff --git a/src/app/zap-templates/templates/app/tests/CHIPClustersWrite-src.zapt b/src/app/zap-templates/templates/app/tests/CHIPClustersWrite-src.zapt
@@ -0,0 +1,50 @@
+{{#if (chip_has_client_clusters)}}
+{{> header}}
+
+#include <zap-generated/tests/CHIPClustersTest.h>
+
+#include <app-common/zap-generated/cluster-objects.h>
+
+namespace chip {
+namespace Controller {
+
+{{#chip_client_clusters}}
+
+{{#chip_server_cluster_attributes}}
+{{#unless isWritableAttribute}}
+{{#*inline "attributeTypeInfo"}}chip::app::Clusters::{{asUpperCamelCase parent.name}}::Attributes::{{asUpperCamelCase name}}::TypeInfo{{/inline}}
+template CHIP_ERROR ClusterBase::WriteAttribute<{{>attributeTypeInfo}}>(const {{>attributeTypeInfo}}::Type & requestData, void *context,
+                    WriteResponseSuccessCallback successCb, WriteResponseFailureCallback failureCb);
+{{/unless}}
+{{/chip_server_cluster_attributes}}
+{{/chip_client_clusters}}
+
+template <typename AttributeInfo>
+CHIP_ERROR ClusterBase::WriteAttribute(const typename AttributeInfo::Type & requestData, void * context,
+                                       WriteResponseSuccessCallback successCb, WriteResponseFailureCallback failureCb)
+{
+    VerifyOrReturnError(mDevice != nullptr, CHIP_ERROR_INCORRECT_STATE);
+    ReturnErrorOnFailure(mDevice->LoadSecureSessionParametersIfNeeded());
+
+    auto onSuccessCb = [context, successCb](const app::ConcreteAttributePath & commandPath) {
+        if (successCb != nullptr)
+        {
+            successCb(context);
+        }
+    };
+
+    auto onFailureCb = [context, failureCb](const app::ConcreteAttributePath * commandPath, app::StatusIB status,
+                                            CHIP_ERROR aError) {
+        if (failureCb != nullptr)
+        {
+            failureCb(context, app::ToEmberAfStatus(status.mStatus));
+        }
+    };
+
+    return chip::Controller::WriteAttribute<AttributeInfo>(mDevice->GetExchangeManager(), mDevice->GetSecureSession().Value(),
+                                                           mEndpoint, requestData, onSuccessCb, onFailureCb);
+}
+
+} // namespace Controller
+} // namespace chip
+{{/if}}
diff --git a/src/controller/data_model/BUILD.gn b/src/controller/data_model/BUILD.gn
@@ -22,7 +22,12 @@ chip_data_model("data_model") {
   zap_pregenerated_dir =
       "${chip_root}/zzz_generated/controller-clusters/zap-generated"
 
-  use_tests_apis = true
+  # On Android, we don't really need the tests APIs, and the compiler has
+  # trouble compiling some of those files anyway.
+  #
+  # TODO: This should default to false and consumers should opt in to it
+  # as needed.
+  use_tests_apis = current_os != "android"
   use_default_client_callbacks = true
   allow_circular_includes_from = [ "${chip_root}/src/controller" ]
 }
diff --git a/src/darwin/Framework/CHIP.xcodeproj/project.pbxproj b/src/darwin/Framework/CHIP.xcodeproj/project.pbxproj
@@ -55,6 +55,7 @@
 		2F79A67726CE6672006377B0 /* im-client-callbacks.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 2F79A67626CE6672006377B0 /* im-client-callbacks.cpp */; };
 		2FD775552695557E00FF4B12 /* error-mapping.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 2FD775542695557E00FF4B12 /* error-mapping.cpp */; };
 		5129BCFD26A9EE3300122DDF /* CHIPError.h in Headers */ = {isa = PBXBuildFile; fileRef = 5129BCFC26A9EE3300122DDF /* CHIPError.h */; settings = {ATTRIBUTES = (Public, ); }; };
+		515CCFD227351183002A3C82 /* CHIPClustersTestWrite.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 515CCFD127351183002A3C82 /* CHIPClustersTestWrite.cpp */; };
 		991DC0842475F45400C13860 /* CHIPDeviceController.h in Headers */ = {isa = PBXBuildFile; fileRef = 991DC0822475F45400C13860 /* CHIPDeviceController.h */; settings = {ATTRIBUTES = (Public, ); }; };
 		991DC0892475F47D00C13860 /* CHIPDeviceController.mm in Sources */ = {isa = PBXBuildFile; fileRef = 991DC0872475F47D00C13860 /* CHIPDeviceController.mm */; };
 		991DC08B247704DC00C13860 /* CHIPLogging.h in Headers */ = {isa = PBXBuildFile; fileRef = 991DC08A247704DC00C13860 /* CHIPLogging.h */; };
@@ -141,6 +142,7 @@
 		2F79A67626CE6672006377B0 /* im-client-callbacks.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = "im-client-callbacks.cpp"; path = "../../../app/util/im-client-callbacks.cpp"; sourceTree = "<group>"; };
 		2FD775542695557E00FF4B12 /* error-mapping.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = "error-mapping.cpp"; path = "../../../app/util/error-mapping.cpp"; sourceTree = "<group>"; };
 		5129BCFC26A9EE3300122DDF /* CHIPError.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = CHIPError.h; path = CHIP/CHIPError.h; sourceTree = "<group>"; };
+		515CCFD127351183002A3C82 /* CHIPClustersTestWrite.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; name = CHIPClustersTestWrite.cpp; path = "../../../zzz_generated/controller-clusters/zap-generated/tests/CHIPClustersTestWrite.cpp"; sourceTree = "<group>"; };
 		991DC0822475F45400C13860 /* CHIPDeviceController.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = CHIPDeviceController.h; sourceTree = "<group>"; };
 		991DC0872475F47D00C13860 /* CHIPDeviceController.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = CHIPDeviceController.mm; sourceTree = "<group>"; };
 		991DC08A247704DC00C13860 /* CHIPLogging.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = CHIPLogging.h; sourceTree = "<group>"; };
@@ -240,6 +242,7 @@
 		B20252832459E34F00F97062 = {
 			isa = PBXGroup;
 			children = (
+				515CCFD127351183002A3C82 /* CHIPClustersTestWrite.cpp */,
 				5129BCFC26A9EE3300122DDF /* CHIPError.h */,
 				BA107AEE2470CFBB004287EB /* chip_xcode_build_connector.sh */,
 				B202528F2459E34F00F97062 /* CHIP */,
@@ -478,6 +481,7 @@
 			isa = PBXSourcesBuildPhase;
 			buildActionMask = 2147483647;
 			files = (
+				515CCFD227351183002A3C82 /* CHIPClustersTestWrite.cpp in Sources */,
 				2C8C8FC2253E0C2100797F05 /* CHIPPersistentStorageDelegateBridge.mm in Sources */,
 				2CB7163C252E8A7C0026E2BB /* CHIPDevicePairingDelegateBridge.mm in Sources */,
 				997DED162695343400975E97 /* CHIPThreadOperationalDataset.mm in Sources */,