diff --git a/src/app/CommandSender.cpp b/src/app/CommandSender.cpp index 56399f8d3312bb..7f77d6c98c2bad 100644 --- a/src/app/CommandSender.cpp +++ b/src/app/CommandSender.cpp @@ -443,7 +443,8 @@ CHIP_ERROR CommandSender::SetCommandSenderConfig(CommandSender::ConfigParameters #endif } -CHIP_ERROR CommandSender::PrepareCommand(const CommandPathParams & aCommandPathParams, AdditionalCommandParameters & aOptionalArgs) +CHIP_ERROR CommandSender::PrepareCommand(const CommandPathParams & aCommandPathParams, + PrepareCommandParameters & aPrepareCommandParams) { ReturnErrorOnFailure(AllocateBuffer()); @@ -454,10 +455,10 @@ CHIP_ERROR CommandSender::PrepareCommand(const CommandPathParams & aCommandPathP VerifyOrReturnError(mState == State::Idle || canAddAnotherCommand, CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(mFinishedCommandCount < mRemoteMaxPathsPerInvoke, CHIP_ERROR_MAXIMUM_PATHS_PER_INVOKE_EXCEEDED); - VerifyOrReturnError(!aOptionalArgs.commandRef.HasValue(), CHIP_ERROR_INVALID_ARGUMENT); if (mBatchCommandsEnabled) { - aOptionalArgs.commandRef.SetValue(mFinishedCommandCount); + VerifyOrReturnError(aPrepareCommandParams.commandRef.HasValue(), CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(aPrepareCommandParams.commandRef.Value() == mFinishedCommandCount, CHIP_ERROR_INVALID_ARGUMENT); } InvokeRequests::Builder & invokeRequests = mInvokeRequestBuilder.GetInvokeRequests(); @@ -467,7 +468,7 @@ CHIP_ERROR CommandSender::PrepareCommand(const CommandPathParams & aCommandPathP ReturnErrorOnFailure(invokeRequest.GetError()); ReturnErrorOnFailure(path.Encode(aCommandPathParams)); - if (aOptionalArgs.startOrEndDataStruct) + if (aPrepareCommandParams.startDataStruct) { ReturnErrorOnFailure(invokeRequest.GetWriter()->StartContainer(TLV::ContextTag(CommandDataIB::Tag::kFields), TLV::kTLVType_Structure, mDataElementContainerType)); @@ -477,7 +478,18 @@ CHIP_ERROR CommandSender::PrepareCommand(const CommandPathParams & aCommandPathP return CHIP_NO_ERROR; } -CHIP_ERROR CommandSender::FinishCommand(const AdditionalCommandParameters & aOptionalArgs) +CHIP_ERROR CommandSender::FinishCommand(FinishCommandParameters & aFinishCommandParams) +{ + if (mBatchCommandsEnabled) + { + VerifyOrReturnError(aFinishCommandParams.commandRef.HasValue(), CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(aFinishCommandParams.commandRef.Value() == mFinishedCommandCount, CHIP_ERROR_INVALID_ARGUMENT); + } + + return FinishCommandInternal(aFinishCommandParams); +} + +CHIP_ERROR CommandSender::FinishCommandInternal(FinishCommandParameters & aFinishCommandParams) { CHIP_ERROR err = CHIP_NO_ERROR; @@ -485,20 +497,14 @@ CHIP_ERROR CommandSender::FinishCommand(const AdditionalCommandParameters & aOpt CommandDataIB::Builder & commandData = mInvokeRequestBuilder.GetInvokeRequests().GetCommandData(); - if (aOptionalArgs.startOrEndDataStruct) + if (aFinishCommandParams.endDataStruct) { ReturnErrorOnFailure(commandData.GetWriter()->EndContainer(mDataElementContainerType)); } - if (mBatchCommandsEnabled) - { - // If error below occurs, aOptionalArgs.commandRef was modified since PerpareCommand was called. - VerifyOrReturnError(aOptionalArgs.commandRef.HasValue(), CHIP_ERROR_INVALID_ARGUMENT); - } - - if (aOptionalArgs.commandRef.HasValue()) + if (aFinishCommandParams.commandRef.HasValue()) { - ReturnErrorOnFailure(commandData.Ref(aOptionalArgs.commandRef.Value())); + ReturnErrorOnFailure(commandData.Ref(aFinishCommandParams.commandRef.Value())); } ReturnErrorOnFailure(commandData.EndOfCommandDataIB()); @@ -506,6 +512,12 @@ CHIP_ERROR CommandSender::FinishCommand(const AdditionalCommandParameters & aOpt MoveToState(State::AddedCommand); mFinishedCommandCount++; + + if (aFinishCommandParams.timedInvokeTimeoutMs.HasValue()) + { + SetTimedInvokeTimeoutMs(aFinishCommandParams.timedInvokeTimeoutMs); + } + return CHIP_NO_ERROR; } @@ -519,10 +531,8 @@ TLV::TLVWriter * CommandSender::GetCommandDataIBTLVWriter() return mInvokeRequestBuilder.GetInvokeRequests().GetCommandData().GetWriter(); } -CHIP_ERROR CommandSender::FinishCommand(const Optional & aTimedInvokeTimeoutMs, - const AdditionalCommandParameters & aOptionalArgs) +void CommandSender::SetTimedInvokeTimeoutMs(const Optional & aTimedInvokeTimeoutMs) { - ReturnErrorOnFailure(FinishCommand(aOptionalArgs)); if (!mTimedInvokeTimeoutMs.HasValue()) { mTimedInvokeTimeoutMs = aTimedInvokeTimeoutMs; @@ -532,7 +542,6 @@ CHIP_ERROR CommandSender::FinishCommand(const Optional & aTimedInvokeT uint16_t newValue = std::min(mTimedInvokeTimeoutMs.Value(), aTimedInvokeTimeoutMs.Value()); mTimedInvokeTimeoutMs.SetValue(newValue); } - return CHIP_NO_ERROR; } size_t CommandSender::GetInvokeResponseMessageCount() diff --git a/src/app/CommandSender.h b/src/app/CommandSender.h index 08d518f5dbc843..fa213ac0699832 100644 --- a/src/app/CommandSender.h +++ b/src/app/CommandSender.h @@ -181,29 +181,101 @@ class CommandSender final : public Messaging::ExchangeDelegate uint16_t remoteMaxPathsPerInvoke = 1; }; - // PrepareCommand and FinishCommand are public SDK APIs, so we cannot break source - // compatibility for them. By having parameters to those APIs use this struct instead - // of individual function arguments, we centralize required changes to one file - // when adding new functionality. - struct AdditionalCommandParameters + // AddRequestData is a public SDK API, so we must maintain source compatibility. + // Using this struct for API parameters instead of individual parameters allows us + // to make necessary changes for new functionality in a single location. + struct AddRequestDataParameters { // gcc bug requires us to have the constructor below // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96645 - AdditionalCommandParameters() {} + AddRequestDataParameters() {} - AdditionalCommandParameters & SetStartOrEndDataStruct(bool aStartOrEndDataStruct) + AddRequestDataParameters(const Optional & aTimedInvokeTimeoutMs) : timedInvokeTimeoutMs(aTimedInvokeTimeoutMs) {} + + // When a value is provided for timedInvokeTimeoutMs, this invoke becomes a timed + // invoke. CommandSender will use the minimum of all provided timeouts for execution. + const Optional timedInvokeTimeoutMs; + // The command reference is required when sending multiple commands. It allows the caller + // to associate this request with its corresponding response. + Optional commandRef; + }; + + // PrepareCommand is a public SDK API, so we must maintain source compatibility. + // Using this struct for API parameters instead of individual parameters allows us + // to make necessary changes for new functionality in a single location. + struct PrepareCommandParameters + { + // gcc bug requires us to have the constructor below + // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96645 + PrepareCommandParameters() {} + + PrepareCommandParameters(const AddRequestDataParameters & aAddRequestDataParam) : + commandRef(aAddRequestDataParam.commandRef) + {} + + PrepareCommandParameters & SetStartDataStruct(bool aStartDataStruct) { - startOrEndDataStruct = aStartOrEndDataStruct; + startDataStruct = aStartDataStruct; return *this; } - // From the perspective of `PrepareCommand`, this is an out parameter. This value will be - // set by `PrepareCommand` and is expected to be unchanged by caller until it is provided - // to `FinishCommand`. + PrepareCommandParameters & SetCommandRef(uint16_t aCommandRef) + { + commandRef.SetValue(aCommandRef); + return *this; + } + // The command reference is required when sending multiple commands. It allows the caller + // to associate this request with its corresponding response. We validate the reference + // early in PrepareCommand, even though it's not used until FinishCommand. This proactive + // validation helps prevent unnecessary writing an InvokeRequest into the packet that later + // needs to be undone. + // + // Currently, provided commandRefs for the first request must start at 0 and increment by one + // for each subsequent request. This requirement can be relaxed in the future if a compelling + // need arises. + // TODO(#30453): After introducing Request/Response tracking, remove statement above about + // this currently enforced requirement on commandRefs. Optional commandRef; - // For both `PrepareCommand` and `FinishCommand` this is an in parameter. It must have - // the same value when calling `PrepareCommand` and `FinishCommand` for a given command. - bool startOrEndDataStruct = false; + // If the InvokeRequest needs to be in a state with a started data TLV struct container + bool startDataStruct = false; + }; + + // FinishCommand is a public SDK API, so we must maintain source compatibility. + // Using this struct for API parameters instead of individual parameters allows us + // to make necessary changes for new functionality in a single location. + struct FinishCommandParameters + { + // gcc bug requires us to have the constructor below + // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96645 + FinishCommandParameters() {} + + FinishCommandParameters(const Optional & aTimedInvokeTimeoutMs) : timedInvokeTimeoutMs(aTimedInvokeTimeoutMs) {} + FinishCommandParameters(const AddRequestDataParameters & aAddRequestDataParam) : + timedInvokeTimeoutMs(aAddRequestDataParam.timedInvokeTimeoutMs), commandRef(aAddRequestDataParam.commandRef) + {} + + FinishCommandParameters & SetEndDataStruct(bool aEndDataStruct) + { + endDataStruct = aEndDataStruct; + return *this; + } + + FinishCommandParameters & SetCommandRef(uint16_t aCommandRef) + { + commandRef.SetValue(aCommandRef); + return *this; + } + + // When a value is provided for timedInvokeTimeoutMs, this invoke becomes a timed + // invoke. CommandSender will use the minimum of all provided timeouts for execution. + const Optional timedInvokeTimeoutMs; + // The command reference is required when sending multiple commands. It allows the caller + // to associate this request with its corresponding response. This value must be + // the same as the one provided in PrepareCommandParameters when calling PrepareCommand. + Optional commandRef; + // If InvokeRequest is in a state where the data TLV struct container is currently open + // and FinishCommand should close it. + bool endDataStruct = false; }; /* @@ -243,24 +315,30 @@ class CommandSender final : public Messaging::ExchangeDelegate */ CHIP_ERROR SetCommandSenderConfig(ConfigParameters & aConfigParams); - CHIP_ERROR PrepareCommand(const CommandPathParams & aCommandPathParams, AdditionalCommandParameters & aOptionalArgs); + CHIP_ERROR PrepareCommand(const CommandPathParams & aCommandPathParams, PrepareCommandParameters & aPrepareCommandParams); - [[deprecated("PrepareCommand should migrate to calling PrepareCommand with AdditionalCommandParameters")]] CHIP_ERROR + [[deprecated("PrepareCommand should migrate to calling PrepareCommand with PrepareCommandParameters")]] CHIP_ERROR PrepareCommand(const CommandPathParams & aCommandPathParams, bool aStartDataStruct = true) { - AdditionalCommandParameters optionalArgs; - optionalArgs.SetStartOrEndDataStruct(aStartDataStruct); - return PrepareCommand(aCommandPathParams, optionalArgs); + PrepareCommandParameters prepareCommandParams; + prepareCommandParams.SetStartDataStruct(aStartDataStruct); + return PrepareCommand(aCommandPathParams, prepareCommandParams); } - CHIP_ERROR FinishCommand(const AdditionalCommandParameters & aOptionalArgs); + CHIP_ERROR FinishCommand(FinishCommandParameters & aFinishCommandParams); - [[deprecated("FinishCommand should migrate to calling FinishCommand with AdditionalCommandParameters")]] CHIP_ERROR + [[deprecated("FinishCommand should migrate to calling FinishCommand with FinishCommandParameters")]] CHIP_ERROR FinishCommand(bool aEndDataStruct = true) { - AdditionalCommandParameters optionalArgs; - optionalArgs.SetStartOrEndDataStruct(aEndDataStruct); - return FinishCommand(optionalArgs); + FinishCommandParameters finishCommandParams; + finishCommandParams.SetEndDataStruct(aEndDataStruct); + return FinishCommand(finishCommandParams); + } + [[deprecated("FinishCommand should migrate to calling FinishCommand with FinishCommandParameters")]] CHIP_ERROR + FinishCommand(const Optional & aTimedInvokeTimeoutMs) + { + FinishCommandParameters finishCommandParams(aTimedInvokeTimeoutMs); + return FinishCommand(finishCommandParams); } TLV::TLVWriter * GetCommandDataIBTLVWriter(); @@ -275,47 +353,27 @@ class CommandSender final : public Messaging::ExchangeDelegate * @param [in] aData The data for the request. */ template = 0> - CHIP_ERROR AddRequestData(const CommandPathParams & aCommandPath, const CommandDataT & aData, - AdditionalCommandParameters & aOptionalArgs) - { - return AddRequestData(aCommandPath, aData, NullOptional, aOptionalArgs); - } - template = 0> CHIP_ERROR AddRequestData(const CommandPathParams & aCommandPath, const CommandDataT & aData) { - AdditionalCommandParameters optionalArgs; - return AddRequestData(aCommandPath, aData, optionalArgs); + AddRequestDataParameters addRequestDataParams; + return AddRequestData(aCommandPath, aData, addRequestDataParams); } - /** - * 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, AdditionalCommandParameters & aOptionalArgs) + AddRequestDataParameters & aAddRequestDataParams) { - VerifyOrReturnError(!CommandDataT::MustUseTimedInvoke() || aTimedInvokeTimeoutMs.HasValue(), CHIP_ERROR_INVALID_ARGUMENT); - // AddRequestDataInternal encodes the data and requires startOrEndDataStruct to be false. - VerifyOrReturnError(aOptionalArgs.startOrEndDataStruct == false, CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(!CommandDataT::MustUseTimedInvoke() || aAddRequestDataParams.timedInvokeTimeoutMs.HasValue(), + CHIP_ERROR_INVALID_ARGUMENT); - return AddRequestDataInternal(aCommandPath, aData, aTimedInvokeTimeoutMs, aOptionalArgs); + return AddRequestDataInternal(aCommandPath, aData, aAddRequestDataParams); } template CHIP_ERROR AddRequestData(const CommandPathParams & aCommandPath, const CommandDataT & aData, const Optional & aTimedInvokeTimeoutMs) { - AdditionalCommandParameters optionalArgs; - return AddRequestData(aCommandPath, aData, aTimedInvokeTimeoutMs, optionalArgs); - } - - CHIP_ERROR FinishCommand(const Optional & aTimedInvokeTimeoutMs, const AdditionalCommandParameters & aOptionalArgs); - - CHIP_ERROR FinishCommand(const Optional & aTimedInvokeTimeoutMs) - { - AdditionalCommandParameters optionalArgs; - return FinishCommand(aTimedInvokeTimeoutMs, optionalArgs); + AddRequestDataParameters addRequestDataParams(aTimedInvokeTimeoutMs); + return AddRequestData(aCommandPath, aData, addRequestDataParams); } /** @@ -333,19 +391,19 @@ class CommandSender final : public Messaging::ExchangeDelegate * timeout parameter. For use in tests only. */ template - CHIP_ERROR AddRequestDataNoTimedCheck(const CommandPathParams & aCommandPath, const CommandDataT & aData, - const Optional & aTimedInvokeTimeoutMs, - AdditionalCommandParameters & aOptionalArgs) + CHIP_ERROR TestOnlyAddRequestDataNoTimedCheck(const CommandPathParams & aCommandPath, const CommandDataT & aData, + AddRequestDataParameters & aAddRequestDataParams) { - return AddRequestDataInternal(aCommandPath, aData, aTimedInvokeTimeoutMs, aOptionalArgs); + return AddRequestDataInternal(aCommandPath, aData, aAddRequestDataParams); } - template - CHIP_ERROR AddRequestDataNoTimedCheck(const CommandPathParams & aCommandPath, const CommandDataT & aData, - const Optional & aTimedInvokeTimeoutMs) + CHIP_ERROR TestOnlyFinishCommand(FinishCommandParameters & aFinishCommandParams) { - AdditionalCommandParameters optionalArgs; - return AddRequestDataNoTimedCheck(aCommandPath, aData, aTimedInvokeTimeoutMs, optionalArgs); + if (mBatchCommandsEnabled) + { + VerifyOrReturnError(aFinishCommandParams.commandRef.HasValue(), CHIP_ERROR_INVALID_ARGUMENT); + } + return FinishCommandInternal(aFinishCommandParams); } /** @@ -360,15 +418,19 @@ class CommandSender final : public Messaging::ExchangeDelegate private: template CHIP_ERROR AddRequestDataInternal(const CommandPathParams & aCommandPath, const CommandDataT & aData, - const Optional & aTimedInvokeTimeoutMs, AdditionalCommandParameters & aOptionalArgs) + AddRequestDataParameters & aAddRequestDataParams) { - ReturnErrorOnFailure(PrepareCommand(aCommandPath, aOptionalArgs)); + PrepareCommandParameters prepareCommandParams(aAddRequestDataParams); + ReturnErrorOnFailure(PrepareCommand(aCommandPath, prepareCommandParams)); TLV::TLVWriter * writer = GetCommandDataIBTLVWriter(); VerifyOrReturnError(writer != nullptr, CHIP_ERROR_INCORRECT_STATE); ReturnErrorOnFailure(DataModel::Encode(*writer, TLV::ContextTag(CommandDataIB::Tag::kFields), aData)); - return FinishCommand(aTimedInvokeTimeoutMs, aOptionalArgs); + FinishCommandParameters finishCommandParams(aAddRequestDataParams); + return FinishCommand(finishCommandParams); } + CHIP_ERROR FinishCommandInternal(FinishCommandParameters & aFinishCommandParams); + public: // Sends a queued up command request to the target encapsulated by the secureSession handle. // @@ -453,6 +515,8 @@ class CommandSender final : public Messaging::ExchangeDelegate CHIP_ERROR ProcessInvokeResponse(System::PacketBufferHandle && payload, bool & moreChunkedMessages); CHIP_ERROR ProcessInvokeResponseIB(InvokeResponseIB::Parser & aInvokeResponse); + void SetTimedInvokeTimeoutMs(const Optional & aTimedInvokeTimeoutMs); + // Send our queued-up Invoke Request message. Assumes the exchange is ready // and mPendingInvokeData is populated. CHIP_ERROR SendInvokeRequest(); diff --git a/src/app/tests/TestCommandInteraction.cpp b/src/app/tests/TestCommandInteraction.cpp index 442d155242b3ec..d67b389b1df6aa 100644 --- a/src/app/tests/TestCommandInteraction.cpp +++ b/src/app/tests/TestCommandInteraction.cpp @@ -1266,25 +1266,25 @@ void TestCommandInteraction::TestCommandSenderLegacyCallbackBuildingBatchCommand CHIP_ERROR err = CHIP_NO_ERROR; mockCommandSenderDelegate.ResetCounter(); app::CommandSender commandSender(&mockCommandSenderDelegate, &ctx.GetExchangeManager()); - app::CommandSender::AdditionalCommandParameters commandParameters; - commandParameters.SetStartOrEndDataStruct(true); + app::CommandSender::PrepareCommandParameters prepareCommandParams; + app::CommandSender::FinishCommandParameters finishCommandParams; + prepareCommandParams.SetStartDataStruct(true).SetCommandRef(0); + finishCommandParams.SetEndDataStruct(true).SetCommandRef(0); - // TODO(#30453): Once CHIP_CONFIG_SENDING_BATCH_COMMANDS_ENABLED is removed we will need - // to call SetCommandSenderConfig with remoteMaxPathsPerInvoke set to 2. commandSender.mBatchCommandsEnabled = true; commandSender.mRemoteMaxPathsPerInvoke = 2; auto commandPathParams = MakeTestCommandPath(); - err = commandSender.PrepareCommand(commandPathParams, commandParameters); + err = commandSender.PrepareCommand(commandPathParams, prepareCommandParams); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); chip::TLV::TLVWriter * writer = commandSender.GetCommandDataIBTLVWriter(); err = writer->PutBoolean(chip::TLV::ContextTag(1), true); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - err = commandSender.FinishCommand(commandParameters); + err = commandSender.FinishCommand(finishCommandParams); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); // Preparing second command. - commandParameters.commandRef.ClearValue(); - err = commandSender.PrepareCommand(commandPathParams, commandParameters); + prepareCommandParams.SetCommandRef(1); + err = commandSender.PrepareCommand(commandPathParams, prepareCommandParams); NL_TEST_ASSERT(apSuite, err == CHIP_ERROR_INCORRECT_STATE); NL_TEST_ASSERT(apSuite, GetNumActiveHandlerObjects() == 0); @@ -1297,30 +1297,31 @@ void TestCommandInteraction::TestCommandSenderExtendableCallbackBuildingBatchCom CHIP_ERROR err = CHIP_NO_ERROR; mockCommandSenderExtendedDelegate.ResetCounter(); app::CommandSender commandSender(&mockCommandSenderExtendedDelegate, &ctx.GetExchangeManager()); - app::CommandSender::AdditionalCommandParameters commandParameters; - commandParameters.SetStartOrEndDataStruct(true); + app::CommandSender::PrepareCommandParameters prepareCommandParams; + app::CommandSender::FinishCommandParameters finishCommandParams; + prepareCommandParams.SetStartDataStruct(true).SetCommandRef(0); + finishCommandParams.SetEndDataStruct(true).SetCommandRef(0); - // TODO(#30453): Once CHIP_CONFIG_SENDING_BATCH_COMMANDS_ENABLED is removed we will need - // to call SetCommandSenderConfig with remoteMaxPathsPerInvoke set to 2. commandSender.mBatchCommandsEnabled = true; commandSender.mRemoteMaxPathsPerInvoke = 2; auto commandPathParams = MakeTestCommandPath(); - err = commandSender.PrepareCommand(commandPathParams, commandParameters); + err = commandSender.PrepareCommand(commandPathParams, prepareCommandParams); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); chip::TLV::TLVWriter * writer = commandSender.GetCommandDataIBTLVWriter(); err = writer->PutBoolean(chip::TLV::ContextTag(1), true); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - err = commandSender.FinishCommand(commandParameters); + err = commandSender.FinishCommand(finishCommandParams); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); // Preparing second command. - commandParameters.commandRef.ClearValue(); - err = commandSender.PrepareCommand(commandPathParams, commandParameters); + prepareCommandParams.SetCommandRef(1); + finishCommandParams.SetCommandRef(1); + err = commandSender.PrepareCommand(commandPathParams, prepareCommandParams); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); writer = commandSender.GetCommandDataIBTLVWriter(); err = writer->PutBoolean(chip::TLV::ContextTag(1), true); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - err = commandSender.FinishCommand(commandParameters); + err = commandSender.FinishCommand(finishCommandParams); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); NL_TEST_ASSERT(apSuite, GetNumActiveHandlerObjects() == 0); diff --git a/src/app/tests/suites/commands/interaction_model/InteractionModel.h b/src/app/tests/suites/commands/interaction_model/InteractionModel.h index dbbf4246680c5f..8070c6d001b47d 100644 --- a/src/app/tests/suites/commands/interaction_model/InteractionModel.h +++ b/src/app/tests/suites/commands/interaction_model/InteractionModel.h @@ -255,7 +255,9 @@ class InteractionModelCommands mCallback, device->GetExchangeManager(), mTimedInteractionTimeoutMs.HasValue(), mSuppressResponse.ValueOr(false)); VerifyOrReturnError(commandSender != nullptr, CHIP_ERROR_NO_MEMORY); - ReturnErrorOnFailure(commandSender->AddRequestDataNoTimedCheck(commandPath, value, mTimedInteractionTimeoutMs)); + chip::app::CommandSender::AddRequestDataParameters addRequestDataParams(mTimedInteractionTimeoutMs); + // Using TestOnly AddRequestData to allow for an intentionally malformed request for server validation testing. + ReturnErrorOnFailure(commandSender->TestOnlyAddRequestDataNoTimedCheck(commandPath, value, addRequestDataParams)); ReturnErrorOnFailure(commandSender->SendCommandRequest(device->GetSecureSession().Value())); mCommandSender.push_back(std::move(commandSender)); @@ -282,7 +284,9 @@ class InteractionModelCommands VerifyOrReturnError(exchangeManager != nullptr, CHIP_ERROR_INCORRECT_STATE); chip::app::CommandSender commandSender(mCallback, exchangeManager); - ReturnErrorOnFailure(commandSender.AddRequestDataNoTimedCheck(commandPath, value, chip::NullOptional)); + chip::app::CommandSender::AddRequestDataParameters addRequestDataParams; + // Using TestOnly AddRequestData to allow for an intentionally malformed request for server validation testing. + ReturnErrorOnFailure(commandSender.TestOnlyAddRequestDataNoTimedCheck(commandPath, value, addRequestDataParams)); chip::Transport::OutgoingGroupSession session(groupId, fabricIndex); return commandSender.SendGroupCommandRequest(chip::SessionHandle(session)); diff --git a/src/controller/python/chip/clusters/command.cpp b/src/controller/python/chip/clusters/command.cpp index 87240999b360d0..417483048d0a67 100644 --- a/src/controller/python/chip/clusters/command.cpp +++ b/src/controller/python/chip/clusters/command.cpp @@ -257,13 +257,10 @@ PyChipError SendBatchCommandsInternal(void * appContext, DeviceProxy * device, u app::CommandPathParams cmdParams = { endpointId, /* group id */ 0, clusterId, commandId, (app::CommandPathFlags::kEndpointIdValid) }; - CommandSender::AdditionalCommandParameters additionalParams; + CommandSender::PrepareCommandParameters prepareCommandParams; + prepareCommandParams.commandRef.SetValue(static_cast(i)); - SuccessOrExit(err = sender->PrepareCommand(cmdParams, additionalParams)); - if (testOnlyCommandRefsOverride != nullptr) - { - additionalParams.commandRef.SetValue(testOnlyCommandRefsOverride[i]); - } + SuccessOrExit(err = sender->PrepareCommand(cmdParams, prepareCommandParams)); { auto writer = sender->GetCommandDataIBTLVWriter(); VerifyOrExit(writer != nullptr, err = CHIP_ERROR_INCORRECT_STATE); @@ -273,23 +270,32 @@ PyChipError SendBatchCommandsInternal(void * appContext, DeviceProxy * device, u SuccessOrExit(err = writer->CopyContainer(TLV::ContextTag(CommandDataIB::Tag::kFields), reader)); } - SuccessOrExit(err = sender->FinishCommand(timedRequestTimeoutMs != 0 ? Optional(timedRequestTimeoutMs) - : Optional::Missing(), - additionalParams)); + Optional timedRequestTimeout = + timedRequestTimeoutMs != 0 ? Optional(timedRequestTimeoutMs) : Optional::Missing(); + CommandSender::FinishCommandParameters finishCommandParams(timedRequestTimeout); + if (testOnlyCommandRefsOverride != nullptr) + { + finishCommandParams.commandRef.SetValue(testOnlyCommandRefsOverride[i]); + } + else + { + finishCommandParams.commandRef = prepareCommandParams.commandRef; + } + SuccessOrExit(err = sender->TestOnlyFinishCommand(finishCommandParams)); // CommandSender provides us with the CommandReference for this associated command. In order to match responses // we have to add CommandRef to index lookup. - VerifyOrExit(additionalParams.commandRef.HasValue(), err = CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrExit(finishCommandParams.commandRef.HasValue(), err = CHIP_ERROR_INVALID_ARGUMENT); if (testOnlyCommandRefsOverride != nullptr) { // Making sure the value we used to override CommandRef was actually used. - VerifyOrDie(additionalParams.commandRef.Value() == testOnlyCommandRefsOverride[i]); + VerifyOrDie(finishCommandParams.commandRef.Value() == testOnlyCommandRefsOverride[i]); // Ignoring the result of adding to index as the test might be trying to set duplicate CommandRefs. - callback->AddCommandRefToIndexLookup(additionalParams.commandRef.Value(), i); + callback->AddCommandRefToIndexLookup(finishCommandParams.commandRef.Value(), i); } else { - SuccessOrExit(err = callback->AddCommandRefToIndexLookup(additionalParams.commandRef.Value(), i)); + SuccessOrExit(err = callback->AddCommandRefToIndexLookup(finishCommandParams.commandRef.Value(), i)); } }