Suggestions from code review

project-chip · Sep 1, 2024 · d1fe36d · d1fe36d
1 parent 79edbd5
commit d1fe36d
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 14 deletions.
diff --git a/src/app/clusters/thermostat-server/thermostat-server-presets.cpp b/src/app/clusters/thermostat-server/thermostat-server-presets.cpp
@@ -171,7 +171,7 @@ CHIP_ERROR MaximumPresetScenarioCount(Delegate * delegate, PresetScenarioEnum pr
             // We exhausted the list trying to find the preset scenario
             return CHIP_NO_ERROR;
         }
-        else if (err != CHIP_NO_ERROR)
+        if (err != CHIP_NO_ERROR)
         {
             return err;
         }
@@ -326,10 +326,6 @@ 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))
@@ -381,8 +377,7 @@ CHIP_ERROR ThermostatAttrAccess::AppendPendingPreset(Thermostat::Delegate * dele
         }
     }
 
-    size_t maximumPresetCount = delegate->GetNumberOfPresets();
-
+    size_t maximumPresetCount         = delegate->GetNumberOfPresets();
     size_t maximumPresetScenarioCount = 0;
     if (MaximumPresetScenarioCount(delegate, preset.GetPresetScenario(), maximumPresetScenarioCount) != CHIP_NO_ERROR)
     {
@@ -403,6 +398,9 @@ 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.
 
+    // 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;
     for (uint8_t i = 0; true; i++)
     {
         PresetStructWithOwnedMembers otherPreset;

diff --git a/src/python_testing/TC_TSTAT_4_2.py b/src/python_testing/TC_TSTAT_4_2.py
@@ -29,6 +29,7 @@
 import copy
 import logging
 import random
+from collections import namedtuple
 
 import chip.clusters as Clusters
 from chip import ChipDeviceCtrl  # Needed before chip.FabricAdmin
@@ -642,20 +643,20 @@ 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")):
 
+            ScenarioHeadroom = namedtuple("ScenarioHeadroom", "presetScenario remaining")
             # Generate list of tuples of scenarios and number of remaining presets per scenario allowed
-            presetScenarioHeadroom = list((presetType.presetScenario,
+            presetScenarioHeadrooms = list(ScenarioHeadroom(presetType.presetScenario,
                                            presetType.numberOfPresets - presetScenarioCounts.get(presetType.presetScenario, 0)) for presetType in presetTypes)
 
-            if len(presetScenarioHeadroom) > 0:
+            if presetScenarioHeadrooms:
                 # Find the preset scenario with the smallest number of remaining allowed presets
-                presetScenarioHeadroom = sorted(presetScenarioHeadroom, key=lambda diff: diff[1])
-                (presetScenario, headroom) = presetScenarioHeadroom[0]
+                presetScenarioHeadrooms = sorted(presetScenarioHeadrooms, key=lambda psh: psh.remaining)
+                presetScenarioHeadroom = presetScenarioHeadrooms[0]
 
                 # 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))
+                test_presets.extend([cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=presetScenarioHeadroom.presetScenario,
+                                                                  coolingSetpoint=coolSetpoint, heatingSetpoint=heatSetpoint, builtIn=False)] * (presetScenarioHeadroom.remaining + 1))
 
                 await self.send_atomic_request_begin_command()