diff --git a/src/app/CommandHandler.cpp b/src/app/CommandHandler.cpp index 8fd13bab7247f0..1b4f8ea92998ba 100644 --- a/src/app/CommandHandler.cpp +++ b/src/app/CommandHandler.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -44,6 +45,13 @@ using Status = Protocols::InteractionModel::Status; CommandHandler::CommandHandler(Callback * apCallback) : mExchangeCtx(*this), mpCallback(apCallback), mSuppressResponse(false) {} +CommandHandler::CommandHandler(TestOnlyMarker aTestMarker, Callback * apCallback, CommandPathRegistry * apCommandPathRegistry) : + CommandHandler(apCallback) +{ + mMaxPathsPerInvoke = apCommandPathRegistry->MaxSize(); + mCommandPathRegistry = apCommandPathRegistry; +} + CHIP_ERROR CommandHandler::AllocateBuffer() { if (!mBufferAllocated) @@ -97,11 +105,74 @@ void CommandHandler::OnInvokeCommandRequest(Messaging::ExchangeContext * ec, con mGoneAsync = true; } +CHIP_ERROR CommandHandler::ValidateInvokeRequestsAndBuildRegistry(TLV::TLVReader & invokeRequestsReader) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + size_t commandCount = 0; + bool commandRefExpected = false; + + ReturnErrorOnFailure(TLV::Utilities::Count(invokeRequestsReader, commandCount, false /* recurse */)); + + // If this is a GroupRequest the only thing to check is that there is only one + // CommandDataIB. + if (IsGroupRequest()) + { + VerifyOrReturnError(commandCount == 1, CHIP_ERROR_INVALID_ARGUMENT); + return CHIP_NO_ERROR; + } + // While technically any commandCount == 1 should already be unique and does not need + // any further validation, we do need to read and populate the registry to help + // in building the InvokeResponse. + + VerifyOrReturnError(commandCount <= MaxPathsPerInvoke(), CHIP_ERROR_INVALID_ARGUMENT); + + // If there is more than one CommandDataIB, spec states that CommandRef must be provided. + commandRefExpected = commandCount > 1; + + while (CHIP_NO_ERROR == (err = invokeRequestsReader.Next())) + { + VerifyOrReturnError(TLV::AnonymousTag() == invokeRequestsReader.GetTag(), CHIP_ERROR_INVALID_ARGUMENT); + CommandDataIB::Parser commandData; + ReturnErrorOnFailure(commandData.Init(invokeRequestsReader)); + + // First validate that we can get a ConcreteCommandPath. + CommandPathIB::Parser commandPath; + ConcreteCommandPath concretePath(0, 0, 0); + ReturnErrorOnFailure(commandData.GetPath(&commandPath)); + ReturnErrorOnFailure(commandPath.GetConcreteCommandPath(concretePath)); + + // Grab the CommandRef if there is one, and validate that it's there when it + // has to be. + Optional commandRef; + uint16_t ref; + err = commandData.GetRef(&ref); + VerifyOrReturnError(err == CHIP_NO_ERROR || err == CHIP_END_OF_TLV, err); + if (err == CHIP_END_OF_TLV && commandRefExpected) + { + return CHIP_ERROR_INVALID_ARGUMENT; + } + if (err == CHIP_NO_ERROR) + { + commandRef.SetValue(ref); + } + + // Adding can fail if concretePath is not unique, or if commandRef is a value + // and is not unique, or if we have already added more paths than we support. + ReturnErrorOnFailure(GetCommandPathRegistry().Add(concretePath, commandRef)); + } + + // It's OK/expected to have reached the end of the container without failure. + if (CHIP_END_OF_TLV == err) + { + err = CHIP_NO_ERROR; + } + return err; +} + Status CommandHandler::ProcessInvokeRequest(System::PacketBufferHandle && payload, bool isTimedInvoke) { CHIP_ERROR err = CHIP_NO_ERROR; System::PacketBufferTLVReader reader; - TLV::TLVReader invokeRequestsReader; InvokeRequestMessage::Parser invokeRequestMessage; InvokeRequests::Parser invokeRequests; reader.Init(std::move(payload)); @@ -109,28 +180,33 @@ Status CommandHandler::ProcessInvokeRequest(System::PacketBufferHandle && payloa #if CHIP_CONFIG_IM_PRETTY_PRINT invokeRequestMessage.PrettyPrint(); #endif + if (mExchangeCtx->IsGroupExchangeContext()) + { + SetGroupRequest(true); + } VerifyOrReturnError(invokeRequestMessage.GetSuppressResponse(&mSuppressResponse) == CHIP_NO_ERROR, Status::InvalidAction); VerifyOrReturnError(invokeRequestMessage.GetTimedRequest(&mTimedRequest) == CHIP_NO_ERROR, Status::InvalidAction); VerifyOrReturnError(invokeRequestMessage.GetInvokeRequests(&invokeRequests) == CHIP_NO_ERROR, Status::InvalidAction); VerifyOrReturnError(mTimedRequest == isTimedInvoke, Status::UnsupportedAccess); - invokeRequests.GetReader(&invokeRequestsReader); { - // We don't support handling multiple commands but the protocol is ready to support it in the future, reject all of them and - // IM Engine will send a status response. - size_t commandCount = 0; - TLV::Utilities::Count(invokeRequestsReader, commandCount, false /* recurse */); - VerifyOrReturnError(commandCount == 1, Status::InvalidAction); + TLV::TLVReader validationInvokeRequestsReader; + invokeRequests.GetReader(&validationInvokeRequestsReader); + VerifyOrReturnError(ValidateInvokeRequestsAndBuildRegistry(validationInvokeRequestsReader) == CHIP_NO_ERROR, + Status::InvalidAction); } + TLV::TLVReader invokeRequestsReader; + invokeRequests.GetReader(&invokeRequestsReader); + while (CHIP_NO_ERROR == (err = invokeRequestsReader.Next())) { VerifyOrReturnError(TLV::AnonymousTag() == invokeRequestsReader.GetTag(), Status::InvalidAction); CommandDataIB::Parser commandData; VerifyOrReturnError(commandData.Init(invokeRequestsReader) == CHIP_NO_ERROR, Status::InvalidAction); Status status = Status::Success; - if (mExchangeCtx->IsGroupExchangeContext()) + if (IsGroupRequest()) { status = ProcessGroupCommandDataIB(commandData); } @@ -200,7 +276,7 @@ void CommandHandler::DecrementHoldOff() { ChipLogProgress(DataManagement, "Skipping command response: exchange context is null"); } - else if (!mExchangeCtx->IsGroupExchangeContext()) + else if (!IsGroupRequest()) { CHIP_ERROR err = SendCommandResponse(); if (err != CHIP_NO_ERROR) @@ -251,8 +327,6 @@ Status CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommandElem ConcreteCommandPath concretePath(0, 0, 0); TLV::TLVReader commandDataReader; - SetGroupRequest(false); - // NOTE: errors may occur before the concrete command path is even fully decoded. err = aCommandElement.GetPath(&commandPath); @@ -354,7 +428,6 @@ Status CommandHandler::ProcessGroupCommandDataIB(CommandDataIB::Parser & aComman Credentials::GroupDataProvider::GroupEndpoint mapping; Credentials::GroupDataProvider * groupDataProvider = Credentials::GetGroupDataProvider(); Credentials::GroupDataProvider::EndpointIterator * iterator; - SetGroupRequest(true); err = aCommandElement.GetPath(&commandPath); VerifyOrReturnError(err == CHIP_NO_ERROR, Status::InvalidAction); @@ -465,7 +538,7 @@ CHIP_ERROR CommandHandler::AddStatusInternal(const ConcreteCommandPath & aComman void CommandHandler::AddStatus(const ConcreteCommandPath & aCommandPath, const Protocols::InteractionModel::Status aStatus, const char * context) { - // Return prematurely in case of requests targeted to a group that should not add the status for response purposes. + // Return early in case of requests targeted to a group, since they should not add a response. VerifyOrReturn(!IsGroupRequest()); VerifyOrDie(FallibleAddStatus(aCommandPath, aStatus, context) == CHIP_NO_ERROR); } @@ -500,15 +573,51 @@ CHIP_ERROR CommandHandler::AddClusterSpecificFailure(const ConcreteCommandPath & return AddStatusInternal(aCommandPath, StatusIB(Status::Failure, aClusterStatus)); } -CHIP_ERROR CommandHandler::PrepareCommand(const ConcreteCommandPath & aCommandPath, bool aStartDataStruct) +CHIP_ERROR CommandHandler::PrepareInvokeResponseCommand(const ConcreteCommandPath & aResponseCommandPath, + const CommandHandler::InvokeResponseParameters & aPrepareParameters) +{ + auto commandPathRegistryEntry = GetCommandPathRegistry().Find(aPrepareParameters.mRequestCommandPath); + VerifyOrReturnValue(commandPathRegistryEntry.HasValue(), CHIP_ERROR_INCORRECT_STATE); + + return PrepareInvokeResponseCommand(commandPathRegistryEntry.Value(), aResponseCommandPath, + aPrepareParameters.mStartOrEndDataStruct); +} + +CHIP_ERROR CommandHandler::PrepareCommand(const ConcreteCommandPath & aResponseCommandPath, bool aStartDataStruct) +{ + // Legacy code is calling the deprecated version of PrepareCommand. If we are in a case where + // there was a single command in the request, we can just assume this response is triggered by + // the single command. + size_t countOfPathRegistryEntries = GetCommandPathRegistry().Count(); + + // At this point application supports Batch Invoke Commands since CommandPathRegistry has more than 1 entry, + // but application is calling the deprecated PrepareCommand. We have no way to determine the associated CommandRef + // to put into the InvokeResponse. + VerifyOrDieWithMsg(countOfPathRegistryEntries == 1, DataManagement, + "Seemingly device supports batch commands, but is calling the deprecated PrepareCommand API"); + + auto commandPathRegistryEntry = GetCommandPathRegistry().GetFirstEntry(); + VerifyOrReturnValue(commandPathRegistryEntry.HasValue(), CHIP_ERROR_INCORRECT_STATE); + + return PrepareInvokeResponseCommand(commandPathRegistryEntry.Value(), aResponseCommandPath, aStartDataStruct); +} + +CHIP_ERROR CommandHandler::PrepareInvokeResponseCommand(const CommandPathRegistryEntry & apCommandPathRegistryEntry, + const ConcreteCommandPath & aCommandPath, bool aStartDataStruct) { ReturnErrorOnFailure(AllocateBuffer()); mInvokeResponseBuilder.Checkpoint(mBackupWriter); + mBackupState = mState; // // We must not be in the middle of preparing a command, or having prepared or sent one. // - VerifyOrReturnError(mState == State::Idle, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(mState == State::Idle || mState == State::AddedCommand, CHIP_ERROR_INCORRECT_STATE); + + // TODO(#30453): See if we can pass this back up the stack so caller can provide this instead of taking up + // space in CommandHanlder. + mRefForResponse = apCommandPathRegistryEntry.ref; + MoveToState(State::Preparing); InvokeResponseIBs::Builder & invokeResponses = mInvokeResponseBuilder.GetInvokeResponses(); InvokeResponseIB::Builder & invokeResponse = invokeResponses.CreateInvokeResponse(); @@ -536,10 +645,14 @@ CHIP_ERROR CommandHandler::FinishCommand(bool aStartDataStruct) { ReturnErrorOnFailure(commandData.GetWriter()->EndContainer(mDataElementContainerType)); } + + if (mRefForResponse.HasValue()) + { + ReturnErrorOnFailure(commandData.Ref(mRefForResponse.Value())); + } + ReturnErrorOnFailure(commandData.EndOfCommandDataIB()); ReturnErrorOnFailure(mInvokeResponseBuilder.GetInvokeResponses().GetInvokeResponse().EndOfInvokeResponseIB()); - ReturnErrorOnFailure(mInvokeResponseBuilder.GetInvokeResponses().EndOfInvokeResponses()); - ReturnErrorOnFailure(mInvokeResponseBuilder.EndOfInvokeResponseMessage()); MoveToState(State::AddedCommand); return CHIP_NO_ERROR; } @@ -550,7 +663,12 @@ CHIP_ERROR CommandHandler::PrepareStatus(const ConcreteCommandPath & aCommandPat // // We must not be in the middle of preparing a command, or having prepared or sent one. // - VerifyOrReturnError(mState == State::Idle, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(mState == State::Idle || mState == State::AddedCommand, CHIP_ERROR_INCORRECT_STATE); + + auto commandPathRegistryEntry = GetCommandPathRegistry().Find(aCommandPath); + VerifyOrReturnError(commandPathRegistryEntry.HasValue(), CHIP_ERROR_INCORRECT_STATE); + mRefForResponse = commandPathRegistryEntry.Value().ref; + MoveToState(State::Preparing); InvokeResponseIBs::Builder & invokeResponses = mInvokeResponseBuilder.GetInvokeResponses(); InvokeResponseIB::Builder & invokeResponse = invokeResponses.CreateInvokeResponse(); @@ -567,10 +685,15 @@ CHIP_ERROR CommandHandler::PrepareStatus(const ConcreteCommandPath & aCommandPat CHIP_ERROR CommandHandler::FinishStatus() { VerifyOrReturnError(mState == State::AddingCommand, CHIP_ERROR_INCORRECT_STATE); + + CommandStatusIB::Builder & commandStatus = mInvokeResponseBuilder.GetInvokeResponses().GetInvokeResponse().GetStatus(); + if (mRefForResponse.HasValue()) + { + ReturnErrorOnFailure(commandStatus.Ref(mRefForResponse.Value())); + } + ReturnErrorOnFailure(mInvokeResponseBuilder.GetInvokeResponses().GetInvokeResponse().GetStatus().EndOfCommandStatusIB()); ReturnErrorOnFailure(mInvokeResponseBuilder.GetInvokeResponses().GetInvokeResponse().EndOfInvokeResponseIB()); - ReturnErrorOnFailure(mInvokeResponseBuilder.GetInvokeResponses().EndOfInvokeResponses()); - ReturnErrorOnFailure(mInvokeResponseBuilder.EndOfInvokeResponseMessage()); MoveToState(State::AddedCommand); return CHIP_NO_ERROR; } @@ -579,9 +702,7 @@ CHIP_ERROR CommandHandler::RollbackResponse() { VerifyOrReturnError(mState == State::Preparing || mState == State::AddingCommand, CHIP_ERROR_INCORRECT_STATE); mInvokeResponseBuilder.Rollback(mBackupWriter); - // Note: We only support one command per request, so we reset the state to Idle here, need to review the states when adding - // supports of having multiple requests in the same transaction. - MoveToState(State::Idle); + MoveToState(mBackupState); return CHIP_NO_ERROR; } @@ -635,6 +756,8 @@ CommandHandler::Handle::Handle(CommandHandler * handle) CHIP_ERROR CommandHandler::Finalize(System::PacketBufferHandle & commandPacket) { VerifyOrReturnError(mState == State::AddedCommand, CHIP_ERROR_INCORRECT_STATE); + ReturnErrorOnFailure(mInvokeResponseBuilder.GetInvokeResponses().EndOfInvokeResponses()); + ReturnErrorOnFailure(mInvokeResponseBuilder.EndOfInvokeResponseMessage()); return mCommandMessageWriter.Finalize(&commandPacket); } diff --git a/src/app/CommandHandler.h b/src/app/CommandHandler.h index 65542ef0587f55..037e2c5619f50e 100644 --- a/src/app/CommandHandler.h +++ b/src/app/CommandHandler.h @@ -30,6 +30,8 @@ #pragma once +#include "CommandPathRegistry.h" + #include #include #include @@ -151,6 +153,32 @@ class CommandHandler : public Messaging::ExchangeDelegate uint32_t mMagic = 0; }; + // Previously we kept adding arguments with default values individually as parameters. This is because there + // is legacy code outside of the SDK that would call PrepareCommand. With the new PrepareInvokeResponseCommand + // replacing PrepareCommand, we took this opportunity to create a new parameter structure to make it easier to + // add new parameters without there needing to be an ever increasing parameter list with defaults. + struct InvokeResponseParameters + { + InvokeResponseParameters(ConcreteCommandPath aRequestCommandPath) : mRequestCommandPath(aRequestCommandPath) {} + + InvokeResponseParameters & SetStartOrEndDataStruct(bool aStartOrEndDataStruct) + { + mStartOrEndDataStruct = aStartOrEndDataStruct; + return *this; + } + + ConcreteCommandPath mRequestCommandPath; + /** + * Whether the method this is being provided to should start/end the TLV container for the CommandFields element + * within CommandDataIB. + */ + bool mStartOrEndDataStruct = true; + }; + + class TestOnlyMarker + { + }; + /* * Constructor. * @@ -158,6 +186,14 @@ class CommandHandler : public Messaging::ExchangeDelegate */ CommandHandler(Callback * apCallback); + /* + * Constructor to override number of supported paths per invoke. + * + * The callback and command path registry passed in has to outlive this CommandHandler object. + * For testing purposes. + */ + CommandHandler(TestOnlyMarker aTestMarker, Callback * apCallback, CommandPathRegistry * apCommandPathRegistry); + /* * Main entrypoint for this class to handle an invoke request. * @@ -171,6 +207,15 @@ class CommandHandler : public Messaging::ExchangeDelegate void OnInvokeCommandRequest(Messaging::ExchangeContext * ec, const PayloadHeader & payloadHeader, System::PacketBufferHandle && payload, bool isTimedInvoke); + /** + * Checks that all CommandDataIB within InvokeRequests satisfy the spec's general + * constraints for CommandDataIB. + * + * This also builds a registry that to ensure that all commands can be responded + * to with the data required as per spec. + */ + CHIP_ERROR ValidateInvokeRequestsAndBuildRegistry(TLV::TLVReader & invokeRequestsReader); + /** * Adds the given command status and returns any failures in adding statuses (e.g. out * of buffer space) to the caller @@ -190,9 +235,77 @@ class CommandHandler : public Messaging::ExchangeDelegate CHIP_ERROR AddClusterSpecificFailure(const ConcreteCommandPath & aCommandPath, ClusterStatus aClusterStatus); Protocols::InteractionModel::Status ProcessInvokeRequest(System::PacketBufferHandle && payload, bool isTimedInvoke); - CHIP_ERROR PrepareCommand(const ConcreteCommandPath & aCommandPath, bool aStartDataStruct = true); + + /** + * This adds a new CommandDataIB element into InvokeResponses for the associated + * aRequestCommandPath. This adds up until the `CommandFields` element within + * `CommandDataIB`. + * + * This call will fail if CommandHandler is already in the middle of building a + * CommandStatusIB or CommandDataIB (i.e. something has called Prepare*, without + * calling Finish*), or is already sending InvokeResponseMessage. + * + * Upon success, the caller is expected to call `FinishCommand` once they have added + * all the fields into the CommandFields element of CommandDataIB. + * + * @param [in] aResponseCommandPath the concrete response path that we are sending to Requester. + * @param [in] aPrepareParameters struct containing paramters needs for preparing a command. Data + * such as request path, and whether this method should start the CommandFields element within + * CommandDataIB. + */ + CHIP_ERROR PrepareInvokeResponseCommand(const ConcreteCommandPath & aResponseCommandPath, + const InvokeResponseParameters & aPrepareParameters); + + [[deprecated("PrepareCommand now needs the requested command path. Please use PrepareInvokeResponseCommand")]] CHIP_ERROR + PrepareCommand(const ConcreteCommandPath & aCommandPath, bool aStartDataStruct = true); + + /** + * Finishes the CommandDataIB element within the InvokeResponses. + * + * Caller must have first successfully called `PrepareInvokeResponseCommand`. + * + * @param [in] aEndDataStruct end the TLV container for the CommandFields element within + * CommandDataIB. This should match the boolean passed into Prepare*. + * + * @return CHIP_ERROR_INCORRECT_STATE + * If device has not previously successfully called + * `PrepareInvokeResponseCommand`. + * @return CHIP_ERROR_BUFFER_TOO_SMALL + * If writing the values needed to finish the InvokeReponseIB + * with the current contents of the InvokeResponseMessage + * would exceed the limit. When this error occurs, it is possible + * we have already closed some of the IB Builders that were + * previously started in `PrepareInvokeResponseCommand`. + * @return CHIP_ERROR_NO_MEMORY + * If TLVWriter attempted to allocate an output buffer failed due to + * lack of memory. + * @return other Other TLVWriter related errors. Typically occurs if + * `GetCommandDataIBTLVWriter()` was called and used incorrectly. + */ + // TODO(#30453): We should be able to eliminate the chances of OOM issues with reserve. + // This will be completed in a follow up PR. CHIP_ERROR FinishCommand(bool aEndDataStruct = true); + + /** + * This will add a new CommandStatusIB element into InvokeResponses. It will put the + * aCommandPath into the CommandPath element within CommandStatusIB. + * + * This call will fail if CommandHandler is already in the middle of building a + * CommandStatusIB or CommandDataIB (i.e. something has called Prepare*, without + * calling Finish*), or is already sending InvokeResponseMessage. + * + * Upon success, the caller is expected to call `FinishStatus` once they have encoded + * StatusIB. + * + * @param [in] aCommandPath the concrete path of the command we are responding to. + */ CHIP_ERROR PrepareStatus(const ConcreteCommandPath & aCommandPath); + + /** + * Finishes the CommandStatusIB element within the InvokeResponses. + * + * Caller must have first successfully called `PrepareStatus`. + */ CHIP_ERROR FinishStatus(); TLV::TLVWriter * GetCommandDataIBTLVWriter(); @@ -315,7 +428,7 @@ class CommandHandler : public Messaging::ExchangeDelegate VerifyOrDie(false); } - enum class State + enum class State : uint8_t { Idle, ///< Default state that the object starts out in, where no work has commenced Preparing, ///< We are prepaing the command or status header. @@ -363,6 +476,9 @@ class CommandHandler : public Messaging::ExchangeDelegate */ CHIP_ERROR AllocateBuffer(); + CHIP_ERROR PrepareInvokeResponseCommand(const CommandPathRegistryEntry & apCommandPathRegistryEntry, + const ConcreteCommandPath & aCommandPath, bool aStartDataStruct); + CHIP_ERROR Finalize(System::PacketBufferHandle & commandPacket); /** @@ -397,8 +513,15 @@ class CommandHandler : public Messaging::ExchangeDelegate template CHIP_ERROR TryAddResponseData(const ConcreteCommandPath & aRequestCommandPath, const CommandData & aData) { - ConcreteCommandPath path = { aRequestCommandPath.mEndpointId, aRequestCommandPath.mClusterId, CommandData::GetCommandId() }; - ReturnErrorOnFailure(PrepareCommand(path, false)); + // Return early in case of requests targeted to a group, since they should not add a response. + VerifyOrReturnValue(!IsGroupRequest(), CHIP_NO_ERROR); + + InvokeResponseParameters prepareParams(aRequestCommandPath); + prepareParams.SetStartOrEndDataStruct(false); + + ConcreteCommandPath responsePath = { aRequestCommandPath.mEndpointId, aRequestCommandPath.mClusterId, + CommandData::GetCommandId() }; + ReturnErrorOnFailure(PrepareInvokeResponseCommand(responsePath, prepareParams)); TLV::TLVWriter * writer = GetCommandDataIBTLVWriter(); VerifyOrReturnError(writer != nullptr, CHIP_ERROR_INCORRECT_STATE); ReturnErrorOnFailure(DataModel::Encode(*writer, TLV::ContextTag(CommandDataIB::Tag::kFields), aData)); @@ -416,21 +539,32 @@ class CommandHandler : public Messaging::ExchangeDelegate */ void SetGroupRequest(bool isGroupRequest) { mGroupRequest = isGroupRequest; } + CommandPathRegistry & GetCommandPathRegistry() const { return *mCommandPathRegistry; } + + size_t MaxPathsPerInvoke() const { return mMaxPathsPerInvoke; } + Messaging::ExchangeHolder mExchangeCtx; Callback * mpCallback = nullptr; InvokeResponseMessage::Builder mInvokeResponseBuilder; TLV::TLVType mDataElementContainerType = TLV::kTLVType_NotSpecified; size_t mPendingWork = 0; - bool mSuppressResponse = false; - bool mTimedRequest = false; - bool mSentStatusResponse = false; - - State mState = State::Idle; - bool mGroupRequest = false; chip::System::PacketBufferTLVWriter mCommandMessageWriter; TLV::TLVWriter mBackupWriter; - bool mBufferAllocated = false; + size_t mMaxPathsPerInvoke = CHIP_CONFIG_MAX_PATHS_PER_INVOKE; + // TODO(#30453): See if we can reduce this size for the default cases + // TODO Allow flexibility in registration. + BasicCommandPathRegistry mBasicCommandPathRegistry; + CommandPathRegistry * mCommandPathRegistry = &mBasicCommandPathRegistry; + Optional mRefForResponse; + + State mState = State::Idle; + State mBackupState; + bool mSuppressResponse = false; + bool mTimedRequest = false; + bool mSentStatusResponse = false; + bool mGroupRequest = false; + bool mBufferAllocated = false; // If mGoneAsync is true, we have finished out initial processing of the // incoming invoke. After this point, our session could go away at any // time. diff --git a/src/app/CommandPathRegistry.h b/src/app/CommandPathRegistry.h new file mode 100644 index 00000000000000..0d914ef1de3348 --- /dev/null +++ b/src/app/CommandPathRegistry.h @@ -0,0 +1,118 @@ +/* + * + * Copyright (c) 2023 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include + +#include +#include +#include + +namespace chip { +namespace app { + +struct CommandPathRegistryEntry +{ + ConcreteCommandPath requestPath = ConcreteCommandPath(0, 0, 0); + Optional ref; +}; + +class CommandPathRegistry +{ +public: + virtual ~CommandPathRegistry() = default; + + virtual Optional Find(const ConcreteCommandPath & requestPath) const = 0; + virtual Optional GetFirstEntry() const = 0; + virtual CHIP_ERROR Add(const ConcreteCommandPath & requestPath, const Optional & ref) = 0; + virtual size_t Count() const = 0; + virtual size_t MaxSize() const = 0; +}; + +/** + * @class BasicCommandPathRegistry + * + * @brief Allows looking up CommandRef using the requested ConcreteCommandPath. + * + * While there are faster implementations, right now batch commands are capped at a low number due to + * message size constraints. All commands need to be contained within a single InvokeRequest. In + * practice this is usually less than 60 commands (but could be much more with TCP transports or + * newer transports). + */ +template +class BasicCommandPathRegistry : public CommandPathRegistry +{ +public: + Optional Find(const ConcreteCommandPath & requestPath) const override + { + for (size_t i = 0; i < mCount; i++) + { + if (mTable[i].requestPath == requestPath) + { + return MakeOptional(mTable[i]); + } + } + return NullOptional; + } + + Optional GetFirstEntry() const override + { + if (mCount > 0) + { + return MakeOptional(mTable[0]); + } + return NullOptional; + } + + CHIP_ERROR Add(const ConcreteCommandPath & requestPath, const Optional & ref) override + { + if (mCount >= N) + { + return CHIP_ERROR_NO_MEMORY; + } + for (size_t i = 0; i < mCount; i++) + { + if (mTable[i].requestPath == requestPath) + { + return CHIP_ERROR_DUPLICATE_KEY_ID; + } + // No need to check if either has value. This is because if there is more than + // 1 entry in the table expectation is to have all entirely unique ref values + // so duplicate optional would mean we would want to error out. + if (mTable[i].ref == ref) + { + return CHIP_ERROR_DUPLICATE_KEY_ID; + } + } + + mTable[mCount] = CommandPathRegistryEntry{ requestPath, ref }; + mCount++; + return CHIP_NO_ERROR; + } + + virtual size_t Count() const override { return mCount; } + virtual size_t MaxSize() const override { return N; } + +private: + size_t mCount = 0; + CommandPathRegistryEntry mTable[N]; +}; + +} // namespace app +} // namespace chip diff --git a/src/app/clusters/network-commissioning/network-commissioning.cpp b/src/app/clusters/network-commissioning/network-commissioning.cpp index 471b6038b85b09..5d9cee930677f8 100644 --- a/src/app/clusters/network-commissioning/network-commissioning.cpp +++ b/src/app/clusters/network-commissioning/network-commissioning.cpp @@ -815,8 +815,10 @@ void Instance::OnFinished(Status status, CharSpan debugText, ThreadScanResponseI size_t scanResponseArrayLength = 0; uint8_t extendedAddressBuffer[Thread::kSizeExtendedPanId]; - SuccessOrExit(err = commandHandle->PrepareCommand( - ConcreteCommandPath(mPath.mEndpointId, NetworkCommissioning::Id, Commands::ScanNetworksResponse::Id))); + const CommandHandler::InvokeResponseParameters prepareParams(mPath); + SuccessOrExit( + err = commandHandle->PrepareInvokeResponseCommand( + ConcreteCommandPath(mPath.mEndpointId, NetworkCommissioning::Id, Commands::ScanNetworksResponse::Id), prepareParams)); VerifyOrExit((writer = commandHandle->GetCommandDataIBTLVWriter()) != nullptr, err = CHIP_ERROR_INCORRECT_STATE); SuccessOrExit(err = writer->Put(TLV::ContextTag(Commands::ScanNetworksResponse::Fields::kNetworkingStatus), status)); @@ -927,8 +929,10 @@ void Instance::OnFinished(Status status, CharSpan debugText, WiFiScanResponseIte WiFiScanResponse scanResponse; size_t networksEncoded = 0; - SuccessOrExit(err = commandHandle->PrepareCommand( - ConcreteCommandPath(mPath.mEndpointId, NetworkCommissioning::Id, Commands::ScanNetworksResponse::Id))); + const CommandHandler::InvokeResponseParameters prepareParams(mPath); + SuccessOrExit( + err = commandHandle->PrepareInvokeResponseCommand( + ConcreteCommandPath(mPath.mEndpointId, NetworkCommissioning::Id, Commands::ScanNetworksResponse::Id), prepareParams)); VerifyOrExit((writer = commandHandle->GetCommandDataIBTLVWriter()) != nullptr, err = CHIP_ERROR_INCORRECT_STATE); SuccessOrExit(err = writer->Put(TLV::ContextTag(Commands::ScanNetworksResponse::Fields::kNetworkingStatus), status)); diff --git a/src/app/tests/BUILD.gn b/src/app/tests/BUILD.gn index 78e3bdb229e49c..577e9c69cdcffb 100644 --- a/src/app/tests/BUILD.gn +++ b/src/app/tests/BUILD.gn @@ -128,6 +128,7 @@ chip_test_suite_using_nltest("tests") { "TestAttributePersistenceProvider.cpp", "TestAttributeValueDecoder.cpp", "TestAttributeValueEncoder.cpp", + "TestBasicCommandPathRegistry.cpp", "TestBindingTable.cpp", "TestBuilderParser.cpp", "TestClusterInfo.cpp", diff --git a/src/app/tests/TestBasicCommandPathRegistry.cpp b/src/app/tests/TestBasicCommandPathRegistry.cpp new file mode 100644 index 00000000000000..61e859b95498f0 --- /dev/null +++ b/src/app/tests/TestBasicCommandPathRegistry.cpp @@ -0,0 +1,147 @@ +/* + * + * Copyright (c) 2023 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include + +#include + +namespace chip { +namespace app { +namespace TestBasicCommandPathRegistry { +namespace { + +size_t constexpr kQuickTestSize = 10; + +} // namespace + +void TestAddingSameConcretePath(nlTestSuite * apSuite, void * apContext) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + BasicCommandPathRegistry basicCommandPathRegistry; + + ConcreteCommandPath concretePath(0, 0, 0); + Optional commandRef; + uint16_t commandRefValue = 0; + + size_t idx = 0; + for (idx = 0; idx < kQuickTestSize && err == CHIP_NO_ERROR; idx++) + { + commandRef.SetValue(commandRefValue); + commandRefValue++; + err = basicCommandPathRegistry.Add(concretePath, commandRef); + } + + NL_TEST_ASSERT(apSuite, err == CHIP_ERROR_DUPLICATE_KEY_ID); + NL_TEST_ASSERT(apSuite, basicCommandPathRegistry.Count() == 1); +} + +void TestAddingSameCommandRef(nlTestSuite * apSuite, void * apContext) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + BasicCommandPathRegistry basicCommandPathRegistry; + + Optional commandRef; + commandRef.SetValue(0); + + uint16_t endpointValue = 0; + + size_t idx = 0; + for (idx = 0; idx < kQuickTestSize && err == CHIP_NO_ERROR; idx++) + { + ConcreteCommandPath concretePath(endpointValue, 0, 0); + endpointValue++; + err = basicCommandPathRegistry.Add(concretePath, commandRef); + } + + NL_TEST_ASSERT(apSuite, err == CHIP_ERROR_DUPLICATE_KEY_ID); + NL_TEST_ASSERT(apSuite, basicCommandPathRegistry.Count() == 1); +} + +void TestAddingMaxNumberOfEntries(nlTestSuite * apSuite, void * apContext) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + BasicCommandPathRegistry basicCommandPathRegistry; + + Optional commandRef; + uint16_t commandRefAndEndpointValue = 0; + + size_t idx = 0; + for (idx = 0; idx < kQuickTestSize && err == CHIP_NO_ERROR; idx++) + { + ConcreteCommandPath concretePath(commandRefAndEndpointValue, 0, 0); + commandRef.SetValue(commandRefAndEndpointValue); + commandRefAndEndpointValue++; + err = basicCommandPathRegistry.Add(concretePath, commandRef); + } + + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + NL_TEST_ASSERT(apSuite, basicCommandPathRegistry.Count() == kQuickTestSize); +} + +void TestAddingTooManyEntries(nlTestSuite * apSuite, void * apContext) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + BasicCommandPathRegistry basicCommandPathRegistry; + size_t maxPlusOne = kQuickTestSize + 1; + + Optional commandRef; + uint16_t commandRefAndEndpointValue = 0; + + size_t idx = 0; + for (idx = 0; idx < maxPlusOne && err == CHIP_NO_ERROR; idx++) + { + ConcreteCommandPath concretePath(commandRefAndEndpointValue, 0, 0); + commandRef.SetValue(commandRefAndEndpointValue); + commandRefAndEndpointValue++; + err = basicCommandPathRegistry.Add(concretePath, commandRef); + } + + NL_TEST_ASSERT(apSuite, err == CHIP_ERROR_NO_MEMORY); + NL_TEST_ASSERT(apSuite, basicCommandPathRegistry.Count() == kQuickTestSize); +} + +} // namespace TestBasicCommandPathRegistry +} // namespace app +} // namespace chip + +namespace { +// clang-format off +const nlTest sTests[] = +{ + NL_TEST_DEF("TestAddingSameConcretePath", chip::app::TestBasicCommandPathRegistry::TestAddingSameConcretePath), + NL_TEST_DEF("TestAddingSameCommandRef", chip::app::TestBasicCommandPathRegistry::TestAddingSameCommandRef), + NL_TEST_DEF("TestAddingMaxNumberOfEntries", chip::app::TestBasicCommandPathRegistry::TestAddingMaxNumberOfEntries), + NL_TEST_DEF("TestAddingTooManyEntries", chip::app::TestBasicCommandPathRegistry::TestAddingTooManyEntries), + + NL_TEST_SENTINEL() +}; +// clang-format on + +} // namespace + +int TestBasicCommandPathRegistry() +{ + nlTestSuite theSuite = { "CommandPathRegistry", &sTests[0], nullptr, nullptr }; + + nlTestRunner(&theSuite, nullptr); + + return (nlTestRunnerStats(&theSuite)); +} + +CHIP_REGISTER_TEST_SUITE(TestBasicCommandPathRegistry) diff --git a/src/app/tests/TestCommandInteraction.cpp b/src/app/tests/TestCommandInteraction.cpp index 718f7d38e567c2..0e589243026a7a 100644 --- a/src/app/tests/TestCommandInteraction.cpp +++ b/src/app/tests/TestCommandInteraction.cpp @@ -68,7 +68,8 @@ void CheckForInvalidAction(nlTestSuite * apSuite, chip::Test::MessageCapturer & namespace chip { namespace { -bool isCommandDispatched = false; +bool isCommandDispatched = false; +size_t commandDispatchedCount = 0; bool sendResponse = true; bool asyncCommand = false; @@ -82,28 +83,30 @@ constexpr CommandId kTestCommandIdWithData = 4; constexpr CommandId kTestCommandIdNoData = 5; constexpr CommandId kTestCommandIdCommandSpecificResponse = 6; constexpr CommandId kTestNonExistCommandId = 0; + +const app::CommandHandler::TestOnlyMarker kThisIsForTestOnly; } // namespace namespace app { CommandHandler::Handle asyncCommandHandle; -InteractionModel::Status ServerClusterCommandExists(const ConcreteCommandPath & aCommandPath) +InteractionModel::Status ServerClusterCommandExists(const ConcreteCommandPath & aRequestCommandPath) { // Mock cluster catalog, only support commands on one cluster on one endpoint. using InteractionModel::Status; - if (aCommandPath.mEndpointId != kTestEndpointId) + if (aRequestCommandPath.mEndpointId != kTestEndpointId) { return Status::UnsupportedEndpoint; } - if (aCommandPath.mClusterId != kTestClusterId) + if (aRequestCommandPath.mClusterId != kTestClusterId) { return Status::UnsupportedCluster; } - if (aCommandPath.mCommandId == kTestNonExistCommandId) + if (aRequestCommandPath.mCommandId == kTestNonExistCommandId) { return Status::UnsupportedCommand; } @@ -111,17 +114,18 @@ InteractionModel::Status ServerClusterCommandExists(const ConcreteCommandPath & return Status::Success; } -void DispatchSingleClusterCommand(const ConcreteCommandPath & aCommandPath, chip::TLV::TLVReader & aReader, +void DispatchSingleClusterCommand(const ConcreteCommandPath & aRequestCommandPath, chip::TLV::TLVReader & aReader, CommandHandler * apCommandObj) { ChipLogDetail(Controller, "Received Cluster Command: Endpoint=%x Cluster=" ChipLogFormatMEI " Command=" ChipLogFormatMEI, - aCommandPath.mEndpointId, ChipLogValueMEI(aCommandPath.mClusterId), ChipLogValueMEI(aCommandPath.mCommandId)); + aRequestCommandPath.mEndpointId, ChipLogValueMEI(aRequestCommandPath.mClusterId), + ChipLogValueMEI(aRequestCommandPath.mCommandId)); // Duplicate what our normal command-field-decode code does, in terms of // checking for a struct and then entering it before getting the fields. if (aReader.GetType() != TLV::kTLVType_Structure) { - apCommandObj->AddStatus(aCommandPath, Protocols::InteractionModel::Status::InvalidAction); + apCommandObj->AddStatus(aRequestCommandPath, Protocols::InteractionModel::Status::InvalidAction); return; } @@ -130,7 +134,7 @@ void DispatchSingleClusterCommand(const ConcreteCommandPath & aCommandPath, chip NL_TEST_ASSERT(gSuite, err == CHIP_NO_ERROR); err = aReader.Next(); - if (aCommandPath.mCommandId == kTestCommandIdNoData) + if (aRequestCommandPath.mCommandId == kTestCommandIdNoData) { NL_TEST_ASSERT(gSuite, err == CHIP_ERROR_END_OF_TLV); } @@ -155,13 +159,15 @@ void DispatchSingleClusterCommand(const ConcreteCommandPath & aCommandPath, chip if (sendResponse) { - if (aCommandPath.mCommandId == kTestCommandIdNoData || aCommandPath.mCommandId == kTestCommandIdWithData) + if (aRequestCommandPath.mCommandId == kTestCommandIdNoData || aRequestCommandPath.mCommandId == kTestCommandIdWithData) { - apCommandObj->AddStatus(aCommandPath, Protocols::InteractionModel::Status::Success); + apCommandObj->AddStatus(aRequestCommandPath, Protocols::InteractionModel::Status::Success); } else { - apCommandObj->PrepareCommand(aCommandPath); + const CommandHandler::InvokeResponseParameters prepareParams(aRequestCommandPath); + const ConcreteCommandPath responseCommandPath = aRequestCommandPath; + apCommandObj->PrepareInvokeResponseCommand(responseCommandPath, prepareParams); chip::TLV::TLVWriter * writer = apCommandObj->GetCommandDataIBTLVWriter(); writer->PutBoolean(chip::TLV::ContextTag(1), true); apCommandObj->FinishCommand(); @@ -169,6 +175,7 @@ void DispatchSingleClusterCommand(const ConcreteCommandPath & aCommandPath, chip } chip::isCommandDispatched = true; + commandDispatchedCount++; } class MockCommandSenderCallback : public CommandSender::Callback @@ -218,6 +225,8 @@ class MockCommandHandlerCallback : public CommandHandler::Callback return ServerClusterCommandExists(aCommandPath); } + void ResetCounter() { onFinalCalledTimes = 0; } + int onFinalCalledTimes = 0; } mockCommandHandlerDelegate; @@ -244,7 +253,10 @@ class TestCommandInteraction static void TestCommandHandlerWithSendEmptyResponse(nlTestSuite * apSuite, void * apContext); static void TestCommandHandlerWithProcessReceivedEmptyDataMsg(nlTestSuite * apSuite, void * apContext); - static void TestCommandHandlerRejectMultipleCommands(nlTestSuite * apSuite, void * apContext); + static void TestCommandHandlerRejectMultipleIdenticalCommands(nlTestSuite * apSuite, void * apContext); + static void TestCommandHandlerRejectsMultipleCommandsWithIdenticalCommandRef(nlTestSuite * apSuite, void * apContext); + static void TestCommandHandlerRejectMultipleCommandsWhenHandlerOnlySupportsOne(nlTestSuite * apSuite, void * apContext); + static void TestCommandHandlerAcceptMultipleCommands(nlTestSuite * apSuite, void * apContext); #if CONFIG_BUILD_FOR_HOST_UNIT_TEST static void TestCommandHandlerReleaseWithExchangeClosed(nlTestSuite * apSuite, void * apContext); @@ -263,6 +275,24 @@ class TestCommandInteraction } private: + /** + * With the introduction of batch invoke commands, CommandHandler keeps track of incoming + * ConcreteCommandPath and the associated CommandRef. Normally this is populated as a part + * of OnMessageReceived from the incoming request. For some unit tests were we want to just + * test some of the processing messages we need to inject commands into the table of + * ConcreteCommandPath/CommandRef pairs. + */ + class CommandHandlerWithOutstandingCommand : public app::CommandHandler + { + public: + CommandHandlerWithOutstandingCommand(CommandHandler::Callback * apCallback, const ConcreteCommandPath & aRequestCommandPath, + const Optional & aRef) : + CommandHandler(apCallback) + { + GetCommandPathRegistry().Add(aRequestCommandPath, aRef); + } + }; + // Generate an invoke request. If aCommandId is kTestCommandIdWithData, a // payload will be included. Otherwise no payload will be included. static void GenerateInvokeRequest(nlTestSuite * apSuite, void * apContext, System::PacketBufferHandle & aPayload, @@ -451,18 +481,19 @@ void TestCommandInteraction::AddInvokeResponseData(nlTestSuite * apSuite, void * { CHIP_ERROR err = CHIP_NO_ERROR; + constexpr EndpointId kTestEndpointId = 1; + constexpr ClusterId kTestClusterId = 3; + constexpr CommandId kTestCommandIdWithData = 4; + ConcreteCommandPath requestCommandPath = { kTestEndpointId, kTestClusterId, kTestCommandIdWithData }; if (aNeedStatusCode) { - chip::app::ConcreteCommandPath commandPath(1, // Endpoint - 3, // ClusterId - 4 // CommandId - ); - apCommandHandler->AddStatus(commandPath, Protocols::InteractionModel::Status::Success); + apCommandHandler->AddStatus(requestCommandPath, Protocols::InteractionModel::Status::Success); } else { - ConcreteCommandPath path = { kTestEndpointId, kTestClusterId, aCommandId }; - err = apCommandHandler->PrepareCommand(path); + const CommandHandler::InvokeResponseParameters prepareParams(requestCommandPath); + ConcreteCommandPath responseCommandPath = { kTestEndpointId, kTestClusterId, aCommandId }; + err = apCommandHandler->PrepareInvokeResponseCommand(responseCommandPath, prepareParams); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); chip::TLV::TLVWriter * writer = apCommandHandler->GetCommandDataIBTLVWriter(); @@ -489,13 +520,16 @@ void TestCommandInteraction::TestCommandSenderWithWrongState(nlTestSuite * apSui void TestCommandInteraction::TestCommandHandlerWithWrongState(nlTestSuite * apSuite, void * apContext) { - TestContext & ctx = *static_cast(apContext); - CHIP_ERROR err = CHIP_NO_ERROR; - ConcreteCommandPath path = { kTestEndpointId, kTestClusterId, kTestCommandIdNoData }; + TestContext & ctx = *static_cast(apContext); + CHIP_ERROR err = CHIP_NO_ERROR; + ConcreteCommandPath requestCommandPath = { kTestEndpointId, kTestClusterId, kTestCommandIdNoData }; + ConcreteCommandPath responseCommandPath = { kTestEndpointId, kTestClusterId, kTestCommandIdNoData }; - app::CommandHandler commandHandler(&mockCommandHandlerDelegate); + CommandHandlerWithOutstandingCommand commandHandler(&mockCommandHandlerDelegate, requestCommandPath, + /* aRef = */ NullOptional); - err = commandHandler.PrepareCommand(path); + const CommandHandler::InvokeResponseParameters prepareParams(requestCommandPath); + err = commandHandler.PrepareInvokeResponseCommand(responseCommandPath, prepareParams); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); TestExchangeDelegate delegate; @@ -538,18 +572,21 @@ void TestCommandInteraction::TestCommandSenderWithSendCommand(nlTestSuite * apSu void TestCommandInteraction::TestCommandHandlerWithSendEmptyCommand(nlTestSuite * apSuite, void * apContext) { - TestContext & ctx = *static_cast(apContext); - CHIP_ERROR err = CHIP_NO_ERROR; - ConcreteCommandPath path = { kTestEndpointId, kTestClusterId, kTestCommandIdNoData }; + TestContext & ctx = *static_cast(apContext); + CHIP_ERROR err = CHIP_NO_ERROR; + ConcreteCommandPath requestCommandPath = { kTestEndpointId, kTestClusterId, kTestCommandIdNoData }; + ConcreteCommandPath responseCommandPath = { kTestEndpointId, kTestClusterId, kTestCommandIdNoData }; - app::CommandHandler commandHandler(&mockCommandHandlerDelegate); + CommandHandlerWithOutstandingCommand commandHandler(&mockCommandHandlerDelegate, requestCommandPath, + /* aRef = */ NullOptional); System::PacketBufferHandle commandDatabuf = System::PacketBufferHandle::New(System::PacketBuffer::kMaxSize); TestExchangeDelegate delegate; auto exchange = ctx.NewExchangeToAlice(&delegate, false); commandHandler.mExchangeCtx.Grab(exchange); - err = commandHandler.PrepareCommand(path); + const CommandHandler::InvokeResponseParameters prepareParams(requestCommandPath); + err = commandHandler.PrepareInvokeResponseCommand(responseCommandPath, prepareParams); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); err = commandHandler.FinishCommand(); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); @@ -577,8 +614,10 @@ void TestCommandInteraction::ValidateCommandHandlerWithSendCommand(nlTestSuite * { TestContext & ctx = *static_cast(apContext); CHIP_ERROR err = CHIP_NO_ERROR; - app::CommandHandler commandHandler(&mockCommandHandlerDelegate); System::PacketBufferHandle commandPacket; + chip::app::ConcreteCommandPath requestCommandPath(kTestEndpointId, kTestClusterId, kTestCommandIdWithData); + CommandHandlerWithOutstandingCommand commandHandler(&mockCommandHandlerDelegate, requestCommandPath, + /* aRef = */ NullOptional); TestExchangeDelegate delegate; auto exchange = ctx.NewExchangeToAlice(&delegate, false); @@ -643,18 +682,18 @@ struct BadFields void TestCommandInteraction::TestCommandHandlerCommandDataEncoding(nlTestSuite * apSuite, void * apContext) { - TestContext & ctx = *static_cast(apContext); - CHIP_ERROR err = CHIP_NO_ERROR; - app::CommandHandler commandHandler(nullptr); + TestContext & ctx = *static_cast(apContext); + CHIP_ERROR err = CHIP_NO_ERROR; + auto path = MakeTestCommandPath(); + auto requestCommandPath = ConcreteCommandPath(path.mEndpointId, path.mClusterId, path.mCommandId); + CommandHandlerWithOutstandingCommand commandHandler(nullptr, requestCommandPath, /* aRef = */ NullOptional); System::PacketBufferHandle commandPacket; TestExchangeDelegate delegate; auto exchange = ctx.NewExchangeToAlice(&delegate, false); commandHandler.mExchangeCtx.Grab(exchange); - auto path = MakeTestCommandPath(); - - commandHandler.AddResponse(ConcreteCommandPath(path.mEndpointId, path.mClusterId, path.mCommandId), Fields()); + commandHandler.AddResponse(requestCommandPath, Fields()); err = commandHandler.Finalize(commandPacket); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); @@ -678,18 +717,18 @@ void TestCommandInteraction::TestCommandHandlerCommandDataEncoding(nlTestSuite * void TestCommandInteraction::TestCommandHandlerCommandEncodeFailure(nlTestSuite * apSuite, void * apContext) { - TestContext & ctx = *static_cast(apContext); - CHIP_ERROR err = CHIP_NO_ERROR; - app::CommandHandler commandHandler(nullptr); + TestContext & ctx = *static_cast(apContext); + CHIP_ERROR err = CHIP_NO_ERROR; + auto path = MakeTestCommandPath(); + auto requestCommandPath = ConcreteCommandPath(path.mEndpointId, path.mClusterId, path.mCommandId); + CommandHandlerWithOutstandingCommand commandHandler(nullptr, requestCommandPath, NullOptional); System::PacketBufferHandle commandPacket; TestExchangeDelegate delegate; auto exchange = ctx.NewExchangeToAlice(&delegate, false); commandHandler.mExchangeCtx.Grab(exchange); - auto path = MakeTestCommandPath(); - - commandHandler.AddResponse(ConcreteCommandPath(path.mEndpointId, path.mClusterId, path.mCommandId), BadFields()); + commandHandler.AddResponse(requestCommandPath, BadFields()); err = commandHandler.Finalize(commandPacket); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); @@ -1067,9 +1106,15 @@ void TestCommandInteraction::TestCommandHandlerInvalidMessageAsync(nlTestSuite * err = commandSender.SendCommandRequest(ctx.GetSessionBobToAlice()); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + // Warning! This test is not testing what the testname is suggesting. Because the invoke request is malformed, + // DispatchSingleClusterCommand is never called and asyncCommandHandle is never set. Possible fix to this test + // might be incoming when CommandSender is capable of sending multiple invoke command requests. + // Decrease CommandHandler refcount and send response asyncCommandHandle = nullptr; + // Prevent breaking other tests. + asyncCommand = false; ctx.DrainAndServiceIO(); NL_TEST_ASSERT(apSuite, @@ -1082,21 +1127,20 @@ void TestCommandInteraction::TestCommandHandlerInvalidMessageAsync(nlTestSuite * void TestCommandInteraction::TestCommandHandlerCommandEncodeExternalFailure(nlTestSuite * apSuite, void * apContext) { - TestContext & ctx = *static_cast(apContext); - CHIP_ERROR err = CHIP_NO_ERROR; - app::CommandHandler commandHandler(nullptr); + TestContext & ctx = *static_cast(apContext); + CHIP_ERROR err = CHIP_NO_ERROR; + auto path = MakeTestCommandPath(); + auto requestCommandPath = ConcreteCommandPath(path.mEndpointId, path.mClusterId, path.mCommandId); + CommandHandlerWithOutstandingCommand commandHandler(nullptr, requestCommandPath, NullOptional); System::PacketBufferHandle commandPacket; TestExchangeDelegate delegate; auto exchange = ctx.NewExchangeToAlice(&delegate, false); commandHandler.mExchangeCtx.Grab(exchange); - auto path = MakeTestCommandPath(); - - err = commandHandler.AddResponseData(ConcreteCommandPath(path.mEndpointId, path.mClusterId, path.mCommandId), BadFields()); + err = commandHandler.AddResponseData(requestCommandPath, BadFields()); NL_TEST_ASSERT(apSuite, err != CHIP_NO_ERROR); - commandHandler.AddStatus(ConcreteCommandPath(path.mEndpointId, path.mClusterId, path.mCommandId), - Protocols::InteractionModel::Status::Failure); + commandHandler.AddStatus(requestCommandPath, Protocols::InteractionModel::Status::Failure); err = commandHandler.Finalize(commandPacket); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); @@ -1331,7 +1375,7 @@ void TestCommandInteraction::TestCommandSenderAbruptDestruction(nlTestSuite * ap NL_TEST_ASSERT(apSuite, GetNumActiveHandlerObjects() == 0); } -void TestCommandInteraction::TestCommandHandlerRejectMultipleCommands(nlTestSuite * apSuite, void * apContext) +void TestCommandInteraction::TestCommandHandlerRejectMultipleIdenticalCommands(nlTestSuite * apSuite, void * apContext) { TestContext & ctx = *static_cast(apContext); CHIP_ERROR err = CHIP_NO_ERROR; @@ -1387,6 +1431,236 @@ void TestCommandInteraction::TestCommandHandlerRejectMultipleCommands(nlTestSuit NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); } +void TestCommandInteraction::TestCommandHandlerRejectsMultipleCommandsWithIdenticalCommandRef(nlTestSuite * apSuite, + void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + CHIP_ERROR err = CHIP_NO_ERROR; + + isCommandDispatched = false; + mockCommandSenderDelegate.ResetCounter(); + + // Using commandSender to help build afterward we take the buffer to feed into standalone CommandHandler + app::CommandSender commandSender(&mockCommandSenderDelegate, &ctx.GetExchangeManager()); + + size_t numberOfCommandsToSend = 2; + { + CommandPathParams requestCommandPaths[] = { + MakeTestCommandPath(kTestCommandIdWithData), + MakeTestCommandPath(kTestCommandIdCommandSpecificResponse), + }; + + commandSender.AllocateBuffer(); + + // CommandSender does not support sending multiple commands with public API, so we craft a message manaully. + for (size_t i = 0; i < numberOfCommandsToSend; i++) + { + InvokeRequests::Builder & invokeRequests = commandSender.mInvokeRequestBuilder.GetInvokeRequests(); + CommandDataIB::Builder & invokeRequest = invokeRequests.CreateCommandData(); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequests.GetError()); + CommandPathIB::Builder & path = invokeRequest.CreatePath(); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.GetError()); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == path.Encode(requestCommandPaths[i])); + NL_TEST_ASSERT(apSuite, + CHIP_NO_ERROR == + invokeRequest.GetWriter()->StartContainer(TLV::ContextTag(CommandDataIB::Tag::kFields), + TLV::kTLVType_Structure, + commandSender.mDataElementContainerType)); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.GetWriter()->PutBoolean(chip::TLV::ContextTag(1), true)); + NL_TEST_ASSERT(apSuite, + CHIP_NO_ERROR == invokeRequest.GetWriter()->EndContainer(commandSender.mDataElementContainerType)); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.Ref(1)); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.EndOfCommandDataIB()); + } + + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == commandSender.mInvokeRequestBuilder.GetInvokeRequests().EndOfInvokeRequests()); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == commandSender.mInvokeRequestBuilder.EndOfInvokeRequestMessage()); + + commandSender.MoveToState(app::CommandSender::State::AddedCommand); + } + + CommandHandler commandHandler(&mockCommandHandlerDelegate); + TestExchangeDelegate delegate; + auto exchange = ctx.NewExchangeToAlice(&delegate, false); + commandHandler.mExchangeCtx.Grab(exchange); + + // Hackery to steal the InvokeRequest buffer from commandSender. + System::PacketBufferHandle commandDatabuf; + err = commandSender.Finalize(commandDatabuf); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + mockCommandHandlerDelegate.ResetCounter(); + commandDispatchedCount = 0; + + InteractionModel::Status status = commandHandler.ProcessInvokeRequest(std::move(commandDatabuf), false); + NL_TEST_ASSERT(apSuite, status == InteractionModel::Status::InvalidAction); + + NL_TEST_ASSERT(apSuite, commandDispatchedCount == 0); + + // + // Ordinarily, the ExchangeContext will close itself on a responder exchange when unwinding back from an + // OnMessageReceived callback and not having sent a subsequent message (as is the case when calling ProcessInvokeRequest + // above, which doesn't actually send back a response in these cases). Since that isn't the case in this artificial + // setup here (where we created a responder exchange that's not responding to anything), we need to explicitly close it + // out. This is not expected in normal application logic. + // + exchange->Close(); +} + +void TestCommandInteraction::TestCommandHandlerRejectMultipleCommandsWhenHandlerOnlySupportsOne(nlTestSuite * apSuite, + void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + CHIP_ERROR err = CHIP_NO_ERROR; + + isCommandDispatched = false; + mockCommandSenderDelegate.ResetCounter(); + + // Using commandSender to help build afterward we take the buffer to feed into standalone CommandHandler + app::CommandSender commandSender(&mockCommandSenderDelegate, &ctx.GetExchangeManager()); + + size_t numberOfCommandsToSend = 2; + { + CommandPathParams requestCommandPaths[] = { + MakeTestCommandPath(kTestCommandIdWithData), + MakeTestCommandPath(kTestCommandIdCommandSpecificResponse), + }; + + commandSender.AllocateBuffer(); + + // CommandSender does not support sending multiple commands with public API, so we craft a message manaully. + for (size_t i = 0; i < numberOfCommandsToSend; i++) + { + InvokeRequests::Builder & invokeRequests = commandSender.mInvokeRequestBuilder.GetInvokeRequests(); + CommandDataIB::Builder & invokeRequest = invokeRequests.CreateCommandData(); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequests.GetError()); + CommandPathIB::Builder & path = invokeRequest.CreatePath(); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.GetError()); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == path.Encode(requestCommandPaths[i])); + NL_TEST_ASSERT(apSuite, + CHIP_NO_ERROR == + invokeRequest.GetWriter()->StartContainer(TLV::ContextTag(CommandDataIB::Tag::kFields), + TLV::kTLVType_Structure, + commandSender.mDataElementContainerType)); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.GetWriter()->PutBoolean(chip::TLV::ContextTag(1), true)); + NL_TEST_ASSERT(apSuite, + CHIP_NO_ERROR == invokeRequest.GetWriter()->EndContainer(commandSender.mDataElementContainerType)); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.Ref(static_cast(i))); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.EndOfCommandDataIB()); + } + + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == commandSender.mInvokeRequestBuilder.GetInvokeRequests().EndOfInvokeRequests()); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == commandSender.mInvokeRequestBuilder.EndOfInvokeRequestMessage()); + + commandSender.MoveToState(app::CommandSender::State::AddedCommand); + } + + BasicCommandPathRegistry<4> mBasicCommandPathRegistry; + CommandHandler commandHandler(kThisIsForTestOnly, &mockCommandHandlerDelegate, &mBasicCommandPathRegistry); + TestExchangeDelegate delegate; + auto exchange = ctx.NewExchangeToAlice(&delegate, false); + commandHandler.mExchangeCtx.Grab(exchange); + + // Hackery to steal the InvokeRequest buffer from commandSender. + System::PacketBufferHandle commandDatabuf; + err = commandSender.Finalize(commandDatabuf); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + mockCommandHandlerDelegate.ResetCounter(); + sendResponse = true; + commandDispatchedCount = 0; + + InteractionModel::Status status = commandHandler.ProcessInvokeRequest(std::move(commandDatabuf), false); + NL_TEST_ASSERT(apSuite, status == InteractionModel::Status::Success); + + NL_TEST_ASSERT(apSuite, commandDispatchedCount == 2); + + // + // Ordinarily, the ExchangeContext will close itself on a responder exchange when unwinding back from an + // OnMessageReceived callback and not having sent a subsequent message (as is the case when calling ProcessInvokeRequest + // above, which doesn't actually send back a response in these cases). Since that isn't the case in this artificial + // setup here (where we created a responder exchange that's not responding to anything), we need to explicitly close it + // out. This is not expected in normal application logic. + // + exchange->Close(); +} + +void TestCommandInteraction::TestCommandHandlerAcceptMultipleCommands(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + CHIP_ERROR err = CHIP_NO_ERROR; + + isCommandDispatched = false; + mockCommandSenderDelegate.ResetCounter(); + + // Using commandSender to help build afterward we take the buffer to feed into standalone CommandHandler + app::CommandSender commandSender(&mockCommandSenderDelegate, &ctx.GetExchangeManager()); + + size_t numberOfCommandsToSend = 2; + { + CommandPathParams requestCommandPaths[] = { + MakeTestCommandPath(kTestCommandIdWithData), + MakeTestCommandPath(kTestCommandIdCommandSpecificResponse), + }; + + commandSender.AllocateBuffer(); + + // CommandSender does not support sending multiple commands with public API, so we craft a message manaully. + for (size_t i = 0; i < numberOfCommandsToSend; i++) + { + InvokeRequests::Builder & invokeRequests = commandSender.mInvokeRequestBuilder.GetInvokeRequests(); + CommandDataIB::Builder & invokeRequest = invokeRequests.CreateCommandData(); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequests.GetError()); + CommandPathIB::Builder & path = invokeRequest.CreatePath(); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.GetError()); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == path.Encode(requestCommandPaths[i])); + NL_TEST_ASSERT(apSuite, + CHIP_NO_ERROR == + invokeRequest.GetWriter()->StartContainer(TLV::ContextTag(CommandDataIB::Tag::kFields), + TLV::kTLVType_Structure, + commandSender.mDataElementContainerType)); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.GetWriter()->PutBoolean(chip::TLV::ContextTag(1), true)); + NL_TEST_ASSERT(apSuite, + CHIP_NO_ERROR == invokeRequest.GetWriter()->EndContainer(commandSender.mDataElementContainerType)); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.Ref(static_cast(i))); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.EndOfCommandDataIB()); + } + + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == commandSender.mInvokeRequestBuilder.GetInvokeRequests().EndOfInvokeRequests()); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == commandSender.mInvokeRequestBuilder.EndOfInvokeRequestMessage()); + + commandSender.MoveToState(app::CommandSender::State::AddedCommand); + } + + BasicCommandPathRegistry<4> mBasicCommandPathRegistry; + CommandHandler commandHandler(kThisIsForTestOnly, &mockCommandHandlerDelegate, &mBasicCommandPathRegistry); + TestExchangeDelegate delegate; + auto exchange = ctx.NewExchangeToAlice(&delegate, false); + commandHandler.mExchangeCtx.Grab(exchange); + + // Hackery to steal the InvokeRequest buffer from commandSender. + System::PacketBufferHandle commandDatabuf; + err = commandSender.Finalize(commandDatabuf); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + mockCommandHandlerDelegate.ResetCounter(); + commandDispatchedCount = 0; + + InteractionModel::Status status = commandHandler.ProcessInvokeRequest(std::move(commandDatabuf), false); + NL_TEST_ASSERT(apSuite, status == InteractionModel::Status::Success); + + NL_TEST_ASSERT(apSuite, commandDispatchedCount == 2); + + // + // Ordinarily, the ExchangeContext will close itself on a responder exchange when unwinding back from an + // OnMessageReceived callback and not having sent a subsequent message (as is the case when calling ProcessInvokeRequest + // above, which doesn't actually send back a response in these cases). Since that isn't the case in this artificial + // setup here (where we created a responder exchange that's not responding to anything), we need to explicitly close it + // out. This is not expected in normal application logic. + // + exchange->Close(); +} + #if CONFIG_BUILD_FOR_HOST_UNIT_TEST // // This test needs a special unit-test only API being exposed in ExchangeContext to be able to correctly simulate @@ -1443,7 +1717,11 @@ const nlTest sTests[] = NL_TEST_DEF("TestCommandHandlerWithSendSimpleStatusCode", chip::app::TestCommandInteraction::TestCommandHandlerWithSendSimpleStatusCode), NL_TEST_DEF("TestCommandHandlerWithProcessReceivedNotExistCommand", chip::app::TestCommandInteraction::TestCommandHandlerWithProcessReceivedNotExistCommand), NL_TEST_DEF("TestCommandHandlerWithProcessReceivedEmptyDataMsg", chip::app::TestCommandInteraction::TestCommandHandlerWithProcessReceivedEmptyDataMsg), - NL_TEST_DEF("TestCommandHandlerRejectMultipleCommands", chip::app::TestCommandInteraction::TestCommandHandlerRejectMultipleCommands), + NL_TEST_DEF("TestCommandHandlerRejectMultipleIdenticalCommands", chip::app::TestCommandInteraction::TestCommandHandlerRejectMultipleIdenticalCommands), + NL_TEST_DEF("TestCommandHandlerRejectsMultipleCommandsWithIdenticalCommandRef", chip::app::TestCommandInteraction::TestCommandHandlerRejectsMultipleCommandsWithIdenticalCommandRef), + NL_TEST_DEF("TestCommandHandlerRejectMultipleCommandsWhenHandlerOnlySupportsOne", chip::app::TestCommandInteraction::TestCommandHandlerRejectMultipleCommandsWhenHandlerOnlySupportsOne), + NL_TEST_DEF("TestCommandHandlerAcceptMultipleCommands", chip::app::TestCommandInteraction::TestCommandHandlerAcceptMultipleCommands), + #if CONFIG_BUILD_FOR_HOST_UNIT_TEST NL_TEST_DEF("TestCommandHandlerReleaseWithExchangeClosed", chip::app::TestCommandInteraction::TestCommandHandlerReleaseWithExchangeClosed), diff --git a/src/app/tests/integration/chip_im_responder.cpp b/src/app/tests/integration/chip_im_responder.cpp index e8fa74d9baa26c..17d47a06ea803d 100644 --- a/src/app/tests/integration/chip_im_responder.cpp +++ b/src/app/tests/integration/chip_im_responder.cpp @@ -73,12 +73,12 @@ Protocols::InteractionModel::Status ServerClusterCommandExists(const ConcreteCom return Status::Success; } -void DispatchSingleClusterCommand(const ConcreteCommandPath & aCommandPath, chip::TLV::TLVReader & aReader, +void DispatchSingleClusterCommand(const ConcreteCommandPath & aRequestCommandPath, chip::TLV::TLVReader & aReader, CommandHandler * apCommandObj) { static bool statusCodeFlipper = false; - if (ServerClusterCommandExists(aCommandPath) != Protocols::InteractionModel::Status::Success) + if (ServerClusterCommandExists(aRequestCommandPath) != Protocols::InteractionModel::Status::Success) { return; } @@ -106,7 +106,8 @@ void DispatchSingleClusterCommand(const ConcreteCommandPath & aCommandPath, chip chip::TLV::TLVWriter * writer; - ReturnOnFailure(apCommandObj->PrepareCommand(path)); + const CommandHandler::InvokeResponseParameters prepareParams(aRequestCommandPath); + ReturnOnFailure(apCommandObj->PrepareInvokeResponseCommand(path, prepareParams)); writer = apCommandObj->GetCommandDataIBTLVWriter(); ReturnOnFailure(writer->Put(chip::TLV::ContextTag(kTestFieldId1), kTestFieldValue1));