address comments

project-chip · Nov 12, 2021 · 460ca44 · 460ca44
1 parent ce537e9
commit 460ca44
Show file tree

Hide file tree

Showing 9 changed files with 43 additions and 56 deletions.
diff --git a/src/app/CommandSender.cpp b/src/app/CommandSender.cpp
@@ -92,26 +92,18 @@ CHIP_ERROR CommandSender::OnMessageReceived(Messaging::ExchangeContext * apExcha
                                             System::PacketBufferHandle && aPayload)
 {
     CHIP_ERROR err = CHIP_NO_ERROR;
+    StatusIB status(Protocols::InteractionModel::Status::Failure);
     VerifyOrExit(apExchangeContext == mpExchangeCtx, err = CHIP_ERROR_INCORRECT_STATE);
     if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::InvokeCommandResponse))
     {
         err = ProcessInvokeResponse(std::move(aPayload));
         SuccessOrExit(err);
+        status.mStatus = Protocols::InteractionModel::Status::Success;
     }
     else if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::StatusResponse))
     {
-        Protocols::InteractionModel::Status status;
-        err           = StatusResponse::ProcessStatusResponse(apExchangeContext, std::move(aPayload), status);
-        mpExchangeCtx = nullptr;
+        err = StatusResponse::ProcessStatusResponse(std::move(aPayload), status);
         SuccessOrExit(err);
-        if (status == Protocols::InteractionModel::Status::ResourceExhausted)
-        {
-            err = CHIP_ERROR_NO_MEMORY;
-        }
-        else
-        {
-            err = CHIP_ERROR_INCORRECT_STATE;
-        }
     }
     else
     {
@@ -123,8 +115,6 @@ CHIP_ERROR CommandSender::OnMessageReceived(Messaging::ExchangeContext * apExcha
     {
         if (err != CHIP_NO_ERROR)
         {
-            StatusIB status;
-            status.mStatus = Protocols::InteractionModel::Status::Failure;
             mpCallback->OnError(this, status, err);
         }
     }

diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp
@@ -402,7 +402,7 @@ CHIP_ERROR InteractionModelEngine::OnMessageReceived(Messaging::ExchangeContext
     else if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::ReadRequest))
     {
         SuccessOrExit(OnReadInitialRequest(apExchangeContext, aPayloadHeader, std::move(aPayload),
-                                                  ReadHandler::InteractionType::Read, status));
+                                           ReadHandler::InteractionType::Read, status));
     }
     else if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::WriteRequest))
     {
@@ -411,11 +411,12 @@ CHIP_ERROR InteractionModelEngine::OnMessageReceived(Messaging::ExchangeContext
     else if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::SubscribeRequest))
     {
         SuccessOrExit(OnReadInitialRequest(apExchangeContext, aPayloadHeader, std::move(aPayload),
-                                                  ReadHandler::InteractionType::Subscribe, status));
+                                           ReadHandler::InteractionType::Subscribe, status));
     }
     else if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::ReportData))
     {
         ReturnErrorOnFailure(OnUnsolicitedReportData(apExchangeContext, aPayloadHeader, std::move(aPayload)));
+        status = Protocols::InteractionModel::Status::Success;
     }
     else
     {
@@ -426,10 +427,6 @@ CHIP_ERROR InteractionModelEngine::OnMessageReceived(Messaging::ExchangeContext
     if (status != Protocols::InteractionModel::Status::Success)
     {
         err = StatusResponse::SendStatusResponse(status, apExchangeContext, false /*aExpectResponse*/);
-        if (err != CHIP_NO_ERROR)
-        {
-            apExchangeContext->Abort();
-        }
     }
     return err;
 }

diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h
@@ -200,6 +200,11 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate, public Comman
 
     void OnDone(CommandHandler & apCommandObj) override;
 
+    /**
+     * Called when Interaction Model receives a Command Request message.  Errors processing
+     * the Command Request are handled entirely within this function. The caller pre-sets status to failure and the callee is
+     * expected to set it to success if it does not want an automatic status response message to be sent.
+     */
     CHIP_ERROR OnInvokeCommandRequest(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader,
                                       System::PacketBufferHandle && aPayload, Protocols::InteractionModel::Status & aStatus);
     CHIP_ERROR OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader,
@@ -208,7 +213,8 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate, public Comman
 
     /**
      * Called when Interaction Model receives a Read Request message.  Errors processing
-     * the Read Request are handled entirely within this function.
+     * the Read Request are handled entirely within this function. The caller pre-sets status to failure and the callee is
+     * expected to set it to success if it does not want an automatic status response message to be sent.
      */
 
     CHIP_ERROR OnReadInitialRequest(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader,
@@ -217,7 +223,8 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate, public Comman
 
     /**
      * Called when Interaction Model receives a Write Request message.  Errors processing
-     * the Write Request are handled entirely within this function.
+     * the Write Request are handled entirely within this function. The caller pre-sets status to failure and the callee is
+     * expected to set it to success if it does not want an automatic status response message to be sent.
      */
     CHIP_ERROR OnWriteRequest(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader,
                               System::PacketBufferHandle && aPayload, Protocols::InteractionModel::Status & aStatus);

diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp
@@ -250,19 +250,10 @@ CHIP_ERROR ReadClient::OnMessageReceived(Messaging::ExchangeContext * apExchange
     }
     else if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::StatusResponse))
     {
-        Protocols::InteractionModel::Status status;
+        StatusIB status;
         VerifyOrExit(apExchangeContext == mpExchangeCtx, err = CHIP_ERROR_INCORRECT_STATE);
-        err           = StatusResponse::ProcessStatusResponse(apExchangeContext, std::move(aPayload), status);
-        mpExchangeCtx = nullptr;
+        err = StatusResponse::ProcessStatusResponse(std::move(aPayload), status);
         SuccessOrExit(err);
-        if (status == Protocols::InteractionModel::Status::ResourceExhausted)
-        {
-            err = CHIP_ERROR_NO_MEMORY;
-        }
-        else
-        {
-            err = CHIP_ERROR_INCORRECT_STATE;
-        }
     }
     else
     {

diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp
@@ -134,10 +134,9 @@ CHIP_ERROR ReadHandler::OnReadInitialRequest(System::PacketBufferHandle && aPayl
 CHIP_ERROR ReadHandler::OnStatusResponse(Messaging::ExchangeContext * apExchangeContext, System::PacketBufferHandle && aPayload)
 {
     CHIP_ERROR err = CHIP_NO_ERROR;
-    Protocols::InteractionModel::Status status;
-    err = StatusResponse::ProcessStatusResponse(apExchangeContext, std::move(aPayload), status);
-    VerifyOrExit((err == CHIP_NO_ERROR) && (status == Protocols::InteractionModel::Status::Success),
-                 err = CHIP_ERROR_INVALID_ARGUMENT);
+    StatusIB status;
+    err = StatusResponse::ProcessStatusResponse(std::move(aPayload), status);
+    SuccessOrExit(err);
     switch (mState)
     {
     case HandlerState::AwaitingReportResponse:

diff --git a/src/app/StatusResponse.cpp b/src/app/StatusResponse.cpp
@@ -24,7 +24,6 @@ namespace app {
 CHIP_ERROR StatusResponse::SendStatusResponse(Protocols::InteractionModel::Status aStatus,
                                               Messaging::ExchangeContext * apExchangeContext, bool aExpectResponse)
 {
-    using Protocols::InteractionModel::Status;
     VerifyOrReturnError(apExchangeContext != nullptr, CHIP_ERROR_INCORRECT_STATE);
     System::PacketBufferHandle msgBuf = System::PacketBufferHandle::New(kMaxSecureSduLengthBytes);
     VerifyOrReturnError(!msgBuf.IsNull(), CHIP_ERROR_NO_MEMORY);
@@ -43,10 +42,9 @@ CHIP_ERROR StatusResponse::SendStatusResponse(Protocols::InteractionModel::Statu
     return CHIP_NO_ERROR;
 }
 
-CHIP_ERROR StatusResponse::ProcessStatusResponse(Messaging::ExchangeContext * apExchangeContext,
-                                                 System::PacketBufferHandle && aPayload,
-                                                 Protocols::InteractionModel::Status & aStatus)
+CHIP_ERROR StatusResponse::ProcessStatusResponse(System::PacketBufferHandle && aPayload, StatusIB & aStatus)
 {
+    CHIP_ERROR err = CHIP_NO_ERROR;
     StatusResponseMessage::Parser response;
     System::PacketBufferTLVReader reader;
     reader.Init(std::move(aPayload));
@@ -55,9 +53,22 @@ CHIP_ERROR StatusResponse::ProcessStatusResponse(Messaging::ExchangeContext * ap
 #if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
     ReturnErrorOnFailure(response.CheckSchemaValidity());
 #endif
-    ReturnErrorOnFailure(response.GetStatus(aStatus));
-    ChipLogProgress(InteractionModel, "Receive status response, status is %" PRIu16, to_underlying(aStatus));
-    return CHIP_NO_ERROR;
+    ReturnErrorOnFailure(response.GetStatus(aStatus.mStatus));
+    ChipLogProgress(InteractionModel, "Received status response, status is %" PRIu16, to_underlying(aStatus.mStatus));
+
+    if (aStatus.mStatus == Protocols::InteractionModel::Status::Success)
+    {
+        err = CHIP_NO_ERROR;
+    }
+    else if (aStatus.mStatus == Protocols::InteractionModel::Status::ResourceExhausted)
+    {
+        err = CHIP_ERROR_NO_MEMORY;
+    }
+    else
+    {
+        err = CHIP_ERROR_INCORRECT_STATE;
+    }
+    return err;
 }
 } // namespace app
 } // namespace chip
diff --git a/src/app/StatusResponse.h b/src/app/StatusResponse.h
@@ -18,6 +18,7 @@
 
 #pragma once
 
+#include <app/MessageDef/StatusIB.h>
 #include <messaging/ExchangeContext.h>
 #include <protocols/interaction_model/Constants.h>
 #include <system/TLVPacketBufferBackingStore.h>
@@ -31,8 +32,7 @@ class StatusResponse
 public:
     static CHIP_ERROR SendStatusResponse(Protocols::InteractionModel::Status aStatus,
                                          Messaging::ExchangeContext * apExchangeContext, bool aExpectResponse);
-    static CHIP_ERROR ProcessStatusResponse(Messaging::ExchangeContext * apExchangeContext, System::PacketBufferHandle && aPayload,
-                                            Protocols::InteractionModel::Status & aStatus);
+    static CHIP_ERROR ProcessStatusResponse(System::PacketBufferHandle && aPayload, StatusIB & aStatus);
 };
 } // namespace app
 } // namespace chip
diff --git a/src/app/WriteClient.cpp b/src/app/WriteClient.cpp
@@ -286,18 +286,9 @@ CHIP_ERROR WriteClient::OnMessageReceived(Messaging::ExchangeContext * apExchang
     }
     else if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::StatusResponse))
     {
-        Protocols::InteractionModel::Status status;
-        err           = StatusResponse::ProcessStatusResponse(apExchangeContext, std::move(aPayload), status);
-        mpExchangeCtx = nullptr;
+        StatusIB status;
+        err = StatusResponse::ProcessStatusResponse(std::move(aPayload), status);
         SuccessOrExit(err);
-        if (status == Protocols::InteractionModel::Status::ResourceExhausted)
-        {
-            err = CHIP_ERROR_NO_MEMORY;
-        }
-        else
-        {
-            err = CHIP_ERROR_INCORRECT_STATE;
-        }
     }
     else
     {

diff --git a/src/app/util/basic-types.h b/src/app/util/basic-types.h
@@ -29,6 +29,7 @@
 #include <lib/core/DataModelTypes.h>
 
 namespace chip {
+
 typedef uint8_t Percent;
 typedef uint16_t Percent100ths;