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

Daikin heat mode doesn't work from HomeKit #14405

Closed
andrews05 opened this issue May 12, 2018 · 22 comments · Fixed by #18438
Closed

Daikin heat mode doesn't work from HomeKit #14405

andrews05 opened this issue May 12, 2018 · 22 comments · Fixed by #18438

Comments

@andrews05
Copy link

andrews05 commented May 12, 2018

Home Assistant release with the issue:
0.69

Operating environment (Hass.io/Docker/Windows/etc.):
Hassbian, Pi Zero

Component/platform:
climate.daikin

Description of problem:
The HomeKit bridge offers four modes for Daikin heat pumps: Off, Heat, Cool, Auto. The heat mode doesn't work because pydaikin for some reason decided to use the term "Hot" instead, so it doesn't understand "Heat".

Additional information:
I understand Home Assistant doesn't control pydaikin. I would be happy to fork pydaikin and fix this myself plus a number of other fixes/improvements but I'm not sure how that would get integrated back to Home Assistant.

@cdce8p
Copy link
Member

cdce8p commented May 12, 2018

The best way would be to contribute to pydaikin directly through PRs. A fork would only be accepted if the repo is now unmaintained.

As for the hot state. The daikin platform already has a mapping dictionary for the other way round: HA_STATE_TO_DAIKIN. It might make sense to implement the something like DAIKIN_TO_HA_STATE as well.

@andrews05
Copy link
Author

I had assumed it was unmaintained but I guess I should try a PR and find out :)

"Heat" is the more correct term, I would argue, so I think it would be better to just fix it in pydaikin.

@andrews05
Copy link
Author

I'm confused as to where pydaikin is coming from now. PyPi points to https://bitbucket.org/mustang51/pydaikin so I assumed it was this one, but when looking at the code that gets installed by Home Assistant it seems to match with https://github.com/andrei4002/pydaikin instead. Which should I submit a PR to?

@cdce8p
Copy link
Member

cdce8p commented May 13, 2018

You can take a look at the PKG-INFO file for the package.
There it says: Home-page: https://bitbucket.org/mustang51/pydaikin

@cdce8p
Copy link
Member

cdce8p commented May 13, 2018

As for the change to Heat: I might be the better name, but from a maintainer standpoint it would be a breaking change. I'm not sure if that will be done.
A fix for the Home Assistant platform might be easier and quicker.

@andrews05
Copy link
Author

So I posted an issue on pydaikin but it's been two weeks now with no response. The real issue is that the codebase for v0.4 doesn't seem to exist anywhere - the bitbucket repo is v0.1. Which means I can't submit PRs.

Would be such a bad thing for HA to simply incorporate and adapt the code itself? This could at least save multiple layers of value translations.

@cdce8p
Copy link
Member

cdce8p commented May 27, 2018

You can incorporate it into the daikin component. I just won't be adding a workaround inside the homekit one.

@charlietomo
Copy link

I am also having this issue (commented in issue #13122 ) but this comment thread seems more progressed.

I am able to test but don't code and am not sure what changes are required? It sounds like changes to the daiken component - is that this one?

@andrews05 is there anything I can do to help - considering I can't code?

@andrews05
Copy link
Author

I fixed it in my local installation by changing 'hot' to 'heat' in both of these locations:
pydaikin/applicance.py:44
homeassistant/components/climate/daikin.py:38

I would like to integrate pydaikin into HA and make fixes to it there, but I can't promise a PR anytime soon.

@cdce8p
Copy link
Member

cdce8p commented Jun 1, 2018

@rofrantz Can you take a look at this? I belive you're one of the original authors of the daikin climate platform.

@GeradB
Copy link

GeradB commented Jun 5, 2018

Im also keen to contribute but my python skills are non existent. I cant even get to applicance.py or daikin.py on my hass.io

@charlietomo
Copy link

@GeradB did you ever get anywhere with finding how to edit those files on hass.io ? I am having a similar issue. Thanks.

@GeradB
Copy link

GeradB commented Jul 24, 2018

@charlietomo No sorry , couldn't get it to work maybe @cdce8p can point us in the right direction.

@maknz
Copy link

maknz commented Sep 29, 2018

Just wanted to add my 👍 to this, all works perfectly well in HA itself (except 'Hot' should be 'Heat', but it seems that mistake is upstream of HA). Will be attempting the hacks from @andrews05 and will report back.

@maknz
Copy link

maknz commented Sep 29, 2018

Alright, I've done a quick-and-dirty hack to daikin.py in order to show 'Heat' in HA, but send 'hot' over to the pydaikin dependency. Two additions have been made, search for # HACK to find them.

File @ https://gist.github.com/maknz/5a0936c369d81993f21d30c57951d61d

Place at /config/custom_components/climate/daikin.py (if using Hass.io)

Interestingly, pydaikin 0.4 (which daikin.py uses) uses 'cool' correctly, rather than 'cold'. However, the file in the master branch still shows 'cold' 🙄. Anyway, it means we only need to correct hot->heat.

@MatteGary
Copy link
Contributor

Hi all,
I had the same issue, also regarding the interaction between the Daikin AC component and the Homekit interaction.
I made a PR for that, and now it is working fine (at least it seems to me 😄 ).
If you want you can try testing my PR you can find at

#16913

It support correct operation mode interaction, and with that now it also support correct localization.
Also Homekit interaction works good.

The only bug remaining is the automatic reset of the target temperature to 22°, but I will have a look at it later.

Thanks in advance if you can run some tests on my PR.

@maknz
Copy link

maknz commented Oct 3, 2018 via email

@MatteGary
Copy link
Contributor

Hi @maknz !
Yes I made a commit for an issu regarding setting Operation Mode both from Homekit and from HA UI, now it should work fine. If you can have it for a test in the actual version it would be great!

I found the issue you are referring to, as soon as this PR get merged in the stable stream I will investigate in the issue discussion to understand whether the patches to the pydaikin library works and if it is the only solution, or there are other possible solution.

@charlietomo
Copy link

Thanks for your work @MatteGary and @maknz , this looks great. Is the fix likely to make its way to the main release branch then Hass.io ?

Do you have a link to the 22deg reset issue as it would be good to follow that. Thanks.

@MatteGary
Copy link
Contributor

Regarding the 22deg issue, I think @maknz was referring to this #13111

@maknz
Copy link

maknz commented Oct 15, 2018 via email

@fredrike
Copy link
Contributor

This is fixed in #16913

@home-assistant home-assistant locked and limited conversation to collaborators Mar 7, 2019
@ghost ghost added the integration: daikin label Mar 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants