Add properties for surround speakers in TV, music and ambient modes #780

rytilahti · 2020-12-27T17:55:12Z

Adds following (setable) properties (see https://support.sonos.com/s/article/4804?language=en_US):

~~surround_mode: surround enabled/disabled~~ (already done in Add sub & surround properties and controls #870)
surround_ambient_enabled: ambient/full volume in the music mode (bool)
surround_volume_tv: surround volume adjustment [-15,15] for the tv mode (int)
surround_volume_music: surround volume adjustment [15,15] for the music mode (int)

Currently missing reseting to defaults, and adjusting the surround speaker balance (which can only be done when trueplay is disabled, see #646 (review)).

Obsoletes #646. Fixes #461.

rytilahti · 2020-12-27T20:17:35Z

Just a side note, after doing some testing (and XML checking), it looks like that changes to the audio input format is not sending events -> requires polling to get this information :-(

rytilahti · 2021-05-16T16:17:45Z

@pwt do you mind giving this one a quick review? What do you think should be done to get this closer to get merged? Something besides fixing the docstrings & separating the soundbar_audio_input_format?

soco/core.py

pwt · 2021-05-18T08:48:58Z

@pwt do you mind giving this one a quick review? What do you think should be done to get this closer to get merged? Something besides fixing the docstrings & separating the soundbar_audio_input_format?

This is a great. I've made a couple of comments: if you can push a new commit with the f-string replaced, that should initiate a check job run to spot any hygiene issues.

One problem I have is that I'm not able to test this code, since none of my Sonos equipment is set up in HT configs, so I'd love to get a some other folks to try it out.

soco/core.py

soco/services.py

rytilahti · 2021-05-18T15:48:55Z

Thanks @pwt! I added some responses in the comments, and also created a separate PR (#832) for the audio input format so that's why I force-pushed this branch.

I was working to add tests to the functionality when I realized that the helper methods I introduced makes testing the API interface more complicated thanks to indirection. I'll have to look for a suitable solution for that, I'm open for suggestions!

coveralls · 2021-05-18T15:54:40Z

Coverage increased (+0.08%) to 54.254% when pulling 1b34036 on rytilahti:feat/add_surround_options into d1744f8 on SoCo:master.

pwt · 2021-07-08T11:19:05Z

I was working to add tests to the functionality when I realized that the helper methods I introduced makes testing the API interface more complicated thanks to indirection. I'll have to look for a suitable solution for that, I'm open for suggestions!

I may not have understood this fully, but I'd be minded to keep the unit tests simple and just pattern them after the existing tests for simple getters/setters.

Adds following (setable) properties (see https://support.sonos.com/s/article/4804?language=en_US): * surround_mode: surround enabled/disabled * surround_mode_music_playback: ambient/full volume in the music mode * surround_volume_tv: surround volume adjustment [-15,15] for the tv mode * surround_volume_music: surround volume adjustment [15,15] for the music mode Missing settings for reseting to defaults, and adjusting the surround speaker balance (which can only be done when trueplay is disabled)

rytilahti · 2022-02-16T19:09:44Z

Oops, looks like I forgot to push this earlier, but I think this should be good to go now. Mind taking a look @pwt & @jjlawren?

I added tests to all surround properties (including the already existing surround_enabled) and renamed the surround_music_playback to surround_ambient_enabled.

edit: pre-commit config pins down to a version that's different from what's used by the CI, that's why all the hassle with force pushes, sorry for that. pre-commit autoupdate should be run to pin the version to the newest, opinions on whether it should be done in this PR or in a separate one?

pwt · 2022-02-17T20:42:01Z

Thanks for following up on this. I'll take a look in the next day or two.

pwt · 2022-02-19T14:40:28Z

Hi @rytilahti . I've reviewed the code and it looks good. I've also pushed a couple of suggested commits that:

Fix a docstring reference error and tidy up some of the doctstring text.
Remove the type hints. (I'm an advocate of using typing, but I think we should maintain consistency with the rest of the SoCo codebase until such time as we decide to add typing more universally.)

Please review and check that the changes look OK to you.

Unfortunately, I'm not able to test this PR properly as I don't have any Sonos HT setups, so I'll have to depend on the testing that you and others are able to do.

rytilahti · 2022-02-20T17:03:57Z

Hi @pwt and thanks for the review and the changes you made, looks good to me! I re-tested all the new settings on my Beam setup with the most current software and they work as expected.

pwt · 2022-02-20T17:15:46Z

@rytilahti Great! Thanks for the contribution.

rytilahti · 2022-02-20T18:17:40Z

soco/core.py

+    @surround_ambient_enabled.setter
+    @only_on_soundbars
+    def surround_ambient_enabled(self, value):
+        """Toggle surround music playback mode. True = ambient mode.


I just realized that having the ambient mode as True makes using the eventing API awkward (ambient mode is report as 0), so this may need to be changed to be other way around. IIRC I didn't do it previously as I couldn't think of a good name for the "full mode". Maybe surround_full_volume_enabled?

rytilahti · 2022-02-20T18:18:17Z

tests/test_core.py

+            if variable == "SurroundMode":  # negate desired value for surround mode
+                desired_value = not value


This could be removed if the surround mode would be reversed.

pwt force-pushed the master branch from 5be4ea6 to 3465547 Compare February 4, 2021 15:49

pwt mentioned this pull request May 16, 2021

Surround Sound Mode #461

Closed

pwt self-assigned this May 16, 2021

pwt reviewed May 18, 2021

View reviewed changes

soco/core.py Outdated Show resolved Hide resolved

pwt reviewed May 18, 2021

View reviewed changes

soco/core.py Outdated Show resolved Hide resolved

pwt added this to the 0.23.0 milestone May 18, 2021

rytilahti mentioned this pull request May 18, 2021

Add SoCo method to determine HDMI/Optical input audio format #832

Merged

rytilahti commented May 18, 2021

View reviewed changes

soco/core.py Outdated Show resolved Hide resolved

soco/core.py Outdated Show resolved Hide resolved

soco/services.py Outdated Show resolved Hide resolved

rytilahti force-pushed the feat/add_surround_options branch from 3d74eca to 5ced306 Compare May 18, 2021 15:44

pwt modified the milestones: 0.23.0, 0.24.0 Jul 6, 2021

svrooij mentioned this pull request Jul 22, 2021

Documentation: Eq variables svrooij/sonos-api-docs#16

Closed

pwt modified the milestones: 0.24.0, 0.25.0 Sep 1, 2021

pwt modified the milestones: 0.25.0, 0.26.0 Dec 3, 2021

rytilahti force-pushed the feat/add_surround_options branch from 1b34036 to eb703f9 Compare December 14, 2021 00:44

pwt modified the milestones: 0.26.0, 0.27.0 Jan 23, 2022

rytilahti added 2 commits February 16, 2022 19:43

Add set_eq_variable and get_eq_variable helpers for RenderingControl

91880a6

rytilahti force-pushed the feat/add_surround_options branch from eb703f9 to f141533 Compare February 16, 2022 18:44

rytilahti changed the title ~~[WIP/RFC] Improve soundbar support~~ Improve soundbar support Feb 16, 2022

Fix docstrings, remove f-string usage

d2b1f58

rytilahti added 4 commits February 16, 2022 20:00

Remove {get,set}_eq_variable() helpers, surround_mode

27d9358

Use @only_on_soundbars for surround_enabled setter

f542a78

Rename surround_music_playback -> surround_ambiend_enabled, add tests

a2bf614

Add appdirs as dep for pylint hook

dee8588

rytilahti force-pushed the feat/add_surround_options branch from f3d1677 to c8f565f Compare February 16, 2022 19:00

Fix linting

c2bfea1

rytilahti force-pushed the feat/add_surround_options branch from c8f565f to c2bfea1 Compare February 16, 2022 19:07

pwt added 2 commits February 19, 2022 14:29

Docstring fixes/modifications

acd9134

Remove type hints for code consistency

421fd38

pwt changed the title ~~Improve soundbar support~~ Add properties for surround speakers in TV, music and ambient modes Feb 20, 2022

pwt merged commit 52b798f into SoCo:master Feb 20, 2022

rytilahti mentioned this pull request Feb 20, 2022

Add more surround settings for Sonos home-assistant/core#66962

Closed

22 tasks

rytilahti deleted the feat/add_surround_options branch February 20, 2022 18:15

rytilahti commented Feb 20, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add properties for surround speakers in TV, music and ambient modes #780

Add properties for surround speakers in TV, music and ambient modes #780

rytilahti commented Dec 27, 2020 •

edited

rytilahti commented Dec 27, 2020

rytilahti commented May 16, 2021

pwt commented May 18, 2021

rytilahti commented May 18, 2021

coveralls commented May 18, 2021 •

edited

pwt commented Jul 8, 2021

rytilahti commented Feb 16, 2022

pwt commented Feb 17, 2022

pwt commented Feb 19, 2022

rytilahti commented Feb 20, 2022

pwt commented Feb 20, 2022

rytilahti Feb 20, 2022 •

edited

rytilahti Feb 20, 2022

		if variable == "SurroundMode": # negate desired value for surround mode
		desired_value = not value

Add properties for surround speakers in TV, music and ambient modes #780

Add properties for surround speakers in TV, music and ambient modes #780

Conversation

rytilahti commented Dec 27, 2020 • edited

rytilahti commented Dec 27, 2020

rytilahti commented May 16, 2021

pwt commented May 18, 2021

rytilahti commented May 18, 2021

coveralls commented May 18, 2021 • edited

pwt commented Jul 8, 2021

rytilahti commented Feb 16, 2022

pwt commented Feb 17, 2022

pwt commented Feb 19, 2022

rytilahti commented Feb 20, 2022

pwt commented Feb 20, 2022

rytilahti Feb 20, 2022 • edited

Choose a reason for hiding this comment

rytilahti Feb 20, 2022

Choose a reason for hiding this comment

rytilahti commented Dec 27, 2020 •

edited

coveralls commented May 18, 2021 •

edited

rytilahti Feb 20, 2022 •

edited