search

home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
74.01k stars 31.05k forks source link

Generic Thermostat - cycles if both tolerances are zero #79667

Open borpin opened 2 years ago

borpin commented 2 years ago

The problem

Generic Thermostat - cycles if both tolerances are zero and the current temperature = target temperature.

I am using the Generic Thermostat to control my UFH. I want the thermostat to turn the heating off when at target temp, and turn it on when it falls below target temp by 0.1 degrees.

https://github.com/home-assistant/core/blob/41d2ab5b375564ccbc836736fa8896a758cffc2e/homeassistant/components/generic_thermostat/climate.py#L479-L480

If the hot and cold tolerances are 0 (zero), both too_cold and too_hot evaluate to True when current temp = target temp.

This results in the thermostat cycling (while current temp = target temp) at the min cycle rate.

https://github.com/home-assistant/core/blob/41d2ab5b375564ccbc836736fa8896a758cffc2e/homeassistant/components/generic_thermostat/climate.py#L482-L482

https://github.com/home-assistant/core/blob/41d2ab5b375564ccbc836736fa8896a758cffc2e/homeassistant/components/generic_thermostat/climate.py#L493-L493

Example: - if the room is currently at 20.5 with heating off and a schedule sets the target temperature to 20.5, the heating will needlessly come on.

What version of Home Assistant Core has the issue?

2022.9.7

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant OS

Integration causing the issue

Generic Thermostat

Link to integration documentation on our website

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

Diagnostics information

No response

Example YAML snippet

- platform: generic_thermostat
  name: sunroom
  heater: switch.tasmota4
  target_sensor: sensor.sunroomtemp
  min_temp: 18
  max_temp: 22
  cold_tolerance: 0.0
  hot_tolerance: 0.0
  precision: 0.1
  min_cycle_duration:
    minutes: 10
  initial_hvac_mode : "heat"
  away_temp: 16

Anything in the logs that might be useful for us?

No response

Additional information

Solution: add an extra condition such that if both too_hot and too_cold are true, the device is always switched off or if already off no action.

homeassistant commented 2 years ago

generic_thermostat documentation generic_thermostat source

borpin commented 2 years ago

Anyone?

borpin commented 2 years ago

Anyone? 😄

borpin commented 2 years ago

Just preventing this going stale.

borpin commented 2 years ago

Thoughts?

borpin commented 2 years ago

Please don't go stale

borpin commented 1 year ago

Still an issue

issue-triage-workflows[bot] commented 1 year ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

borpin commented 1 year ago

Still an Issue

https://github.com/home-assistant/core/blob/e904edb12e7ff5b96ee86741fcb52bffd72fc498/homeassistant/components/generic_thermostat/climate.py#L484

issue-triage-workflows[bot] commented 1 year ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

borpin commented 1 year ago

This is still an issue as it hasn't been fixed.

rrooggiieerr commented 1 year ago

@borpin What would be your proposed solution?

borpin commented 1 year ago

What would be your proposed solution?

@rrooggiieerr, both too_hot and too_cold need to be false when target = curr and both tolerances are zero.

For compatibility (as I'm probably slightly edgecase having tolerances of 0) and excuse my pseudocode.

if self._cold_tolerance = 0.0 then
  too_cold = self._target_temp > self._cur_temp
else
  too_cold = self._target_temp => self._cur_temp + self._cold_tolerance 
end if

If self._hot_tolerance = 0.0 then
  too_hot = self._cur_temp > self._target_temp
else
  too_hot = self._cur_temp => self._target_temp + self._hot_tolerance
end if

If then a user adds a tolerance of 0.1 to either, the thermostat will trigger even if target = curr (with precision of 0.1), and equally, with a tolerance of 0 it will trigger as when target /= curr by 0.1.

However, when target = curr and tolerances are 0, both tool_cold and too_hot will be false (as it should be).

Anyone who had it set as zero and was quite happy potentially needs to add a minimum value at their precision. Chances are they had not noticed the cycling.

rrooggiieerr commented 1 year ago

Why not make the hot and cold tolerances 0.1 if they are 0.0?

if self._cold_tolerance == 0.0:
  self._cold_tolerance = 0.1
if self._hot_tolerance == 0.0:
  self._hot_tolerance = 0.1
borpin commented 1 year ago

Had a big think about this 😄

Why not make the hot and cold tolerances 0.1 if they are 0.0?

Your solution does assume a step of 0.1 though and it would have to be heavily comment that to explain why it has been added, whereas the other code is more verbose so should be apparent why it is there. Seems too much of a kludge to me.

However, in running through the scenarios, I realised that I had missed a very important state - just_right 😄 This is where I came unstuck last time I tried to solve the cycling.

There is an implicit assumption in the code currently that a room is always either too_hot or too_cold (one is TRUE and the other is FALSE). This is a false assumption. There are a number of combinations (negative tolerances) where both can be true, i.e. the room is just_right.

Is this statement true - At just_right the device should always be off (heating or cooling)?

If it is (and I think it is), then the solution is that if both too_hot and too_cold are TRUE, then the device should be switched off. What the tolerances actually are is irrelevant.

If so, the solution is to test for this first, and if true then if the device is on, switch it off else do nothing.

https://github.com/home-assistant/core/blob/be2cee228cc50cd9558a80f7a56b1d5b515ec03b/homeassistant/components/generic_thermostat/climate.py#L495-L498

          if (too_cold and too_hot):
              if self._is_device_active: 
                _LOGGER.info("Turning off heater %s", self.heater_entity_id)
                await self._async_heater_turn_off()
          elif self._is_device_active:
              if (self.ac_mode and too_cold) or (not self.ac_mode and too_hot):
rrooggiieerr commented 1 year ago

I still have to think about your proposed solution, I agree that code should be self explanatory. But in the meantime I have one question for you. How often is it going to happen that the temperature is exactly right?

By the time you have powered on your heater there will be energy stored in whatever kind of medium your heater uses, water/air/oil/etc, and this energy will be released over time. Also when you have already powered off your heater. There will always be some under/over shoot. The whole idea of having this tolerance is to create a certain bandwidth wherein your heater operates. A tolerance of 0 is unrealistic.

borpin commented 1 year ago

How often is it going to happen that the temperature is exactly right?

I have a near PassiveHaus standard build with a low temperature UFH system. The temperature is often very stable. A tolerance of 0 at a step of 0.1, means it will be triggered 'on' if the temp drops 0.1 degrees below target and go off again when the 2 temps match (fine so far). The issue is that, it should not then come on until the temp drops again. In fact, it comes on again because the temps are the same.

A graph of the cycling (target 21 min cycle 10 mins). The first on shouldn't happen as it has simply dropped to target. You can also see the cycling eventually pushes it above target - i.e. overheats. This is a good example as there was no one in the property at this time so no other factors except the UFH.

image

Fundamentally, there is an error in the state machine for this integration in that it does not account for the state of just_right.

issue-triage-workflows[bot] commented 9 months ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

borpin commented 9 months ago

Still an issue that should be addressed.

issue-triage-workflows[bot] commented 6 months ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

borpin commented 6 months ago

Still an issue. I have an automation to override the switch on when target temp has been reached.

issue-triage-workflows[bot] commented 3 months ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

borpin commented 3 months ago

The logic is still not correct.

wltng commented 1 month ago

Looking after another issue, I came across this one. Isn't what you want a hot_tolerance of 0 and a cold_tolerance of 0.1? This means heating until: ambient >= set temperature +0 . Then not heating until: set temperature >=(ambient + 0.1)

Besides: The fact that too_hot and too_cold are true at the same time doesn't matter, since the code also looks at wether you're in AC mode (cooling) So when (cooling and too_cold) OR (heating and too_hot): turn off cooler/heater etc.

borpin commented 1 month ago

Isn't what you want a hot_tolerance of 0 and a cold_tolerance of 0.1?

No, because it only then changes when the temperature reaches 0.2 beyond the tolerance (if you do this hot or cold).

It is a bug - programmatically, the logic is flawed.