-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
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
Add webos customize option to add custom sources #2561
Conversation
74d9b0f
to
3695941
Compare
@@ -100,6 +112,7 @@ def request_configuration(host, hass, add_devices): | |||
# pylint: disable=unused-argument | |||
def lgtv_configuration_callback(data): | |||
"""The actions to do when our configuration callback is called.""" | |||
# pylint: disable=no-value-for-parameter |
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 add this. You are actually introducing a bug. You added a parameter to setup_tv
and you are not passing that in here.
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.
right. didn't even notice as the tv is already paired. I added it because pylint complained about add_devices.
I'll fix the issue and check pairing again with the tv.
Shouldn't the pylint comment still be here for add_devices? then I'll pass another parameter so pylint will also complain about that one.
New logic means new unit test cases |
@Teagan42 we generally only test the pure Python code. So core, helpers, util and automation components. We do have some tests for other components if they are important (MQTT) or when a developer has provided tests. |
4b768b4
to
7b171ad
Compare
@@ -50,10 +61,11 @@ def setup_platform(hass, config, add_devices, discovery_info=None): | |||
if host in _CONFIGURING: | |||
return | |||
|
|||
setup_tv(host, hass, add_devices) | |||
customize = config.get(CONF_CUSTOMIZE, 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.
You're defaulting it to None
which means that if people do not have a customize config entry, the platform will crash. You need to default it to an empty dict {}
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.
right. i'll fix this and do checks with and without the additional config. thanks.
7b171ad
to
de5aa14
Compare
@@ -100,7 +112,8 @@ def request_configuration(host, hass, add_devices): | |||
# pylint: disable=unused-argument | |||
def lgtv_configuration_callback(data): | |||
"""The actions to do when our configuration callback is called.""" | |||
setup_tv(host, hass, add_devices) | |||
# pylint: disable=no-value-for-parameter |
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.
Please remove this directive. If it is needed it means you have a bug, if it is not needed it will prevent PyLint from finding bugs.
Currently there are only hw inputs in the sources list. Other interesting inputs can be live tv (dvbt) and vod apps. * add customize option for webos * add short names for livetv, youtube, mako apps * add current app as a source * use large icon (largeIcon is the same as icon if doesn't exists) * filter out hw inputs that are not connected Signed-off-by: Roi Dayan <[email protected]>
de5aa14
to
cd70de6
Compare
Bueno 🐬 |
Doc update: home-assistant/home-assistant.io@30b16d1 Feedback and improvements would be welcome 😉 |
Description:
Add webos customize option to add custom sources
Currently there are only hw inputs in the sources list.
Other interesting inputs can be live tv (dvbt) and vod apps.
Related issue (if applicable): fixes #
Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#
Example entry for
configuration.yaml
(if applicable):Checklist:
tox
run successfully. Your PR cannot be merged unless tests passSigned-off-by: Roi Dayan [email protected]