Implement cluster-specific status APIs on WriteHandler.

Also cleans up the implementation of the equivalent CommandHandler APIs a bit. Fixes #9908
project-chip · May 4, 2022 · 60b8bab · 60b8bab
1 parent 0a9e871
commit 60b8bab
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 29 deletions.
diff --git a/src/app/CommandHandler.cpp b/src/app/CommandHandler.cpp
@@ -439,43 +439,32 @@ CHIP_ERROR CommandHandler::ProcessGroupCommandDataIB(CommandDataIB::Parser & aCo
     return CHIP_NO_ERROR;
 }
 
-CHIP_ERROR CommandHandler::AddStatusInternal(const ConcreteCommandPath & aCommandPath,
-                                             const Protocols::InteractionModel::Status aStatus,
-                                             const Optional<ClusterStatus> & aClusterStatus)
+CHIP_ERROR CommandHandler::AddStatusInternal(const ConcreteCommandPath & aCommandPath, const StatusIB & aStatus)
 {
-    StatusIB statusIB;
     ReturnErrorOnFailure(PrepareStatus(aCommandPath));
     CommandStatusIB::Builder & commandStatus = mInvokeResponseBuilder.GetInvokeResponses().GetInvokeResponse().GetStatus();
     StatusIB::Builder & statusIBBuilder      = commandStatus.CreateErrorStatus();
     ReturnErrorOnFailure(commandStatus.GetError());
-    //
-    // TODO: Most of the callers are incorrectly passing SecureChannel as the protocol ID, when in fact, the status code provided
-    // above is always an IM code. Instead of fixing all the callers (which is a fairly sizeable change), we'll embark on fixing
-    // this more completely when we fix #9530.
-    //
-    statusIB.mStatus        = aStatus;
-    statusIB.mClusterStatus = aClusterStatus;
-    statusIBBuilder.EncodeStatusIB(statusIB);
+    statusIBBuilder.EncodeStatusIB(aStatus);
     ReturnErrorOnFailure(statusIBBuilder.GetError());
     return FinishStatus();
 }
 
 CHIP_ERROR CommandHandler::AddStatus(const ConcreteCommandPath & aCommandPath, const Protocols::InteractionModel::Status aStatus)
 {
-    Optional<ClusterStatus> clusterStatus = Optional<ClusterStatus>::Missing();
-    return AddStatusInternal(aCommandPath, aStatus, clusterStatus);
+    return AddStatusInternal(aCommandPath, StatusIB(aStatus));
 }
 
 CHIP_ERROR CommandHandler::AddClusterSpecificSuccess(const ConcreteCommandPath & aCommandPath, ClusterStatus aClusterStatus)
 {
-    Optional<ClusterStatus> clusterStatus(aClusterStatus);
-    return AddStatusInternal(aCommandPath, Protocols::InteractionModel::Status::Success, clusterStatus);
+    using Protocols::InteractionModel::Status;
+    return AddStatusInternal(aCommandPath, StatusIB(Status::Success, aClusterStatus));
 }
 
 CHIP_ERROR CommandHandler::AddClusterSpecificFailure(const ConcreteCommandPath & aCommandPath, ClusterStatus aClusterStatus)
 {
-    Optional<ClusterStatus> clusterStatus(aClusterStatus);
-    return AddStatusInternal(aCommandPath, Protocols::InteractionModel::Status::Failure, clusterStatus);
+    using Protocols::InteractionModel::Status;
+    return AddStatusInternal(aCommandPath, StatusIB(Status::Failure, aClusterStatus));
 }
 
 CHIP_ERROR CommandHandler::PrepareCommand(const ConcreteCommandPath & aCommandPath, bool aStartDataStruct)

diff --git a/src/app/CommandHandler.h b/src/app/CommandHandler.h
@@ -302,8 +302,7 @@ class CommandHandler
      */
     CHIP_ERROR ProcessGroupCommandDataIB(CommandDataIB::Parser & aCommandElement);
     CHIP_ERROR SendCommandResponse();
-    CHIP_ERROR AddStatusInternal(const ConcreteCommandPath & aCommandPath, const Protocols::InteractionModel::Status aStatus,
-                                 const Optional<ClusterStatus> & aClusterStatus);
+    CHIP_ERROR AddStatusInternal(const ConcreteCommandPath & aCommandPath, const StatusIB & aStatus);
 
     /**
      * If this function fails, it may leave our TLV buffer in an inconsistent state.  Callers should snapshot as needed before

diff --git a/src/app/WriteHandler.cpp b/src/app/WriteHandler.cpp
@@ -639,6 +639,18 @@ CHIP_ERROR WriteHandler::AddStatus(const ConcreteDataAttributePath & aPath, cons
     return AddStatus(aPath, StatusIB(aStatus));
 }
 
+CHIP_ERROR WriteHandler::AddClusterSpecificSuccess(const ConcreteDataAttributePath & aPath, ClusterStatus aClusterStatus)
+{
+    using Protocols::InteractionModel::Status;
+    return AddStatus(aPath, StatusIB(Status::Success, aClusterStatus));
+}
+
+CHIP_ERROR WriteHandler::AddClusterSpecificFailure(const ConcreteDataAttributePath & aPath, ClusterStatus aClusterStatus)
+{
+    using Protocols::InteractionModel::Status;
+    return AddStatus(aPath, StatusIB(Status::Failure, aClusterStatus));
+}
+
 CHIP_ERROR WriteHandler::AddStatus(const ConcreteDataAttributePath & aPath, const StatusIB & aStatus)
 {
     AttributeStatusIBs::Builder & writeResponses   = mWriteResponseBuilder.GetWriteResponses();

diff --git a/src/app/WriteHandler.h b/src/app/WriteHandler.h
@@ -82,17 +82,10 @@ class WriteHandler : public Messaging::ExchangeDelegate
     CHIP_ERROR ProcessGroupAttributeDataIBs(TLV::TLVReader & aAttributeDataIBsReader);
 
     CHIP_ERROR AddStatus(const ConcreteDataAttributePath & aPath, const Protocols::InteractionModel::Status aStatus);
-    CHIP_ERROR AddStatus(const ConcreteDataAttributePath & aPath, const StatusIB & aStatus);
 
-    CHIP_ERROR AddClusterSpecificSuccess(const AttributePathParams & aAttributePathParams, uint8_t aClusterStatus)
-    {
-        return CHIP_ERROR_NOT_IMPLEMENTED;
-    }
+    CHIP_ERROR AddClusterSpecificSuccess(const ConcreteDataAttributePath & aAttributePathParams, uint8_t aClusterStatus);
 
-    CHIP_ERROR AddClusterSpecificFailure(const AttributePathParams & aAttributePathParams, uint8_t aClusterStatus)
-    {
-        return CHIP_ERROR_NOT_IMPLEMENTED;
-    }
+    CHIP_ERROR AddClusterSpecificFailure(const ConcreteDataAttributePath & aAttributePathParams, uint8_t aClusterStatus);
 
     FabricIndex GetAccessingFabricIndex() const;
 
@@ -157,6 +150,8 @@ class WriteHandler : public Messaging::ExchangeDelegate
     // ProcessGroupAttributeDataIBs.
     CHIP_ERROR DeliverFinalListWriteEndForGroupWrite(bool writeWasSuccessful);
 
+    CHIP_ERROR AddStatus(const ConcreteDataAttributePath & aPath, const StatusIB & aStatus);
+
 private:
     // ExchangeDelegate
     CHIP_ERROR OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader,