Addressing some review concerns

project-chip · Jan 25, 2024 · e4bb1c7 · e4bb1c7
1 parent bfc06e1
commit e4bb1c7
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 21 deletions.
diff --git a/examples/all-clusters-app/all-clusters-common/src/energy-preference-delegate.cpp b/examples/all-clusters-app/all-clusters-common/src/energy-preference-delegate.cpp
@@ -24,7 +24,7 @@ static BalanceStruct::Type gsPowerBalances[] = {
 };
 
 // assumes it'll be the only delegate for it's lifetime.
-struct EPrefDelegate : public EnergyPreferenceDelegate
+struct EPrefDelegate : public Delegate
 {
     EPrefDelegate();
     virtual ~EPrefDelegate();
@@ -37,16 +37,16 @@ struct EPrefDelegate : public EnergyPreferenceDelegate
     size_t GetNumLowPowerModes(chip::EndpointId aEndpoint) override;
 };
 
-EPrefDelegate::EPrefDelegate() : EnergyPreferenceDelegate()
+EPrefDelegate::EPrefDelegate() : Delegate()
 {
-    VerifyOrDie(GetMatterEnergyPreferencesDelegate() == nullptr);
-    SetMatterEnergyPreferencesDelegate(this);
+    VerifyOrDie(GetDelegate() == nullptr);
+    SetDelegate(this);
 }
 
 EPrefDelegate::~EPrefDelegate()
 {
-    VerifyOrDie(GetMatterEnergyPreferencesDelegate() == this);
-    SetMatterEnergyPreferencesDelegate(nullptr);
+    VerifyOrDie(GetDelegate() == this);
+    SetDelegate(nullptr);
 }
 
 size_t EPrefDelegate::GetNumEnergyBalances(chip::EndpointId aEndpoint)

diff --git a/src/app/clusters/energy-preference-server/energy-preference-server.cpp b/src/app/clusters/energy-preference-server/energy-preference-server.cpp
@@ -17,15 +17,12 @@
 
 #include "energy-preference-server.h"
 
-#include <app/util/af.h>
-
 #include <app/util/attribute-storage.h>
 
 #include <app-common/zap-generated/attributes/Accessors.h>
 #include <app-common/zap-generated/callback.h>
 #include <app-common/zap-generated/cluster-objects.h>
 #include <app-common/zap-generated/ids/Attributes.h>
-#include <app/CommandHandler.h>
 #include <app/ConcreteAttributePath.h>
 #include <app/ConcreteCommandPath.h>
 #include <app/util/error-mapping.h>
@@ -52,7 +49,7 @@ class EnergyPrefAttrAccess : public AttributeAccessInterface
 };
 
 EnergyPrefAttrAccess gEnergyPrefAttrAccess;
-EnergyPreferenceDelegate * gsDelegate = nullptr;
+Delegate * gsDelegate = nullptr;
 
 CHIP_ERROR EnergyPrefAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder)
 {
@@ -157,22 +154,27 @@ CHIP_ERROR EnergyPrefAttrAccess::Write(const ConcreteDataAttributePath & aPath,
 
 } // anonymous namespace
 
-void SetMatterEnergyPreferencesDelegate(EnergyPreferenceDelegate * aDelegate)
+namespace chip::app::Clusters::EnergyPreference
+{
+
+void SetDelegate(Delegate * aDelegate)
 {
     gsDelegate = aDelegate;
 }
 
-EnergyPreferenceDelegate * GetMatterEnergyPreferencesDelegate()
+Delegate * GetDelegate()
 {
     return gsDelegate;
 }
 
+} // Set matter energy preferences delegate
+
 Protocols::InteractionModel::Status
 MatterEnergyPreferenceClusterServerPreAttributeChangedCallback(const app::ConcreteAttributePath & attributePath,
                                                                EmberAfAttributeType attributeType, uint16_t size, uint8_t * value)
 {
     EndpointId endpoint                 = attributePath.mEndpointId;
-    EnergyPreferenceDelegate * delegate = GetMatterEnergyPreferencesDelegate();
+    Delegate * delegate = GetDelegate();
     uint32_t ourFeatureMap;
     bool balanceSupported = (FeatureMap::Get(attributePath.mEndpointId, &ourFeatureMap) == EMBER_ZCL_STATUS_SUCCESS) &&
         ((ourFeatureMap & to_underlying(Feature::kEnergyBalance)) != 0);
@@ -185,27 +187,31 @@ MatterEnergyPreferenceClusterServerPreAttributeChangedCallback(const app::Concre
     {
     case CurrentEnergyBalance::Id: {
         if (balanceSupported == false)
+        {
             return imcode::UnsupportedAttribute;
+        }
 
-        uint8_t index    = chip::Encoding::Get8(value);
+        uint8_t index    = Encoding::Get8(value);
         size_t arraySize = delegate->GetNumEnergyBalances(endpoint);
         if (index >= arraySize)
         {
-            return imcode::InvalidValue;
+            return imcode::ConstraintError;
         }
 
         return imcode::Success;
     }
 
     case CurrentLowPowerModeSensitivity::Id: {
         if (lowPowerSupported == false)
+        {
             return imcode::UnsupportedAttribute;
+        }
 
-        uint8_t index    = chip::Encoding::Get8(value);
+        uint8_t index    = Encoding::Get8(value);
         size_t arraySize = delegate->GetNumLowPowerModes(endpoint);
         if (index >= arraySize)
         {
-            return imcode::InvalidValue;
+            return imcode::ConstraintError;
         }
 
         return imcode::Success;

diff --git a/src/app/clusters/energy-preference-server/energy-preference-server.h b/src/app/clusters/energy-preference-server/energy-preference-server.h
@@ -24,14 +24,25 @@
 #include <app/util/basic-types.h>
 #include <lib/core/CHIPError.h>
 
-struct EnergyPreferenceDelegate
+namespace chip::app::Clusters::EnergyPreference
 {
-    virtual ~EnergyPreferenceDelegate() {}
 
+struct Delegate
+{
+    virtual ~Delegate() {}
+
+    // Gives a reference to the energy balance struct at aIndex
+    // Balance struct should exist for the life time of the matter server
     virtual CHIP_ERROR GetEnergyBalanceAtIndex(chip::EndpointId aEndpoint, size_t aIndex,
                                                chip::app::Clusters::EnergyPreference::Structs::BalanceStruct::Type & balance) = 0;
+
+    // Gives a reference to the  at aIndex
     virtual CHIP_ERROR GetEnergyPriorityAtIndex(chip::EndpointId aEndpoint, size_t aIndex,
                                                 chip::app::Clusters::EnergyPreference::EnergyPriorityEnum & priority)         = 0;
+
+
+    // Gives a reference to the low power mode sensitivity balance struct at aIndex
+    // Balance struct should exist for the life time of the matter server
     virtual CHIP_ERROR
     GetLowPowerModeSensitivityAtIndex(chip::EndpointId aEndpoint, size_t aIndex,
                                       chip::app::Clusters::EnergyPreference::Structs::BalanceStruct::Type & balance) = 0;
@@ -40,5 +51,7 @@ struct EnergyPreferenceDelegate
     virtual size_t GetNumLowPowerModes(chip::EndpointId aEndpoint)  = 0;
 };
 
-void SetMatterEnergyPreferencesDelegate(EnergyPreferenceDelegate * aDelegate);
-EnergyPreferenceDelegate * GetMatterEnergyPreferencesDelegate();
+void SetDelegate(Delegate * aDelegate);
+Delegate * GetDelegate();
+
+} // namespace chip::app::Clusters::EnergyPreference
diff --git a/src/app/common/templates/config-data.yaml b/src/app/common/templates/config-data.yaml
@@ -82,5 +82,6 @@ ClustersWithPreAttributeChangeFunctions:
     - Mode Select
     - Fan Control
     - Thermostat
+    - Energy Preference
     - Laundry Washer Controls
     - Laundry Dryer Controls