Switch server-side command decoding to using the cluster-objects infr…

…astructure (#10296)

* Switch command decoding to using the cluster-objects infrastructure.

For now we are still passing the existing arguments to cluster
callbacks, so we don't need to update cluster implementations as part
of this change.

The change to how we encode CHAR_STRING command arguments is needed
because the new setup expects to decode them as TLV UTF-8 strings, not
TLV octet strings.

* Regenerate generated files

examples/all-clusters-app/esp32/main/CMakeLists.txt

@@ -29,7 +29,9 @@ set(PRIV_INCLUDE_DIRS_LIST
 set(SRC_DIRS_LIST
                       "${CMAKE_CURRENT_LIST_DIR}"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/zzz_generated/all-clusters-app/zap-generated"
+                      "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/zzz_generated/app-common/app-common/zap-generated"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/zzz_generated/app-common/app-common/zap-generated/attributes"
+                      "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/zzz_generated/app-common/app-common/zap-generated"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/examples/platform/esp32/shell_extension"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/server"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/util"

examples/bridge-app/esp32/main/CMakeLists.txt

@@ -25,6 +25,7 @@ idf_component_register(PRIV_INCLUDE_DIRS
                       "${CMAKE_CURRENT_LIST_DIR}"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/zzz_generated/bridge-app/zap-generated"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/zzz_generated/app-common/app-common/zap-generated/attributes"
+                      "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/zzz_generated/app-common/app-common/zap-generated"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/server"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/util"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/reporting"

examples/lighting-app/mbed/CMakeLists.txt

@@ -50,6 +50,7 @@ target_sources(${APP_TARGET} PRIVATE
                ${GEN_DIR}/lighting-app/zap-generated/IMClusterCommandHandler.cpp
                ${MBED_COMMON}/util/LEDWidget.cpp
                ${CHIP_ROOT}/zzz_generated/app-common/app-common/zap-generated/attributes/Accessors.cpp
+               ${CHIP_ROOT}/zzz_generated/app-common/app-common/zap-generated/cluster-objects.cpp
                ${CHIP_ROOT}/src/app/util/DataModelHandler.cpp
                ${CHIP_ROOT}/src/app/reporting/reporting-default-configuration.cpp
                ${CHIP_ROOT}/src/app/reporting/reporting.cpp

examples/lighting-app/telink/CMakeLists.txt

@@ -52,6 +52,7 @@ target_sources(app PRIVATE
                ${TELINK_COMMON}/util/src/ButtonManager.cpp
                ${TELINK_COMMON}/util/src/ThreadUtil.cpp
                ${CHIP_ROOT}/zzz_generated/app-common/app-common/zap-generated/attributes/Accessors.cpp
+               ${CHIP_ROOT}/zzz_generated/app-common/app-common/zap-generated/cluster-objects.cpp
                ${CHIP_ROOT}/src/app/util/DataModelHandler.cpp
                ${CHIP_ROOT}/src/app/reporting/reporting-default-configuration.cpp
                ${CHIP_ROOT}/src/app/reporting/reporting.cpp

examples/lock-app/esp32/main/CMakeLists.txt

@@ -36,6 +36,7 @@ idf_component_register(INCLUDE_DIRS
                      "${CMAKE_SOURCE_DIR}/../../common/pigweed"
                      "${CMAKE_SOURCE_DIR}/../../common/pigweed/esp32"
                      "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/zzz_generated/app-common/app-common/zap-generated/attributes"
+                     "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/zzz_generated/app-common/app-common/zap-generated"
                      "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/zzz_generated/lock-app/zap-generated"
                      "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/server"
                      "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/util"
@@ -128,6 +129,7 @@ idf_component_register(PRIV_INCLUDE_DIRS
                       SRC_DIRS
                       "${CMAKE_CURRENT_LIST_DIR}"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/zzz_generated/app-common/app-common/zap-generated/attributes"
+                      "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/zzz_generated/app-common/app-common/zap-generated"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/zzz_generated/lock-app/zap-generated"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/examples/platform/esp32/shell_extension"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/server"

examples/lock-app/mbed/CMakeLists.txt

@@ -50,6 +50,7 @@ target_sources(${APP_TARGET} PRIVATE
                ${MBED_COMMON}/util/LEDWidget.cpp
                ${CHIP_ROOT}/src/app/util/DataModelHandler.cpp
                ${CHIP_ROOT}/zzz_generated/app-common/app-common/zap-generated/attributes/Accessors.cpp
+               ${CHIP_ROOT}/zzz_generated/app-common/app-common/zap-generated/cluster-objects.cpp
                ${CHIP_ROOT}/src/app/reporting/reporting-default-configuration.cpp
                ${CHIP_ROOT}/src/app/reporting/reporting.cpp
                ${CHIP_ROOT}/src/app/util/af-event.cpp

examples/temperature-measurement-app/esp32/main/CMakeLists.txt

@@ -27,6 +27,7 @@ idf_component_register(PRIV_INCLUDE_DIRS
                       "${CMAKE_CURRENT_LIST_DIR}"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/zzz_generated/temperature-measurement-app/zap-generated"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/zzz_generated/app-common/app-common/zap-generated/attributes"
+                      "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/zzz_generated/app-common/app-common/zap-generated"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/server"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/util"
                       "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/reporting"

src/app/chip_data_model.cmake

@@ -72,6 +72,7 @@ function(chip_configure_data_model APP_TARGET)
 
     target_sources(${APP_TARGET} PRIVATE
         ${CHIP_APP_BASE_DIR}/../../zzz_generated/app-common/app-common/zap-generated/attributes/Accessors.cpp
+        ${CHIP_APP_BASE_DIR}/../../zzz_generated/app-common/app-common/zap-generated/cluster-objects.cpp
         ${CHIP_APP_BASE_DIR}/reporting/reporting-default-configuration.cpp
         ${CHIP_APP_BASE_DIR}/reporting/reporting.cpp
         ${CHIP_APP_BASE_DIR}/util/af-event.cpp

src/app/zap-templates/partials/im_command_handler_cluster_commands.zapt

@@ -1,6 +1,34 @@
 {{#if (isServer parent.side)}}
 Commands::{{asUpperCamelCase name}}::DecodableType commandData;
+TLVError = DataModel::Decode(aDataTlv, commandData);
+if (TLVError == CHIP_NO_ERROR) {
+{{! A few things going on here:
+  1) isArray should override other type checks, because you can test true for
+     both isArray and other things.
+  2) Enums are enum classes in the struct but raw integers in the args.  Need
+     to use to_underlying for them.
+  3) Char strings are Span<const char> in the struct but uint8_t* in the args.
+     Do the same broken casting we used to do.  Need to stop using the args!
+  4) Lists are DecodableList<> in the struct but the args expect
+     uint8_t*.  Just pass nullptr and convert the relevant commands to
+     using the struct argument later.  They don't work properly right
+     now anyway.
+}}
+wasHandled = emberAf{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback(apCommandObj, aCommandPath, aCommandPath.mEndpointId, {{#zcl_command_arguments}}{{#if isArray}}
+nullptr
+{{else}}
+{{#if_is_enum type}}
+to_underlying(commandData.{{asLowerCamelCase label}})
+{{else}}
+{{#if (isCharString type)}}
+const_cast<uint8_t*>(Uint8::from_const_char(commandData.{{asLowerCamelCase label}}.data()))
+{{else}}
+commandData.{{asLowerCamelCase label}}
 {{/if}}
+{{/if_is_enum}}
+{{/if}},{{/zcl_command_arguments}} commandData);
+}
+{{else}}
 {{#if (zcl_command_arguments_count this.id)}}
 expectArgumentCount = {{ zcl_command_arguments_count this.id }};
 {{#zcl_command_arguments}}
@@ -74,12 +102,8 @@ if (CHIP_END_OF_TLV == TLVError)
 if (CHIP_NO_ERROR == TLVError && CHIP_NO_ERROR == TLVUnpackError && {{zcl_command_arguments_count this.id}} == validArgumentCount)
 {
 {{/if}}
-{{! TODO: if the command has a response command defined, put its id in the command path. }}
-{{#if (isServer parent.side)}}
-wasHandled = emberAf{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback(apCommandObj, aCommandPath, aCommandPath.mEndpointId, {{#zcl_command_arguments}}{{#if (isCharString type)}}const_cast<uint8_t*>({{asSymbol label}}){{else}}{{asSymbol label}}{{/if}},{{/zcl_command_arguments}} commandData);
-{{else}}
 wasHandled = emberAf{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback(aCommandPath.mEndpointId, apCommandObj{{#zcl_command_arguments}}, {{#if (isCharString type)}}const_cast<uint8_t*>({{asSymbol label}}){{else}}{{asSymbol label}}{{/if}}{{/zcl_command_arguments}});
-{{/if}}
 {{#if (zcl_command_arguments_count this.id)}}
 }
 {{/if}}
+{{/if}}

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

@@ -10,6 +10,7 @@
 #include <app/chip-zcl-zpro-codec.h>
 #include <app/util/basic-types.h>
 #include <app-common/zap-generated/ids/Attributes.h>
+#include <lib/core/CHIPSafeCasts.h>
 #include <lib/support/BufferWriter.h>
 #include <lib/support/CodeUtils.h>
 #include <lib/support/SafeInt.h>
@@ -58,7 +59,13 @@ CHIP_ERROR {{asUpperCamelCase clusterName}}Cluster::{{asUpperCamelCase name}}(Ca
     VerifyOrExit((writer = sender->GetCommandDataElementTLVWriter()) != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
 {{/first}}
     // {{asLowerCamelCase label}}: {{asLowerCamelCase type}}
+{{#if (isCharString type)}}
+    {{! The server expects to get a UTF-8 string, but we have ByteSpan.  Do
+        some conversion to get the right thing to happen. }}
+    SuccessOrExit(err = writer->PutString(TLV::ContextTag(argSeqNumber++), Span<const char>(Uint8::to_const_char({{asLowerCamelCase label}}.data()), {{asLowerCamelCase label}}.size())));
+{{else}}
     SuccessOrExit(err = writer->Put(TLV::ContextTag(argSeqNumber++), {{asLowerCamelCase label}}));
+{{/if}}
 {{else}}
     // Command takes no arguments.
 {{/chip_cluster_command_arguments}}

src/app/zap-templates/templates/app/im-cluster-command-handler.zapt

@@ -9,8 +9,9 @@
 #include <app-common/zap-generated/ids/Clusters.h>
 #include <app-common/zap-generated/ids/Commands.h>
 #include "app/util/util.h"
-
 #include <app/InteractionModelEngine.h>
+#include <lib/core/CHIPSafeCasts.h>
+#include <lib/support/TypeTraits.h>
 
 // Currently we need some work to keep compatible with ember lib.
 #include <app/util/ember-compatibility-functions.h>
@@ -44,10 +45,12 @@ void Dispatch{{asUpperCamelCase side}}Command({{#if (isServer side)}}CommandHand
     // Any error value TLVUnpackError means we have received an illegal value.
     // The following variables are used for all commands to save code size.
     CHIP_ERROR TLVError = CHIP_NO_ERROR;
+    {{#unless (isServer parent.side)}}
     CHIP_ERROR TLVUnpackError = CHIP_NO_ERROR;
     uint32_t validArgumentCount = 0;
     uint32_t expectArgumentCount = 0;
     uint32_t currentDecodeTagId = 0;
+    {{/unless}}
     bool wasHandled = false;
     {
         switch (aCommandPath.mCommandId)
@@ -66,15 +69,19 @@ void Dispatch{{asUpperCamelCase side}}Command({{#if (isServer side)}}CommandHand
         }
     }
 
-    if (CHIP_NO_ERROR != TLVError || CHIP_NO_ERROR != TLVUnpackError || expectArgumentCount != validArgumentCount || !wasHandled)
+    if (CHIP_NO_ERROR != TLVError {{#unless (isServer parent.side)}}|| CHIP_NO_ERROR != TLVUnpackError || expectArgumentCount != validArgumentCount{{/unless}} || !wasHandled)
     {
       apCommandObj->AddStatusCode(aCommandPath, Protocols::SecureChannel::GeneralStatusCode::kBadRequest, Protocols::SecureChannel::Id,
         Protocols::InteractionModel::Status::InvalidCommand);
+      {{#if (isServer parent.side)}}
+      ChipLogProgress(Zcl, "Failed to dispatch command, TLVError=%" CHIP_ERROR_FORMAT, TLVError.Format());
+      {{else}}
       ChipLogProgress(Zcl, "Failed to dispatch command, %" PRIu32 "/%" PRIu32 " arguments parsed, TLVError=%" CHIP_ERROR_FORMAT ", UnpackError=%" CHIP_ERROR_FORMAT " (last decoded tag = %" PRIu32, validArgumentCount, expectArgumentCount, TLVError.Format(), TLVUnpackError.Format(), currentDecodeTagId);
       // A command with no arguments would never write currentDecodeTagId.  If
       // progress logging is also disabled, it would look unused.  Silence that
       // warning.
       UNUSED_VAR(currentDecodeTagId);
+      {{/if}}
     }
     {{/last}}
     {{else}}
@@ -95,8 +102,6 @@ void DispatchSingleClusterCommand(const ConcreteCommandPath & aCommandPath, TLV:
 
     Compatibility::SetupEmberAfObjects(apCommandObj, aCommandPath);
 
-    TLV::TLVType dataTlvType;
-    SuccessOrExit(aReader.EnterContainer(dataTlvType));
     switch (aCommandPath.mClusterId)
     {
     {{#chip_server_clusters}}
@@ -117,8 +122,6 @@ void DispatchSingleClusterCommand(const ConcreteCommandPath & aCommandPath, TLV:
         break;
     }
 
-exit:
-    aReader.ExitContainer(dataTlvType);
     Compatibility::ResetEmberAfObjects();
 }