Skip to content

Commit bacc207

Browse files
andy31415restyled-commits
andy31415
and
restyled-commits
authored
General commissioning dependency injection (project-chip#41833)
* Add a context go generalcommissioning cluster, so that everything is dependency injected. * Pass in optional attributes in the general commissioning constructor. * Remove extra include. Network commissioning cluster should not depend on singletons. * Cleanup unneeded changes. * More cleanups. * No gn check for header that is conditional. * Some code re-use in tests. * A bit more cleanup. * Code review comments. * Fix all devices build. * Restyled by clang-format --------- Co-authored-by: Restyled.io <[email protected]>
1 parent 1e4ad75 commit bacc207

File tree

6 files changed

+140
-74
lines changed

6 files changed

+140
-74
lines changed

‎examples/all-devices-app/all-devices-common/devices/root-node/RootNodeDevice.cpp‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,19 @@ CHIP_ERROR RootNodeDevice::Register(EndpointId endpointId, CodeDrivenDataModelPr
5050

5151
ReturnErrorOnFailure(provider.AddCluster(mBasicInformationCluster.Registration()));
5252

53-
mGeneralCommissioningCluster.Create();
53+
mGeneralCommissioningCluster.Create(
54+
GeneralCommissioningCluster::Context {
55+
.commissioningWindowManager = Server::GetInstance().GetCommissioningWindowManager(), //
56+
.configurationManager = DeviceLayer::ConfigurationMgr(), //
57+
.deviceControlServer = DeviceLayer::DeviceControlServer::DeviceControlSvr(), //
58+
.fabricTable = Server::GetInstance().GetFabricTable(), //
59+
.failsafeContext = Server::GetInstance().GetFailSafeContext(), //
60+
.platformManager = DeviceLayer::PlatformMgr(), //
61+
#if CHIP_CONFIG_TERMS_AND_CONDITIONS_REQUIRED
62+
.termsAndConditionsProvider = TermsAndConditionsManager::GetInstance(),
63+
#endif // CHIP_CONFIG_TERMS_AND_CONDITIONS_REQUIRED
64+
},
65+
GeneralCommissioningCluster::OptionalAttributes());
5466
ReturnErrorOnFailure(provider.AddCluster(mGeneralCommissioningCluster.Registration()));
5567

5668
mAdministratorCommissioningCluster.Create(endpointId, BitFlags<AdministratorCommissioning::Feature>{});

‎src/app/clusters/general-commissioning-server/CodegenIntegration.cpp‎

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,18 @@
1919

2020
#include <app/clusters/general-commissioning-server/general-commissioning-cluster.h>
2121
#include <app/server-cluster/ServerClusterInterfaceRegistry.h>
22+
#include <app/server/Server.h>
2223
#include <app/static-cluster-config/GeneralCommissioning.h>
2324
#include <data-model-providers/codegen/ClusterIntegration.h>
25+
#include <lib/core/DataModelTypes.h>
2426
#include <lib/support/CodeUtils.h>
27+
#include <platform/ConfigurationManager.h>
28+
#include <platform/DeviceControlServer.h>
29+
#include <platform/PlatformManager.h>
30+
31+
#if CHIP_CONFIG_TERMS_AND_CONDITIONS_REQUIRED
32+
#include <app/server/TermsAndConditionsManager.h> // nogncheck
33+
#endif
2534

2635
using namespace chip;
2736
using namespace chip::app;
@@ -46,24 +55,39 @@ static_assert((kGeneralCommissioningFixedClusterCount == 0) ||
4655
GeneralCommissioning::StaticApplicationConfig::kFixedClusterConfig[0].endpointNumber == kRootEndpointId),
4756
"General Commissioning cluster MUST be on endpoint 0");
4857

49-
RegisteredServerCluster<GeneralCommissioningCluster> gRegistration;
58+
LazyRegisteredServerCluster<GeneralCommissioningCluster> gServer;
5059

5160
class IntegrationDelegate : public CodegenClusterIntegration::Delegate
5261
{
5362
public:
5463
ServerClusterRegistration & CreateRegistration(EndpointId endpointId, unsigned clusterInstanceIndex,
5564
uint32_t optionalAttributeBits, uint32_t featureMap) override
5665
{
57-
// Configure optional attributes based on fetched bits
58-
gRegistration.Cluster().GetOptionalAttributes() = GeneralCommissioningCluster::OptionalAttributes(optionalAttributeBits);
5966

60-
return gRegistration.Registration();
67+
gServer.Create(
68+
GeneralCommissioningCluster::Context {
69+
.commissioningWindowManager = Server::GetInstance().GetCommissioningWindowManager(), //
70+
.configurationManager = DeviceLayer::ConfigurationMgr(), //
71+
.deviceControlServer = DeviceLayer::DeviceControlServer::DeviceControlSvr(), //
72+
.fabricTable = Server::GetInstance().GetFabricTable(), //
73+
.failsafeContext = Server::GetInstance().GetFailSafeContext(), //
74+
.platformManager = DeviceLayer::PlatformMgr(), //
75+
#if CHIP_CONFIG_TERMS_AND_CONDITIONS_REQUIRED
76+
.termsAndConditionsProvider = TermsAndConditionsManager::GetInstance(),
77+
#endif // CHIP_CONFIG_TERMS_AND_CONDITIONS_REQUIRED
78+
},
79+
GeneralCommissioningCluster::OptionalAttributes(optionalAttributeBits));
80+
81+
return gServer.Registration();
6182
}
6283

63-
ServerClusterInterface * FindRegistration(unsigned clusterInstanceIndex) override { return &gRegistration.Cluster(); }
84+
ServerClusterInterface * FindRegistration(unsigned clusterInstanceIndex) override
85+
{
86+
VerifyOrReturnValue(gServer.IsConstructed(), nullptr);
87+
return &gServer.Cluster();
88+
}
6489

65-
// Nothing to destroy: gRegistration is a global static that handles destruction
66-
void ReleaseRegistration(unsigned clusterInstanceIndex) override {}
90+
void ReleaseRegistration(unsigned clusterInstanceIndex) override { gServer.Destroy(); }
6791
};
6892

6993
} // namespace
@@ -72,24 +96,22 @@ namespace chip::app::Clusters::GeneralCommissioning {
7296

7397
GeneralCommissioningCluster * Instance()
7498
{
75-
// we ALWAYS return this for now, however in the future this may be instantiated
76-
// at runtime (i.e only after server is initialized.)
77-
return &gRegistration.Cluster();
99+
VerifyOrReturnValue(gServer.IsConstructed(), nullptr);
100+
return &gServer.Cluster();
78101
}
79102

80103
} // namespace chip::app::Clusters::GeneralCommissioning
81104

82105
void MatterGeneralCommissioningClusterInitCallback(EndpointId endpointId)
83106
{
84107
VerifyOrReturn(endpointId == kRootEndpointId);
85-
86108
IntegrationDelegate integrationDelegate;
87109

88110
// register a singleton server (root endpoint only)
89111
// Startup() will be called automatically by the registry when context is set
90112
CodegenClusterIntegration::RegisterServer(
91113
{
92-
.endpointId = endpointId,
114+
.endpointId = kRootEndpointId,
93115
.clusterId = GeneralCommissioning::Id,
94116
.fixedClusterInstanceCount = GeneralCommissioning::StaticApplicationConfig::kFixedClusterConfig.size(),
95117
.maxClusterInstanceCount = 1, // Cluster is a singleton on the root node and this is the only thing supported

‎src/app/clusters/general-commissioning-server/app_config_dependent_sources.cmake‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ TARGET_SOURCES(
1818
${APP_TARGET}
1919
PRIVATE
2020
"${CLUSTER_DIR}/CodegenIntegration.cpp"
21+
"${CLUSTER_DIR}/CodegenIntegration.h"
2122
)
2223

2324
# These are the things that BUILD.gn dependencies would pull

‎src/app/clusters/general-commissioning-server/general-commissioning-cluster.cpp‎

Lines changed: 34 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -26,16 +26,12 @@
2626
#include <clusters/GeneralCommissioning/Metadata.h>
2727
#include <cstdint>
2828
#include <optional>
29+
#include <platform/ConfigurationManager.h>
2930
#include <platform/DeviceControlServer.h>
3031
#include <platform/PlatformManager.h>
3132
#include <tracing/macros.h>
3233
#include <transport/SecureSession.h>
3334

34-
#if CHIP_CONFIG_TERMS_AND_CONDITIONS_REQUIRED
35-
#include <app/server/TermsAndConditionsManager.h> //nogncheck
36-
#include <app/server/TermsAndConditionsProvider.h> //nogncheck
37-
#endif
38-
3935
using namespace chip;
4036
using namespace chip::app;
4137
using namespace chip::app::Clusters;
@@ -57,10 +53,11 @@ namespace {
5753
} \
5854
} while (false)
5955

60-
CHIP_ERROR ReadIfSupported(CHIP_ERROR (ConfigurationManager::*getter)(uint8_t &), AttributeValueEncoder & aEncoder)
56+
CHIP_ERROR ReadIfSupported(ConfigurationManager & mgr, CHIP_ERROR (ConfigurationManager::*getter)(uint8_t &),
57+
AttributeValueEncoder & aEncoder)
6158
{
6259
uint8_t data = 0;
63-
CHIP_ERROR err = (DeviceLayer::ConfigurationMgr().*getter)(data);
60+
CHIP_ERROR err = (mgr.*getter)(data);
6461
if (err == CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE)
6562
{
6663
data = 0;
@@ -139,18 +136,20 @@ void NotifyTermsAndConditionsAttributeChangeIfRequired(const TermsAndConditionsS
139136
}
140137
#endif // CHIP_CONFIG_TERMS_AND_CONDITIONS_REQUIRED
141138

142-
void OnPlatformEventHandler(const DeviceLayer::ChipDeviceEvent * event, intptr_t self)
139+
void OnPlatformEventHandler(const DeviceLayer::ChipDeviceEvent * event, intptr_t self_ptr_arg)
143140
{
144141
if (event->Type == DeviceLayer::DeviceEventType::kFailSafeTimerExpired)
145142
{
143+
auto self = reinterpret_cast<GeneralCommissioningCluster *>(self_ptr_arg);
144+
146145
// Spec says to reset Breadcrumb attribute to 0.
147-
reinterpret_cast<GeneralCommissioningCluster *>(self)->SetBreadCrumb(0);
146+
self->SetBreadCrumb(0);
148147

149148
#if CHIP_CONFIG_TERMS_AND_CONDITIONS_REQUIRED
150149
if (event->FailSafeTimerExpired.updateTermsAndConditionsHasBeenInvoked)
151150
{
152151
// Clear terms and conditions acceptance on failsafe timer expiration
153-
TermsAndConditionsProvider & tcProvider = TermsAndConditionsManager::GetInstance();
152+
TermsAndConditionsProvider & tcProvider = self->ClusterContext().termsAndConditionsProvider;
154153
TermsAndConditionsState initialState, updatedState;
155154
VerifyOrReturn(CHIP_NO_ERROR == GetTermsAndConditionsAttributeState(tcProvider, initialState));
156155
VerifyOrReturn(CHIP_NO_ERROR == tcProvider.RevertAcceptance());
@@ -190,56 +189,44 @@ DataModel::ActionReturnStatus GeneralCommissioningCluster::ReadAttribute(const D
190189
return encoder.Encode(info);
191190
}
192191
case RegulatoryConfig::Id:
193-
return ReadIfSupported(&ConfigurationManager::GetRegulatoryLocation, encoder);
192+
return ReadIfSupported(mClusterContext.configurationManager, &ConfigurationManager::GetRegulatoryLocation, encoder);
194193
case LocationCapability::Id:
195-
return ReadIfSupported(&ConfigurationManager::GetLocationCapability, encoder);
194+
return ReadIfSupported(mClusterContext.configurationManager, &ConfigurationManager::GetLocationCapability, encoder);
196195
case SupportsConcurrentConnection::Id:
197196
return encoder.Encode(CHIP_DEVICE_CONFIG_SUPPORTS_CONCURRENT_CONNECTION != 0);
198197

199198
#if CHIP_CONFIG_TERMS_AND_CONDITIONS_REQUIRED
200199
case TCAcceptedVersion::Id: {
201-
TermsAndConditionsProvider & tcProvider = TermsAndConditionsManager::GetInstance();
202200
Optional<TermsAndConditions> outTermsAndConditions;
203-
204-
ReturnErrorOnFailure(tcProvider.GetAcceptance(outTermsAndConditions));
205-
201+
ReturnErrorOnFailure(mClusterContext.termsAndConditionsProvider.GetAcceptance(outTermsAndConditions));
206202
return encoder.Encode(outTermsAndConditions.ValueOr(TermsAndConditions(0, 0)).GetVersion());
207203
}
208204
case TCMinRequiredVersion::Id: {
209-
TermsAndConditionsProvider & tcProvider = TermsAndConditionsManager::GetInstance();
210205
Optional<TermsAndConditions> outTermsAndConditions;
211206

212-
ReturnErrorOnFailure(tcProvider.GetRequirements(outTermsAndConditions));
213-
207+
ReturnErrorOnFailure(mClusterContext.termsAndConditionsProvider.GetRequirements(outTermsAndConditions));
214208
return encoder.Encode(outTermsAndConditions.ValueOr(TermsAndConditions(0, 0)).GetVersion());
215209
}
216210
case TCAcknowledgements::Id: {
217-
TermsAndConditionsProvider & tcProvider = TermsAndConditionsManager::GetInstance();
218211
Optional<TermsAndConditions> outTermsAndConditions;
219212

220-
ReturnErrorOnFailure(tcProvider.GetAcceptance(outTermsAndConditions));
221-
213+
ReturnErrorOnFailure(mClusterContext.termsAndConditionsProvider.GetAcceptance(outTermsAndConditions));
222214
return encoder.Encode(outTermsAndConditions.ValueOr(TermsAndConditions(0, 0)).GetValue());
223215
}
224216
case TCAcknowledgementsRequired::Id: {
225-
TermsAndConditionsProvider & tcProvider = TermsAndConditionsManager::GetInstance();
226217
bool acknowledgementsRequired;
227-
228-
ReturnErrorOnFailure(tcProvider.GetAcknowledgementsRequired(acknowledgementsRequired));
229-
218+
ReturnErrorOnFailure(mClusterContext.termsAndConditionsProvider.GetAcknowledgementsRequired(acknowledgementsRequired));
230219
return encoder.Encode(acknowledgementsRequired);
231220
}
232221
case TCUpdateDeadline::Id: {
233-
TermsAndConditionsProvider & tcProvider = TermsAndConditionsManager::GetInstance();
234222
Optional<uint32_t> outUpdateAcceptanceDeadline;
223+
ReturnErrorOnFailure(mClusterContext.termsAndConditionsProvider.GetUpdateAcceptanceDeadline(outUpdateAcceptanceDeadline));
235224

236-
ReturnErrorOnFailure(tcProvider.GetUpdateAcceptanceDeadline(outUpdateAcceptanceDeadline));
237-
225+
// NOTE: encoding an optional as a Nullable (they are not fully compatible)
238226
if (!outUpdateAcceptanceDeadline.HasValue())
239227
{
240228
return encoder.EncodeNull();
241229
}
242-
243230
return encoder.Encode(outUpdateAcceptanceDeadline.Value());
244231
}
245232
#endif
@@ -372,12 +359,12 @@ void GeneralCommissioningCluster::OnFabricRemoved(const FabricTable & fabricTabl
372359
#if CHIP_CONFIG_TERMS_AND_CONDITIONS_REQUIRED
373360
// If the FabricIndex matches the last remaining entry in the Fabrics list, then the device SHALL delete all Matter
374361
// related data on the node which was created since it was commissioned.
375-
if (Server::GetInstance().GetFabricTable().FabricCount() == 0)
362+
if (fabricTable.FabricCount() == 0)
376363
{
377364
ChipLogProgress(Zcl, "general-commissioning-server: Last Fabric index 0x%x was removed",
378365
static_cast<unsigned>(fabricIndex));
379366

380-
TermsAndConditionsProvider & tcProvider = TermsAndConditionsManager::GetInstance();
367+
TermsAndConditionsProvider & tcProvider = mClusterContext.termsAndConditionsProvider;
381368
TermsAndConditionsState initialState, updatedState;
382369
VerifyOrReturn(CHIP_NO_ERROR == GetTermsAndConditionsAttributeState(tcProvider, initialState));
383370
VerifyOrReturn(CHIP_NO_ERROR == tcProvider.ResetAcceptance());
@@ -397,14 +384,14 @@ void GeneralCommissioningCluster::SetBreadCrumb(uint64_t value)
397384
CHIP_ERROR GeneralCommissioningCluster::Startup(ServerClusterContext & context)
398385
{
399386
ReturnErrorOnFailure(DefaultServerCluster::Startup(context));
400-
ReturnErrorOnFailure(PlatformMgrImpl().AddEventHandler(OnPlatformEventHandler, reinterpret_cast<intptr_t>(this)));
401-
return Server::GetInstance().GetFabricTable().AddFabricDelegate(this);
387+
ReturnErrorOnFailure(mClusterContext.platformManager.AddEventHandler(OnPlatformEventHandler, reinterpret_cast<intptr_t>(this)));
388+
return mClusterContext.fabricTable.AddFabricDelegate(this);
402389
}
403390

404391
void GeneralCommissioningCluster::Shutdown()
405392
{
406-
PlatformMgrImpl().RemoveEventHandler(OnPlatformEventHandler, reinterpret_cast<intptr_t>(this));
407-
Server::GetInstance().GetFabricTable().RemoveFabricDelegate(this);
393+
mClusterContext.platformManager.RemoveEventHandler(OnPlatformEventHandler, reinterpret_cast<intptr_t>(this));
394+
mClusterContext.fabricTable.RemoveFabricDelegate(this);
408395
DefaultServerCluster::Shutdown();
409396
}
410397

@@ -413,7 +400,7 @@ GeneralCommissioningCluster::HandleArmFailSafe(const DataModel::InvokeRequest &
413400
const GeneralCommissioning::Commands::ArmFailSafe::DecodableType & commandData)
414401
{
415402
MATTER_TRACE_SCOPE("ArmFailSafe", "GeneralCommissioning");
416-
auto & failSafeContext = Server::GetInstance().GetFailSafeContext();
403+
auto & failSafeContext = mClusterContext.failsafeContext;
417404
Commands::ArmFailSafeResponse::Type response;
418405

419406
ChipLogProgress(FailSafe, "GeneralCommissioning: Received ArmFailSafe (%us)",
@@ -434,8 +421,7 @@ GeneralCommissioningCluster::HandleArmFailSafe(const DataModel::InvokeRequest &
434421
{
435422
// We do not allow CASE connections to arm the failsafe for the first time while the commissioning window is open in order
436423
// to allow commissioners the opportunity to obtain this failsafe for the purpose of commissioning
437-
if (!failSafeContext.IsFailSafeArmed() &&
438-
Server::GetInstance().GetCommissioningWindowManager().IsCommissioningWindowOpen() &&
424+
if (!failSafeContext.IsFailSafeArmed() && mClusterContext.commissioningWindowManager.IsCommissioningWindowOpen() &&
439425
request.subjectDescriptor->authMode == Access::AuthMode::kCase)
440426
{
441427
response.errorCode = CommissioningErrorEnum::kBusyWithOtherAdmin;
@@ -471,9 +457,7 @@ GeneralCommissioningCluster::HandleCommissioningComplete(const DataModel::Invoke
471457
{
472458
MATTER_TRACE_SCOPE("CommissioningComplete", "GeneralCommissioning");
473459

474-
DeviceControlServer * devCtrl = &DeviceLayer::DeviceControlServer::DeviceControlSvr();
475-
auto & failSafe = Server::GetInstance().GetFailSafeContext();
476-
auto & fabricTable = Server::GetInstance().GetFabricTable();
460+
auto & failSafe = mClusterContext.failsafeContext;
477461

478462
ChipLogProgress(FailSafe, "GeneralCommissioning: Received CommissioningComplete");
479463

@@ -489,7 +473,7 @@ GeneralCommissioningCluster::HandleCommissioningComplete(const DataModel::Invoke
489473
}
490474

491475
#if CHIP_CONFIG_TERMS_AND_CONDITIONS_REQUIRED
492-
TermsAndConditionsProvider & tcProvider = TermsAndConditionsManager::GetInstance();
476+
TermsAndConditionsProvider & tcProvider = mClusterContext.termsAndConditionsProvider;
493477

494478
// Ensure required terms and conditions have been accepted, then attempt to commit
495479
Optional<TermsAndConditions> requiredTermsAndConditionsMaybe;
@@ -558,7 +542,7 @@ GeneralCommissioningCluster::HandleCommissioningComplete(const DataModel::Invoke
558542
// Handle NOC commands
559543
if (failSafe.NocCommandHasBeenInvoked())
560544
{
561-
err = fabricTable.CommitPendingFabricData();
545+
err = mClusterContext.fabricTable.CommitPendingFabricData();
562546
if (err != CHIP_NO_ERROR)
563547
{
564548
ChipLogError(FailSafe, "GeneralCommissioning: Failed to commit pending fabric data: %" CHIP_ERROR_FORMAT, err.Format());
@@ -573,7 +557,8 @@ GeneralCommissioningCluster::HandleCommissioningComplete(const DataModel::Invoke
573557

574558
// Disarm the fail-safe and notify the DeviceControlServer
575559
failSafe.DisarmFailSafe();
576-
err = devCtrl->PostCommissioningCompleteEvent(handle->AsSecureSession()->GetPeerNodeId(), handle->GetFabricIndex());
560+
err = mClusterContext.deviceControlServer.PostCommissioningCompleteEvent(handle->AsSecureSession()->GetPeerNodeId(),
561+
handle->GetFabricIndex());
577562
CheckSuccess(err, Failure);
578563

579564
SetBreadCrumb(0);
@@ -587,7 +572,6 @@ GeneralCommissioningCluster::HandleSetRegulatoryConfig(const DataModel::InvokeRe
587572
const Commands::SetRegulatoryConfig::DecodableType & commandData)
588573
{
589574
MATTER_TRACE_SCOPE("SetRegulatoryConfig", "GeneralCommissioning");
590-
DeviceControlServer * server = &DeviceLayer::DeviceControlServer::DeviceControlSvr();
591575
Commands::SetRegulatoryConfigResponse::Type response;
592576
auto & countryCode = commandData.countryCode;
593577

@@ -605,7 +589,7 @@ GeneralCommissioningCluster::HandleSetRegulatoryConfig(const DataModel::InvokeRe
605589
}
606590

607591
uint8_t locationCapability;
608-
if (ConfigurationMgr().GetLocationCapability(locationCapability) != CHIP_NO_ERROR)
592+
if (mClusterContext.configurationManager.GetLocationCapability(locationCapability) != CHIP_NO_ERROR)
609593
{
610594
return Protocols::InteractionModel::Status::Failure;
611595
}
@@ -621,7 +605,7 @@ GeneralCommissioningCluster::HandleSetRegulatoryConfig(const DataModel::InvokeRe
621605
return std::nullopt;
622606
}
623607

624-
CheckSuccess(server->SetRegulatoryConfig(location, countryCode), Failure);
608+
CheckSuccess(mClusterContext.deviceControlServer.SetRegulatoryConfig(location, countryCode), Failure);
625609
SetBreadCrumb(commandData.breadcrumb);
626610
response.errorCode = CommissioningErrorEnum::kOk;
627611
handler->AddResponse(request.path, response);
@@ -635,8 +619,8 @@ GeneralCommissioningCluster::HandleSetTCAcknowledgements(const DataModel::Invoke
635619
{
636620
MATTER_TRACE_SCOPE("SetTCAcknowledgements", "GeneralCommissioning");
637621

638-
auto & failSafeContext = Server::GetInstance().GetFailSafeContext();
639-
TermsAndConditionsProvider & tcProvider = TermsAndConditionsManager::GetInstance();
622+
auto & failSafeContext = mClusterContext.failsafeContext;
623+
TermsAndConditionsProvider & tcProvider = mClusterContext.termsAndConditionsProvider;
640624

641625
Optional<TermsAndConditions> requiredTermsAndConditionsMaybe;
642626
Optional<TermsAndConditions> previousAcceptedTermsAndConditionsMaybe;

0 commit comments

Comments
 (0)