Skip to content

Matter Thermostat setpoint limits and deadband are shared across thermostat endpoints #3264

Open

Description

@ldeora

Summary

The driver supports multi-thermostat devices (notably the heat-pump profile), but caches thermostat setpoint limits and MinSetpointDeadBand with device-wide set_field() keys. Reports from one thermostat endpoint can therefore overwrite the constraints used for another endpoint.

Validated against main commit 43f47501b217336d17ed2fc92b64d08971f305ce.

Affected code

attribute_handlers.lua lines 151-185 stores deadband and heat limits without the reporting endpoint:

function AttributeHandlers.min_setpoint_deadband_handler(driver, device, ib, response)
  local val = ib.data.value / 10.0
  device:set_field(fields.setpoint_limit_device_field.MIN_DEADBAND, val, { persist = true })
  device:set_field(fields.setpoint_limit_device_field.MIN_SETPOINT_DEADBAND_CHECKED, true, {persist = true})
end
...
device:set_field(minOrMax, val)
local min = device:get_field(fields.setpoint_limit_device_field.MIN_HEAT)
local max = device:get_field(fields.setpoint_limit_device_field.MAX_HEAT)

capability_handlers.lua lines 171-245 correctly resolves the target thermostat endpoint, but validates against the same device-wide fields:

local endpoint_id = thermostat_utils.component_to_endpoint(device, cmd.component, clusters.Thermostat.ID)
...
local deadband = device:get_field(fields.setpoint_limit_device_field.MIN_DEADBAND) or 2.5
...
local min = device:get_field(fields.setpoint_limit_device_field.MIN_HEAT) or MIN_TEMP_IN_C
local max = device:get_field(fields.setpoint_limit_device_field.MAX_HEAT) or MAX_TEMP_IN_C

The driver already contains endpoint-field helpers in thermostat_utils/utils.lua lines 22-28:

function ThermostatUtils.get_field_for_endpoint(device, field, endpoint)
  return device:get_field(string.format("%s_%d", field, endpoint))
end

function ThermostatUtils.set_field_for_endpoint(device, field, endpoint, value, additional_params)
  device:set_field(string.format("%s_%d", field, endpoint), value, additional_params)
end

Failure mechanism

Matter thermostat attributes are endpoint-scoped. The driver's cached constraints are not. On a device with two Thermostat server endpoints, the last limit/deadband report wins globally even though commands are routed to the correct endpoint.

Reproduction case

Use a composed/heat-pump device with two thermostat endpoints:

EP 2: AbsMinHeatSetpointLimit = 5 C,  AbsMaxHeatSetpointLimit = 30 C
EP 3: AbsMinHeatSetpointLimit = 15 C, AbsMaxHeatSetpointLimit = 25 C
  1. Report the EP 2 limits.
  2. Report the EP 3 limits afterwards.
  3. Send setHeatingSetpoint(10) to the SmartThings component mapped to EP 2.
  4. The command handler resolves EP 2 correctly, but reads the device-wide cached range last written by EP 3 (15-25 C).
  5. The valid EP 2 command is rejected locally.

MinSetpointDeadBand can be overwritten in the same way.

Expected behavior

Each thermostat component/endpoint must be validated against the limits and deadband reported by that same Matter endpoint.

Proposed fix

Store and retrieve thermostat constraints with the reporting/target endpoint rather than device-wide keys. The driver already provides get_field_for_endpoint() and set_field_for_endpoint() helpers, so this can be done without introducing a new storage abstraction.

For example:

function AttributeHandlers.min_setpoint_deadband_handler(driver, device, ib, response)
  if ib.data.value == nil then
    return
  end

  local val = ib.data.value / 10.0
  thermostat_utils.set_field_for_endpoint(
    device,
    fields.setpoint_limit_device_field.MIN_DEADBAND,
    ib.endpoint_id,
    val,
    { persist = true }
  )
  device:set_field(
    fields.setpoint_limit_device_field.MIN_SETPOINT_DEADBAND_CHECKED,
    true,
    { persist = true }
  )
end

The setpoint-limit handlers should likewise cache and combine limits on the reporting endpoint:

thermostat_utils.set_field_for_endpoint(device, minOrMax, ib.endpoint_id, val, { persist = true })

local min = thermostat_utils.get_field_for_endpoint(
  device, fields.setpoint_limit_device_field.MIN_HEAT, ib.endpoint_id
)
local max = thermostat_utils.get_field_for_endpoint(
  device, fields.setpoint_limit_device_field.MAX_HEAT, ib.endpoint_id
)

And command validation should use the endpoint already resolved from the SmartThings component:

local endpoint_id = thermostat_utils.component_to_endpoint(
  device, cmd.component, clusters.Thermostat.ID
)

local deadband = thermostat_utils.get_field_for_endpoint(
  device, fields.setpoint_limit_device_field.MIN_DEADBAND, endpoint_id
) or 2.5

local min = thermostat_utils.get_field_for_endpoint(
  device, fields.setpoint_limit_device_field.MIN_HEAT, endpoint_id
) or MIN_TEMP_IN_C
local max = thermostat_utils.get_field_for_endpoint(
  device, fields.setpoint_limit_device_field.MAX_HEAT, endpoint_id
) or MAX_TEMP_IN_C

Apply the same endpoint scoping to cooling constraints and to range events emitted by temperature_handler_factory().

If the configurable-limit issue is fixed at the same time, both the absolute (ABS_*) and configurable (MIN_* / MAX_*) fields should be endpoint-scoped, with configurable values taking precedence over absolute values.

Suggested regression test

Extend test_matter_heat_pump.lua so thermostatOne and thermostatTwo report different ranges/deadbands, then issue setpoint commands to both components and verify each command is validated only against its mapped endpoint's values.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions