Fixed minor issues from review

project-chip · Jan 22, 2024 · 3848b7d · 3848b7d
1 parent 66e24ad
commit 3848b7d
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 40 deletions.
diff --git a/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter b/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter
@@ -4334,7 +4334,7 @@ provisional cluster EnergyEvse = 153 {
 
 /** This cluster provides an interface to specify preferences for how devices should consume energy. */
 cluster EnergyPreference = 155 {
-  revision 1; // NOTE: Default/not specifically set
+  revision 1;
 
   enum EnergyPriorityEnum : enum8 {
     kComfort = 0;

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
@@ -4,37 +4,23 @@ using namespace chip;
 using namespace chip::app::Clusters::EnergyPreference;
 using namespace chip::app::Clusters::EnergyPreference::Structs;
 
-static constexpr const char * kEfficientLabel = "Efficient";
-static constexpr const char * kComfortLabel   = "Comfort";
-
 static BalanceStruct::Type gsEnergyBalances[] = {
-    { .step = 0, .label = Optional<chip::CharSpan>(chip::CharSpan(kEfficientLabel, strlen(kEfficientLabel))) },
+    { .step = 0, .label = Optional<chip::CharSpan>("Efficient"_span) },
     { .step = 50, .label = Optional<chip::CharSpan>() },
-    { .step = 100, .label = Optional<chip::CharSpan>(chip::CharSpan(kComfortLabel, strlen(kComfortLabel))) },
+    { .step = 100, .label = Optional<chip::CharSpan>("Comfort"_span) },
 };
 
-static constexpr const char * k1MinuteLabel    = "1 Minute";
-static constexpr const char * k5MinutesLabel   = "5 Minutes";
-static constexpr const char * k10MinutesLabel  = "10 Minutes";
-static constexpr const char * k15MinutesLabel  = "15 Minutes";
-static constexpr const char * k20MinutesLabel  = "20 Minutes";
-static constexpr const char * k25MinutesLabel  = "25 Minutes";
-static constexpr const char * k30MinutesLabel  = "30 Minutes";
-static constexpr const char * k60MinutesLabel  = "60 Minutes";
-static constexpr const char * k120MinutesLabel = "120 Minutes";
-static constexpr const char * kNeverLabel      = "Never";
-
 static BalanceStruct::Type gsPowerBalances[] = {
-    { .step = 0, .label = Optional<chip::CharSpan>(chip::CharSpan(k1MinuteLabel, strlen(k1MinuteLabel))) },
-    { .step = 12, .label = Optional<chip::CharSpan>(chip::CharSpan(k5MinutesLabel, strlen(k5MinutesLabel))) },
-    { .step = 24, .label = Optional<chip::CharSpan>(chip::CharSpan(k10MinutesLabel, strlen(k10MinutesLabel))) },
-    { .step = 36, .label = Optional<chip::CharSpan>(chip::CharSpan(k15MinutesLabel, strlen(k15MinutesLabel))) },
-    { .step = 48, .label = Optional<chip::CharSpan>(chip::CharSpan(k20MinutesLabel, strlen(k20MinutesLabel))) },
-    { .step = 60, .label = Optional<chip::CharSpan>(chip::CharSpan(k25MinutesLabel, strlen(k25MinutesLabel))) },
-    { .step = 70, .label = Optional<chip::CharSpan>(chip::CharSpan(k30MinutesLabel, strlen(k30MinutesLabel))) },
-    { .step = 80, .label = Optional<chip::CharSpan>(chip::CharSpan(k60MinutesLabel, strlen(k60MinutesLabel))) },
-    { .step = 90, .label = Optional<chip::CharSpan>(chip::CharSpan(k120MinutesLabel, strlen(k120MinutesLabel))) },
-    { .step = 100, .label = Optional<chip::CharSpan>(chip::CharSpan(kNeverLabel, strlen(kNeverLabel))) },
+    { .step = 0, .label = Optional<chip::CharSpan>("1 Minute"_span) },
+    { .step = 12, .label = Optional<chip::CharSpan>("5 Minutes"_span) },
+    { .step = 24, .label = Optional<chip::CharSpan>("10 Minutes"_span) },
+    { .step = 36, .label = Optional<chip::CharSpan>("15 Minutes"_span) },
+    { .step = 48, .label = Optional<chip::CharSpan>("20 Minutes"_span) },
+    { .step = 60, .label = Optional<chip::CharSpan>("25 Minutes"_span) },
+    { .step = 70, .label = Optional<chip::CharSpan>("30 Minutes"_span) },
+    { .step = 80, .label = Optional<chip::CharSpan>("60 Minutes"_span) },
+    { .step = 90, .label = Optional<chip::CharSpan>("120 Minutes"_span) },
+    { .step = 100, .label = Optional<chip::CharSpan>("Never"_span) },
 };
 
 // assumes it'll be the only delegate for it's lifetime.
@@ -53,22 +39,24 @@ struct EPrefDelegate : public EnergyPreferenceDelegate
 
 EPrefDelegate::EPrefDelegate() : EnergyPreferenceDelegate()
 {
+    VerifyOrDie(GetMatterEnergyPreferencesDelegate() == nullptr);
     SetMatterEnergyPreferencesDelegate(this);
 }
 
 EPrefDelegate::~EPrefDelegate()
 {
+    VerifyOrDie(GetMatterEnergyPreferencesDelegate() == this);
     SetMatterEnergyPreferencesDelegate(nullptr);
 }
 
 size_t EPrefDelegate::GetNumEnergyBalances(chip::EndpointId aEndpoint)
 {
-    return (sizeof(gsEnergyBalances) / sizeof(gsEnergyBalances[0]));
+    return (ArraySize(gsEnergyBalances));
 }
 
 size_t EPrefDelegate::GetNumLowPowerModes(chip::EndpointId aEndpoint)
 {
-    return (sizeof(gsEnergyBalances) / sizeof(gsEnergyBalances[0]));
+    return (ArraySize(gsEnergyBalances));
 }
 
 CHIP_ERROR

diff --git a/src/app/clusters/energy-preference-server/energy-preference-server.cpp b/src/app/clusters/energy-preference-server/energy-preference-server.cpp
@@ -65,8 +65,8 @@ CHIP_ERROR EnergyPrefAttrAccess::Read(const ConcreteReadAttributePath & aPath, A
 
     switch (aPath.mAttributeId)
     {
-    case EnergyBalances::Id: {
-        if (balanceSupported == false)
+    case EnergyBalances::Id:
+        if (!balanceSupported)
         {
             return aEncoder.EncodeNull();
         }
@@ -90,9 +90,7 @@ CHIP_ERROR EnergyPrefAttrAccess::Read(const ConcreteReadAttributePath & aPath, A
             });
         }
         return CHIP_ERROR_INCORRECT_STATE;
-    }
-    break;
-    case EnergyPriorities::Id: {
+    case EnergyPriorities::Id: 
         if (balanceSupported == false)
         {
             return aEncoder.EncodeNull();
@@ -117,9 +115,7 @@ CHIP_ERROR EnergyPrefAttrAccess::Read(const ConcreteReadAttributePath & aPath, A
             });
         }
         return CHIP_ERROR_INCORRECT_STATE;
-    }
-    break;
-    case LowPowerModeSensitivities::Id: {
+    case LowPowerModeSensitivities::Id:
         if (lowPowerSupported == false)
         {
             return aEncoder.EncodeNull();
@@ -144,8 +140,6 @@ CHIP_ERROR EnergyPrefAttrAccess::Read(const ConcreteReadAttributePath & aPath, A
             });
         }
         return CHIP_ERROR_INCORRECT_STATE;
-    }
-    break;
     default: // return CHIP_NO_ERROR and just read from the attribute store in default
         break;
     }
@@ -196,7 +190,10 @@ MatterEnergyPreferenceClusterServerPreAttributeChangedCallback(const app::Concre
         uint8_t index    = chip::Encoding::Get8(value);
         size_t arraySize = delegate->GetNumEnergyBalances(endpoint);
         if (index >= arraySize)
+        {
             return imcode::InvalidValue;
+        }
+
         return imcode::Success;
     }
 
@@ -207,7 +204,10 @@ MatterEnergyPreferenceClusterServerPreAttributeChangedCallback(const app::Concre
         uint8_t index    = chip::Encoding::Get8(value);
         size_t arraySize = delegate->GetNumLowPowerModes(endpoint);
         if (index >= arraySize)
+        {
             return imcode::InvalidValue;
+        }
+
         return imcode::Success;
     }
     default:

diff --git a/src/app/zap-templates/zcl/data-model/chip/energy-preference-cluster.xml b/src/app/zap-templates/zcl/data-model/chip/energy-preference-cluster.xml
@@ -22,6 +22,7 @@ limitations under the License.
     <server init="false" tick="false">true</server>
     <description>This cluster provides an interface to specify preferences for how devices should consume energy.</description>
     <!--Attributes-->
+    <globalAttribute side="either" code="0xFFFD" value="1"/>
 
     <!--Conformance feature BALA - for now optional-->
     <attribute code="0x0000" side="server" type="array" entryType="BalanceStruct" define="ENERGY_PREFERENCE_ENERGY_BALANCES" isNullable="false" min="2" max="10" writable="false" optional="true">EnergyBalances</attribute>

diff --git a/src/controller/data_model/controller-clusters.matter b/src/controller/data_model/controller-clusters.matter
@@ -4765,7 +4765,7 @@ provisional cluster EnergyEvse = 153 {
 
 /** This cluster provides an interface to specify preferences for how devices should consume energy. */
 cluster EnergyPreference = 155 {
-  revision 1; // NOTE: Default/not specifically set
+  revision 1;
 
   enum EnergyPriorityEnum : enum8 {
     kComfort = 0;