Skip to content

Commit

Permalink
Make OnMessageReceived return error (#7855)
Browse files Browse the repository at this point in the history 
Summary of Changes:
OnMessageReceived in ExchangeDelegate don't return error so that we
cannot test OnMessageReceived in Interaction Model protocol
implementation.
  • Loading branch information
@yunhanw-google @pull
yunhanw-google authored and pull[bot] committed Jun 25, 2021
1 parent 9dd0e52 commit 5269ef9
Show file tree
Hide file tree
Showing 38 changed files with 165 additions and 129 deletions.
5 changes: 3 additions & 2 deletions examples/shell/shell_common/cmd_send.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ class SendArguments
class MockAppDelegate : public Messaging::ExchangeDelegate
{
public:
void OnMessageReceived(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader, const PayloadHeader & payloadHeader,
System::PacketBufferHandle && buffer) override
CHIP_ERROR OnMessageReceived(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader,
const PayloadHeader & payloadHeader, System::PacketBufferHandle && buffer) override
{
uint32_t respTime = System::Timer::GetCurrentEpoch();
uint32_t transitTime = respTime - gSendArguments.GetLastSendTime();
Expand All @@ -112,6 +112,7 @@ class MockAppDelegate : public Messaging::ExchangeDelegate

gExchangeCtx->Close();
gExchangeCtx = nullptr;
return CHIP_NO_ERROR;
}

void OnResponseTimeout(Messaging::ExchangeContext * ec) override
Expand Down
5 changes: 3 additions & 2 deletions src/app/CommandHandler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ using GeneralStatusCode = chip::Protocols::SecureChannel::GeneralStatusCode;

namespace chip {
namespace app {
void CommandHandler::OnMessageReceived(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader,
const PayloadHeader & payloadHeader, System::PacketBufferHandle && payload)
CHIP_ERROR CommandHandler::OnInvokeCommandRequest(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader,
const PayloadHeader & payloadHeader, System::PacketBufferHandle && payload)
{
CHIP_ERROR err = CHIP_NO_ERROR;
System::PacketBufferHandle response;
Expand All @@ -51,6 +51,7 @@ void CommandHandler::OnMessageReceived(Messaging::ExchangeContext * ec, const Pa

exit:
ChipLogFunctError(err);
return err;
}

CHIP_ERROR CommandHandler::SendCommandResponse()
Expand Down
4 changes: 2 additions & 2 deletions src/app/CommandHandler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ namespace app {
class CommandHandler : public Command
{
public:
void OnMessageReceived(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader, const PayloadHeader & payloadHeader,
System::PacketBufferHandle && payload);
CHIP_ERROR OnInvokeCommandRequest(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader,
const PayloadHeader & payloadHeader, System::PacketBufferHandle && payload);
CHIP_ERROR AddStatusCode(const CommandPathParams & aCommandPathParams,
const Protocols::SecureChannel::GeneralStatusCode aGeneralCode, const Protocols::Id aProtocolId,
const Protocols::InteractionModel::ProtocolCode aProtocolCode) override;
Expand Down
5 changes: 3 additions & 2 deletions src/app/CommandSender.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ CHIP_ERROR CommandSender::SendCommandRequest(NodeId aNodeId, Transport::AdminId
return err;
}

void CommandSender::OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload)
CHIP_ERROR CommandSender::OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload)
{
CHIP_ERROR err = CHIP_NO_ERROR;

Expand Down Expand Up @@ -110,6 +110,7 @@ void CommandSender::OnMessageReceived(Messaging::ExchangeContext * apExchangeCon
}

Shutdown();
return err;
}

void CommandSender::OnResponseTimeout(Messaging::ExchangeContext * apExchangeContext)
Expand Down
4 changes: 2 additions & 2 deletions src/app/CommandSender.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ class CommandSender : public Command, public Messaging::ExchangeDelegate
// ExchangeDelegate interface implementation. Private so people won't
// accidentally call it on us when we're not being treated as an actual
// ExchangeDelegate.
void OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload) override;
CHIP_ERROR OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload) override;
void OnResponseTimeout(Messaging::ExchangeContext * apExchangeContext) override;

CHIP_ERROR ProcessCommandDataElement(CommandDataElement::Parser & aCommandElement) override;
Expand Down
48 changes: 27 additions & 21 deletions src/app/InteractionModelEngine.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,9 @@ CHIP_ERROR InteractionModelEngine::NewWriteClient(WriteClient ** const apWriteCl
return CHIP_ERROR_NO_MEMORY;
}

void InteractionModelEngine::OnUnknownMsgType(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload)
CHIP_ERROR InteractionModelEngine::OnUnknownMsgType(Messaging::ExchangeContext * apExchangeContext,
const PacketHeader & aPacketHeader, const PayloadHeader & aPayloadHeader,
System::PacketBufferHandle && aPayload)
{
CHIP_ERROR err = CHIP_NO_ERROR;

Expand All @@ -199,11 +200,12 @@ void InteractionModelEngine::OnUnknownMsgType(Messaging::ExchangeContext * apExc
{
apExchangeContext->Abort();
}
return err;
}

void InteractionModelEngine::OnInvokeCommandRequest(Messaging::ExchangeContext * apExchangeContext,
const PacketHeader & aPacketHeader, const PayloadHeader & aPayloadHeader,
System::PacketBufferHandle && aPayload)
CHIP_ERROR InteractionModelEngine::OnInvokeCommandRequest(Messaging::ExchangeContext * apExchangeContext,
const PacketHeader & aPacketHeader, const PayloadHeader & aPayloadHeader,
System::PacketBufferHandle && aPayload)
{
CHIP_ERROR err = CHIP_NO_ERROR;

Expand All @@ -213,7 +215,7 @@ void InteractionModelEngine::OnInvokeCommandRequest(Messaging::ExchangeContext *
{
err = commandHandler.Init(mpExchangeMgr, mpDelegate);
SuccessOrExit(err);
commandHandler.OnMessageReceived(apExchangeContext, aPacketHeader, aPayloadHeader, std::move(aPayload));
err = commandHandler.OnInvokeCommandRequest(apExchangeContext, aPacketHeader, aPayloadHeader, std::move(aPayload));
apExchangeContext = nullptr;
break;
}
Expand All @@ -226,10 +228,11 @@ void InteractionModelEngine::OnInvokeCommandRequest(Messaging::ExchangeContext *
{
apExchangeContext->Abort();
}
return err;
}

void InteractionModelEngine::OnReadRequest(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload)
CHIP_ERROR InteractionModelEngine::OnReadRequest(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload)
{
CHIP_ERROR err = CHIP_NO_ERROR;

Expand All @@ -241,8 +244,7 @@ void InteractionModelEngine::OnReadRequest(Messaging::ExchangeContext * apExchan
{
err = readHandler.Init(mpDelegate);
SuccessOrExit(err);
err = readHandler.OnReadRequest(apExchangeContext, std::move(aPayload));
SuccessOrExit(err);
err = readHandler.OnReadRequest(apExchangeContext, std::move(aPayload));
apExchangeContext = nullptr;
break;
}
Expand All @@ -255,10 +257,12 @@ void InteractionModelEngine::OnReadRequest(Messaging::ExchangeContext * apExchan
{
apExchangeContext->Abort();
}
return err;
}

void InteractionModelEngine::OnWriteRequest(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload)
CHIP_ERROR InteractionModelEngine::OnWriteRequest(Messaging::ExchangeContext * apExchangeContext,
const PacketHeader & aPacketHeader, const PayloadHeader & aPayloadHeader,
System::PacketBufferHandle && aPayload)
{
CHIP_ERROR err = CHIP_NO_ERROR;

Expand All @@ -270,8 +274,7 @@ void InteractionModelEngine::OnWriteRequest(Messaging::ExchangeContext * apExcha
{
err = writeHandler.Init(mpDelegate);
SuccessOrExit(err);
err = writeHandler.OnWriteRequest(apExchangeContext, std::move(aPayload));
SuccessOrExit(err);
err = writeHandler.OnWriteRequest(apExchangeContext, std::move(aPayload));
apExchangeContext = nullptr;
break;
}
Expand All @@ -284,28 +287,31 @@ void InteractionModelEngine::OnWriteRequest(Messaging::ExchangeContext * apExcha
{
apExchangeContext->Abort();
}
return err;
}

void InteractionModelEngine::OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload)
CHIP_ERROR InteractionModelEngine::OnMessageReceived(Messaging::ExchangeContext * apExchangeContext,
const PacketHeader & aPacketHeader, const PayloadHeader & aPayloadHeader,
System::PacketBufferHandle && aPayload)
{
CHIP_ERROR err = CHIP_NO_ERROR;
if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::InvokeCommandRequest))
{

OnInvokeCommandRequest(apExchangeContext, aPacketHeader, aPayloadHeader, std::move(aPayload));
err = OnInvokeCommandRequest(apExchangeContext, aPacketHeader, aPayloadHeader, std::move(aPayload));
}
else if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::ReadRequest))
{
OnReadRequest(apExchangeContext, aPacketHeader, aPayloadHeader, std::move(aPayload));
err = OnReadRequest(apExchangeContext, aPacketHeader, aPayloadHeader, std::move(aPayload));
}
else if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::WriteRequest))
{
OnWriteRequest(apExchangeContext, aPacketHeader, aPayloadHeader, std::move(aPayload));
err = OnWriteRequest(apExchangeContext, aPacketHeader, aPayloadHeader, std::move(aPayload));
}
else
{
OnUnknownMsgType(apExchangeContext, aPacketHeader, aPayloadHeader, std::move(aPayload));
err = OnUnknownMsgType(apExchangeContext, aPacketHeader, aPayloadHeader, std::move(aPayload));
}
return err;
}

void InteractionModelEngine::OnResponseTimeout(Messaging::ExchangeContext * ec)
Expand Down
18 changes: 9 additions & 9 deletions src/app/InteractionModelEngine.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,27 +155,27 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate

private:
friend class reporting::Engine;
void OnUnknownMsgType(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload);
void OnInvokeCommandRequest(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
CHIP_ERROR OnUnknownMsgType(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload);
void OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload);
CHIP_ERROR OnInvokeCommandRequest(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload);
CHIP_ERROR OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload);
void OnResponseTimeout(Messaging::ExchangeContext * ec);

/**
* Called when Interaction Model receives a Read Request message. Errors processing
* the Read Request are handled entirely within this function.
*/
void OnReadRequest(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload);
CHIP_ERROR OnReadRequest(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload);

/**
* Called when Interaction Model receives a Write Request message. Errors processing
* the Write Request are handled entirely within this function.
*/
void OnWriteRequest(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload);
CHIP_ERROR OnWriteRequest(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload);

Messaging::ExchangeManager * mpExchangeMgr = nullptr;
InteractionModelDelegate * mpDelegate = nullptr;
Expand Down
6 changes: 3 additions & 3 deletions src/app/ReadClient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,8 @@ CHIP_ERROR ReadClient::GenerateAttributePathList(ReadRequest::Builder & aRequest
return attributePathListBuilder.GetError();
}

void ReadClient::OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload)
CHIP_ERROR ReadClient::OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload)
{
CHIP_ERROR err = CHIP_NO_ERROR;

Expand Down Expand Up @@ -224,7 +224,7 @@ void ReadClient::OnMessageReceived(Messaging::ExchangeContext * apExchangeContex
}
}

return;
return err;
}

CHIP_ERROR ReadClient::AbortExistingExchangeContext()
Expand Down
4 changes: 2 additions & 2 deletions src/app/ReadClient.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ class ReadClient : public Messaging::ExchangeDelegate

virtual ~ReadClient() = default;

void OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload) override;
CHIP_ERROR OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload) override;
void OnResponseTimeout(Messaging::ExchangeContext * apExchangeContext) override;

/**
Expand Down
5 changes: 3 additions & 2 deletions src/app/WriteClient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,8 @@ CHIP_ERROR WriteClient::SendWriteRequest(NodeId aNodeId, Transport::AdminId aAdm
return err;
}

void WriteClient::OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload)
CHIP_ERROR WriteClient::OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload)
{
CHIP_ERROR err = CHIP_NO_ERROR;
// Assert that the exchange context matches the client's current context.
Expand Down Expand Up @@ -324,6 +324,7 @@ void WriteClient::OnMessageReceived(Messaging::ExchangeContext * apExchangeConte
}
}
Shutdown();
return err;
}

void WriteClient::OnResponseTimeout(Messaging::ExchangeContext * apExchangeContext)
Expand Down
4 changes: 2 additions & 2 deletions src/app/WriteClient.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,8 @@ class WriteClient : public Messaging::ExchangeDelegate

virtual ~WriteClient() = default;

void OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload) override;
CHIP_ERROR OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload) override;
void OnResponseTimeout(Messaging::ExchangeContext * apExchangeContext) override;

/**
Expand Down
7 changes: 4 additions & 3 deletions src/app/server/Server.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -323,9 +323,10 @@ static CHIP_ERROR OpenPairingWindowUsingVerifier(uint16_t discriminator, PASEVer
class ServerCallback : public ExchangeDelegate
{
public:
void OnMessageReceived(Messaging::ExchangeContext * exchangeContext, const PacketHeader & packetHeader,
const PayloadHeader & payloadHeader, System::PacketBufferHandle && buffer) override
CHIP_ERROR OnMessageReceived(Messaging::ExchangeContext * exchangeContext, const PacketHeader & packetHeader,
const PayloadHeader & payloadHeader, System::PacketBufferHandle && buffer) override
{
CHIP_ERROR err = CHIP_NO_ERROR;
// as soon as a client connects, assume it is connected
VerifyOrExit(!buffer.IsNull(), ChipLogError(AppServer, "Received data but couldn't process it..."));
VerifyOrExit(packetHeader.GetSourceNodeId().HasValue(), ChipLogError(AppServer, "Unknown source for received message"));
Expand All @@ -342,7 +343,6 @@ class ServerCallback : public ExchangeDelegate
// Issue: https://github.com/project-chip/connectedhomeip/issues/4725
if (payloadHeader.HasProtocol(chip::Protocols::ServiceProvisioning::Id))
{
CHIP_ERROR err = CHIP_NO_ERROR;
uint32_t timeout;
uint16_t discriminator;
PASEVerifier verifier;
Expand Down Expand Up @@ -388,6 +388,7 @@ class ServerCallback : public ExchangeDelegate

exit:
exchangeContext->Close();
return err;
}

void OnResponseTimeout(ExchangeContext * ec) override
Expand Down
8 changes: 5 additions & 3 deletions src/app/tests/TestCommandInteraction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,11 @@ class TestCommandInteraction

class TestExchangeDelegate : public Messaging::ExchangeDelegate
{
void OnMessageReceived(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader, const PayloadHeader & payloadHeader,
System::PacketBufferHandle && payload) override
{}
CHIP_ERROR OnMessageReceived(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader,
const PayloadHeader & payloadHeader, System::PacketBufferHandle && payload) override
{
return CHIP_NO_ERROR;
}

void OnResponseTimeout(Messaging::ExchangeContext * ec) override {}
};
Expand Down
8 changes: 5 additions & 3 deletions src/app/tests/TestReportingEngine.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,11 @@ class TestReportingEngine

class TestExchangeDelegate : public Messaging::ExchangeDelegate
{
void OnMessageReceived(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader, const PayloadHeader & payloadHeader,
System::PacketBufferHandle && payload) override
{}
CHIP_ERROR OnMessageReceived(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader,
const PayloadHeader & payloadHeader, System::PacketBufferHandle && payload) override
{
return CHIP_NO_ERROR;
}

void OnResponseTimeout(Messaging::ExchangeContext * ec) override {}
};
Expand Down
Loading

0 comments on commit 5269ef9

Please sign in to comment.