Add initial cluster status support (#10732)

src/app/Command.h

@@ -83,12 +83,12 @@ class Command
         return CHIP_ERROR_NOT_IMPLEMENTED;
     }
 
-    virtual CHIP_ERROR AddClusterSpecificSuccess(ConcreteCommandPath & aCommandPath, uint8_t aClusterStatus)
+    virtual CHIP_ERROR AddClusterSpecificSuccess(const ConcreteCommandPath & aCommandPath, ClusterStatus aClusterStatus)
     {
         return CHIP_ERROR_NOT_IMPLEMENTED;
     }
 
-    virtual CHIP_ERROR AddClusterSpecificFailure(ConcreteCommandPath & aCommandPath, uint8_t aClusterStatus)
+    virtual CHIP_ERROR AddClusterSpecificFailure(const ConcreteCommandPath & aCommandPath, ClusterStatus aClusterStatus)
     {
         return CHIP_ERROR_NOT_IMPLEMENTED;
     }

src/app/CommandHandler.cpp

@@ -153,7 +153,9 @@ CHIP_ERROR CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommand
     return CHIP_NO_ERROR;
 }
 
-CHIP_ERROR CommandHandler::AddStatus(const ConcreteCommandPath & aCommandPath, const Protocols::InteractionModel::Status aStatus)
+CHIP_ERROR CommandHandler::AddStatusInternal(const ConcreteCommandPath & aCommandPath,
+                                             const Protocols::InteractionModel::Status aStatus,
+                                             const Optional<ClusterStatus> & aClusterStatus)
 {
     CHIP_ERROR err = CHIP_NO_ERROR;
     StatusIB::Builder statusIBBuilder;
@@ -174,7 +176,8 @@ CHIP_ERROR CommandHandler::AddStatus(const ConcreteCommandPath & aCommandPath, c
     // 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.mStatus        = aStatus;
+    statusIB.mClusterStatus = aClusterStatus;
     statusIBBuilder.EncodeStatusIB(statusIB);
     err = statusIBBuilder.GetError();
     SuccessOrExit(err);
@@ -185,6 +188,24 @@ CHIP_ERROR CommandHandler::AddStatus(const ConcreteCommandPath & aCommandPath, c
     return err;
 }
 
+CHIP_ERROR CommandHandler::AddStatus(const ConcreteCommandPath & aCommandPath, const Protocols::InteractionModel::Status aStatus)
+{
+    Optional<ClusterStatus> clusterStatus = Optional<ClusterStatus>::Missing();
+    return AddStatusInternal(aCommandPath, aStatus, clusterStatus);
+}
+
+CHIP_ERROR CommandHandler::AddClusterSpecificSuccess(const ConcreteCommandPath & aCommandPath, ClusterStatus aClusterStatus)
+{
+    Optional<ClusterStatus> clusterStatus(aClusterStatus);
+    return AddStatusInternal(aCommandPath, Protocols::InteractionModel::Status::Success, clusterStatus);
+}
+
+CHIP_ERROR CommandHandler::AddClusterSpecificFailure(const ConcreteCommandPath & aCommandPath, ClusterStatus aClusterStatus)
+{
+    Optional<ClusterStatus> clusterStatus(aClusterStatus);
+    return AddStatusInternal(aCommandPath, Protocols::InteractionModel::Status::Failure, clusterStatus);
+}
+
 CHIP_ERROR CommandHandler::PrepareResponse(const ConcreteCommandPath & aRequestCommandPath, CommandId aResponseCommand)
 {
     CommandPathParams params = { aRequestCommandPath.mEndpointId,

src/app/CommandHandler.h

@@ -74,6 +74,10 @@ class CommandHandler : public Command
                                       System::PacketBufferHandle && payload);
     CHIP_ERROR AddStatus(const ConcreteCommandPath & aCommandPath, const Protocols::InteractionModel::Status aStatus) override;
 
+    CHIP_ERROR AddClusterSpecificSuccess(const ConcreteCommandPath & aCommandPath, ClusterStatus aClusterStatus) override;
+
+    CHIP_ERROR AddClusterSpecificFailure(const ConcreteCommandPath & aCommandPath, ClusterStatus aClusterStatus) override;
+
     /**
      * API for adding a data response.  The template parameter T is generally
      * expected to be a ClusterName::Commands::CommandName::Type struct, but any
@@ -107,6 +111,9 @@ class CommandHandler : public Command
     CHIP_ERROR SendCommandResponse();
     CHIP_ERROR ProcessCommandDataIB(CommandDataIB::Parser & aCommandElement) override;
     CHIP_ERROR PrepareResponse(const ConcreteCommandPath & aRequestCommandPath, CommandId aResponseCommand);
+    CHIP_ERROR AddStatusInternal(const ConcreteCommandPath & aCommandPath, const Protocols::InteractionModel::Status aStatus,
+                                 const Optional<ClusterStatus> & aClusterStatus);
+
     Callback * mpCallback = nullptr;
 };
 } // namespace app

src/app/CommandSender.cpp

@@ -79,7 +79,9 @@ CHIP_ERROR CommandSender::OnMessageReceived(Messaging::ExchangeContext * apExcha
     {
         if (err != CHIP_NO_ERROR)
         {
-            mpCallback->OnError(this, Protocols::InteractionModel::Status::Failure, err);
+            StatusIB status;
+            status.mStatus = Protocols::InteractionModel::Status::Failure;
+            mpCallback->OnError(this, status, err);
         }
     }
 
@@ -95,7 +97,9 @@ void CommandSender::OnResponseTimeout(Messaging::ExchangeContext * apExchangeCon
 
     if (mpCallback != nullptr)
     {
-        mpCallback->OnError(this, Protocols::InteractionModel::Status::Failure, CHIP_ERROR_TIMEOUT);
+        StatusIB status;
+        status.mStatus = Protocols::InteractionModel::Status::Failure;
+        mpCallback->OnError(this, status, CHIP_ERROR_TIMEOUT);
     }
 
     Close();
@@ -119,6 +123,8 @@ CHIP_ERROR CommandSender::ProcessCommandDataIB(CommandDataIB::Parser & aCommandE
     chip::ClusterId clusterId;
     chip::CommandId commandId;
     chip::EndpointId endpointId;
+    // Default to success when an invoke response is received.
+    StatusIB statusIB;
 
     {
         CommandPathIB::Parser commandPath;
@@ -140,8 +146,6 @@ CHIP_ERROR CommandSender::ProcessCommandDataIB(CommandDataIB::Parser & aCommandE
         bool hasDataResponse = false;
         chip::TLV::TLVReader commandDataReader;
 
-        // Default to success when an invoke response is received.
-        StatusIB statusIB;
         StatusIB::Parser statusIBParser;
         err = aCommandElement.GetStatusIB(&statusIBParser);
         if (CHIP_NO_ERROR == err)
@@ -182,12 +186,12 @@ CHIP_ERROR CommandSender::ProcessCommandDataIB(CommandDataIB::Parser & aCommandE
         {
             if (statusIB.mStatus == Protocols::InteractionModel::Status::Success)
             {
-                mpCallback->OnResponse(this, ConcreteCommandPath(endpointId, clusterId, commandId),
+                mpCallback->OnResponse(this, ConcreteCommandPath(endpointId, clusterId, commandId), statusIB,
                                        hasDataResponse ? &commandDataReader : nullptr);
             }
             else
             {
-                mpCallback->OnError(this, statusIB.mStatus, CHIP_ERROR_IM_STATUS_CODE_RECEIVED);
+                mpCallback->OnError(this, statusIB, CHIP_ERROR_IM_STATUS_CODE_RECEIVED);
             }
         }
     }

src/app/CommandSender.h

@@ -64,11 +64,16 @@ class CommandSender final : public Command, public Messaging::ExchangeDelegate
          * The CommandSender object MUST continue to exist after this call is completed. The application shall wait until it
          * receives an OnDone call to destroy the object.
          *
-         * @param[in] apCommandSender The command sender object that initiated the command transaction.
-         * @param[in] aPath           The command path field in invoke command response.
-         * @param[in] aData           The command data, will be nullptr if the server returns a StatusIB.
+         * @param[in] apCommandSender: The command sender object that initiated the command transaction.
+         * @param[in] aPath: The command path field in invoke command response.
+         * @param[in] aStatusIB: It will always have a success status. If apData is null, it can be any success status, including
+         *                       possibly a cluster-specific one.   If apData is not null it aStatusIB will always be a generic
+         * SUCCESS status with no-cluster specific information.
+         * @param[in] aData: The command data, will be nullptr if the server returns a StatusIB.
          */
-        virtual void OnResponse(CommandSender * apCommandSender, const ConcreteCommandPath & aPath, TLV::TLVReader * aData) {}
+        virtual void OnResponse(CommandSender * apCommandSender, const ConcreteCommandPath & aPath, const StatusIB & aStatusIB,
+                                TLV::TLVReader * apData)
+        {}
 
         /**
          * OnError will be called when an error occurr *after* a successful call to SendCommandRequest(). The following
@@ -84,14 +89,11 @@ class CommandSender final : public Command, public Messaging::ExchangeDelegate
          * The CommandSender object MUST continue to exist after this call is completed. The application shall wait until it
          * receives an OnDone call to destroy and free the object.
          *
-         * @param[in] apCommandSender         The command sender object that initiated the command transaction.
-         * @param[in] aInteractionModelStatus Contains an IM status code. This SHALL never be IM::Success, and will contain a valid
-         *                                    server-side emitted error if aProtocolError == CHIP_ERROR_IM_STATUS_CODE_RECEIVED.
-         * @param[in] aError                  A system error code that conveys the overall error code.
+         * @param[in] apCommandSender: The command sender object that initiated the command transaction.
+         * @param[in] aStatusIB: The status code including IM status code and optional cluster status code
+         * @param[in] aError: A system error code that conveys the overall error code.
          */
-        virtual void OnError(const CommandSender * apCommandSender, Protocols::InteractionModel::Status aInteractionModelStatus,
-                             CHIP_ERROR aError)
-        {}
+        virtual void OnError(const CommandSender * apCommandSender, const StatusIB & aStatusIB, CHIP_ERROR aError) {}
 
         /**
          * OnDone will be called when CommandSender has finished all work and is safe to destory and free the

src/app/tests/TestCommandInteraction.cpp

@@ -116,18 +116,18 @@ class MockCommandSenderCallback : public CommandSender::Callback
 {
 public:
     void OnResponse(chip::app::CommandSender * apCommandSender, const chip::app::ConcreteCommandPath & aPath,
-                    chip::TLV::TLVReader * aData) override
+                    const chip::app::StatusIB & aStatus, chip::TLV::TLVReader * aData) override
     {
         IgnoreUnusedVariable(apCommandSender);
         IgnoreUnusedVariable(aData);
         ChipLogDetail(Controller, "Received Cluster Command: Cluster=%" PRIx32 " Command=%" PRIx32 " Endpoint=%" PRIx16,
                       aPath.mClusterId, aPath.mCommandId, aPath.mEndpointId);
         onResponseCalledTimes++;
     }
-    void OnError(const chip::app::CommandSender * apCommandSender, chip::Protocols::InteractionModel::Status aStatus,
-                 CHIP_ERROR aError) override
+    void OnError(const chip::app::CommandSender * apCommandSender, const chip::app::StatusIB & aStatus, CHIP_ERROR aError) override
     {
-        ChipLogError(Controller, "OnError happens with %" PRIx16 " %" CHIP_ERROR_FORMAT, to_underlying(aStatus), aError.Format());
+        ChipLogError(Controller, "OnError happens with %" PRIx16 " %" CHIP_ERROR_FORMAT, to_underlying(aStatus.mStatus),
+                     aError.Format());
         onErrorCalledTimes++;
     }
     void OnDone(chip::app::CommandSender * apCommandSender) override { onFinalCalledTimes++; }

src/app/tests/integration/chip_im_initiator.cpp

@@ -163,7 +163,7 @@ class MockInteractionModelApp : public chip::app::InteractionModelDelegate,
     }
 
     void OnResponse(chip::app::CommandSender * apCommandSender, const chip::app::ConcreteCommandPath & aPath,
-                    chip::TLV::TLVReader * aData) override
+                    const chip::app::StatusIB & aStatus, chip::TLV::TLVReader * aData) override
     {
         printf("Command Response Success with EndpointId %d, ClusterId %d, CommandId %d", aPath.mEndpointId, aPath.mClusterId,
                aPath.mCommandId);
@@ -178,8 +178,7 @@ class MockInteractionModelApp : public chip::app::InteractionModelDelegate,
                static_cast<double>(gCommandRespCount) * 100 / static_cast<double>(gCommandCount),
                static_cast<double>(transitTime) / 1000);
     }
-    void OnError(const chip::app::CommandSender * apCommandSender, chip::Protocols::InteractionModel::Status aProtocolCode,
-                 CHIP_ERROR aError) override
+    void OnError(const chip::app::CommandSender * apCommandSender, const chip::app::StatusIB & aStatus, CHIP_ERROR aError) override
     {
         gCommandRespCount += (aError == CHIP_ERROR_IM_STATUS_CODE_RECEIVED);
         gLastCommandResult = TestCommandResult::kFailure;

src/app/zap-templates/templates/app/CHIPClusters-src.zapt

@@ -146,12 +146,12 @@ CHIP_ERROR ClusterBase::InvokeCommand(const RequestDataT & requestData, void * c
     VerifyOrReturnError(mDevice != nullptr, CHIP_ERROR_INCORRECT_STATE);
     ReturnErrorOnFailure(mDevice->LoadSecureSessionParametersIfNeeded());
 
-    auto onSuccessCb = [context, successCb](const app::ConcreteCommandPath & commandPath, const ResponseDataT & responseData) {
+    auto onSuccessCb = [context, successCb](const app::ConcreteCommandPath & commandPath, const app::StatusIB & aStatus, const ResponseDataT & responseData) {
         successCb(context, responseData);
     };
 
-    auto onFailureCb = [context, failureCb](Protocols::InteractionModel::Status aIMStatus, CHIP_ERROR aError) {
-        failureCb(context, app::ToEmberAfStatus(aIMStatus));
+    auto onFailureCb = [context, failureCb](const app::StatusIB & aStatus, CHIP_ERROR aError) {
+        failureCb(context, app::ToEmberAfStatus(aStatus.mStatus));
     };
 
     return InvokeCommandRequest<ResponseDataT>(mDevice->GetExchangeManager(), mDevice->GetSecureSession().Value(), mEndpoint,

src/controller/CHIPDeviceController.cpp

@@ -1669,7 +1669,8 @@ void DeviceCommissioner::OnNodeDiscoveryComplete(const chip::Dnssd::DiscoveredNo
 #endif // CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY
 
 void DeviceControllerInteractionModelDelegate::OnResponse(app::CommandSender * apCommandSender,
-                                                          const app::ConcreteCommandPath & aPath, TLV::TLVReader * aData)
+                                                          const app::ConcreteCommandPath & aPath,
+                                                          const chip::app::StatusIB & aStatus, TLV::TLVReader * aData)
 {
     // Generally IM has more detailed errors than ember library, here we always use the, the actual handling of the
     // commands should implement full IMDelegate.
@@ -1687,7 +1688,7 @@ void DeviceControllerInteractionModelDelegate::OnResponse(app::CommandSender * a
 }
 
 void DeviceControllerInteractionModelDelegate::OnError(const app::CommandSender * apCommandSender,
-                                                       Protocols::InteractionModel::Status aClusterStatus, CHIP_ERROR aError)
+                                                       const chip::app::StatusIB & aStatus, CHIP_ERROR aError)
 {
     // The IMDefaultResponseCallback started out life as an Ember function, so it only accepted
     // Ember status codes. Consequently, let's convert the IM code over to a meaningful Ember status before dispatching.
@@ -1696,7 +1697,7 @@ void DeviceControllerInteractionModelDelegate::OnError(const app::CommandSender
     // well, this will be an even bigger problem.
     //
     // For now, #10331 tracks this issue.
-    IMDefaultResponseCallback(apCommandSender, app::ToEmberAfStatus(aClusterStatus));
+    IMDefaultResponseCallback(apCommandSender, app::ToEmberAfStatus(aStatus.mStatus));
 }
 
 void DeviceControllerInteractionModelDelegate::OnDone(app::CommandSender * apCommandSender)

src/controller/DeviceControllerInteractionModelDelegate.h

@@ -20,8 +20,9 @@ class DeviceControllerInteractionModelDelegate : public chip::app::InteractionMo
                                                  public chip::app::WriteClient::Callback
 {
 public:
-    void OnResponse(app::CommandSender * apCommandSender, const app::ConcreteCommandPath & aPath, TLV::TLVReader * aData) override;
-    void OnError(const app::CommandSender * apCommandSender, Protocols::InteractionModel::Status aInteractionModelStatus,
+    void OnResponse(app::CommandSender * apCommandSender, const app::ConcreteCommandPath & aPath,
+                    const chip::app::StatusIB & aStatus, TLV::TLVReader * aData) override;
+    void OnError(const app::CommandSender * apCommandSender, const chip::app::StatusIB & aStatus,
                  CHIP_ERROR aProtocolError) override;
     void OnDone(app::CommandSender * apCommandSender) override;