Skip to content

Commit 775c118

Browse files
bzbarsky-applenivi-apple
authored andcommitted
Fix door lock endpoint initialization. (project-chip#34270)
* Fix door lock endpoint initialization. A few issues here: * DoorLockServer::InitServer was resetting endpoint state for all endpoints when initializing any endpoint. * We had two separate arrays of per-endpoint data, one for the endpoint context, one for the delegate. The fix is to not touch the state of other endpoints when initializing an endpoint and to put all the per-endpoint state in one place. * Apply suggestions from code review Co-authored-by: Nivi Sarkar <55898241+nivi-apple@users.noreply.github.com> --------- Co-authored-by: Nivi Sarkar <55898241+nivi-apple@users.noreply.github.com>
1 parent 18870ad commit 775c118

File tree

2 files changed

+93
-50
lines changed

2 files changed

+93
-50
lines changed

src/app/clusters/door-lock-server/door-lock-server.cpp

+63-37
Original file line numberDiff line numberDiff line change
@@ -51,39 +51,6 @@ static constexpr uint8_t DOOR_LOCK_ALIRO_CREDENTIAL_SIZE = 65;
5151

5252
static constexpr uint32_t DOOR_LOCK_MAX_LOCK_TIMEOUT_SEC = MAX_INT32U_VALUE / MILLISECOND_TICKS_PER_SECOND;
5353

54-
static constexpr size_t kDoorLockDelegateTableSize =
55-
MATTER_DM_DOOR_LOCK_CLUSTER_SERVER_ENDPOINT_COUNT + CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT;
56-
57-
static_assert(kDoorLockDelegateTableSize <= kEmberInvalidEndpointIndex, "Door Lock Delegate table size error");
58-
59-
namespace chip {
60-
namespace app {
61-
namespace Clusters {
62-
namespace DoorLock {
63-
64-
Delegate * gDelegateTable[kDoorLockDelegateTableSize] = { nullptr };
65-
66-
Delegate * GetDelegate(EndpointId endpoint)
67-
{
68-
uint16_t ep = emberAfGetClusterServerEndpointIndex(endpoint, DoorLock::Id, MATTER_DM_DOOR_LOCK_CLUSTER_SERVER_ENDPOINT_COUNT);
69-
return (ep >= kDoorLockDelegateTableSize ? nullptr : gDelegateTable[ep]);
70-
}
71-
72-
void SetDefaultDelegate(EndpointId endpoint, Delegate * delegate)
73-
{
74-
uint16_t ep = emberAfGetClusterServerEndpointIndex(endpoint, DoorLock::Id, MATTER_DM_DOOR_LOCK_CLUSTER_SERVER_ENDPOINT_COUNT);
75-
// if endpoint is found
76-
if (ep < ArraySize(gDelegateTable))
77-
{
78-
gDelegateTable[ep] = delegate;
79-
}
80-
}
81-
82-
} // namespace DoorLock
83-
} // namespace Clusters
84-
} // namespace app
85-
} // namespace chip
86-
8754
DoorLockServer DoorLockServer::instance;
8855

8956
class DoorLockClusterFabricDelegate : public chip::FabricTable::Delegate
@@ -117,7 +84,18 @@ DoorLockServer & DoorLockServer::Instance()
11784
*
11885
* @param endpointId
11986
*/
120-
void DoorLockServer::InitServer(chip::EndpointId endpointId)
87+
void DoorLockServer::InitServer(EndpointId endpointId)
88+
{
89+
CHIP_ERROR err = InitEndpoint(endpointId);
90+
91+
// We have no way to communicate this error, so just log it.
92+
if (err != CHIP_NO_ERROR)
93+
{
94+
ChipLogError(Zcl, "Door Lock cluster initialization on endpoint %d failed: %" CHIP_ERROR_FORMAT, endpointId, err.Format());
95+
}
96+
}
97+
98+
CHIP_ERROR DoorLockServer::InitEndpoint(EndpointId endpointId, Delegate * delegate)
12199
{
122100
ChipLogProgress(Zcl, "Door Lock cluster initialized at endpoint #%u", endpointId);
123101

@@ -128,11 +106,48 @@ void DoorLockServer::InitServer(chip::EndpointId endpointId)
128106
}
129107
SetActuatorEnabled(endpointId, true);
130108

131-
for (auto & ep : mEndpointCtx)
109+
auto * endpointContext = getContext(endpointId);
110+
if (!endpointContext)
111+
{
112+
ChipLogError(Zcl, "Invalid endpoint %d for initializing lock server: no endpoint context available", endpointId);
113+
return CHIP_ERROR_INVALID_ARGUMENT;
114+
}
115+
116+
endpointContext->lockoutEndTimestamp = endpointContext->lockoutEndTimestamp.zero();
117+
endpointContext->wrongCodeEntryAttempts = 0;
118+
endpointContext->delegate = delegate;
119+
return CHIP_NO_ERROR;
120+
}
121+
122+
void DoorLockServer::ShutdownEndpoint(EndpointId endpointId)
123+
{
124+
auto * endpointContext = getContext(endpointId);
125+
if (!endpointContext)
132126
{
133-
ep.lockoutEndTimestamp = ep.lockoutEndTimestamp.zero();
134-
ep.wrongCodeEntryAttempts = 0;
127+
ChipLogError(Zcl, "Invalid endpoint %d for shutting down lock server: no endpoint context available", endpointId);
128+
return;
135129
}
130+
131+
endpointContext->delegate = nullptr;
132+
}
133+
134+
CHIP_ERROR DoorLockServer::SetDelegate(chip::EndpointId endpointId, chip::app::Clusters::DoorLock::Delegate * delegate)
135+
{
136+
if (!delegate)
137+
{
138+
ChipLogError(Zcl, "Trying to set a null DoorLock::Delegate on endpoint %d", endpointId);
139+
return CHIP_ERROR_INVALID_ARGUMENT;
140+
}
141+
142+
auto * endpointContext = getContext(endpointId);
143+
if (!endpointContext)
144+
{
145+
ChipLogError(Zcl, "Invalid endpoint %d for setting a delegate: no endpoint context available", endpointId);
146+
return CHIP_ERROR_INVALID_ARGUMENT;
147+
}
148+
149+
endpointContext->delegate = delegate;
150+
return CHIP_NO_ERROR;
136151
}
137152

138153
bool DoorLockServer::SetLockState(chip::EndpointId endpointId, DlLockState newLockState)
@@ -3445,6 +3460,17 @@ EmberAfDoorLockEndpointContext * DoorLockServer::getContext(chip::EndpointId end
34453460
return nullptr;
34463461
}
34473462

3463+
Delegate * DoorLockServer::GetDelegate(EndpointId endpointId)
3464+
{
3465+
auto * endpointContext = getContext(endpointId);
3466+
if (!endpointContext)
3467+
{
3468+
return nullptr;
3469+
}
3470+
3471+
return endpointContext->delegate;
3472+
}
3473+
34483474
bool DoorLockServer::HandleRemoteLockOperation(chip::app::CommandHandler * commandObj,
34493475
const chip::app::ConcreteCommandPath & commandPath, LockOperationTypeEnum opType,
34503476
RemoteLockOpHandler opHandler, const Optional<ByteSpan> & pinCode)

src/app/clusters/door-lock-server/door-lock-server.h

+30-13
Original file line numberDiff line numberDiff line change
@@ -86,20 +86,9 @@ struct EmberAfDoorLockEndpointContext
8686
{
8787
chip::System::Clock::Timestamp lockoutEndTimestamp;
8888
int wrongCodeEntryAttempts;
89+
chip::app::Clusters::DoorLock::Delegate * delegate = nullptr;
8990
};
9091

91-
namespace chip {
92-
namespace app {
93-
namespace Clusters {
94-
namespace DoorLock {
95-
96-
void SetDefaultDelegate(EndpointId endpoint, Delegate * delegate);
97-
98-
} // namespace DoorLock
99-
} // namespace Clusters
100-
} // namespace app
101-
} // namespace chip
102-
10392
/**
10493
* @brief Door Lock Server Plugin class.
10594
*/
@@ -112,7 +101,28 @@ class DoorLockServer : public chip::app::AttributeAccessInterface
112101
using Feature = chip::app::Clusters::DoorLock::Feature;
113102
using OnFabricRemovedCustomCallback = void (*)(chip::EndpointId endpointId, chip::FabricIndex fabricIndex);
114103

115-
void InitServer(chip::EndpointId endpointId);
104+
/**
105+
* Multiple InitEndpoint calls can happen for different endpoints. Calling
106+
* InitEndpoint twice for the same endpoint requires a ShutdownEndpoint call
107+
* for that endpoint in between.
108+
*
109+
* A DoorLock::Delegate is optional, but needs to be provided in either
110+
* InitEndpoint or in a separate SetDelegate call for Aliro features, and
111+
* possibly other new features, to work.
112+
*/
113+
CHIP_ERROR InitEndpoint(chip::EndpointId endpointId, chip::app::Clusters::DoorLock::Delegate * delegate = nullptr);
114+
115+
void ShutdownEndpoint(chip::EndpointId endpointId);
116+
117+
// InitServer is a deprecated alias for InitEndpoint with no delegate.
118+
void InitServer(chip::EndpointId endpointid);
119+
120+
/**
121+
* Delegate is not supposed to be null. Removing a delegate
122+
* should only happen when shutting down the door lock cluster on the
123+
* endpoint, via ShutdownEndpoint.
124+
*/
125+
CHIP_ERROR SetDelegate(chip::EndpointId endpointId, chip::app::Clusters::DoorLock::Delegate * delegate);
116126

117127
/**
118128
* Updates the LockState attribute with new value and sends LockOperation event.
@@ -488,6 +498,13 @@ class DoorLockServer : public chip::app::AttributeAccessInterface
488498
static void sendClusterResponse(chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath,
489499
chip::Protocols::InteractionModel::ClusterStatusCode status);
490500

501+
/**
502+
* Get the DoorLock::Delegate for the given endpoint, if any. Will return
503+
* null if there is no door lock server initialized on that endpoint or if
504+
* there is no delegate associated with the initialized server.
505+
*/
506+
chip::app::Clusters::DoorLock::Delegate * GetDelegate(chip::EndpointId endpointId);
507+
491508
/**
492509
* @brief Common handler for LockDoor, UnlockDoor, UnlockWithTimeout commands
493510
*

0 commit comments

Comments
 (0)