-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
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
Code cleanup of velux scene #12390
Code cleanup of velux scene #12390
Conversation
"""Activate the scene.""" | ||
self.hass.async_add_job(self.scene.run()) | ||
yield from self.scene.run() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is scene.run
a coroutine?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also please remove unnecessary return from setup_platform
.
After this cleanup, the velux component can also be cleaned up, eg by removing the initialized
and hass
attributes.
Does this have anything to do with the knx component, or is that a copy/paste leftover in the description?
@@ -4,6 +4,7 @@ | |||
For more details about this platform, please refer to the documentation at | |||
https://home-assistant.io/components/scene.velux/ | |||
""" | |||
import asyncio | |||
from homeassistant.components.scene import Scene |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a blank line between standard library and homeassistant imports.
if DATA_VELUX not in hass.data \ | ||
or not hass.data[DATA_VELUX].initialized: | ||
return False | ||
|
||
entities = [] | ||
for scene in hass.data[DATA_VELUX].pyvlx.scenes: | ||
entities.append(VeluxScene(hass, scene)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't pass in hass
.
@@ -13,10 +14,6 @@ | |||
|
|||
def setup_platform(hass, config, add_devices, discovery_info=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this doesn't do any I/O, you can change this to the async version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
Description:
Code cleanup
def is_on
does not exist within sceneshould_poll
is already Falseasync_activate
DATA_VELUX
always present inhass.data
if base component was set up correctly.