[HVAC] Check if number of preset scenarios exceeds maximum number of scenarios

Author: hasty

examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h

@@ -39,6 +39,8 @@ static constexpr uint8_t kMaxNumberOfPresetTypes = 6;
 // We will support only one preset of each preset type.
 static constexpr uint8_t kMaxNumberOfPresetsOfEachType = 1;
 
+static constexpr uint8_t kMaxNumberOfPresetsSupported = kMaxNumberOfPresetTypes * kMaxNumberOfPresetsOfEachType - 1;
+
 class ThermostatDelegate : public Delegate
 {
 public:

examples/thermostat/thermostat-common/src/thermostat-delegate-impl.cpp

@@ -31,7 +31,7 @@ ThermostatDelegate ThermostatDelegate::sInstance;
 
 ThermostatDelegate::ThermostatDelegate()
 {
-    mNumberOfPresets                   = kMaxNumberOfPresetTypes * kMaxNumberOfPresetsOfEachType;
+    mNumberOfPresets                   = kMaxNumberOfPresetsSupported;
     mNextFreeIndexInPresetsList        = 0;
     mNextFreeIndexInPendingPresetsList = 0;
 
@@ -87,6 +87,9 @@ CHIP_ERROR ThermostatDelegate::GetPresetTypeAtIndex(size_t index, PresetTypeStru
         { .presetScenario     = PresetScenarioEnum::kVacation,
           .numberOfPresets    = kMaxNumberOfPresetsOfEachType,
           .presetTypeFeatures = to_underlying(PresetTypeFeaturesBitmap::kSupportsNames) },
+        { .presetScenario     = PresetScenarioEnum::kUserDefined,
+          .numberOfPresets    = kMaxNumberOfPresetsOfEachType,
+          .presetTypeFeatures = to_underlying(PresetTypeFeaturesBitmap::kSupportsNames) },
     };
     if (index < ArraySize(presetTypes))
     {

src/app/clusters/thermostat-server/thermostat-server-presets.cpp

@@ -152,51 +152,14 @@ bool GetMatchingPresetInPresets(Delegate * delegate, const DataModel::Nullable<B
 }
 
 /**
- * @brief Returns the length of the list of presets if the pending presets were to be applied. The size of the pending presets list
- *        calculated, after all the constraint checks are done, is the new size of the updated Presets attribute since the pending
- *        preset list is expected to have all existing presets with or without edits plus new presets.
- *        This is called before changes are actually applied.
- *
- * @param[in] delegate The delegate to use.
- *
- * @return count of the updated Presets attribute if the pending presets were applied to it. Return 0 for error cases.
- */
-uint8_t CountNumberOfPendingPresets(Delegate * delegate)
-{
-    uint8_t numberOfPendingPresets = 0;
-
-    VerifyOrReturnValue(delegate != nullptr, 0);
-
-    for (uint8_t i = 0; true; i++)
-    {
-        PresetStructWithOwnedMembers pendingPreset;
-        CHIP_ERROR err = delegate->GetPendingPresetAtIndex(i, pendingPreset);
-
-        if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED)
-        {
-            break;
-        }
-        if (err != CHIP_NO_ERROR)
-        {
-            ChipLogError(Zcl, "CountNumberOfPendingPresets: GetPendingPresetAtIndex failed with error %" CHIP_ERROR_FORMAT,
-                         err.Format());
-            return 0;
-        }
-        numberOfPendingPresets++;
-    }
-
-    return numberOfPendingPresets;
-}
-
-/**
- * @brief Checks if the presetScenario is present in the PresetTypes attribute.
+ * @brief Gets the maximum number of presets allowed for a given preset scenario.
  *
  * @param[in] delegate The delegate to use.
  * @param[in] presetScenario The presetScenario to match with.
  *
- * @return true if the presetScenario is found, false otherwise.
+ * @return The maximum number of presets allowed for the preset scenario
  */
-bool PresetScenarioExistsInPresetTypes(Delegate * delegate, PresetScenarioEnum presetScenario)
+uint8_t MaximumPresetScenarioCount(Delegate * delegate, PresetScenarioEnum presetScenario)
 {
     VerifyOrReturnValue(delegate != nullptr, false);
 
@@ -206,15 +169,17 @@ bool PresetScenarioExistsInPresetTypes(Delegate * delegate, PresetScenarioEnum p
         auto err = delegate->GetPresetTypeAtIndex(i, presetType);
         if (err != CHIP_NO_ERROR)
         {
-            return false;
+            // Either we failed to fetch the next preset type, in which case we should error higher,
+            // or we exhausted the list trying to find the preset type
+            return 0;
         }
 
         if (presetType.presetScenario == presetScenario)
         {
-            return true;
+            return presetType.numberOfPresets;
         }
     }
-    return false;
+    return 0;
 }
 
 /**
@@ -359,6 +324,10 @@ CHIP_ERROR ThermostatAttrAccess::AppendPendingPreset(Thermostat::Delegate * dele
         return CHIP_IM_GLOBAL_STATUS(ConstraintError);
     }
 
+    // We're going to append this preset, so let's assume a count as though it had already been inserted
+    size_t presetCount         = 1;
+    size_t presetScenarioCount = 1;
+
     if (preset.GetPresetHandle().IsNull())
     {
         if (IsBuiltIn(preset))
@@ -410,8 +379,12 @@ CHIP_ERROR ThermostatAttrAccess::AppendPendingPreset(Thermostat::Delegate * dele
         }
     }
 
-    if (!PresetScenarioExistsInPresetTypes(delegate, preset.GetPresetScenario()))
+    size_t maximumPresetCount         = delegate->GetNumberOfPresets();
+    size_t maximumPresetScenarioCount = MaximumPresetScenarioCount(delegate, preset.GetPresetScenario());
+
+    if (maximumPresetScenarioCount == 0)
     {
+        // This is not a supported preset scenario
         return CHIP_IM_GLOBAL_STATUS(ConstraintError);
     }
 
@@ -423,16 +396,37 @@ CHIP_ERROR ThermostatAttrAccess::AppendPendingPreset(Thermostat::Delegate * dele
     // Before adding this preset to the pending presets, if the expected length of the pending presets' list
     // exceeds the total number of presets supported, return RESOURCE_EXHAUSTED. Note that the preset has not been appended yet.
 
-    uint8_t numberOfPendingPresets = CountNumberOfPendingPresets(delegate);
+    for (uint8_t i = 0; true; i++)
+    {
+        PresetStructWithOwnedMembers otherPreset;
+        CHIP_ERROR err = delegate->GetPendingPresetAtIndex(i, otherPreset);
 
-    // We will be adding one more preset, so reject if the length is already at max.
-    if (numberOfPendingPresets >= delegate->GetNumberOfPresets())
+        if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED)
+        {
+            break;
+        }
+        if (err != CHIP_NO_ERROR)
+        {
+            return CHIP_IM_GLOBAL_STATUS(InvalidInState);
+        }
+        presetCount++;
+        if (preset.GetPresetScenario() == otherPreset.GetPresetScenario())
+        {
+            presetScenarioCount++;
+        }
+    }
+
+    if (presetCount > maximumPresetCount)
     {
+        ChipLogError(Zcl, "Preset count exceeded %zu: %zu ", maximumPresetCount, presetCount);
         return CHIP_IM_GLOBAL_STATUS(ResourceExhausted);
     }
 
-    // TODO #34556 : Check if the number of presets for each presetScenario exceeds the max number of presets supported for that
-    // scenario. We plan to support only one preset for each presetScenario for our use cases so defer this for re-evaluation.
+    if (presetScenarioCount > maximumPresetScenarioCount)
+    {
+        ChipLogError(Zcl, "Preset scenario count exceeded %zu: %zu ", maximumPresetScenarioCount, presetScenarioCount);
+        return CHIP_IM_GLOBAL_STATUS(ResourceExhausted);
+    }
 
     return delegate->AppendToPendingPresetList(preset);
 }