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.

Sign up for GitHub

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

Jump to bottom

[climate] Fix ecobee flaky test #25019

Merged
merged 1 commit into from
Jul 8, 2019
Merged

[climate] Fix ecobee flaky test #25019

merged 1 commit into from
Jul 8, 2019

Conversation

balloob
Copy link
Member

@balloob balloob commented Jul 8, 2019 •
edited
Loading

Description:

Fix ecobee flaky test. Found by @elupus

Related issue (if applicable): fixes #23899 (comment)

@balloob balloob mentioned this pull request Jul 8, 2019
Merged
59 tasks
andrewsayre
andrewsayre reviewed Jul 8, 2019
View reviewed changes
homeassistant/components/ecobee/climate.py
ECOBEE_HVAC_TO_HASS = collections.OrderedDict([
('heat', HVAC_MODE_HEAT),
('cool', HVAC_MODE_COOL),
('auto', HVAC_MODE_AUTO),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this wasn't the scope of this PR, but this should be mapped to HVAC_MODE_HEAT_COOL. Ecobee allows you to set heat/cool setpoints when in "Auto" mode. This is true for probably nearly all of the US thermostats and anyone using "auto" in the new schema likely mapped incorrectly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

home-assistant/architecture#260

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, hence why I opened that issue -- here is an example of where it was done incorrectly under the new schema. :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should not block this pull thou since it fixes broken test in master

@elupus
Copy link
Contributor

elupus commented Jul 8, 2019

Was the order change intended? so that it no longer map it to auxHeatOnly?

@balloob
Copy link
Member Author

balloob commented Jul 8, 2019

Yeah, we want heat to map to heat, not aux heat`

@balloob balloob merged commit 0b7a901 into dev Jul 8, 2019
@delete-merged-branch delete-merged-branch bot deleted the ecobee-ordered-dict branch July 8, 2019 20:10
@lock lock bot locked and limited conversation to collaborators Jul 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@andrewsayre andrewsayre andrewsayre left review comments

@elupus elupus elupus left review comments

Assignees
No one assigned
Labels
cla-signed has-tests integration: ecobee small-pr PRs with less than 30 lines.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@balloob @elupus @andrewsayre @homeassistant