Ember invoke implementation with unit tests inside DataModel

Author: andy31415

src/app/BUILD.gn

@@ -207,7 +207,7 @@ static_library("interaction-model") {
 
   public_deps = [
     ":app_config",
-    ":command-handler",
+    ":command-handler-impl",
     ":constants",
     ":paths",
     ":subscription-info-provider",
@@ -333,16 +333,32 @@ source_set("status-response") {
   ]
 }
 
-source_set("command-handler") {
+source_set("command-handler-interface") {
   sources = [
     "CommandHandler.cpp",
     "CommandHandler.h",
     "CommandHandlerExchangeInterface.h",
+  ]
+
+  public_deps = [
+    ":paths",
+    "${chip_root}/src/access:types",
+    "${chip_root}/src/app/data-model",
+    "${chip_root}/src/lib/core",
+    "${chip_root}/src/lib/support",
+    "${chip_root}/src/messaging",
+    "${chip_root}/src/protocols/interaction_model",
+  ]
+}
+
+source_set("command-handler-impl") {
+  sources = [
     "CommandHandlerImpl.cpp",
     "CommandHandlerImpl.h",
   ]
 
   public_deps = [
+    ":command-handler-interface",
     ":paths",
     ":required-privileges",
     ":status-response",

src/app/codegen-data-model/CodegenDataModel.cpp

@@ -18,6 +18,7 @@
 
 #include <app-common/zap-generated/attribute-type.h>
 #include <app/RequiredPrivilege.h>
+#include <app/util/IMClusterCommandHandler.h>
 #include <app/util/attribute-storage.h>
 #include <app/util/endpoint-config-api.h>
 #include <lib/core/DataModelTypes.h>
@@ -232,10 +233,17 @@ bool CodegenDataModel::EmberCommandListIterator::Exists(const CommandId * list,
 }
 
 CHIP_ERROR CodegenDataModel::Invoke(const InteractionModel::InvokeRequest & request, TLV::TLVReader & input_arguments,
-                                    InteractionModel::InvokeReply & reply)
+                                    CommandHandler * handler)
 {
-    // TODO: this needs an implementation
-    return CHIP_ERROR_NOT_IMPLEMENTED;
+    // TODO: CommandHandlerInterface support is currently
+    //       residing in InteractionModelEngine itself. We may want to separate this out
+    //       into its own registry, similar to attributes, so that IM is decoupled from actual storage of things.
+
+    // Ember dispatching automatically uses `handler` to set an appropriate result or status
+    // This never fails (as handler error is encoded as needed).
+    DispatchSingleClusterCommand(request.path, input_arguments, handler);
+
+    return CHIP_NO_ERROR;
 }
 
 EndpointId CodegenDataModel::FirstEndpoint()

src/app/codegen-data-model/CodegenDataModel.h

@@ -71,7 +71,7 @@ class CodegenDataModel : public chip::app::InteractionModel::DataModel
     CHIP_ERROR ReadAttribute(const InteractionModel::ReadAttributeRequest & request, AttributeValueEncoder & encoder) override;
     CHIP_ERROR WriteAttribute(const InteractionModel::WriteAttributeRequest & request, AttributeValueDecoder & decoder) override;
     CHIP_ERROR Invoke(const InteractionModel::InvokeRequest & request, chip::TLV::TLVReader & input_arguments,
-                      InteractionModel::InvokeReply & reply) override;
+                      CommandHandler * handler) override;
 
     /// attribute tree iteration
     EndpointId FirstEndpoint() override;

src/app/codegen-data-model/tests/BUILD.gn

@@ -24,6 +24,8 @@ source_set("ember_extra_files") {
     "${chip_root}/src/app/util/ember-io-storage.cpp",
     "AttributeReportIBEncodeDecode.cpp",
     "AttributeReportIBEncodeDecode.h",
+    "EmberInvokeOverride.cpp",
+    "EmberInvokeOverride.h",
     "EmberReadWriteOverride.cpp",
     "EmberReadWriteOverride.h",
     "InteractionModelTemporaryOverrides.cpp",

src/app/codegen-data-model/tests/EmberInvokeOverride.cpp

@@ -0,0 +1,55 @@
+/*
+ *    Copyright (c) 2024 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 "EmberInvokeOverride.h"
+
+#include <app/util/IMClusterCommandHandler.h>
+
+namespace {
+
+chip::app::ConcreteCommandPath gLastDispatchPath;
+uint32_t gDispatchCount = 0;
+
+} // namespace
+
+namespace chip {
+namespace Test {
+
+app::ConcreteCommandPath GetLastDispatchPath()
+{
+    return gLastDispatchPath;
+}
+
+uint32_t DispatchCount()
+{
+    return gDispatchCount;
+}
+
+} // namespace Test
+} // namespace chip
+
+namespace chip {
+namespace app {
+
+void DispatchSingleClusterCommand(const ConcreteCommandPath & aRequestCommandPath, chip::TLV::TLVReader & aReader,
+                                  CommandHandler * apCommandObj)
+{
+    gLastDispatchPath = aRequestCommandPath;
+    gDispatchCount++;
+}
+
+} // namespace app
+} // namespace chip

src/app/codegen-data-model/tests/EmberInvokeOverride.h

@@ -0,0 +1,31 @@
+/*
+ *    Copyright (c) 2024 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.
+ */
+#pragma once
+
+#include <app/ConcreteCommandPath.h>
+
+namespace chip {
+namespace Test {
+
+/// what was the last path on which DispatchSingleClusterCommand was called
+app::ConcreteCommandPath GetLastDispatchPath();
+
+/// How many times was DispatchSingleClusterCommand called
+uint32_t DispatchCount();
+
+} // namespace Test
+} // namespace chip

src/app/codegen-data-model/tests/InteractionModelTemporaryOverrides.cpp

@@ -74,12 +74,6 @@ CHIP_ERROR ReadSingleClusterData(const Access::SubjectDescriptor & aSubjectDescr
     return CHIP_ERROR_NOT_IMPLEMENTED;
 }
 
-void DispatchSingleClusterCommand(const ConcreteCommandPath & aRequestCommandPath, chip::TLV::TLVReader & aReader,
-                                  CommandHandler * apCommandObj)
-{
-    // TODO: total hardcoded noop
-}
-
 } // namespace app
 } // namespace chip
 

src/app/codegen-data-model/tests/TestCodegenModelViaMocks.cpp

@@ -14,9 +14,11 @@
  *    See the License for the specific language governing permissions and
  *    limitations under the License.
  */
+#include "app/ConcreteCommandPath.h"
 #include <app/codegen-data-model/CodegenDataModel.h>
 
 #include <app/codegen-data-model/tests/AttributeReportIBEncodeDecode.h>
+#include <app/codegen-data-model/tests/EmberInvokeOverride.h>
 #include <app/codegen-data-model/tests/EmberReadWriteOverride.h>
 
 #include <access/AccessControl.h>
@@ -69,6 +71,9 @@ constexpr NodeId kTestNodeId           = 0xFFFF'1234'ABCD'4321;
 constexpr AttributeId kAttributeIdReadOnly   = 0x3001;
 constexpr AttributeId kAttributeIdTimedWrite = 0x3002;
 
+constexpr CommandId kMockCommandId1 = 0x1234;
+constexpr CommandId kMockCommandId2 = 0x1122;
+
 constexpr EndpointId kEndpointIdThatIsMissing = kMockEndpointMin - 1;
 
 constexpr AttributeId kReadOnlyAttributeId = 0x5001;
@@ -2430,6 +2435,41 @@ TEST(TestCodegenModelViaMocks, EmberWriteAttributeAccessInterfaceTest)
     TestEmberScalarNullWrite<int64_t, ZCL_INT64S_ATTRIBUTE_TYPE>();
 }
 
+TEST(TestCodegenModelViaMocks, EmberInvokeTest)
+{
+    // Ember invoke is extrealy hard-coded, so generally we just need to validate that
+    // correct paths are invoked.
+
+    UseMockNodeConfig config(gTestNodeConfig);
+    chip::app::CodegenDataModel model;
+
+    {
+        const ConcreteCommandPath kCommandPath(kMockEndpoint1, MockClusterId(1), kMockCommandId1);
+        const InvokeRequest kInvokeRequest{ .path = kCommandPath };
+        chip::TLV::TLVReader tlvReader;
+
+        const uint32_t kDispatchCountPre = chip::Test::DispatchCount();
+
+        ASSERT_EQ(model.Invoke(kInvokeRequest, tlvReader, nullptr /* handler, NOT used by impl*/), CHIP_NO_ERROR);
+
+        EXPECT_EQ(chip::Test::DispatchCount(), kDispatchCountPre + 1); // single dispatch
+        EXPECT_EQ(chip::Test::GetLastDispatchPath(), kCommandPath);    // for the right path
+    }
+
+    {
+        const ConcreteCommandPath kCommandPath(kMockEndpoint1, MockClusterId(1), kMockCommandId2);
+        const InvokeRequest kInvokeRequest{ .path = kCommandPath };
+        chip::TLV::TLVReader tlvReader;
+
+        const uint32_t kDispatchCountPre = chip::Test::DispatchCount();
+
+        ASSERT_EQ(model.Invoke(kInvokeRequest, tlvReader, nullptr /* handler, NOT used by impl*/), CHIP_NO_ERROR);
+
+        EXPECT_EQ(chip::Test::DispatchCount(), kDispatchCountPre + 1); // single dispatch
+        EXPECT_EQ(chip::Test::GetLastDispatchPath(), kCommandPath);    // for the right path
+    }
+}
+
 TEST(TestCodegenModelViaMocks, EmberWriteAttributeAccessInterfaceReturningError)
 {
     UseMockNodeConfig config(gTestNodeConfig);

src/app/data-model-interface/BUILD.gn

@@ -20,7 +20,6 @@ source_set("data-model-interface") {
     "DataModel.h",
     "DataModelChangeListener.h",
     "EventsGenerator.h",
-    "InvokeResponder.h",
     "MetadataTypes.cpp",
     "MetadataTypes.h",
     "OperationTypes.h",
@@ -29,6 +28,7 @@ source_set("data-model-interface") {
   public_deps = [
     "${chip_root}/src/access:types",
     "${chip_root}/src/app:attribute-access",
+    "${chip_root}/src/app:command-handler-interface",
     "${chip_root}/src/app:events",
     "${chip_root}/src/app:paths",
     "${chip_root}/src/app/MessageDef",

src/app/data-model-interface/DataModel.h

@@ -21,9 +21,9 @@
 
 #include <app/AttributeValueDecoder.h>
 #include <app/AttributeValueEncoder.h>
+#include <app/CommandHandler.h>
 
 #include <app/data-model-interface/Context.h>
-#include <app/data-model-interface/InvokeResponder.h>
 #include <app/data-model-interface/MetadataTypes.h>
 #include <app/data-model-interface/OperationTypes.h>
 
@@ -99,25 +99,9 @@ class DataModel : public DataModelMetadataTree
     ///         - `NeedsTimedInteraction` for writes that are not timed however are required to be so
     virtual CHIP_ERROR WriteAttribute(const WriteAttributeRequest & request, AttributeValueDecoder & decoder) = 0;
 
-    /// `reply` is used to send back the reply.
-    ///    - calling Reply() or ReplyAsync() will let the application control the reply
-    ///    - returning a CHIP_NO_ERROR without reply/reply_async implies a Status::Success reply without data
+    /// `handler` is used to send back the reply.
     ///    - returning a value other than CHIP_NO_ERROR implies an error reply (error and data are mutually exclusive)
-    ///
-    /// See InvokeReply/AutoCompleteInvokeResponder for details on how to send back replies and expected
-    /// error handling. If you need to know weather a response was successfully sent, use the underlying
-    /// `reply` object instead of returning an error code from Invoke.
-    ///
-    /// Return codes
-    ///   CHIP_IM_GLOBAL_STATUS(code):
-    ///       - error codes that are translatable to specific IM codes
-    ///       - in particular, the following codes are interesting/expected
-    ///         - `UnsupportedEndpoint` for invalid endpoint
-    ///         - `UnsupportedCluster` for no such cluster on the endpoint
-    ///         - `UnsupportedCommand` for no such command in the cluster
-    ///         - `UnsupportedAccess` for permission errors (ACL or fabric scoped with invalid fabric)
-    ///         - `NeedsTimedInteraction` if the invoke requires timed interaction support
-    virtual CHIP_ERROR Invoke(const InvokeRequest & request, chip::TLV::TLVReader & input_arguments, InvokeReply & reply) = 0;
+    virtual CHIP_ERROR Invoke(const InvokeRequest & request, chip::TLV::TLVReader & input_arguments, CommandHandler * handler) = 0;
 
 private:
     InteractionModelContext mContext = { nullptr };