Skip to content
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

Revert "[Fan Control]Circular callback fix" #36475

Merged
merged 1 commit into from
Nov 12, 2024
Merged

Revert "[Fan Control]Circular callback fix" #36475

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 5 additions & 2 deletions examples/all-clusters-app/all-clusters-common/src/fan-stub.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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;
Expand Down
77 changes: 17 additions & 60 deletions src/app/clusters/fan-control-server/fan-control-server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
10 changes: 0 additions & 10 deletions src/app/clusters/fan-control-server/fan-control-server.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion src/app/util/util.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
Expand Down
Loading