From a14ec9f884d56630e032bb89047c893fdc5717fc Mon Sep 17 00:00:00 2001
From: Boris Zbarsky <bzbarsky@apple.com>
Date: Mon, 13 Jun 2022 14:14:29 -0400
Subject: [PATCH] Make Software Diagnostics not claim to support
 ResetWatermarks if it's not supported.

This modifies the AcceptedCommandList for Software Diagnostics to only
list ResetWatermarks if the command is in fact supported (which makes
it match the feature map).

Also makes the response UNSUPPORTED_COMMAND instead of FAILURE when
it's not supported.
---
 .../software-diagnostics-server.cpp           | 66 +++++++++++++++----
 1 file changed, 54 insertions(+), 12 deletions(-)

diff --git a/src/app/clusters/software-diagnostics-server/software-diagnostics-server.cpp b/src/app/clusters/software-diagnostics-server/software-diagnostics-server.cpp
index ed89554fef6cab..433201422b2e5d 100644
--- a/src/app/clusters/software-diagnostics-server/software-diagnostics-server.cpp
+++ b/src/app/clusters/software-diagnostics-server/software-diagnostics-server.cpp
@@ -22,8 +22,10 @@
 #include <app-common/zap-generated/ids/Clusters.h>
 #include <app/AttributeAccessInterface.h>
 #include <app/CommandHandler.h>
+#include <app/CommandHandlerInterface.h>
 #include <app/ConcreteCommandPath.h>
 #include <app/EventLogging.h>
+#include <app/InteractionModelEngine.h>
 #include <app/util/af.h>
 #include <app/util/attribute-storage.h>
 #include <lib/core/Optional.h>
@@ -52,8 +54,21 @@ class SoftwareDiagosticsAttrAccess : public AttributeAccessInterface
     CHIP_ERROR ReadThreadMetrics(AttributeValueEncoder & aEncoder);
 };
 
+class SoftwareDiagnosticsCommandHandler : public CommandHandlerInterface
+{
+public:
+    // Register for the SoftwareDiagnostics cluster on all endpoints.
+    SoftwareDiagnosticsCommandHandler() : CommandHandlerInterface(Optional<EndpointId>::Missing(), SoftwareDiagnostics::Id) {}
+
+    void InvokeCommand(HandlerContext & handlerContext) override;
+
+    CHIP_ERROR EnumerateAcceptedCommands(const ConcreteClusterPath & cluster, CommandIdCallback callback, void * context) override;
+};
+
 SoftwareDiagosticsAttrAccess gAttrAccess;
 
+SoftwareDiagnosticsCommandHandler gCommandHandler;
+
 CHIP_ERROR SoftwareDiagosticsAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder)
 {
     if (aPath.mClusterId != SoftwareDiagnostics::Id)
@@ -131,6 +146,42 @@ CHIP_ERROR SoftwareDiagosticsAttrAccess::ReadThreadMetrics(AttributeValueEncoder
     return err;
 }
 
+void SoftwareDiagnosticsCommandHandler::InvokeCommand(HandlerContext & handlerContext)
+{
+    using Protocols::InteractionModel::Status;
+    if (handlerContext.mRequestPath.mCommandId != Commands::ResetWatermarks::Id)
+    {
+        // Normal error handling
+        return;
+    }
+
+    handlerContext.SetCommandHandled();
+    Status status = Status::Success;
+    if (!DeviceLayer::GetDiagnosticDataProvider().SupportsWatermarks())
+    {
+        status = Status::UnsupportedCommand;
+    }
+    else if (DeviceLayer::GetDiagnosticDataProvider().ResetWatermarks() != CHIP_NO_ERROR)
+    {
+        status = Status::Failure;
+    }
+    handlerContext.mCommandHandler.AddStatus(handlerContext.mRequestPath, status);
+}
+
+CHIP_ERROR SoftwareDiagnosticsCommandHandler::EnumerateAcceptedCommands(const ConcreteClusterPath & cluster,
+                                                                        CommandIdCallback callback, void * context)
+{
+    if (!DeviceLayer::GetDiagnosticDataProvider().SupportsWatermarks())
+    {
+        // No commmands.
+        return CHIP_NO_ERROR;
+    }
+
+    callback(Commands::ResetWatermarks::Id, context);
+
+    return CHIP_NO_ERROR;
+}
+
 } // anonymous namespace
 
 namespace chip {
@@ -174,21 +225,12 @@ bool emberAfSoftwareDiagnosticsClusterResetWatermarksCallback(app::CommandHandle
                                                               const app::ConcreteCommandPath & commandPath,
                                                               const Commands::ResetWatermarks::DecodableType & commandData)
 {
-    EmberAfStatus status = EMBER_ZCL_STATUS_SUCCESS;
-
-    // If implemented, the server SHALL set the value of the CurrentHeapHighWatermark attribute to the
-    // value of the CurrentHeapUsed.
-    if (DeviceLayer::GetDiagnosticDataProvider().ResetWatermarks() != CHIP_NO_ERROR)
-    {
-        status = EMBER_ZCL_STATUS_FAILURE;
-    }
-
-    emberAfSendImmediateDefaultResponse(status);
-
-    return true;
+    // Shouldn't be called at all.
+    return false;
 }
 
 void MatterSoftwareDiagnosticsPluginServerInitCallback()
 {
     registerAttributeAccessOverride(&gAttrAccess);
+    InteractionModelEngine::GetInstance()->RegisterCommandHandler(&gCommandHandler);
 }