Skip to content

Commit

Permalink
Remove unnecessary exchange manager from CommandHandler. (#10351)
Browse files Browse the repository at this point in the history
CommandHandler never needs to create exchanges.

Fixes #10329
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Nov 18, 2021
1 parent 211266a commit 1fdfb46
Show file tree
Hide file tree
Showing 8 changed files with 16 additions and 23 deletions.
5 changes: 1 addition & 4 deletions src/app/Command.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,7 @@
namespace chip {
namespace app {

Command::Command(Messaging::ExchangeManager * apExchangeMgr)
{
mpExchangeMgr = apExchangeMgr;
}
Command::Command() {}

CHIP_ERROR Command::AllocateBuffer()
{
Expand Down
4 changes: 1 addition & 3 deletions src/app/Command.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
#include <lib/support/DLLUtil.h>
#include <lib/support/logging/CHIPLogging.h>
#include <messaging/ExchangeContext.h>
#include <messaging/ExchangeMgr.h>
#include <messaging/Flags.h>
#include <protocols/Protocols.h>
#include <system/SystemPacketBuffer.h>
Expand Down Expand Up @@ -97,7 +96,7 @@ class Command
virtual CHIP_ERROR ProcessCommandDataElement(CommandDataElement::Parser & aCommandElement) = 0;

protected:
Command(Messaging::ExchangeManager * apExchangeMgr);
Command();

/*
* Allocates a packet buffer used for encoding an invoke request/response payload.
Expand All @@ -120,7 +119,6 @@ class Command
const char * GetStateStr() const;

InvokeCommand::Builder mInvokeCommandBuilder;
Messaging::ExchangeManager * mpExchangeMgr = nullptr;
Messaging::ExchangeContext * mpExchangeCtx = nullptr;
uint8_t mCommandIndex = 0;
CommandState mState = CommandState::Idle;
Expand Down
6 changes: 2 additions & 4 deletions src/app/CommandHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
#include "Command.h"
#include "CommandSender.h"
#include "InteractionModelEngine.h"
#include "messaging/ExchangeMgr.h"
#include "messaging/ExchangeContext.h"

#include <lib/support/TypeTraits.h>
#include <protocols/secure_channel/Constants.h>
Expand All @@ -36,9 +36,7 @@ using GeneralStatusCode = chip::Protocols::SecureChannel::GeneralStatusCode;
namespace chip {
namespace app {

CommandHandler::CommandHandler(Messaging::ExchangeManager * apExchangeMgr, Callback * apCallback) :
Command(apExchangeMgr), mpCallback(apCallback)
{}
CommandHandler::CommandHandler(Callback * apCallback) : mpCallback(apCallback) {}

CHIP_ERROR CommandHandler::OnInvokeCommandRequest(Messaging::ExchangeContext * ec, const PayloadHeader & payloadHeader,
System::PacketBufferHandle && payload)
Expand Down
3 changes: 1 addition & 2 deletions src/app/CommandHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
#include <lib/support/DLLUtil.h>
#include <lib/support/logging/CHIPLogging.h>
#include <messaging/ExchangeContext.h>
#include <messaging/ExchangeMgr.h>
#include <messaging/Flags.h>
#include <protocols/Protocols.h>
#include <system/SystemPacketBuffer.h>
Expand Down Expand Up @@ -63,7 +62,7 @@ class CommandHandler : public Command
*
* The callback passed in has to outlive this CommandHandler object.
*/
CommandHandler(Messaging::ExchangeManager * apExchangeMgr, Callback * apCallback);
CommandHandler(Callback * apCallback);

/*
* Main entrypoint for this class to handle an invoke request.
Expand Down
2 changes: 1 addition & 1 deletion src/app/CommandSender.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ namespace chip {
namespace app {

CommandSender::CommandSender(Callback * apCallback, Messaging::ExchangeManager * apExchangeMgr) :
Command(apExchangeMgr), mpCallback(apCallback)
mpCallback(apCallback), mpExchangeMgr(apExchangeMgr)
{}

CHIP_ERROR CommandSender::SendCommandRequest(NodeId aNodeId, FabricIndex aFabricIndex, Optional<SessionHandle> secureSession,
Expand Down
3 changes: 2 additions & 1 deletion src/app/CommandSender.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ class CommandSender final : public Command, public Messaging::ExchangeDelegate

CHIP_ERROR ProcessCommandDataElement(CommandDataElement::Parser & aCommandElement) override;

Callback * mpCallback = nullptr;
Callback * mpCallback = nullptr;
Messaging::ExchangeManager * mpExchangeMgr = nullptr;
};

} // namespace app
Expand Down
2 changes: 1 addition & 1 deletion src/app/InteractionModelEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ CHIP_ERROR InteractionModelEngine::OnInvokeCommandRequest(Messaging::ExchangeCon
const PayloadHeader & aPayloadHeader,
System::PacketBufferHandle && aPayload)
{
CommandHandler * commandHandler = mCommandHandlerObjs.CreateObject(mpExchangeMgr, this);
CommandHandler * commandHandler = mCommandHandlerObjs.CreateObject(this);
if (commandHandler == nullptr)
{
return CHIP_ERROR_NO_MEMORY;
Expand Down
14 changes: 7 additions & 7 deletions src/app/tests/TestCommandInteraction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ void TestCommandInteraction::TestCommandHandlerWithWrongState(nlTestSuite * apSu
CHIP_ERROR err = CHIP_NO_ERROR;
auto commandPathParams = MakeTestCommandPath();

app::CommandHandler commandHandler(gExchangeManager, &mockCommandHandlerDelegate);
app::CommandHandler commandHandler(&mockCommandHandlerDelegate);

err = commandHandler.PrepareCommand(commandPathParams);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
Expand Down Expand Up @@ -346,7 +346,7 @@ void TestCommandInteraction::TestCommandHandlerWithSendEmptyCommand(nlTestSuite
CHIP_ERROR err = CHIP_NO_ERROR;
auto commandPathParams = MakeTestCommandPath();

app::CommandHandler commandHandler(gExchangeManager, &mockCommandHandlerDelegate);
app::CommandHandler commandHandler(&mockCommandHandlerDelegate);
System::PacketBufferHandle commandDatabuf = System::PacketBufferHandle::New(System::PacketBuffer::kMaxSize);

auto exchangeCtx = gExchangeManager->NewContext(SessionHandle(0, 0, 0, 0), nullptr);
Expand Down Expand Up @@ -378,7 +378,7 @@ void TestCommandInteraction::TestCommandSenderWithProcessReceivedMsg(nlTestSuite
void TestCommandInteraction::ValidateCommandHandlerWithSendCommand(nlTestSuite * apSuite, void * apContext, bool aNeedStatusCode)
{
CHIP_ERROR err = CHIP_NO_ERROR;
app::CommandHandler commandHandler(gExchangeManager, &mockCommandHandlerDelegate);
app::CommandHandler commandHandler(&mockCommandHandlerDelegate);
System::PacketBufferHandle commandPacket;

auto exchangeCtx = gExchangeManager->NewContext(SessionHandle(0, 0, 0, 0), nullptr);
Expand Down Expand Up @@ -425,7 +425,7 @@ struct Fields
void TestCommandInteraction::TestCommandHandlerCommandDataEncoding(nlTestSuite * apSuite, void * apContext)
{
CHIP_ERROR err = CHIP_NO_ERROR;
app::CommandHandler commandHandler(gExchangeManager, nullptr);
app::CommandHandler commandHandler(nullptr);
System::PacketBufferHandle commandPacket;

commandHandler.mpExchangeCtx = gExchangeManager->NewContext(SessionHandle(0, 0, 0, 0), nullptr);
Expand Down Expand Up @@ -461,7 +461,7 @@ void TestCommandInteraction::TestCommandHandlerWithSendSimpleStatusCode(nlTestSu
void TestCommandInteraction::TestCommandHandlerWithProcessReceivedMsg(nlTestSuite * apSuite, void * apContext)
{
CHIP_ERROR err = CHIP_NO_ERROR;
app::CommandHandler commandHandler(gExchangeManager, &mockCommandHandlerDelegate);
app::CommandHandler commandHandler(&mockCommandHandlerDelegate);
System::PacketBufferHandle commandDatabuf = System::PacketBufferHandle::New(System::PacketBuffer::kMaxSize);

GenerateReceivedCommand(apSuite, apContext, commandDatabuf, true /*aNeedCommandData*/);
Expand All @@ -472,7 +472,7 @@ void TestCommandInteraction::TestCommandHandlerWithProcessReceivedMsg(nlTestSuit
void TestCommandInteraction::TestCommandHandlerWithProcessReceivedNotExistCommand(nlTestSuite * apSuite, void * apContext)
{
CHIP_ERROR err = CHIP_NO_ERROR;
app::CommandHandler commandHandler(gExchangeManager, &mockCommandHandlerDelegate);
app::CommandHandler commandHandler(&mockCommandHandlerDelegate);
System::PacketBufferHandle commandDatabuf = System::PacketBufferHandle::New(System::PacketBuffer::kMaxSize);

// Use some invalid endpoint / cluster / command.
Expand All @@ -489,7 +489,7 @@ void TestCommandInteraction::TestCommandHandlerWithProcessReceivedNotExistComman
void TestCommandInteraction::TestCommandHandlerWithProcessReceivedEmptyDataMsg(nlTestSuite * apSuite, void * apContext)
{
CHIP_ERROR err = CHIP_NO_ERROR;
app::CommandHandler commandHandler(gExchangeManager, &mockCommandHandlerDelegate);
app::CommandHandler commandHandler(&mockCommandHandlerDelegate);
System::PacketBufferHandle commandDatabuf = System::PacketBufferHandle::New(System::PacketBuffer::kMaxSize);

chip::isCommandDispatched = false;
Expand Down

0 comments on commit 1fdfb46

Please sign in to comment.