Generic Thermostat min_cycle_duration not taken into account

ovidiulaz7 commented 2 years ago

The problem

If something else turned on or off the switch defined under the 'heater' property in the thermostat configuration, the thermostat won't turn it off after the min_cycle_duration if the 'sensor' will report above the setpoint, or won't turn it on if the sensor reports it under the setpoint.

What version of Home Assistant Core has the issue?

core-2021.12.10

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant Supervised

Integration causing the issue

Generic Thermostat

Link to integration documentation on our website

https://www.home-assistant.io/integrations/generic_thermostat/

Example YAML snippet

- platform: generic_thermostat
  name: Kid Room
  heater: group.floor_heating_group
  target_sensor: sensor.kid_temperature
  hot_tolerance: 0.3
  cold_tolerance: 0.0
  min_cycle_duration:
    minutes: 10
  away_temp: 20
  precision: 0.1

Anything in the logs that might be useful for us?

No response

Additional information

No response

probot-home-assistant[bot] commented 2 years ago

generic_thermostat documentation generic_thermostat source _{^{(message by IssueLinks)}}

javicalle commented 2 years ago

Is the thermostat operating when the switch changes state? The generic_thermostat entity will not control anything if the entity is not running.

ovidiulaz7 commented 2 years ago

Yes, sorry. Forgot to mention, so the thermostat is in heating state. The thermostat state changes from Idle to Heating or vice-verse but it won't take action after that min_cycle_duration passed.

ovidiulaz7 commented 2 years ago

I might have to add, if the temperature doesn't change from above or below set point. e.g. temp is above when switch is triggered, and it stays above, (doesn't reach below setpoint), then the thermostat won't take action on the switch after min_cycle_duration.

javicalle commented 2 years ago

Ummmm, I'm not sure if the meaning of the min_cycle_duration attribute has been misunderstood:

https://www.home-assistant.io/integrations/generic_thermostat#min_cycle_duration

Maybe a better definition can be:

Set a minimum amount of time that the switch specified in the heater option must be in its current state prior to being switched either off or on again.

So, this attribute prevents thermostat from switch on/off too fast (because temp changes). But it does not define the time that it will be in the on/off state.

Would this point of view explain the behavior you have experienced in your case?

ovidiulaz7 commented 2 years ago

My expectation is, that if the switch changed state.. while the thermostat is active.. it should take action after min_cycle_duration time.

qulixus commented 2 years ago

Think I am having the same issue. If any manual change to the thermostat target temperature results in the system ignoring the min_cyle_duration and turning off my switch.

I have some auto balancing actuators on my system, and if the power is cut to them within 1 minute they factory rest for reconfiguration, so my min_cycle_duration is set to 2 minutes to be sure this doesn't happen, but isn't working.

Eg, I change temperature to say 21c, switch is turned on, 10 seconds later I change to 19c so target temp is met, switch is turned off straight away not 1min 50secs I would expect with a 2min min_cyle_duration.

javicalle commented 2 years ago

Think I am having the same issue. If any manual change to the thermostat target temperature results in the system ignoring the min_cyle_duration and turning off my switch.

I think it is not the same case that is being reviewed here. When the target temperature is changed, it is implemented that the min_cycle_duration is ignored: https://github.com/home-assistant/core/blob/2772437a2b0455c3b5dde92f6af58fb45b7206c5/homeassistant/components/generic_thermostat/climate.py#L388-L394

It is not my design, but I agree that if the user changes the temperature of the thermostat, the normal behavior would be that the switch react immediately and not after a time that is configured in the HA inners.

javicalle commented 2 years ago

@ovidiulaz7 Hi again, From what I've seen, the min_cycle_duration is reset on every switch change.

The code checks if the switch has been in the same state for at least min_cycle_duration time: https://github.com/home-assistant/core/blob/7cc6770f8356068a5d5b0149178fd9ac06c9bc10/homeassistant/components/generic_thermostat/climate.py#L478-L488

So, every time status change, the counter resets.

If this is not the behavior detected, it could be a bug that should be corrected. But if this is the behavior detected in your case, I would say that this is how it has been implemented. Obviously anyone can propose a PR to change the behavior that would be reviewed and approved or not.

ovidiulaz7 commented 2 years ago

Hi @javicalle Thanks for sharing the code. I'm not that familiar with python, but I believe I've found the issue.

The method _async_control_heating seems to be called when user actions takes place, like turning it on or off, away or not, setting the target.. and when the sensor temperature changes.

So, I believe that's my use-case, I have some bluetooth temperature sensors that won't send updates that often if the temperature didn't changed by some margin I believe, to preserve battery.

So, if the switch was turned off, the temperature is below target, but the sensor didn't get a new value after min_cycle_duration passed, the switch won't be turned on.

I hope you'll understand now..

Maybe we should schedule some sort of 'notification' based event after the last counter reset and if none of the events above occurs we should trigger another evaluation of the heating conditions.

Thank you again!

javicalle commented 2 years ago

The method _async_control_heating seems to be called when user actions takes place, like turning it on or off, away or not, setting the target.. and when the sensor temperature changes.

That's correct, the _async_control_heating only triggers because of those events (and not because an amount of time has elapsed)

So, if the switch was turned off, the temperature is below target, but the sensor didn't get a new value after min_cycle_duration passed, the switch won't be turned on.

In that case you would see a flat graph in the temperature of the thermostat history.

Maybe we should schedule some sort of 'notification' based event after the last counter reset and if none of the events above occurs we should trigger another evaluation of the heating conditions.

But there is already an attribute for that! (or so I think)

I'm not sure, but I thought the keep_alive attribute could be useful:

https://www.home-assistant.io/integrations/generic_thermostat/#keep_alive

If the keep_alive is settled, periodically the _async_control_heating will be evaluated: https://github.com/home-assistant/core/blob/7cc6770f8356068a5d5b0149178fd9ac06c9bc10/homeassistant/components/generic_thermostat/climate.py#L237-L242

And, if I have understood correctly, your use case would be solved.

ovidiulaz7 commented 2 years ago

Hmm, you're right, the keep alive might do the job, I took it more literally as the documentation said. I didn't knew actually that it will re-trigger the evaluation conditions.

Thanks! I'll try it and I'll come back with updates.

If set, the switch specified in the heater option will be triggered every time the interval elapses.

That means it will turn it on either way?

javicalle commented 2 years ago

That means it will turn it on either way?

No, just will 'trigger' the event to reevaluate the switch status.

ovidiulaz7 commented 2 years ago

Ok, seems like this works, at least when the temperature is below target.

I'll re-open if it's not the same for the above target case.

Thanks for the head's up!

home-assistant / core