Skip to content

Commit

Permalink
Switch yaml tests to use ClusterBase::WriteAttribute for attribute wr…
Browse files Browse the repository at this point in the history
…ite. (project-chip#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.
  • Loading branch information
bzbarsky-apple authored and PSONALl committed Dec 2, 2021
1 parent 71a95ae commit 811868e
Show file tree
Hide file tree
Showing 10 changed files with 2,696 additions and 1,560 deletions.
21 changes: 19 additions & 2 deletions examples/chip-tool/templates/partials/test_cluster.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -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}})
{
Expand Down Expand Up @@ -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}}
}
Expand Down
1 change: 1 addition & 0 deletions src/app/WriteClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
1 change: 1 addition & 0 deletions src/app/chip_data_model.gni
Original file line number Diff line number Diff line change
Expand Up @@ -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",
]
}
}
Expand Down
5 changes: 5 additions & 0 deletions src/app/tests/suites/templates/templates.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
]
}
3 changes: 3 additions & 0 deletions src/app/zap-templates/common/ClusterTestGeneration.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down
Original file line number Diff line number Diff line change
@@ -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}}
7 changes: 6 additions & 1 deletion src/controller/data_model/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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" ]
}
4 changes: 4 additions & 0 deletions src/darwin/Framework/CHIP.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -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 */; };
Expand Down Expand Up @@ -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>"; };
Expand Down Expand Up @@ -240,6 +242,7 @@
B20252832459E34F00F97062 = {
isa = PBXGroup;
children = (
515CCFD127351183002A3C82 /* CHIPClustersTestWrite.cpp */,
5129BCFC26A9EE3300122DDF /* CHIPError.h */,
BA107AEE2470CFBB004287EB /* chip_xcode_build_connector.sh */,
B202528F2459E34F00F97062 /* CHIP */,
Expand Down Expand Up @@ -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 */,
Expand Down
Loading

0 comments on commit 811868e

Please sign in to comment.