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

Fix for errors on missing preview on LG webos TV #6755

Merged
merged 2 commits into from
Apr 20, 2017
Merged

Fix for errors on missing preview on LG webos TV #6755

merged 2 commits into from
Apr 20, 2017

Conversation

masarliev
Copy link
Contributor

Description:

largeIcon property on my TV and I guess other versions is file path. That's why it can not be requested as HTTP URI. I changed it to icon property that is the icon of current playing source.

Related issue (if applicable): fixes #5113

@mention-bot
Copy link

@masarliev, thanks for your PR! By analyzing the history of the files in this pull request, we identified @TheRealLink, @trisk and @fabaff to be potential reviewers.

@andrey-git
Copy link
Contributor

@masarliev Could you explain the issue with largeIcon?
It was specifically changed in #2561 by @roidayan

@andrey-git
Copy link
Contributor

OK, I see details in the linked issue.
What is the value of self._app_list[self._current_source_id]['icon'] ?

@masarliev
Copy link
Contributor Author

I don't remember exact value but is something like http://ip-address:port/path/to/icon.png
and this icon is current active source

@roidayan
Copy link
Contributor

Maybe it depends on webos version
Icon is so small it looked bad in the ui than using large icon.
Maybe should use large icon if possible or fallback to icon.
Large icon should be accessible from URL may e your webos version just missing the http URL prefix?
Can you show large icon and icon strings together ?
I'll check it as well later when I can.

@roidayan
Copy link
Contributor

I looked in the paths in the issue. so it looks like it depends in the webos version. the issue specify webos v1.4 while in my webos v2 both icon and largeIcon are urls.

'icon': 'http://192.168.31.64:3000/resources/09216e4e921f2209fe0749fbbf99952236fd032e/icon.png',
'largeIcon': 'http://192.168.31.64:3000/resources/27f9d4b79b9d1caf3530f1d8cd25cfa79591e19c/livetv_130x130.png'

bummer. I guess we need to use icon (or use largeIcon if it's url and fallback to icon if not)

@@ -240,7 +240,7 @@ def media_content_type(self):
def media_image_url(self):
"""Image url of current playing media."""
if self._current_source_id in self._app_list:
return self._app_list[self._current_source_id]['largeIcon']
return self._app_list[self._current_source_id]['icon']
Copy link
Contributor

Choose a reason for hiding this comment

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

in webos v2 largeIcon is a url and looks better so I suggest maybe check if largeIcon is not a url fallback to icon?

e.g.

if self._current_source_id in self._app_list:
  icon = self._app_list[self._current_source_id]['largeIcon']
  if not icon.startswith('http'):
    icon = self._app_list[self._current_source_id]['icon']
  return icon
return None

any thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LGTM. I will change like this

@roidayan
Copy link
Contributor

LGTM. the 2 commits could be squashed into 1 in my opinion.

@balloob
Copy link
Member

balloob commented Apr 20, 2017

I have a squash and merge button @roidayan 👍

@balloob balloob merged commit 2d5ab52 into home-assistant:dev Apr 20, 2017
@balloob balloob mentioned this pull request Apr 21, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Aug 12, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LG webOS lead to exceptions when HA tries to show preview
6 participants