Codegen a ResponseType for command cluster objects and use it to simplify templates (#11624)

* Output a ResponseType for command Type structs.

* Use ResponseType in our InvokeCommand templates.

Author: bzbarsky-apple

examples/chip-tool/templates/partials/test_cluster.zapt

@@ -141,22 +141,21 @@ class {{filename}}: public TestCommand
         cluster.Associate(mDevice, {{endpoint}});
 
         {{#if isCommand}}
-        using requestType  = chip::app::Clusters::{{asUpperCamelCase cluster}}::Commands::{{asUpperCamelCase command}}::Type;
-        using responseType = chip::app::{{chip_tests_item_response_type}};
+        using RequestType = chip::app::Clusters::{{asUpperCamelCase cluster}}::Commands::{{asUpperCamelCase command}}::Type;
 
-        chip::app::Clusters::{{asUpperCamelCase cluster}}::Commands::{{asUpperCamelCase command}}::Type request;
+        RequestType request;
         {{#chip_tests_item_parameters}}
         {{>commandValue ns=parent.cluster container=(concat "request." (asLowerCamelCase label)) definedValue=definedValue}}
         {{/chip_tests_item_parameters}}
 
-        auto success = [](void * context, const responseType & data) {
+        auto success = [](void * context, const typename RequestType::ResponseType & data) {
             (static_cast<{{filename}} *>(context))->OnSuccessResponse_{{index}}({{#chip_tests_item_response_parameters}}{{#not_first}}, {{/not_first}}data.{{asLowerCamelCase name}}{{/chip_tests_item_response_parameters}});
         };
 
         auto failure = [](void * context, EmberAfStatus status) {
             (static_cast<{{filename}} *>(context))->OnFailureResponse_{{index}}(status);
         };
-        {{#if async}}ReturnErrorOnFailure({{else}}return {{/if}}cluster.InvokeCommand<requestType, responseType>(request, this, success, failure){{#if async}}){{/if}};
+        {{#if async}}ReturnErrorOnFailure({{else}}return {{/if}}cluster.InvokeCommand(request, this, success, failure){{#if async}}){{/if}};
         {{else}}
         {{#chip_tests_item_parameters}}
         {{zapTypeToEncodableClusterObjectType type ns=parent.cluster}} {{asLowerCamelCase name}}Argument;

src/app/zap-templates/common/ClusterTestGeneration.js

@@ -453,20 +453,6 @@ function isTestOnlyCluster(name)
   return testOnlyClusters.includes(name);
 }
 
-function chip_tests_item_response_type(options)
-{
-  const promise = assertCommandOrAttribute(this).then(item => {
-    if (item.hasSpecificResponse) {
-      return 'Clusters::' + asUpperCamelCase(this.cluster) + '::Commands::' + asUpperCamelCase(item.response.name)
-          + '::DecodableType';
-    }
-
-    return 'DataModel::NullObjectType';
-  });
-
-  return asPromise.call(this, promise, options);
-}
-
 // test_cluster_command_value and test_cluster_value-equals are recursive partials using #each. At some point the |global|
 // context is lost and it fails. Make sure to attach the global context as a property of the | value |
 // that is evaluated.
@@ -602,7 +588,6 @@ function expectedValueHasProp(value, name)
 exports.chip_tests                          = chip_tests;
 exports.chip_tests_items                    = chip_tests_items;
 exports.chip_tests_item_parameters          = chip_tests_item_parameters;
-exports.chip_tests_item_response_type       = chip_tests_item_response_type;
 exports.chip_tests_item_response_parameters = chip_tests_item_response_parameters;
 exports.chip_tests_pics                     = chip_tests_pics;
 exports.isTestOnlyCluster                   = isTestOnlyCluster;

src/app/zap-templates/templates/app/CHIPClustersInvoke-src.zapt

@@ -18,27 +18,26 @@ namespace Controller {
 
 {{#chip_cluster_commands}}
 {{#*inline "requestType"}}chip::app::Clusters::{{asUpperCamelCase parent.name}}::Commands::{{asUpperCamelCase name}}::Type{{/inline}}
-{{#*inline "responseType"}}chip::app::{{#if hasSpecificResponse}}Clusters::{{asUpperCamelCase parent.name}}::Commands::{{asUpperCamelCase response.name}}::DecodableType{{else}}DataModel::NullObjectType{{/if}}{{/inline}}
-template CHIP_ERROR ClusterBase::InvokeCommand<{{>requestType}}, {{>responseType}}>(const {{>requestType}} &, void *, CommandResponseSuccessCallback<{{>responseType}}>, CommandResponseFailureCallback);
+template CHIP_ERROR ClusterBase::InvokeCommand<{{>requestType}}>(const {{>requestType}} &, void *, CommandResponseSuccessCallback<typename {{>requestType}}::ResponseType>, CommandResponseFailureCallback);
 {{/chip_cluster_commands}}
 {{/chip_client_clusters}}
 
-template <typename RequestDataT, typename ResponseDataT>
+template <typename RequestDataT>
 CHIP_ERROR ClusterBase::InvokeCommand(const RequestDataT & requestData, void * context,
-                         CommandResponseSuccessCallback<ResponseDataT> successCb, CommandResponseFailureCallback failureCb)
+                         CommandResponseSuccessCallback<typename RequestDataT::ResponseType> successCb, CommandResponseFailureCallback failureCb)
 {
     VerifyOrReturnError(mDevice != nullptr, CHIP_ERROR_INCORRECT_STATE);
 
-    auto onSuccessCb = [context, successCb](const app::ConcreteCommandPath & commandPath, const app::StatusIB & aStatus, const ResponseDataT & responseData) {
+    auto onSuccessCb = [context, successCb](const app::ConcreteCommandPath & commandPath, const app::StatusIB & aStatus, const typename RequestDataT::ResponseType & responseData) {
         successCb(context, responseData);
     };
 
     auto onFailureCb = [context, failureCb](const app::StatusIB & aStatus, CHIP_ERROR aError) {
         failureCb(context, app::ToEmberAfStatus(aStatus.mStatus));
     };
 
-    return InvokeCommandRequest<ResponseDataT>(mDevice->GetExchangeManager(), mDevice->GetSecureSession().Value(), mEndpoint,
-                                               requestData, onSuccessCb, onFailureCb);
+    return InvokeCommandRequest(mDevice->GetExchangeManager(), mDevice->GetSecureSession().Value(), mEndpoint,
+                                requestData, onSuccessCb, onFailureCb);
 };
 
 } // namespace Controller

src/app/zap-templates/templates/app/cluster-objects.zapt

@@ -6,6 +6,7 @@
 #include <app/data-model/Decode.h>
 #include <app/data-model/Encode.h>
 #include <app/data-model/List.h>
+#include <app/data-model/NullObject.h>
 #include <app/EventLoggingTypes.h>
 #include <app/util/basic-types.h>
 #include <lib/support/BitFlags.h>
@@ -87,6 +88,22 @@ namespace {{asUpperCamelCase name}} {
 {{/last}}
 {{/zcl_structs}}
 
+{{#zcl_commands}}
+{{#first}}
+namespace Commands {
+ // Forward-declarations so we can reference these later.
+{{/first}}
+
+namespace {{asUpperCamelCase name}} {
+ struct Type;
+ struct DecodableType;
+} // namespace {{asUpperCamelCase name}}
+{{#last}}
+
+} // namespace Commands
+{{/last}}
+{{/zcl_commands}}
+
 {{#zcl_commands}}
 {{#first}}
 namespace Commands {
@@ -110,6 +127,13 @@ public:
     {{/zcl_command_arguments}}
 
     CHIP_ERROR Encode(TLV::TLVWriter &writer, TLV::Tag tag) const;
+
+    using ResponseType =
+    {{~#if responseRef}}
+      Clusters::{{asUpperCamelCase parent.name}}::Commands::{{getResponseCommandName responseRef}}::DecodableType;
+    {{else}}
+      DataModel::NullObjectType;
+    {{/if}}
 };
 
 struct DecodableType {

src/app/zap-templates/templates/app/helper.js

@@ -19,6 +19,7 @@
 const zapPath      = '../../../../../third_party/zap/repo/dist/src-electron/';
 const templateUtil = require(zapPath + 'generator/template-util.js')
 const zclHelper    = require(zapPath + 'generator/helper-zcl.js')
+const queryCommand = require(zapPath + 'db/query-command.js')
 const zclQuery     = require(zapPath + 'db/query-zcl.js')
 const cHelper      = require(zapPath + 'generator/helper-c.js')
 const string       = require(zapPath + 'util/string.js')
@@ -476,6 +477,14 @@ function zapTypeToPythonClusterObjectType(type, options)
   return templateUtil.templatePromise(this.global, promise)
 }
 
+async function getResponseCommandName(responseRef, options)
+{
+  let pkgId = await templateUtil.ensureZclPackageId(this);
+
+  const { db, sessionId } = this.global;
+  return queryCommand.selectCommandById(db, responseRef, pkgId).then(response => asUpperCamelCase(response.name));
+}
+
 //
 // Module exports
 //
@@ -491,3 +500,4 @@ exports.asMEI                               = asMEI;
 exports.zapTypeToEncodableClusterObjectType = zapTypeToEncodableClusterObjectType;
 exports.zapTypeToDecodableClusterObjectType = zapTypeToDecodableClusterObjectType;
 exports.zapTypeToPythonClusterObjectType    = zapTypeToPythonClusterObjectType;
+exports.getResponseCommandName              = getResponseCommandName;

src/controller/CHIPCluster.h

@@ -62,9 +62,10 @@ class DLL_EXPORT ClusterBase
      * Success and Failure callbacks must be passed in through which the decoded response is provided as well as notification of any
      * failure.
      */
-    template <typename RequestDataT, typename ResponseDataT>
+    template <typename RequestDataT>
     CHIP_ERROR InvokeCommand(const RequestDataT & requestData, void * context,
-                             CommandResponseSuccessCallback<ResponseDataT> successCb, CommandResponseFailureCallback failureCb);
+                             CommandResponseSuccessCallback<typename RequestDataT::ResponseType> successCb,
+                             CommandResponseFailureCallback failureCb);
 
     /**
      * Functions for writing attributes.  We have lots of different

src/controller/InvokeInteraction.h

@@ -30,29 +30,31 @@ namespace Controller {
  * the provided success callback or calls the provided failure callback.
  *
  * The RequestObjectT is generally expected to be a ClusterName::Commands::CommandName::Type struct, but any object
- * that can be encoded using the DataModel::Encode machinery and exposes the GetClusterId() and GetCommandId() functions
+ * that can be encoded using the DataModel::Encode machinery and exposes the
+ * GetClusterId() and GetCommandId() functions and a ResponseType type
  * is expected to work.
  *
- * The ResponseObjectT is expected to be one of two things:
+ * The ResponseType is expected to be one of two things:
  *
  *    - If a data response is expected on success, a struct type decodable via DataModel::Decode which has GetClusterId() and
  * GetCommandId() methods.  A ClusterName::Commands::ResponseCommandName::DecodableType is typically used.
- *    - If a status response is expected on success, a DataModel::NullObjectType.
+ *    - If a status response is expected on success, DataModel::NullObjectType.
  *
  */
-template <typename ResponseObjectT = app::DataModel::NullObjectType, typename RequestObjectT>
-CHIP_ERROR InvokeCommandRequest(Messaging::ExchangeManager * aExchangeMgr, SessionHandle sessionHandle, chip::EndpointId endpointId,
-                                const RequestObjectT & requestCommandData,
-                                typename TypedCommandCallback<ResponseObjectT>::OnSuccessCallbackType onSuccessCb,
-                                typename TypedCommandCallback<ResponseObjectT>::OnErrorCallbackType onErrorCb)
+template <typename RequestObjectT>
+CHIP_ERROR
+InvokeCommandRequest(Messaging::ExchangeManager * aExchangeMgr, SessionHandle sessionHandle, chip::EndpointId endpointId,
+                     const RequestObjectT & requestCommandData,
+                     typename TypedCommandCallback<typename RequestObjectT::ResponseType>::OnSuccessCallbackType onSuccessCb,
+                     typename TypedCommandCallback<typename RequestObjectT::ResponseType>::OnErrorCallbackType onErrorCb)
 {
     app::CommandPathParams commandPath = { endpointId, 0, RequestObjectT::GetClusterId(), RequestObjectT::GetCommandId(),
                                            (app::CommandPathFlags::kEndpointIdValid) };
 
     //
     // Let's create a handle version of the decoder to ensure we do correct clean-up of it if things go south at any point below
     //
-    auto decoder = chip::Platform::MakeUnique<TypedCommandCallback<ResponseObjectT>>(onSuccessCb, onErrorCb);
+    auto decoder = chip::Platform::MakeUnique<TypedCommandCallback<typename RequestObjectT::ResponseType>>(onSuccessCb, onErrorCb);
     VerifyOrReturnError(decoder != nullptr, CHIP_ERROR_NO_MEMORY);
 
     //

src/controller/tests/TestServerCommandDispatch.cpp

@@ -114,10 +114,18 @@ class TestCommandInteraction
 private:
 };
 
+// We want to send a TestSimpleArgumentRequest::Type, but get a
+// TestStructArrayArgumentResponse in return, so need to shadow the actual
+// ResponseType that TestSimpleArgumentRequest has.
+struct FakeRequest : public TestCluster::Commands::TestSimpleArgumentRequest::Type
+{
+    using ResponseType = TestCluster::Commands::TestStructArrayArgumentResponse::DecodableType;
+};
+
 void TestCommandInteraction::TestNoHandler(nlTestSuite * apSuite, void * apContext)
 {
     TestContext & ctx = *static_cast<TestContext *>(apContext);
-    TestCluster::Commands::TestSimpleArgumentRequest::Type request;
+    FakeRequest request;
     auto sessionHandle = ctx.GetSessionBobToAlice();
 
     request.arg1 = true;
@@ -142,8 +150,8 @@ void TestCommandInteraction::TestNoHandler(nlTestSuite * apSuite, void * apConte
 
     ctx.EnableAsyncDispatch();
 
-    chip::Controller::InvokeCommandRequest<TestCluster::Commands::TestStructArrayArgumentResponse::DecodableType>(
-        &ctx.GetExchangeManager(), sessionHandle, kTestEndpointId, request, onSuccessCb, onFailureCb);
+    chip::Controller::InvokeCommandRequest(&ctx.GetExchangeManager(), sessionHandle, kTestEndpointId, request, onSuccessCb,
+                                           onFailureCb);
 
     ctx.DrainAndServiceIO();
 
@@ -176,7 +184,7 @@ DECLARE_DYNAMIC_ENDPOINT(testEndpoint, testEndpointClusters);
 void TestCommandInteraction::TestDataResponse(nlTestSuite * apSuite, void * apContext)
 {
     TestContext & ctx = *static_cast<TestContext *>(apContext);
-    TestCluster::Commands::TestSimpleArgumentRequest::Type request;
+    FakeRequest request;
     auto sessionHandle = ctx.GetSessionBobToAlice();
     TestClusterCommandHandler commandHandler;
 
@@ -220,8 +228,8 @@ void TestCommandInteraction::TestDataResponse(nlTestSuite * apSuite, void * apCo
 
     responseDirective = kSendDataResponse;
 
-    chip::Controller::InvokeCommandRequest<TestCluster::Commands::TestStructArrayArgumentResponse::DecodableType>(
-        &ctx.GetExchangeManager(), sessionHandle, kTestEndpointId, request, onSuccessCb, onFailureCb);
+    chip::Controller::InvokeCommandRequest(&ctx.GetExchangeManager(), sessionHandle, kTestEndpointId, request, onSuccessCb,
+                                           onFailureCb);
 
     ctx.DrainAndServiceIO();
 

src/controller/tests/data_model/TestCommands.cpp

@@ -159,7 +159,15 @@ class TestCommandInteraction
 void TestCommandInteraction::TestDataResponse(nlTestSuite * apSuite, void * apContext)
 {
     TestContext & ctx = *static_cast<TestContext *>(apContext);
-    TestCluster::Commands::TestSimpleArgumentRequest::Type request;
+    // We want to send a TestSimpleArgumentRequest::Type, but get a
+    // TestStructArrayArgumentResponse in return, so need to shadow the actual
+    // ResponseType that TestSimpleArgumentRequest has.
+    struct FakeRequest : public TestCluster::Commands::TestSimpleArgumentRequest::Type
+    {
+        using ResponseType = TestCluster::Commands::TestStructArrayArgumentResponse::DecodableType;
+    };
+
+    FakeRequest request;
     auto sessionHandle = ctx.GetSessionBobToAlice();
 
     bool onSuccessWasCalled = false;
@@ -196,8 +204,8 @@ void TestCommandInteraction::TestDataResponse(nlTestSuite * apSuite, void * apCo
 
     responseDirective = kSendDataResponse;
 
-    chip::Controller::InvokeCommandRequest<TestCluster::Commands::TestStructArrayArgumentResponse::DecodableType>(
-        &ctx.GetExchangeManager(), sessionHandle, kTestEndpointId, request, onSuccessCb, onFailureCb);
+    chip::Controller::InvokeCommandRequest(&ctx.GetExchangeManager(), sessionHandle, kTestEndpointId, request, onSuccessCb,
+                                           onFailureCb);
 
     ctx.DrainAndServiceIO();