Revert "[SL-UP] Circular callback fix (#85) (project-chip#36406)" (pr…

…oject-chip#36475)

This reverts commit 078bc30.

examples/all-clusters-app/all-clusters-common/src/fan-stub.cpp

@@ -19,6 +19,7 @@
 #include <app-common/zap-generated/cluster-objects.h>
 #include <app-common/zap-generated/ids/Attributes.h>
 #include <app-common/zap-generated/ids/Clusters.h>
+#include <app/AttributeAccessInterface.h>
 #include <app/AttributeAccessInterfaceRegistry.h>
 #include <app/clusters/fan-control-server/fan-control-server.h>
 #include <app/util/attribute-storage.h>
@@ -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<EndpointId>(aEndpointId), FanControl::Id), Delegate(aEndpointId)
+    {}
 
     CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override;
     Status HandleStep(StepDirectionEnum aDirection, bool aWrap, bool aLowestOff) override;

src/app/clusters/fan-control-server/fan-control-server.cpp

@@ -24,7 +24,6 @@
 #include <app-common/zap-generated/cluster-objects.h>
 #include <app-common/zap-generated/ids/Attributes.h>
 #include <app-common/zap-generated/ids/Clusters.h>
-#include <app/AttributeAccessInterfaceRegistry.h>
 #include <app/CommandHandler.h>
 #include <app/ConcreteCommandPath.h>
 #include <app/clusters/fan-control-server/fan-control-server.h>
@@ -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<uint8_t>((speedMax * percent + 99) / 100);
 
-            if (currentSpeedSetting != speedSetting)
+            if (currentSpeedSetting.IsNull() || speedSetting != currentSpeedSetting.Value())
             {
                 status = SpeedSetting::Set(attributePath.mEndpointId, speedSetting);
                 VerifyOrReturn(Status::Success == status,
@@ -414,75 +413,33 @@ 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<uint8_t> 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<uint8_t> currentSpeedSetting;
-        status = SpeedSetting::Get(aPath.mEndpointId, currentSpeedSetting);
-        ReturnLogErrorOnFailure(StatusIB(status).ToChipError());
+            DataModel::Nullable<Percent> 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<Percent>(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<Percent> currentPercentSetting;
-                status = PercentSetting::Get(aPath.mEndpointId, currentPercentSetting);
-                ReturnLogErrorOnFailure(StatusIB(status).ToChipError());
-
-                float speed            = speedSetting.Value();
-                Percent percentSetting = static_cast<Percent>(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;
     }
     default:
         break;
     }
-    return CHIP_NO_ERROR;
 }
 
 bool emberAfFanControlClusterStepCallback(app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath,

src/app/clusters/fan-control-server/fan-control-server.h

@@ -19,23 +19,13 @@
 
 #include "fan-control-delegate.h"
 #include <app-common/zap-generated/cluster-objects.h>
-#include <app/AttributeAccessInterface.h>
 #include <app/util/af-types.h>
 
 namespace chip {
 namespace app {
 namespace Clusters {
 namespace FanControl {
 
-class FanControlAttributeAccessInterface : public AttributeAccessInterface
-{
-public:
-    FanControlAttributeAccessInterface(EndpointId aEndpoint) : AttributeAccessInterface(Optional<EndpointId>(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);
 

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() {}