Extract common code of deserialize procedure

src/app/tests/TestCommandInteraction.cpp

@@ -22,6 +22,8 @@
  *
  */
 
+#include <cinttypes>
+
 #include <app/InteractionModelEngine.h>
 #include <core/CHIPCore.h>
 #include <core/CHIPTLV.h>
@@ -33,6 +35,7 @@
 #include <platform/CHIPDeviceLayer.h>
 #include <support/ErrorStr.h>
 #include <support/UnitTestRegistration.h>
+#include <support/logging/CHIPLogging.h>
 #include <system/SystemPacketBuffer.h>
 #include <system/TLVPacketBufferBackingStore.h>
 #include <transport/PASESession.h>
@@ -49,6 +52,14 @@ static TransportMgr<Transport::UDP> gTransportManager;
 static Transport::AdminId gAdminId = 0;
 
 namespace app {
+
+void DispatchSingleClusterCommand(chip::ClusterId aClusterId, chip::CommandId aCommandId, chip::EndpointId aEndPointId,
+                                  chip::TLV::TLVReader & aReader, Command * apCommandObj)
+{
+    ChipLogDetail(Controller, "Received Cluster Command: Cluster=%" PRIx16 " Command=%" PRIx8 " Endpoint=%" PRIx8, aClusterId,
+                  aCommandId, aEndPointId);
+}
+
 class TestCommandInteraction
 {
 public:

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

@@ -1,9 +1,5 @@
 {{#if (zcl_command_arguments_count this.id)}}
-// We are using TLVUnpackError and TLVError here since both of them can be CHIP_END_OF_TLV
-// When TLVError is CHIP_END_OF_TLV, it means we have iterated all of the items, which is not a real error.
-// Any error value TLVUnpackError means we have received an illegal value.
-CHIP_ERROR TLVError = CHIP_NO_ERROR;
-CHIP_ERROR TLVUnpackError = CHIP_NO_ERROR;
+expectArgumentCount = {{ zcl_command_arguments_count this.id }};
 {{#zcl_command_arguments}}
 {{#if (isOctetString type)}}
 chip::ByteSpan {{asSymbol label}};
@@ -12,22 +8,38 @@ const uint8_t * {{asSymbol label}};
 {{else}}
 {{asUnderlyingZclType type}} {{asSymbol label}};
 {{/if}}
-bool {{asSymbol label}}Exists = false;
 {{/zcl_command_arguments}}
-uint32_t validArgumentCount = 0;
+bool argExists[{{zcl_command_arguments_count this.id}}];
+
+memset(argExists, 0, sizeof argExists);
 
 while ((TLVError = aDataTlv.Next()) == CHIP_NO_ERROR)
 {
-  switch (TLV::TagNumFromTag(aDataTlv.GetTag()))
+  // Since call to aDataTlv.Next() is CHIP_NO_ERROR, the read head always points to an element.
+  // Skip this element if it is not a ContextTag, not consider it as an error if other values are valid.
+  if (!TLV::IsContextTag(aDataTlv.GetTag()))
+  {
+    continue;
+  }
+  currentDecodeTagId = TLV::TagNumFromTag(aDataTlv.GetTag());
+  if (currentDecodeTagId < {{zcl_command_arguments_count this.id}})
+  {
+    if (argExists[currentDecodeTagId])
+    {
+      ChipLogProgress(Zcl, "Duplicate TLV tag %" PRIx32, TLV::TagNumFromTag(aDataTlv.GetTag()));
+      TLVUnpackError = CHIP_ERROR_IM_MALFORMED_COMMAND_DATA_ELEMENT;
+      break;
+    }
+    else
+    {
+      argExists[currentDecodeTagId] = true;
+      validArgumentCount++;
+    }
+  }
+  switch (currentDecodeTagId)
   {
 {{#zcl_command_arguments}}
     case {{index}}:
-      if ({{asSymbol label}}Exists)
-      {
-            ChipLogProgress(Zcl, "Duplicate TLV tag %" PRIx32, TLV::TagNumFromTag(aDataTlv.GetTag()));
-            TLVUnpackError = CHIP_ERROR_IM_MALFORMED_COMMAND_DATA_ELEMENT;
-            break;
-      }
 {{#if (isOctetString type)}}
     {
         const uint8_t * data = nullptr;
@@ -43,49 +55,33 @@ while ((TLVError = aDataTlv.Next()) == CHIP_NO_ERROR)
 {{else}}
       TLVUnpackError = aDataTlv.Get({{asSymbol label}});
 {{/if}}
-      if (CHIP_NO_ERROR == TLVUnpackError)
-      {
-            {{asSymbol label}}Exists = true;
-            validArgumentCount++;
-      }
       break;
 {{/zcl_command_arguments}}
     default:
       // Unsupported tag, ignore it.
       ChipLogProgress(Zcl, "Unknown TLV tag during processing.");
       break;
   }
-  if (TLVUnpackError != CHIP_NO_ERROR)
+  if (CHIP_NO_ERROR != TLVUnpackError)
   {
-    ChipLogProgress(Zcl, "Failed to decode TLV data with tag %" PRIx32 ": %" PRId32, TLV::TagNumFromTag(aDataTlv.GetTag()), TLVUnpackError);
     break;
   }
 }
 
 if (CHIP_END_OF_TLV == TLVError)
 {
-      // CHIP_END_OF_TLV means we have iterated all items in the structure, which is not a real error.
-      TLVError = CHIP_NO_ERROR;
-}
-else
-{
-      ChipLogProgress(Zcl, "Failed to decode TLV data: %" PRId32, TLVError);
+  // CHIP_END_OF_TLV means we have iterated all items in the structure, which is not a real error.
+  TLVError = CHIP_NO_ERROR;
 }
 {{/if}}
 
 {{#if (zcl_command_arguments_count this.id)}}
-// TODO(#5590) We should encode a response of status code for invalid TLV.
 if (CHIP_NO_ERROR == TLVError && CHIP_NO_ERROR == TLVUnpackError && {{zcl_command_arguments_count this.id}} == validArgumentCount)
 {
 {{/if}}
 // TODO(#5098) We should pass the Command Object and EndpointId to the cluster callbacks.
 emberAf{{asCamelCased parent.name false}}Cluster{{asCamelCased name false}}Callback({{#zcl_command_arguments}}{{#if (isCharString type)}}const_cast<uint8_t*>({{asSymbol label}}){{else}}{{asSymbol label}}{{/if}}{{#unless (isLastElement index count)}}, {{/unless}}{{/zcl_command_arguments}});
+return;
 {{#if (zcl_command_arguments_count this.id)}}
 }
-else
-{
-      apCommandObj->AddStatusCode(nullptr, Protocols::SecureChannel::GeneralStatusCode::kBadRequest, Protocols::SecureChannel::Id,
-                      Protocols::SecureChannel::kProtocolCodeGeneralFailure);
-      ChipLogProgress(Zcl, "Failed to dispatch command, %d/%" PRIu32 " arguments parsed, TLVError=%" PRIu32 ", UnpackError=%" PRIu32, {{zcl_command_arguments_count this.id}}, validArgumentCount, TLVError, TLVUnpackError);
-}
 {{/if}}

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

@@ -28,6 +28,15 @@ namespace {{asCamelCased name false}} {
 
 void Dispatch{{asCamelCased side false}}Command(app::Command * apCommandObj, CommandId aCommandId, EndpointId aEndpointId, TLV::TLVReader & aDataTlv)
 {
+    // We are using TLVUnpackError and TLVError here since both of them can be CHIP_END_OF_TLV
+    // When TLVError is CHIP_END_OF_TLV, it means we have iterated all of the items, which is not a real error.
+    // 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;
+    CHIP_ERROR TLVUnpackError = CHIP_NO_ERROR;
+    uint32_t validArgumentCount = 0;
+    uint32_t expectArgumentCount = 0;
+    uint32_t currentDecodeTagId = 0;
     {{#if (user_cluster_has_enabled_manufacturer_command name side)}}
     {{else}}
     {{/if}}
@@ -51,10 +60,17 @@ void Dispatch{{asCamelCased side false}}Command(app::Command * apCommandObj, Com
             apCommandObj->AddStatusCode(nullptr, Protocols::SecureChannel::GeneralStatusCode::kNotFound, Protocols::SecureChannel::Id,
                             Protocols::SecureChannel::kProtocolCodeGeneralFailure);
             ChipLogError(Zcl, "Unknown command %" PRIx16 " for cluster %" PRIx16, aCommandId, ZCL_{{asDelimitedMacro define}}_ID);
-            break;
+            return;
         }
         }
     }
+
+    if (CHIP_NO_ERROR != TLVError || CHIP_NO_ERROR != TLVUnpackError || expectArgumentCount == validArgumentCount)
+    {
+      apCommandObj->AddStatusCode(nullptr, Protocols::SecureChannel::GeneralStatusCode::kBadRequest, Protocols::SecureChannel::Id,
+                      Protocols::SecureChannel::kProtocolCodeGeneralFailure);
+      ChipLogProgress(Zcl, "Failed to dispatch command, %d/%" PRIu32 " arguments parsed, TLVError=%" PRIu32 ", UnpackError=%" PRIu32 " (last decoded tag = %" PRIu32, {{zcl_command_arguments_count this.id}}, validArgumentCount, TLVError, TLVUnpackError, currentDecodeTagId);
+    }
 }
 
 }

src/controller/java/AndroidCommandHandler.cpp

@@ -0,0 +1,38 @@
+/*
+ *   Copyright (c) 2020 Project CHIP Authors
+ *   All rights reserved.
+ *
+ *   Licensed under the Apache License, Version 2.0 (the "License");
+ *   you may not use this file except in compliance with the License.
+ *   You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *   Unless required by applicable law or agreed to in writing, software
+ *   distributed under the License is distributed on an "AS IS" BASIS,
+ *   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *   See the License for the specific language governing permissions and
+ *   limitations under the License.
+ */
+
+#include <cinttypes>
+
+#include <app/Command.h>
+#include <app/InteractionModelDelegate.h>
+#include <support/logging/CHIPLogging.h>
+
+namespace chip {
+namespace app {
+
+void DispatchSingleClusterCommand(chip::ClusterId aClusterId, chip::CommandId aCommandId, chip::EndpointId aEndPointId,
+                                  chip::TLV::TLVReader & aReader, Command * apCommandObj)
+{
+    ChipLogDetail(Controller, "Received Cluster Command: Cluster=%" PRIx16 " Command=%" PRIx8 " Endpoint=%" PRIx8, aClusterId,
+                  aCommandId, aEndPointId);
+    ChipLogError(
+        Controller,
+        "Default DispatchSingleClusterCommand is called, this should be replaced by actual dispatched for cluster commands");
+}
+
+} // namespace app
+} // namespace chip

src/controller/java/BUILD.gn

@@ -28,6 +28,7 @@ shared_library("jni") {
     "AndroidBleConnectionDelegate.h",
     "AndroidBlePlatformDelegate.cpp",
     "AndroidBlePlatformDelegate.h",
+    "AndroidCommandHandler.cpp",
     "AndroidDeviceControllerWrapper.cpp",
     "AndroidDeviceControllerWrapper.h",
     "AndroidKeyValueStoreManagerImpl.cpp",