Skip to content

Commit

Permalink
Changes to CommandSender and CommandHandler for supporting commands
Browse files Browse the repository at this point in the history
with large payloads.
  • Loading branch information
pidarped committed Jun 12, 2024
1 parent 1a8c6d2 commit 8da04a9
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 12 deletions.
12 changes: 12 additions & 0 deletions src/app/CommandHandlerExchangeInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,18 @@ class CommandHandlerExchangeInterface
* Called by CommandHandler to relay this information to the requester.
*/
virtual void ResponseDropped() = 0;

/**
* @brief Gets the maximum size of a packet buffer to encode a Command
* message. This size depends on the underlying session used by the
* exchange.
*
* The size returned here is the size not including the prepended headers.
*
* Called by CommandHandler when allocating buffer for encoding the Command
* response.
*/
virtual size_t GetCommandMaxBufferSize() = 0;
};

} // namespace app
Expand Down
5 changes: 4 additions & 1 deletion src/app/CommandHandlerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,17 @@ CommandHandlerImpl::~CommandHandlerImpl()

CHIP_ERROR CommandHandlerImpl::AllocateBuffer()
{
System::PacketBufferHandle commandPacket = nullptr;
// We should only allocate a buffer if we will be sending out a response.
VerifyOrReturnError(ResponsesAccepted(), CHIP_ERROR_INCORRECT_STATE);

if (!mBufferAllocated)
{
mCommandMessageWriter.Reset();

System::PacketBufferHandle commandPacket = System::PacketBufferHandle::New(chip::app::kMaxSecureSduLengthBytes);
size_t commandBufferMaxSize = mpResponder->GetCommandMaxBufferSize();
commandPacket = System::PacketBufferHandle::New(commandBufferMaxSize);

VerifyOrReturnError(!commandPacket.IsNull(), CHIP_ERROR_NO_MEMORY);

mCommandMessageWriter.Init(std::move(commandPacket));
Expand Down
19 changes: 19 additions & 0 deletions src/app/CommandResponseSender.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,10 @@ void CommandResponseSender::OnInvokeCommandRequest(Messaging::ExchangeContext *
VerifyOrDieWithMsg(ec != nullptr, DataManagement, "Incoming exchange context should not be null");
VerifyOrDieWithMsg(mState == State::ReadyForInvokeResponses, DataManagement, "state should be ReadyForInvokeResponses");

if (ec->GetSessionHandle()->AllowsLargePayload())
{
mUseLargePayloadBuffer = true;
}
// NOTE: we already know this is an InvokeRequestMessage because we explicitly registered with the
// Exchange Manager for unsolicited InvokeRequestMessages.
mExchangeCtx.Grab(ec);
Expand All @@ -226,6 +230,21 @@ void CommandResponseSender::OnInvokeCommandRequest(Messaging::ExchangeContext *
}
}

size_t CommandResponseSender::GetCommandMaxBufferSize()
{
VerifyOrExit(mExchangeCtx && mExchangeCtx->HasSessionHandle(),
ChipLogError(DataManagement, "Session not available. Unable to infer session-specific buffer capacities."));

if (mExchangeCtx->GetSessionHandle()->AllowsLargePayload())
{
return kMaxLargeSecureSduLengthBytes;
}

exit:

return kMaxSecureSduLengthBytes;
}

#if CHIP_WITH_NLFAULTINJECTION

void CommandResponseSender::TestOnlyInvokeCommandRequestWithFaultsInjected(Messaging::ExchangeContext * ec,
Expand Down
3 changes: 3 additions & 0 deletions src/app/CommandResponseSender.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ class CommandResponseSender : public Messaging::ExchangeDelegate,

void ResponseDropped() override { mReportResponseDropped = true; }

size_t GetCommandMaxBufferSize() override;

/*
* Main entrypoint for this class to handle an invoke request.
*
Expand Down Expand Up @@ -188,6 +190,7 @@ class CommandResponseSender : public Messaging::ExchangeDelegate,
State mState = State::ReadyForInvokeResponses;

bool mReportResponseDropped = false;
bool mUseLargePayloadBuffer = false;
};

} // namespace app
Expand Down
25 changes: 20 additions & 5 deletions src/app/CommandSender.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,18 +55,19 @@ CHIP_ERROR GetRef(ParserT aParser, Optional<uint16_t> & aRef, bool commandRefReq
} // namespace

CommandSender::CommandSender(Callback * apCallback, Messaging::ExchangeManager * apExchangeMgr, bool aIsTimedRequest,
bool aSuppressResponse) :
bool aSuppressResponse, bool aRequireLargePayload) :
mExchangeCtx(*this),
mCallbackHandle(apCallback), mpExchangeMgr(apExchangeMgr), mSuppressResponse(aSuppressResponse), mTimedRequest(aIsTimedRequest)
mCallbackHandle(apCallback), mpExchangeMgr(apExchangeMgr), mSuppressResponse(aSuppressResponse), mTimedRequest(aIsTimedRequest),
mRequireLargePayload(aRequireLargePayload)
{
assertChipStackLockedByCurrentThread();
}

CommandSender::CommandSender(ExtendableCallback * apExtendableCallback, Messaging::ExchangeManager * apExchangeMgr,
bool aIsTimedRequest, bool aSuppressResponse) :
bool aIsTimedRequest, bool aSuppressResponse, bool aRequireLargePayload) :
mExchangeCtx(*this),
mCallbackHandle(apExtendableCallback), mpExchangeMgr(apExchangeMgr), mSuppressResponse(aSuppressResponse),
mTimedRequest(aIsTimedRequest), mUseExtendableCallback(true)
mTimedRequest(aIsTimedRequest), mUseExtendableCallback(true), mRequireLargePayload(aRequireLargePayload)
{
assertChipStackLockedByCurrentThread();
#if CHIP_CONFIG_COMMAND_SENDER_BUILTIN_SUPPORT_FOR_BATCHED_COMMANDS
Expand All @@ -85,7 +86,15 @@ CHIP_ERROR CommandSender::AllocateBuffer()
{
mCommandMessageWriter.Reset();

System::PacketBufferHandle commandPacket = System::PacketBufferHandle::New(chip::app::kMaxSecureSduLengthBytes);
System::PacketBufferHandle commandPacket;
if (mRequireLargePayload)
{
commandPacket = System::PacketBufferHandle::New(kMaxLargeSecureSduLengthBytes);
}
else
{
commandPacket = System::PacketBufferHandle::New(kMaxSecureSduLengthBytes);
}
VerifyOrReturnError(!commandPacket.IsNull(), CHIP_ERROR_NO_MEMORY);

mCommandMessageWriter.Init(std::move(commandPacket));
Expand Down Expand Up @@ -139,6 +148,12 @@ CHIP_ERROR CommandSender::TestOnlyCommandSenderTimedRequestFlagWithNoTimedInvoke

CHIP_ERROR CommandSender::SendCommandRequest(const SessionHandle & session, Optional<System::Clock::Timeout> timeout)
{
// If the command is expected to be large, ensure that the underlying
// session supports it.
if (mRequireLargePayload)
{
VerifyOrReturnError(session->AllowsLargePayload(), CHIP_ERROR_INCORRECT_STATE);
}

if (mTimedRequest != mTimedInvokeTimeoutMs.HasValue())
{
Expand Down
14 changes: 8 additions & 6 deletions src/app/CommandSender.h
Original file line number Diff line number Diff line change
Expand Up @@ -308,18 +308,19 @@ class CommandSender final : public Messaging::ExchangeDelegate
* If callbacks are passed the only one that will be called in a group sesttings is the onDone
*/
CommandSender(Callback * apCallback, Messaging::ExchangeManager * apExchangeMgr, bool aIsTimedRequest = false,
bool aSuppressResponse = false);
bool aSuppressResponse = false, bool aRequireLargePayload = false);
CommandSender(std::nullptr_t, Messaging::ExchangeManager * apExchangeMgr, bool aIsTimedRequest = false,
bool aSuppressResponse = false) :
CommandSender(static_cast<Callback *>(nullptr), apExchangeMgr, aIsTimedRequest, aSuppressResponse)
bool aSuppressResponse = false, bool aRequireLargePayload = false) :
CommandSender(static_cast<Callback *>(nullptr), apExchangeMgr, aIsTimedRequest, aSuppressResponse, aRequireLargePayload)
{}
CommandSender(ExtendableCallback * apCallback, Messaging::ExchangeManager * apExchangeMgr, bool aIsTimedRequest = false,
bool aSuppressResponse = false);
bool aSuppressResponse = false, bool aRequireLargePayload = false);
// TODO(#32138): After there is a macro that is always defined for all unit tests, the constructor with
// TestOnlyMarker should only be compiled if that macro is defined.
CommandSender(TestOnlyMarker aTestMarker, ExtendableCallback * apCallback, Messaging::ExchangeManager * apExchangeMgr,
PendingResponseTracker * apPendingResponseTracker, bool aIsTimedRequest = false, bool aSuppressResponse = false) :
CommandSender(apCallback, apExchangeMgr, aIsTimedRequest, aSuppressResponse)
PendingResponseTracker * apPendingResponseTracker, bool aIsTimedRequest = false, bool aSuppressResponse = false,
bool aRequireLargePayload = false) :
CommandSender(apCallback, apExchangeMgr, aIsTimedRequest, aSuppressResponse, aRequireLargePayload)
{
mpPendingResponseTracker = apPendingResponseTracker;
}
Expand Down Expand Up @@ -636,6 +637,7 @@ class CommandSender final : public Messaging::ExchangeDelegate
bool mBufferAllocated = false;
bool mBatchCommandsEnabled = false;
bool mUseExtendableCallback = false;
bool mRequireLargePayload = false;
};

} // namespace app
Expand Down
2 changes: 2 additions & 0 deletions src/app/tests/TestCommandInteraction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,8 @@ class MockCommandResponder : public CommandHandlerExchangeInterface
void AddInvokeResponseToSend(System::PacketBufferHandle && aPacket) override { mChunks.AddToEnd(std::move(aPacket)); }
void ResponseDropped() override { mResponseDropped = true; }

size_t GetCommandMaxBufferSize() override { return kMaxSecureSduLengthBytes; }

System::PacketBufferHandle mChunks;
bool mResponseDropped = false;
};
Expand Down

0 comments on commit 8da04a9

Please sign in to comment.