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

Moved Wink water heater from climate to water heater. #17504

Merged
merged 5 commits into from Oct 16, 2018

Conversation

@w1ll1am23
Collaborator

w1ll1am23 commented Oct 16, 2018

Description:

This moves the Wink water heater from climate to the new water heater component.

This is part of #13539 and the last change required to remove all the water heater stuff from the climate component.

Breaking change:

  • Wink water heaters were moved to the new water heater implementation. All Wink water heaters will automatically be moved from climate.wate_heater to water_heater.water_heater Please update your configuration to use water_heater now.
  • Toon "comfort" setting is now available as 'auto' instead of 'performance'

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#6842

Example entry for configuration.yaml (if applicable):

wink:

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • New files were added to .coveragerc.
@@ -48,11 +48,6 @@
STATE_DRY = 'dry'
STATE_FAN_ONLY = 'fan_only'
STATE_ECO = 'eco'
STATE_ELECTRIC = 'electric'

This comment has been minimized.

@balloob
@balloob

This comment has been minimized.

Member

balloob commented Oct 16, 2018

@huangyupeng we have been working on a water heater component for a while and we just realized that Tuya climate component started using these states. That's not correct and so we're removing those states from the Tuya climate component in this PR. If Tuya supports water heater, they should be added the water heater component.

@balloob

This comment has been minimized.

Member

balloob commented Oct 16, 2018

Also updated Toon, as the way Toon uses states makes 0 sense, so might as well just swap a value out.

@huangyupeng

This comment has been minimized.

Contributor

huangyupeng commented Oct 16, 2018

OK, I get it. We only use hot, code, wind, auto currently

balloob added some commits Oct 16, 2018

@balloob balloob merged commit 71ab8a9 into home-assistant:dev Oct 16, 2018

4 checks passed

Hound No violations found. Woof!
WIP ready for review
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@wafflebot wafflebot bot removed the in progress label Oct 16, 2018

Show resolved Hide resolved homeassistant/components/water_heater/wink.py
else:
current_op = WINK_STATE_TO_HA.get(self.wink.current_mode())
if current_op is None:
current_op = STATE_UNKNOWN

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Oct 16, 2018

Member

Basically never use STATE_UNKNOWN. Use None to represent unknown state.

This comment has been minimized.

@w1ll1am23

w1ll1am23 Oct 16, 2018

Collaborator

Noted, I'll get this in the next PR.

for water_heater in pywink.get_water_heaters():
_id = water_heater.object_id() + water_heater.name()
if _id not in hass.data[DOMAIN]['unique_ids']:
add_entities([WinkWaterHeater(water_heater, hass)])

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Oct 16, 2018

Member

Passing in hass to the entity is usually a sign that we're doing something else wrong. Here it looks like we're subscribing to updates before the entity has been added to home assistant in WinkDevice.

This comment has been minimized.

@w1ll1am23

w1ll1am23 Oct 16, 2018

Collaborator

Hmm not too sure I know what you mean. How would I not pass in hass? I think you are correct, we are subscribing to updates before it gets added, but is that an issue? This is how all of the Wink components work.

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Oct 16, 2018

Member

If an update would come in before the entity has been added to home assistant there would be an error since the entity isn't set up properly yet. This is why subscribing for updates or registering callbacks etc should be done in async_added_to_hass. Edit: Of course for some integrations, subscribing early might not be a problem. But it's better to be safe than sorry.

hass will be set on the entity when it has been added to home assistant:
https://developers.home-assistant.io/docs/en/creating_platform_code_review.html#5-entity

@balloob balloob referenced this pull request Oct 26, 2018

Merged

0.81 #17809

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