Skip to content

Commit

Permalink
Make OnMessageReceived return error
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
yunhanw-google committed Jun 23, 2021
1 parent 039dfbd commit be97a05
Show file tree
Hide file tree
Showing 38 changed files with 98 additions and 75 deletions.
3 changes: 2 additions & 1 deletion examples/shell/shell_common/cmd_send.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ class SendArguments
class MockAppDelegate : public Messaging::ExchangeDelegate
{
public:
void OnMessageReceived(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader, const PayloadHeader & payloadHeader,
CHIP_ERROR OnMessageReceived(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader, const PayloadHeader & payloadHeader,
System::PacketBufferHandle && buffer) override
{
uint32_t respTime = System::Timer::GetCurrentEpoch();
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
3 changes: 2 additions & 1 deletion src/app/CommandHandler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ using GeneralStatusCode = chip::Protocols::SecureChannel::GeneralStatusCode;

namespace chip {
namespace app {
void CommandHandler::OnMessageReceived(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader,
CHIP_ERROR CommandHandler::OnInvokeCommandRequest(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader,
const PayloadHeader & payloadHeader, System::PacketBufferHandle && payload)
{
CHIP_ERROR err = CHIP_NO_ERROR;
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
2 changes: 1 addition & 1 deletion src/app/CommandHandler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ namespace app {
class CommandHandler : public Command
{
public:
void OnMessageReceived(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader, const PayloadHeader & payloadHeader,
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,
Expand Down
3 changes: 2 additions & 1 deletion src/app/CommandSender.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ CHIP_ERROR CommandSender::SendCommandRequest(NodeId aNodeId, Transport::AdminId
return err;
}

void CommandSender::OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
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
2 changes: 1 addition & 1 deletion src/app/CommandSender.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ 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,
CHIP_ERROR OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload) override;
void OnResponseTimeout(Messaging::ExchangeContext * apExchangeContext) override;

Expand Down
29 changes: 16 additions & 13 deletions src/app/InteractionModelEngine.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ CHIP_ERROR InteractionModelEngine::NewWriteClient(WriteClient ** const apWriteCl
return CHIP_ERROR_NO_MEMORY;
}

void InteractionModelEngine::OnUnknownMsgType(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
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,9 +199,10 @@ void InteractionModelEngine::OnUnknownMsgType(Messaging::ExchangeContext * apExc
{
apExchangeContext->Abort();
}
return err;
}

void InteractionModelEngine::OnInvokeCommandRequest(Messaging::ExchangeContext * apExchangeContext,
CHIP_ERROR InteractionModelEngine::OnInvokeCommandRequest(Messaging::ExchangeContext * apExchangeContext,
const PacketHeader & aPacketHeader, const PayloadHeader & aPayloadHeader,
System::PacketBufferHandle && aPayload)
{
Expand All @@ -213,7 +214,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,9 +227,10 @@ void InteractionModelEngine::OnInvokeCommandRequest(Messaging::ExchangeContext *
{
apExchangeContext->Abort();
}
return err;
}

void InteractionModelEngine::OnReadRequest(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
CHIP_ERROR InteractionModelEngine::OnReadRequest(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload)
{
CHIP_ERROR err = CHIP_NO_ERROR;
Expand All @@ -242,7 +244,6 @@ void InteractionModelEngine::OnReadRequest(Messaging::ExchangeContext * apExchan
err = readHandler.Init(mpDelegate);
SuccessOrExit(err);
err = readHandler.OnReadRequest(apExchangeContext, std::move(aPayload));
SuccessOrExit(err);
apExchangeContext = nullptr;
break;
}
Expand All @@ -255,9 +256,10 @@ void InteractionModelEngine::OnReadRequest(Messaging::ExchangeContext * apExchan
{
apExchangeContext->Abort();
}
return err;
}

void InteractionModelEngine::OnWriteRequest(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
CHIP_ERROR InteractionModelEngine::OnWriteRequest(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload)
{
CHIP_ERROR err = CHIP_NO_ERROR;
Expand All @@ -271,7 +273,6 @@ void InteractionModelEngine::OnWriteRequest(Messaging::ExchangeContext * apExcha
err = writeHandler.Init(mpDelegate);
SuccessOrExit(err);
err = writeHandler.OnWriteRequest(apExchangeContext, std::move(aPayload));
SuccessOrExit(err);
apExchangeContext = nullptr;
break;
}
Expand All @@ -284,28 +285,30 @@ void InteractionModelEngine::OnWriteRequest(Messaging::ExchangeContext * apExcha
{
apExchangeContext->Abort();
}
return err;
}

void InteractionModelEngine::OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
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
10 changes: 5 additions & 5 deletions src/app/InteractionModelEngine.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,26 +155,26 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate

private:
friend class reporting::Engine;
void OnUnknownMsgType(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
CHIP_ERROR OnUnknownMsgType(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload);
void OnInvokeCommandRequest(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
CHIP_ERROR OnInvokeCommandRequest(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload);
void OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
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,
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,
CHIP_ERROR OnWriteRequest(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload);

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

void ReadClient::OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
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
2 changes: 1 addition & 1 deletion src/app/ReadClient.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ class ReadClient : public Messaging::ExchangeDelegate

virtual ~ReadClient() = default;

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

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

void WriteClient::OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
CHIP_ERROR WriteClient::OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload)
{
CHIP_ERROR err = CHIP_NO_ERROR;
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
2 changes: 1 addition & 1 deletion src/app/WriteClient.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ class WriteClient : public Messaging::ExchangeDelegate

virtual ~WriteClient() = default;

void OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PacketHeader & aPacketHeader,
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/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,
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
4 changes: 2 additions & 2 deletions src/app/tests/TestCommandInteraction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,9 @@ class TestCommandInteraction

class TestExchangeDelegate : public Messaging::ExchangeDelegate
{
void OnMessageReceived(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader, const PayloadHeader & payloadHeader,
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
4 changes: 2 additions & 2 deletions src/app/tests/TestReportingEngine.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,9 @@ class TestReportingEngine

class TestExchangeDelegate : public Messaging::ExchangeDelegate
{
void OnMessageReceived(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader, const PayloadHeader & payloadHeader,
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
4 changes: 2 additions & 2 deletions src/app/tests/TestWriteInteraction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,9 @@ class TestWriteInteraction

class TestExchangeDelegate : public Messaging::ExchangeDelegate
{
void OnMessageReceived(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader, const PayloadHeader & payloadHeader,
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
4 changes: 2 additions & 2 deletions src/app/util/chip-message-send.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ namespace chip {
// Delete this class when Device::SendMessage() is obsoleted.
class DeviceExchangeDelegate : public Messaging::ExchangeDelegate
{
void OnMessageReceived(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader, const PayloadHeader & payloadHeader,
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
3 changes: 2 additions & 1 deletion src/channel/tests/TestChannel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,11 @@ TransportMgr<LoopbackTransport> gTransportMgr;
class MockAppDelegate : public ExchangeDelegate
{
public:
void OnMessageReceived(ExchangeContext * ec, const PacketHeader & packetHeader, const PayloadHeader & payloadHeader,
CHIP_ERROR OnMessageReceived(ExchangeContext * ec, const PacketHeader & packetHeader, const PayloadHeader & payloadHeader,
System::PacketBufferHandle && buffer) override
{
IsOnMessageReceivedCalled = true;
return CHIP_NO_ERROR;
}

void OnResponseTimeout(ExchangeContext * ec) override {}
Expand Down
3 changes: 2 additions & 1 deletion src/controller/CHIPDevice.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ void Device::OnConnectionExpired(SecureSessionHandle session)
mSecureSession = SecureSessionHandle{};
}

void Device::OnMessageReceived(Messaging::ExchangeContext * exchange, const PacketHeader & header,
CHIP_ERROR Device::OnMessageReceived(Messaging::ExchangeContext * exchange, const PacketHeader & header,
const PayloadHeader & payloadHeader, System::PacketBufferHandle && msgBuf)
{
if (mState == ConnectionState::SecureConnected)
Expand All @@ -339,6 +339,7 @@ void Device::OnMessageReceived(Messaging::ExchangeContext * exchange, const Pack
}
}
exchange->Close();
return CHIP_NO_ERROR;
}

void Device::OnResponseTimeout(Messaging::ExchangeContext * ec)
Expand Down
2 changes: 1 addition & 1 deletion src/controller/CHIPDevice.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ class DLL_EXPORT Device : public Messaging::ExchangeDelegate, public SessionEsta
* @param[in] payloadHeader Reference to payload header in the message
* @param[in] msgBuf The message buffer
*/
void OnMessageReceived(Messaging::ExchangeContext * exchange, const PacketHeader & header, const PayloadHeader & payloadHeader,
CHIP_ERROR OnMessageReceived(Messaging::ExchangeContext * exchange, const PacketHeader & header, const PayloadHeader & payloadHeader,
System::PacketBufferHandle && msgBuf) override;

/**
Expand Down
Loading

0 comments on commit be97a05

Please sign in to comment.