geoffdavis / esphome-mitsubishiheatpump

ESPHome Climate Component for Mitsubishi Heatpumps using direct serial connection
BSD 2-Clause "Simplified" License
560 stars 153 forks source link

Over-optimistic status changes #35

Closed bulletmark closed 3 years ago

bulletmark commented 3 years ago

I have noticed an issue where if you turn a unit on then immediately turn it off then it appears to go off but if you check 20 or 30 secs later you see it changes back on. This is very undesirable to a user because, e.g. he sees that he has turned the unit off via the HA Android app, but in fact he has not so it continues to operate (and will later re-display back to on but only after he has left the app). I think this issue arises due to PR #7 (from issue #6) which "fakes" the state change to the anticipated target state before the unit has actually done it but in many cases if you do the change quickly the unit just ignores it. I think it is better to always represent the actual unit state to the user, even if it means the UI response is somewhat slow. Ultimately, the slow response is just how the heat pump works so there is little you can do about that.

bulletmark commented 3 years ago

I've (also?) found errors in the state handling. E.g if I set the unit to heating then I see the HA display show "heating" but then a few secs later it shows "idle" even though it is clearly still heating (set temp is above current temp). E.g. here is the ESP log (with added line numbers for reference):

  0 [10:27:03][D][climate:010]: 'Main Bedroom Air Conditioner' - Setting
  1 [10:27:03][D][climate:014]:   Mode: HEAT
  2 [10:27:03][D][MitsubishiHeatPump:242]: control - Was HeatPump updated? YES
  3 [10:27:03][D][climate:262]: 'Main Bedroom Air Conditioner' - Sending state:
  4 [10:27:03][D][climate:265]:   Mode: HEAT
  5 [10:27:03][D][climate:267]:   Action: IDLE
  6 [10:27:03][D][climate:270]:   Fan Mode: AUTO
  7 [10:27:03][D][climate:273]:   Swing Mode: OFF
  8 [10:27:03][D][climate:276]:   Current Temperature: 19.00°C
  9 [10:27:03][D][climate:282]:   Target Temperature: 23.00°C
 10 [10:27:17][D][MitsubishiHeatPump:364]: ########## status changed, operating = 1 ###########
 11 [10:27:17][D][climate:262]: 'Main Bedroom Air Conditioner' - Sending state:
 12 [10:27:17][D][climate:265]:   Mode: HEAT
 13 [10:27:17][D][climate:267]:   Action: HEATING
 14 [10:27:17][D][climate:270]:   Fan Mode: AUTO
 15 [10:27:17][D][climate:273]:   Swing Mode: OFF
 16 [10:27:17][D][climate:276]:   Current Temperature: 19.00°C
 17 [10:27:17][D][climate:282]:   Target Temperature: 23.00°C
 18 [10:27:22][D][MitsubishiHeatPump:251]: ########## settings changed ###########
 19 [10:27:22][I][MitsubishiHeatPump:313]: Climate mode is: 3
 20 [10:27:22][I][MitsubishiHeatPump:333]: Fan mode is: 2
 21 [10:27:22][I][MitsubishiHeatPump:344]: Swing mode is: 0
 22 [10:27:22][I][MitsubishiHeatPump:352]: Target temp is: 23.000000
 23 [10:27:22][D][climate:262]: 'Main Bedroom Air Conditioner' - Sending state:
 24 [10:27:22][D][climate:265]:   Mode: HEAT
 25 [10:27:22][D][climate:267]:   Action: IDLE
 26 [10:27:22][D][climate:270]:   Fan Mode: AUTO
 27 [10:27:22][D][climate:273]:   Swing Mode: OFF
 28 [10:27:22][D][climate:276]:   Current Temperature: 19.00°C
 29 [10:27:22][D][climate:282]:   Target Temperature: 23.00°C
 30 [10:32:03][D][MitsubishiHeatPump:364]: ########## status changed, operating = 1 ###########
 31 [10:32:03][D][climate:262]: 'Main Bedroom Air Conditioner' - Sending state:
 32 [10:32:03][D][climate:265]:   Mode: HEAT
 33 [10:32:03][D][climate:267]:   Action: HEATING
 34 [10:32:03][D][climate:270]:   Fan Mode: AUTO
 35 [10:32:03][D][climate:273]:   Swing Mode: OFF
 36 [10:32:03][D][climate:276]:   Current Temperature: 19.50°C
 37 [10:32:03][D][climate:282]:   Target Temperature: 23.00°C
  1. At line 0 I set the unit to heating.
  2. At line 10 the status comes back that the unit is heating (operating flag = 1 is set by unit). Action is "Heating".
  3. At line 18 the settings come back but this component erroneously sets the Action = "Idle".
  4. At line 30, the unit temp has raised 0.5 degrees so a new status comes back (still operating = 1) and then the component restores action as "Heating".

I don't understand the code well enough but it seems to me that all those lines this->action = climate::CLIMATE_ACTION_IDLE in function hpSettingsChanged() are wrong. Possibly they should just be deleted?

bulletmark commented 3 years ago

I have changed the code to fix the issues here with what I think is a better approach but will wait for a response from @geoffdavis before I proceed with another PR, further to the PR #34 that I already pushed.

jamescadd commented 3 years ago

I haven't tried the fix yet, but the old behavior before optimistic state setting was a far worse UX for me. Previously I would press a button to activate a state in lovelace (like cool) and the UI wouldn't update for 30 seconds. It just hung with the button state showing pressed and I would stand there waiting to see if the unit got the state change. Since this is a more common use case than changing states twice within 30s I'd rather keep this behavior.

bulletmark commented 3 years ago

Note I haven't pushed any PR for the problems I describe here. My PR #34 is a separate and trivial issue. Geoff just merged that PR #34 so I will push another PR to address the issues I describe here.

bulletmark commented 3 years ago

I'd like to close this issue as not sure my description above is correct.

jamescadd commented 3 years ago

Fwiw I do agree there's an underlying problem here that you can send updates to the mini split when it's in a state that cannot accept them. It would be better to have some extra state that showed "currently setting cool mode" or similar and disabled the UI so users can't falsely hope to make any changes at that time. I couldn't think of any precedent for that in HA though so living with the bug may be the best we can do for now.