Skip to content

Commit 8da04a9

Browse files
committed
Changes to CommandSender and CommandHandler for supporting commands
with large payloads.
1 parent 1a8c6d2 commit 8da04a9

7 files changed

+68
-12
lines changed

src/app/CommandHandlerExchangeInterface.h

+12
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,18 @@ class CommandHandlerExchangeInterface
102102
* Called by CommandHandler to relay this information to the requester.
103103
*/
104104
virtual void ResponseDropped() = 0;
105+
106+
/**
107+
* @brief Gets the maximum size of a packet buffer to encode a Command
108+
* message. This size depends on the underlying session used by the
109+
* exchange.
110+
*
111+
* The size returned here is the size not including the prepended headers.
112+
*
113+
* Called by CommandHandler when allocating buffer for encoding the Command
114+
* response.
115+
*/
116+
virtual size_t GetCommandMaxBufferSize() = 0;
105117
};
106118

107119
} // namespace app

src/app/CommandHandlerImpl.cpp

+4-1
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,17 @@ CommandHandlerImpl::~CommandHandlerImpl()
5858

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

6465
if (!mBufferAllocated)
6566
{
6667
mCommandMessageWriter.Reset();
6768

68-
System::PacketBufferHandle commandPacket = System::PacketBufferHandle::New(chip::app::kMaxSecureSduLengthBytes);
69+
size_t commandBufferMaxSize = mpResponder->GetCommandMaxBufferSize();
70+
commandPacket = System::PacketBufferHandle::New(commandBufferMaxSize);
71+
6972
VerifyOrReturnError(!commandPacket.IsNull(), CHIP_ERROR_NO_MEMORY);
7073

7174
mCommandMessageWriter.Init(std::move(commandPacket));

src/app/CommandResponseSender.cpp

+19
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,10 @@ void CommandResponseSender::OnInvokeCommandRequest(Messaging::ExchangeContext *
206206
VerifyOrDieWithMsg(ec != nullptr, DataManagement, "Incoming exchange context should not be null");
207207
VerifyOrDieWithMsg(mState == State::ReadyForInvokeResponses, DataManagement, "state should be ReadyForInvokeResponses");
208208

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

233+
size_t CommandResponseSender::GetCommandMaxBufferSize()
234+
{
235+
VerifyOrExit(mExchangeCtx && mExchangeCtx->HasSessionHandle(),
236+
ChipLogError(DataManagement, "Session not available. Unable to infer session-specific buffer capacities."));
237+
238+
if (mExchangeCtx->GetSessionHandle()->AllowsLargePayload())
239+
{
240+
return kMaxLargeSecureSduLengthBytes;
241+
}
242+
243+
exit:
244+
245+
return kMaxSecureSduLengthBytes;
246+
}
247+
229248
#if CHIP_WITH_NLFAULTINJECTION
230249

231250
void CommandResponseSender::TestOnlyInvokeCommandRequestWithFaultsInjected(Messaging::ExchangeContext * ec,

src/app/CommandResponseSender.h

+3
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,8 @@ class CommandResponseSender : public Messaging::ExchangeDelegate,
124124

125125
void ResponseDropped() override { mReportResponseDropped = true; }
126126

127+
size_t GetCommandMaxBufferSize() override;
128+
127129
/*
128130
* Main entrypoint for this class to handle an invoke request.
129131
*
@@ -188,6 +190,7 @@ class CommandResponseSender : public Messaging::ExchangeDelegate,
188190
State mState = State::ReadyForInvokeResponses;
189191

190192
bool mReportResponseDropped = false;
193+
bool mUseLargePayloadBuffer = false;
191194
};
192195

193196
} // namespace app

src/app/CommandSender.cpp

+20-5
Original file line numberDiff line numberDiff line change
@@ -55,18 +55,19 @@ CHIP_ERROR GetRef(ParserT aParser, Optional<uint16_t> & aRef, bool commandRefReq
5555
} // namespace
5656

5757
CommandSender::CommandSender(Callback * apCallback, Messaging::ExchangeManager * apExchangeMgr, bool aIsTimedRequest,
58-
bool aSuppressResponse) :
58+
bool aSuppressResponse, bool aRequireLargePayload) :
5959
mExchangeCtx(*this),
60-
mCallbackHandle(apCallback), mpExchangeMgr(apExchangeMgr), mSuppressResponse(aSuppressResponse), mTimedRequest(aIsTimedRequest)
60+
mCallbackHandle(apCallback), mpExchangeMgr(apExchangeMgr), mSuppressResponse(aSuppressResponse), mTimedRequest(aIsTimedRequest),
61+
mRequireLargePayload(aRequireLargePayload)
6162
{
6263
assertChipStackLockedByCurrentThread();
6364
}
6465

6566
CommandSender::CommandSender(ExtendableCallback * apExtendableCallback, Messaging::ExchangeManager * apExchangeMgr,
66-
bool aIsTimedRequest, bool aSuppressResponse) :
67+
bool aIsTimedRequest, bool aSuppressResponse, bool aRequireLargePayload) :
6768
mExchangeCtx(*this),
6869
mCallbackHandle(apExtendableCallback), mpExchangeMgr(apExchangeMgr), mSuppressResponse(aSuppressResponse),
69-
mTimedRequest(aIsTimedRequest), mUseExtendableCallback(true)
70+
mTimedRequest(aIsTimedRequest), mUseExtendableCallback(true), mRequireLargePayload(aRequireLargePayload)
7071
{
7172
assertChipStackLockedByCurrentThread();
7273
#if CHIP_CONFIG_COMMAND_SENDER_BUILTIN_SUPPORT_FOR_BATCHED_COMMANDS
@@ -85,7 +86,15 @@ CHIP_ERROR CommandSender::AllocateBuffer()
8586
{
8687
mCommandMessageWriter.Reset();
8788

88-
System::PacketBufferHandle commandPacket = System::PacketBufferHandle::New(chip::app::kMaxSecureSduLengthBytes);
89+
System::PacketBufferHandle commandPacket;
90+
if (mRequireLargePayload)
91+
{
92+
commandPacket = System::PacketBufferHandle::New(kMaxLargeSecureSduLengthBytes);
93+
}
94+
else
95+
{
96+
commandPacket = System::PacketBufferHandle::New(kMaxSecureSduLengthBytes);
97+
}
8998
VerifyOrReturnError(!commandPacket.IsNull(), CHIP_ERROR_NO_MEMORY);
9099

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

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

143158
if (mTimedRequest != mTimedInvokeTimeoutMs.HasValue())
144159
{

src/app/CommandSender.h

+8-6
Original file line numberDiff line numberDiff line change
@@ -308,18 +308,19 @@ class CommandSender final : public Messaging::ExchangeDelegate
308308
* If callbacks are passed the only one that will be called in a group sesttings is the onDone
309309
*/
310310
CommandSender(Callback * apCallback, Messaging::ExchangeManager * apExchangeMgr, bool aIsTimedRequest = false,
311-
bool aSuppressResponse = false);
311+
bool aSuppressResponse = false, bool aRequireLargePayload = false);
312312
CommandSender(std::nullptr_t, Messaging::ExchangeManager * apExchangeMgr, bool aIsTimedRequest = false,
313-
bool aSuppressResponse = false) :
314-
CommandSender(static_cast<Callback *>(nullptr), apExchangeMgr, aIsTimedRequest, aSuppressResponse)
313+
bool aSuppressResponse = false, bool aRequireLargePayload = false) :
314+
CommandSender(static_cast<Callback *>(nullptr), apExchangeMgr, aIsTimedRequest, aSuppressResponse, aRequireLargePayload)
315315
{}
316316
CommandSender(ExtendableCallback * apCallback, Messaging::ExchangeManager * apExchangeMgr, bool aIsTimedRequest = false,
317-
bool aSuppressResponse = false);
317+
bool aSuppressResponse = false, bool aRequireLargePayload = false);
318318
// TODO(#32138): After there is a macro that is always defined for all unit tests, the constructor with
319319
// TestOnlyMarker should only be compiled if that macro is defined.
320320
CommandSender(TestOnlyMarker aTestMarker, ExtendableCallback * apCallback, Messaging::ExchangeManager * apExchangeMgr,
321-
PendingResponseTracker * apPendingResponseTracker, bool aIsTimedRequest = false, bool aSuppressResponse = false) :
322-
CommandSender(apCallback, apExchangeMgr, aIsTimedRequest, aSuppressResponse)
321+
PendingResponseTracker * apPendingResponseTracker, bool aIsTimedRequest = false, bool aSuppressResponse = false,
322+
bool aRequireLargePayload = false) :
323+
CommandSender(apCallback, apExchangeMgr, aIsTimedRequest, aSuppressResponse, aRequireLargePayload)
323324
{
324325
mpPendingResponseTracker = apPendingResponseTracker;
325326
}
@@ -636,6 +637,7 @@ class CommandSender final : public Messaging::ExchangeDelegate
636637
bool mBufferAllocated = false;
637638
bool mBatchCommandsEnabled = false;
638639
bool mUseExtendableCallback = false;
640+
bool mRequireLargePayload = false;
639641
};
640642

641643
} // namespace app

src/app/tests/TestCommandInteraction.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,8 @@ class MockCommandResponder : public CommandHandlerExchangeInterface
330330
void AddInvokeResponseToSend(System::PacketBufferHandle && aPacket) override { mChunks.AddToEnd(std::move(aPacket)); }
331331
void ResponseDropped() override { mResponseDropped = true; }
332332

333+
size_t GetCommandMaxBufferSize() override { return kMaxSecureSduLengthBytes; }
334+
333335
System::PacketBufferHandle mChunks;
334336
bool mResponseDropped = false;
335337
};

0 commit comments

Comments
 (0)