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 <[email protected]>

src/app/CommandSender.cpp

@@ -224,6 +224,7 @@ CHIP_ERROR CommandSender::OnMessageReceived(Messaging::ExchangeContext * apExcha
  {
  err = ProcessInvokeResponse(std::move(aPayload), moreChunkedMessages);
  SuccessOrExit(err);
+ mInvokeResponseMessageCount++;
  if (moreChunkedMessages)
  {
  StatusResponse::Send(Status::Success, apExchangeContext, /*aExpectResponse = */ true);
@@ -534,6 +535,11 @@ CHIP_ERROR CommandSender::FinishCommand(const Optional<uint16_t> & aTimedInvokeT
  return CHIP_NO_ERROR;
 }
 
+size_t CommandSender::GetInvokeResponseMessageCount()
+{
+ return static_cast<size_t>(mInvokeResponseMessageCount);
+}
+
 CHIP_ERROR CommandSender::Finalize(System::PacketBufferHandle & commandPacket)
 {
  VerifyOrReturnError(mState == State::AddedCommand, CHIP_ERROR_INCORRECT_STATE);

src/app/CommandSender.h

@@ -318,6 +318,14 @@ class CommandSender final : public Messaging::ExchangeDelegate
  return FinishCommand(aTimedInvokeTimeoutMs, optionalArgs);
  }
 
+ /**
+ * @brief Returns the number of InvokeResponseMessages received.
+ *
+ * Responses to multiple requests might be split across several InvokeResponseMessages.
+ * This function helps track the total count. Primarily for test validation purposes.
+ */
+ size_t GetInvokeResponseMessageCount();
+
 #if CONFIG_BUILD_FOR_HOST_UNIT_TEST
  /**
  * Version of AddRequestData that allows sending a message that is
@@ -507,8 +515,10 @@ class CommandSender final : public Messaging::ExchangeDelegate
  TLV::TLVType mDataElementContainerType = TLV::kTLVType_NotSpecified;
 
  chip::System::PacketBufferTLVWriter mCommandMessageWriter;
- uint16_t mFinishedCommandCount = 0;
- uint16_t mRemoteMaxPathsPerInvoke = 1;
+
+ uint16_t mInvokeResponseMessageCount = 0;
+ uint16_t mFinishedCommandCount = 0;
+ uint16_t mRemoteMaxPathsPerInvoke = 1;
 
  State mState = State::Idle;
  bool mSuppressResponse = false;

src/app/tests/TestCommandInteraction.cpp

@@ -883,6 +883,7 @@ void TestCommandInteraction::TestCommandInvalidMessage1(nlTestSuite * apSuite, v
  NL_TEST_ASSERT(apSuite,
  mockCommandSenderDelegate.onResponseCalledTimes == 0 && mockCommandSenderDelegate.onFinalCalledTimes == 1 &&
  mockCommandSenderDelegate.onErrorCalledTimes == 1);
+ NL_TEST_ASSERT(apSuite, commandSender.GetInvokeResponseMessageCount() == 0);
 
  ctx.DrainAndServiceIO();
 
@@ -1338,12 +1339,14 @@ void TestCommandInteraction::TestCommandSenderCommandSuccessResponseFlow(nlTestS
  err = commandSender.SendCommandRequest(ctx.GetSessionBobToAlice());
 
  NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
+ NL_TEST_ASSERT(apSuite, commandSender.GetInvokeResponseMessageCount() == 0);
 
  ctx.DrainAndServiceIO();
 
  NL_TEST_ASSERT(apSuite,
  mockCommandSenderDelegate.onResponseCalledTimes == 1 && mockCommandSenderDelegate.onFinalCalledTimes == 1 &&
  mockCommandSenderDelegate.onErrorCalledTimes == 0);
+ NL_TEST_ASSERT(apSuite, commandSender.GetInvokeResponseMessageCount() == 1);
 
  NL_TEST_ASSERT(apSuite, GetNumActiveHandlerObjects() == 0);
  NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0);

src/controller/python/chip/ChipDeviceCtrl.py

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

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

@@ -27,7 +27,7 @@
 
 import chip.exceptions
 import chip.interaction_model
-from chip.interaction_model import PyInvokeRequestData, TestOnlyPyBatchCommandsOverrides
+from chip.interaction_model import PyInvokeRequestData, TestOnlyPyBatchCommandsOverrides, TestOnlyPyOnDoneInfo
 from chip.native import PyChipError
 
 from .ClusterObjects import ClusterCommand
@@ -56,6 +56,12 @@ class Status:
  ClusterStatus: int
 
 
+@dataclass
+class TestOnlyBatchCommandResponse:
+ Responses: object
+ ResponseMessageCount: int
+
+
 def FindCommandClusterObject(isClientSideCommand: bool, path: CommandPath):
  ''' Locates the right generated cluster object given a set of parameters.
 
@@ -215,12 +221,29 @@ def handleDone(self):
  )
 
 
+class TestOnlyAsyncBatchCommandsTransaction(AsyncBatchCommandsTransaction):
+ def __init__(self, future: Future, eventLoop, expectTypes: List[Type]):
+ self._responseMessageCount = 0
+ super().__init__(future, eventLoop, expectTypes)
+
+ def _handleDone(self):
+ # Future might already be set with exception from `handleError`
+ if not self._future.done():
+ self._future.set_result(TestOnlyBatchCommandResponse(self._responses, self._responseMessageCount))
+ ctypes.pythonapi.Py_DecRef(ctypes.py_object(self))
+
+ def testOnlyDoneInfo(self, testOnlyDoneInfo: TestOnlyPyOnDoneInfo):
+ self._responseMessageCount = testOnlyDoneInfo.responseMessageCount
+
+
 _OnCommandSenderResponseCallbackFunct = CFUNCTYPE(
  None, py_object, c_uint16, c_uint32, c_uint32, c_size_t, c_uint16, c_uint8, c_void_p, c_uint32)
 _OnCommandSenderErrorCallbackFunct = CFUNCTYPE(
  None, py_object, c_uint16, c_uint8, PyChipError)
 _OnCommandSenderDoneCallbackFunct = CFUNCTYPE(
  None, py_object)
+_TestOnlyOnCommandSenderDoneCallbackFunct = CFUNCTYPE(
+ None, py_object, TestOnlyPyOnDoneInfo)
 
 
 @_OnCommandSenderResponseCallbackFunct
@@ -241,6 +264,12 @@ def _OnCommandSenderDoneCallback(closure):
  closure.handleDone()
 
 
+@_TestOnlyOnCommandSenderDoneCallbackFunct
+def _TestOnlyOnCommandSenderDoneCallback(closure, testOnlyDoneInfo: TestOnlyPyOnDoneInfo):
+ closure.testOnlyDoneInfo(testOnlyDoneInfo)
+ closure.handleDone()
+
+
 def TestOnlySendCommandTimedRequestFlagWithNoTimedInvoke(future: Future, eventLoop, responseType, device, commandPath, payload):
  ''' ONLY TO BE USED FOR TEST: Sends the payload with a TimedRequest flag but no TimedInvoke transaction
  '''
@@ -392,7 +421,7 @@ def TestOnlySendBatchCommands(future: Future, eventLoop, device, commands: List[
  pyBatchCommandsData = _BuildPyInvokeRequestData(commands, timedRequestTimeoutMs,
  responseTypes, suppressTimedRequestMessage=suppressTimedRequestMessage)
 
- transaction = AsyncBatchCommandsTransaction(future, eventLoop, responseTypes)
+ transaction = TestOnlyAsyncBatchCommandsTransaction(future, eventLoop, responseTypes)
  ctypes.pythonapi.Py_IncRef(ctypes.py_object(transaction))
 
  testOnlyOverrides = TestOnlyPyBatchCommandsOverrides()
@@ -448,7 +477,7 @@ def Init():
  setter.Set('pychip_CommandSender_SendGroupCommand',
  PyChipError, [c_uint16, c_void_p, c_uint32, c_uint32, c_char_p, c_size_t, c_uint16])
  setter.Set('pychip_CommandSender_InitCallbacks', None, [
- _OnCommandSenderResponseCallbackFunct, _OnCommandSenderErrorCallbackFunct, _OnCommandSenderDoneCallbackFunct])
+ _OnCommandSenderResponseCallbackFunct, _OnCommandSenderErrorCallbackFunct, _OnCommandSenderDoneCallbackFunct, _TestOnlyOnCommandSenderDoneCallbackFunct])
 
  handle.pychip_CommandSender_InitCallbacks(
- _OnCommandSenderResponseCallback, _OnCommandSenderErrorCallback, _OnCommandSenderDoneCallback)
+ _OnCommandSenderResponseCallback, _OnCommandSenderErrorCallback, _OnCommandSenderDoneCallback, _TestOnlyOnCommandSenderDoneCallback)

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

@@ -55,24 +55,26 @@ PyChipError pychip_CommandSender_SendGroupCommand(chip::GroupId groupId, chip::C
 namespace chip {
 namespace python {
 
-using OnCommandSenderResponseCallback = void (*)(PyObject appContext, chip::EndpointId endpointId, chip::ClusterId clusterId,
+using OnCommandSenderResponseCallback  = void (*)(PyObject appContext, chip::EndpointId endpointId, chip::ClusterId clusterId,
  chip::CommandId commandId, size_t index,
  std::underlying_type_t<Protocols::InteractionModel::Status> status,
  chip::ClusterStatus clusterStatus, const uint8_t * payload, uint32_t length);
-using OnCommandSenderErrorCallback = void (*)(PyObject appContext,
+using OnCommandSenderErrorCallback  = void (*)(PyObject appContext,
  std::underlying_type_t<Protocols::InteractionModel::Status> status,
  chip::ClusterStatus clusterStatus, PyChipError chiperror);
-using OnCommandSenderDoneCallback = void (*)(PyObject appContext);
+using OnCommandSenderDoneCallback = void (*)(PyObject appContext);
+using TestOnlyOnCommandSenderDoneCallback = void (*)(PyObject appContext, python::TestOnlyPyOnDoneInfo testOnlyDoneInfo);
 
-OnCommandSenderResponseCallback gOnCommandSenderResponseCallback = nullptr;
-OnCommandSenderErrorCallback gOnCommandSenderErrorCallback = nullptr;
-OnCommandSenderDoneCallback gOnCommandSenderDoneCallback = nullptr;
+OnCommandSenderResponseCallback gOnCommandSenderResponseCallback = nullptr;
+OnCommandSenderErrorCallback gOnCommandSenderErrorCallback = nullptr;
+OnCommandSenderDoneCallback gOnCommandSenderDoneCallback = nullptr;
+TestOnlyOnCommandSenderDoneCallback gTestOnlyOnCommandSenderDoneCallback = nullptr;
 
 class CommandSenderCallback : public CommandSender::ExtendableCallback
 {
 public:
- CommandSenderCallback(PyObject appContext, bool isBatchedCommands) :
- mAppContext(appContext), mIsBatchedCommands(isBatchedCommands)
+ CommandSenderCallback(PyObject appContext, bool isBatchedCommands, bool callTestOnlyOnDone) :
+ mAppContext(appContext), mIsBatchedCommands(isBatchedCommands), mCallTestOnlyOnDone(callTestOnlyOnDone)
  {}
 
  void OnResponse(CommandSender * apCommandSender, const CommandSender::ResponseData & aResponseData) override
@@ -148,7 +150,17 @@ class CommandSenderCallback : public CommandSender::ExtendableCallback
 
  void OnDone(CommandSender * apCommandSender) override
  {
- gOnCommandSenderDoneCallback(mAppContext);
+ if (mCallTestOnlyOnDone)
+ {
+ python::TestOnlyPyOnDoneInfo testOnlyOnDoneInfo;
+ testOnlyOnDoneInfo.responseMessageCount = apCommandSender->GetInvokeResponseMessageCount();
+ gTestOnlyOnCommandSenderDoneCallback(mAppContext, testOnlyOnDoneInfo);
+ }
+ else
+ {
+ gOnCommandSenderDoneCallback(mAppContext);
+ }
+
  delete apCommandSender;
  delete this;
  };
@@ -179,6 +191,7 @@ class CommandSenderCallback : public CommandSender::ExtendableCallback
  PyObject mAppContext = nullptr;
  std::unordered_map<uint16_t, size_t> commandRefToIndex;
  bool mIsBatchedCommands;
+ bool mCallTestOnlyOnDone;
 };
 
 PyChipError SendBatchCommandsInternal(void * appContext, DeviceProxy * device, uint16_t timedRequestTimeoutMs,
@@ -220,8 +233,10 @@ PyChipError SendBatchCommandsInternal(void * appContext, DeviceProxy * device, u
  config.SetRemoteMaxPathsPerInvoke(remoteSessionParameters.GetMaxPathsPerInvoke());
  }
 
+ bool isBatchedCommands = true;
+ bool callTestOnlyOnDone = testOnlyOverrides != nullptr;
  std::unique_ptr<CommandSenderCallback> callback =
- std::make_unique<CommandSenderCallback>(appContext, /* isBatchedCommands =*/true);
+ std::make_unique<CommandSenderCallback>(appContext, isBatchedCommands, callTestOnlyOnDone);
 
  bool isTimedRequest = timedRequestTimeoutMs != 0 || testOnlySuppressTimedRequestMessage;
  std::unique_ptr<CommandSender> sender =
@@ -315,11 +330,13 @@ using namespace chip::python;
 extern "C" {
 void pychip_CommandSender_InitCallbacks(OnCommandSenderResponseCallback onCommandSenderResponseCallback,
  OnCommandSenderErrorCallback onCommandSenderErrorCallback,
- OnCommandSenderDoneCallback onCommandSenderDoneCallback)
+ OnCommandSenderDoneCallback onCommandSenderDoneCallback,
+ TestOnlyOnCommandSenderDoneCallback testOnlyOnCommandSenderDoneCallback)
 {
- gOnCommandSenderResponseCallback = onCommandSenderResponseCallback;
- gOnCommandSenderErrorCallback = onCommandSenderErrorCallback;
- gOnCommandSenderDoneCallback = onCommandSenderDoneCallback;
+ gOnCommandSenderResponseCallback = onCommandSenderResponseCallback;
+ gOnCommandSenderErrorCallback = onCommandSenderErrorCallback;
+ gOnCommandSenderDoneCallback = onCommandSenderDoneCallback;
+ gTestOnlyOnCommandSenderDoneCallback = testOnlyOnCommandSenderDoneCallback;
 }
 
 PyChipError pychip_CommandSender_SendCommand(void * appContext, DeviceProxy * device, uint16_t timedRequestTimeoutMs,
@@ -331,8 +348,10 @@ PyChipError pychip_CommandSender_SendCommand(void * appContext, DeviceProxy * de
 
  VerifyOrReturnError(device->GetSecureSession().HasValue(), ToPyChipError(CHIP_ERROR_MISSING_SECURE_SESSION));
 
+ bool isBatchedCommands = false;
+ bool callTestOnlyOnDone = false;
  std::unique_ptr<CommandSenderCallback> callback =
- std::make_unique<CommandSenderCallback>(appContext, /* isBatchedCommands =*/false);
+ std::make_unique<CommandSenderCallback>(appContext, isBatchedCommands, callTestOnlyOnDone);
  std::unique_ptr<CommandSender> sender =
  std::make_unique<CommandSender>(callback.get(), device->GetExchangeManager(),
  /* is timed request */ timedRequestTimeoutMs != 0, suppressResponse);
@@ -406,8 +425,10 @@ PyChipError pychip_CommandSender_TestOnlySendCommandTimedRequestNoTimedInvoke(
 
  VerifyOrReturnError(device->GetSecureSession().HasValue(), ToPyChipError(CHIP_ERROR_MISSING_SECURE_SESSION));
 
+ bool isBatchedCommands = false;
+ bool callTestOnlyOnDone = false;
  std::unique_ptr<CommandSenderCallback> callback =
- std::make_unique<CommandSenderCallback>(appContext, /* isBatchedCommands =*/false);
+ std::make_unique<CommandSenderCallback>(appContext, isBatchedCommands, callTestOnlyOnDone);
  std::unique_ptr<CommandSender> sender = std::make_unique<CommandSender>(callback.get(), device->GetExchangeManager(),
  /* is timed request */ true, suppressResponse);
 

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

@@ -71,6 +71,11 @@ struct TestOnlyPyBatchCommandsOverrides
  size_t overrideCommandRefsListLength;
 };
 
+struct TestOnlyPyOnDoneInfo
+{
+ size_t responseMessageCount;
+};
+
 } // namespace python
 
 namespace Controller {

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

@@ -28,11 +28,11 @@
 
 from .delegate import (AttributePath, AttributePathIBstruct, DataVersionFilterIBstruct, EventPath, EventPathIBstruct,
  PyInvokeRequestData, PyWriteAttributeData, SessionParameters, SessionParametersStruct,
- TestOnlyPyBatchCommandsOverrides)
+ TestOnlyPyBatchCommandsOverrides, TestOnlyPyOnDoneInfo)
 
 __all__ = ["AttributePath", "AttributePathIBstruct", "DataVersionFilterIBstruct",
  "EventPath", "EventPathIBstruct", "InteractionModelError", "PyInvokeRequestData",
- "PyWriteAttributeData", "SessionParameters", "SessionParametersStruct", "Status", "TestOnlyPyBatchCommandsOverrides"]
+ "PyWriteAttributeData", "SessionParameters", "SessionParametersStruct", "Status", "TestOnlyPyBatchCommandsOverrides", "TestOnlyPyOnDoneInfo"]
 
 
 # defined src/controller/python/chip/interaction_model/Delegate.h

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

@@ -218,6 +218,21 @@ class TestOnlyPyBatchCommandsOverrides(ctypes.Structure):
  ('overrideCommandRefsList', POINTER(ctypes.c_uint16)), ('overrideCommandRefsListLength', ctypes.c_size_t)]
 
 
+class TestOnlyPyOnDoneInfo(ctypes.Structure):
+ ''' TestOnly struct for overriding aspects of batch command to send invalid commands.
+
+ We are using the following struct for passing the information of TestOnlyPyBatchCommandsOverrides between Python and C++:
+
+ ```c
+ struct TestOnlyPyOnDoneInfo
+ {
+ size_t responseMessageCount;
+ };
+ ```
+ '''
+ _fields_ = [('responseMessageCount', ctypes.c_size_t)]
+
+
 # typedef void (*PythonInteractionModelDelegate_OnCommandResponseStatusCodeReceivedFunct)(uint64_t commandSenderPtr,
 # void * commandStatusBuf);
 # typedef void (*PythonInteractionModelDelegate_OnCommandResponseProtocolErrorFunct)(uint64_t commandSenderPtr,

src/python_testing/TC_IDM_1_4.py

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

src/python_testing/TestBatchInvoke.py

@@ -88,10 +88,13 @@ async def test_batch_invoke(self):
  sleepBeforeResponseTimeMs=0, sizeOfResponseBuffer=response_size, fillCharacter=ord(request_2_fill_character))
  invoke_request_2 = Clusters.Command.InvokeRequestInfo(endpoint, command)
  try:
- result = await dev_ctrl.SendBatchCommands(dut_node_id, [invoke_request_1, invoke_request_2])
+ testOnlyResponse = await dev_ctrl.TestOnlySendBatchCommands(dut_node_id, [invoke_request_1, invoke_request_2])
  except InteractionModelError:
  asserts.fail("DUT failed to successfully responded to a InvokeRequest action with two valid commands")
 
+ asserts.assert_greater(testOnlyResponse.ResponseMessageCount, 1,
+ "Unexpected, DUT sent response back in single InvokeResponseMessage")
+ result = testOnlyResponse.Responses
  asserts.assert_true(type_matches(result, list), "Unexpected return from SendBatchCommands")
  asserts.assert_equal(len(result), 2, "Unexpected number of InvokeResponses sent back from DUT")
  asserts.assert_true(type_matches(