Fix handling of deadband in themostat constraint enforcement. (#19564)

The deadband support was incorrectly conditioned on the OCC bit (feature bit 2), and we were never setting OccupancySupported to true. Our test apps don't set the OCC bit, so this last was not getting caught. Fixes #19551
project-chip · Nov 17, 2023 · 2537321 · 2537321
1 parent 3f73ee1
commit 2537321
Show file tree

Hide file tree

Showing 4 changed files with 329 additions and 627 deletions.
diff --git a/src/app/clusters/thermostat-server/thermostat-server.cpp b/src/app/clusters/thermostat-server/thermostat-server.cpp
@@ -127,15 +127,16 @@ MatterThermostatClusterServerPreAttributeChangedCallback(const app::ConcreteAttr
         CoolSupported = true;
 
     if (OurFeatureMap & 1 << 2)
+        OccupancySupported = true;
 
-        if (AutoSupported)
+    if (AutoSupported)
+    {
+        if (MinSetpointDeadBand::Get(endpoint, &DeadBand) != EMBER_ZCL_STATUS_SUCCESS)
         {
-            if (MinSetpointDeadBand::Get(endpoint, &DeadBand) != EMBER_ZCL_STATUS_SUCCESS)
-            {
-                DeadBand = kDefaultDeadBand;
-            }
-            DeadBandTemp = static_cast<int16_t>(DeadBand * 10);
+            DeadBand = kDefaultDeadBand;
         }
+        DeadBandTemp = static_cast<int16_t>(DeadBand * 10);
+    }
 
     if (AbsMinCoolSetpointLimit::Get(endpoint, &AbsMinCoolSetpointLimit) != EMBER_ZCL_STATUS_SUCCESS)
         AbsMinCoolSetpointLimit = kDefaultAbsMinCoolSetpointLimit;

diff --git a/src/app/tests/suites/certification/Test_TC_TSTAT_2_2.yaml b/src/app/tests/suites/certification/Test_TC_TSTAT_2_2.yaml
@@ -50,7 +50,7 @@ tests:
       attribute: "OccupiedCoolingSetpoint"
       PICS: A_OCCUPIEDCOOLINGSETPOINT
       arguments:
-          value: 2000
+          value: 2250
 
     - label:
           "Reads it back again to confirm the successful write of
@@ -60,7 +60,7 @@ tests:
       attribute: "OccupiedCoolingSetpoint"
       PICS: A_OCCUPIEDCOOLINGSETPOINT
       response:
-          value: 2000
+          value: 2250
 
     - label:
           "Writes OccupiedCoolingSetpoint to value below the
@@ -508,9 +508,13 @@ tests:
       arguments:
           value: 700
 
+    # Disabled: This test makes no sense: It's setting MaxHeatSetpointLimit to
+    # a value too close to MaxCoolSetpointLimit, ignoring deadband.
+    # See https://github.com/CHIP-Specifications/chip-test-plans/issues/1379
     - label:
           "Writes the limit of AbsMaxHeatSetpointLimit to MaxHeatSetpointLimit
           attribute"
+      disabled: true
       optional: true
       command: "writeAttribute"
       attribute: "MaxHeatSetpointLimit"
@@ -691,7 +695,11 @@ tests:
       arguments:
           value: 700
 
+    # Disabled: This test makes no sense: It's setting MaxHeatSetpointLimit to
+    # a value too close to MaxCoolSetpointLimit, ignoring deadband.
+    # See https://github.com/CHIP-Specifications/chip-test-plans/issues/1379
     - label: "Writes (sets back)default value of MaxHeatSetpointLimit"
+      disabled: true
       optional: true
       command: "writeAttribute"
       attribute: "MaxHeatSetpointLimit"
@@ -814,8 +822,11 @@ tests:
       response:
           value: 2
 
+    # Disabled: This test can't work because our setpoint limits are all messed up.
+    # See https://github.com/CHIP-Specifications/chip-test-plans/issues/1379
     - label: "Sets OccupiedHeatingSetpoint to default value"
       optional: true
+      disabled: true
       command: "writeAttribute"
       attribute: "OccupiedHeatingSetpoint"
       PICS: A_OCCUPIEDHEATINGSETPOINT
@@ -843,8 +854,11 @@ tests:
       response:
           value: -30
 
+    # Disabled: This test can't work because our setpoint limits are all messed up.
+    # See https://github.com/CHIP-Specifications/chip-test-plans/issues/1379
     - label: "Sets OccupiedHeatingSetpoint to default value"
       optional: true
+      disabled: true
       command: "writeAttribute"
       attribute: "OccupiedHeatingSetpoint"
       PICS: A_OCCUPIEDHEATINGSETPOINT
@@ -938,7 +952,10 @@ tests:
       arguments:
           value: 2600
 
+    # Disabled: This test can't work because our setpoint limits are all messed up.
+    # See https://github.com/CHIP-Specifications/chip-test-plans/issues/1379
     - label: "Sets OccupiedHeatingSetpoint to default value"
+      disabled: true
       optional: true
       command: "writeAttribute"
       attribute: "OccupiedHeatingSetpoint"
@@ -985,8 +1002,11 @@ tests:
       arguments:
           value: 2600
 
+    # Disabled: This test can't work because our setpoint limits are all messed up.
+    # See https://github.com/CHIP-Specifications/chip-test-plans/issues/1379
     - label: "Sets OccupiedHeatingSetpoint to default value"
       optional: true
+      disabled: true
       command: "writeAttribute"
       attribute: "OccupiedHeatingSetpoint"
       PICS: A_OCCUPIEDHEATINGSETPOINT