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

Decouple InteractionModelEngine from TimedHandler #32076

Merged
merged 10 commits into from
Feb 15, 2024
2 changes: 1 addition & 1 deletion src/app/InteractionModelEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -840,7 +840,7 @@ CHIP_ERROR InteractionModelEngine::OnTimedRequest(Messaging::ExchangeContext * a
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload,
Protocols::InteractionModel::Status & aStatus)
{
TimedHandler * handler = mTimedHandlers.CreateObject();
TimedHandler * handler = mTimedHandlers.CreateObject(this);
if (handler == nullptr)
{
ChipLogProgress(InteractionModel, "no resource for Timed interaction");
Expand Down
25 changes: 6 additions & 19 deletions src/app/InteractionModelEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
public CommandHandler::Callback,
public ReadHandler::ManagementCallback,
public FabricTable::Delegate,
public SubscriptionsInfoProvider
public SubscriptionsInfoProvider,
public TimedHandlerDelegate
{
public:
/**
Expand Down Expand Up @@ -218,26 +219,12 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
}
void UnregisterReadHandlerAppCallback() { mpReadHandlerApplicationCallback = nullptr; }

/**
* Called when a timed interaction has failed (i.e. the exchange it was
* happening on has closed while the exchange delegate was the timed
* handler).
*/
void OnTimedInteractionFailed(TimedHandler * apTimedHandler);

/**
* Called when a timed invoke is received. This function takes over all
* handling of the exchange, status reporting, and so forth.
*/
// TimedHandlerDelegate implementation
void OnTimedInteractionFailed(TimedHandler * apTimedHandler) override;
void OnTimedInvoke(TimedHandler * apTimedHandler, Messaging::ExchangeContext * apExchangeContext,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload);

/**
* Called when a timed write is received. This function takes over all
* handling of the exchange, status reporting, and so forth.
*/
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload) override;
void OnTimedWrite(TimedHandler * apTimedHandler, Messaging::ExchangeContext * apExchangeContext,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload);
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload) override;

#if CHIP_CONFIG_ENABLE_READ_CLIENT
/**
Expand Down
10 changes: 4 additions & 6 deletions src/app/TimedHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
*/

#include "TimedHandler.h"
#include <app/InteractionModelEngine.h>
#include <app/InteractionModelTimeout.h>
#include <app/MessageDef/TimedRequestMessage.h>
#include <app/StatusResponse.h>
#include <lib/core/TLV.h>
Expand Down Expand Up @@ -74,19 +74,17 @@ CHIP_ERROR TimedHandler::OnMessageReceived(Messaging::ExchangeContext * aExchang

if (aPayloadHeader.HasMessageType(MsgType::InvokeCommandRequest))
{
auto * imEngine = InteractionModelEngine::GetInstance();
ChipLogDetail(DataManagement, "Handing timed invoke to IM engine: handler %p exchange " ChipLogFormatExchange, this,
ChipLogValueExchange(aExchangeContext));
imEngine->OnTimedInvoke(this, aExchangeContext, aPayloadHeader, std::move(aPayload));
mDelegate->OnTimedInvoke(this, aExchangeContext, aPayloadHeader, std::move(aPayload));
return CHIP_NO_ERROR;
}

if (aPayloadHeader.HasMessageType(MsgType::WriteRequest))
{
auto * imEngine = InteractionModelEngine::GetInstance();
ChipLogDetail(DataManagement, "Handing timed write to IM engine: handler %p exchange " ChipLogFormatExchange, this,
ChipLogValueExchange(aExchangeContext));
imEngine->OnTimedWrite(this, aExchangeContext, aPayloadHeader, std::move(aPayload));
mDelegate->OnTimedWrite(this, aExchangeContext, aPayloadHeader, std::move(aPayload));
return CHIP_NO_ERROR;
}
}
Expand All @@ -101,7 +99,7 @@ CHIP_ERROR TimedHandler::OnMessageReceived(Messaging::ExchangeContext * aExchang

void TimedHandler::OnExchangeClosing(Messaging::ExchangeContext *)
{
InteractionModelEngine::GetInstance()->OnTimedInteractionFailed(this);
mDelegate->OnTimedInteractionFailed(this);
}

CHIP_ERROR TimedHandler::HandleTimedRequestAction(Messaging::ExchangeContext * aExchangeContext,
Expand Down
40 changes: 32 additions & 8 deletions src/app/TimedHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@
#include <system/SystemPacketBuffer.h>
#include <transport/raw/MessageHeader.h>

namespace chip {
namespace app {

class TimedHandler;

/**
* A TimedHandler handles a Timed Request action and then waits for a
* subsequent Invoke or Write action and hands those on to
Expand All @@ -43,14 +48,37 @@
* either the exchange is closed or the interaction is handed on to the
* InteractionModelEngine.
*/
class TimedHandlerDelegate
{
public:
virtual ~TimedHandlerDelegate() = default;

namespace chip {
namespace app {
/**
* Called when a timed invoke is received. This function takes over all
* handling of the exchange, status reporting, and so forth.
*/
virtual void OnTimedInvoke(TimedHandler * apTimedHandler, Messaging::ExchangeContext * apExchangeContext,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload) = 0;

/**
* Called when a timed write is received. This function takes over all
* handling of the exchange, status reporting, and so forth.
*/
virtual void OnTimedWrite(TimedHandler * apTimedHandler, Messaging::ExchangeContext * apExchangeContext,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload) = 0;

/**
* Called when a timed interaction has failed (i.e. the exchange it was
* happening on has closed while the exchange delegate was the timed
* handler).
*/
virtual void OnTimedInteractionFailed(TimedHandler * apTimedHandler) = 0;
};

class TimedHandler : public Messaging::ExchangeDelegate
{
public:
TimedHandler() {}
TimedHandler(TimedHandlerDelegate * delegate) : mDelegate(delegate) {}
~TimedHandler() override {}

// ExchangeDelegate implementation.
Expand Down Expand Up @@ -83,11 +111,7 @@ class TimedHandler : public Messaging::ExchangeDelegate
kExpectingFollowingAction, // Expecting write or invoke.
};

// Because we have a vtable pointer and mTimeLimit needs to be 8-byte
// aligned on ARM, putting mState first here means we fit in 16 bytes on
// 32-bit ARM, whereas if we put it second we'd be 24 bytes.
// On platforms where either vtable pointers are 8 bytes or 64-bit ints can
// be 4-byte-aligned the ordering here does not matter.
andy31415 marked this conversation as resolved.
Show resolved Hide resolved
TimedHandlerDelegate * mDelegate;
State mState = State::kExpectingTimedAction;
// We keep track of the time limit for message reception, in case our
// exchange's "response expected" timer gets delayed and does not fire when
Expand Down
Loading