Added write attribute access interface to prevent circular callbacks when changing speed

Author: lpbeliveau-silabs

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

@@ -24,6 +24,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/AttributeAccessInterfaceRegistry.h>
 #include <app/CommandHandler.h>
 #include <app/ConcreteCommandPath.h>
 #include <app/clusters/fan-control-server/fan-control-server.h>
@@ -50,6 +51,8 @@ static_assert(kFanControlDelegateTableSize <= kEmberInvalidEndpointIndex, "FanCo
 
 Delegate * gDelegateTable[kFanControlDelegateTableSize] = { nullptr };
 
+FanControlAttributeAccessInterface gFanControlAttributeAccess;
+
 } // anonymous namespace
 
 namespace chip {
@@ -413,33 +416,75 @@ 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
-            // 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)));
+            // 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;
+    }
+}
 
-            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)));
+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));
 
-            float speed            = speedSetting.Value();
-            Percent percentSetting = static_cast<Percent>(speed / speedMax * 100);
+        DataModel::Nullable<uint8_t> currentSpeedSetting;
+        status = SpeedSetting::Get(aPath.mEndpointId, currentSpeedSetting);
+        ReturnLogErrorOnFailure(StatusIB(status).ToChipError());
 
-            if (currentPercentSetting.IsNull() || percentSetting != currentPercentSetting.Value())
+        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);
+
+            if (SupportsMultiSpeed(aPath.mEndpointId))
             {
-                status = PercentSetting::Set(attributePath.mEndpointId, percentSetting);
-                VerifyOrReturn(Status::Success == status,
-                               ChipLogError(Zcl, "Failed to set PercentSetting with error: 0x%02x", to_underlying(status)));
+                // 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.IsNull() || percentSetting != currentPercentSetting.Value())
+                {
+                    status = PercentSetting::Set(aPath.mEndpointId, percentSetting);
+                    ReturnLogErrorOnFailure(StatusIB(status).ToChipError());
+                }
             }
         }
         break;
     }
     default:
         break;
     }
+    return CHIP_NO_ERROR;
 }
 
 bool emberAfFanControlClusterStepCallback(app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath,
@@ -477,3 +522,8 @@ bool emberAfFanControlClusterStepCallback(app::CommandHandler * commandObj, cons
     commandObj->AddStatus(commandPath, status);
     return true;
 }
+
+void MatterFanControlPluginServerInitCallback()
+{
+    AttributeAccessInterfaceRegistry::Instance().Register(&gFanControlAttributeAccess);
+}

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

@@ -19,13 +19,23 @@
 
 #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() : AttributeAccessInterface(Optional<EndpointId>(), 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,7 +107,6 @@ void MatterUnitLocalizationPluginServerInitCallback() {}
 void MatterProxyValidPluginServerInitCallback() {}
 void MatterProxyDiscoveryPluginServerInitCallback() {}
 void MatterProxyConfigurationPluginServerInitCallback() {}
-void MatterFanControlPluginServerInitCallback() {}
 void MatterActivatedCarbonFilterMonitoringPluginServerInitCallback() {}
 void MatterHepaFilterMonitoringPluginServerInitCallback() {}
 void MatterAirQualityPluginServerInitCallback() {}