Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set evohome temperature, mode, preset doesn't behave as expected #25984

Closed
bennealon opened this issue Aug 16, 2019 · 10 comments
Closed

Set evohome temperature, mode, preset doesn't behave as expected #25984

bennealon opened this issue Aug 16, 2019 · 10 comments
Assignees

Comments

@bennealon
Copy link

Home Assistant release with the issue: 0.97.2

Last working Home Assistant release (if known): Unknown, new user.

Operating environment (Hass.io/Docker/Windows/etc.): Python Virtual Environment on Raspberry Pi 3 B+: Raspbian Buster

2015 Honeywell Evohome Connected thermostat WiFi, including 4 HR92UK Radiator Controllers, and the Hot Water Kit.

Component/platform: honeywell evohome

Description of problem:
When you try to simultaneously set Target temperature, Preset and/or Operation using the HA interface for a Honeywell Evohome Connected thermostat: as you change each value in turn an individual request appears to be sent to the Honeywell API. The value on the UI then immediately jumps back to the old one, and the relevant update is sent to the thermostat.

UI example work through for changing temperature and preset:

UI Action Evohome Result
Initial UI: Target-16, Operation-Heat, Preset-None Initial: Target-16, Operation-Heat, Preset-None
Only Change Temperature to 21 degrees on UI Target-21, Operation-Heat, Preset-Permanent
Then change Preset to Temporary on UI Target-16, Operation-Heat, Preset-Temporary
  • Note: in the above table, when we changed the UI temperature, it also submitted a faux Preset=permanent, when the UI was showing as Preset=None. Then changing the Preset on the UI submitted the correct Preset, but the incorrect temperature got sent due to the UI reverting back.
  • Doing these steps in any other order also yields unexpected results.
  • If the above changes are done with enough time for the UI to properly poll the evohome API, the second call to set the Preset=temporary contains the correct temperature.

Adding a Submit/Send button, rather than making the UI automatically send on change would improve the User Experience for a multiple field submission.

It would also be nice to have the default Preset mimick what the evohome controller (through the API) is reporting, ie if it is currently following a schedule, the default evohome behaviour on setting a new temperature (through their app) is to set the temperature on a temporary basis, ie until the next set point.

Problem-relevant configuration.yaml entries and (fill out even if it seems unimportant):

evohome:
  username: !secret evohome_username
  password: !secret evohome_password
  scan_interval: 180

Traceback (if applicable): No error's logged.

Additional information:
Related to #25400

@zxdavb
Copy link
Contributor

zxdavb commented Aug 17, 2019

[EDITED]

In summary, there are two issues when changing the target temperature for a Zone.

Firstly, if you make a change, and immediately make another change, the first change may get lost, or other unexpected changes may occur.

This is a convergence problem (an issue with all internet-polled components). It has been greatly ameliorated with #26042 (the convergence was minutes before), but there remains a window of approx 3-5 seconds where these issues can still occur.

I have a solution in mind, but it is WontFix on the basis of effort vs reward.

Secondly (fixed in #26810), things are not behaving as expected when a Zone's preset mode is not Permanent (known as PermanentOverride to evohome) before the target temperature is changed:

  • if the Zone is in FollowSchedule orTemporaryOverridemode, then
  • setting a new temperature for the Zone switches it's preset mode to Permanent in HA!

The expected behaviour would be it to behave as per the Vendor's UI/UX:

  • if in FollowSchedule mode, then switch to TemporaryOverride until next scheduled setpoint, or
  • if in TemporaryOverride mode, stay in that mode with the existing duration

Note that HA only allows you to set a target temperature, and not a duration (there is currently no means to do this in HA). To be clear, there is currently (HA v0.100) no way to set a target temperature for a set duration of time and have the Zone automatically revert to the scheduled target temperature.

[ORIGINAL POST]

@bennealon thanks for submitting this very useful post - I will look at it ASAP, probably after I come up with a solution for #25400.

I am sure I tested this - will have another look and get back to you here.

@zxdavb zxdavb self-assigned this Aug 17, 2019
@zxdavb
Copy link
Contributor

zxdavb commented Aug 20, 2019

Work on this is on hold until after #26042 is merged, as resolving this issue will be made easier by that PR.

@zxdavb
Copy link
Contributor

zxdavb commented Sep 14, 2019

@bennealon The async PR is now part of 0.99 beta, would you mind re-checking/confirming this is still an issue.

@bennealon
Copy link
Author

bennealon commented Sep 17, 2019

@zxdavb ive changed my local install to run Hass.io through docker (running on Buster), do you know how i can update to get the beta to give this a try?

@bennealon
Copy link
Author

bennealon commented Sep 20, 2019

@zxdavb Hi David,
I've updated HA to 0.99.0 and given the evohome asyncio library a go. So far seems better to use than the previous implementation, faster to make changes and the changes appear to sync up better than before. I especially noticed that the preset and heat values are correctly populated with the correct status' when entering the thermostat info page, something which had blank values in the previous implementation 👍 . however part of the above issue still persists - if i make a single change to the temperature ONLY, the Preset incorrectly gets forced to permanent, despite it already showing as temporary on the UI.
Heres a walkthrough of the reproduction steps:

UI Action Evohome Result
Initial UI: Target-19, Operation-Heat, Preset-temporary Initial: Target-19, Operation-Heat, Preset-temporary
Only Change Temperature to 21 degrees on UI (UI is showing preset as temporary, which is correct Target-21, Operation-Heat, Preset-Permanent

After this change, when the UI next updates it updates the preset to now show Permanent. Which is correct, as it has updated to show the correct status from evohome: but i did not request this change of preset.

I'm presuming here that the call to the evohome API also contains the incorrect preset value, and should re-send the same value that the UI correctly displays?

Manually changing the preset to temporary does fix this issue, but means that a temperature change still requires two steps to maintain default evohome application behaviour.

EDIT: I tried the above steps again, changing the temperature first then immediately changing the preset afterwards within the same info screen: and the temperature change got ignored in favour of the preset

@bennealon
Copy link
Author

I've also just noticed that we now get green-shading under the graphs whilst the system is heating, you just made me a happy chappy 😁 👍
Really do appreciate the work you have put into this changeset, thank you very much!

@zxdavb
Copy link
Contributor

zxdavb commented Sep 20, 2019

EDIT: I tried the above steps again, changing the temperature first then immediately changing the preset afterwards within the same info screen: and the temperature change got ignored in favour of the preset

This is a convergence problem (an issue with internet polled components). I have a solution in mind, but it is WontFix for now.

This other issue is a tricky one, for boring reasons I won't go into... I think a fully correct solution is a bit too much like hard work...

OK, here is the relevant HA code (note what happens if until=None):

async def async_set_temperature(self, **kwargs) -> None:
    until = kwargs.get("until")
    if until:
        until = parse_datetime(until)
    await self._set_temperature(kwargs["temperature"], until)

... and the fix is (probably):

async def async_set_temperature(self, **kwargs) -> None:
    if self._evo_device.setpointStatus["setpointMode"] == EVO_PERMOVER:
        until = kwargs.get("until")
    else:
        until = kwargs.get("until", self.setpoints["next"]["from"])
    await self._set_temperature(kwargs["temperature"], parse_datetime(until))

This won't be perfect, but I feel a fair balance.

@zxdavb zxdavb mentioned this issue Sep 21, 2019
9 tasks
@bennealon
Copy link
Author

bennealon commented Sep 26, 2019

EDIT: I tried the above steps again, changing the temperature first then immediately changing the preset afterwards within the same info screen: and the temperature change got ignored in favour of the preset

This is a convergence problem (an issue with internet polled components). I have a solution in mind, but it is WontFix for now.

This other issue is a tricky one, for boring reasons I won't go into... I think a fully correct solution is a bit too much like hard work...

OK, here is the relevant HA code (note what happens if until=None):

--SNIP--

This won't be perfect, but I feel a fair balance.

this seems fair David 👍

@zxdavb zxdavb changed the title Evohome unable to set target temperature and preset values simultaneously. Set evohome temperature, mode, preset doesn't behave as expected Sep 28, 2019
@zxdavb
Copy link
Contributor

zxdavb commented Sep 28, 2019

I have edited my first post, above - please refer to that.

balloob pushed a commit that referenced this issue Oct 1, 2019
* address issues #25984, #25985

* small tweak

* refactor - fix bugs, coding erros, consolidate

* some zones don't have schedules

* some zones don't have schedules 2

* some zones don't have schedules 3

* fix water_heater, add away mode

* readbility tweak

* bugfix: no refesh after state change

* bugfix: no refesh after state change 2

* temove dodgy wrappers (protected-access), fix until logic

* remove dodgy _set_zone_mode wrapper

* tweak

* tweak docstrings

* refactor as per PR review

* refactor as per PR review 3

* refactor to use dt_util

* small tweak

* tweak doc strings

* remove packet from _refresh

* set_temp() don't have until

* add unique_id

* add unique_id 2
@zxdavb
Copy link
Contributor

zxdavb commented Oct 3, 2019

Fixed by #26810.

@zxdavb zxdavb closed this as completed Oct 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants