search

dbuezas / eq3btsmart

58 stars 9 forks source link

fix: display target temperature immediately #78

Closed EuleMitKeule closed 4 months ago

EuleMitKeule commented 4 months ago

This PR changes the behaviour of the target temperature in such a way that the UI control will no longer reset to the previous target temperature after a new temperature has been selected. This is especially useful for cases where the thermostat does not immediately apply the requested temperature due to latency or connection issues and works very well together with the CurrentTemperatureSelector.DEVICE setting.

Closes #77

EuleMitKeule commented 4 months ago

@dbuezas I added error handling, so that when the temperature could not be set the previous temperature will be restored. This should prevent the issue you described in #77 while still making the UI representation more intuitive and responsive. To my understanding the way climate entities are supposed to work is that the current temperature displays the (last known) actual value the temperature sensor / valve / etc. has announced and the target temperature should always hold the value the user wants the climate control to apply. With this PR together with setting the current temperature mode to CurrentTemperatureSelector.DEVICE this is exactly the case.

dbuezas commented 4 months ago

Does it update if the temperature is changed in the thermostat itself?

EuleMitKeule commented 4 months ago

Yes, when an update is received from the thermostat due to manual changes on the hardware the new reported temperature is then set as the target temperature in the entity as well as the current temperature in case the CurrentTemperatureSelector.DEVICE mode is selected. The only part that is changed is when a new target temperature is set via Home Assistant.

dbuezas commented 4 months ago

Could you put this behind an option (like for current temperature)? I prefer the current behavior, otherwise It's impossible to tell if the command got through quickly

EuleMitKeule commented 4 months ago

Could you put this behind an option (like for current temperature)? I prefer the current behavior, otherwise It's impossible to tell if the command got through quickly

Yes, I can definetely do that. What would you say should be the default selection? I would vote for the new mode, since it most closely resembles what Home Assistant intended for the climate UI and is in my opinion more intuitive than the target temperature resetting right after it was altered by the user.

EuleMitKeule commented 4 months ago

There is now a new config option where the target temperature mode can be selected to either display the latest user input (default) or the latest reported value by the thermostat.

grafik

dbuezas commented 4 months ago

Very cool, thanks! You've convinced me about the default.

Le me know if you need to do some more changes and I'll merge

EuleMitKeule commented 4 months ago

I don't see anything that would have to be changed about this specific topic, so I would say you can merge. Thank you!