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

Unable To Change Mode On Zwave Thermostat Via Google Assistant #13158

Closed
dshokouhi opened this issue Mar 12, 2018 · 51 comments · Fixed by #15162
Closed

Unable To Change Mode On Zwave Thermostat Via Google Assistant #13158

dshokouhi opened this issue Mar 12, 2018 · 51 comments · Fixed by #15162

Comments

@dshokouhi
Copy link
Member

Make sure you are running the latest version of Home Assistant before reporting an issue.

You should only file an issue if you found a bug. Feature and enhancement requests should go in the Feature Requests section of our community forum:

Home Assistant release (hass --version):

0.65.4

Python release (python3 --version):

3.6.3

Component/platform:

zwave
google_assistant

Description of problem:

I am unable to ask google assistant to set the thermostat to Heat or Cool or even turn it off. I am able to query the device and see if it is set to Heat, Cool or off.

Expected:

I should be able to turn on the heater or cooler

Problem-relevant configuration.yaml entries and steps to reproduce:

  1. Install latest version of home assistant
  2. Setup zwave with a Honeywell zwave thermostat
  3. Setup google assistant with the thermostat
  4. Sync the device
  5. Ask "Hey google is the thermostat on?"
  6. Observe proper response
  7. Say "Hey google turn on the heater."

Google responds with that mode isnt available for the thermostat. However it will see the heat or cool modes are on if I turn them on manually.

Traceback (if applicable):
n/a

Additional info:

@balloob
Copy link
Member

balloob commented Mar 13, 2018

This will happen if the Z-Wave thermostat integration does not use the STATE_* values from the climate component. It looks like Z-Wave just uses their own provided value which is wrong: https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/climate/zwave.py#L90

@janLo
Copy link
Contributor

janLo commented Mar 13, 2018

Same Problem appears now with homematic thermostats. It worked before.

@janLo

This comment has been minimized.

@janLo

This comment has been minimized.

@balloob
Copy link
Member

balloob commented Mar 14, 2018

I've hid your screenshots as they consume too much vertical space.

Homematic too does not use the STATE_ constants as defined by the climate component.

@ttroy50
Copy link
Contributor

ttroy50 commented Mar 14, 2018

Possibly related to this I've found that the EphEmber climate device is no longer appearing in google assistant. After upgrading to 0.65.3 I did a resync and they disappeared from the home app.

I see no errors in the logs.

Taking a quick look at the code for this it appears that it is because the ephember component doesn't support climate.SUPPORT_OPERATION_MODE. I had a change in 0.64 that had a fallback to reading state if operation mode wasn't supported. It looks like the recent refactor has added a hard requirement on OPERATION_MODE.

@balloob
Copy link
Member

balloob commented Mar 14, 2018

There is indeed a hard requirement on SUPPORT_OPERATION_MODE. Without it, code should not call the set operation mode service (because it's not supported).

If Ephember supports set operation mode, it should add the support flag.

@ttroy50
Copy link
Contributor

ttroy50 commented Mar 14, 2018

But if you want your climate device to be read only and still report the state and temperature then this isn't possible now. Previously I could query the state of the device without being able to change it. I think a change state was just a empty call that did nothing but am not 100% certain on that.

@janLo
Copy link
Contributor

janLo commented Mar 14, 2018

I think we need better documentation of how components of a certain domain have to be designed to work as expected and to not break in other places if something unrelated gets changed. I also think, that we might need some type of subsystem maintainer who can overlook new code and changes of a certain domain to keep it in line with the bigger picture.

@balloob
Copy link
Member

balloob commented Mar 14, 2018

@ttroy50 you can mimic this with the generic thermostat.

@janLo this was not something unrelated changing, we did change Google Assistant. We made Google Assistant follow our exact implementation. I did a call for subsystem maintainers but did not get a lot of response. We're open source and we got to play with the cards we're dealt. Feel free to open a PR for Homematic to address the current operation issue.

@ttroy50
Copy link
Contributor

ttroy50 commented Mar 15, 2018

@balloob That doesn't make much sense to me. You then have to configure each thermostat twice. Once in the main climate component and once in the generic thermostat component.

Different thermostats may support different things and I think we should be aiming to expose any available functionality when we can. Preventing reading / setting the temperature because it can't change on / off in operation mode seems to be making a hard line in the wrong place.

@balloob
Copy link
Member

balloob commented Mar 16, 2018

Google Assistant is built around traits. Currently Google Assistant is limited to reporting temperature on devices that implement the TemperatureSetting trait. This trait requires the thermostat to support setting mode and temperature. If you want to deviate from this, feel free to run an adjusted Google Assistant component locally. Home Assistant won't deviate in it's integration from any of Googles requirements.

Nest launched temperature sensors for rooms today so I would not be surprised if some sort of temperature sensor trait would be made available at some point.

@dshokouhi
Copy link
Member Author

Didn't see any fixes come in but just wanted to mention the issue still exists in 0.66

@spacesuitdiver
Copy link

spacesuitdiver commented Apr 17, 2018

This also seems like an issue with HomeKit. I can update setpoint (temperature) but mode changes don't send or display, generic thermo seems to work fine, I'm currently trying to just create a proxy temporarily using a generic thermo.

@blackshoals
Copy link

I'm in support of consistent code, even at the cost of functionality but I don't really understand this change since voice control seems a very common use of controlling climate. My limited understanding of the thread so far is that the homeassistant system maintainers have passed the requirement for a fix to the z-wave component maintainers with no response. From an integration view it seems unusual to force a change that way, but not something I can help fix. 2 options then:

  1. Possibly use Generic thermostat. Don't think that will work for me since trying to control the local setting and the HA commands will cause conflicts.

  2. As balloob suggested "If you want to deviate from this, feel free to run an adjusted Google Assistant component locally". I tried this:
    a) Download an older version of the Google Assistant component from Git (10 Feb 2018)
    b) Create a custom_components folder in config and drop the Git folder there renamed "google_assistant_old".
    c) change "google_assistant" to "google_assistant_old" in configuration.yaml and restart.

No luck so far. I assume the references to "google_assistant" might need to be changed to "google_assistant_old" in the individual files? Since it's a set of files rather than one .py it's hard to see the structure. Any other suggestions? Thanks

@janLo
Copy link
Contributor

janLo commented Apr 22, 2018

I think the main problem is, that home assistant is "move fast and break things". The new release model is a step in the right direction but before it has found maintainers for its subsystems it will not get any better. The Homematic and zwave climate components got introduced with the climate component itself and get still blamed that they do not implement the methods the the climate component expect. I doubt that anyone in mentioned components is even aware of the necessary changes that came at some point. And so I doubt it will get fixed by them.

I would expect that the change that broke things (Google assistant change) would get reverted for a stable version until the fixes are made and applied in the broken components.

@dshokouhi
Copy link
Member Author

dshokouhi commented Apr 22, 2018

You would imagine a popular component like zwave (that has its own discord thread) would have a device maintainer who would get notified about this. If I knew who it was I would tag them in here but I don't and don't see a codeowner listed either so its hard to imagine how they get notified about changes like this.

@balloob
Copy link
Member

balloob commented Apr 23, 2018

@janLo you're wrong. The climate component used to be the thermostat component. Z-Wave was not at the beginning of the thermostat component, the component got introduced together with the Nest integration.

If no developer is stepping up to fix a broken component, I can only assume that it's not that big of a deal? It's easy to post here in the comments and hoping other people spend their time to fix your problems, but last time I checked, that's not how open source works.

@janLo
Copy link
Contributor

janLo commented Apr 23, 2018

If no developer is stepping up to fix a broken component, I can only assume that it's not that big of a deal?

Thats probably because the component that is broken is the google-assistant component. Sorry, but from a users perspective its clearly a regression in this component. That its caused by implementation details of another component is just a detail. A proper solution would have been to hold the change in the google-assistant back until the issues in the components it depends on are fixed.

@balloob
Copy link
Member

balloob commented Apr 23, 2018

The Google Assistant component is made to work with the Home Assistant abstraction of climate devices. It's not broken, it works exactly as intended. Last comment I am going to make in this thread.

@dshokouhi
Copy link
Member Author

@andrey-git I saw several PR's and zwave bug fixes come from you lately, are you considered the zwave codeowner? If so just wanted to make sure this bug was on your radar :) if not apologies for tagging you.

@andrey-git
Copy link
Contributor

I am a zwave codeowner, but I don't have a zwave climate device, so I doubt I'll get to checking this integration.

@spacesuitdiver
Copy link

spacesuitdiver commented May 4, 2018

@andrey-git can I donate one? I have a CT100 🙂

@andrey-git
Copy link
Contributor

Thanks for the offer :) I don't have anything to connect it to anyway.

@turbokongen @armills do any of you have zwave climate device and Google home?

@turbokongen
Copy link
Contributor

I have a climate device, but not Google home.

@spacesuitdiver
Copy link

spacesuitdiver commented May 4, 2018

I do think our best bet is going to be implementing this (#11472) once if we can get it through PR, the CT100 Thermos are a bit awkward anyway because of the multiple climate devices it creates so we need a way to aggregate the two together.

@dshokouhi
Copy link
Member Author

@cdce8p I would be more than happy to test :)

@kdjordjev
Copy link

I am not familiar with all modes, so my mappings above reflect what my thermostat supported.
I have 2GIG CT80

Thank you @cdce8p . I think not mapping back set_operation_mode might be my issue.
In any case I will also try to test your changes

@PRabahy
Copy link
Contributor

PRabahy commented Jun 26, 2018

FYI, my thermostat (Linear GC-TBZ48) shows Auto, Off, Aux Heat, Heat, and Cool as the modes listed under Operation.

@cdce8p
Copy link
Member

cdce8p commented Jun 26, 2018

@dshokouhi, @kdjordjev Who's up for some testing? I just opened the PR.
The Link to the raw file: download. It's a first draft, but, unless I missed something, should work.

@cdce8p
Copy link
Member

cdce8p commented Jun 26, 2018

Thanks to @dshokouhi for debugging and testing! My PR works.
If you want to try yourself:

  1. Inside the config folder create a dictionary custom_components.
  2. Inside this folder, create a climate folder.
  3. Download the new file (link) and place it inside the climate folder. The path should look like this: .homeassistant/custom_components/climate/zwave.py
  4. Restart

This is a breaking change!!
Please take a look at the PR description: #15162


Edit:
The file path was incorrect! The python file should be placed inside the climate folder. The zwave folder can be deleted.

@kdjordjev
Copy link

I will test it tonight with Alexa
Can you also add the following mappings

"Cool Econ":STATE_COOL,"Heat Econ":STATE_HEAT

@cdce8p
Copy link
Member

cdce8p commented Jun 27, 2018

@kdjordjev Unfortunately that won't be possible. My fix is desinged for a 1:1 mapping and Cool is already mapped to STATE_COOL.

The only options are to either wait for the redesign of the climate component or change the mapping dict manually in your custom_components folder, although you would remove mapping for Cool then.

@kdjordjev
Copy link

To report on the test:
The Alexa behavior is the same as before.

  • HA working
  • Alexa App is functional
  • Alexa replies with: "Hm, Heating Thermostat is not responding", but sets the T as requested

@dshokouhi
Copy link
Member Author

Interesting in my testing Google Assistant responded as expected "changing thermostat to cool" and it changed the mode accordingly

@kdjordjev
Copy link

I think I know what's going on.
I think my thermostat is slow to respond to the command SetTemperature. It takes 5-6s for the command to be send and the thermostat to respond and update the GUI with the new set point. I suspect Alexa times out after 4s, and responds with "Not responding".
I also noticed the Alexa app when I change the T, i got a message "not responding" (with small font, did not see it before) but after a sec or two the new setpont gets updated.

Seems the current issue is solved, but we have a new issue due to the timeout? How do we solve this?

@cdce8p
Copy link
Member

cdce8p commented Jun 27, 2018

@kdjordjev Can you open a new issue for that? That needs to be solved independently from this one.

@cdce8p
Copy link
Member

cdce8p commented Jun 29, 2018

I just merged the fix onto the dev branch, so it will be included in the beta for 0.73.

@everyone
Remember to delete the file in the custom_components folder to receive future updates.

@austinmroczek austinmroczek mentioned this issue Sep 3, 2018
4 tasks
@maxill1
Copy link

maxill1 commented Sep 3, 2018

I just want to report that the uppercase/lowercase problem on google assistant component (cloud in my case) exists on MQTT HVAC climate devices too and it was causing resposes like " does not support this mode" ("La modalità non è disponibile per ").

In my configuration.yaml i had to change all modes to lower case and handle them on my node-red instance to revert them to uppercase.

@Krid86a
Copy link

Krid86a commented Oct 3, 2018

Hi, any solution for google home? I tried to use eq3 Buetooth Thermostat with google assistant, but it replies "can't set this temperature for this device", alexa works

@reaper7
Copy link
Contributor

reaper7 commented Oct 11, 2018

@Krid86a - I have the same issue.
Appropriate issue ( #13712 ) was closed and marked as duplicate with this issue.

As @awarecan wrote in this issue (hiden comments): #17302 ,eq3btsmart.py do not support STATE_COOL, STATE_HEAT which are supported by google assistant:
https://github.com/home-assistant/home-assistant/blob/3f87d413813de84935ea67b5212c55348524447f/homeassistant/components/climate/eq3btsmart.py#L64-L71

https://github.com/home-assistant/home-assistant/blob/3f87d413813de84935ea67b5212c55348524447f/homeassistant/components/google_assistant/trait.py#L408-L415

@awarecan
Copy link
Contributor

@Krid86a @reaper7 please track eq3 issue in #13712, I reopened it.

I am going to lock this issue since z-wave thermostat already fixed. Please open new issue for other platform.

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Oct 11, 2018
reaper7 referenced this issue Oct 11, 2018
* Implement turn_off and turn_on actions for eq3btsmart

This commit implements the turn_off and turn_on methods for eq3btsmart. Turning the device off will set the thermostat to "OFF". Turning it on will set it to "AUTO".

* Add missing support flags for on/off feature

* Fix line length
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.