Matter Thermostat: Shift the other setpoint on deadband violation

[kshypachov]

Check all that apply

Type of Change

• WWST Certification Request
  • If this is your first time contributing code:
    • I have reviewed the README.md file
    • I have reviewed the CODE_OF_CONDUCT.md file
    • I have signed the CLA
  • I plan on entering a WWST Certification Request or have entered a request through the WWST Certification console at developer.smartthings.com
• Bug fix
• New feature
• Refactor

Checklist

• I have performed a self-review of my code
• I have commented my code in hard-to-understand areas
• I have verified my changes by testing with a device or have communicated a plan for testing
• I am adding new behavior, such as adding a sub-driver, and have added and run new unit tests to cover the new behavior

Description of Change

On thermostats with the AutoMode feature the driver rejects a heating setpoint above cooling - MinSetpointDeadBand (and a cooling setpoint below heating + MinSetpointDeadBand) locally, so for example in Heat mode the heating setpoint cannot be raised above the cooling setpoint minus the deadband, and nothing is sent to the device.

The Thermostat cluster specification expects the other setpoint to be moved in this case; connectedhomeip does this since project-chip/connectedhomeip#35673 and rejects the write only if the other setpoint would leave its limits.

• If the requested setpoint breaks the deadband, the driver first writes the other setpoint (cooling = heating + deadband, or heating = cooling - deadband) and then the requested one.
• The command is still rejected locally (previous value re-emitted) when the moved setpoint would leave its own limits. Devices without AutoMode are not affected.
• The moved setpoint is computed and compared in 0.01 °C units to avoid floating point errors at the limits. The warnings now print the cached values instead of capability tables.

Ordering: written first, the other setpoint keeps the deadband against the current value of the requested side, and the requested value then keeps it against the new other setpoint. Servers that reject deadband violations (before #35673) therefore accept both writes, and servers that adjust by themselves find nothing to adjust.

The two writes are separate requests: on a hub with firmware 0.62 a single Write request with both attribute paths was answered with FAILURE for both paths and did not reach the device.

Forum: https://community.smartthings.com/t/incorrect-thermostat-behavior-in-mode-auto/311620

Summary of Completed Tests

• test_matter_thermo_setpoint_limits.lua: the two "inside deadband" tests now expect both writes; added a boundary case (moved setpoint exactly at its max) and rejection cases when the other setpoint cannot be moved, in both directions.
• test_matter_thermostat.lua: without AutoMode a heating setpoint above the cooling setpoint is written alone.
• python3 tools/run_driver_tests.py -f matter-thermostat: 131/131 pass (lua_libs API v21); luacheck clean on the changed files.
• Verified on a hub (firmware 0.62) with a Matter bridge thermostat endpoint (Thermostat 0x0301 with AutoMode, MinSetpointDeadBand 2.5 °C): raising the heating setpoint above the cooling setpoint and lowering the cooling setpoint below the heating setpoint both reach the device, both writes return SUCCESS, and both sliders follow each other in the app.

[CLAassistant]

All committers have signed the CLA.

[ldeora]

The current driver rejects setpoint changes locally even though the Thermostat server could satisfy the request by adjusting the complementary setpoint.

However, after reviewing the implementation, I have some concerns about moving that adjustment logic into the Edge driver.

1. Deadband enforcement belongs to the Thermostat server!

Matter 1.4.2, sections 4.3.9.13 and 4.3.9.14, defines the behavior when a setpoint change would violate MinSetpointDeadBand.

For example, when OccupiedHeatingSetpoint is increased beyond OccupiedCoolingSetpoint - MinSetpointDeadBand, the server adjusts OccupiedCoolingSetpoint accordingly, provided the resulting values remain within the applicable limits.

This behavior is implemented in connectedhomeip#35673.

Consequently, a compliant server should need only a single write to the requested attribute:

device:send(
  clusters.Thermostat.attributes.OccupiedHeatingSetpoint:write(
    device, endpoint_id, requested_setpoint
  )
)

The server maintains the deadband, and the driver processes the resulting attribute reports!

I would therefore favor removing the driver's cached-setpoint deadband rejection rather than replacing it with two client-side writes.

2. Two independent writes introduce consistency and concurrency risks

The proposed implementation writes the complementary setpoint first and the requested setpoint afterward.

Although the ordering preserves the deadband when both writes succeed, these are separate Matter interactions. The first write may succeed while the second fails, leaving the device with a modified complementary setpoint but without the requested change.

There is also a potentially more significant issue involving multiple Matter controllers.

The decision to adjust the complementary setpoint is based on device:get_latest_state(), which may not reflect the thermostat's current state.

Consider:

Cached cooling setpoint: 21.0 °C
Actual cooling setpoint: 27.0 °C
Requested heating setpoint: 23.0 °C
Deadband: 2.5 °C

The request is valid against the device's actual state.

However, the proposed implementation would calculate a new cooling setpoint of 25.5 °C based on the stale cached value and write it before updating the heating setpoint.

This would unnecessarily lower the cooling setpoint from 27.0 °C to 25.5 °C.

A single write to OccupiedHeatingSetpoint avoids that problem because the server evaluates the request against its authoritative state.

3. Endpoint-specific feature detection and constraints

The patch also retains two existing problems in the current driver:

• #3264: Setpoint limits and MinSetpointDeadBand are stored at device scope rather than per Thermostat endpoint.
• #3265: Auto support is determined by checking whether any Thermostat endpoint advertises the feature, rather than checking the endpoint targeted by the command.

These become especially relevant when calculating and writing complementary setpoints.

For a device exposing multiple Thermostat endpoints, the current implementation could use the deadband, setpoint limits, or Auto feature information belonging to another endpoint.

Any client-side validation should use metadata associated with the specific endpoint receiving the command.

4. Compatibility with older implementations

I understand the motivation for accommodating older Thermostat servers that reject deadband-violating writes rather than adjusting the complementary setpoint.

However, I would prefer not to introduce additional Matter writes for every device to accommodate that behavior.

A cleaner approach would be to use the specification-defined single-write behavior by default and investigate a compatibility fallback for servers demonstrably requiring it.

Such a fallback would need careful handling of write responses, current device state, endpoint-specific constraints, and partial failures.

Suggested approach

For the generic Matter Thermostat driver, I would recommend:

1. Validate the requested setpoint against its own effective limits, using endpoint-specific metadata.
2. Do not reject the request solely because it violates the deadband relative to a cached complementary setpoint.
3. Send a single write to the requested Matter attribute.
4. Let the Thermostat server enforce the deadband and adjust the complementary setpoint.
5. Update both SmartThings capabilities from the resulting device reports rather than assuming either write's outcome.

This would simplify the driver, avoid unnecessary state modifications, and keep responsibility for enforcing Matter's thermostat constraints where the specification places it.

The existing test coverage could then be extended to verify that a setpoint request crossing the cached deadband still results in exactly one Matter write, without any client-side modification of the complementary setpoint.

My concern is specifically about introducing additional client-side state management where the Matter server already has the necessary information and responsibility.

[kshypachov]

Hi @ldeora

Thank you for the detailed review. You are right, I will change the PR to a single write.

The main argument for me is the stale cache. In my forum reply I wrote that the current driver depends on the same cache, but that was inaccurate. With a stale cache, the current driver can only reject a request by mistake, without changing anything on the device. My version can write a setpoint calculated from stale data to the device and overwrite a value set by another controller. That is worse.

My device moves the other setpoint by itself: a single write of OccupiedHeatingSetpoint = 2200 with heating at 17.70 °C and cooling at 20.40 °C resulted in heating 22.00 °C and cooling 24.50 °C.

What I plan to change:

• validate the requested value only against its own limits;
• send a single write of the requested attribute;
• remove the calculation of the other setpoint. The AutoMode check goes away from the handler together with it, so Matter Thermostat mode discovery state and Auto feature detection are not endpoint-specific #3265 no longer affects this path. I will leave Matter Thermostat setpoint limits and deadband are shared across thermostat endpoints #3264 out of the scope of this PR;
• tests: a request crossing the deadband in either direction results in exactly one write, limit validation, both capabilities are updated from the device's reports.

Two clarifications:

1. The second write in the PR was not sent for every device, only with AutoMode and when the request crossed the deadband relative to the cache.
2. There may be quite a few devices that reject such a write: the stock thermostat server in the connectedhomeip v1.4 branch, which esp-matter uses, returns InvalidValue (thermostat-server.cpp). Moving the other setpoint only appeared in 1.5. The change will not make things worse for such devices, since they cannot move the setpoint now either, but the bug will remain for them. A fallback based on the write response is not easy: the default dispatcher in lua_libs passes only REPORT_DATA and COMMAND_RESPONSE to the handlers, WRITE_RESPONSE does not reach them. I would leave it for later.

One question. If the device rejects the write, there is no event, and the slider in the app stays where it was moved. On a local rejection the current driver emits the cached value, and the slider goes back. I saw this on my hub when the hub answered FAILURE to a write: the slider stayed in place until the device was reopened. I suggest reading the requested attribute after the write: the response arrives as a report, and the app shows the real value. What do you think?

I will update the PR after testing on my hub.