dlarrick / hass-kumo

Home Assistant module interfacing with Mitsubishi mini-split units
MIT License
97 stars 21 forks source link

KumoThermostat.set_temperature not working for heat_cool HVAC mode #27

Closed FrancisLab closed 4 years ago

FrancisLab commented 4 years ago

I was having a couple issues writing automations setting the hvac mode to heat_cool. After some digging, I found a couple KumoThermostat.set_temperature issues preventing the heat_cool HVAC mode from working as expected.

  1. Not converting self.hvac_mode from an HA state string to a Kumo state string.
  2. Checking if mode_to_set, a Kumo state string, is in a set of valid HA states. This causes issues because the HA state string for heat_cool ("heat_cool") isn't the same as the Kumo state string ("auto")
  3. The call to set_cool_setpoint passes target instead of target["cool"] as a parameter
  4. The concatenation of the two responses into a string isn't a valid Python operation

I've fixed them in my local fork of the repo.

I was able to test an earlier version of my changes on v0.1.3 (or maybe v0.1.2, can't remember what version I was on before updating), but I'm now having a lot of other issues with 0.1.4.

dlarrick commented 4 years ago

I don't use heat_cool mode myself; that support was added by another user. Assuming you can get it working again I'll accept a pull request. Your current changes look about right to me, but I have no way of testing it so I'm relying on you or other users to verify it works before I can merge.

dlarrick commented 4 years ago

One potential thought regarding 0.1.4 issues: make certain you're using an updated version of the pykumo library. The latest change to avoid async warnings in the log means the latest HA code needs the latest pykumo code (and vice versa).

FrancisLab commented 4 years ago

Sounds good! I'll see if I can figure out the issues I'm having with v0.1.4 sometime this weekend.

Do I have to manually update pykumo if I'm running hass-kumo as an HACS integration? I figured it would be updating dependencies as necessary.

I must admit, I've been running this component for a while now and can't remember if I had to do anything manually to get pykumo install :) Maybe I need to update it with a pip install?

FrancisLab commented 4 years ago

For context, the issues I'm seeing with Home Assistant v0.110.1 and hass-kumo to v0.1.4 are:

  1. Unresponsive thermostat UI. For example, clicking on a different HVAC mode won't update the UI for 5 to 30+ seconds. Sometimes it won't update it at all.

  2. Service calls not working. For example this service call won't update either the hvac_mode or the temperature.

image

dlarrick commented 4 years ago

It should update automatically. Unless it didn't, which I've seen happen on occasion. But the issues you're seeing sure sound like it's not re-polling the unit, which is what would happen if you don't have the changes to one component or the other.

Make certain this line: https://github.com/FrancisLab/hass-kumo/blob/master/custom_components/kumo/climate.py#L199 is in fact present in the code you're running.

dlarrick commented 4 years ago

Oh, and looking at your example service call: in my own automation I set the operation mode first then set the target temperature. Maybe something changed in 0.110 in terms of how the thermostat service calls the component. Though I have a vague recollection that setting both in the same service call can be flaky.

FrancisLab commented 4 years ago

It should update automatically. Unless it didn't, which I've seen happen on occasion. But the issues you're seeing sure sound like it's not re-polling the unit, which is what would happen if you don't have the changes to one component or the other.

Make certain this line: https://github.com/FrancisLab/hass-kumo/blob/master/custom_components/kumo/climate.py#L199 is in fact present in the code you're running.

I made sure the line was included and that I was running against the latest version of pykumo.

I think the extra latency might just be from hass not calling update() often enough, the recent changes to pykumo to not raise the updates on change would have made this more apparent. No idea why hass would poll so slowly though...

Anyway, it's not a big deal for me. As long as the UI ends up updating, it'll do!

Oh, and looking at your example service call: in my own automation I set the operation mode first then set the target temperature. Maybe something changed in 0.110 in terms of how the thermostat service calls the component. Though I have a vague recollection that setting both in the same service call can be flaky.

Spot on! Looks like something in 0.110 broke changing the hvac_mode with the set_temperature service call. I can repro the issue with the demo thermostats. Using set_hvac_mode works just fine.

This should unblock me from testing my pull request on v0.1.4 which I'll do some time soon.

dlarrick commented 4 years ago

I added some debug logging, and we're getting polled every 60 seconds. Indeed this is the default scan_interval for climate devices. See: https://github.com/home-assistant/core/blame/dev/homeassistant/components/climate/__init__.py#L81

This should really only affect external changes (such as from KumoCloud). However with some more digging, I've discovered that the indoor unit does not always immediately reflect the newly-set state via its API and so there is a race as to whether HA gets the (correct) new state or the (stale) old state. I have a workaround and will push an update sometime today, with luck.

dlarrick commented 4 years ago

Fixed by @FrancisLab in #28

FrancisLab commented 4 years ago

Thanks for all the help!