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

PS4: When selecting source (ie from a service call), add support for turning the device on #22005

Closed
wants to merge 3 commits into
base: dev
from

Conversation

Projects
None yet
7 participants
@heikkih
Copy link

heikkih commented Mar 13, 2019

Description:

PS4: When selecting source (ie from a service call), add support for turning the device on

This allows HA to turn on a PS4, and switch to the selected title in the same call (handles a synchronous wait until the device is turned on, before selecting the source.

Checklist:

  • [V] The code change is tested and works locally.
  • [V] Local tests pass with tox. Your PR cannot be merged unless tests pass
  • [V] There is no commented out code in this PR.

If user exposed functionality or configuration variables are added/changed:

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

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

If the code does not interact with devices:

  • [NA] Tests have been added to verify that the new code works.
@homeassistant

This comment has been minimized.

Copy link

homeassistant commented Mar 13, 2019

Hi @heikkih,

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!

@@ -375,7 +375,11 @@ def media_stop(self):
self.send_remote_control('ps')

def select_source(self, source):
"""Select input source."""
"""Select input source. If device is off, turn it on and wait until it's on."""

This comment has been minimized.

@houndci-bot

houndci-bot Mar 13, 2019

line too long (87 > 79 characters)

@homeassistant homeassistant added cla-signed and removed cla-needed labels Mar 13, 2019

if (self._state == STATE_OFF):
self.turn_on()
while (self._state == STATE_OFF):
pass

This comment has been minimized.

@amelchio

amelchio Mar 13, 2019

Member

This will eat all CPU until the state changes, that is not acceptable.

You will probably need a synchronization primitive such as asyncio.Event but that is a bit tricky because this integration is not async.

This comment has been minimized.

@heikkih

heikkih Mar 13, 2019

Author

Would a time.sleep(1) be acceptable?

This comment has been minimized.

@cgtobi

cgtobi Mar 13, 2019

Collaborator

No, sleeping isn't fine either I'd say.

This comment has been minimized.

@bachya

bachya Mar 13, 2019

Contributor

We definitely can't use time.sleep in our thread pool. Agreed with @amelchio: you need some way for the underlying component to notify you that the system is on.

This comment has been minimized.

@heikkih

heikkih Mar 14, 2019

Author

Thanks for the feedback. I'll with withdraw this PR og look into solving this some other way.

@elupus

This comment has been minimized.

Copy link
Contributor

elupus commented Mar 13, 2019

Use a scene to turn on and change source. No need to handle it in the service.
state: on
source: blaha

May need some block after the turn_on service has fired before accepting next service.

@heikkih

This comment has been minimized.

Copy link
Author

heikkih commented Mar 14, 2019

May need some block after the turn_on service has fired before accepting next service.

Thanks for the feedback. I'll look into a solution with scenes and if there is some way to check for a state-change there.

@heikkih heikkih closed this Mar 14, 2019

@wafflebot wafflebot bot removed the in progress label Mar 14, 2019

@amelchio

This comment has been minimized.

Copy link
Member

amelchio commented Mar 14, 2019

FYI, you should be able to achieve this with wait_template in an automation.

@heikkih

This comment has been minimized.

Copy link
Author

heikkih commented Mar 14, 2019

FYI, you should be able to achieve this with wait_template in an automation.

Thanks, Anders.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.