core icon indicating copy to clipboard operation
core copied to clipboard

Integration Not Properly Handling Holds

Open jhemak opened this issue 1 year ago • 8 comments

The problem

I am using this integration with a RTH6580WF and seeing two unexpected behaviors when dealing with holds:

  1. When the thermostat is in a temporary hold, permanent_hold = false (as expected) but preset_mode = none. I would have expected preset_mode to be hold. Even though it's set to none, if I call the below the temporary hold is cleared (as expected). This is visible on the display of the thermostat itself (and in the Honeywell app) but there is nothing in HA that indicates a change has occurred. I would have expected preset_mode to change from hold to none.
action: climate.set_preset_mode
target:
  entity_id: climate.thermostat
data:
  preset_mode: none
  1. When the thermostat is in a permanent hold, permanent_hold = true (as expected) and preset_mode = hold (as expected). Calling the action above does not clear the hold. I would have expected preset_mode to change from hold to none and permanent_hold to change from true to false. Instead, the action appears to be ignored. *********** UPDATE: After further testing, I am no longer seeing issue 2. I believe I had some other issue ongoing when I made the observation above ***********

What version of Home Assistant Core has the issue?

core-2024.10.1

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

Honeywell Total Connect Comfort (US)

Link to integration documentation on our website

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

Diagnostics information

No response

Example YAML snippet

No response

Anything in the logs that might be useful for us?

No response

Additional information

No response

jhemak avatar Oct 06 '24 18:10 jhemak

Hey there @rdfurman, @mkmer, mind taking a look at this issue as it has been labeled with an integration (honeywell) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of honeywell can trigger bot actions by commenting:

  • @home-assistant close Closes the issue.
  • @home-assistant rename Awesome new title Renames the issue.
  • @home-assistant reopen Reopen the issue.
  • @home-assistant unassign honeywell Removes the current integration label and assignees on the issue, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the issue.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the issue.

(message by CodeOwnersMention)


honeywell documentation honeywell source (message by IssueLinks)

home-assistant[bot] avatar Oct 06 '24 18:10 home-assistant[bot]

Can you share any integration errors found in the logs? (May need to enable logging for the integration catch them)

mkmer avatar Oct 07 '24 13:10 mkmer

No errors in the debug logs (see below). However, I did find an error in my comments above. I have edited them to reflect what I am observing now. The main (only) issue now appears to be no way to detect when a temporary hold is active.

DEBUG (MainThread) [somecomfort] Sending Data: {'DeviceID': xxxxxxx, 'SystemSwitch': None, 'HeatSetpoint': None, 'CoolSetpoint': None, 'HeatNextPeriod': None, 'CoolNextPeriod': None, 'StatusHeat': 0, 'StatusCool': 0, 'FanMode': None}
DEBUG (MainThread) [somecomfort] Received setting response {'success': 1}
DEBUG (MainThread) [somecomfort] Sending Data: {'DeviceID': xxxxxxx, 'SystemSwitch': None, 'HeatSetpoint': None, 'CoolSetpoint': None, 'HeatNextPeriod': None, 'CoolNextPeriod': None, 'StatusHeat': 0, 'StatusCool': 0, 'FanMode': None}
DEBUG (MainThread) [somecomfort] Received setting response {'success': 1}
DEBUG (MainThread) [somecomfort] Sending Data: {'DeviceID': xxxxxxx, 'SystemSwitch': None, 'HeatSetpoint': None, 'CoolSetpoint': None, 'HeatNextPeriod': None, 'CoolNextPeriod': None, 'StatusHeat': 0, 'StatusCool': 0, 'FanMode': None}
DEBUG (MainThread) [somecomfort] Received setting response {'success': 1}
DEBUG (MainThread) [somecomfort] Sending Data: {'DeviceID': xxxxxxx, 'SystemSwitch': None, 'HeatSetpoint': None, 'CoolSetpoint': None, 'HeatNextPeriod': None, 'CoolNextPeriod': None, 'StatusHeat': 0, 'StatusCool': 0, 'FanMode': None}
DEBUG (MainThread) [somecomfort] Received setting response {'success': 1}
DEBUG (MainThread) [somecomfort] Sending Data: {'DeviceID': xxxxxxx, 'SystemSwitch': None, 'HeatSetpoint': None, 'CoolSetpoint': None, 'HeatNextPeriod': None, 'CoolNextPeriod': None, 'StatusHeat': 0, 'StatusCool': 0, 'FanMode': None}
DEBUG (MainThread) [somecomfort] Received setting response {'success': 1}
DEBUG (MainThread) [somecomfort] Sending Data: {'DeviceID': xxxxxxx, 'SystemSwitch': None, 'HeatSetpoint': None, 'CoolSetpoint': None, 'HeatNextPeriod': None, 'CoolNextPeriod': None, 'StatusHeat': 0, 'StatusCool': 0, 'FanMode': None}
DEBUG (MainThread) [somecomfort] Received setting response {'success': 1}
DEBUG (MainThread) [somecomfort] Sending Data: {'DeviceID': xxxxxxx, 'SystemSwitch': None, 'HeatSetpoint': None, 'CoolSetpoint': None, 'HeatNextPeriod': None, 'CoolNextPeriod': None, 'StatusHeat': 0, 'StatusCool': 0, 'FanMode': None}
DEBUG (MainThread) [somecomfort] Received setting response {'success': 1}
DEBUG (MainThread) [somecomfort] Sending Data: {'DeviceID': xxxxxxx, 'SystemSwitch': None, 'HeatSetpoint': None, 'CoolSetpoint': None, 'HeatNextPeriod': None, 'CoolNextPeriod': None, 'StatusHeat': 0, 'StatusCool': 0, 'FanMode': None}
DEBUG (MainThread) [somecomfort] Received setting response {'success': 1}
DEBUG (MainThread) [somecomfort] Sending Data: {'DeviceID': xxxxxxx, 'SystemSwitch': None, 'HeatSetpoint': None, 'CoolSetpoint': None, 'HeatNextPeriod': None, 'CoolNextPeriod': None, 'StatusHeat': 0, 'StatusCool': 0, 'FanMode': None}
DEBUG (MainThread) [somecomfort] Received setting response {'success': 1}
DEBUG (MainThread) [somecomfort] Sending Data: {'DeviceID': xxxxxxx, 'SystemSwitch': None, 'HeatSetpoint': None, 'CoolSetpoint': None, 'HeatNextPeriod': None, 'CoolNextPeriod': None, 'StatusHeat': 0, 'StatusCool': 0, 'FanMode': None}
DEBUG (MainThread) [somecomfort] Received setting response {'success': 1}
DEBUG (MainThread) [somecomfort] Sending Data: {'DeviceID': xxxxxxx, 'SystemSwitch': None, 'HeatSetpoint': None, 'CoolSetpoint': None, 'HeatNextPeriod': None, 'CoolNextPeriod': None, 'StatusHeat': 0, 'StatusCool': 0, 'FanMode': None}
DEBUG (MainThread) [somecomfort] Received setting response {'success': 1}
DEBUG (MainThread) [somecomfort] Sending Data: {'DeviceID': xxxxxxx, 'SystemSwitch': None, 'HeatSetpoint': None, 'CoolSetpoint': None, 'HeatNextPeriod': None, 'CoolNextPeriod': None, 'StatusHeat': 0, 'StatusCool': 0, 'FanMode': None}
DEBUG (MainThread) [somecomfort] Received setting response {'success': 1}

jhemak avatar Oct 09 '24 02:10 jhemak

It appears from the log that no device settings are changing - all of the sent messages are for the same command?

The whole "mode" thing is a bit confusing - AWAY mode is really a hold (at the thermostat side) set to the temperatures configured in the integration setup, while hold just "holds" the temperature at whatever set point is set.

As this is a polled solution, you may not be waiting long enough to see the change?
There is a known problem with API failures as HA doesn't have a retry mechanism (The API fails more than it should). If we change modes and the API reports a failure, it never changes the thermostat settings. This is especially problematic when using presence to set away/home modes with an automation. While your logs don't appear to show this, it may still be the root problem. I'll have a chance next week to test this out with my thermostat and try to replicate.

mkmer avatar Oct 09 '24 11:10 mkmer

Thanks! I completely understand everything you are saying here. Let me clarify exactly how you can test to reproduce what I am experiencing. I do have my thermostat programmed to follow a schedule on the device itself ... from there:

  1. Confirm the thermostat is running in normal, scheduled mode by inspecting the physical device
  2. On the physical device, change the temperature up or down by 1 degree from the programmed temperature
  3. Confirm the physical device says "Hold Until" with a time displayed on the physical screen
  4. In HA, confirm current_temperature matches the new temperature set on the thermostat (wait for refresh if needed)
  5. In HA, confirm preset_mode: none (this is the problem -- i think it should say hold) and that permanent_hold: false (this makes sense)
  6. Run climate.set_preset_mode as shown above to change the mode to none (even though it already is)
  7. On the physical device, confirm the temporary hold is cleared.

As mentioned, the issue is that nothing in the device attributes would enable one to know the device is on a temporary hold after step 5. The attributes (aside from current_temperature) are the same after Step 5 as they are after step 7.

Thanks for taking time to investigate!

jhemak avatar Oct 09 '24 21:10 jhemak

Well... it looks like we only set the mode when it's permanent_hold(): https://github.com/home-assistant/core/blob/260d919f80d7b968509d624c89edc367da883b8c/homeassistant/components/honeywell/climate.py#L328-L329, while the away mode is "internal", not even checking the thermostat state - so if we set away in HA, then change the hold mode on the T-stat, it's going to stay "away". https://github.com/home-assistant/core/blob/260d919f80d7b968509d624c89edc367da883b8c/homeassistant/components/honeywell/climate.py#L326-L327 Probably not "good"... I'll have to see if we can work around this (aka turn off the away mode if the hold goes away or something like that) We should probably return hold for temporary holds as well, either adding a temporary hold function or check for temporary hold values here: https://github.com/home-assistant/core/blob/260d919f80d7b968509d624c89edc367da883b8c/homeassistant/components/honeywell/climate.py#L338-L341

Can you confirm permanent hold on the stat does show up in HA for your T-stat? (It does for mine)

mkmer avatar Oct 15 '24 12:10 mkmer

Yes, if I physically set a permanent hold on the stat, I see preset_mode: hold and permanent_hold: true in HA. Is that what you were asking?

In my opinion, if I physically set a temporary hold on the stat, we should see preset_mode: hold and permanent_hold: false in HA. That is not what happens today.

I'm not too worried about away mode (because I only use away mode in HA when no one is physically home to touch the stat). However, you are right there is an edge case. Setting away mode in HA results in preset_mode: away and permanent_hold: true. That seems right. If I clear the hold on the stat, then permanent_hold changes to false but preset_mode is still away. Would ideally change to none at that point, as you said.

jhemak avatar Oct 16 '24 00:10 jhemak

Perfect. Wanted to confirm your stat behaves the same as mine. I've submitted a PR to handle the temporary hold and fix the away issue.

mkmer avatar Oct 16 '24 01:10 mkmer

@mkmer - thanks a ton for fixing this! It looks to be working perfectly now.

jhemak avatar Oct 26 '24 02:10 jhemak