Skip to content

Commit 21b998a

Browse files
Allow chip-repl to know how many InvokeResponseMessages were recieved (project-chip#31781)
* Allow chip-repl to know how many InvokeResponseMessages were recieved * Restyled by autopep8 * Clean up callback to be test only specifically * Restyled by clang-format * Address PR comments * Small edit to add line that shouldn't be removed in this PR * Address PR comment --------- Co-authored-by: Restyled.io <commits@restyled.io>
1 parent 20e4359 commit 21b998a

File tree

11 files changed

+122
-27
lines changed

11 files changed

+122
-27
lines changed

src/app/CommandSender.cpp

+6
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ CHIP_ERROR CommandSender::OnMessageReceived(Messaging::ExchangeContext * apExcha
224224
{
225225
err = ProcessInvokeResponse(std::move(aPayload), moreChunkedMessages);
226226
SuccessOrExit(err);
227+
mInvokeResponseMessageCount++;
227228
if (moreChunkedMessages)
228229
{
229230
StatusResponse::Send(Status::Success, apExchangeContext, /*aExpectResponse = */ true);
@@ -534,6 +535,11 @@ CHIP_ERROR CommandSender::FinishCommand(const Optional<uint16_t> & aTimedInvokeT
534535
return CHIP_NO_ERROR;
535536
}
536537

538+
size_t CommandSender::GetInvokeResponseMessageCount()
539+
{
540+
return static_cast<size_t>(mInvokeResponseMessageCount);
541+
}
542+
537543
CHIP_ERROR CommandSender::Finalize(System::PacketBufferHandle & commandPacket)
538544
{
539545
VerifyOrReturnError(mState == State::AddedCommand, CHIP_ERROR_INCORRECT_STATE);

src/app/CommandSender.h

+12-2
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,14 @@ class CommandSender final : public Messaging::ExchangeDelegate
318318
return FinishCommand(aTimedInvokeTimeoutMs, optionalArgs);
319319
}
320320

321+
/**
322+
* @brief Returns the number of InvokeResponseMessages received.
323+
*
324+
* Responses to multiple requests might be split across several InvokeResponseMessages.
325+
* This function helps track the total count. Primarily for test validation purposes.
326+
*/
327+
size_t GetInvokeResponseMessageCount();
328+
321329
#if CONFIG_BUILD_FOR_HOST_UNIT_TEST
322330
/**
323331
* Version of AddRequestData that allows sending a message that is
@@ -507,8 +515,10 @@ class CommandSender final : public Messaging::ExchangeDelegate
507515
TLV::TLVType mDataElementContainerType = TLV::kTLVType_NotSpecified;
508516

509517
chip::System::PacketBufferTLVWriter mCommandMessageWriter;
510-
uint16_t mFinishedCommandCount = 0;
511-
uint16_t mRemoteMaxPathsPerInvoke = 1;
518+
519+
uint16_t mInvokeResponseMessageCount = 0;
520+
uint16_t mFinishedCommandCount = 0;
521+
uint16_t mRemoteMaxPathsPerInvoke = 1;
512522

513523
State mState = State::Idle;
514524
bool mSuppressResponse = false;

src/app/tests/TestCommandInteraction.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -883,6 +883,7 @@ void TestCommandInteraction::TestCommandInvalidMessage1(nlTestSuite * apSuite, v
883883
NL_TEST_ASSERT(apSuite,
884884
mockCommandSenderDelegate.onResponseCalledTimes == 0 && mockCommandSenderDelegate.onFinalCalledTimes == 1 &&
885885
mockCommandSenderDelegate.onErrorCalledTimes == 1);
886+
NL_TEST_ASSERT(apSuite, commandSender.GetInvokeResponseMessageCount() == 0);
886887

887888
ctx.DrainAndServiceIO();
888889

@@ -1338,12 +1339,14 @@ void TestCommandInteraction::TestCommandSenderCommandSuccessResponseFlow(nlTestS
13381339
err = commandSender.SendCommandRequest(ctx.GetSessionBobToAlice());
13391340

13401341
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
1342+
NL_TEST_ASSERT(apSuite, commandSender.GetInvokeResponseMessageCount() == 0);
13411343

13421344
ctx.DrainAndServiceIO();
13431345

13441346
NL_TEST_ASSERT(apSuite,
13451347
mockCommandSenderDelegate.onResponseCalledTimes == 1 && mockCommandSenderDelegate.onFinalCalledTimes == 1 &&
13461348
mockCommandSenderDelegate.onErrorCalledTimes == 0);
1349+
NL_TEST_ASSERT(apSuite, commandSender.GetInvokeResponseMessageCount() == 1);
13471350

13481351
NL_TEST_ASSERT(apSuite, GetNumActiveHandlerObjects() == 0);
13491352
NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0);

src/controller/python/chip/ChipDeviceCtrl.py

+3
Original file line numberDiff line numberDiff line change
@@ -867,6 +867,9 @@ async def TestOnlySendBatchCommands(self, nodeid: int, commands: typing.List[Clu
867867
remoteMaxPathsPerInvoke: Overrides the number of batch commands we think can be sent to remote node.
868868
suppressTimedRequestMessage: When set to true, we suppress sending Timed Request Message.
869869
commandRefsOverride: List of commandRefs to use for each command with the same index in `commands`.
870+
871+
Returns:
872+
- TestOnlyBatchCommandResponse
870873
'''
871874
self.CheckIsActive()
872875

src/controller/python/chip/clusters/Command.py

+33-4
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727

2828
import chip.exceptions
2929
import chip.interaction_model
30-
from chip.interaction_model import PyInvokeRequestData, TestOnlyPyBatchCommandsOverrides
30+
from chip.interaction_model import PyInvokeRequestData, TestOnlyPyBatchCommandsOverrides, TestOnlyPyOnDoneInfo
3131
from chip.native import PyChipError
3232

3333
from .ClusterObjects import ClusterCommand
@@ -56,6 +56,12 @@ class Status:
5656
ClusterStatus: int
5757

5858

59+
@dataclass
60+
class TestOnlyBatchCommandResponse:
61+
Responses: object
62+
ResponseMessageCount: int
63+
64+
5965
def FindCommandClusterObject(isClientSideCommand: bool, path: CommandPath):
6066
''' Locates the right generated cluster object given a set of parameters.
6167
@@ -215,12 +221,29 @@ def handleDone(self):
215221
)
216222

217223

224+
class TestOnlyAsyncBatchCommandsTransaction(AsyncBatchCommandsTransaction):
225+
def __init__(self, future: Future, eventLoop, expectTypes: List[Type]):
226+
self._responseMessageCount = 0
227+
super().__init__(future, eventLoop, expectTypes)
228+
229+
def _handleDone(self):
230+
# Future might already be set with exception from `handleError`
231+
if not self._future.done():
232+
self._future.set_result(TestOnlyBatchCommandResponse(self._responses, self._responseMessageCount))
233+
ctypes.pythonapi.Py_DecRef(ctypes.py_object(self))
234+
235+
def testOnlyDoneInfo(self, testOnlyDoneInfo: TestOnlyPyOnDoneInfo):
236+
self._responseMessageCount = testOnlyDoneInfo.responseMessageCount
237+
238+
218239
_OnCommandSenderResponseCallbackFunct = CFUNCTYPE(
219240
None, py_object, c_uint16, c_uint32, c_uint32, c_size_t, c_uint16, c_uint8, c_void_p, c_uint32)
220241
_OnCommandSenderErrorCallbackFunct = CFUNCTYPE(
221242
None, py_object, c_uint16, c_uint8, PyChipError)
222243
_OnCommandSenderDoneCallbackFunct = CFUNCTYPE(
223244
None, py_object)
245+
_TestOnlyOnCommandSenderDoneCallbackFunct = CFUNCTYPE(
246+
None, py_object, TestOnlyPyOnDoneInfo)
224247

225248

226249
@_OnCommandSenderResponseCallbackFunct
@@ -241,6 +264,12 @@ def _OnCommandSenderDoneCallback(closure):
241264
closure.handleDone()
242265

243266

267+
@_TestOnlyOnCommandSenderDoneCallbackFunct
268+
def _TestOnlyOnCommandSenderDoneCallback(closure, testOnlyDoneInfo: TestOnlyPyOnDoneInfo):
269+
closure.testOnlyDoneInfo(testOnlyDoneInfo)
270+
closure.handleDone()
271+
272+
244273
def TestOnlySendCommandTimedRequestFlagWithNoTimedInvoke(future: Future, eventLoop, responseType, device, commandPath, payload):
245274
''' ONLY TO BE USED FOR TEST: Sends the payload with a TimedRequest flag but no TimedInvoke transaction
246275
'''
@@ -392,7 +421,7 @@ def TestOnlySendBatchCommands(future: Future, eventLoop, device, commands: List[
392421
pyBatchCommandsData = _BuildPyInvokeRequestData(commands, timedRequestTimeoutMs,
393422
responseTypes, suppressTimedRequestMessage=suppressTimedRequestMessage)
394423

395-
transaction = AsyncBatchCommandsTransaction(future, eventLoop, responseTypes)
424+
transaction = TestOnlyAsyncBatchCommandsTransaction(future, eventLoop, responseTypes)
396425
ctypes.pythonapi.Py_IncRef(ctypes.py_object(transaction))
397426

398427
testOnlyOverrides = TestOnlyPyBatchCommandsOverrides()
@@ -448,7 +477,7 @@ def Init():
448477
setter.Set('pychip_CommandSender_SendGroupCommand',
449478
PyChipError, [c_uint16, c_void_p, c_uint32, c_uint32, c_char_p, c_size_t, c_uint16])
450479
setter.Set('pychip_CommandSender_InitCallbacks', None, [
451-
_OnCommandSenderResponseCallbackFunct, _OnCommandSenderErrorCallbackFunct, _OnCommandSenderDoneCallbackFunct])
480+
_OnCommandSenderResponseCallbackFunct, _OnCommandSenderErrorCallbackFunct, _OnCommandSenderDoneCallbackFunct, _TestOnlyOnCommandSenderDoneCallbackFunct])
452481

453482
handle.pychip_CommandSender_InitCallbacks(
454-
_OnCommandSenderResponseCallback, _OnCommandSenderErrorCallback, _OnCommandSenderDoneCallback)
483+
_OnCommandSenderResponseCallback, _OnCommandSenderErrorCallback, _OnCommandSenderDoneCallback, _TestOnlyOnCommandSenderDoneCallback)

src/controller/python/chip/clusters/command.cpp

+37-16
Original file line numberDiff line numberDiff line change
@@ -55,24 +55,26 @@ PyChipError pychip_CommandSender_SendGroupCommand(chip::GroupId groupId, chip::C
5555
namespace chip {
5656
namespace python {
5757

58-
using OnCommandSenderResponseCallback = void (*)(PyObject appContext, chip::EndpointId endpointId, chip::ClusterId clusterId,
58+
using OnCommandSenderResponseCallback = void (*)(PyObject appContext, chip::EndpointId endpointId, chip::ClusterId clusterId,
5959
chip::CommandId commandId, size_t index,
6060
std::underlying_type_t<Protocols::InteractionModel::Status> status,
6161
chip::ClusterStatus clusterStatus, const uint8_t * payload, uint32_t length);
62-
using OnCommandSenderErrorCallback = void (*)(PyObject appContext,
62+
using OnCommandSenderErrorCallback = void (*)(PyObject appContext,
6363
std::underlying_type_t<Protocols::InteractionModel::Status> status,
6464
chip::ClusterStatus clusterStatus, PyChipError chiperror);
65-
using OnCommandSenderDoneCallback = void (*)(PyObject appContext);
65+
using OnCommandSenderDoneCallback = void (*)(PyObject appContext);
66+
using TestOnlyOnCommandSenderDoneCallback = void (*)(PyObject appContext, python::TestOnlyPyOnDoneInfo testOnlyDoneInfo);
6667

67-
OnCommandSenderResponseCallback gOnCommandSenderResponseCallback = nullptr;
68-
OnCommandSenderErrorCallback gOnCommandSenderErrorCallback = nullptr;
69-
OnCommandSenderDoneCallback gOnCommandSenderDoneCallback = nullptr;
68+
OnCommandSenderResponseCallback gOnCommandSenderResponseCallback = nullptr;
69+
OnCommandSenderErrorCallback gOnCommandSenderErrorCallback = nullptr;
70+
OnCommandSenderDoneCallback gOnCommandSenderDoneCallback = nullptr;
71+
TestOnlyOnCommandSenderDoneCallback gTestOnlyOnCommandSenderDoneCallback = nullptr;
7072

7173
class CommandSenderCallback : public CommandSender::ExtendableCallback
7274
{
7375
public:
74-
CommandSenderCallback(PyObject appContext, bool isBatchedCommands) :
75-
mAppContext(appContext), mIsBatchedCommands(isBatchedCommands)
76+
CommandSenderCallback(PyObject appContext, bool isBatchedCommands, bool callTestOnlyOnDone) :
77+
mAppContext(appContext), mIsBatchedCommands(isBatchedCommands), mCallTestOnlyOnDone(callTestOnlyOnDone)
7678
{}
7779

7880
void OnResponse(CommandSender * apCommandSender, const CommandSender::ResponseData & aResponseData) override
@@ -148,7 +150,17 @@ class CommandSenderCallback : public CommandSender::ExtendableCallback
148150

149151
void OnDone(CommandSender * apCommandSender) override
150152
{
151-
gOnCommandSenderDoneCallback(mAppContext);
153+
if (mCallTestOnlyOnDone)
154+
{
155+
python::TestOnlyPyOnDoneInfo testOnlyOnDoneInfo;
156+
testOnlyOnDoneInfo.responseMessageCount = apCommandSender->GetInvokeResponseMessageCount();
157+
gTestOnlyOnCommandSenderDoneCallback(mAppContext, testOnlyOnDoneInfo);
158+
}
159+
else
160+
{
161+
gOnCommandSenderDoneCallback(mAppContext);
162+
}
163+
152164
delete apCommandSender;
153165
delete this;
154166
};
@@ -179,6 +191,7 @@ class CommandSenderCallback : public CommandSender::ExtendableCallback
179191
PyObject mAppContext = nullptr;
180192
std::unordered_map<uint16_t, size_t> commandRefToIndex;
181193
bool mIsBatchedCommands;
194+
bool mCallTestOnlyOnDone;
182195
};
183196

184197
PyChipError SendBatchCommandsInternal(void * appContext, DeviceProxy * device, uint16_t timedRequestTimeoutMs,
@@ -220,8 +233,10 @@ PyChipError SendBatchCommandsInternal(void * appContext, DeviceProxy * device, u
220233
config.SetRemoteMaxPathsPerInvoke(remoteSessionParameters.GetMaxPathsPerInvoke());
221234
}
222235

236+
bool isBatchedCommands = true;
237+
bool callTestOnlyOnDone = testOnlyOverrides != nullptr;
223238
std::unique_ptr<CommandSenderCallback> callback =
224-
std::make_unique<CommandSenderCallback>(appContext, /* isBatchedCommands =*/true);
239+
std::make_unique<CommandSenderCallback>(appContext, isBatchedCommands, callTestOnlyOnDone);
225240

226241
bool isTimedRequest = timedRequestTimeoutMs != 0 || testOnlySuppressTimedRequestMessage;
227242
std::unique_ptr<CommandSender> sender =
@@ -315,11 +330,13 @@ using namespace chip::python;
315330
extern "C" {
316331
void pychip_CommandSender_InitCallbacks(OnCommandSenderResponseCallback onCommandSenderResponseCallback,
317332
OnCommandSenderErrorCallback onCommandSenderErrorCallback,
318-
OnCommandSenderDoneCallback onCommandSenderDoneCallback)
333+
OnCommandSenderDoneCallback onCommandSenderDoneCallback,
334+
TestOnlyOnCommandSenderDoneCallback testOnlyOnCommandSenderDoneCallback)
319335
{
320-
gOnCommandSenderResponseCallback = onCommandSenderResponseCallback;
321-
gOnCommandSenderErrorCallback = onCommandSenderErrorCallback;
322-
gOnCommandSenderDoneCallback = onCommandSenderDoneCallback;
336+
gOnCommandSenderResponseCallback = onCommandSenderResponseCallback;
337+
gOnCommandSenderErrorCallback = onCommandSenderErrorCallback;
338+
gOnCommandSenderDoneCallback = onCommandSenderDoneCallback;
339+
gTestOnlyOnCommandSenderDoneCallback = testOnlyOnCommandSenderDoneCallback;
323340
}
324341

325342
PyChipError pychip_CommandSender_SendCommand(void * appContext, DeviceProxy * device, uint16_t timedRequestTimeoutMs,
@@ -331,8 +348,10 @@ PyChipError pychip_CommandSender_SendCommand(void * appContext, DeviceProxy * de
331348

332349
VerifyOrReturnError(device->GetSecureSession().HasValue(), ToPyChipError(CHIP_ERROR_MISSING_SECURE_SESSION));
333350

351+
bool isBatchedCommands = false;
352+
bool callTestOnlyOnDone = false;
334353
std::unique_ptr<CommandSenderCallback> callback =
335-
std::make_unique<CommandSenderCallback>(appContext, /* isBatchedCommands =*/false);
354+
std::make_unique<CommandSenderCallback>(appContext, isBatchedCommands, callTestOnlyOnDone);
336355
std::unique_ptr<CommandSender> sender =
337356
std::make_unique<CommandSender>(callback.get(), device->GetExchangeManager(),
338357
/* is timed request */ timedRequestTimeoutMs != 0, suppressResponse);
@@ -406,8 +425,10 @@ PyChipError pychip_CommandSender_TestOnlySendCommandTimedRequestNoTimedInvoke(
406425

407426
VerifyOrReturnError(device->GetSecureSession().HasValue(), ToPyChipError(CHIP_ERROR_MISSING_SECURE_SESSION));
408427

428+
bool isBatchedCommands = false;
429+
bool callTestOnlyOnDone = false;
409430
std::unique_ptr<CommandSenderCallback> callback =
410-
std::make_unique<CommandSenderCallback>(appContext, /* isBatchedCommands =*/false);
431+
std::make_unique<CommandSenderCallback>(appContext, isBatchedCommands, callTestOnlyOnDone);
411432
std::unique_ptr<CommandSender> sender = std::make_unique<CommandSender>(callback.get(), device->GetExchangeManager(),
412433
/* is timed request */ true, suppressResponse);
413434

src/controller/python/chip/interaction_model/Delegate.h

+5
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,11 @@ struct TestOnlyPyBatchCommandsOverrides
7171
size_t overrideCommandRefsListLength;
7272
};
7373

74+
struct TestOnlyPyOnDoneInfo
75+
{
76+
size_t responseMessageCount;
77+
};
78+
7479
} // namespace python
7580

7681
namespace Controller {

src/controller/python/chip/interaction_model/__init__.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,11 @@
2828

2929
from .delegate import (AttributePath, AttributePathIBstruct, DataVersionFilterIBstruct, EventPath, EventPathIBstruct,
3030
PyInvokeRequestData, PyWriteAttributeData, SessionParameters, SessionParametersStruct,
31-
TestOnlyPyBatchCommandsOverrides)
31+
TestOnlyPyBatchCommandsOverrides, TestOnlyPyOnDoneInfo)
3232

3333
__all__ = ["AttributePath", "AttributePathIBstruct", "DataVersionFilterIBstruct",
3434
"EventPath", "EventPathIBstruct", "InteractionModelError", "PyInvokeRequestData",
35-
"PyWriteAttributeData", "SessionParameters", "SessionParametersStruct", "Status", "TestOnlyPyBatchCommandsOverrides"]
35+
"PyWriteAttributeData", "SessionParameters", "SessionParametersStruct", "Status", "TestOnlyPyBatchCommandsOverrides", "TestOnlyPyOnDoneInfo"]
3636

3737

3838
# defined src/controller/python/chip/interaction_model/Delegate.h

src/controller/python/chip/interaction_model/delegate.py

+15
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,21 @@ class TestOnlyPyBatchCommandsOverrides(ctypes.Structure):
218218
('overrideCommandRefsList', POINTER(ctypes.c_uint16)), ('overrideCommandRefsListLength', ctypes.c_size_t)]
219219

220220

221+
class TestOnlyPyOnDoneInfo(ctypes.Structure):
222+
''' TestOnly struct for overriding aspects of batch command to send invalid commands.
223+
224+
We are using the following struct for passing the information of TestOnlyPyBatchCommandsOverrides between Python and C++:
225+
226+
```c
227+
struct TestOnlyPyOnDoneInfo
228+
{
229+
size_t responseMessageCount;
230+
};
231+
```
232+
'''
233+
_fields_ = [('responseMessageCount', ctypes.c_size_t)]
234+
235+
221236
# typedef void (*PythonInteractionModelDelegate_OnCommandResponseStatusCodeReceivedFunct)(uint64_t commandSenderPtr,
222237
# void * commandStatusBuf);
223238
# typedef void (*PythonInteractionModelDelegate_OnCommandResponseProtocolErrorFunct)(uint64_t commandSenderPtr,

src/python_testing/TC_IDM_1_4.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ async def steps_3_to_9(self, dummy_value):
146146
invoke_request_2 = Clusters.Command.InvokeRequestInfo(endpoint, command)
147147
commandRefsOverride = [1, 1]
148148
try:
149-
result = await dev_ctrl.TestOnlySendBatchCommands(dut_node_id, [invoke_request_1, invoke_request_2], commandRefsOverride=commandRefsOverride)
149+
await dev_ctrl.TestOnlySendBatchCommands(dut_node_id, [invoke_request_1, invoke_request_2], commandRefsOverride=commandRefsOverride)
150150
asserts.fail("Unexpected success return after sending two unique commands with identical CommandRef in the InvokeRequest")
151151
except InteractionModelError as e:
152152
asserts.assert_equal(e.status, Status.InvalidAction,
@@ -216,7 +216,7 @@ async def steps_3_to_9(self, dummy_value):
216216
# receiving a path-specific response to the same command, with the TimedRequestMessage sent before
217217
# the InvokeRequestMessage.
218218
try:
219-
result = await dev_ctrl.TestOnlySendBatchCommands(dut_node_id, [invoke_request_1, invoke_request_2], suppressTimedRequestMessage=True)
219+
await dev_ctrl.TestOnlySendBatchCommands(dut_node_id, [invoke_request_1, invoke_request_2], suppressTimedRequestMessage=True)
220220
asserts.fail("Unexpected success call to sending Batch command when non-path specific error expected")
221221
except InteractionModelError as e:
222222
asserts.assert_equal(e.status, Status.TimedRequestMismatch,

src/python_testing/TestBatchInvoke.py

+4-1
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,13 @@ async def test_batch_invoke(self):
8888
sleepBeforeResponseTimeMs=0, sizeOfResponseBuffer=response_size, fillCharacter=ord(request_2_fill_character))
8989
invoke_request_2 = Clusters.Command.InvokeRequestInfo(endpoint, command)
9090
try:
91-
result = await dev_ctrl.SendBatchCommands(dut_node_id, [invoke_request_1, invoke_request_2])
91+
testOnlyResponse = await dev_ctrl.TestOnlySendBatchCommands(dut_node_id, [invoke_request_1, invoke_request_2])
9292
except InteractionModelError:
9393
asserts.fail("DUT failed to successfully responded to a InvokeRequest action with two valid commands")
9494

95+
asserts.assert_greater(testOnlyResponse.ResponseMessageCount, 1,
96+
"Unexpected, DUT sent response back in single InvokeResponseMessage")
97+
result = testOnlyResponse.Responses
9598
asserts.assert_true(type_matches(result, list), "Unexpected return from SendBatchCommands")
9699
asserts.assert_equal(len(result), 2, "Unexpected number of InvokeResponses sent back from DUT")
97100
asserts.assert_true(type_matches(

0 commit comments

Comments
 (0)