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

Add missing features for Soundbar #40

Merged
merged 6 commits into from
Nov 21, 2022
Merged

Conversation

Censored3
Copy link
Contributor

@Censored3 Censored3 commented Nov 13, 2022

Hello

I was really missing some features to control for my YAS-408 Soundbar, most notably the Subwoofer volume, so I added the features my Soundbar was missing. Most of the groundwork was already in place, it was really just implementing them. Would love to get this integrated upstream.

  • Subwoofer volume
  • Clear Voice
  • 3D Surround
  • Sound Program >> This is probably more universal across the models and needs translation strings in HA.

I put everything in seperate commits for easier review - feel free to squash.

I can deliver the getFeatures output for your reference at your request.

@Censored3
Copy link
Contributor Author

HA en translation strings, combination of MusicCast20 and YAS-408 lists:

        "yamaha_musiccast__zone_sound_program": {
            "standard": "Standard",
            "bass_booster": "Bass Booster",
            "stereo": "Stereo",
            "sports": "Sports",
            "game": "Game",
            "music": "Music",
            "tv_program": "TV Program",
            "movie": "Movie"
          }

And updated DEVICE_CLASS_MAPPING:

    "zone_SOUND_PROGRAM": "yamaha_musiccast__zone_sound_program",

@Censored3
Copy link
Contributor Author

In the meantime I discovered Sound Program could already be selected through the main entity in HA.
That makes use of the MusicCastDevice.select_sound_mode() method, which I renamed in an attempt to streamline things and only checked this repo for references, not the HA integration - so my change broke that function. I reverted that rename, now.
Imho it's more "clean" and streamlined to also create a separate entity for the sound program, so we can adjust it through automations.

Checked HA for references to the 3D Surround which I renamed as well to streamline, couldn't find any.

@micha91
Copy link
Collaborator

micha91 commented Nov 15, 2022

First of all thanks for your contribution, hopefully I will be able to review it by the mid of this week.

Regarding the selection of sound modes, I think we should keep things as they are, because it could be confusing to have two places to select the sound mode. In addition it is more consistent to the way other integrations set the sound mode. If you want to set the sound mode using automations or scripting, you can already do that by calling the service media_player.select_sound_mode. Imho there is only downside of the sound mode in the media_player widget - we cannot display any translations (a discussion regarding this issue can be found here.

@Censored3
Copy link
Contributor Author

thanks for your feedback regarding this, that makes sense, and also explains the use of the different method name select_sound_mode vs set_sound_program in the backend.
All right, let's just erase the sound program addition, then.

Copy link
Collaborator

@micha91 micha91 left a comment

Choose a reason for hiding this comment

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

If you remove the sound program capability, we should be good to go. I was able to test subwoofer volume and clear voice on my devices and they seem to work (at least the status updates on changes)

aiomusiccast/capability_registry.py Outdated Show resolved Hide resolved
@Censored3
Copy link
Contributor Author

Awesome, I reverted the Sound Program.
I have used these additions quite extensively for the past week, they worked perfectly for me (but I just have the one device supporting them).

@micha91 micha91 merged commit 68ed9b9 into vigonotion:main Nov 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants