Skip to content

General commissioning dependency injection #41833

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.

Sign up for GitHub

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
+128 −74
Open

General commissioning dependency injection #41833

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d482d77
Add a context go generalcommissioning cluster, so that everything is
andy31415 Nov 6, 2025
cb90e37
Pass in optional attributes in the general commissioning constructor.
andy31415 Nov 6, 2025
41c9590
Remove extra include. Network commissioning cluster should not depend on
andy31415 Nov 6, 2025
83966d4
Cleanup unneeded changes.
andy31415 Nov 6, 2025
d70ee04
More cleanups.
andy31415 Nov 6, 2025
d2cc1e0
No gn check for header that is conditional.
andy31415 Nov 6, 2025
f2d79b0
Some code re-use in tests.
andy31415 Nov 6, 2025
4d885df
A bit more cleanup.
andy31415 Nov 6, 2025
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
46 changes: 34 additions & 12 deletions src/app/clusters/general-commissioning-server/CodegenIntegration.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,18 @@

#include <app/clusters/general-commissioning-server/general-commissioning-cluster.h>
#include <app/server-cluster/ServerClusterInterfaceRegistry.h>
#include <app/server/Server.h>
#include <app/static-cluster-config/GeneralCommissioning.h>
#include <data-model-providers/codegen/ClusterIntegration.h>
#include <lib/core/DataModelTypes.h>
#include <lib/support/CodeUtils.h>
#include <platform/ConfigurationManager.h>
#include <platform/DeviceControlServer.h>
#include <platform/PlatformManager.h>

#if CHIP_CONFIG_TERMS_AND_CONDITIONS_REQUIRED
#include <app/server/TermsAndConditionsManager.h> // nogncheck
#endif

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

RegisteredServerCluster<GeneralCommissioningCluster> gRegistration;
LazyRegisteredServerCluster<GeneralCommissioningCluster> gServer;

class IntegrationDelegate : public CodegenClusterIntegration::Delegate
{
public:
ServerClusterRegistration & CreateRegistration(EndpointId endpointId, unsigned clusterInstanceIndex,
uint32_t optionalAttributeBits, uint32_t featureMap) override
{
// Configure optional attributes based on fetched bits
gRegistration.Cluster().GetOptionalAttributes() = GeneralCommissioningCluster::OptionalAttributes(optionalAttributeBits);

return gRegistration.Registration();
gServer.Create(
GeneralCommissioningCluster::Context {
.commissioningWindowManager = Server::GetInstance().GetCommissioningWindowManager(), //
.configurationManager = DeviceLayer::ConfigurationMgr(), //
.deviceControlServer = DeviceLayer::DeviceControlServer::DeviceControlSvr(), //
.fabricTable = Server::GetInstance().GetFabricTable(), //
.failsafeContext = Server::GetInstance().GetFailSafeContext(), //
.platformManager = DeviceLayer::PlatformMgr(), //
#if CHIP_CONFIG_TERMS_AND_CONDITIONS_REQUIRED
.termsAndConditionsProvider = TermsAndConditionsManager::GetInstance(),
#endif // CHIP_CONFIG_TERMS_AND_CONDITIONS_REQUIRED
},
GeneralCommissioningCluster::OptionalAttributes(optionalAttributeBits));

return gServer.Registration();
}

ServerClusterInterface * FindRegistration(unsigned clusterInstanceIndex) override { return &gRegistration.Cluster(); }
ServerClusterInterface * FindRegistration(unsigned clusterInstanceIndex) override
{
VerifyOrReturnValue(gServer.IsConstructed(), nullptr);
return &gServer.Cluster();
}

// Nothing to destroy: gRegistration is a global static that handles destruction
void ReleaseRegistration(unsigned clusterInstanceIndex) override {}
void ReleaseRegistration(unsigned clusterInstanceIndex) override { gServer.Destroy(); }
};

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

GeneralCommissioningCluster * Instance()
{
// we ALWAYS return this for now, however in the future this may be instantiated
// at runtime (i.e only after server is initialized.)
return &gRegistration.Cluster();
VerifyOrReturnValue(gServer.IsConstructed(), nullptr);
return &gServer.Cluster();
}

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

void MatterGeneralCommissioningClusterInitCallback(EndpointId endpointId)
{
VerifyOrReturn(endpointId == kRootEndpointId);

IntegrationDelegate integrationDelegate;

// register a singleton server (root endpoint only)
// Startup() will be called automatically by the registry when context is set
CodegenClusterIntegration::RegisterServer(
{
.endpointId = endpointId,
.endpointId = kRootEndpointId,
.clusterId = GeneralCommissioning::Id,
.fixedClusterInstanceCount = GeneralCommissioning::StaticApplicationConfig::kFixedClusterConfig.size(),
.maxClusterInstanceCount = 1, // Cluster is a singleton on the root node and this is the only thing supported
Expand Down
1 change: 1 addition & 0 deletions src/app/clusters/general-commissioning-server/app_config_dependent_sources.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ TARGET_SOURCES(
${APP_TARGET}
PRIVATE
"${CLUSTER_DIR}/CodegenIntegration.cpp"
"${CLUSTER_DIR}/CodegenIntegration.h"
Copy link
Contributor

@bzbarsky-apple bzbarsky-apple Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No such file in this PR?

Copy link
Contributor Author

@andy31415 andy31415 Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was probably missed before and things just compile because headers will work. File does exist, but I did not change it:

https://github.com/project-chip/connectedhomeip/blob/master/src/app/clusters/general-commissioning-server/CodegenIntegration.h

)

# These are the things that BUILD.gn dependencies would pull
Expand Down
84 changes: 34 additions & 50 deletions src/app/clusters/general-commissioning-server/general-commissioning-cluster.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,12 @@
#include <clusters/GeneralCommissioning/Metadata.h>
#include <cstdint>
#include <optional>
#include <platform/ConfigurationManager.h>
#include <platform/DeviceControlServer.h>
#include <platform/PlatformManager.h>
#include <tracing/macros.h>
#include <transport/SecureSession.h>

#if CHIP_CONFIG_TERMS_AND_CONDITIONS_REQUIRED
#include <app/server/TermsAndConditionsManager.h> //nogncheck
#include <app/server/TermsAndConditionsProvider.h> //nogncheck
#endif

using namespace chip;
using namespace chip::app;
using namespace chip::app::Clusters;
Expand All @@ -57,10 +53,11 @@ namespace {
} \
} while (false)

CHIP_ERROR ReadIfSupported(CHIP_ERROR (ConfigurationManager::*getter)(uint8_t &), AttributeValueEncoder & aEncoder)
CHIP_ERROR ReadIfSupported(ConfigurationManager & mgr, CHIP_ERROR (ConfigurationManager::*getter)(uint8_t &),
AttributeValueEncoder & aEncoder)
{
uint8_t data = 0;
CHIP_ERROR err = (DeviceLayer::ConfigurationMgr().*getter)(data);
CHIP_ERROR err = (mgr.*getter)(data);
if (err == CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE)
{
data = 0;
Expand Down Expand Up @@ -139,18 +136,20 @@ void NotifyTermsAndConditionsAttributeChangeIfRequired(const TermsAndConditionsS
}
#endif // CHIP_CONFIG_TERMS_AND_CONDITIONS_REQUIRED

void OnPlatformEventHandler(const DeviceLayer::ChipDeviceEvent * event, intptr_t self)
void OnPlatformEventHandler(const DeviceLayer::ChipDeviceEvent * event, intptr_t arg)
{
if (event->Type == DeviceLayer::DeviceEventType::kFailSafeTimerExpired)
{
auto self = reinterpret_cast<GeneralCommissioningCluster *>(arg);

// Spec says to reset Breadcrumb attribute to 0.
reinterpret_cast<GeneralCommissioningCluster *>(self)->SetBreadCrumb(0);
self->SetBreadCrumb(0);

#if CHIP_CONFIG_TERMS_AND_CONDITIONS_REQUIRED
if (event->FailSafeTimerExpired.updateTermsAndConditionsHasBeenInvoked)
{
// Clear terms and conditions acceptance on failsafe timer expiration
TermsAndConditionsProvider & tcProvider = TermsAndConditionsManager::GetInstance();
TermsAndConditionsProvider & tcProvider = self->ClusterContext().termsAndConditionsProvider;
TermsAndConditionsState initialState, updatedState;
VerifyOrReturn(CHIP_NO_ERROR == GetTermsAndConditionsAttributeState(tcProvider, initialState));
VerifyOrReturn(CHIP_NO_ERROR == tcProvider.RevertAcceptance());
Expand Down Expand Up @@ -190,56 +189,44 @@ DataModel::ActionReturnStatus GeneralCommissioningCluster::ReadAttribute(const D
return encoder.Encode(info);
}
case RegulatoryConfig::Id:
return ReadIfSupported(&ConfigurationManager::GetRegulatoryLocation, encoder);
return ReadIfSupported(mClusterContext.configurationManager, &ConfigurationManager::GetRegulatoryLocation, encoder);
case LocationCapability::Id:
return ReadIfSupported(&ConfigurationManager::GetLocationCapability, encoder);
return ReadIfSupported(mClusterContext.configurationManager, &ConfigurationManager::GetLocationCapability, encoder);
case SupportsConcurrentConnection::Id:
return encoder.Encode(CHIP_DEVICE_CONFIG_SUPPORTS_CONCURRENT_CONNECTION != 0);

#if CHIP_CONFIG_TERMS_AND_CONDITIONS_REQUIRED
case TCAcceptedVersion::Id: {
TermsAndConditionsProvider & tcProvider = TermsAndConditionsManager::GetInstance();
Optional<TermsAndConditions> outTermsAndConditions;

ReturnErrorOnFailure(tcProvider.GetAcceptance(outTermsAndConditions));

ReturnErrorOnFailure(mClusterContext.termsAndConditionsProvider.GetAcceptance(outTermsAndConditions));
return encoder.Encode(outTermsAndConditions.ValueOr(TermsAndConditions(0, 0)).GetVersion());
}
case TCMinRequiredVersion::Id: {
TermsAndConditionsProvider & tcProvider = TermsAndConditionsManager::GetInstance();
Optional<TermsAndConditions> outTermsAndConditions;

ReturnErrorOnFailure(tcProvider.GetRequirements(outTermsAndConditions));

ReturnErrorOnFailure(mClusterContext.termsAndConditionsProvider.GetRequirements(outTermsAndConditions));
return encoder.Encode(outTermsAndConditions.ValueOr(TermsAndConditions(0, 0)).GetVersion());
}
case TCAcknowledgements::Id: {
TermsAndConditionsProvider & tcProvider = TermsAndConditionsManager::GetInstance();
Optional<TermsAndConditions> outTermsAndConditions;

ReturnErrorOnFailure(tcProvider.GetAcceptance(outTermsAndConditions));

ReturnErrorOnFailure(mClusterContext.termsAndConditionsProvider.GetAcceptance(outTermsAndConditions));
return encoder.Encode(outTermsAndConditions.ValueOr(TermsAndConditions(0, 0)).GetValue());
}
case TCAcknowledgementsRequired::Id: {
TermsAndConditionsProvider & tcProvider = TermsAndConditionsManager::GetInstance();
bool acknowledgementsRequired;

ReturnErrorOnFailure(tcProvider.GetAcknowledgementsRequired(acknowledgementsRequired));

ReturnErrorOnFailure(mClusterContext.termsAndConditionsProvider.GetAcknowledgementsRequired(acknowledgementsRequired));
return encoder.Encode(acknowledgementsRequired);
}
case TCUpdateDeadline::Id: {
TermsAndConditionsProvider & tcProvider = TermsAndConditionsManager::GetInstance();
Optional<uint32_t> outUpdateAcceptanceDeadline;
ReturnErrorOnFailure(mClusterContext.termsAndConditionsProvider.GetUpdateAcceptanceDeadline(outUpdateAcceptanceDeadline));

ReturnErrorOnFailure(tcProvider.GetUpdateAcceptanceDeadline(outUpdateAcceptanceDeadline));

// NOTE: encoding an optional as a Nullable (they are not fully compatible)
if (!outUpdateAcceptanceDeadline.HasValue())
{
return encoder.EncodeNull();
}

return encoder.Encode(outUpdateAcceptanceDeadline.Value());
}
#endif
Expand Down Expand Up @@ -372,12 +359,12 @@ void GeneralCommissioningCluster::OnFabricRemoved(const FabricTable & fabricTabl
#if CHIP_CONFIG_TERMS_AND_CONDITIONS_REQUIRED
// If the FabricIndex matches the last remaining entry in the Fabrics list, then the device SHALL delete all Matter
// related data on the node which was created since it was commissioned.
if (Server::GetInstance().GetFabricTable().FabricCount() == 0)
if (fabricTable.FabricCount() == 0)
{
ChipLogProgress(Zcl, "general-commissioning-server: Last Fabric index 0x%x was removed",
static_cast<unsigned>(fabricIndex));

TermsAndConditionsProvider & tcProvider = TermsAndConditionsManager::GetInstance();
TermsAndConditionsProvider & tcProvider = mClusterContext.termsAndConditionsProvider;
TermsAndConditionsState initialState, updatedState;
VerifyOrReturn(CHIP_NO_ERROR == GetTermsAndConditionsAttributeState(tcProvider, initialState));
VerifyOrReturn(CHIP_NO_ERROR == tcProvider.ResetAcceptance());
Expand All @@ -397,15 +384,15 @@ void GeneralCommissioningCluster::SetBreadCrumb(uint64_t value)
CHIP_ERROR GeneralCommissioningCluster::Startup(ServerClusterContext & context)
{
ReturnErrorOnFailure(DefaultServerCluster::Startup(context));
PlatformMgrImpl().AddEventHandler(OnPlatformEventHandler, reinterpret_cast<intptr_t>(this));
Server::GetInstance().GetFabricTable().AddFabricDelegate(this);
mClusterContext.platformManager.AddEventHandler(OnPlatformEventHandler, reinterpret_cast<intptr_t>(this));
mClusterContext.fabricTable.AddFabricDelegate(this);
return CHIP_NO_ERROR;
}

void GeneralCommissioningCluster::Shutdown()
{
PlatformMgrImpl().RemoveEventHandler(OnPlatformEventHandler, reinterpret_cast<intptr_t>(this));
Server::GetInstance().GetFabricTable().RemoveFabricDelegate(this);
mClusterContext.platformManager.RemoveEventHandler(OnPlatformEventHandler, reinterpret_cast<intptr_t>(this));
mClusterContext.fabricTable.RemoveFabricDelegate(this);
DefaultServerCluster::Shutdown();
}

Expand All @@ -414,7 +401,7 @@ GeneralCommissioningCluster::HandleArmFailSafe(const DataModel::InvokeRequest &
const GeneralCommissioning::Commands::ArmFailSafe::DecodableType & commandData)
{
MATTER_TRACE_SCOPE("ArmFailSafe", "GeneralCommissioning");
auto & failSafeContext = Server::GetInstance().GetFailSafeContext();
auto & failSafeContext = mClusterContext.failsafeContext;
Commands::ArmFailSafeResponse::Type response;

ChipLogProgress(FailSafe, "GeneralCommissioning: Received ArmFailSafe (%us)",
Expand All @@ -435,8 +422,7 @@ GeneralCommissioningCluster::HandleArmFailSafe(const DataModel::InvokeRequest &
{
// We do not allow CASE connections to arm the failsafe for the first time while the commissioning window is open in order
// to allow commissioners the opportunity to obtain this failsafe for the purpose of commissioning
if (!failSafeContext.IsFailSafeArmed() &&
Server::GetInstance().GetCommissioningWindowManager().IsCommissioningWindowOpen() &&
if (!failSafeContext.IsFailSafeArmed() && mClusterContext.commissioningWindowManager.IsCommissioningWindowOpen() &&
request.subjectDescriptor->authMode == Access::AuthMode::kCase)
{
response.errorCode = CommissioningErrorEnum::kBusyWithOtherAdmin;
Expand Down Expand Up @@ -472,9 +458,7 @@ GeneralCommissioningCluster::HandleCommissioningComplete(const DataModel::Invoke
{
MATTER_TRACE_SCOPE("CommissioningComplete", "GeneralCommissioning");

DeviceControlServer * devCtrl = &DeviceLayer::DeviceControlServer::DeviceControlSvr();
auto & failSafe = Server::GetInstance().GetFailSafeContext();
auto & fabricTable = Server::GetInstance().GetFabricTable();
auto & failSafe = mClusterContext.failsafeContext;

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

Expand All @@ -490,7 +474,7 @@ GeneralCommissioningCluster::HandleCommissioningComplete(const DataModel::Invoke
}

#if CHIP_CONFIG_TERMS_AND_CONDITIONS_REQUIRED
TermsAndConditionsProvider & tcProvider = TermsAndConditionsManager::GetInstance();
TermsAndConditionsProvider & tcProvider = mClusterContext.termsAndConditionsProvider;

// Ensure required terms and conditions have been accepted, then attempt to commit
Optional<TermsAndConditions> requiredTermsAndConditionsMaybe;
Expand Down Expand Up @@ -559,7 +543,7 @@ GeneralCommissioningCluster::HandleCommissioningComplete(const DataModel::Invoke
// Handle NOC commands
if (failSafe.NocCommandHasBeenInvoked())
{
err = fabricTable.CommitPendingFabricData();
err = mClusterContext.fabricTable.CommitPendingFabricData();
if (err != CHIP_NO_ERROR)
{
ChipLogError(FailSafe, "GeneralCommissioning: Failed to commit pending fabric data: %" CHIP_ERROR_FORMAT, err.Format());
Expand All @@ -574,7 +558,8 @@ GeneralCommissioningCluster::HandleCommissioningComplete(const DataModel::Invoke

// Disarm the fail-safe and notify the DeviceControlServer
failSafe.DisarmFailSafe();
err = devCtrl->PostCommissioningCompleteEvent(handle->AsSecureSession()->GetPeerNodeId(), handle->GetFabricIndex());
err = mClusterContext.deviceControlServer.PostCommissioningCompleteEvent(handle->AsSecureSession()->GetPeerNodeId(),
handle->GetFabricIndex());
CheckSuccess(err, Failure);

SetBreadCrumb(0);
Expand All @@ -588,7 +573,6 @@ GeneralCommissioningCluster::HandleSetRegulatoryConfig(const DataModel::InvokeRe
const Commands::SetRegulatoryConfig::DecodableType & commandData)
{
MATTER_TRACE_SCOPE("SetRegulatoryConfig", "GeneralCommissioning");
DeviceControlServer * server = &DeviceLayer::DeviceControlServer::DeviceControlSvr();
Commands::SetRegulatoryConfigResponse::Type response;
auto & countryCode = commandData.countryCode;

Expand All @@ -606,7 +590,7 @@ GeneralCommissioningCluster::HandleSetRegulatoryConfig(const DataModel::InvokeRe
}

uint8_t locationCapability;
if (ConfigurationMgr().GetLocationCapability(locationCapability) != CHIP_NO_ERROR)
if (mClusterContext.configurationManager.GetLocationCapability(locationCapability) != CHIP_NO_ERROR)
{
return Protocols::InteractionModel::Status::Failure;
}
Expand All @@ -622,7 +606,7 @@ GeneralCommissioningCluster::HandleSetRegulatoryConfig(const DataModel::InvokeRe
return std::nullopt;
}

CheckSuccess(server->SetRegulatoryConfig(location, countryCode), Failure);
CheckSuccess(mClusterContext.deviceControlServer.SetRegulatoryConfig(location, countryCode), Failure);
SetBreadCrumb(commandData.breadcrumb);
response.errorCode = CommissioningErrorEnum::kOk;
handler->AddResponse(request.path, response);
Expand All @@ -636,8 +620,8 @@ GeneralCommissioningCluster::HandleSetTCAcknowledgements(const DataModel::Invoke
{
MATTER_TRACE_SCOPE("SetTCAcknowledgements", "GeneralCommissioning");

auto & failSafeContext = Server::GetInstance().GetFailSafeContext();
TermsAndConditionsProvider & tcProvider = TermsAndConditionsManager::GetInstance();
auto & failSafeContext = mClusterContext.failsafeContext;
TermsAndConditionsProvider & tcProvider = mClusterContext.termsAndConditionsProvider;

Optional<TermsAndConditions> requiredTermsAndConditionsMaybe;
Optional<TermsAndConditions> previousAcceptedTermsAndConditionsMaybe;
Expand Down
Loading
Loading