Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow chip-repl to know how many InvokeResponseMessages were recieved #31781

Merged
6 changes: 6 additions & 0 deletions src/app/CommandSender.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
14 changes: 12 additions & 2 deletions src/app/CommandSender.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
tehampson marked this conversation as resolved.
Show resolved Hide resolved

#if CONFIG_BUILD_FOR_HOST_UNIT_TEST
/**
* Version of AddRequestData that allows sending a message that is
Expand Down Expand Up @@ -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;
bzbarsky-apple marked this conversation as resolved.
Show resolved Hide resolved
uint16_t mFinishedCommandCount = 0;
uint16_t mRemoteMaxPathsPerInvoke = 1;

State mState = State::Idle;
bool mSuppressResponse = false;
Expand Down
3 changes: 3 additions & 0 deletions src/app/tests/TestCommandInteraction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions src/controller/python/chip/ChipDeviceCtrl.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
37 changes: 33 additions & 4 deletions src/controller/python/chip/clusters/Command.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -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
Expand All @@ -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
'''
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
53 changes: 37 additions & 16 deletions src/controller/python/chip/clusters/command.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
};
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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,
Expand All @@ -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);
Expand Down Expand Up @@ -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);

Expand Down
5 changes: 5 additions & 0 deletions src/controller/python/chip/interaction_model/Delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ struct TestOnlyPyBatchCommandsOverrides
size_t overrideCommandRefsListLength;
};

struct TestOnlyPyOnDoneInfo
{
size_t responseMessageCount;
};

} // namespace python

namespace Controller {
Expand Down
4 changes: 2 additions & 2 deletions src/controller/python/chip/interaction_model/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 15 additions & 0 deletions src/controller/python/chip/interaction_model/delegate.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions src/python_testing/TC_IDM_1_4.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 4 additions & 1 deletion src/python_testing/TestBatchInvoke.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading