[HVAC] Add check for per-preset-scenario limit

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

@@ -152,69 +152,36 @@ 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.
+ * @brief Gets the maximum number of presets allowed for a given preset scenario.
  *
- * @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.
+ * @param[in]  delegate The delegate to use.
+ * @param[in]  presetScenario The presetScenario to match with.
+ * @param[out] count The maximum number of presets for the specified presetScenario
+ * @return CHIP_NO_ERROR if the maximum number was determined, or an error if not
  */
-uint8_t CountNumberOfPendingPresets(Delegate * delegate)
+CHIP_ERROR MaximumPresetScenarioCount(Delegate * delegate, PresetScenarioEnum presetScenario, size_t & count)
 {
-    uint8_t numberOfPendingPresets = 0;
-
-    VerifyOrReturnValue(delegate != nullptr, 0);
-
+    count = 0;
     for (uint8_t i = 0; true; i++)
     {
-        PresetStructWithOwnedMembers pendingPreset;
-        CHIP_ERROR err = delegate->GetPendingPresetAtIndex(i, pendingPreset);
-
+        PresetTypeStruct::Type presetType;
+        auto err = delegate->GetPresetTypeAtIndex(i, presetType);
         if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED)
         {
-            break;
+            // We exhausted the list trying to find the preset scenario
+            return CHIP_NO_ERROR;
         }
-        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.
- *
- * @param[in] delegate The delegate to use.
- * @param[in] presetScenario The presetScenario to match with.
- *
- * @return true if the presetScenario is found, false otherwise.
- */
-bool PresetScenarioExistsInPresetTypes(Delegate * delegate, PresetScenarioEnum presetScenario)
-{
-    VerifyOrReturnValue(delegate != nullptr, false);
-
-    for (uint8_t i = 0; true; i++)
-    {
-        PresetTypeStruct::Type presetType;
-        auto err = delegate->GetPresetTypeAtIndex(i, presetType);
-        if (err != CHIP_NO_ERROR)
+        else if (err != CHIP_NO_ERROR)
         {
-            return false;
+            return err;
         }
-
         if (presetType.presetScenario == presetScenario)
         {
-            return true;
+            count = presetType.numberOfPresets;
+            return CHIP_NO_ERROR;
         }
     }
-    return false;
+    return CHIP_NO_ERROR;
 }
 
 /**
@@ -359,6 +326,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 +381,17 @@ CHIP_ERROR ThermostatAttrAccess::AppendPendingPreset(Thermostat::Delegate * dele
         }
     }
 
-    if (!PresetScenarioExistsInPresetTypes(delegate, preset.GetPresetScenario()))
+    size_t maximumPresetCount = delegate->GetNumberOfPresets();
+
+    size_t maximumPresetScenarioCount = 0;
+    if (MaximumPresetScenarioCount(delegate, preset.GetPresetScenario(), maximumPresetScenarioCount) != CHIP_NO_ERROR)
     {
+        return CHIP_IM_GLOBAL_STATUS(InvalidInState);
+    }
+
+    if (maximumPresetScenarioCount == 0)
+    {
+        // This is not a supported preset scenario
         return CHIP_IM_GLOBAL_STATUS(ConstraintError);
     }
 
@@ -423,16 +403,38 @@ 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);
+
+        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++;
+        }
+    }
 
-    // We will be adding one more preset, so reject if the length is already at max.
-    if (numberOfPendingPresets >= delegate->GetNumberOfPresets())
+    if (presetCount > maximumPresetCount)
     {
+        ChipLogError(Zcl, "Preset count exceeded %u: %u ", (unsigned) maximumPresetCount, (unsigned) 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 %u: %u ", (unsigned) maximumPresetScenarioCount,
+                     (unsigned) presetScenarioCount);
+        return CHIP_IM_GLOBAL_STATUS(ResourceExhausted);
+    }
 
     return delegate->AppendToPendingPresetList(preset);
 }

src/python_testing/TC_TSTAT_4_2.py

@@ -356,8 +356,9 @@ async def test_TC_TSTAT_4_2(self):
                 logger.info(
                     "Couldn't run test step 4 since there were no built-in presets")
 
-            # Send the SetActivePresetRequest command
-            await self.send_set_active_preset_handle_request_command(value=b'\x03')
+            notBuiltInPresets = list(preset for preset in current_presets if preset.builtIn is False)
+            if len(notBuiltInPresets) > 0:
+                activePreset = notBuiltInPresets[0]
 
             activePresetHandle = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.ActivePresetHandle)
 
@@ -631,7 +632,7 @@ async def test_TC_TSTAT_4_2(self):
             if len(unavailableScenarios) > 0:
                 test_presets = current_presets.copy()
                 test_presets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=unavailableScenarios[0],
-                                                                 name="Preset", coolingSetpoint=coolSetpoint, heatingSetpoint=heatSetpoint, builtIn=False))
+                                                                 coolingSetpoint=coolSetpoint, heatingSetpoint=heatSetpoint, builtIn=False))
 
                 # Send the AtomicRequest begin command
                 await self.send_atomic_request_begin_command()
@@ -647,6 +648,29 @@ async def test_TC_TSTAT_4_2(self):
         self.step("18")
         if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")):
 
+            # Generate list of tuples of scenarios and number of remaining presets per scenario allowed
+            presetScenarioHeadroom = list([presetType.presetScenario,
+                                           presetType.numberOfPresets - presetScenarioCounts.get(presetType.presetScenario, 0)] for presetType in presetTypes)
+
+            if len(presetScenarioHeadroom) > 0:
+                # Find the preset scenario with the fewest number of remaining allowed presets
+                presetScenarioHeadroom = sorted(presetScenarioHeadroom, key=lambda diff: diff[1])
+                presetScenario = presetScenarioHeadroom[0][0]
+                headroom = presetScenarioHeadroom[0][1]
+
+                # Add one more preset than is allowed by the preset type
+                test_presets = copy.deepcopy(current_presets)
+                for _ in range(0, headroom + 1):
+                    test_presets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=presetScenario,
+                                                                     coolingSetpoint=coolSetpoint, heatingSetpoint=heatSetpoint, builtIn=False))
+
+                await self.send_atomic_request_begin_command()
+
+                await self.write_presets(endpoint=endpoint, presets=test_presets, expected_status=Status.ResourceExhausted)
+
+                # Clear state for next test.
+                await self.send_atomic_request_rollback_command()
+
             # Calculate the length of the Presets list that could be created using the preset scenarios in PresetTypes and numberOfPresets supported for each scenario.
             totalExpectedPresetsLength = sum(presetType.numberOfPresets for presetType in presetTypes)