Add condition to call the toggle service for media_players#4250
Add condition to call the toggle service for media_players#4250ktnrg45 wants to merge 2 commits into
Conversation
ktnrg45
changed the title
ktnrg45
changed the title
ktnrg45
changed the title
There was a problem hiding this comment.
Is standby an official state of the media player domain?
EDIT: Guess we'll wait for home-assistant/core#28897
Yes that would be preferable. Thanks for the approve! |
|
Has there already been an architecture issue for adding the standby state? |
|
Just this one: home-assistant/architecture#327 Which has been closed by the author. The parent PR in the backend is closed as well. |
|
Reopened architecture issue. Was going to just do a revert to resolve the backend issue which is why I closed it. |
|
@ktnrg45 is there a backend change pending then? |
No not currently. |
|
@ktnrg45 just gave my 2 cents in home-assistant/core#28891 to the whole thing, because I already put quite some time into this issue and now ended up monkey patching the pieces with a custom_component because lovelace UI, an mini-media-player custom UI and homekit are broken and no fix in sight. btw. I also stumbled over this select_source thing you describe, even worse, when using homekit the UI part is disabled and because the source attribute is missing in STATE_OFF, there's this nasty Sources out of sync. Restart Home Assistant error. |
Ok I just saw your comment here. Yeah I think using toggle would be best. Ill change it to this tonight. |
|
@ktnrg45 Nice! But watch out for is_on property, which is how ToggleEntity decides how to switch. In my opinion, a well chosen and unambiguous way to do so. But nearly nobody with a media_player has it yet. But it has to be implemented now for it to work. |
ktnrg45
force-pushed
the
media-player
branch
from
ec50a8d to
ed6acc0
Compare
ktnrg45
changed the title
|
|
||
| togglePower() { | ||
| if (this.isOff) { | ||
| if (this.supportsTurnOn && this.supportsTurnOff) { |
There was a problem hiding this comment.
This condition may not be necessary, we may be able to simply call toggle(). The Plex integration seems to be the only media_player that supports 'turnoff' but not 'turnon'. Used grep -rl 'SUPPORT_TURN_ON' homeassistant/components/*/media_player.py
There was a problem hiding this comment.
Honestly the Plex integration should probably not advertise as supporting either power on/off as it doesn't have the ability to do either.
There was a problem hiding this comment.
There are more that can't turn on. Philips js, arcam. They can support it with external wol or it blasters.
|
@bramkragten Sorry about taking so long to resolve this. I've changed this PR so that the toggle service would be called instead so that the backend would handle the toggling. |
|
This PR is no longer current with the recent changes in the media control card. Please create a new PR if needed. |
8 participants
Partial fix for Issue: home-assistant/core#28891.
Don't close issue.
Add condition to call the media_player entity's toggle service if turn on and turn off are both supported. This allows for the turn off/turn on logic to be handled in the backend via the toggle service.