Skip to content

Add ESPHome climate support#22859

Merged
OttoWinter merged 5 commits into
home-assistant:devfrom
OttoWinter:climate
Apr 10, 2019
Merged

Add ESPHome climate support#22859
OttoWinter merged 5 commits into
home-assistant:devfrom
OttoWinter:climate

Conversation

@OttoWinter
Copy link
Copy Markdown
Member

@OttoWinter OttoWinter commented Apr 7, 2019

Description:

Adds climate device support for esphome. Not much happening here, service calls are just forwarded and state is received as usual

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

Plz ... merge my PR for 0.92 ... 💛

out

@codecov

This comment has been minimized.

Comment thread .coveragerc
@codecov

This comment has been minimized.

Copy link
Copy Markdown
Contributor

@awarecan awarecan left a comment

Choose a reason for hiding this comment

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

away_mode is an optional, so that we need check if this feature enabled before send command

Comment thread homeassistant/components/esphome/climate.py Outdated
Comment thread homeassistant/components/esphome/climate.py Outdated
@OttoWinter
Copy link
Copy Markdown
Member Author

OttoWinter commented Apr 8, 2019

away_mode is an optional, so that we need check if this feature enabled before send command

@awarecan What do you mean by that? The state attribute is already filtered here: https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/climate/__init__.py#L267

@awarecan
Copy link
Copy Markdown
Contributor

awarecan commented Apr 8, 2019

@OttoWinter
Copy link
Copy Markdown
Member Author

@awarecan

For away mode command, that should ultimately be handled in climate base? Anyway, attempting to send an away command would just result in a warning in the esphome log like "This climate device does not support setting away mode". ESPHome also does validation of commands on the esp side, so that's not a problem.

I'll wait for #22878 and then apply the same things for climate in the base.

@balloob
Copy link
Copy Markdown
Member

balloob commented Apr 10, 2019

22878 merged.

Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Looks good!

@MartinHjelmare
Copy link
Copy Markdown
Member

Should we merge?

@OttoWinter OttoWinter merged commit 72af427 into home-assistant:dev Apr 10, 2019
@ghost ghost removed the in progress label Apr 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants