[im] Return error when some cluster is not enabled on endpoint

The current code may return wrong success or unexpected error code when one cluster is not enabled on endpoint, this commit added a function thay will query cluster from ember cluster catalog, the function will be replaced by cluster catalog.
project-chip · May 21, 2021 · ad7b3d9 · ad7b3d9
1 parent 82a8544
commit ad7b3d9
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 8 deletions.
diff --git a/src/app/CommandHandler.cpp b/src/app/CommandHandler.cpp
@@ -90,6 +90,8 @@ CHIP_ERROR CommandHandler::ProcessCommandDataElement(CommandDataElement::Parser
     err = commandPath.GetEndpointId(&endpointId);
     SuccessOrExit(err);
 
+    SuccessOrExit(err = CheckIfClusterCommandExists(clusterId, commandId, endpointId));
+
     err = aCommandElement.GetData(&commandDataReader);
     if (CHIP_END_OF_TLV == err)
     {
@@ -108,10 +110,15 @@ CHIP_ERROR CommandHandler::ProcessCommandDataElement(CommandDataElement::Parser
 exit:
     if (err != CHIP_NO_ERROR)
     {
+        chip::app::CommandPathParams returnStatusParam = { endpointId,
+                                                           0, // GroupId
+                                                           clusterId, commandId, (chip::app::CommandPathFlags::kEndpointIdValid) };
+
         // The Path is not present when there is an error to be conveyed back. ResponseCommandElement would only have status code,
         // set the error with CHIP_NO_ERROR, then continue to process rest of commands
-        AddStatusCode(nullptr, GeneralStatusCode::kInvalidArgument, Protocols::SecureChannel::Id,
-                      Protocols::SecureChannel::kProtocolCodeGeneralFailure);
+        AddStatusCode(&returnStatusParam,
+                      err == CHIP_ERROR_INVALID_PROFILE_ID ? GeneralStatusCode::kNotFound : GeneralStatusCode::kInvalidArgument,
+                      Protocols::SecureChannel::Id, Protocols::SecureChannel::kProtocolCodeGeneralFailure);
         err = CHIP_NO_ERROR;
     }
     return err;

diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h
@@ -164,6 +164,7 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate
 
 void DispatchSingleClusterCommand(chip::ClusterId aClusterId, chip::CommandId aCommandId, chip::EndpointId aEndPointId,
                                   chip::TLV::TLVReader & aReader, Command * apCommandObj);
+CHIP_ERROR CheckIfClusterCommandExists(chip::ClusterId aClusterId, chip::CommandId aCommandId, chip::EndpointId aEndPointId);
 CHIP_ERROR ReadSingleClusterData(ClusterInfo & aClusterInfo, TLV::TLVWriter & aWriter);
 CHIP_ERROR WriteSingleClusterData(ClusterInfo & aClusterInfo, TLV::TLVReader & aReader);
 } // namespace app

diff --git a/src/app/tests/TestCommandInteraction.cpp b/src/app/tests/TestCommandInteraction.cpp
@@ -62,6 +62,12 @@ void DispatchSingleClusterCommand(chip::ClusterId aClusterId, chip::CommandId aC
                   aCommandId, aEndPointId);
 }
 
+CHIP_ERROR CheckIfClusterCommandExists(chip::ClusterId aClusterId, chip::CommandId aCommandId, chip::EndpointId aEndPointId)
+{
+    // Always return no error in test.
+    return CHIP_NO_ERROR;
+}
+
 class TestCommandInteraction
 {
 public:

diff --git a/src/app/tests/integration/chip_im_initiator.cpp b/src/app/tests/integration/chip_im_initiator.cpp
@@ -255,6 +255,12 @@ class MockInteractionModelApp : public chip::app::InteractionModelDelegate
 namespace chip {
 namespace app {
 
+CHIP_ERROR CheckIfClusterCommandExists(chip::ClusterId aClusterId, chip::CommandId aCommandId, chip::EndpointId aEndPointId)
+{
+    // Always return no error in test.
+    return CHIP_NO_ERROR;
+}
+
 void DispatchSingleClusterCommand(chip::ClusterId aClusterId, chip::CommandId aCommandId, chip::EndpointId aEndPointId,
                                   chip::TLV::TLVReader & aReader, Command * apCommandObj)
 {

diff --git a/src/app/tests/integration/chip_im_responder.cpp b/src/app/tests/integration/chip_im_responder.cpp
@@ -44,6 +44,13 @@
 
 namespace chip {
 namespace app {
+
+CHIP_ERROR CheckIfClusterCommandExists(chip::ClusterId aClusterId, chip::CommandId aCommandId, chip::EndpointId aEndPointId)
+{
+    // Always return no error in test.
+    return CHIP_NO_ERROR;
+}
+
 void DispatchSingleClusterCommand(chip::ClusterId aClusterId, chip::CommandId aCommandId, chip::EndpointId aEndPointId,
                                   chip::TLV::TLVReader & aReader, Command * apCommandObj)
 {

diff --git a/src/app/util/ember-compatibility-functions.cpp b/src/app/util/ember-compatibility-functions.cpp
@@ -21,9 +21,9 @@
  *          when calling ember callbacks.
  */
 
-#include <app/util/ember-compatibility-functions.h>
-
 #include <app/Command.h>
+#include <app/InteractionModelEngine.h>
+#include <app/util/ember-compatibility-functions.h>
 #include <app/util/util.h>
 #include <lib/core/CHIPCore.h>
 #include <lib/core/CHIPTLV.h>
@@ -65,15 +65,17 @@ bool IMEmberAfSendDefaultResponseWithCallback(EmberAfStatus status)
         return false;
     }
 
-    chip::app::CommandPathParams returnStatusParam = { imCompatibilityEmberApsFrame.sourceEndpoint,
+    chip::app::CommandPathParams returnStatusParam = { imCompatibilityEmberApsFrame.destinationEndpoint,
                                                        0, // GroupId
                                                        imCompatibilityEmberApsFrame.clusterId,
                                                        imCompatibilityEmberAfCluster.commandId,
                                                        (chip::app::CommandPathFlags::kEndpointIdValid) };
 
-    CHIP_ERROR err =
-        currentCommandObject->AddStatusCode(&returnStatusParam, chip::Protocols::SecureChannel::GeneralStatusCode::kSuccess,
-                                            chip::Protocols::InteractionModel::Id, status);
+    CHIP_ERROR err = currentCommandObject->AddStatusCode(&returnStatusParam,
+                                                         status == EMBER_ZCL_STATUS_SUCCESS
+                                                             ? chip::Protocols::SecureChannel::GeneralStatusCode::kSuccess
+                                                             : chip::Protocols::SecureChannel::GeneralStatusCode::kFailure,
+                                                         chip::Protocols::InteractionModel::Id, status);
     return CHIP_NO_ERROR == err;
 }
 
@@ -84,5 +86,13 @@ void ResetEmberAfObjects()
 }
 
 } // namespace Compatibility
+
+CHIP_ERROR CheckIfClusterCommandExists(chip::ClusterId aClusterId, chip::CommandId aCommandId, chip::EndpointId aEndPointId)
+{
+    // TODO: Currently, we are using cluster catalog from the ember library, this should be modified or replaced after several
+    // updates to Commands.
+    return emberAfContainsServer(aEndPointId, aClusterId) ? CHIP_NO_ERROR : CHIP_ERROR_INVALID_PROFILE_ID;
+}
+
 } // namespace app
 } // namespace chip