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

Report correct thermostat mode to Alexa #18053

Merged
merged 1 commit into from Oct 31, 2018

Conversation

Projects
None yet
5 participants
@bitglue
Member

bitglue commented Oct 31, 2018

Description:

We were erroneously reporting the previous mode. So if the thermostat was off
and the user asks, "Alexa, set the thermostat to heat", the thermostat would be
set to heat but Alexa would respond, "The thermostat is off."

Bug caught by @Thunderbird2086 at
#17969 (comment)

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
Report correct thermostat mode to Alexa
We were erroneously reporting the _previous_ mode. So if the thermostat was off
and the user asks, "Alexa, set the thermostat to heat", the thermostat would be
set to heat but Alexa would respond, "The thermostat is off."

Bug caught by @Thunderbird2086 at
#17969 (comment)

@pvizeli pvizeli merged commit 93706fa into home-assistant:dev Oct 31, 2018

5 checks passed

Hound No violations found. Woof!
WIP Legacy commit status override — see details
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.005%) to 93.093%
Details
await hass.services.async_call(
entity.domain, climate.SERVICE_SET_OPERATION_MODE, data,
blocking=False, context=context)
response.add_context_property({

This comment has been minimized.

@balloob

balloob Oct 31, 2018

Member

Alternative is to change blocking=False to blocking=True and we get the real value.

This comment has been minimized.

@bitglue

bitglue Oct 31, 2018

Member

You know I thought I tried that once, and it didn't work. I didn't dig into it. In any case, I'd expect that to increase response latency, which might not be good for UX. There is https://developer.amazon.com/docs/device-apis/alexa-interface.html#deferredresponse, but I don't know how that would impact UX.

This comment has been minimized.

@balloob

balloob Oct 31, 2018

Member

the reason it doesn't always work is because it will only work with polled devices. It won't work with devices that push their state to HA, as that state change will be received too late for the service to be marked done. I think that for now I am fine with pretending it worked.

@fabaff fabaff added the Hacktoberfest label Nov 1, 2018

@balloob balloob referenced this pull request Nov 9, 2018

Merged

0.82 #18335

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment