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

Update fan.zha platform. #16551

Merged
merged 2 commits into from
Sep 12, 2018
Merged

Update fan.zha platform. #16551

merged 2 commits into from
Sep 12, 2018

Conversation

Adminiuga
Copy link
Contributor

Description:

switch from asyncio to async def()
catch DeliveryError exceptions
keep previous state, if reading 'fan_mode' attribute fails

Checklist:

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

switch from asyncio to async def()
catch DeliveryError exceptions
keep previous state, if reading 'fan_mode' attribute fails
@ghost ghost added the in progress label Sep 11, 2018
Copy link
Member

@pvizeli pvizeli 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

new_value = result.get('fan_mode', None)
self._state = VALUE_TO_SPEED.get(new_value, None)
self._state = VALUE_TO_SPEED.get(new_value, self._state)
Copy link
Member

Choose a reason for hiding this comment

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

The state is defined on core and need to be in that format. Please revert this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you lost me here for a bit. self._state is defined by ZhaFan and pushed as a state. This code just keeps the previous state in case we fail to read the 'fan_mode' attribute. Although, from the other hand if we fail to get 'fan_mode' then the state should be None to represent the 'unknown'.
So is it better to keep old state or return None for failed updates?

if we fail to read 'fan_mode' attribute, use None for state
@pvizeli pvizeli merged commit 501f2b0 into home-assistant:dev Sep 12, 2018
@ghost ghost removed the in progress label Sep 12, 2018
@Adminiuga Adminiuga deleted the zha-fan-async branch September 13, 2018 14:53
@balloob balloob mentioned this pull request Sep 28, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Feb 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-signed integration: zha small-pr PRs with less than 30 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants