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

Make light.yeelight stop doing IO when accessing properties #17917

Merged
merged 1 commit into from Oct 29, 2018

Conversation

Projects
None yet
5 participants
@rohankapoorcom
Contributor

rohankapoorcom commented Oct 28, 2018

Description:

I noticed that @bieniu posted that light.yeelight was taking a long time to update the state machine. (#4210 (comment)) Taking a look, it appears that there is some IO happening when the property _properties is accessed.

Related issue (if applicable): related: #4210

I cannot actually check that this works because I do not have an Yeelight devices.

Checklist:

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

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

  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.
@houndci-bot

Some files could not be reviewed due to errors:

Traceback (most recent call last):
Traceback (most recent call last):
  File "/home/linters/.local/bin/flake8", line 7, in 
    from flake8.main.cli import main
ModuleNotFoundError: No module named 'flake8'
@@ -276,7 +276,9 @@ def hs_color(self) -> tuple:

@property
def _properties(self) -> dict:
return self._bulb.last_properties
if self._bulb_device is None:

This comment has been minimized.

@rohankapoorcom

rohankapoorcom Oct 28, 2018

Contributor

self._bulb_device is created by the _bulb property which is called in update so this should be accessible at some time after the first update has completed.

@bieniu

This comment has been minimized.

Contributor

bieniu commented Oct 28, 2018

I tried component with this change and for now everything works fine. There is no inormation in the log about the long state update so far.

@balloob

Good catch !

@balloob balloob merged commit 851d7e2 into home-assistant:dev Oct 29, 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 First build on yeelight-asyncio at 93.195%
Details

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

@rohankapoorcom rohankapoorcom deleted the rohankapoorcom:yeelight-asyncio branch Oct 29, 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