diff --git a/examples/fabric-sync/admin/BridgeSubscription.cpp b/examples/fabric-sync/admin/BridgeSubscription.cpp index e7052d7df08a96..1df9371ff74dff 100644 --- a/examples/fabric-sync/admin/BridgeSubscription.cpp +++ b/examples/fabric-sync/admin/BridgeSubscription.cpp @@ -80,7 +80,7 @@ void BridgeSubscription::OnAttributeData(const ConcreteDataAttributePath & path, return; } - DeviceMgr().HandleAttributeData(path, *data); + DeviceManager::Instance().HandleAttributeData(path, *data); } void BridgeSubscription::OnEventData(const app::EventHeader & eventHeader, TLV::TLVReader * data, const app::StatusIB * status) @@ -101,7 +101,7 @@ void BridgeSubscription::OnEventData(const app::EventHeader & eventHeader, TLV:: return; } - DeviceMgr().HandleEventData(eventHeader, *data); + DeviceManager::Instance().HandleEventData(eventHeader, *data); } void BridgeSubscription::OnError(CHIP_ERROR error) diff --git a/examples/fabric-sync/admin/CommissionerControl.cpp b/examples/fabric-sync/admin/CommissionerControl.cpp index 45c7bc02da69a5..a0c24eb956d5bb 100644 --- a/examples/fabric-sync/admin/CommissionerControl.cpp +++ b/examples/fabric-sync/admin/CommissionerControl.cpp @@ -84,7 +84,7 @@ void CommissionerControl::OnResponse(app::CommandSender * client, const app::Con if (data != nullptr) { - DeviceMgr().HandleCommandResponse(path, *data); + DeviceManager::Instance().HandleCommandResponse(path, *data); } } diff --git a/examples/fabric-sync/admin/DeviceManager.cpp b/examples/fabric-sync/admin/DeviceManager.cpp index 9848d2f68d2550..6dfdad1213bc68 100644 --- a/examples/fabric-sync/admin/DeviceManager.cpp +++ b/examples/fabric-sync/admin/DeviceManager.cpp @@ -36,9 +36,6 @@ constexpr EndpointId kAggregatorEndpointId = 1; } // namespace -// Define the static member -DeviceManager DeviceManager::sInstance; - LinuxCommissionableDataProvider sCommissionableDataProvider; void DeviceManager::Init() diff --git a/examples/fabric-sync/admin/DeviceManager.h b/examples/fabric-sync/admin/DeviceManager.h index b92154c1e6e008..e114a62748549a 100644 --- a/examples/fabric-sync/admin/DeviceManager.h +++ b/examples/fabric-sync/admin/DeviceManager.h @@ -54,6 +54,12 @@ class DeviceManager public: DeviceManager() = default; + static DeviceManager & Instance() + { + static DeviceManager instance; + return instance; + } + void Init(); chip::NodeId GetNextAvailableNodeId(); @@ -170,8 +176,6 @@ class DeviceManager SyncedDevice * FindDeviceByNode(chip::NodeId nodeId); private: - friend DeviceManager & DeviceMgr(); - void RequestCommissioningApproval(); void HandleReadSupportedDeviceCategories(chip::TLV::TLVReader & data); @@ -184,8 +188,6 @@ class DeviceManager void HandleReverseOpenCommissioningWindow(chip::TLV::TLVReader & data); - static DeviceManager sInstance; - chip::NodeId mLastUsedNodeId = 0; // The Node ID of the remote bridge used for Fabric-Sync @@ -201,19 +203,4 @@ class DeviceManager FabricSyncGetter mFabricSyncGetter; }; -/** - * Returns the public interface of the DeviceManager singleton object. - * - * Applications should use this to access features of the DeviceManager - * object. - */ -inline DeviceManager & DeviceMgr() -{ - if (!DeviceManager::sInstance.mInitialized) - { - DeviceManager::sInstance.Init(); - } - return DeviceManager::sInstance; -} - } // namespace admin diff --git a/examples/fabric-sync/admin/DeviceSynchronization.cpp b/examples/fabric-sync/admin/DeviceSynchronization.cpp index 2f36eb9cb03f69..05a4fa9db8f398 100644 --- a/examples/fabric-sync/admin/DeviceSynchronization.cpp +++ b/examples/fabric-sync/admin/DeviceSynchronization.cpp @@ -148,7 +148,7 @@ void DeviceSynchronizer::OnDone(app::ReadClient * apReadClient) { ChipLogProgress(NotSpecified, "Synchronization complete for NodeId:" ChipLogFormatX64, ChipLogValueX64(mNodeId)); - if (mState == State::ReceivedResponse && !DeviceMgr().IsCurrentBridgeDevice(mNodeId)) + if (mState == State::ReceivedResponse && !DeviceManager::Instance().IsCurrentBridgeDevice(mNodeId)) { GetUniqueId(); if (mState == State::GettingUid) @@ -229,15 +229,15 @@ void DeviceSynchronizer::GetUniqueId() // If we have a UniqueId we can return leaving state in ReceivedResponse. VerifyOrReturn(!mCurrentDeviceData.uniqueId.has_value(), ChipLogDetail(NotSpecified, "We already have UniqueId")); - auto * device = DeviceMgr().FindDeviceByNode(mNodeId); + auto * device = DeviceManager::Instance().FindDeviceByNode(mNodeId); // If there is no associated remote Fabric Sync Aggregator there is no other place for us to try // getting the UniqueId from and can return leaving the state in ReceivedResponse. VerifyOrReturn(device, ChipLogDetail(NotSpecified, "No remote Fabric Sync Aggregator to get UniqueId from")); // Because device is not-null we expect IsFabricSyncReady to be true. IsFabricSyncReady indicates we have a // connection to the remote Fabric Sync Aggregator. - VerifyOrDie(DeviceMgr().IsFabricSyncReady()); - auto remoteBridgeNodeId = DeviceMgr().GetRemoteBridgeNodeId(); + VerifyOrDie(DeviceManager::Instance().IsFabricSyncReady()); + auto remoteBridgeNodeId = DeviceManager::Instance().GetRemoteBridgeNodeId(); EndpointId remoteEndpointIdOfInterest = device->GetEndpointId(); ChipLogDetail(NotSpecified, "Attempting to get UniqueId from remote Fabric Sync Aggregator"); diff --git a/examples/fabric-sync/admin/FabricAdmin.cpp b/examples/fabric-sync/admin/FabricAdmin.cpp index a012ac9d8c9a95..65b5a5a8e8fc30 100644 --- a/examples/fabric-sync/admin/FabricAdmin.cpp +++ b/examples/fabric-sync/admin/FabricAdmin.cpp @@ -65,13 +65,13 @@ FabricAdmin::CommissionRemoteBridge(Controller::CommissioningWindowPasscodeParam if (err == CHIP_NO_ERROR) { - NodeId nodeId = DeviceMgr().GetNextAvailableNodeId(); + NodeId nodeId = DeviceManager::Instance().GetNextAvailableNodeId(); // After responding with RequestCommissioningApproval to the node where the client initiated the // RequestCommissioningApproval, you need to wait for it to open a commissioning window on its bridge. usleep(kCommissionPrepareTimeMs * 1000); - DeviceMgr().PairRemoteDevice(nodeId, code.c_str()); + DeviceManager::Instance().PairRemoteDevice(nodeId, code.c_str()); } else { diff --git a/examples/fabric-sync/bridge/include/BridgedDeviceManager.h b/examples/fabric-sync/bridge/include/BridgedDeviceManager.h index be6c6a41db262d..67efe74addb038 100644 --- a/examples/fabric-sync/bridge/include/BridgedDeviceManager.h +++ b/examples/fabric-sync/bridge/include/BridgedDeviceManager.h @@ -30,6 +30,12 @@ class BridgedDeviceManager public: BridgedDeviceManager() = default; + static BridgedDeviceManager & Instance() + { + static BridgedDeviceManager instance; + return instance; + } + /** * @brief Initializes the BridgedDeviceManager. * @@ -111,29 +117,14 @@ class BridgedDeviceManager BridgedDevice * GetDeviceByUniqueId(const std::string & id); private: - friend BridgedDeviceManager & BridgeDeviceMgr(); - /** * Creates a new unique ID that is not used by any other mDevice */ std::string GenerateUniqueId(); - static BridgedDeviceManager sInstance; - chip::EndpointId mCurrentEndpointId; chip::EndpointId mFirstDynamicEndpointId; std::unique_ptr mDevices[CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT + 1]; }; -/** - * Returns the public interface of the BridgedDeviceManager singleton object. - * - * Applications should use this to access features of the BridgedDeviceManager - * object. - */ -inline BridgedDeviceManager & BridgeDeviceMgr() -{ - return BridgedDeviceManager::sInstance; -} - } // namespace bridge diff --git a/examples/fabric-sync/bridge/src/Bridge.cpp b/examples/fabric-sync/bridge/src/Bridge.cpp index 02bda529caec01..3d0a4af56ff064 100644 --- a/examples/fabric-sync/bridge/src/Bridge.cpp +++ b/examples/fabric-sync/bridge/src/Bridge.cpp @@ -81,7 +81,7 @@ void BridgedDeviceInformationCommandHandler::InvokeCommand(HandlerContext & hand return; } - BridgedDevice * device = BridgeDeviceMgr().GetDevice(endpointId); + BridgedDevice * device = BridgedDeviceManager::Instance().GetDevice(endpointId); if (device == nullptr || !device->IsIcd()) { handlerContext.mCommandHandler.AddStatus(handlerContext.mRequestPath, Status::Failure); @@ -125,7 +125,7 @@ CHIP_ERROR BridgeInit(FabricAdminDelegate * delegate) CommandHandlerInterfaceRegistry::Instance().RegisterCommandHandler(&gBridgedDeviceInformationCommandHandler); AttributeAccessInterfaceRegistry::Instance().Register(&gBridgedDeviceBasicInformationAttributes); - BridgeDeviceMgr().Init(); + BridgedDeviceManager::Instance().Init(); FabricBridge::Instance().SetDelegate(delegate); ReturnErrorOnFailure(gBridgedAdministratorCommissioning.Init()); ReturnErrorOnFailure(CommissionerControlInit(delegate)); diff --git a/examples/fabric-sync/bridge/src/BridgedAdministratorCommissioning.cpp b/examples/fabric-sync/bridge/src/BridgedAdministratorCommissioning.cpp index 3299df75a9039e..6cfb5a5caaedd7 100644 --- a/examples/fabric-sync/bridge/src/BridgedAdministratorCommissioning.cpp +++ b/examples/fabric-sync/bridge/src/BridgedAdministratorCommissioning.cpp @@ -45,7 +45,7 @@ CHIP_ERROR BridgedAdministratorCommissioning::Read(const ConcreteReadAttributePa { VerifyOrDie(aPath.mClusterId == Clusters::AdministratorCommissioning::Id); EndpointId endpointId = aPath.mEndpointId; - BridgedDevice * device = BridgeDeviceMgr().GetDevice(endpointId); + BridgedDevice * device = BridgedDeviceManager::Instance().GetDevice(endpointId); if (!device) { diff --git a/examples/fabric-sync/bridge/src/BridgedDeviceBasicInformationImpl.cpp b/examples/fabric-sync/bridge/src/BridgedDeviceBasicInformationImpl.cpp index d021671c8bf1bd..3a03d8f84a81a0 100644 --- a/examples/fabric-sync/bridge/src/BridgedDeviceBasicInformationImpl.cpp +++ b/examples/fabric-sync/bridge/src/BridgedDeviceBasicInformationImpl.cpp @@ -36,7 +36,7 @@ CHIP_ERROR BridgedDeviceBasicInformationImpl::Read(const ConcreteReadAttributePa // Registration is done for the bridged device basic information only VerifyOrDie(path.mClusterId == app::Clusters::BridgedDeviceBasicInformation::Id); - BridgedDevice * dev = BridgeDeviceMgr().GetDevice(path.mEndpointId); + BridgedDevice * dev = BridgedDeviceManager::Instance().GetDevice(path.mEndpointId); VerifyOrReturnError(dev != nullptr, CHIP_ERROR_NOT_FOUND); switch (path.mAttributeId) @@ -93,7 +93,7 @@ CHIP_ERROR BridgedDeviceBasicInformationImpl::Write(const ConcreteDataAttributeP { VerifyOrDie(path.mClusterId == app::Clusters::BridgedDeviceBasicInformation::Id); - BridgedDevice * dev = BridgeDeviceMgr().GetDevice(path.mEndpointId); + BridgedDevice * dev = BridgedDeviceManager::Instance().GetDevice(path.mEndpointId); VerifyOrReturnError(dev != nullptr, CHIP_ERROR_NOT_FOUND); if (!dev->IsReachable()) diff --git a/examples/fabric-sync/bridge/src/BridgedDeviceManager.cpp b/examples/fabric-sync/bridge/src/BridgedDeviceManager.cpp index 62d423dc9a3ec7..0ead453572aa4f 100644 --- a/examples/fabric-sync/bridge/src/BridgedDeviceManager.cpp +++ b/examples/fabric-sync/bridge/src/BridgedDeviceManager.cpp @@ -175,9 +175,6 @@ const EmberAfDeviceType sBridgedDeviceTypes[] = { { DEVICE_TYPE_BRIDGED_NODE, DE } // namespace -// Define the static member -BridgedDeviceManager BridgedDeviceManager::sInstance; - void BridgedDeviceManager::Init() { mFirstDynamicEndpointId = static_cast( diff --git a/examples/fabric-sync/bridge/src/FabricBridge.cpp b/examples/fabric-sync/bridge/src/FabricBridge.cpp index d1c27d2409262d..aa856ccf129ccc 100644 --- a/examples/fabric-sync/bridge/src/FabricBridge.cpp +++ b/examples/fabric-sync/bridge/src/FabricBridge.cpp @@ -103,7 +103,7 @@ CHIP_ERROR FabricBridge::AddSynchronizedDevice(const SynchronizedDevice & data) device->SetIcd(data.isIcd.value_or(false)); // Add the device to the bridge manager with a parent endpoint - auto result = BridgeDeviceMgr().AddDeviceEndpoint(std::move(device), /* parentEndpointId= */ 1); + auto result = BridgedDeviceManager::Instance().AddDeviceEndpoint(std::move(device), /* parentEndpointId= */ 1); if (!result.has_value()) { ChipLogError(NotSpecified, "Failed to add device with Id=[%d:0x" ChipLogFormatX64 "]", data.id.GetFabricIndex(), @@ -112,7 +112,7 @@ CHIP_ERROR FabricBridge::AddSynchronizedDevice(const SynchronizedDevice & data) } // Retrieve and verify the added device by ScopedNodeId - BridgedDevice * addedDevice = BridgeDeviceMgr().GetDeviceByScopedNodeId(data.id); + BridgedDevice * addedDevice = BridgedDeviceManager::Instance().GetDeviceByScopedNodeId(data.id); VerifyOrDie(addedDevice); ChipLogProgress(NotSpecified, "Added device with Id=[%d:0x" ChipLogFormatX64 "]", data.id.GetFabricIndex(), @@ -137,7 +137,7 @@ CHIP_ERROR FabricBridge::RemoveSynchronizedDevice(ScopedNodeId scopedNodeId) ChipLogProgress(NotSpecified, "Received RemoveSynchronizedDevice: Id=[%d:" ChipLogFormatX64 "]", scopedNodeId.GetFabricIndex(), ChipLogValueX64(scopedNodeId.GetNodeId())); - auto removedIdx = BridgeDeviceMgr().RemoveDeviceByScopedNodeId(scopedNodeId); + auto removedIdx = BridgedDeviceManager::Instance().RemoveDeviceByScopedNodeId(scopedNodeId); if (!removedIdx.has_value()) { ChipLogError(NotSpecified, "Failed to remove device with Id=[%d:0x" ChipLogFormatX64 "]", scopedNodeId.GetFabricIndex(), @@ -153,7 +153,7 @@ CHIP_ERROR FabricBridge::ActiveChanged(ScopedNodeId scopedNodeId, uint32_t promi ChipLogProgress(NotSpecified, "Received ActiveChanged: Id=[%d:" ChipLogFormatX64 "]", scopedNodeId.GetFabricIndex(), ChipLogValueX64(scopedNodeId.GetNodeId())); - auto * device = BridgeDeviceMgr().GetDeviceByScopedNodeId(scopedNodeId); + auto * device = BridgedDeviceManager::Instance().GetDeviceByScopedNodeId(scopedNodeId); if (device == nullptr) { ChipLogError(NotSpecified, "Could not find bridged device associated with Id=[%d:0x" ChipLogFormatX64 "]", @@ -170,7 +170,7 @@ CHIP_ERROR FabricBridge::AdminCommissioningAttributeChanged(const AdministratorC ChipLogProgress(NotSpecified, "Received CADMIN attribute change: Id=[%d:" ChipLogFormatX64 "]", data.id.GetFabricIndex(), ChipLogValueX64(data.id.GetNodeId())); - auto * device = BridgeDeviceMgr().GetDeviceByScopedNodeId(data.id); + auto * device = BridgedDeviceManager::Instance().GetDeviceByScopedNodeId(data.id); if (device == nullptr) { ChipLogError(NotSpecified, "Could not find bridged device associated with Id=[%d:0x" ChipLogFormatX64 "]", @@ -207,7 +207,7 @@ CHIP_ERROR FabricBridge::DeviceReachableChanged(ScopedNodeId scopedNodeId, bool ChipLogProgress(NotSpecified, "Received device reachable changed: Id=[%d:" ChipLogFormatX64 "]", scopedNodeId.GetFabricIndex(), ChipLogValueX64(scopedNodeId.GetNodeId())); - auto * device = BridgeDeviceMgr().GetDeviceByScopedNodeId(scopedNodeId); + auto * device = BridgedDeviceManager::Instance().GetDeviceByScopedNodeId(scopedNodeId); if (device == nullptr) { ChipLogError(NotSpecified, "Could not find bridged device associated with Id=[%d:0x" ChipLogFormatX64 "]", diff --git a/examples/fabric-sync/shell/AddBridgeCommand.cpp b/examples/fabric-sync/shell/AddBridgeCommand.cpp index 4b70998dccd273..e6b9dd060351e6 100644 --- a/examples/fabric-sync/shell/AddBridgeCommand.cpp +++ b/examples/fabric-sync/shell/AddBridgeCommand.cpp @@ -48,13 +48,13 @@ void AddBridgeCommand::OnCommissioningComplete(NodeId deviceId, CHIP_ERROR err) if (err == CHIP_NO_ERROR) { - admin::DeviceMgr().SetRemoteBridgeNodeId(mBridgeNodeId); + admin::DeviceManager::Instance().SetRemoteBridgeNodeId(mBridgeNodeId); ChipLogProgress(NotSpecified, "Successfully paired bridge device: NodeId: " ChipLogFormatX64, ChipLogValueX64(mBridgeNodeId)); - admin::DeviceMgr().UpdateLastUsedNodeId(mBridgeNodeId); - admin::DeviceMgr().SubscribeRemoteFabricBridge(); + admin::DeviceManager::Instance().UpdateLastUsedNodeId(mBridgeNodeId); + admin::DeviceManager::Instance().SubscribeRemoteFabricBridge(); // After successful commissioning of the Commissionee, initiate Reverse Commissioning // via the Commissioner Control Cluster. However, we must first verify that the @@ -62,7 +62,7 @@ void AddBridgeCommand::OnCommissioningComplete(NodeId deviceId, CHIP_ERROR err) // // Note: The Fabric-Admin MUST NOT send the RequestCommissioningApproval command // if the remote Fabric-Bridge lacks Fabric Synchronization support. - DeviceLayer::SystemLayer().ScheduleLambda([]() { admin::DeviceMgr().ReadSupportedDeviceCategories(); }); + DeviceLayer::SystemLayer().ScheduleLambda([]() { admin::DeviceManager::Instance().ReadSupportedDeviceCategories(); }); } else { @@ -75,7 +75,7 @@ void AddBridgeCommand::OnCommissioningComplete(NodeId deviceId, CHIP_ERROR err) CHIP_ERROR AddBridgeCommand::RunCommand() { - if (admin::DeviceMgr().IsFabricSyncReady()) + if (admin::DeviceManager::Instance().IsFabricSyncReady()) { // print to console fprintf(stderr, "Remote Fabric Bridge has already been configured.\n"); @@ -87,7 +87,7 @@ CHIP_ERROR AddBridgeCommand::RunCommand() ChipLogProgress(NotSpecified, "Running AddBridgeCommand with Node ID: %lu, PIN Code: %u, Address: %s, Port: %u", mBridgeNodeId, mSetupPINCode, mRemoteAddr, mRemotePort); - return admin::DeviceMgr().PairRemoteFabricBridge(mBridgeNodeId, mSetupPINCode, mRemoteAddr, mRemotePort); + return admin::DeviceManager::Instance().PairRemoteFabricBridge(mBridgeNodeId, mSetupPINCode, mRemoteAddr, mRemotePort); } } // namespace commands diff --git a/examples/fabric-sync/shell/AddDeviceCommand.cpp b/examples/fabric-sync/shell/AddDeviceCommand.cpp index a85de7695deb98..a79e4696caaf4e 100644 --- a/examples/fabric-sync/shell/AddDeviceCommand.cpp +++ b/examples/fabric-sync/shell/AddDeviceCommand.cpp @@ -51,7 +51,7 @@ void AddDeviceCommand::OnCommissioningComplete(NodeId deviceId, CHIP_ERROR err) { ChipLogProgress(NotSpecified, "Successfully paired device: NodeId: " ChipLogFormatX64, ChipLogValueX64(mNodeId)); - admin::DeviceMgr().UpdateLastUsedNodeId(mNodeId); + admin::DeviceManager::Instance().UpdateLastUsedNodeId(mNodeId); } else { @@ -64,7 +64,7 @@ void AddDeviceCommand::OnCommissioningComplete(NodeId deviceId, CHIP_ERROR err) CHIP_ERROR AddDeviceCommand::RunCommand() { - if (admin::DeviceMgr().IsCurrentBridgeDevice(mNodeId)) + if (admin::DeviceManager::Instance().IsCurrentBridgeDevice(mNodeId)) { // print to console fprintf(stderr, "The specified node ID has been reserved by the Fabric Bridge.\n"); @@ -76,7 +76,7 @@ CHIP_ERROR AddDeviceCommand::RunCommand() admin::PairingManager::Instance().SetPairingDelegate(this); - return admin::DeviceMgr().PairRemoteDevice(mNodeId, mSetupPINCode, mRemoteAddr, mRemotePort); + return admin::DeviceManager::Instance().PairRemoteDevice(mNodeId, mSetupPINCode, mRemoteAddr, mRemotePort); } } // namespace commands diff --git a/examples/fabric-sync/shell/RemoveBridgeCommand.cpp b/examples/fabric-sync/shell/RemoveBridgeCommand.cpp index 77bd76be33675a..9a33b0b11ba94e 100644 --- a/examples/fabric-sync/shell/RemoveBridgeCommand.cpp +++ b/examples/fabric-sync/shell/RemoveBridgeCommand.cpp @@ -35,7 +35,7 @@ void RemoveBridgeCommand::OnDeviceRemoved(NodeId deviceId, CHIP_ERROR err) if (err == CHIP_NO_ERROR) { - admin::DeviceMgr().SetRemoteBridgeNodeId(kUndefinedNodeId); + admin::DeviceManager::Instance().SetRemoteBridgeNodeId(kUndefinedNodeId); // print to console fprintf(stderr, "Successfully removed bridge device: NodeId: " ChipLogFormatX64 "\n", ChipLogValueX64(mBridgeNodeId)); @@ -51,7 +51,7 @@ void RemoveBridgeCommand::OnDeviceRemoved(NodeId deviceId, CHIP_ERROR err) CHIP_ERROR RemoveBridgeCommand::RunCommand() { - NodeId bridgeNodeId = admin::DeviceMgr().GetRemoteBridgeNodeId(); + NodeId bridgeNodeId = admin::DeviceManager::Instance().GetRemoteBridgeNodeId(); if (bridgeNodeId == kUndefinedNodeId) { @@ -66,7 +66,7 @@ CHIP_ERROR RemoveBridgeCommand::RunCommand() admin::PairingManager::Instance().SetPairingDelegate(this); - return admin::DeviceMgr().UnpairRemoteFabricBridge(); + return admin::DeviceManager::Instance().UnpairRemoteFabricBridge(); } } // namespace commands diff --git a/examples/fabric-sync/shell/RemoveDeviceCommand.cpp b/examples/fabric-sync/shell/RemoveDeviceCommand.cpp index 78440f854283aa..5e316658d5b303 100644 --- a/examples/fabric-sync/shell/RemoveDeviceCommand.cpp +++ b/examples/fabric-sync/shell/RemoveDeviceCommand.cpp @@ -52,7 +52,7 @@ void RemoveDeviceCommand::OnDeviceRemoved(NodeId deviceId, CHIP_ERROR err) CHIP_ERROR RemoveDeviceCommand::RunCommand() { - if (admin::DeviceMgr().IsCurrentBridgeDevice(mNodeId)) + if (admin::DeviceManager::Instance().IsCurrentBridgeDevice(mNodeId)) { // print to console fprintf(stderr, "The specified node ID has been reserved by the Fabric Bridge.\n"); @@ -63,7 +63,7 @@ CHIP_ERROR RemoveDeviceCommand::RunCommand() ChipLogProgress(NotSpecified, "Running RemoveDeviceCommand with Node ID: %lu", mNodeId); - return admin::DeviceMgr().UnpairRemoteDevice(mNodeId); + return admin::DeviceManager::Instance().UnpairRemoteDevice(mNodeId); } } // namespace commands diff --git a/src/app/clusters/thermostat-server/thermostat-server.cpp b/src/app/clusters/thermostat-server/thermostat-server.cpp index bda97c5cb1f22f..a1f0d27779571f 100644 --- a/src/app/clusters/thermostat-server/thermostat-server.cpp +++ b/src/app/clusters/thermostat-server/thermostat-server.cpp @@ -220,6 +220,292 @@ int16_t EnforceCoolingSetpointLimits(int16_t CoolingSetpoint, EndpointId endpoin return CoolingSetpoint; } +struct SetpointLimits +{ + bool autoSupported = false; + bool heatSupported = false; + bool coolSupported = false; + bool occupancySupported = false; + + int16_t absMinHeatSetpointLimit; + int16_t absMaxHeatSetpointLimit; + int16_t minHeatSetpointLimit; + int16_t maxHeatSetpointLimit; + int16_t absMinCoolSetpointLimit; + int16_t absMaxCoolSetpointLimit; + int16_t minCoolSetpointLimit; + int16_t maxCoolSetpointLimit; + int16_t deadBand = 0; + int16_t occupiedCoolingSetpoint; + int16_t occupiedHeatingSetpoint; + int16_t unoccupiedCoolingSetpoint; + int16_t unoccupiedHeatingSetpoint; +}; + +/** + * @brief Reads all the attributes for enforcing setpoint limits + * + * @param endpoint The endpoint for the server whose limits are being enforced + * @param setpointLimits The SetpointLimits to populate + * @return Success if the limits were read completely, otherwise an error code + */ +Status GetSetpointLimits(EndpointId endpoint, SetpointLimits & setpointLimits) +{ + uint32_t flags; + if (FeatureMap::Get(endpoint, &flags) != Status::Success) + { + ChipLogError(Zcl, "GetSetpointLimits: could not get feature flags"); + flags = FEATURE_MAP_DEFAULT; + } + + auto featureMap = BitMask(flags); + setpointLimits.autoSupported = featureMap.Has(Feature::kAutoMode); + setpointLimits.heatSupported = featureMap.Has(Feature::kHeating); + setpointLimits.coolSupported = featureMap.Has(Feature::kCooling); + setpointLimits.occupancySupported = featureMap.Has(Feature::kOccupancy); + + if (setpointLimits.autoSupported) + { + int8_t deadBand; + if (MinSetpointDeadBand::Get(endpoint, &deadBand) != Status::Success) + { + deadBand = kDefaultDeadBand; + } + setpointLimits.deadBand = static_cast(deadBand * 10); + } + + if (AbsMinCoolSetpointLimit::Get(endpoint, &setpointLimits.absMinCoolSetpointLimit) != Status::Success) + setpointLimits.absMinCoolSetpointLimit = kDefaultAbsMinCoolSetpointLimit; + + if (AbsMaxCoolSetpointLimit::Get(endpoint, &setpointLimits.absMaxCoolSetpointLimit) != Status::Success) + setpointLimits.absMaxCoolSetpointLimit = kDefaultAbsMaxCoolSetpointLimit; + + if (MinCoolSetpointLimit::Get(endpoint, &setpointLimits.minCoolSetpointLimit) != Status::Success) + setpointLimits.minCoolSetpointLimit = setpointLimits.absMinCoolSetpointLimit; + + if (MaxCoolSetpointLimit::Get(endpoint, &setpointLimits.maxCoolSetpointLimit) != Status::Success) + setpointLimits.maxCoolSetpointLimit = setpointLimits.absMaxCoolSetpointLimit; + + if (AbsMinHeatSetpointLimit::Get(endpoint, &setpointLimits.absMinHeatSetpointLimit) != Status::Success) + setpointLimits.absMinHeatSetpointLimit = kDefaultAbsMinHeatSetpointLimit; + + if (AbsMaxHeatSetpointLimit::Get(endpoint, &setpointLimits.absMaxHeatSetpointLimit) != Status::Success) + setpointLimits.absMaxHeatSetpointLimit = kDefaultAbsMaxHeatSetpointLimit; + + if (MinHeatSetpointLimit::Get(endpoint, &setpointLimits.minHeatSetpointLimit) != Status::Success) + setpointLimits.minHeatSetpointLimit = setpointLimits.absMinHeatSetpointLimit; + + if (MaxHeatSetpointLimit::Get(endpoint, &setpointLimits.maxHeatSetpointLimit) != Status::Success) + setpointLimits.maxHeatSetpointLimit = setpointLimits.absMaxHeatSetpointLimit; + + if (setpointLimits.coolSupported) + { + if (OccupiedCoolingSetpoint::Get(endpoint, &setpointLimits.occupiedCoolingSetpoint) != Status::Success) + { + // We're substituting the failure code here for backwards-compatibility reasons + ChipLogError(Zcl, "Error: Can not read Occupied Cooling Setpoint"); + return Status::Failure; + } + } + + if (setpointLimits.heatSupported) + { + if (OccupiedHeatingSetpoint::Get(endpoint, &setpointLimits.occupiedHeatingSetpoint) != Status::Success) + { + // We're substituting the failure code here for backwards-compatibility reasons + ChipLogError(Zcl, "Error: Can not read Occupied Heating Setpoint"); + return Status::Failure; + } + } + + if (setpointLimits.coolSupported && setpointLimits.occupancySupported) + { + if (UnoccupiedCoolingSetpoint::Get(endpoint, &setpointLimits.unoccupiedCoolingSetpoint) != Status::Success) + { + // We're substituting the failure code here for backwards-compatibility reasons + ChipLogError(Zcl, "Error: Can not read Unoccupied Cooling Setpoint"); + return Status::Failure; + } + } + + if (setpointLimits.heatSupported && setpointLimits.occupancySupported) + { + if (UnoccupiedHeatingSetpoint::Get(endpoint, &setpointLimits.unoccupiedHeatingSetpoint) != Status::Success) + { + // We're substituting the failure code here for backwards-compatibility reasons + ChipLogError(Zcl, "Error: Can not read Unoccupied Heating Setpoint"); + return Status::Failure; + } + } + return Status::Success; +} + +/** + * @brief Checks to see if it's possible to adjust the heating setpoint to preserve a given deadband + * if the cooling setpoint is changed + * + * @param autoSupported Whether or not the thermostat supports Auto mode + * @param newCoolingSetpoing The desired cooling setpoint + * @param minHeatingSetpoint The minimum allowed heating setpoint + * @param deadband The deadband to preserve + * @return Success if the deadband can be preserved, InvalidValue if it cannot + */ +Status CheckHeatingSetpointDeadband(bool autoSupported, int16_t newCoolingSetpoint, int16_t minHeatingSetpoint, int16_t deadband) +{ + if (!autoSupported) + { + return Status::Success; + } + int16_t maxValidHeatingSetpoint = static_cast(newCoolingSetpoint - deadband); + if (maxValidHeatingSetpoint < minHeatingSetpoint) + { + // If we need to adjust the heating setpoint to preserve the deadband, it will go below the min heat setpoint + return Status::InvalidValue; + } + // It's possible to adjust the heating setpoint, if needed + return Status::Success; +} + +/** + * @brief Checks to see if it's possible to adjust the cooling setpoint to preserve a given deadband + * if the heating setpoint is changed + * + * @param autoSupported Whether or not the thermostat supports Auto mode + * @param newHeatingSetpoint The desired heating setpoint + * @param maxCoolingSetpoint The maximum allowed cooling setpoint + * @param deadband The deadband to preserve + * @return Success if the deadband can be preserved, InvalidValue if it cannot + */ +Status CheckCoolingSetpointDeadband(bool autoSupported, int16_t newHeatingSetpoint, int16_t maxCoolingSetpoint, int16_t deadband) +{ + if (!autoSupported) + { + return Status::Success; + } + int16_t minValidCoolingSetpoint = static_cast(newHeatingSetpoint + deadband); + if (minValidCoolingSetpoint > maxCoolingSetpoint) + { + // If we need to adjust the cooling setpoint to preserve the deadband, it will go above the max cool setpoint + return Status::InvalidValue; + } + // It's possible to adjust the cooling setpoint, if needed + return Status::Success; +} + +typedef Status (*SetpointSetter)(EndpointId endpoint, int16_t value); + +/** + * @brief Attempts to ensure that a change to the heating setpoint maintains the deadband with the cooling setpoint + * by adjusting the cooling setpoint + * + * @param endpoint The endpoint on which the heating setpoint has been changed + * @param currentCoolingSetpoint The current cooling setpoint + * @param newHeatingSetpoint The newly adjusted heating setpoint + * @param maxCoolingSetpoint The maximum allowed cooling setpoint + * @param deadband The deadband to preserve + * @param setter A function for setting the cooling setpoint + */ +void EnsureCoolingSetpointDeadband(EndpointId endpoint, int16_t currentCoolingSetpoint, int16_t newHeatingSetpoint, + int16_t maxCoolingSetpoint, int16_t deadband, SetpointSetter setter) +{ + int16_t minValidCoolingSetpoint = static_cast(newHeatingSetpoint + deadband); + if (currentCoolingSetpoint >= minValidCoolingSetpoint) + { + // The current cooling setpoint doesn't violate the deadband + return; + } + if (minValidCoolingSetpoint > maxCoolingSetpoint) + { + // Adjusting the cool setpoint to preserve the deadband would violate the max cool setpoint + // This should have been caught in CheckCoolingSetpointDeadband, so log and exit + ChipLogError(Zcl, "Failed ensuring cooling setpoint deadband"); + return; + } + // Adjust the cool setpoint to preserve deadband + auto status = setter(endpoint, minValidCoolingSetpoint); + if (status != Status::Success) + { + ChipLogError(Zcl, "Error: EnsureCoolingSetpointDeadband failed!"); + } +} + +/** + * @brief Attempts to ensure that a change to the cooling setpoint maintains the deadband with the heating setpoint + * by adjusting the heating setpoint + * + * @param endpoint The endpoint on which the cooling setpoint has been changed + * @param currentHeatingSetpointThe current heating setpoint + * @param newCoolingSetpoint The newly adjusted cooling setpoint + * @param minHeatingSetpoint The minimum allowed cooling setpoint + * @param deadband The deadband to preserve + * @param setter A function for setting the heating setpoint + */ +void EnsureHeatingSetpointDeadband(EndpointId endpoint, int16_t currentHeatingSetpoint, int16_t newCoolingSetpoint, + int16_t minHeatingSetpoint, int16_t deadband, SetpointSetter setter) +{ + int16_t maxValidHeatingSetpoint = static_cast(newCoolingSetpoint - deadband); + if (currentHeatingSetpoint <= maxValidHeatingSetpoint) + { + // The current heating setpoint doesn't violate the deadband + return; + } + if (maxValidHeatingSetpoint < minHeatingSetpoint) + { + // Adjusting the heating setpoint to preserve the deadband would violate the min heating setpoint + // This should have been caught in CheckHeatingSetpointDeadband, so log and exit + ChipLogError(Zcl, "Failed ensuring heating setpoint deadband"); + return; + } + // Adjust the heating setpoint to preserve deadband + auto status = setter(endpoint, maxValidHeatingSetpoint); + if (status != Status::Success) + { + ChipLogError(Zcl, "Error: EnsureHeatingSetpointDeadband failed!"); + } +} + +/** + * @brief For thermostats that support auto, shift setpoints to maintain the current deadband + * Note: this assumes that the shift is possible; setpoint changes which prevent the deadband + * from being maintained due to the min/max limits for setpoints should be rejected by + * MatterThermostatClusterServerPreAttributeChangedCallback + * + * @param attributePath + */ +void EnsureDeadband(const ConcreteAttributePath & attributePath) +{ + auto endpoint = attributePath.mEndpointId; + SetpointLimits setpointLimits; + auto status = GetSetpointLimits(endpoint, setpointLimits); + if (status != Status::Success) + { + return; + } + if (!setpointLimits.autoSupported) + { + return; + } + switch (attributePath.mAttributeId) + { + case OccupiedHeatingSetpoint::Id: + EnsureCoolingSetpointDeadband(endpoint, setpointLimits.occupiedCoolingSetpoint, setpointLimits.occupiedHeatingSetpoint, + setpointLimits.maxCoolSetpointLimit, setpointLimits.deadBand, OccupiedCoolingSetpoint::Set); + break; + case OccupiedCoolingSetpoint::Id: + EnsureHeatingSetpointDeadband(endpoint, setpointLimits.occupiedHeatingSetpoint, setpointLimits.occupiedCoolingSetpoint, + setpointLimits.minHeatSetpointLimit, setpointLimits.deadBand, OccupiedHeatingSetpoint::Set); + break; + case UnoccupiedHeatingSetpoint::Id: + EnsureCoolingSetpointDeadband(endpoint, setpointLimits.unoccupiedCoolingSetpoint, setpointLimits.unoccupiedHeatingSetpoint, + setpointLimits.maxCoolSetpointLimit, setpointLimits.deadBand, UnoccupiedCoolingSetpoint::Set); + break; + case UnoccupiedCoolingSetpoint::Id: + EnsureHeatingSetpointDeadband(endpoint, setpointLimits.unoccupiedHeatingSetpoint, setpointLimits.unoccupiedCoolingSetpoint, + setpointLimits.minHeatSetpointLimit, setpointLimits.deadBand, UnoccupiedHeatingSetpoint::Set); + break; + } +} + Delegate * GetDelegate(EndpointId endpoint) { uint16_t ep = @@ -477,14 +763,9 @@ void MatterThermostatClusterServerAttributeChangedCallback(const ConcreteAttribu return; } - auto featureMap = BitMask(flags); - if (!featureMap.Has(Feature::kPresets)) - { - // This server does not support presets, so nothing to do - return; - } - - bool occupied = true; + auto featureMap = BitMask(flags); + bool supportsPresets = featureMap.Has(Feature::kPresets); + bool occupied = true; if (featureMap.Has(Feature::kOccupancy)) { BitMask occupancy; @@ -499,19 +780,20 @@ void MatterThermostatClusterServerAttributeChangedCallback(const ConcreteAttribu { case OccupiedHeatingSetpoint::Id: case OccupiedCoolingSetpoint::Id: - clearActivePreset = occupied; + clearActivePreset = supportsPresets && occupied; + EnsureDeadband(attributePath); break; case UnoccupiedHeatingSetpoint::Id: case UnoccupiedCoolingSetpoint::Id: - clearActivePreset = !occupied; + clearActivePreset = supportsPresets && !occupied; + EnsureDeadband(attributePath); break; } - if (!clearActivePreset) + if (clearActivePreset) { - return; + ChipLogProgress(Zcl, "Setting active preset to null"); + gThermostatAttrAccess.SetActivePreset(attributePath.mEndpointId, std::nullopt); } - ChipLogProgress(Zcl, "Setting active preset to null"); - gThermostatAttrAccess.SetActivePreset(attributePath.mEndpointId, std::nullopt); } } // namespace Thermostat @@ -535,15 +817,15 @@ void emberAfThermostatClusterServerInitCallback(chip::EndpointId endpoint) // or should this just be the responsibility of the thermostat application? } -Protocols::InteractionModel::Status -MatterThermostatClusterServerPreAttributeChangedCallback(const app::ConcreteAttributePath & attributePath, - EmberAfAttributeType attributeType, uint16_t size, uint8_t * value) +Status MatterThermostatClusterServerPreAttributeChangedCallback(const app::ConcreteAttributePath & attributePath, + EmberAfAttributeType attributeType, uint16_t size, uint8_t * value) { EndpointId endpoint = attributePath.mEndpointId; int16_t requested; // Limits will be needed for all checks // so we just get them all now + // TODO: use GetSetpointLimits to fetch this information int16_t AbsMinHeatSetpointLimit; int16_t AbsMaxHeatSetpointLimit; int16_t MinHeatSetpointLimit; @@ -649,12 +931,8 @@ MatterThermostatClusterServerPreAttributeChangedCallback(const app::ConcreteAttr if (requested < AbsMinHeatSetpointLimit || requested < MinHeatSetpointLimit || requested > AbsMaxHeatSetpointLimit || requested > MaxHeatSetpointLimit) return Status::InvalidValue; - if (AutoSupported) - { - if (requested > OccupiedCoolingSetpoint - DeadBandTemp) - return Status::InvalidValue; - } - return Status::Success; + return CheckCoolingSetpointDeadband(AutoSupported, requested, std::min(MaxCoolSetpointLimit, AbsMaxCoolSetpointLimit), + DeadBandTemp); } case OccupiedCoolingSetpoint::Id: { @@ -664,12 +942,8 @@ MatterThermostatClusterServerPreAttributeChangedCallback(const app::ConcreteAttr if (requested < AbsMinCoolSetpointLimit || requested < MinCoolSetpointLimit || requested > AbsMaxCoolSetpointLimit || requested > MaxCoolSetpointLimit) return Status::InvalidValue; - if (AutoSupported) - { - if (requested < OccupiedHeatingSetpoint + DeadBandTemp) - return Status::InvalidValue; - } - return Status::Success; + return CheckHeatingSetpointDeadband(AutoSupported, requested, std::max(MinHeatSetpointLimit, AbsMinHeatSetpointLimit), + DeadBandTemp); } case UnoccupiedHeatingSetpoint::Id: { @@ -679,12 +953,8 @@ MatterThermostatClusterServerPreAttributeChangedCallback(const app::ConcreteAttr if (requested < AbsMinHeatSetpointLimit || requested < MinHeatSetpointLimit || requested > AbsMaxHeatSetpointLimit || requested > MaxHeatSetpointLimit) return Status::InvalidValue; - if (AutoSupported) - { - if (requested > UnoccupiedCoolingSetpoint - DeadBandTemp) - return Status::InvalidValue; - } - return Status::Success; + return CheckCoolingSetpointDeadband(AutoSupported, requested, std::min(MaxCoolSetpointLimit, AbsMaxCoolSetpointLimit), + DeadBandTemp); } case UnoccupiedCoolingSetpoint::Id: { requested = static_cast(chip::Encoding::LittleEndian::Get16(value)); @@ -693,12 +963,8 @@ MatterThermostatClusterServerPreAttributeChangedCallback(const app::ConcreteAttr if (requested < AbsMinCoolSetpointLimit || requested < MinCoolSetpointLimit || requested > AbsMaxCoolSetpointLimit || requested > MaxCoolSetpointLimit) return Status::InvalidValue; - if (AutoSupported) - { - if (requested < UnoccupiedHeatingSetpoint + DeadBandTemp) - return Status::InvalidValue; - } - return Status::Success; + return CheckHeatingSetpointDeadband(AutoSupported, requested, std::max(MinHeatSetpointLimit, AbsMinHeatSetpointLimit), + DeadBandTemp); } case MinHeatSetpointLimit::Id: {