You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
adding to --trace-to json:log or --trace-to json:/tmp/messages.json will make for a good debug experience. It probably works without as well, but instead of saying nice text of CommandRef: ... it will be something like Context(2): ...
Remove CommandResponseSender from friend in CommandHandler. Introduced in #31378. Needed right now for CommandResponseSenderto callClose()`
Add unit testing for TestOnlyInvokeCommandRequestWithFaultsInjected
TODO issues discovered while working on this feature that would be good to follow up on
When CommandHandler fails to send the first message (when calling CommandHandler::StartSendingCommandResponses), we should be sending back a failure if possible. Existing unit tests start failing when this was added in
It is pretty dangerous for CommandHandler to provide access to GetExchangeContext(). Unfortunately there is already a precedent of using in, but it seems like everything wants info that could be alternatively provided instead of giving access to the ExchangeContext directly. More info here
CommandHandler::GetCommandDataIBTLVWriter should not be public. This is a footgun in causing issue with CommandHandler state that is a larger issue for batch commands
Rolling back InvokeResponseMessag and InvokeRequestMessage's Builders requires manually resetting internal errors directly instead of it being handled for us. This would be a good quality of life improvement.
Nice to complete TODO, but shouldn't affect feature launch
CommandSender
times out figure out what we need to do.Remove comment about what should be done for batch commands on timeout #32261
CommandSender::Callback
andCommandSender::ExtendableCallback
.Reorder
CommandSender::Callback
andCommandSender::ExtendableCallback
. #31370CommandPathRegistry * mCommandPathRegistry
for the default casemRefForResponse
in CommandHandler, by passing the value on stack instead of taking up memory here.ref
to help with automated decoding and it understanding datastructures in the future.I believe a "ref" item is needed after the spec updates and that would make automated decoding work a bit better and it helps understanding data structures in the future.
--trace-to json:log
or--trace-to json:/tmp/messages.json
will make for a good debug experience. It probably works without as well, but instead of saying nice text ofCommandRef: ...
it will be something likeContext(2): ...
CommandResponseSender
from friend inCommandHandler. Introduced in #31378. Needed right now for
CommandResponseSenderto call
Close()`Cleanup and fix batch command tests in TestCommandInteraction #32288
SetExchangeContext
to internally callWillSendMessage
. But unit tests start failingNo longer relevant after refactor Refactor CommandHandler to be more unittestable #32467
TestCommandInteraction::FillCurrentInvokeResponseBuffer
uses a magic number to fill upInvokeResponseMessage
exactly. This should be derived from other sources instead of being a magic numberReplace hardcoded assumption with one dynamically calculated #32291
TestOnlyInvokeCommandRequestWithFaultsInjected
TODO issues discovered while working on this feature that would be good to follow up on
CommandHandler::StartSendingCommandResponses
), we should be sending back a failure if possible. Existing unit tests start failing when this was added inIf this action is marked with TimedRequest as TRUE, but this action is not part of a Timed Invoke transaction (i.e. there was no immediately previous Timed Invoke action), then a Status Response action with the TIMED_REQUEST_MISMATCH Status Code SHALL be submitted to the message layer and this interaction SHALL terminate.
But code send UnsupportedAccess. Note TestPlan expected behavior is looking for
UnsupportedAccess
https://github.com/CHIP-Specifications/chip-test-plans/pull/3841
Fix IM error when TimeRequest flag doesn't state of isTimedInvoke #31485
CommandHandler
to provide access toGetExchangeContext()
. Unfortunately there is already a precedent of using in, but it seems like everything wants info that could be alternatively provided instead of giving access to the ExchangeContext directly. More info hereCommandHandler::GetCommandDataIBTLVWriter
should not be public. This is a footgun in causing issue withCommandHandler
state that is a larger issue for batch commandsInvokeResponseMessag
andInvokeRequestMessage
'sBuilders
requires manually resetting internal errors directly instead of it being handled for us. This would be a good quality of life improvement.Archived section (as work is completed)
TODO before FC (Dec 2023):
Update session parameters to be inline with matter 1.3 spec #30405
Add Spec version and Max Paths Per Invoke to Basic Information cluster #30496
CommandDataIB
to allow Building and Parsing multiple commandsChanges to CommandDataIB to reflect 1.3 spec #30522
InvokeResponseMessage
to allow Building and ParsingMoreChunkedMessages
Changes to InvokeResponseMessage to reflect 1.3 spec #30639
SessionParamters
plumbing so that SDK APIs knowMaxPathsPerInvoke
how many batch commands remote peer can accept.Allow plumbing of
SessionParameters
fromSession
#30525CommandSender
to support sending multiple commands.Update
CommandSender
to support sending batch commands #30675CommandHandler
to support receiving and processing multiple commands.Update
CommandHandler
to support batch commands #30614Introduce CommandSender::ExtendedCallback #31324
CSG Cert work (Before TE2):
Enable chip-repl to send batch commands #30851
Implement updates to IDM test spec for batch commands #31042
Add TestOnlySendBatchCommands to chip-repl #31455
Update skipped test step in IDM_1_4 #31458
https://github.com/CHIP-Specifications/chip-test-plans/pull/3734
Enable batch commands for the linux example all-clusters-app #31531
Critical bug fixes required (Before TE2):
Enabling MoreChunkedMessages for Invoke Interaction #31378
CommandHandler
actually send multipleInvokeResponseMessage
. We should also make sure that when we run out of space, send out all currently complete command responsesUpon running out of space in InvokeResponseMessage for additional responses, allocate another #31516
Between TE2 and SVE
These items focus on improvements not with the feature itself, but with QoL improvements for those to more easily send or receive batch commands:
CommandSender
APIs to be more extendable in the futureRefactor CommandSender newly added APIs to be more extendable #31837
CommandSender
should be responsible for managing/matching response to requests that have come back, instead of application code.CommandSender: For batch commands, track requests are responded to #32105
Allow chip-repl to know how many InvokeResponseMessages were recieved #31781
PayloadTestRequest
to finish off all automation for IDM_1_4Add test steps to IDM_1_4 to validate multiple InvokeResponseMessages #31979
Use FaultInjection to help with IDM_1_3 automation testing #32045
Update IDM_1_3 yaml to latest testplan spec which uses fault injection #32149
Important TODOs:
PairingSession::EncodeSessionParameters
figure out what MRP values we should sent whenprovidedMrpConfig.HasValue() == false
#30456Have PairingSession::EncodeSessionParameters require local MRP values #32324
Fix incorrectly reported NoReponse when single batch command sent to … #32362
Add batch invoke integration tests to CI #31642
Now that we haveExtendedCallbacks
we should removeCHIP_CONFIG_SENDING_BATCH_COMMANDS_ENABLED
. For context read Introduce CommandSender::ExtendedCallback #31324Remove CHIP_CONFIG_SENDING_BATCH_COMMANDS_ENABLED #31371CommandSender
andCommandHandler
CommandSender and CommandHandler reserves space for Finalize to succeed #31077
Prevent TLVWrite from possibly performing buffer overrun if trying to reserve #30714
InvokeRequestMessage
before processing any commandsValidate InvokeRequestMessage TLV properly terminated before running commands #31218
Add batch invoke integration tests to CI #31642
The text was updated successfully, but these errors were encountered: