Skip to content

Commit bdc4a48

Browse files
Actions: Use instance of the Actions cluster per endpoint instead of having global delegate array.
1 parent 88e9921 commit bdc4a48

File tree

5 files changed

+97
-149
lines changed

5 files changed

+97
-149
lines changed

examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp

+12
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ using namespace chip::app::Clusters::Actions;
2424
using namespace chip::app::Clusters::Actions::Attributes;
2525
using namespace chip::Protocols::InteractionModel;
2626

27+
Clusters::Actions::ActionsDelegateImpl * sActionsDelegateImpl = nullptr;
28+
Clusters::Actions::ActionsServer * sActionsServer = nullptr;
29+
2730
CHIP_ERROR ActionsDelegateImpl::ReadActionAtIndex(uint16_t index, ActionStructStorage & action)
2831
{
2932
if (index >= kActionList.size())
@@ -129,3 +132,12 @@ Status ActionsDelegateImpl::HandleDisableActionWithDuration(uint16_t actionId, u
129132
// Not implemented
130133
return Status::NotFound;
131134
}
135+
136+
void emberAfActionsClusterInitCallback(EndpointId endpoint)
137+
{
138+
VerifyOrReturn(endpoint == 1);
139+
VerifyOrReturn(sActionsDelegateImpl == nullptr && sActionsServer == nullptr);
140+
sActionsDelegateImpl = new Actions::ActionsDelegateImpl;
141+
sActionsServer = new Actions::ActionsServer(endpoint, sActionsDelegateImpl);
142+
sActionsServer->Init();
143+
}

examples/all-clusters-app/linux/main-common.cpp

-8
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@
5454
#include <app/clusters/valve-configuration-and-control-server/valve-configuration-and-control-server.h>
5555
#include <app/server/Server.h>
5656
#include <app/util/attribute-storage.h>
57-
#include <bridged-actions-stub.h>
5857
#include <lib/support/CHIPMem.h>
5958
#include <platform/DeviceInstanceInfoProvider.h>
6059
#include <platform/DiagnosticDataProvider.h>
@@ -87,7 +86,6 @@ Clusters::TemperatureControl::AppSupportedTemperatureLevelsDelegate sAppSupporte
8786
Clusters::ModeSelect::StaticSupportedModesManager sStaticSupportedModesManager;
8887
Clusters::ValveConfigurationAndControl::ValveControlDelegate sValveDelegate;
8988
Clusters::TimeSynchronization::ExtendedTimeSyncDelegate sTimeSyncDelegate;
90-
Clusters::Actions::ActionsDelegateImpl sActionsDelegateImpl;
9189

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

345-
void emberAfActionsClusterInitCallback(EndpointId endpoint)
346-
{
347-
VerifyOrReturn(endpoint == 1);
348-
Clusters::Actions::ActionsServer::Instance().SetDefaultDelegate(endpoint, &sActionsDelegateImpl);
349-
}
350-
351343
Status emberAfExternalAttributeReadCallback(EndpointId endpoint, ClusterId clusterId,
352344
const EmberAfAttributeMetadata * attributeMetadata, uint8_t * buffer,
353345
uint16_t maxReadLength)

src/app/clusters/actions-server/actions-server.cpp

+40-122
Original file line numberDiff line numberDiff line change
@@ -32,44 +32,25 @@ using namespace chip::app::Clusters::Actions;
3232
using namespace chip::app::Clusters::Actions::Attributes;
3333
using namespace chip::Protocols::InteractionModel;
3434

35-
namespace {
36-
static constexpr size_t kActionsDelegateTableSize =
37-
MATTER_DM_ACTIONS_CLUSTER_SERVER_ENDPOINT_COUNT + CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT;
38-
static_assert(kActionsDelegateTableSize <= kEmberInvalidEndpointIndex, "Actions Delegate table size error");
39-
40-
// TODO: We should not use global array, instead we can use one cluster instance per endpoint.
41-
Delegate * gDelegateTable[kActionsDelegateTableSize] = { nullptr };
42-
43-
Delegate * GetDelegate(EndpointId aEndpoint)
35+
ActionsServer::~ActionsServer()
4436
{
45-
return (aEndpoint >= kActionsDelegateTableSize ? nullptr : gDelegateTable[aEndpoint]);
37+
Shutdown();
4638
}
4739

48-
CHIP_ERROR ValidateDelegate(Delegate * aDelegate, EndpointId aEndpoint)
40+
void ActionsServer::Shutdown()
4941
{
50-
if (aDelegate == nullptr)
51-
{
52-
ChipLogError(Zcl, "Actions delegate is null for endpoint: %d !!!", aEndpoint);
53-
return CHIP_ERROR_INCORRECT_STATE;
54-
}
55-
return CHIP_NO_ERROR;
42+
CommandHandlerInterfaceRegistry::Instance().UnregisterCommandHandler(this);
43+
AttributeAccessInterfaceRegistry::Instance().Unregister(this);
5644
}
5745

58-
} // namespace
59-
60-
ActionsServer ActionsServer::sInstance;
61-
62-
void ActionsServer::SetDefaultDelegate(EndpointId aEndpoint, Delegate * aDelegate)
46+
CHIP_ERROR ActionsServer::Init()
6347
{
64-
if (aEndpoint < kActionsDelegateTableSize)
65-
{
66-
gDelegateTable[aEndpoint] = aDelegate;
67-
}
68-
}
48+
// Check if the cluster has been selected in zap
49+
VerifyOrDie(emberAfContainsServer(mEndpointId, Actions::Id) == true);
6950

70-
ActionsServer & ActionsServer::Instance()
71-
{
72-
return sInstance;
51+
ReturnErrorOnFailure(CommandHandlerInterfaceRegistry::Instance().RegisterCommandHandler(this));
52+
VerifyOrReturnError(AttributeAccessInterfaceRegistry::Instance().Register(this), CHIP_ERROR_INCORRECT_STATE);
53+
return CHIP_NO_ERROR;
7354
}
7455

7556
void ActionsServer::OnStateChanged(EndpointId aEndpoint, uint16_t aActionId, uint32_t aInvokeId, ActionStateEnum aActionState)
@@ -126,13 +107,10 @@ CHIP_ERROR ActionsServer::Read(const ConcreteReadAttributePath & aPath, Attribut
126107
CHIP_ERROR ActionsServer::ReadActionListAttribute(const ConcreteReadAttributePath & aPath,
127108
const AttributeValueEncoder::ListEncodeHelper & aEncoder)
128109
{
129-
Delegate * delegate = GetDelegate(aPath.mEndpointId);
130-
ReturnErrorOnFailure(ValidateDelegate(delegate, aPath.mEndpointId));
131-
132110
for (uint16_t i = 0; i < kMaxActionListLength; i++)
133111
{
134112
ActionStructStorage action;
135-
CHIP_ERROR err = delegate->ReadActionAtIndex(i, action);
113+
CHIP_ERROR err = mDelegate->ReadActionAtIndex(i, action);
136114

137115
if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED)
138116
{
@@ -147,14 +125,11 @@ CHIP_ERROR ActionsServer::ReadActionListAttribute(const ConcreteReadAttributePat
147125
CHIP_ERROR ActionsServer::ReadEndpointListAttribute(const ConcreteReadAttributePath & aPath,
148126
const AttributeValueEncoder::ListEncodeHelper & aEncoder)
149127
{
150-
Delegate * delegate = GetDelegate(aPath.mEndpointId);
151-
ReturnErrorOnFailure(ValidateDelegate(delegate, aPath.mEndpointId));
152-
153128
for (uint16_t i = 0; i < kMaxEndpointListLength; i++)
154129
{
155130
EndpointListStorage epList;
156131

157-
CHIP_ERROR err = delegate->ReadEndpointListAtIndex(i, epList);
132+
CHIP_ERROR err = mDelegate->ReadEndpointListAtIndex(i, epList);
158133
if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED)
159134
{
160135
return CHIP_NO_ERROR;
@@ -167,9 +142,7 @@ CHIP_ERROR ActionsServer::ReadEndpointListAttribute(const ConcreteReadAttributeP
167142

168143
bool ActionsServer::HaveActionWithId(EndpointId aEndpointId, uint16_t aActionId, uint16_t & aActionIndex)
169144
{
170-
Delegate * delegate = GetDelegate(aEndpointId);
171-
VerifyOrReturnValue(ValidateDelegate(delegate, aEndpointId) == CHIP_NO_ERROR, false);
172-
return delegate->HaveActionWithId(aActionId, aActionIndex);
145+
return mDelegate->HaveActionWithId(aActionId, aActionIndex);
173146
}
174147

175148
template <typename RequestT, typename FuncT>
@@ -201,10 +174,8 @@ void ActionsServer::HandleCommand(HandlerContext & handlerContext, FuncT func)
201174
}
202175
if (actionIndex != kMaxActionListLength)
203176
{
204-
Delegate * delegate = GetDelegate(handlerContext.mRequestPath.mEndpointId);
205-
ReturnOnFailure(ValidateDelegate(delegate, handlerContext.mRequestPath.mEndpointId));
206177
ActionStructStorage action;
207-
delegate->ReadActionAtIndex(actionIndex, action);
178+
mDelegate->ReadActionAtIndex(actionIndex, action);
208179
// Check if the command bit is set in the SupportedCommands of an ations.
209180
if (!(action.supportedCommands.Raw() & (1 << handlerContext.mRequestPath.mCommandId)))
210181
{
@@ -220,139 +191,90 @@ void ActionsServer::HandleCommand(HandlerContext & handlerContext, FuncT func)
220191

221192
void ActionsServer::HandleInstantAction(HandlerContext & ctx, const Commands::InstantAction::DecodableType & commandData)
222193
{
223-
Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId);
224-
Status status = Status::InvalidInState;
225-
if (delegate != nullptr)
226-
{
227-
status = delegate->HandleInstantAction(commandData.actionID, commandData.invokeID);
228-
}
194+
Status status = Status::InvalidInState;
195+
status = mDelegate->HandleInstantAction(commandData.actionID, commandData.invokeID);
229196
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
230197
}
231198

232199
void ActionsServer::HandleInstantActionWithTransition(HandlerContext & ctx,
233200
const Commands::InstantActionWithTransition::DecodableType & commandData)
234201
{
235-
Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId);
236-
Status status = Status::InvalidInState;
237-
if (delegate != nullptr)
238-
{
239-
status =
240-
delegate->HandleInstantActionWithTransition(commandData.actionID, commandData.transitionTime, commandData.invokeID);
241-
}
202+
Status status = Status::InvalidInState;
203+
status = mDelegate->HandleInstantActionWithTransition(commandData.actionID, commandData.transitionTime, commandData.invokeID);
242204
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
243205
}
244206

245207
void ActionsServer::HandleStartAction(HandlerContext & ctx, const Commands::StartAction::DecodableType & commandData)
246208
{
247-
Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId);
248-
Status status = Status::InvalidInState;
249-
if (delegate != nullptr)
250-
{
251-
status = delegate->HandleStartAction(commandData.actionID, commandData.invokeID);
252-
}
209+
Status status = Status::InvalidInState;
210+
status = mDelegate->HandleStartAction(commandData.actionID, commandData.invokeID);
253211
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
254212
}
255213

256214
void ActionsServer::HandleStartActionWithDuration(HandlerContext & ctx,
257215
const Commands::StartActionWithDuration::DecodableType & commandData)
258216
{
259-
Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId);
260-
Status status = Status::InvalidInState;
261-
if (delegate != nullptr)
262-
{
263-
status = delegate->HandleStartActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID);
264-
}
217+
Status status = Status::InvalidInState;
218+
status = mDelegate->HandleStartActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID);
265219
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
266220
}
267221

268222
void ActionsServer::HandleStopAction(HandlerContext & ctx, const Commands::StopAction::DecodableType & commandData)
269223
{
270-
Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId);
271-
Status status = Status::InvalidInState;
272-
if (delegate != nullptr)
273-
{
274-
status = delegate->HandleStopAction(commandData.actionID, commandData.invokeID);
275-
}
224+
Status status = Status::InvalidInState;
225+
status = mDelegate->HandleStopAction(commandData.actionID, commandData.invokeID);
276226
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
277227
}
278228

279229
void ActionsServer::HandlePauseAction(HandlerContext & ctx, const Commands::PauseAction::DecodableType & commandData)
280230
{
281-
Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId);
282-
Status status = Status::InvalidInState;
283-
if (delegate != nullptr)
284-
{
285-
status = delegate->HandlePauseAction(commandData.actionID, commandData.invokeID);
286-
}
231+
Status status = Status::InvalidInState;
232+
status = mDelegate->HandlePauseAction(commandData.actionID, commandData.invokeID);
287233
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
288234
}
289235

290236
void ActionsServer::HandlePauseActionWithDuration(HandlerContext & ctx,
291237
const Commands::PauseActionWithDuration::DecodableType & commandData)
292238
{
293-
Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId);
294-
Status status = Status::InvalidInState;
295-
if (delegate != nullptr)
296-
{
297-
status = delegate->HandlePauseActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID);
298-
}
239+
Status status = Status::InvalidInState;
240+
status = mDelegate->HandlePauseActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID);
299241
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
300242
}
301243

302244
void ActionsServer::HandleResumeAction(HandlerContext & ctx, const Commands::ResumeAction::DecodableType & commandData)
303245
{
304-
Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId);
305-
Status status = Status::InvalidInState;
306-
if (delegate != nullptr)
307-
{
308-
status = delegate->HandleResumeAction(commandData.actionID, commandData.invokeID);
309-
}
246+
Status status = Status::InvalidInState;
247+
status = mDelegate->HandleResumeAction(commandData.actionID, commandData.invokeID);
310248
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
311249
}
312250

313251
void ActionsServer::HandleEnableAction(HandlerContext & ctx, const Commands::EnableAction::DecodableType & commandData)
314252
{
315-
Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId);
316-
Status status = Status::InvalidInState;
317-
if (delegate != nullptr)
318-
{
319-
status = delegate->HandleEnableAction(commandData.actionID, commandData.invokeID);
320-
}
253+
Status status = Status::InvalidInState;
254+
status = mDelegate->HandleEnableAction(commandData.actionID, commandData.invokeID);
321255
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
322256
}
323257

324258
void ActionsServer::HandleEnableActionWithDuration(HandlerContext & ctx,
325259
const Commands::EnableActionWithDuration::DecodableType & commandData)
326260
{
327-
Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId);
328-
Status status = Status::InvalidInState;
329-
if (delegate != nullptr)
330-
{
331-
status = delegate->HandleEnableActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID);
332-
}
261+
Status status = Status::InvalidInState;
262+
status = mDelegate->HandleEnableActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID);
333263
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
334264
}
335265

336266
void ActionsServer::HandleDisableAction(HandlerContext & ctx, const Commands::DisableAction::DecodableType & commandData)
337267
{
338-
Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId);
339-
Status status = Status::InvalidInState;
340-
if (delegate != nullptr)
341-
{
342-
status = delegate->HandleDisableAction(commandData.actionID, commandData.invokeID);
343-
}
268+
Status status = Status::InvalidInState;
269+
status = mDelegate->HandleDisableAction(commandData.actionID, commandData.invokeID);
344270
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
345271
}
346272

347273
void ActionsServer::HandleDisableActionWithDuration(HandlerContext & ctx,
348274
const Commands::DisableActionWithDuration::DecodableType & commandData)
349275
{
350-
Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId);
351-
Status status = Status::InvalidInState;
352-
if (delegate != nullptr)
353-
{
354-
status = delegate->HandleDisableActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID);
355-
}
276+
Status status = Status::InvalidInState;
277+
status = mDelegate->HandleDisableActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID);
356278
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
357279
}
358280

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

429-
void MatterActionsPluginServerInitCallback()
430-
{
431-
AttributeAccessInterfaceRegistry::Instance().Register(&ActionsServer::Instance());
432-
CommandHandlerInterfaceRegistry::Instance().RegisterCommandHandler(&ActionsServer::Instance());
433-
}
351+
void MatterActionsPluginServerInitCallback() {}

src/app/clusters/actions-server/actions-server.h

+19-4
Original file line numberDiff line numberDiff line change
@@ -114,11 +114,24 @@ class ActionsServer : public AttributeAccessInterface, public CommandHandlerInte
114114
{
115115
public:
116116
// Register for the Actions cluster on all endpoints.
117-
ActionsServer() :
118-
AttributeAccessInterface(Optional<EndpointId>::Missing(), Actions::Id),
119-
CommandHandlerInterface(Optional<EndpointId>::Missing(), Actions::Id)
117+
ActionsServer(EndpointId aEndpointId, Delegate * aDelegate) :
118+
AttributeAccessInterface(Optional<EndpointId>(aEndpointId), Actions::Id),
119+
CommandHandlerInterface(Optional<EndpointId>(aEndpointId), Actions::Id), mDelegate(aDelegate), mEndpointId(aEndpointId)
120120
{}
121-
static ActionsServer & Instance();
121+
122+
~ActionsServer();
123+
124+
/**
125+
* Initialise the Actions server instance.
126+
* @return Returns an error if the given endpoint and cluster have not been enabled in zap, if the
127+
* AttributeAccessInterface or AttributeAccessInterface registration fails returns an error.
128+
*/
129+
CHIP_ERROR Init();
130+
131+
/**
132+
* Unregisters the CommandHandlerInterface and AttributeAccessInterface.
133+
*/
134+
void Shutdown();
122135

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

154167
private:
168+
Delegate * mDelegate;
169+
EndpointId mEndpointId;
155170
static ActionsServer sInstance;
156171
static constexpr size_t kMaxEndpointListLength = 256u;
157172
static constexpr size_t kMaxActionListLength = 256u;

0 commit comments

Comments
 (0)