From cbfe5ac57d58ece2e6e39db80e8acf727b568c47 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 2 Dec 2021 00:55:48 -0500 Subject: [PATCH] Fix IM client-side APIs to allow timed invoke. --- .../commands/common/CommandInvoker.h | 33 +++- .../chip-tool/include/CHIPProjectAppConfig.h | 3 + examples/chip-tool/templates/commands.zapt | 8 +- .../templates/partials/test_cluster.zapt | 19 +- .../chip-tool/templates/tests-commands.zapt | 3 + src/app/Command.cpp | 6 + src/app/Command.h | 2 + src/app/CommandSender.cpp | 112 +++++++++-- src/app/CommandSender.h | 70 ++++++- src/app/TimedHandler.cpp | 8 +- src/app/tests/suites/TestCluster.yaml | 1 + .../tests/suites/TestClusterComplexTypes.yaml | 42 +++++ src/controller/CHIPCluster.h | 23 ++- src/controller/InvokeInteraction.h | 31 +++- .../java/templates/CHIPClusters-JNI.zapt | 7 +- .../java/zap-generated/CHIPClusters-JNI.cpp | 2 +- .../CHIP/templates/CHIPClustersObjc-src.zapt | 7 +- .../CHIP/zap-generated/CHIPClustersObjc.mm | 2 +- .../zap-generated/cluster/Commands.h | 2 +- .../chip-tool/zap-generated/test/Commands.h | 175 +++++++++++++++++- 20 files changed, 518 insertions(+), 38 deletions(-) diff --git a/examples/chip-tool/commands/common/CommandInvoker.h b/examples/chip-tool/commands/common/CommandInvoker.h index 3f7f12f276df80..6583cc9124280e 100644 --- a/examples/chip-tool/commands/common/CommandInvoker.h +++ b/examples/chip-tool/commands/common/CommandInvoker.h @@ -23,6 +23,7 @@ #include #include #include +#include namespace chip { namespace Controller { @@ -84,14 +85,16 @@ class CommandInvoker final : public ResponseReceiver(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 & aTimedInvokeTimeoutMs) { app::CommandPathParams commandPath = { aEndpoint, 0 /* groupId */, RequestType::GetClusterId(), RequestType::GetCommandId(), (app::CommandPathFlags::kEndpointIdValid) }; - auto commandSender = Platform::MakeUnique(this, aDevice->GetExchangeManager()); + auto commandSender = + Platform::MakeUnique(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 CHIP_ERROR InvokeCommand(DeviceProxy * aDevice, void * aContext, typename detail::CommandInvoker::SuccessCallback aSuccessCallback, typename detail::CommandInvoker::FailureCallback aFailureCallback, EndpointId aEndpoint, - const RequestType & aRequestData) + const RequestType & aRequestData, const Optional & aTimedInvokeTimeoutMs) { auto invoker = detail::CommandInvoker::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 +CHIP_ERROR InvokeCommand(DeviceProxy * aDevice, void * aContext, + typename detail::CommandInvoker::SuccessCallback aSuccessCallback, + typename detail::CommandInvoker::FailureCallback aFailureCallback, EndpointId aEndpoint, + const RequestType & aRequestData, uint16_t aTimedInvokeTimeoutMs) +{ + return InvokeCommand(aDevice, aContext, aSuccessCallback, aFailureCallback, aEndpoint, aRequestData, + MakeOptional(aTimedInvokeTimeoutMs)); +} + +template = 0> +CHIP_ERROR InvokeCommand(DeviceProxy * aDevice, void * aContext, + typename detail::CommandInvoker::SuccessCallback aSuccessCallback, + typename detail::CommandInvoker::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 = 0> CHIP_ERROR InvokeGroupCommand(DeviceProxy * aDevice, void * aContext, typename detail::CommandInvoker::SuccessCallback aSuccessCallback, typename detail::CommandInvoker::FailureCallback aFailureCallback, GroupId groupId, diff --git a/examples/chip-tool/include/CHIPProjectAppConfig.h b/examples/chip-tool/include/CHIPProjectAppConfig.h index 72a5e43927a75d..81010d4cacffad 100644 --- a/examples/chip-tool/include/CHIPProjectAppConfig.h +++ b/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 */ diff --git a/examples/chip-tool/templates/commands.zapt b/examples/chip-tool/templates/commands.zapt index 8902634914b524..4f7e45dd442f3f 100644 --- a/examples/chip-tool/templates/commands.zapt +++ b/examples/chip-tool/templates/commands.zapt @@ -445,7 +445,13 @@ 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 + {{#if mustUseTimedInvoke}} + {{!TODO figure out where to get useful timeout values here. For + now, pick 10 seconds.}} + , 10000 + {{/if}} + ); } private: diff --git a/examples/chip-tool/templates/partials/test_cluster.zapt b/examples/chip-tool/templates/partials/test_cluster.zapt index b20aaf5f662de2..984a57f680bb74 100644 --- a/examples/chip-tool/templates/partials/test_cluster.zapt +++ b/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; diff --git a/examples/chip-tool/templates/tests-commands.zapt b/examples/chip-tool/templates/tests-commands.zapt index b517d1d2928760..27e6ac1ea84b28 100644 --- a/examples/chip-tool/templates/tests-commands.zapt +++ b/examples/chip-tool/templates/tests-commands.zapt @@ -4,6 +4,9 @@ #include #include +#include +#include + #include // For INFINITY class TestList : public Command diff --git a/src/app/Command.cpp b/src/app/Command.cpp index 22f7198477c68e..1d9d14d68dac6d 100644 --- a/src/app/Command.cpp +++ b/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"; } diff --git a/src/app/Command.h b/src/app/Command.h index 5e8cb64b62f0c8..4e3a698d910f17 100644 --- a/src/app/Command.h +++ b/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. }; diff --git a/src/app/CommandSender.cpp b/src/app/CommandSender.cpp index 9e3da0b7fae067..35e5822081e801 100644 --- a/src/app/CommandSender.cpp +++ b/src/app/CommandSender.cpp @@ -27,14 +27,15 @@ #include "CommandHandler.h" #include "InteractionModelEngine.h" #include "StatusResponse.h" +#include #include #include 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 & 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 diff --git a/src/app/CommandSender.h b/src/app/CommandSender.h index ef52837c19ac06..b017d6ddf2993b 100644 --- a/src/app/CommandSender.h +++ b/src/app/CommandSender.h @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -116,7 +117,7 @@ class CommandSender final : public Command, public Messaging::ExchangeDelegate * * The callback passed in has to outlive this CommandSender object. */ - CommandSender(Callback * apCallback, Messaging::ExchangeManager * apExchangeMgr); + CommandSender(Callback * apCallback, Messaging::ExchangeManager * apExchangeMgr, bool aIsTimedRequest = false); CHIP_ERROR PrepareCommand(const CommandPathParams & aCommandPathParams, bool aStartDataStruct = true); CHIP_ERROR FinishCommand(bool aEndDataStruct = true); TLV::TLVWriter * GetCommandDataIBTLVWriter(); @@ -129,16 +130,53 @@ class CommandSender final : public Command, public Messaging::ExchangeDelegate * @param [in] aCommandPath The path of the command being requested. * @param [in] aData The data for the request. */ - template + template = 0> CHIP_ERROR AddRequestData(const CommandPathParams & aCommandPath, const CommandDataT & aData) + { + return AddRequestData(aCommandPath, aData, NullOptional); + } + + /** + * API for adding a data request that allows caller to provide a timed + * invoke timeout. If provided, this invoke will be a timed invoke, using + * the minimum of the provided timeouts. + */ + template + CHIP_ERROR AddRequestData(const CommandPathParams & aCommandPath, const CommandDataT & aData, + const Optional & aTimedInvokeTimeoutMs) + { + VerifyOrReturnError(!CommandDataT::MustUseTimedInvoke() || aTimedInvokeTimeoutMs.HasValue(), CHIP_ERROR_INVALID_ARGUMENT); + + return AddRequestDataInternal(aCommandPath, aData, aTimedInvokeTimeoutMs); + } + +#if CONFIG_IM_BUILD_FOR_UNIT_TEST + /** + * Version of AddRequestData that allows sending a message that is + * guaranteed to fail due to requiring a timed invoke but not providing a + * timeout parameter. For use in tests only. + */ + template + CHIP_ERROR AddRequestDataNoTimedCheck(const CommandPathParams & aCommandPath, const CommandDataT & aData, + const Optional & aTimedInvokeTimeoutMs) + { + return AddRequestDataInternal(aCommandPath, aData, aTimedInvokeTimeoutMs); + } +#endif // CONFIG_IM_BUILD_FOR_UNIT_TEST + +private: + template + CHIP_ERROR AddRequestDataInternal(const CommandPathParams & aCommandPath, const CommandDataT & aData, + const Optional & aTimedInvokeTimeoutMs) { ReturnErrorOnFailure(PrepareCommand(aCommandPath, /* aStartDataStruct = */ false)); TLV::TLVWriter * writer = GetCommandDataIBTLVWriter(); VerifyOrReturnError(writer != nullptr, CHIP_ERROR_INCORRECT_STATE); ReturnErrorOnFailure(DataModel::Encode(*writer, TLV::ContextTag(to_underlying(CommandDataIB::Tag::kData)), aData)); - return FinishCommand(/* aEndDataStruct = */ false); + return FinishCommand(aTimedInvokeTimeoutMs); } +public: // Sends a queued up command request to the target encapsulated by the secureSession handle. // // Upon successful return from this call, all subsequent errors that occur during this interaction @@ -185,9 +223,35 @@ class CommandSender final : public Command, public Messaging::ExchangeDelegate CHIP_ERROR ProcessInvokeResponse(System::PacketBufferHandle && payload); CHIP_ERROR ProcessInvokeResponseIB(InvokeResponseIB::Parser & aInvokeResponse); + // SendTimedRequest expects to be called after the exchange has already been + // created and we know for sure we need to do a timed invoke. + CHIP_ERROR SendTimedRequest(uint16_t aTimeoutMs); + + // Handle a message received when we are expecting a status response to a + // Timed Request. The caller is assumed to have already checked that our + // exchange context member is the one the message came in on. + // + // aStatusIB will be populated with the returned status if we can parse it + // successfully. + CHIP_ERROR HandleTimedStatus(const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload, + StatusIB & aStatusIB); + + // Send our queued-up Invoke Request message. Assumes the exchange is ready + // and mPendingInvokeData is populated. + CHIP_ERROR SendInvokeRequest(); + + CHIP_ERROR FinishCommand(const Optional & aTimedInvokeTimeoutMs); + Callback * mpCallback = nullptr; Messaging::ExchangeManager * mpExchangeMgr = nullptr; InvokeRequestMessage::Builder mInvokeRequestBuilder; + // TODO Maybe we should change PacketBufferTLVWriter so we can finalize it + // but have it hold on to the buffer, and get the buffer from it later. + // Then we could avoid this extra pointer-sized member. + System::PacketBufferHandle mPendingInvokeData; + // If mTimedInvokeTimeoutMs has a value, we are expected to do a timed + // invoke. + Optional mTimedInvokeTimeoutMs; TLV::TLVType mDataElementContainerType = TLV::kTLVType_NotSpecified; bool mSuppressResponse = false; bool mTimedRequest = false; diff --git a/src/app/TimedHandler.cpp b/src/app/TimedHandler.cpp index 52400bce8feb71..24281e8a67de62 100644 --- a/src/app/TimedHandler.cpp +++ b/src/app/TimedHandler.cpp @@ -61,7 +61,11 @@ CHIP_ERROR TimedHandler::OnMessageReceived(Messaging::ExchangeContext * aExchang if (mState == State::kExpectingFollowingAction) { - if (System::SystemClock().GetMonotonicTimestamp() > mTimeLimit) + System::Clock::Timestamp now = System::SystemClock().GetMonotonicTimestamp(); + ChipLogDetail(DataManagement, + "Timed following action arrived at 0x" ChipLogFormatX64 ": handler %p exchange " ChipLogFormatExchange, + ChipLogValueX64(now.count()), this, ChipLogValueExchange(aExchangeContext)); + if (now > mTimeLimit) { // Time is up. Spec says to send UNSUPPORTED_ACCESS. ChipLogError(DataManagement, "Timeout expired: handler %p exchange " ChipLogFormatExchange, this, @@ -135,6 +139,8 @@ CHIP_ERROR TimedHandler::HandleTimedRequestAction(Messaging::ExchangeContext * a // Now just wait for the client. mState = State::kExpectingFollowingAction; mTimeLimit = System::SystemClock().GetMonotonicTimestamp() + delay; + ChipLogDetail(DataManagement, "Timed Request time limit 0x" ChipLogFormatX64 ": handler %p exchange " ChipLogFormatExchange, + ChipLogValueX64(mTimeLimit.count()), this, ChipLogValueExchange(aExchangeContext)); return CHIP_NO_ERROR; } diff --git a/src/app/tests/suites/TestCluster.yaml b/src/app/tests/suites/TestCluster.yaml index 4637cc60354832..7c62497d437aa0 100644 --- a/src/app/tests/suites/TestCluster.yaml +++ b/src/app/tests/suites/TestCluster.yaml @@ -982,6 +982,7 @@ tests: value: 20003 - name: "arg2" value: 101 + # Tests for Struct - label: "Send Test Command With Struct Argument and arg1.b is true" diff --git a/src/app/tests/suites/TestClusterComplexTypes.yaml b/src/app/tests/suites/TestClusterComplexTypes.yaml index 31afc12ba94677..1118f8b8f51204 100644 --- a/src/app/tests/suites/TestClusterComplexTypes.yaml +++ b/src/app/tests/suites/TestClusterComplexTypes.yaml @@ -42,3 +42,45 @@ tests: value: true - name: "originalValue" value: null + + # The timed invoke tests are not in TestCluster.yaml for now because Darwin + # SDK APIs have not been updated to do timed invoke properly yet. + + - label: "Send command that needs timed invoke without a timeout value" + command: "timedInvokeRequest" + response: + # No timed interaction timeout provided, so not doing a timed interaction. + error: NEEDS_TIMED_INTERACTION + + - label: "Send command that needs timed invoke with a long timeout value" + # Expecting a success response here. + command: "timedInvokeRequest" + timedInteractionTimeoutMs: 10000 + + - label: + "Send command that needs timed invoke with a too-short timeout value" + command: "timedInvokeRequest" + timedInteractionTimeoutMs: 1 + # Try to ensure that we are unresponsive for long enough that the timeout + # expires. + busyWaitMs: 5 + response: + error: UNSUPPORTED_ACCESS + + - label: + "Send command that does not need timed invoke with a long timeout + value" + # Expecting a success response here. + command: "test" + timedInteractionTimeoutMs: 10000 + + - label: + "Send command that does not need timed invoke with a too-short timeout + value" + command: "test" + timedInteractionTimeoutMs: 1 + # Try to ensure that we are unresponsive for long enough that the timeout + # expires. + busyWaitMs: 5 + response: + error: UNSUPPORTED_ACCESS diff --git a/src/controller/CHIPCluster.h b/src/controller/CHIPCluster.h index 2c6d3786b7b6fd..4a3aed96dd8834 100644 --- a/src/controller/CHIPCluster.h +++ b/src/controller/CHIPCluster.h @@ -32,6 +32,7 @@ #include #include #include +#include namespace chip { namespace Controller { @@ -68,7 +69,7 @@ class DLL_EXPORT ClusterBase template CHIP_ERROR InvokeCommand(const RequestDataT & requestData, void * context, CommandResponseSuccessCallback successCb, - CommandResponseFailureCallback failureCb) + CommandResponseFailureCallback failureCb, const Optional & timedInvokeTimeoutMs) { VerifyOrReturnError(mDevice != nullptr, CHIP_ERROR_INCORRECT_STATE); @@ -82,8 +83,24 @@ class DLL_EXPORT ClusterBase }; return InvokeCommandRequest(mDevice->GetExchangeManager(), mDevice->GetSecureSession().Value(), mEndpoint, requestData, - onSuccessCb, onFailureCb); - }; + onSuccessCb, onFailureCb, timedInvokeTimeoutMs); + } + + template + CHIP_ERROR InvokeCommand(const RequestDataT & requestData, void * context, + CommandResponseSuccessCallback successCb, + CommandResponseFailureCallback failureCb, uint16_t timedInvokeTimeoutMs) + { + return InvokeCommand(requestData, context, successCb, failureCb, MakeOptional(timedInvokeTimeoutMs)); + } + + template = 0> + CHIP_ERROR InvokeCommand(const RequestDataT & requestData, void * context, + CommandResponseSuccessCallback successCb, + CommandResponseFailureCallback failureCb) + { + return InvokeCommand(requestData, context, successCb, failureCb, NullOptional); + } /** * Functions for writing attributes. We have lots of different diff --git a/src/controller/InvokeInteraction.h b/src/controller/InvokeInteraction.h index eb96f7c0445f09..d24ad57c64e777 100644 --- a/src/controller/InvokeInteraction.h +++ b/src/controller/InvokeInteraction.h @@ -20,6 +20,7 @@ #include #include +#include namespace chip { namespace Controller { @@ -46,7 +47,8 @@ CHIP_ERROR InvokeCommandRequest(Messaging::ExchangeManager * aExchangeMgr, SessionHandle sessionHandle, chip::EndpointId endpointId, const RequestObjectT & requestCommandData, typename TypedCommandCallback::OnSuccessCallbackType onSuccessCb, - typename TypedCommandCallback::OnErrorCallbackType onErrorCb) + typename TypedCommandCallback::OnErrorCallbackType onErrorCb, + const Optional & timedInvokeTimeoutMs) { app::CommandPathParams commandPath = { endpointId, 0, RequestObjectT::GetClusterId(), RequestObjectT::GetCommandId(), (app::CommandPathFlags::kEndpointIdValid) }; @@ -68,10 +70,11 @@ InvokeCommandRequest(Messaging::ExchangeManager * aExchangeMgr, SessionHandle se decoder->SetOnDoneCallback(onDone); - auto commandSender = chip::Platform::MakeUnique(decoder.get(), aExchangeMgr); + auto commandSender = + chip::Platform::MakeUnique(decoder.get(), aExchangeMgr, timedInvokeTimeoutMs.HasValue()); VerifyOrReturnError(commandSender != nullptr, CHIP_ERROR_NO_MEMORY); - ReturnErrorOnFailure(commandSender->AddRequestData(commandPath, requestCommandData)); + ReturnErrorOnFailure(commandSender->AddRequestData(commandPath, requestCommandData, timedInvokeTimeoutMs)); ReturnErrorOnFailure(commandSender->SendCommandRequest(sessionHandle)); // @@ -86,5 +89,27 @@ InvokeCommandRequest(Messaging::ExchangeManager * aExchangeMgr, SessionHandle se return CHIP_NO_ERROR; } +template +CHIP_ERROR +InvokeCommandRequest(Messaging::ExchangeManager * exchangeMgr, SessionHandle sessionHandle, chip::EndpointId endpointId, + const RequestObjectT & requestCommandData, + typename TypedCommandCallback::OnSuccessCallbackType onSuccessCb, + typename TypedCommandCallback::OnErrorCallbackType onErrorCb, + uint16_t timedInvokeTimeoutMs) +{ + return InvokeCommandRequest(exchangeMgr, sessionHandle, endpointId, requestCommandData, onSuccessCb, onErrorCb, + timedInvokeTimeoutMs); +} + +template = 0> +CHIP_ERROR +InvokeCommandRequest(Messaging::ExchangeManager * exchangeMgr, SessionHandle sessionHandle, chip::EndpointId endpointId, + const RequestObjectT & requestCommandData, + typename TypedCommandCallback::OnSuccessCallbackType onSuccessCb, + typename TypedCommandCallback::OnErrorCallbackType onErrorCb) +{ + return InvokeCommandRequest(exchangeMgr, sessionHandle, endpointId, requestCommandData, onSuccessCb, onErrorCb, NullOptional); +} + } // namespace Controller } // namespace chip diff --git a/src/controller/java/templates/CHIPClusters-JNI.zapt b/src/controller/java/templates/CHIPClusters-JNI.zapt index 60463b0f244fb1..b82200a2a089c1 100644 --- a/src/controller/java/templates/CHIPClusters-JNI.zapt +++ b/src/controller/java/templates/CHIPClusters-JNI.zapt @@ -69,7 +69,12 @@ JNI_METHOD(void, {{asUpperCamelCase ../name}}Cluster, {{asLowerCamelCase name}}) auto successFn = chip::Callback::CallbackcallbackName}}CallbackType>::FromCancelable(onSuccess->Cancel()); auto failureFn = chip::Callback::Callback::FromCancelable(onFailure->Cancel()); - err = cppCluster->InvokeCommand(request, onSuccess->mContext, successFn->mCall, failureFn->mCall); + err = cppCluster->InvokeCommand(request, onSuccess->mContext, successFn->mCall, failureFn->mCall + {{#if mustUseTimedInvoke}} + {{!TODO Fix Java API to pass in this information. For now, 10 seconds.}} + , 10000 + {{/if}} + ); VerifyOrReturn(err == CHIP_NO_ERROR, AndroidClusterExceptions::GetInstance().ReturnIllegalStateException(env, callback, "Error invoking command", CHIP_ERROR_INCORRECT_STATE)); onSuccess.release(); diff --git a/src/controller/java/zap-generated/CHIPClusters-JNI.cpp b/src/controller/java/zap-generated/CHIPClusters-JNI.cpp index cc1aeb019c1f8d..2d42fb1b092f0e 100644 --- a/src/controller/java/zap-generated/CHIPClusters-JNI.cpp +++ b/src/controller/java/zap-generated/CHIPClusters-JNI.cpp @@ -24875,7 +24875,7 @@ JNI_METHOD(void, TestClusterCluster, timedInvokeRequest)(JNIEnv * env, jobject s auto successFn = chip::Callback::Callback::FromCancelable(onSuccess->Cancel()); auto failureFn = chip::Callback::Callback::FromCancelable(onFailure->Cancel()); - err = cppCluster->InvokeCommand(request, onSuccess->mContext, successFn->mCall, failureFn->mCall); + err = cppCluster->InvokeCommand(request, onSuccess->mContext, successFn->mCall, failureFn->mCall, 10000); VerifyOrReturn(err == CHIP_NO_ERROR, AndroidClusterExceptions::GetInstance().ReturnIllegalStateException(env, callback, "Error invoking command", CHIP_ERROR_INCORRECT_STATE)); diff --git a/src/darwin/Framework/CHIP/templates/CHIPClustersObjc-src.zapt b/src/darwin/Framework/CHIP/templates/CHIPClustersObjc-src.zapt index 9ffdade3e8569a..452eec55f3bd22 100644 --- a/src/darwin/Framework/CHIP/templates/CHIPClustersObjc-src.zapt +++ b/src/darwin/Framework/CHIP/templates/CHIPClustersObjc-src.zapt @@ -71,7 +71,12 @@ using namespace chip::app::Clusters; ^(Cancelable * success, Cancelable * failure) { auto successFn = CallbackcallbackName}}CallbackType>::FromCancelable(success); auto failureFn = Callback::FromCancelable(failure); - return self.cppCluster.InvokeCommand(request, successFn->mContext, successFn->mCall, failureFn->mCall); + return self.cppCluster.InvokeCommand(request, successFn->mContext, successFn->mCall, failureFn->mCall + {{#if mustUseTimedInvoke}} + {{!TODO Fix Darwin API to pass in this information. For now, 10 seconds.}} + , 10000 + {{/if}} + ); }); } {{/chip_cluster_commands}} diff --git a/src/darwin/Framework/CHIP/zap-generated/CHIPClustersObjc.mm b/src/darwin/Framework/CHIP/zap-generated/CHIPClustersObjc.mm index e19fe0337c8027..ca164dec92a249 100644 --- a/src/darwin/Framework/CHIP/zap-generated/CHIPClustersObjc.mm +++ b/src/darwin/Framework/CHIP/zap-generated/CHIPClustersObjc.mm @@ -16974,7 +16974,7 @@ new CHIPCommandSuccessCallbackBridge( ^(Cancelable * success, Cancelable * failure) { auto successFn = Callback::FromCancelable(success); auto failureFn = Callback::FromCancelable(failure); - return self.cppCluster.InvokeCommand(request, successFn->mContext, successFn->mCall, failureFn->mCall); + return self.cppCluster.InvokeCommand(request, successFn->mContext, successFn->mCall, failureFn->mCall, 10000); }); } diff --git a/zzz_generated/chip-tool/zap-generated/cluster/Commands.h b/zzz_generated/chip-tool/zap-generated/cluster/Commands.h index 9a800a5409d3c2..4b7f00765515d1 100644 --- a/zzz_generated/chip-tool/zap-generated/cluster/Commands.h +++ b/zzz_generated/chip-tool/zap-generated/cluster/Commands.h @@ -36870,7 +36870,7 @@ class TestClusterTimedInvokeRequest : public ModelCommand { ChipLogProgress(chipTool, "Sending cluster (0x0000050F) command (0x00000012) on endpoint %" PRIu8, endpointId); - return chip::Controller::InvokeCommand(device, this, OnDefaultSuccess, OnDefaultFailure, endpointId, mRequest); + return chip::Controller::InvokeCommand(device, this, OnDefaultSuccess, OnDefaultFailure, endpointId, mRequest, 10000); } private: diff --git a/zzz_generated/chip-tool/zap-generated/test/Commands.h b/zzz_generated/chip-tool/zap-generated/test/Commands.h index 928ed6f5b590db..15ea446adf8130 100644 --- a/zzz_generated/chip-tool/zap-generated/test/Commands.h +++ b/zzz_generated/chip-tool/zap-generated/test/Commands.h @@ -21,6 +21,9 @@ #include #include +#include +#include + #include // For INFINITY class TestList : public Command @@ -41977,6 +41980,28 @@ class TestClusterComplexTypes : public TestCommand ChipLogProgress(chipTool, " ***** Test Step 1 : Send Test Command with optional arg set to null.\n"); err = TestSendTestCommandWithOptionalArgSetToNull_1(); break; + case 2: + ChipLogProgress(chipTool, " ***** Test Step 2 : Send command that needs timed invoke without a timeout value\n"); + err = TestSendCommandThatNeedsTimedInvokeWithoutATimeoutValue_2(); + break; + case 3: + ChipLogProgress(chipTool, " ***** Test Step 3 : Send command that needs timed invoke with a long timeout value\n"); + err = TestSendCommandThatNeedsTimedInvokeWithALongTimeoutValue_3(); + break; + case 4: + ChipLogProgress(chipTool, " ***** Test Step 4 : Send command that needs timed invoke with a too-short timeout value\n"); + err = TestSendCommandThatNeedsTimedInvokeWithATooShortTimeoutValue_4(); + break; + case 5: + ChipLogProgress(chipTool, + " ***** Test Step 5 : Send command that does not need timed invoke with a long timeout value\n"); + err = TestSendCommandThatDoesNotNeedTimedInvokeWithALongTimeoutValue_5(); + break; + case 6: + ChipLogProgress(chipTool, + " ***** Test Step 6 : Send command that does not need timed invoke with a too-short timeout value\n"); + err = TestSendCommandThatDoesNotNeedTimedInvokeWithATooShortTimeoutValue_6(); + break; } if (CHIP_NO_ERROR != err) @@ -41988,7 +42013,7 @@ class TestClusterComplexTypes : public TestCommand private: std::atomic_uint16_t mTestIndex; - const uint16_t mTestCount = 2; + const uint16_t mTestCount = 7; // // Tests methods @@ -42036,6 +42061,154 @@ class TestClusterComplexTypes : public TestCommand NextTest(); } + + CHIP_ERROR TestSendCommandThatNeedsTimedInvokeWithoutATimeoutValue_2() + { + const chip::EndpointId endpoint = mEndpointId.HasValue() ? mEndpointId.Value() : 1; + using RequestType = chip::app::Clusters::TestCluster::Commands::TimedInvokeRequest::Type; + + RequestType request; + + auto success = [](void * context, const typename RequestType::ResponseType & data) { + (static_cast(context))->OnSuccessResponse_2(); + }; + + auto failure = [](void * context, EmberAfStatus status) { + (static_cast(context))->OnFailureResponse_2(status); + }; + + ReturnErrorOnFailure(chip::Controller::InvokeCommand(mDevices[kIdentityAlpha], this, success, failure, endpoint, request, + chip::NullOptional)); + return CHIP_NO_ERROR; + } + + void OnFailureResponse_2(uint8_t status) + { + VerifyOrReturn(CheckValue("status", status, EMBER_ZCL_STATUS_NEEDS_TIMED_INTERACTION)); + NextTest(); + } + + void OnSuccessResponse_2() { ThrowSuccessResponse(); } + + CHIP_ERROR TestSendCommandThatNeedsTimedInvokeWithALongTimeoutValue_3() + { + const chip::EndpointId endpoint = mEndpointId.HasValue() ? mEndpointId.Value() : 1; + using RequestType = chip::app::Clusters::TestCluster::Commands::TimedInvokeRequest::Type; + + RequestType request; + + auto success = [](void * context, const typename RequestType::ResponseType & data) { + (static_cast(context))->OnSuccessResponse_3(); + }; + + auto failure = [](void * context, EmberAfStatus status) { + (static_cast(context))->OnFailureResponse_3(status); + }; + + ReturnErrorOnFailure( + chip::Controller::InvokeCommand(mDevices[kIdentityAlpha], this, success, failure, endpoint, request, 10000)); + return CHIP_NO_ERROR; + } + + void OnFailureResponse_3(uint8_t status) { ThrowFailureResponse(); } + + void OnSuccessResponse_3() { NextTest(); } + + CHIP_ERROR TestSendCommandThatNeedsTimedInvokeWithATooShortTimeoutValue_4() + { + const chip::EndpointId endpoint = mEndpointId.HasValue() ? mEndpointId.Value() : 1; + using RequestType = chip::app::Clusters::TestCluster::Commands::TimedInvokeRequest::Type; + + RequestType request; + + auto success = [](void * context, const typename RequestType::ResponseType & data) { + (static_cast(context))->OnSuccessResponse_4(); + }; + + auto failure = [](void * context, EmberAfStatus status) { + (static_cast(context))->OnFailureResponse_4(status); + }; + + ReturnErrorOnFailure( + chip::Controller::InvokeCommand(mDevices[kIdentityAlpha], this, success, failure, endpoint, request, 1)); + { + using namespace chip::System::Clock::Literals; + // Busy-wait for 5 milliseconds. + auto & clock = chip::System::SystemClock(); + auto start = clock.GetMonotonicTimestamp(); + while (clock.GetMonotonicTimestamp() - start < 5_ms) + ; + } + return CHIP_NO_ERROR; + } + + void OnFailureResponse_4(uint8_t status) + { + VerifyOrReturn(CheckValue("status", status, EMBER_ZCL_STATUS_UNSUPPORTED_ACCESS)); + NextTest(); + } + + void OnSuccessResponse_4() { ThrowSuccessResponse(); } + + CHIP_ERROR TestSendCommandThatDoesNotNeedTimedInvokeWithALongTimeoutValue_5() + { + const chip::EndpointId endpoint = mEndpointId.HasValue() ? mEndpointId.Value() : 1; + using RequestType = chip::app::Clusters::TestCluster::Commands::Test::Type; + + RequestType request; + + auto success = [](void * context, const typename RequestType::ResponseType & data) { + (static_cast(context))->OnSuccessResponse_5(); + }; + + auto failure = [](void * context, EmberAfStatus status) { + (static_cast(context))->OnFailureResponse_5(status); + }; + + ReturnErrorOnFailure( + chip::Controller::InvokeCommand(mDevices[kIdentityAlpha], this, success, failure, endpoint, request, 10000)); + return CHIP_NO_ERROR; + } + + void OnFailureResponse_5(uint8_t status) { ThrowFailureResponse(); } + + void OnSuccessResponse_5() { NextTest(); } + + CHIP_ERROR TestSendCommandThatDoesNotNeedTimedInvokeWithATooShortTimeoutValue_6() + { + const chip::EndpointId endpoint = mEndpointId.HasValue() ? mEndpointId.Value() : 1; + using RequestType = chip::app::Clusters::TestCluster::Commands::Test::Type; + + RequestType request; + + auto success = [](void * context, const typename RequestType::ResponseType & data) { + (static_cast(context))->OnSuccessResponse_6(); + }; + + auto failure = [](void * context, EmberAfStatus status) { + (static_cast(context))->OnFailureResponse_6(status); + }; + + ReturnErrorOnFailure( + chip::Controller::InvokeCommand(mDevices[kIdentityAlpha], this, success, failure, endpoint, request, 1)); + { + using namespace chip::System::Clock::Literals; + // Busy-wait for 5 milliseconds. + auto & clock = chip::System::SystemClock(); + auto start = clock.GetMonotonicTimestamp(); + while (clock.GetMonotonicTimestamp() - start < 5_ms) + ; + } + return CHIP_NO_ERROR; + } + + void OnFailureResponse_6(uint8_t status) + { + VerifyOrReturn(CheckValue("status", status, EMBER_ZCL_STATUS_UNSUPPORTED_ACCESS)); + NextTest(); + } + + void OnSuccessResponse_6() { ThrowSuccessResponse(); } }; class TestConstraints : public TestCommand