Fix IM client-side APIs to allow timed invoke. (#12465)

* Fix IM client-side APIs to allow timed invoke.

* Add timedInteractionTimeoutMs optional argument to chip-tool commands

Co-authored-by: Vivien Nicolas <[email protected]>

examples/chip-tool/commands/clusters/ModelCommand.h

@@ -38,6 +38,7 @@ class ModelCommand : public CHIPCommand
     {
         AddArgument("node-id", 0, UINT64_MAX, &mNodeId);
         AddArgument("endpoint-id", CHIP_ZCL_ENDPOINT_MIN, CHIP_ZCL_ENDPOINT_MAX, &mEndPointId);
+        AddArgument("timedInteractionTimeoutMs", 0, UINT16_MAX, &mTimedInteractionTimeoutMs);
     }
 
     /////////// CHIPCommand Interface /////////
@@ -46,6 +47,9 @@ class ModelCommand : public CHIPCommand
 
     virtual CHIP_ERROR SendCommand(ChipDevice * device, uint8_t endPointId) = 0;
 
+protected:
+    chip::Optional<uint16_t> mTimedInteractionTimeoutMs;
+
 private:
     chip::NodeId mNodeId;
     uint8_t mEndPointId;

examples/chip-tool/commands/common/CommandInvoker.h

@@ -23,6 +23,7 @@
 #include <app/DeviceProxy.h>
 #include <app/InteractionModelEngine.h>
 #include <app/util/error-mapping.h>
+#include <lib/core/Optional.h>
 
 namespace chip {
 namespace Controller {
@@ -84,14 +85,16 @@ class CommandInvoker final : public ResponseReceiver<typename RequestType::Respo
         return Platform::MakeUnique<CommandInvoker>(aContext, aOnSuccess, aOnError);
     }
 
-    CHIP_ERROR InvokeCommand(DeviceProxy * aDevice, EndpointId aEndpoint, const RequestType & aRequestData)
+    CHIP_ERROR InvokeCommand(DeviceProxy * aDevice, EndpointId aEndpoint, const RequestType & aRequestData,
+                             const Optional<uint16_t> & aTimedInvokeTimeoutMs)
     {
         app::CommandPathParams commandPath = { aEndpoint, 0 /* groupId */, RequestType::GetClusterId(), RequestType::GetCommandId(),
                                                (app::CommandPathFlags::kEndpointIdValid) };
-        auto commandSender                 = Platform::MakeUnique<app::CommandSender>(this, aDevice->GetExchangeManager());
+        auto commandSender =
+            Platform::MakeUnique<app::CommandSender>(this, aDevice->GetExchangeManager(), aTimedInvokeTimeoutMs.HasValue());
         VerifyOrReturnError(commandSender != nullptr, CHIP_ERROR_NO_MEMORY);
 
-        ReturnErrorOnFailure(commandSender->AddRequestData(commandPath, aRequestData));
+        ReturnErrorOnFailure(commandSender->AddRequestDataNoTimedCheck(commandPath, aRequestData, aTimedInvokeTimeoutMs));
         ReturnErrorOnFailure(commandSender->SendCommandRequest(aDevice->GetSecureSession().Value()));
         commandSender.release();
         return CHIP_NO_ERROR;
@@ -185,16 +188,36 @@ template <typename RequestType>
 CHIP_ERROR InvokeCommand(DeviceProxy * aDevice, void * aContext,
                          typename detail::CommandInvoker<RequestType>::SuccessCallback aSuccessCallback,
                          typename detail::CommandInvoker<RequestType>::FailureCallback aFailureCallback, EndpointId aEndpoint,
-                         const RequestType & aRequestData)
+                         const RequestType & aRequestData, const Optional<uint16_t> & aTimedInvokeTimeoutMs)
 {
     auto invoker = detail::CommandInvoker<RequestType>::Alloc(aContext, aSuccessCallback, aFailureCallback);
     VerifyOrReturnError(invoker != nullptr, CHIP_ERROR_NO_MEMORY);
-    ReturnErrorOnFailure(invoker->InvokeCommand(aDevice, aEndpoint, aRequestData));
+    ReturnErrorOnFailure(invoker->InvokeCommand(aDevice, aEndpoint, aRequestData, aTimedInvokeTimeoutMs));
     invoker.release();
     return CHIP_NO_ERROR;
 }
 
 template <typename RequestType>
+CHIP_ERROR InvokeCommand(DeviceProxy * aDevice, void * aContext,
+                         typename detail::CommandInvoker<RequestType>::SuccessCallback aSuccessCallback,
+                         typename detail::CommandInvoker<RequestType>::FailureCallback aFailureCallback, EndpointId aEndpoint,
+                         const RequestType & aRequestData, uint16_t aTimedInvokeTimeoutMs)
+{
+    return InvokeCommand(aDevice, aContext, aSuccessCallback, aFailureCallback, aEndpoint, aRequestData,
+                         MakeOptional(aTimedInvokeTimeoutMs));
+}
+
+template <typename RequestType, typename std::enable_if_t<!RequestType::MustUseTimedInvoke(), int> = 0>
+CHIP_ERROR InvokeCommand(DeviceProxy * aDevice, void * aContext,
+                         typename detail::CommandInvoker<RequestType>::SuccessCallback aSuccessCallback,
+                         typename detail::CommandInvoker<RequestType>::FailureCallback aFailureCallback, EndpointId aEndpoint,
+                         const RequestType & aRequestData)
+{
+    return InvokeCommand(aDevice, aContext, aSuccessCallback, aFailureCallback, aEndpoint, aRequestData, NullOptional);
+}
+
+// Group commands can't do timed invoke in a meaningful way.
+template <typename RequestType, typename std::enable_if_t<!RequestType::MustUseTimedInvoke(), int> = 0>
 CHIP_ERROR InvokeGroupCommand(DeviceProxy * aDevice, void * aContext,
                               typename detail::CommandInvoker<RequestType>::SuccessCallback aSuccessCallback,
                               typename detail::CommandInvoker<RequestType>::FailureCallback aFailureCallback, GroupId groupId,

examples/chip-tool/include/CHIPProjectAppConfig.h

@@ -70,4 +70,7 @@
 
 #define CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY 1
 
+// Enable some test-only interaction model APIs.
+#define CONFIG_IM_BUILD_FOR_UNIT_TEST 1
+
 #endif /* CHIPPROJECTCONFIG_H */

examples/chip-tool/templates/commands.zapt

@@ -445,7 +445,7 @@ public:
         {{/if}}
         {{/chip_cluster_command_non_expanded_arguments}}
 
-        return chip::Controller::InvokeCommand(device, this, {{#if hasSpecificResponse}}On{{asUpperCamelCase parent.name}}{{asUpperCamelCase response.name}}Success{{else}}OnDefaultSuccess{{/if}}, OnDefaultFailure, endpointId, mRequest);
+        return chip::Controller::InvokeCommand(device, this, {{#if hasSpecificResponse}}On{{asUpperCamelCase parent.name}}{{asUpperCamelCase response.name}}Success{{else}}OnDefaultSuccess{{/if}}, OnDefaultFailure, endpointId, mRequest, mTimedInteractionTimeoutMs);
     }
 
 private:

examples/chip-tool/templates/partials/test_cluster.zapt

@@ -196,7 +196,24 @@ class {{filename}}: public TestCommand
             (static_cast<{{filename}} *>(context))->OnFailureResponse_{{index}}(status);
         };
 
-        ReturnErrorOnFailure(chip::Controller::{{#if isGroupCommand}}InvokeGroupCommand{{else}}InvokeCommand{{/if}}({{>device}}, this, success, failure, {{#if isGroupCommand}}groupId{{else}}endpoint{{/if}}, request));
+        ReturnErrorOnFailure(chip::Controller::{{#if isGroupCommand}}InvokeGroupCommand{{else}}InvokeCommand{{/if}}({{>device}}, this, success, failure,
+          {{#if isGroupCommand}}groupId{{else}}endpoint{{/if}},
+          request
+          {{#if timedInteractionTimeoutMs}}
+          , {{timedInteractionTimeoutMs}}
+          {{else if commandObject.mustUseTimedInvoke}}
+          , chip::NullOptional
+          {{/if}}
+          ));
+        {{#if busyWaitMs}}
+          {
+            using namespace chip::System::Clock::Literals;
+            // Busy-wait for {{busyWaitMs}} milliseconds.
+            auto & clock = chip::System::SystemClock();
+            auto start = clock.GetMonotonicTimestamp();
+            while (clock.GetMonotonicTimestamp() - start < {{busyWaitMs}}_ms);
+          }
+        {{/if}}
         {{#unless async}}return CHIP_NO_ERROR;{{/unless}}
         {{else}}
         chip::Controller::{{asUpperCamelCase cluster}}ClusterTest cluster;

examples/chip-tool/templates/tests-commands.zapt

@@ -4,6 +4,9 @@
 
 #include <commands/tests/TestCommand.h>
 #include <commands/common/CommandInvoker.h>
+#include <lib/core/Optional.h>
+#include <system/SystemClock.h>
+
 #include <math.h> // For INFINITY
 
 class TestList : public Command

src/app/Command.cpp

@@ -106,9 +106,15 @@ const char * Command::GetStateStr() const
     case CommandState::AddedCommand:
         return "AddedCommand";
 
+    case CommandState::AwaitingTimedStatus:
+        return "AwaitingTimedStatus";
+
     case CommandState::CommandSent:
         return "CommandSent";
 
+    case CommandState::ResponseReceived:
+        return "ResponseReceived";
+
     case CommandState::AwaitingDestruction:
         return "AwaitingDestruction";
     }

src/app/Command.h

@@ -50,7 +50,9 @@ class Command
         Idle,                ///< Default state that the object starts out in, where no work has commenced
         AddingCommand,       ///< In the process of adding a command.
         AddedCommand,        ///< A command has been completely encoded and is awaiting transmission.
+        AwaitingTimedStatus, ///< Sent a Timed Request and waiting for response.
         CommandSent,         ///< The command has been sent successfully.
+        ResponseReceived,    ///< Received a response to our invoke and request and processing the response.
         AwaitingDestruction, ///< The object has completed its work and is awaiting destruction by the application.
     };
 

src/app/CommandSender.cpp

@@ -27,14 +27,15 @@
 #include "CommandHandler.h"
 #include "InteractionModelEngine.h"
 #include "StatusResponse.h"
+#include <app/MessageDef/TimedRequestMessage.h>
 #include <protocols/Protocols.h>
 #include <protocols/interaction_model/Constants.h>
 
 namespace chip {
 namespace app {
 
-CommandSender::CommandSender(Callback * apCallback, Messaging::ExchangeManager * apExchangeMgr) :
-    mpCallback(apCallback), mpExchangeMgr(apExchangeMgr), mSuppressResponse(false), mTimedRequest(false)
+CommandSender::CommandSender(Callback * apCallback, Messaging::ExchangeManager * apExchangeMgr, bool aIsTimedRequest) :
+    mpCallback(apCallback), mpExchangeMgr(apExchangeMgr), mSuppressResponse(false), mTimedRequest(aIsTimedRequest)
 {}
 
 CHIP_ERROR CommandSender::AllocateBuffer()
@@ -64,36 +65,56 @@ CHIP_ERROR CommandSender::AllocateBuffer()
 
 CHIP_ERROR CommandSender::SendCommandRequest(SessionHandle session, System::Clock::Timeout timeout)
 {
-    CHIP_ERROR err = CHIP_NO_ERROR;
-    System::PacketBufferHandle commandPacket;
-
-    VerifyOrExit(mState == CommandState::AddedCommand, err = CHIP_ERROR_INCORRECT_STATE);
+    VerifyOrReturnError(mState == CommandState::AddedCommand, CHIP_ERROR_INCORRECT_STATE);
 
-    err = Finalize(commandPacket);
-    SuccessOrExit(err);
+    ReturnErrorOnFailure(Finalize(mPendingInvokeData));
 
     // Create a new exchange context.
     mpExchangeCtx = mpExchangeMgr->NewContext(session, this);
-    VerifyOrExit(mpExchangeCtx != nullptr, err = CHIP_ERROR_NO_MEMORY);
+    VerifyOrReturnError(mpExchangeCtx != nullptr, CHIP_ERROR_NO_MEMORY);
 
     mpExchangeCtx->SetResponseTimeout(timeout);
 
-    err = mpExchangeCtx->SendMessage(Protocols::InteractionModel::MsgType::InvokeCommandRequest, std::move(commandPacket),
-                                     Messaging::SendFlags(Messaging::SendMessageFlags::kExpectResponse));
-    SuccessOrExit(err);
+    if (mTimedInvokeTimeoutMs.HasValue())
+    {
+        return SendTimedRequest(mTimedInvokeTimeoutMs.Value());
+    }
+
+    return SendInvokeRequest();
+}
+
+CHIP_ERROR CommandSender::SendInvokeRequest()
+{
+    using namespace Protocols::InteractionModel;
+    using namespace Messaging;
 
+    ReturnErrorOnFailure(mpExchangeCtx->SendMessage(MsgType::InvokeCommandRequest, std::move(mPendingInvokeData),
+                                                    SendMessageFlags::kExpectResponse));
     MoveToState(CommandState::CommandSent);
 
-exit:
-    return err;
+    return CHIP_NO_ERROR;
 }
 
 CHIP_ERROR CommandSender::OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader,
                                             System::PacketBufferHandle && aPayload)
 {
+    if (mState == CommandState::CommandSent)
+    {
+        MoveToState(CommandState::ResponseReceived);
+    }
+
     CHIP_ERROR err = CHIP_NO_ERROR;
     StatusIB status(Protocols::InteractionModel::Status::Failure);
     VerifyOrExit(apExchangeContext == mpExchangeCtx, err = CHIP_ERROR_INCORRECT_STATE);
+
+    if (mState == CommandState::AwaitingTimedStatus)
+    {
+        err = HandleTimedStatus(aPayloadHeader, std::move(aPayload), status);
+        // Skip all other processing here (which is for the response to the
+        // invoke request), no matter whether err is success or not.
+        goto exit;
+    }
+
     if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::InvokeCommandResponse))
     {
         err = ProcessInvokeResponse(std::move(aPayload));
@@ -119,7 +140,11 @@ CHIP_ERROR CommandSender::OnMessageReceived(Messaging::ExchangeContext * apExcha
         }
     }
 
-    Close();
+    if (mState != CommandState::CommandSent)
+    {
+        Close();
+    }
+    // Else we got a response to a Timed Request and just send the invoke.
 
     return err;
 }
@@ -330,5 +355,62 @@ TLV::TLVWriter * CommandSender::GetCommandDataIBTLVWriter()
     }
 }
 
+CHIP_ERROR CommandSender::SendTimedRequest(uint16_t aTimeoutMs)
+{
+    using namespace Protocols::InteractionModel;
+    using namespace Messaging;
+
+    // The payload is an anonymous struct (2 bytes) containing a single
+    // 16-bit integer with a context tag (1 control byte, 1 byte tag, at
+    // most 2 bytes for the integer).  Use MessagePacketBuffer::New to
+    // account for other message-global overheads (MIC, etc).
+    System::PacketBufferHandle payload = MessagePacketBuffer::New(6);
+    VerifyOrReturnError(!payload.IsNull(), CHIP_ERROR_NO_MEMORY);
+
+    System::PacketBufferTLVWriter writer;
+    writer.Init(std::move(payload));
+
+    TimedRequestMessage::Builder builder;
+    ReturnErrorOnFailure(builder.Init(&writer));
+
+    builder.TimeoutMs(aTimeoutMs);
+    ReturnErrorOnFailure(builder.GetError());
+
+    ReturnErrorOnFailure(writer.Finalize(&payload));
+
+    ReturnErrorOnFailure(mpExchangeCtx->SendMessage(MsgType::TimedRequest, std::move(payload), SendMessageFlags::kExpectResponse));
+    MoveToState(CommandState::AwaitingTimedStatus);
+    return CHIP_NO_ERROR;
+}
+
+CHIP_ERROR CommandSender::HandleTimedStatus(const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload,
+                                            StatusIB & aStatusIB)
+{
+    using namespace Protocols::InteractionModel;
+
+    VerifyOrReturnError(aPayloadHeader.HasMessageType(MsgType::StatusResponse), CHIP_ERROR_INVALID_MESSAGE_TYPE);
+
+    ReturnErrorOnFailure(StatusResponse::ProcessStatusResponse(std::move(aPayload), aStatusIB));
+
+    VerifyOrReturnError(aStatusIB.mStatus == Status::Success, CHIP_ERROR_IM_STATUS_CODE_RECEIVED);
+
+    return SendInvokeRequest();
+}
+
+CHIP_ERROR CommandSender::FinishCommand(const Optional<uint16_t> & aTimedInvokeTimeoutMs)
+{
+    ReturnErrorOnFailure(FinishCommand(/* aEndDataStruct = */ false));
+    if (!mTimedInvokeTimeoutMs.HasValue())
+    {
+        mTimedInvokeTimeoutMs = aTimedInvokeTimeoutMs;
+    }
+    else if (aTimedInvokeTimeoutMs.HasValue())
+    {
+        uint16_t newValue = std::min(mTimedInvokeTimeoutMs.Value(), aTimedInvokeTimeoutMs.Value());
+        mTimedInvokeTimeoutMs.SetValue(newValue);
+    }
+    return CHIP_NO_ERROR;
+}
+
 } // namespace app
 } // namespace chip