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

Actions: Use an instance of the Actions cluster per endpoint instead of having global delegate array. #37799

Merged
merged 4 commits into from
Mar 4, 2025
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -24,6 +24,11 @@ using namespace chip::app::Clusters::Actions;
using namespace chip::app::Clusters::Actions::Attributes;
using namespace chip::Protocols::InteractionModel;

namespace {
Clusters::Actions::ActionsDelegateImpl * sActionsDelegateImpl = nullptr;
Clusters::Actions::ActionsServer * sActionsServer = nullptr;
} // namespace

CHIP_ERROR ActionsDelegateImpl::ReadActionAtIndex(uint16_t index, ActionStructStorage & action)
{
if (index >= kActionList.size())
@@ -129,3 +134,12 @@ Status ActionsDelegateImpl::HandleDisableActionWithDuration(uint16_t actionId, u
// Not implemented
return Status::NotFound;
}

void emberAfActionsClusterInitCallback(EndpointId endpoint)
{
VerifyOrReturn(endpoint == 1);
VerifyOrReturn(sActionsDelegateImpl == nullptr && sActionsServer == nullptr);
sActionsDelegateImpl = new Actions::ActionsDelegateImpl;
sActionsServer = new Actions::ActionsServer(endpoint, sActionsDelegateImpl);
sActionsServer->Init();
}
8 changes: 0 additions & 8 deletions examples/all-clusters-app/linux/main-common.cpp
Original file line number Diff line number Diff line change
@@ -54,7 +54,6 @@
#include <app/clusters/valve-configuration-and-control-server/valve-configuration-and-control-server.h>
#include <app/server/Server.h>
#include <app/util/attribute-storage.h>
#include <bridged-actions-stub.h>
#include <lib/support/CHIPMem.h>
#include <platform/DeviceInstanceInfoProvider.h>
#include <platform/DiagnosticDataProvider.h>
@@ -87,7 +86,6 @@ Clusters::TemperatureControl::AppSupportedTemperatureLevelsDelegate sAppSupporte
Clusters::ModeSelect::StaticSupportedModesManager sStaticSupportedModesManager;
Clusters::ValveConfigurationAndControl::ValveControlDelegate sValveDelegate;
Clusters::TimeSynchronization::ExtendedTimeSyncDelegate sTimeSyncDelegate;
Clusters::Actions::ActionsDelegateImpl sActionsDelegateImpl;

// Please refer to https://github.com/CHIP-Specifications/connectedhomeip-spec/blob/master/src/namespaces
constexpr const uint8_t kNamespaceCommon = 7;
@@ -342,12 +340,6 @@ void emberAfThermostatClusterInitCallback(EndpointId endpoint)
SetDefaultDelegate(endpoint, &delegate);
}

void emberAfActionsClusterInitCallback(EndpointId endpoint)
{
VerifyOrReturn(endpoint == 1);
Clusters::Actions::ActionsServer::Instance().SetDefaultDelegate(endpoint, &sActionsDelegateImpl);
}

Status emberAfExternalAttributeReadCallback(EndpointId endpoint, ClusterId clusterId,
const EmberAfAttributeMetadata * attributeMetadata, uint8_t * buffer,
uint16_t maxReadLength)
163 changes: 38 additions & 125 deletions src/app/clusters/actions-server/actions-server.cpp
Original file line number Diff line number Diff line change
@@ -32,44 +32,22 @@ using namespace chip::app::Clusters::Actions;
using namespace chip::app::Clusters::Actions::Attributes;
using namespace chip::Protocols::InteractionModel;

namespace {
static constexpr size_t kActionsDelegateTableSize =
MATTER_DM_ACTIONS_CLUSTER_SERVER_ENDPOINT_COUNT + CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT;
static_assert(kActionsDelegateTableSize <= kEmberInvalidEndpointIndex, "Actions Delegate table size error");

// TODO: We should not use global array, instead we can use one cluster instance per endpoint.
Delegate * gDelegateTable[kActionsDelegateTableSize] = { nullptr };

Delegate * GetDelegate(EndpointId aEndpoint)
ActionsServer::~ActionsServer()
{
return (aEndpoint >= kActionsDelegateTableSize ? nullptr : gDelegateTable[aEndpoint]);
Shutdown();
}

CHIP_ERROR ValidateDelegate(Delegate * aDelegate, EndpointId aEndpoint)
void ActionsServer::Shutdown()
{
if (aDelegate == nullptr)
{
ChipLogError(Zcl, "Actions delegate is null for endpoint: %d !!!", aEndpoint);
return CHIP_ERROR_INCORRECT_STATE;
}
return CHIP_NO_ERROR;
}

} // namespace

ActionsServer ActionsServer::sInstance;

void ActionsServer::SetDefaultDelegate(EndpointId aEndpoint, Delegate * aDelegate)
{
if (aEndpoint < kActionsDelegateTableSize)
{
gDelegateTable[aEndpoint] = aDelegate;
}
CommandHandlerInterfaceRegistry::Instance().UnregisterCommandHandler(this);
AttributeAccessInterfaceRegistry::Instance().Unregister(this);
}

ActionsServer & ActionsServer::Instance()
CHIP_ERROR ActionsServer::Init()
{
return sInstance;
ReturnErrorOnFailure(CommandHandlerInterfaceRegistry::Instance().RegisterCommandHandler(this));
VerifyOrReturnError(AttributeAccessInterfaceRegistry::Instance().Register(this), CHIP_ERROR_INCORRECT_STATE);
return CHIP_NO_ERROR;
}

void ActionsServer::OnStateChanged(EndpointId aEndpoint, uint16_t aActionId, uint32_t aInvokeId, ActionStateEnum aActionState)
@@ -126,14 +104,10 @@ CHIP_ERROR ActionsServer::Read(const ConcreteReadAttributePath & aPath, Attribut
CHIP_ERROR ActionsServer::ReadActionListAttribute(const ConcreteReadAttributePath & aPath,
const AttributeValueEncoder::ListEncodeHelper & aEncoder)
{
Delegate * delegate = GetDelegate(aPath.mEndpointId);
ReturnErrorOnFailure(ValidateDelegate(delegate, aPath.mEndpointId));

for (uint16_t i = 0; i < kMaxActionListLength; i++)
{
ActionStructStorage action;
CHIP_ERROR err = delegate->ReadActionAtIndex(i, action);

CHIP_ERROR err = mDelegate->ReadActionAtIndex(i, action);
if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED)
{
return CHIP_NO_ERROR;
@@ -147,14 +121,10 @@ CHIP_ERROR ActionsServer::ReadActionListAttribute(const ConcreteReadAttributePat
CHIP_ERROR ActionsServer::ReadEndpointListAttribute(const ConcreteReadAttributePath & aPath,
const AttributeValueEncoder::ListEncodeHelper & aEncoder)
{
Delegate * delegate = GetDelegate(aPath.mEndpointId);
ReturnErrorOnFailure(ValidateDelegate(delegate, aPath.mEndpointId));

for (uint16_t i = 0; i < kMaxEndpointListLength; i++)
{
EndpointListStorage epList;

CHIP_ERROR err = delegate->ReadEndpointListAtIndex(i, epList);
CHIP_ERROR err = mDelegate->ReadEndpointListAtIndex(i, epList);
if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED)
{
return CHIP_NO_ERROR;
@@ -167,9 +137,7 @@ CHIP_ERROR ActionsServer::ReadEndpointListAttribute(const ConcreteReadAttributeP

bool ActionsServer::HaveActionWithId(EndpointId aEndpointId, uint16_t aActionId, uint16_t & aActionIndex)
{
Delegate * delegate = GetDelegate(aEndpointId);
VerifyOrReturnValue(ValidateDelegate(delegate, aEndpointId) == CHIP_NO_ERROR, false);
return delegate->HaveActionWithId(aActionId, aActionIndex);
return mDelegate->HaveActionWithId(aActionId, aActionIndex);
}

template <typename RequestT, typename FuncT>
@@ -201,10 +169,8 @@ void ActionsServer::HandleCommand(HandlerContext & handlerContext, FuncT func)
}
if (actionIndex != kMaxActionListLength)
{
Delegate * delegate = GetDelegate(handlerContext.mRequestPath.mEndpointId);
ReturnOnFailure(ValidateDelegate(delegate, handlerContext.mRequestPath.mEndpointId));
ActionStructStorage action;
delegate->ReadActionAtIndex(actionIndex, action);
mDelegate->ReadActionAtIndex(actionIndex, action);
// Check if the command bit is set in the SupportedCommands of an ations.
if (!(action.supportedCommands.Raw() & (1 << handlerContext.mRequestPath.mCommandId)))
{
@@ -220,139 +186,90 @@ void ActionsServer::HandleCommand(HandlerContext & handlerContext, FuncT func)

void ActionsServer::HandleInstantAction(HandlerContext & ctx, const Commands::InstantAction::DecodableType & commandData)
{
Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId);
Status status = Status::InvalidInState;
if (delegate != nullptr)
{
status = delegate->HandleInstantAction(commandData.actionID, commandData.invokeID);
}
Status status = Status::InvalidInState;
status = mDelegate->HandleInstantAction(commandData.actionID, commandData.invokeID);
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
}

void ActionsServer::HandleInstantActionWithTransition(HandlerContext & ctx,
const Commands::InstantActionWithTransition::DecodableType & commandData)
{
Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId);
Status status = Status::InvalidInState;
if (delegate != nullptr)
{
status =
delegate->HandleInstantActionWithTransition(commandData.actionID, commandData.transitionTime, commandData.invokeID);
}
Status status = Status::InvalidInState;
status = mDelegate->HandleInstantActionWithTransition(commandData.actionID, commandData.transitionTime, commandData.invokeID);
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
}

void ActionsServer::HandleStartAction(HandlerContext & ctx, const Commands::StartAction::DecodableType & commandData)
{
Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId);
Status status = Status::InvalidInState;
if (delegate != nullptr)
{
status = delegate->HandleStartAction(commandData.actionID, commandData.invokeID);
}
Status status = Status::InvalidInState;
status = mDelegate->HandleStartAction(commandData.actionID, commandData.invokeID);
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
}

void ActionsServer::HandleStartActionWithDuration(HandlerContext & ctx,
const Commands::StartActionWithDuration::DecodableType & commandData)
{
Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId);
Status status = Status::InvalidInState;
if (delegate != nullptr)
{
status = delegate->HandleStartActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID);
}
Status status = Status::InvalidInState;
status = mDelegate->HandleStartActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID);
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
}

void ActionsServer::HandleStopAction(HandlerContext & ctx, const Commands::StopAction::DecodableType & commandData)
{
Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId);
Status status = Status::InvalidInState;
if (delegate != nullptr)
{
status = delegate->HandleStopAction(commandData.actionID, commandData.invokeID);
}
Status status = Status::InvalidInState;
status = mDelegate->HandleStopAction(commandData.actionID, commandData.invokeID);
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
}

void ActionsServer::HandlePauseAction(HandlerContext & ctx, const Commands::PauseAction::DecodableType & commandData)
{
Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId);
Status status = Status::InvalidInState;
if (delegate != nullptr)
{
status = delegate->HandlePauseAction(commandData.actionID, commandData.invokeID);
}
Status status = Status::InvalidInState;
status = mDelegate->HandlePauseAction(commandData.actionID, commandData.invokeID);
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
}

void ActionsServer::HandlePauseActionWithDuration(HandlerContext & ctx,
const Commands::PauseActionWithDuration::DecodableType & commandData)
{
Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId);
Status status = Status::InvalidInState;
if (delegate != nullptr)
{
status = delegate->HandlePauseActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID);
}
Status status = Status::InvalidInState;
status = mDelegate->HandlePauseActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID);
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
}

void ActionsServer::HandleResumeAction(HandlerContext & ctx, const Commands::ResumeAction::DecodableType & commandData)
{
Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId);
Status status = Status::InvalidInState;
if (delegate != nullptr)
{
status = delegate->HandleResumeAction(commandData.actionID, commandData.invokeID);
}
Status status = Status::InvalidInState;
status = mDelegate->HandleResumeAction(commandData.actionID, commandData.invokeID);
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
}

void ActionsServer::HandleEnableAction(HandlerContext & ctx, const Commands::EnableAction::DecodableType & commandData)
{
Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId);
Status status = Status::InvalidInState;
if (delegate != nullptr)
{
status = delegate->HandleEnableAction(commandData.actionID, commandData.invokeID);
}
Status status = Status::InvalidInState;
status = mDelegate->HandleEnableAction(commandData.actionID, commandData.invokeID);
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
}

void ActionsServer::HandleEnableActionWithDuration(HandlerContext & ctx,
const Commands::EnableActionWithDuration::DecodableType & commandData)
{
Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId);
Status status = Status::InvalidInState;
if (delegate != nullptr)
{
status = delegate->HandleEnableActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID);
}
Status status = Status::InvalidInState;
status = mDelegate->HandleEnableActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID);
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
}

void ActionsServer::HandleDisableAction(HandlerContext & ctx, const Commands::DisableAction::DecodableType & commandData)
{
Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId);
Status status = Status::InvalidInState;
if (delegate != nullptr)
{
status = delegate->HandleDisableAction(commandData.actionID, commandData.invokeID);
}
Status status = Status::InvalidInState;
status = mDelegate->HandleDisableAction(commandData.actionID, commandData.invokeID);
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
}

void ActionsServer::HandleDisableActionWithDuration(HandlerContext & ctx,
const Commands::DisableActionWithDuration::DecodableType & commandData)
{
Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId);
Status status = Status::InvalidInState;
if (delegate != nullptr)
{
status = delegate->HandleDisableActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID);
}
Status status = Status::InvalidInState;
status = mDelegate->HandleDisableActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID);
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
}

@@ -426,8 +343,4 @@ void ActionsServer::EndpointListModified(EndpointId aEndpoint)
MarkDirty(aEndpoint, Attributes::EndpointLists::Id);
}

void MatterActionsPluginServerInitCallback()
{
AttributeAccessInterfaceRegistry::Instance().Register(&ActionsServer::Instance());
CommandHandlerInterfaceRegistry::Instance().RegisterCommandHandler(&ActionsServer::Instance());
}
void MatterActionsPluginServerInitCallback() {}
23 changes: 19 additions & 4 deletions src/app/clusters/actions-server/actions-server.h
Original file line number Diff line number Diff line change
@@ -114,11 +114,24 @@ class ActionsServer : public AttributeAccessInterface, public CommandHandlerInte
{
public:
// Register for the Actions cluster on all endpoints.
ActionsServer() :
AttributeAccessInterface(Optional<EndpointId>::Missing(), Actions::Id),
CommandHandlerInterface(Optional<EndpointId>::Missing(), Actions::Id)
ActionsServer(EndpointId aEndpointId, Delegate * aDelegate) :
AttributeAccessInterface(Optional<EndpointId>(aEndpointId), Actions::Id),
CommandHandlerInterface(Optional<EndpointId>(aEndpointId), Actions::Id), mDelegate(aDelegate), mEndpointId(aEndpointId)
{}
static ActionsServer & Instance();

~ActionsServer();

/**
* Initialise the Actions server instance.
* @return Returns an error if the given endpoint and cluster have not been enabled in zap, if the
* AttributeAccessInterface or AttributeAccessInterface registration fails returns an error.
*/
CHIP_ERROR Init();

/**
* Unregisters the CommandHandlerInterface and AttributeAccessInterface.
*/
void Shutdown();

/**
* @brief
@@ -152,6 +165,8 @@ class ActionsServer : public AttributeAccessInterface, public CommandHandlerInte
void EndpointListModified(EndpointId aEndpoint);

private:
Delegate * mDelegate;
EndpointId mEndpointId;
static ActionsServer sInstance;
static constexpr size_t kMaxEndpointListLength = 256u;
static constexpr size_t kMaxActionListLength = 256u;
Loading
Loading