Add sound mode support (media_player frontend) #815
Conversation
- this is part of the added sound mode support to the home assistant backend
Changed icon of the new input select. Tested the code and it works, for other media players that did not implement sound mode support the input select is nicely hidden (not present). For the Denonavr platform it works and sound mode is updated and can be changed in the front pannel.
|
|
|
|
| return isOff || !supportsSelectSoundMode; | ||
| } | ||
|
|
||
| computeSelectedSoundMode(stateObj) { |
There was a problem hiding this comment.
I have removed it
| @@ -45,6 +45,15 @@ | |||
| margin-left: 10px; | |||
| } | |||
|
|
|||
| iron-icon.sound-mode-input { | |||
There was a problem hiding this comment.
Same as source-input. You should combine them
There was a problem hiding this comment.
I combined both
- combine style classes of sound mode and source. - remove unused computeSelectedSoundMode
|
@c727 I commited the changes you proposed, can you now merge this PR? |
|
@c727 I tested the new changes and it seems to work with those unused parts of the code removed. |
|
I can't do this, you have to wait for Andrey, armills or balloob |
|
@andrey-git @armills @balloob, could one of you merge this PR? |
| @@ -106,6 +106,18 @@ | |||
| </paper-listbox> | |||
| </paper-dropdown-menu> | |||
| </div> | |||
| <!-- SOUND MODE PICKER --> | |||
| <div class='controls layout horizontal justified' | |||
| hidden$='[[computeHideSelectSoundMode(isOff, supportsSelectSoundMode)]]'> | |||
There was a problem hiding this comment.
I don't know exactly what you mean but I guess you mean to use a "dom-if" property in the
Again I am not an expert in how this works and what the diffrence is.
But the rest of the file is using the hidden$ property.
Basically I made a copy of the Source input, so everything is done in a similar way as the already implemented Source input.
Would it not be better to keep everything the same as the rest of the file for now.
Maybe someone else who does no what the diffrence is and nows how to use these properties correctly can change it for the whole file at once (also for the SourceInput, Power and Volume).
| @@ -359,6 +397,27 @@ | |||
| this.callService('select_source', { source: sourceInput }); | |||
| } | |||
|
|
|||
| handleSoundModeChanged(soundModeIndex, soundModeIndexOld) { | |||
| var soundModeInput; | |||
There was a problem hiding this comment.
No need to predeclare variables.
Just use const when you assign it.
There was a problem hiding this comment.
All handle's are done with var instead of const throught this file.
honestly I don't know the exact diffrence but I think it might be better to keep the file uniform and do the same thing as done in the rest of the file.
Basically I made a copy of the Source input, so everything is done in a similar way as the already implemented Source input.
|
@andrey-git Do you agree to keep the document uniform? |
|
@andrey-git Shall I try to implement the suggested changes or do you agree with my previous comment? |
Implemented changes suggested by @andrey-git
This comment has been minimized.
This comment has been minimized.
|
Closed as new supported features need to be discussed in the architecture repo first as per https://developers.home-assistant.io/docs/en/entity_index.html#changing-the-entity-model |
Added frontend support for an drop down box select for the sound mode (also updated with status).
This is general support for all media_player components.
This box is not shown in media_players that did not (yet) implement sound mode support in the backend.
I implemented the sound mode support (backend) for the denonavr platform and that one works perfictly.
I will make a pull request for the backend soon (still dooing some cleanup).
I tested this code and it works on Hassbian on my pi.
On the home assistant chat I heard that other platforms like onkyo and sonos could also implement sound mode support and probably most receivers could support this.
Backend pull request:
home-assistant/core#13706
Documentation pull request:
home-assistant/home-assistant.io#4470