home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
73.45k stars 30.69k forks source link

Daikin integration turning Airbase onto Auto mode instead of Cool #32052

Closed djxie2019 closed 4 years ago

djxie2019 commented 4 years ago

The problem

Since the most recent update, when I ask Siri to turn on the air conditioner, the air conditioner turns onto it’s “Auto” mode instead of the last known mode of “Cool”. Using an Airbase. Never had this problem until the last update.

Environment

Problem-relevant configuration.yaml

Traceback/Error logs

Additional information

probot-home-assistant[bot] commented 4 years ago

Hey there @fredrike, @rofrantz, mind taking a look at this issue as its been labeled with a integration (daikin) you are listed as a codeowner for? Thanks!

fredrike commented 4 years ago

Are you saying that it worked in 0.105.3?

Afaik, there have not been any changes to the Daikin code in the latest releases.

djxie2019 commented 4 years ago

Worked fine in the last release, popped up in the new release. Siri commands will only turn the Daikin Airbase to “Auto” mode instead of Cool or Heat. Homekit doesn’t actually have an Auto mode for a thermostat so don’t know how it’s triggering.

fredrike commented 4 years ago

@springstan I don't think this is Daikin related.

springstan commented 4 years ago

@fredrike yeah seems like it, sorry for the inconvenience.

djxie2019 commented 4 years ago

I still think it’s the Daikin integration. Homekit says Cool to 22deg, Lovelace reflects Cool to 22deg, but the Daikin panel says “Auto” mode rather than “Cool” and it’s never done this before.

On the Daikin panel, if I use the button to change the mode to “Cool”, within a few seconds, it pushes back to “Auto”. Something is retriggering it to instantly change back to “Auto”.

fredrike commented 4 years ago

This is the changes made in daikin integration:

https://github.com/home-assistant/home-assistant/commits/dev/homeassistant/components/daikin

The library change added support for auto mode for airbase units but that should not affect the behavior you are referring to.

Check in at the forum at https://community.home-assistant.io/t/support-for-daikin-airbase-units/102622/118 and we will investigate further.

bdraco commented 4 years ago

https://github.com/home-assistant/core/pull/34073 should resolve most of these type of issues

bdraco commented 4 years ago
diff --git a/homeassistant/components/daikin/climate.py b/homeassistant/components/daikin/climate.py
index 8b5724e014..f10d282dc7 100644
--- a/homeassistant/components/daikin/climate.py
+++ b/homeassistant/components/daikin/climate.py
@@ -83,7 +83,7 @@ async def async_setup_platform(hass, config, async_add_entities, discovery_info=
 async def async_setup_entry(hass, entry, async_add_entities):
     """Set up Daikin climate based on config_entry."""
     daikin_api = hass.data[DAIKIN_DOMAIN].get(entry.entry_id)
-    async_add_entities([DaikinClimate(daikin_api)])
+    async_add_entities([DaikinClimate(daikin_api)], True)

 class DaikinClimate(ClimateDevice):

@fredrike Is it possible to update the entities when they are added?

Homekit needs ATTR_HVAC_MODES when it first sees the entity.

        # Target mode characteristics
        hc_modes = state.attributes.get(ATTR_HVAC_MODES)
        if hc_modes is None:
            _LOGGER.error(
                "%s: HVAC modes not yet available. Please disable auto start for homekit.",
                self.entity_id,
            )
bdraco commented 4 years ago

If its not possible, I'm thinking another solution would be to modify homekit to recheck supported features and change valid_values via char.override_properties when we get the first call to update_state

bdraco commented 4 years ago

Leaving this one open as it may no be solved by #34073

fredrike commented 4 years ago

This is not related to Daikin but how the mappi g in homekit is done.

bdraco commented 4 years ago

@fredrike I've tested this with the demo ecobee thermostat and homekit which doesn't have an Auto mode and can't replicate the issue.

It sounds like you know where the bug is. We would love to get a PR to fix the issue from someone who can validate it works with Daikin.

fredrike commented 4 years ago

@bdraco This is the issue: https://github.com/home-assistant/core/blob/5d649b25416d75c31dda8ec227c17e649a3e86d0/homeassistant/components/homekit/type_thermostats.py#L89-L98

In [1]: HC_HASS_TO_HOMEKIT = { 
     ...:     'HVAC_MODE_OFF': 'HC_HEAT_COOL_OFF', 
     ...:     'HVAC_MODE_HEAT': 'HC_HEAT_COOL_HEAT', 
     ...:     'HVAC_MODE_COOL': 'HC_HEAT_COOL_COOL', 
     ...:     'HVAC_MODE_AUTO': 'HC_HEAT_COOL_AUTO', 
     ...:     'HVAC_MODE_HEAT_COOL': 'HC_HEAT_COOL_AUTO', 
     ...:     'HVAC_MODE_DRY': 'HC_HEAT_COOL_COOL', 
     ...:     'HVAC_MODE_FAN_ONLY': 'HC_HEAT_COOL_COOL', 
     ...: } 
     ...: HC_HOMEKIT_TO_HASS = {c: s for s, c in HC_HASS_TO_HOMEKIT.items()} 
     ...:                                                                                                                                                                                                                                  

In [2]: HC_HOMEKIT_TO_HASS                                                                                                                                                                                                               
Out[2]: 
{'HC_HEAT_COOL_OFF': 'HVAC_MODE_OFF',
 'HC_HEAT_COOL_HEAT': 'HVAC_MODE_HEAT',
 'HC_HEAT_COOL_COOL': 'HVAC_MODE_FAN_ONLY',
 'HC_HEAT_COOL_AUTO': 'HVAC_MODE_HEAT_COOL'}

In [3]: HC_HASS_TO_HOMEKIT                                                                                                                                                                                                               
Out[3]: 
{'HVAC_MODE_OFF': 'HC_HEAT_COOL_OFF',
 'HVAC_MODE_HEAT': 'HC_HEAT_COOL_HEAT',
 'HVAC_MODE_COOL': 'HC_HEAT_COOL_COOL',
 'HVAC_MODE_AUTO': 'HC_HEAT_COOL_AUTO',
 'HVAC_MODE_HEAT_COOL': 'HC_HEAT_COOL_AUTO',
 'HVAC_MODE_DRY': 'HC_HEAT_COOL_COOL',
 'HVAC_MODE_FAN_ONLY': 'HC_HEAT_COOL_COOL'}

Don't know how to fix it though.

bdraco commented 4 years ago

Thanks. We use that dict to build a per device dict that only includes the modes the device supports here:

https://github.com/home-assistant/core/blob/5d649b25416d75c31dda8ec227c17e649a3e86d0/homeassistant/components/homekit/type_thermostats.py#L172-L186

If the device is not yet initialized we won't be able to access the list of valid modes so you'll have to defer starting homekit until the device has done its first update.

https://github.com/home-assistant/core/blob/5d649b25416d75c31dda8ec227c17e649a3e86d0/homeassistant/components/homekit/type_thermostats.py#L154

fredrike commented 4 years ago

So HC_HOMEKIT_TO_HASS isn't used? The problem here is that not all modes are represented. But I might be wrong.

bdraco commented 4 years ago

HC_HOMEKIT_TO_HASS

Its only used for water heaters https://github.com/home-assistant/core/blob/5d649b25416d75c31dda8ec227c17e649a3e86d0/homeassistant/components/homekit/type_thermostats.py#L527

bdraco commented 4 years ago

There are potential three ways to fix this:

  1. Modify daikin to do an update when async_add_entities is called : https://github.com/home-assistant/core/blob/5d649b25416d75c31dda8ec227c17e649a3e86d0/homeassistant/components/daikin/climate.py#L86

    by changing it to be

async_add_entities([DaikinClimate(daikin_api)], update_before_add=True)

This will almost definitely fix this.

  1. Have users disable homekit auto start and only start once daikin has valid modes available: https://www.home-assistant.io/integrations/homekit/#disable-auto-start

  2. Modify homekit to look at the state again on the first state update from daikin and figure out the valid modes again. This may not work though (probably why disable auto start was added in the first place) and since I don't have a daikin I can't test to validate any changes I would make here.

fredrike commented 4 years ago
  1. Modify daikin to do an update when async_add_entities is called : https://github.com/home-assistant/core/blob/5d649b25416d75c31dda8ec227c17e649a3e86d0/homeassistant/components/daikin/climate.py#L86 by changing it to be async_add_entities([DaikinClimate(daikin_api)], update_before_add=True)

This will almost definitely fix this.

Ok, I'll send a PR #34248 with that (it wouldn't hurt more than potential longer loading times for the Daikin integration).

bdraco commented 4 years ago

Ok, I'll send a PR #34248 with that (it wouldn't hurt more than potential longer loading times for the Daikin integration).

Thanks @fredrike !