From b20d8aedc44626fff49c51eb77fbfb4d7d2b6902 Mon Sep 17 00:00:00 2001 From: Sergio Soares Date: Tue, 12 Nov 2024 15:55:41 -0500 Subject: [PATCH] Revert "[SL-UP] Circular callback fix (#85) (#36406)" (#36475) This reverts commit 078bc30d3f9f5d46c989272b2a50b3f5e8eb2f55. --- .../all-clusters-common/src/fan-stub.cpp | 7 +- .../fan-control-server/fan-control-server.cpp | 77 ++++--------------- .../fan-control-server/fan-control-server.h | 10 --- src/app/util/util.cpp | 2 +- 4 files changed, 23 insertions(+), 73 deletions(-) diff --git a/examples/all-clusters-app/all-clusters-common/src/fan-stub.cpp b/examples/all-clusters-app/all-clusters-common/src/fan-stub.cpp index 06de80e2f94a06..e0fb68c95f647c 100644 --- a/examples/all-clusters-app/all-clusters-common/src/fan-stub.cpp +++ b/examples/all-clusters-app/all-clusters-common/src/fan-stub.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -33,11 +34,13 @@ using namespace chip::app::Clusters::FanControl::Attributes; using Protocols::InteractionModel::Status; namespace { -class FanControlManager : public FanControlAttributeAccessInterface, public Delegate +class FanControlManager : public AttributeAccessInterface, public Delegate { public: // Register for the FanControl cluster on all endpoints. - FanControlManager(EndpointId aEndpointId) : FanControlAttributeAccessInterface(aEndpointId), Delegate(aEndpointId) {} + FanControlManager(EndpointId aEndpointId) : + AttributeAccessInterface(Optional(aEndpointId), FanControl::Id), Delegate(aEndpointId) + {} CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; Status HandleStep(StepDirectionEnum aDirection, bool aWrap, bool aLowestOff) override; diff --git a/src/app/clusters/fan-control-server/fan-control-server.cpp b/src/app/clusters/fan-control-server/fan-control-server.cpp index 4e10a1b6c92aeb..46f4c6d7c0bb70 100644 --- a/src/app/clusters/fan-control-server/fan-control-server.cpp +++ b/src/app/clusters/fan-control-server/fan-control-server.cpp @@ -24,7 +24,6 @@ #include #include #include -#include #include #include #include @@ -390,7 +389,7 @@ void MatterFanControlClusterServerAttributeChangedCallback(const app::ConcreteAt // Plus 99 then integer divide by 100 instead of multiplying 0.01 to avoid floating point precision error uint8_t speedSetting = static_cast((speedMax * percent + 99) / 100); - if (currentSpeedSetting != speedSetting) + if (currentSpeedSetting.IsNull() || speedSetting != currentSpeedSetting.Value()) { status = SpeedSetting::Set(attributePath.mEndpointId, speedSetting); VerifyOrReturn(Status::Success == status, @@ -414,67 +413,26 @@ void MatterFanControlClusterServerAttributeChangedCallback(const app::ConcreteAt ChipLogError(Zcl, "Failed to set FanMode to off with error: 0x%02x", to_underlying(status))); } - // Adjust PercentSetting from a speed value change for SpeedSetting only when the SpeedSetting change was received - // on a write command, not when it was changed by the server or the app logic. This avoids circular logic such as - //(with a SpeedMax of 10): - // 1. Client sets the PercetSetting to 25% - // 2. Server sets the SpeedSetting to 3 through the server callbackm, which sets the PercentSetting to 30% - // 3. Server sets the PercentSetting to 30% through the server callback - } - break; - } - default: - break; - } -} - -CHIP_ERROR FanControlAttributeAccessInterface::Write(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder) -{ - Status status = Status::Success; - switch (aPath.mAttributeId) - { - case SpeedSetting::Id: { - DataModel::Nullable speedSetting; - ReturnErrorOnFailure(aDecoder.Decode(speedSetting)); + // Adjust PercentSetting from a speed value change for SpeedSetting + // percent = floor( speed/SpeedMax * 100 ) + uint8_t speedMax; + status = SpeedMax::Get(attributePath.mEndpointId, &speedMax); + VerifyOrReturn(Status::Success == status, + ChipLogError(Zcl, "Failed to get SpeedMax with error: 0x%02x", to_underlying(status))); - DataModel::Nullable currentSpeedSetting; - status = SpeedSetting::Get(aPath.mEndpointId, currentSpeedSetting); - ReturnLogErrorOnFailure(StatusIB(status).ToChipError()); + DataModel::Nullable currentPercentSetting; + status = PercentSetting::Get(attributePath.mEndpointId, currentPercentSetting); + VerifyOrReturn(Status::Success == status, + ChipLogError(Zcl, "Failed to get PercentSetting with error: 0x%02x", to_underlying(status))); - if (speedSetting != currentSpeedSetting) - { - status = SpeedSetting::Set(aPath.mEndpointId, speedSetting); - ReturnLogErrorOnFailure(StatusIB(status).ToChipError()); - // Skip the last step if we are writing NULL - VerifyOrReturnValue(!speedSetting.IsNull(), CHIP_NO_ERROR); + float speed = speedSetting.Value(); + Percent percentSetting = static_cast(speed / speedMax * 100); - if (SupportsMultiSpeed(aPath.mEndpointId)) + if (currentPercentSetting.IsNull() || percentSetting != currentPercentSetting.Value()) { - // If SpeedSetting is set to 0, the server SHALL set the FanMode attribute value to Off. - if (speedSetting.Value() == 0) - { - status = SetFanModeToOff(aPath.mEndpointId); - ReturnLogErrorOnFailure(StatusIB(status).ToChipError()); - } - - // Adjust PercentSetting from a speed value change for SpeedSetting - // percent = floor( speed/SpeedMax * 100 ) - uint8_t speedMax; - status = SpeedMax::Get(aPath.mEndpointId, &speedMax); - ReturnLogErrorOnFailure(StatusIB(status).ToChipError()); - - DataModel::Nullable currentPercentSetting; - status = PercentSetting::Get(aPath.mEndpointId, currentPercentSetting); - ReturnLogErrorOnFailure(StatusIB(status).ToChipError()); - - float speed = speedSetting.Value(); - Percent percentSetting = static_cast(speed / speedMax * 100); - - if (currentPercentSetting != percentSetting) - { - status = PercentSetting::Set(aPath.mEndpointId, percentSetting); - ReturnLogErrorOnFailure(StatusIB(status).ToChipError()); - } + status = PercentSetting::Set(attributePath.mEndpointId, percentSetting); + VerifyOrReturn(Status::Success == status, + ChipLogError(Zcl, "Failed to set PercentSetting with error: 0x%02x", to_underlying(status))); } } break; @@ -482,7 +440,6 @@ CHIP_ERROR FanControlAttributeAccessInterface::Write(const ConcreteDataAttribute default: break; } - return CHIP_NO_ERROR; } bool emberAfFanControlClusterStepCallback(app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath, diff --git a/src/app/clusters/fan-control-server/fan-control-server.h b/src/app/clusters/fan-control-server/fan-control-server.h index 99967492dd021b..5721e34b4eba90 100644 --- a/src/app/clusters/fan-control-server/fan-control-server.h +++ b/src/app/clusters/fan-control-server/fan-control-server.h @@ -19,7 +19,6 @@ #include "fan-control-delegate.h" #include -#include #include namespace chip { @@ -27,15 +26,6 @@ namespace app { namespace Clusters { namespace FanControl { -class FanControlAttributeAccessInterface : public AttributeAccessInterface -{ -public: - FanControlAttributeAccessInterface(EndpointId aEndpoint) : AttributeAccessInterface(Optional(aEndpoint), Id) {} - - CHIP_ERROR Write(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder) override; - CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override { return CHIP_NO_ERROR; } -}; - void SetDefaultDelegate(EndpointId aEndpoint, Delegate * aDelegate); Delegate * GetDelegate(EndpointId aEndpoint); diff --git a/src/app/util/util.cpp b/src/app/util/util.cpp index 3c6acab8a07cb3..3072e8aafd2a7f 100644 --- a/src/app/util/util.cpp +++ b/src/app/util/util.cpp @@ -107,10 +107,10 @@ void MatterUnitLocalizationPluginServerInitCallback() {} void MatterProxyValidPluginServerInitCallback() {} void MatterProxyDiscoveryPluginServerInitCallback() {} void MatterProxyConfigurationPluginServerInitCallback() {} +void MatterFanControlPluginServerInitCallback() {} void MatterActivatedCarbonFilterMonitoringPluginServerInitCallback() {} void MatterHepaFilterMonitoringPluginServerInitCallback() {} void MatterAirQualityPluginServerInitCallback() {} -void MatterFanControlPluginServerInitCallback() {} void MatterCarbonMonoxideConcentrationMeasurementPluginServerInitCallback() {} void MatterCarbonDioxideConcentrationMeasurementPluginServerInitCallback() {} void MatterFormaldehydeConcentrationMeasurementPluginServerInitCallback() {}