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.

Sign up for GitHub

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

Jump to bottom

Added service select_video_output and video_out attribute #18081

Merged
merged 8 commits into from Nov 2, 2018
Merged

Added service select_video_output and video_out attribute #18081

merged 8 commits into from Nov 2, 2018

Conversation

leothlon
Copy link
Contributor

@leothlon leothlon commented Nov 1, 2018 •
edited

Description:

Added service to change video hdmi output
service data "video_output" takes a value of:
'no', 'analog', 'yes', 'out', 'out-sub', 'sub', 'hdbaset', 'both', 'up'

Also adds attribute of "video_out" to display current output

Checklist:

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

@homeassistant
Copy link
Contributor

Hi @leothlon,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

@ghost ghost added the in progress label Nov 1, 2018
houndci-bot
houndci-bot reviewed Nov 1, 2018
View reviewed changes
homeassistant/components/media_player/onkyo.py
@@ -276,6 +301,10 @@ def play_media(self, media_type, media_id, **kwargs):
self.command('preset {}'.format(media_id))


def select_output(self, output):
Copy link

Choose a reason for hiding this comment

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

too many blank lines (2)

homeassistant/components/media_player/onkyo.py Outdated
for device in devices:
if service.service == SERVICE_SELECT_VIDEO_OUTPUT:
device.select_output(service.data.get(ATTR_VIDEO_OUTPUT))

Copy link

Choose a reason for hiding this comment

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

blank line contains whitespace

homeassistant/components/media_player/onkyo.py Outdated
"""Handle for services."""
entity_ids = service.data.get(ATTR_ENTITY_ID)
devices = [d for d in hosts if d.entity_id in entity_ids]

Copy link

Choose a reason for hiding this comment

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

blank line contains whitespace

homeassistant/components/media_player/onkyo.py Outdated
@@ -55,6 +55,10 @@

TIMEOUT_MESSAGE = 'Timeout waiting for response.'

ATTR_VIDEO_OUTPUT = 'video_output'
ONKYO_SELECT_OUTPUT_SCHEMA = vol.Schema({vol.Required(ATTR_ENTITY_ID): cv.entity_ids, vol.Required(ATTR_VIDEO_OUTPUT): vol.In(['no', 'analog', 'yes', 'out', 'out-sub', 'sub', 'hdbaset', 'both', 'up'])})
Copy link

Choose a reason for hiding this comment

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

line too long (202 > 79 characters)

homeassistant/components/media_player/onkyo.py Outdated Show resolved Hide resolved
houndci-bot
houndci-bot reviewed Nov 1, 2018
View reviewed changes
homeassistant/components/media_player/onkyo.py Outdated
ACCEPTED_VALUES = ['no', 'analog', 'yes', 'out',
'out-sub', 'sub', 'hdbaset', 'both', 'up']
ONKYO_SELECT_OUTPUT_SCHEMA = vol.Schema({vol.Required(ATTR_ENTITY_ID): cv.entity_ids,
vol.Required(ATTR_VIDEO_OUTPUT): vol.In(ACCEPTED_VALUES)})
Copy link

Choose a reason for hiding this comment

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

line too long (99 > 79 characters)

homeassistant/components/media_player/onkyo.py Outdated
ATTR_VIDEO_OUTPUT = 'video_output'
ACCEPTED_VALUES = ['no', 'analog', 'yes', 'out',
'out-sub', 'sub', 'hdbaset', 'both', 'up']
ONKYO_SELECT_OUTPUT_SCHEMA = vol.Schema({vol.Required(ATTR_ENTITY_ID): cv.entity_ids,
Copy link

Choose a reason for hiding this comment

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

line too long (85 > 79 characters)

balloob
balloob reviewed Nov 1, 2018
View reviewed changes
homeassistant/components/media_player/onkyo.py Outdated
vol.Required(ATTR_VIDEO_OUTPUT): vol.In(ACCEPTED_VALUES)
})
DOMAIN = 'media_player'
SERVICE_SELECT_VIDEO_OUTPUT = 'select_video_output'
Copy link
Member

Choose a reason for hiding this comment

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

prefix with platform name.

balloob
balloob reviewed Nov 1, 2018
View reviewed changes
homeassistant/components/media_player/onkyo.py Outdated
vol.Required(ATTR_ENTITY_ID): cv.entity_ids,
vol.Required(ATTR_VIDEO_OUTPUT): vol.In(ACCEPTED_VALUES)
})
DOMAIN = 'media_player'
Copy link
Member

Choose a reason for hiding this comment

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

import domain.

balloob
balloob reviewed Nov 1, 2018
View reviewed changes
homeassistant/components/media_player/onkyo.py
@@ -55,6 +56,16 @@

TIMEOUT_MESSAGE = 'Timeout waiting for response.'

ATTR_VIDEO_OUTPUT = 'video_output'
ACCEPTED_VALUES = ['no', 'analog', 'yes', 'out',
Copy link
Member

Choose a reason for hiding this comment

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

is this valid for all onkyos?

Copy link
Contributor Author

@leothlon leothlon Nov 1, 2018 •
edited

Choose a reason for hiding this comment

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

Those are the accepted values according to the onkyo-eiscp library, not all of them would work on all recievers i dont think (for me "out" switches to main and "out-sub" switches to sub, "sub" switches to both. so its not totally clear what to use but rather trial and error, planning to update this in the docs when i get time).

The command would only work for recievers with more than one hdmi-out port ofc.

Other than that i wouldn't really know as i only got one reciever to test with. (onkyo-eiscp said the command is for Japanese models but i got a European one and it works fine).

@balloob balloob merged commit 8613694 into home-assistant:dev Nov 2, 2018
@ghost ghost removed the in progress label Nov 2, 2018
@balloob
Copy link
Member

balloob commented Nov 2, 2018

Please also open a PR for the docs repo.

@leothlon leothlon mentioned this pull request Nov 2, 2018
Merged
1 task
@leothlon
Copy link
Contributor Author

leothlon commented Nov 2, 2018 •
edited

PR for docs made, home-assistant/home-assistant.io#7334

@balloob balloob mentioned this pull request Nov 9, 2018
Merged
@bmesuere bmesuere mentioned this pull request Nov 14, 2018
Closed
@BryanJacobs
Copy link
Contributor

BryanJacobs commented Nov 25, 2018 •
edited by MartinHjelmare

This commit breaks my Home Assistant.

2018-11-26 08:05:37 ERROR (MainThread) [homeassistant.components.media_player] onkyo: Error on device update!                                                                                                     
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/homeassistant/helpers/entity_platform.py", line 251, in _async_add_entity                                                                                          
    await entity.async_device_update(warning=False)                                                                                                                                                               
  File "/usr/local/lib/python3.6/site-packages/homeassistant/helpers/entity.py", line 349, in async_device_update                                                                                                 
    await self.hass.async_add_executor_job(self.update)                                                                                                                                                           
  File "/usr/local/lib/python3.6/concurrent/futures/thread.py", line 56, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/usr/local/lib/python3.6/site-packages/homeassistant/components/media_player/onkyo.py", line 223, in update                                                                                               
    self._attributes["video_out"] = ','.join(hdmi_out_raw[1])
TypeError: 'bool' object is not subscriptable

@MartinHjelmare
Copy link
Member

MartinHjelmare commented Nov 26, 2018 •
edited

That's fixed in #18478.

Please don't comment in merged PRs. Open an issue instead.

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Nov 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@houndci-bot houndci-bot houndci-bot left review comments

@balloob balloob balloob approved these changes

Assignees
No one assigned
Labels
cla-signed integration: onkyo
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@leothlon @homeassistant @balloob @BryanJacobs @MartinHjelmare @houndci-bot