Skip to content
34 changes: 31 additions & 3 deletions src/app/clusters/thermostat-server/thermostat-server-presets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -302,10 +302,38 @@ Status ThermostatAttrAccess::SetActivePreset(EndpointId endpoint, DataModel::Nul
return Status::InvalidInState;
}

// If the preset handle passed in the command is not present in the Presets attribute, return INVALID_COMMAND.
if (!presetHandle.IsNull() && !IsPresetHandlePresentInPresets(delegate, presetHandle.Value()))
if (!presetHandle.IsNull())
{
return Status::InvalidCommand;
PresetStructWithOwnedMembers matchingPreset;
// Look up the preset once and reuse it to apply any setpoints it carries.
if (!GetMatchingPresetInPresets(delegate, presetHandle, matchingPreset))
{
return Status::InvalidCommand;
}

Optional<int16_t> coolingSetpointValue = matchingPreset.GetCoolingSetpoint();
if (coolingSetpointValue.HasValue())
{
Status status =
OccupiedCoolingSetpoint::Set(endpoint, EnforceCoolingSetpointLimits(coolingSetpointValue.Value(), endpoint));
if (status != Status::Success)
{
ChipLogError(Zcl, "Failed to set OccupiedCoolingSetpoint with status %u", to_underlying(status));
return status;
}
Comment on lines +331 to +337
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I addressed the re-entrancy concern directly in SetActivePreset by adding an internal guard for preset-application scope.
When setpoint writes triggered during preset application cause a callback-driven SetActivePreset(nullptr), that nested clear is now ignored, so we avoid transient clears/extra writes while preserving normal explicit clear behavior outside this flow.

I also kept the existing error propagation behavior (StatusIB(...)) for lookup/write failures.

}

Optional<int16_t> heatingSetpointValue = matchingPreset.GetHeatingSetpoint();
if (heatingSetpointValue.HasValue())
{
Status status =
OccupiedHeatingSetpoint::Set(endpoint, EnforceHeatingSetpointLimits(heatingSetpointValue.Value(), endpoint));
if (status != Status::Success)
{
ChipLogError(Zcl, "Failed to set OccupiedHeatingSetpoint with status %u", to_underlying(status));
return status;
}
}
Comment on lines +328 to +350
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This re-entrancy path is now guarded in SetActivePreset: while applying a non-null preset, we increment a scoped presetApplicationDepth, and any callback-driven SetActivePreset(..., nullopt) during that window returns early.
So setpoint writes can still trigger the attribute-changed callback, but they no longer clear ActivePresetHandle mid-application (including deadband-adjustment cascades), avoiding transient clears and extra churn/version bumps.

}

CHIP_ERROR err = delegate->SetActivePresetHandle(presetHandle);
Expand Down
Loading