Skip to content

Commit

Permalink
Changes to CommandSender and CommandHandler for supporting commands (p…
Browse files Browse the repository at this point in the history
…roject-chip#33889)

with large payloads.
  • Loading branch information
pidarped committed Jun 14, 2024
1 parent 2f284f4 commit ec8547a
Show file tree
Hide file tree
Showing 7 changed files with 63 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
* Response 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 GetCommandResponseMaxBufferSize() = 0;
};

} // namespace app
Expand Down
4 changes: 3 additions & 1 deletion src/app/CommandHandlerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ CHIP_ERROR CommandHandlerImpl::AllocateBuffer()
{
mCommandMessageWriter.Reset();

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

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

mCommandMessageWriter.Init(std::move(commandPacket));
Expand Down
16 changes: 16 additions & 0 deletions src/app/CommandResponseSender.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,22 @@ void CommandResponseSender::OnInvokeCommandRequest(Messaging::ExchangeContext *
}
}

size_t CommandResponseSender::GetCommandResponseMaxBufferSize()
{
if (!mExchangeCtx || !mExchangeCtx->HasSessionHandle())
{
ChipLogError(DataManagement, "Session not available. Unable to infer session-specific buffer capacities.");
return kMaxSecureSduLengthBytes;
}

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

return kMaxSecureSduLengthBytes;
}

#if CHIP_WITH_NLFAULTINJECTION

void CommandResponseSender::TestOnlyInvokeCommandRequestWithFaultsInjected(Messaging::ExchangeContext * ec,
Expand Down
2 changes: 2 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 GetCommandResponseMaxBufferSize() override;

/*
* Main entrypoint for this class to handle an invoke request.
*
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 aAllowLargePayload) :
mExchangeCtx(*this),
mCallbackHandle(apCallback), mpExchangeMgr(apExchangeMgr), mSuppressResponse(aSuppressResponse), mTimedRequest(aIsTimedRequest)
mCallbackHandle(apCallback), mpExchangeMgr(apExchangeMgr), mSuppressResponse(aSuppressResponse), mTimedRequest(aIsTimedRequest),
mAllowLargePayload(aAllowLargePayload)
{
assertChipStackLockedByCurrentThread();
}

CommandSender::CommandSender(ExtendableCallback * apExtendableCallback, Messaging::ExchangeManager * apExchangeMgr,
bool aIsTimedRequest, bool aSuppressResponse) :
bool aIsTimedRequest, bool aSuppressResponse, bool aAllowLargePayload) :
mExchangeCtx(*this),
mCallbackHandle(apExtendableCallback), mpExchangeMgr(apExchangeMgr), mSuppressResponse(aSuppressResponse),
mTimedRequest(aIsTimedRequest), mUseExtendableCallback(true)
mTimedRequest(aIsTimedRequest), mUseExtendableCallback(true), mAllowLargePayload(aAllowLargePayload)
{
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 (mAllowLargePayload)
{
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 (mAllowLargePayload)
{
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 aAllowLargePayload = 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 aAllowLargePayload = false) :
CommandSender(static_cast<Callback *>(nullptr), apExchangeMgr, aIsTimedRequest, aSuppressResponse, aAllowLargePayload)
{}
CommandSender(ExtendableCallback * apCallback, Messaging::ExchangeManager * apExchangeMgr, bool aIsTimedRequest = false,
bool aSuppressResponse = false);
bool aSuppressResponse = false, bool aAllowLargePayload = 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 aAllowLargePayload = false) :
CommandSender(apCallback, apExchangeMgr, aIsTimedRequest, aSuppressResponse, aAllowLargePayload)
{
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 mAllowLargePayload = 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 GetCommandResponseMaxBufferSize() override { return kMaxSecureSduLengthBytes; }

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

0 comments on commit ec8547a

Please sign in to comment.