architecture
architecture copied to clipboard
generic thermostat (away_mode and restore state) need some attention
There are a couple of opened issues and PR about current and desired behavior of generic thermostat. Issue #20154 started by @b4dpxl shows current and invalid bahiaviour. @MartinHjelmare wrote that ~~this platform needs some rethinking~~ the discussion should move to an architecture issue. also @ivankravets opened this issue again related to generic thermostat
Id like to start discussion about generic thermostat and its current and desired behaviour.
As @Kedryn and @b4dpxl wrote: away temp must be superior, every change that is done via automation can't alter that temperature. Automation should update target temperature and when away is turn off that target temperature should be used. Am I right guys? Please correct me if I'm wrong.
Also when restoring after restart generic thermostat should restore away_mode and temperatures (#21125)
I didn't write that this platform needs some rethinking. I wrote that the discussion should move to an architecture issue. Those two sentences are not the same.
@MartinHjelmare I am sorry for me misunderstanding Your intentions. I've corrected my initial post.
I naively thought that "climate 1.0" might be when there was an attempt to resolve some of these issues, especially as it's an "architectural" update, but as my question about that on https://github.com/home-assistant/home-assistant/pull/23899 was marked as "off topic", it looks like "climate 1.0" is just carrying over the current issues. Hopefully with an intention to actually look at resolving the issues after. Not helped that the PR to partially fix some of the issues was closed as stale. Once again it appears as though generic_thermostat is the poor cousin no one wants to talk about.
IMHO this depends on the thermostat itself. Some allow changing temps, even when a preset (or away mode) is active, some don't. With the Climate 1.0 change, it is up to the integration itself if it accepts new temperature settings when the away preset is active.
@frenck I agree. I even submitted a PR to that effect to address https://github.com/home-assistant/home-assistant/issues/20154 which got rejected (https://github.com/home-assistant/home-assistant/pull/20255#pullrequestreview-209397798) though I don't think the rejector actually fully understood the issue. They kept referring to changing the away temperature, which was exactly what I was trying not to do.
Generic_thermostat has no logic of it's own (except basic "too hot, turn on/too cold, turn off") because it's just controlling switches directly based off an HA temperature sensor, unlike using HA to control a "real" thermostat such as Hive. The name is probably wrong. It would be more accurate to call it a "homeassistant_thermostat" or something similar.
Maybe that's part of the misunderstanding? The assumption that generic_thermostat is to control a thermostat, rather than to be a thermostat?
For that specific PR you are linking, that is the non-"climate 1.0" approach. For 1.0 I would personally opt for a configuration flag like allow_override_preset_temperature (<- obviously needs a better name).
I think the generic_thermostat is a piece of history and, to be honest, I'm not quite sure it fits in our general architecture anymore. I think it should be converted into template thermostat (or add template based thermostat and deprecate/remove the generic). The climate platform is one of the key platforms that is missing a template variant.
However, that last part is a bit out of scope for this discussion.
@frenck I'd like to ask for leaving the generic_thermostat or at least something that could be used for a genetic thermostat. I don't use any thermostat from the market (like Hive) but I want to control the temperature using relays and DS18B20 sensors.
I had one based on ESP8266, but now I want to move everything to HA. Reading temperature is easy, controlling the relays isn't a problem, but I wanted to avoid the need to build custom logic and use a component that already exists.
My intention when creating this issue was to provoke a dialog that would allow settling what is desirable behavior and how things should work. This way related issues could be fixed, right now everything is stopped because we don't have a clear description of how things should work.
I've noticed that Climate 1.0 has an open PR (https://github.com/home-assistant/home-assistant/pull/23899), maybe all the issues mentioned here could be addressed? This would help us a lot.
For that specific PR you are linking, that is the non-"climate 1.0" approach. For 1.0 I would personally opt for a configuration flag like
allow_override_preset_temperature(<- obviously needs a better name).
which is fine, but at the time there was no mention of "climate 1.0", and the rejection reasons were "unhelpful". Although the rejector created their own PR, this didn't actually address the issue. Moot point though.
I think the
generic_thermostatis a piece of history and, to be honest, I'm not quite sure it fits in our general architecture anymore. I think it should be converted into template thermostat (or add template based thermostat and deprecate/remove the generic). The climate platform is one of the key platforms that is missing a template variant.
Fair enough, as long as that would allow us to implement a thermostat based on HA temperature sensor and a basic switch. I currently use a Shelly 1 running Tasmota and previously used a Sonoff SV to actually control the boiler. That's about as dumb as you can get. HA has the "brains" to say when to switch on and off. This model is also easy to extend to create simple zone-based heating with dumb powered radiator valves (which is my intention ready for next winter).
This model is both more flexible and significantly more cost effective than the commercially available offerings, and was actually what triggered me to deploy HA in the first place. Why give Nest/Hive/whoever £100's when I can do something better and more securely (IMO) for <£20.
How would you envisage a template thermostat working. It might be something I could investigate implementing, if I had an idea of what would be architecturally acceptable.
I've noticed that Climate 1.0 has an open PR (home-assistant/home-assistant#23899), maybe all the issues mentioned here could be addressed? This would help us a lot.
@Misiu that was the PR that when I asked about these issues my comment got marked "off topic" :|
I've noticed that Climate 1.0 has an open PR (home-assistant/home-assistant#23899), maybe all the issues mentioned here could be addressed? This would help us a lot.
@Misiu that was the PR that when I asked about these issues my comment got marked "off topic" :|
@frenck could You take a look at referred PR? I think this is the ideal moment to settle on how away_mode should work.
Those are marked off topic, because you are trying to open up a discussion on an implementation which was decided upon and is not open for discussion.
How away_mode works is decided and documented, extensively discussed and done. Hence the implementation of Climate 1.0 is taking place now. There is no away mode anymore, there is a preset mode.
I feel like you guys are missing a bit of context here at this point and using past decisions against the old architecture to get something on the new architecture. Which, IMHO, doesn't make sense since, well, it drastically changed and the new architecture is actually flexible enough to support your wishes.
Please read up here: https://developers.home-assistant.io/blog/2019/07/03/climate-cleanup.html
This is turning into a chat and is no longer an architectural proposal. I would suggest continuing this discussion on a platform that was created for that: Discord chat.
@frenck I read that blog, and I don't care whether it's called away_mode, preset mode, or whatever. The issue, which as far as I can tell will still remain after Climate 1.0, is that generic_thermostat will have functional issues. It's not a platform architectural issue, it's a functional issue with the way generic_thermostat handles requests to change temperature during different modes, and the changes pushed to generic_thermostat so far do not address this.
I might be mistaken of course, hence me asking the question. At this point it feels like our best option is to wait for Climate 1.0, then reopen the issues if they still exist.
I don't care whether it's called away_mode, preset mode, or whatever.
The point is, we do care. Since that is where all the tech dept around the climate platform started in the first place.
It's not a platform architectural issue, it's a functional issue with the way generic_thermostat handles requests to change the temperature during different modes, and the changes pushed to generic_thermostat so far do not address this.
As pointed out in my first response on this request that depends on the thermostat. Which means, it is currently NOT wrongly implemented. You expected a different behavior.
Instead of trying to adding optional behavior, you are trying to push a breaking change and force behavior and using the Climate 1.0 as a hook to get it done. That is not how it works.
At this point it feels like our best option is to wait for Climate 1.0, then reopen the issues if they still exist.
That is not an option, that is mandatory. All changes to climate platform integrations are blocked until the Climate 1.0 implementation has been completed.
Climate 1.0 is not addressing your issue, however, it opens up more flexibility for the future.
As pointed out in my first response on this request that depends on the thermostat. Which means, it is currently NOT wrongly implemented. You expected a different behavior.
Yes, I expected the thermostat to function in a usable way. At current generic_thermostat is not fully usable without what should be unnecessary workarounds (like automations having to check if "away_mode" is currently enabled before setting the scheduled temperature).
Instead of trying to adding optional behavior, you are trying to push a breaking change and force behavior and using the Climate 1.0 as a hook to get it done. That is not how it works.
And my change was rejected. I accepted that. I merely asked if Climate 1.0, which came about after the many issues raised against generic_thermostat, would resolve those issues. I hardly think that's unreasonable, and I wasn't trying to "force" anything. I have offered to try and develop a replacement that would hopefully resolve the issues and meet the architectural requirements, but I need guidance on what is acceptable.
Those of us who use generic_thermostat have concerns about it. Not helped when one of the developers openly admits to not caring about the component. https://github.com/home-assistant/home-assistant/pull/20255#issuecomment-470884516
@frenck could You take a look at this approach: https://github.com/b4dpxl/virtual_thermostat it is based on Climate 1.0. I'm with @b4dpxl on that. Maybe now, with Climate 1.0 released, we can settle on the desired behavior.
-there should be an option to prevent setting the temperature in away mode. This could be set by default to false so it won't break anything - @b4dpxl what do You think? -the saved temperature should be saved and reverted when HA restarts
Instead of creating multiple integrations doing almost one thing we should settle on functionality and continue working on one integration, making it even better.
-there should be an option to prevent setting the temperature in away mode. This could be set by default to false so it won't break anything - @b4dpxl what do You think?
Providing there is a way to set the desired "non-away" temperature. When in away mode, I still want the correct scheduled temperature to be recorded somehow, so that the correct temperature is reverted to once normal mode resumes. E.g. if the preset mode is set to away during the day, I don't want the temperature to revert to the higher daytime temperature if I return late at night when it should be low.
My virtual_thermostat does this by updating and persisting the saved_target_temperature when the preset mode is away. I did think about adding a set_scheduled_temperature service, but there was something about "not adding new services" when I last checked the dev guides.
This architecture issue is old, stale, and possibly obsolete. Things changed a lot over the years. Additionally, we have been moving to discussions for these architectural discussions.
For that reason, I'm going to close this issue.
../Frenck