diff --git a/docs/api/core.rst b/docs/api/core.rst index 21ff79f5..27ab2f57 100644 --- a/docs/api/core.rst +++ b/docs/api/core.rst @@ -64,6 +64,14 @@ Manages the music library, e.g. searching for tracks to be added to a playlist. :members: +Mixer controller +================ + +Manages volume and muting. + +.. autoclass:: mopidy.core.MixerController + :members: + Core listener ============= diff --git a/docs/changelog.rst b/docs/changelog.rst index 8b4a35e6..01476404 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -17,6 +17,10 @@ v0.20.0 (UNRELEASED) - Added :class:`mopidy.core.HistoryController` which keeps track of what tracks have been played. (Fixes: :issue:`423`, PR: :issue:`803`) +- Added :class:`mopidy.core.MixerController` which keeps track of volume and + mute. The old methods on :class:`mopidy.core.PlaybackController` for volume + and mute management has been deprecated. (Fixes: :issue:`962`) + - Removed ``clear_current_track`` keyword argument to :meth:`mopidy.core.Playback.stop`. It was a leaky internal abstraction, which was never intended to be used externally. diff --git a/mopidy/core/__init__.py b/mopidy/core/__init__.py index 7fa7e299..720f9c38 100644 --- a/mopidy/core/__init__.py +++ b/mopidy/core/__init__.py @@ -5,6 +5,7 @@ from .actor import Core from .history import HistoryController from .library import LibraryController from .listener import CoreListener +from .mixer import MixerController from .playback import PlaybackController, PlaybackState from .playlists import PlaylistsController from .tracklist import TracklistController diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 94d4a71b..bc8df64d 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -11,6 +11,7 @@ from mopidy.audio.utils import convert_tags_to_track from mopidy.core.history import HistoryController from mopidy.core.library import LibraryController from mopidy.core.listener import CoreListener +from mopidy.core.mixer import MixerController from mopidy.core.playback import PlaybackController from mopidy.core.playlists import PlaylistsController from mopidy.core.tracklist import TracklistController @@ -31,6 +32,10 @@ class Core( """The playback history controller. An instance of :class:`mopidy.core.HistoryController`.""" + mixer = None + """The mixer controller. An instance of + :class:`mopidy.core.MixerController`.""" + playback = None """The playback controller. An instance of :class:`mopidy.core.PlaybackController`.""" @@ -49,15 +54,10 @@ class Core( self.backends = Backends(backends) self.library = LibraryController(backends=self.backends, core=self) - self.history = HistoryController() - - self.playback = PlaybackController( - mixer=mixer, backends=self.backends, core=self) - - self.playlists = PlaylistsController( - backends=self.backends, core=self) - + self.mixer = MixerController(mixer=mixer) + self.playback = PlaybackController(backends=self.backends, core=self) + self.playlists = PlaylistsController(backends=self.backends, core=self) self.tracklist = TracklistController(core=self) self.audio = audio diff --git a/mopidy/core/mixer.py b/mopidy/core/mixer.py new file mode 100644 index 00000000..e6856f17 --- /dev/null +++ b/mopidy/core/mixer.py @@ -0,0 +1,64 @@ +from __future__ import absolute_import, unicode_literals + +import logging + + +logger = logging.getLogger(__name__) + + +class MixerController(object): + pykka_traversable = True + + def __init__(self, mixer): + self._mixer = mixer + self._volume = None + self._mute = False + + def get_volume(self): + """Get the volume. + + Integer in range [0..100] or :class:`None` if unknown. + + The volume scale is linear. + """ + if self._mixer: + return self._mixer.get_volume().get() + else: + # For testing + return self._volume + + def set_volume(self, volume): + """Set the volume. + + The volume is defined as an integer in range [0..100]. + + The volume scale is linear. + """ + if self._mixer: + self._mixer.set_volume(volume) + else: + # For testing + self._volume = volume + + def get_mute(self): + """Get mute state. + + :class:`True` if muted, :class:`False` otherwise. + """ + if self._mixer: + return self._mixer.get_mute().get() + else: + # For testing + return self._mute + + def set_mute(self, mute): + """Set mute state. + + :class:`True` to mute, :class:`False` to unmute. + """ + mute = bool(mute) + if self._mixer: + self._mixer.set_mute(mute) + else: + # For testing + self._mute = mute diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index fc273965..d4cdce0d 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -2,6 +2,7 @@ from __future__ import absolute_import, unicode_literals import logging import urlparse +import warnings from mopidy.audio import PlaybackState from mopidy.core import listener @@ -11,20 +12,16 @@ from mopidy.utils.deprecation import deprecated_property logger = logging.getLogger(__name__) -# TODO: split mixing out from playback? class PlaybackController(object): pykka_traversable = True - def __init__(self, mixer, backends, core): - self.mixer = mixer + def __init__(self, backends, core): self.backends = backends self.core = core self._current_tl_track = None self._current_metadata_track = None self._state = PlaybackState.STOPPED - self._volume = None - self._mute = False def _get_backend(self): # TODO: take in track instead @@ -139,64 +136,59 @@ class PlaybackController(object): """ def get_volume(self): - """Get the volume. - - Integer in range [0..100] or :class:`None` if unknown. - - The volume scale is linear. """ - if self.mixer: - return self.mixer.get_volume().get() - else: - # For testing - return self._volume + ... deprecated:: 0.20 + Use :meth:`core.mixer.get_volume() + ` instead. + """ + warnings.warn( + 'playback.get_volume() is deprecated', DeprecationWarning) + return self.core.mixer.get_volume() def set_volume(self, volume): - """Set the volume. - - The volume is defined as an integer in range [0..100]. - - The volume scale is linear. """ - if self.mixer: - self.mixer.set_volume(volume) - else: - # For testing - self._volume = volume + ... deprecated:: 0.20 + Use :meth:`core.mixer.set_volume() + ` instead. + """ + warnings.warn( + 'playback.set_volume() is deprecated', DeprecationWarning) + return self.core.mixer.set_volume(volume) volume = deprecated_property(get_volume, set_volume) """ .. deprecated:: 0.20 - Use :meth:`get_volume` and :meth:`set_volume` instead. + Use :meth:`core.mixer.get_volume() + ` and + :meth:`core.mixer.set_volume() + ` instead. """ def get_mute(self): - """Get mute state. - - :class:`True` if muted, :class:`False` otherwise. """ - if self.mixer: - return self.mixer.get_mute().get() - else: - # For testing - return self._mute - - def set_mute(self, value): - """Set mute state. - - :class:`True` to mute, :class:`False` to unmute. + ... deprecated:: 0.20 + Use :meth:`core.mixer.get_mute() + ` instead. """ - value = bool(value) - if self.mixer: - self.mixer.set_mute(value) - else: - # For testing - self._mute = value + warnings.warn('playback.get_mute() is deprecated', DeprecationWarning) + return self.core.mixer.get_mute() + + def set_mute(self, mute): + """ + ... deprecated:: 0.20 + Use :meth:`core.mixer.set_mute() + ` instead. + """ + warnings.warn('playback.set_mute() is deprecated', DeprecationWarning) + return self.core.mixer.set_mute(mute) mute = deprecated_property(get_mute, set_mute) """ .. deprecated:: 0.20 - Use :meth:`get_mute` and :meth:`set_mute` instead. + Use :meth:`core.mixer.get_mute() + ` and + :meth:`core.mixer.set_mute() + ` instead. """ # Methods diff --git a/mopidy/http/handlers.py b/mopidy/http/handlers.py index 721e419c..52bd8217 100644 --- a/mopidy/http/handlers.py +++ b/mopidy/http/handlers.py @@ -43,6 +43,7 @@ def make_jsonrpc_wrapper(core_actor): 'core.get_version': core.Core.get_version, 'core.history': core.HistoryController, 'core.library': core.LibraryController, + 'core.mixer': core.MixerController, 'core.playback': core.PlaybackController, 'core.playlists': core.PlaylistsController, 'core.tracklist': core.TracklistController, @@ -54,6 +55,7 @@ def make_jsonrpc_wrapper(core_actor): 'core.get_version': core_actor.get_version, 'core.history': core_actor.history, 'core.library': core_actor.library, + 'core.mixer': core_actor.mixer, 'core.playback': core_actor.playback, 'core.playlists': core_actor.playlists, 'core.tracklist': core_actor.tracklist, diff --git a/mopidy/mpd/protocol/audio_output.py b/mopidy/mpd/protocol/audio_output.py index 4a5310f5..0152f852 100644 --- a/mopidy/mpd/protocol/audio_output.py +++ b/mopidy/mpd/protocol/audio_output.py @@ -13,7 +13,7 @@ def disableoutput(context, outputid): Turns an output off. """ if outputid == 0: - context.core.playback.set_mute(False) + context.core.mixer.set_mute(False) else: raise exceptions.MpdNoExistError('No such audio output') @@ -28,7 +28,7 @@ def enableoutput(context, outputid): Turns an output on. """ if outputid == 0: - context.core.playback.set_mute(True) + context.core.mixer.set_mute(True) else: raise exceptions.MpdNoExistError('No such audio output') @@ -55,7 +55,7 @@ def outputs(context): Shows information about all outputs. """ - muted = 1 if context.core.playback.get_mute().get() else 0 + muted = 1 if context.core.mixer.get_mute().get() else 0 return [ ('outputid', 0), ('outputname', 'Mute'), diff --git a/mopidy/mpd/protocol/playback.py b/mopidy/mpd/protocol/playback.py index 07102492..f7856a03 100644 --- a/mopidy/mpd/protocol/playback.py +++ b/mopidy/mpd/protocol/playback.py @@ -397,7 +397,7 @@ def setvol(context, volume): - issues ``setvol 50`` without quotes around the argument. """ # NOTE: we use INT as clients can pass in +N etc. - context.core.playback.volume = min(max(0, volume), 100) + context.core.mixer.set_volume(min(max(0, volume), 100)) @protocol.commands.add('single', state=protocol.BOOL) diff --git a/mopidy/mpd/protocol/status.py b/mopidy/mpd/protocol/status.py index eabb9317..d33e0afa 100644 --- a/mopidy/mpd/protocol/status.py +++ b/mopidy/mpd/protocol/status.py @@ -175,7 +175,7 @@ def status(context): futures = { 'tracklist.length': context.core.tracklist.length, 'tracklist.version': context.core.tracklist.version, - 'playback.volume': context.core.playback.volume, + 'mixer.volume': context.core.mixer.get_volume(), 'tracklist.consume': context.core.tracklist.consume, 'tracklist.random': context.core.tracklist.random, 'tracklist.repeat': context.core.tracklist.repeat, @@ -289,7 +289,7 @@ def _status_time_total(futures): def _status_volume(futures): - volume = futures['playback.volume'].get() + volume = futures['mixer.volume'].get() if volume is not None: return volume else: diff --git a/tests/core/test_mixer.py b/tests/core/test_mixer.py new file mode 100644 index 00000000..e3fa6be6 --- /dev/null +++ b/tests/core/test_mixer.py @@ -0,0 +1,28 @@ +from __future__ import absolute_import, unicode_literals + +import unittest + +from mopidy import core + + +class CoreMixerTest(unittest.TestCase): + def setUp(self): # noqa: N802 + self.core = core.Core(mixer=None, backends=[]) + + def test_volume(self): + self.assertEqual(self.core.mixer.get_volume(), None) + + self.core.mixer.set_volume(30) + + self.assertEqual(self.core.mixer.get_volume(), 30) + + self.core.mixer.set_volume(70) + + self.assertEqual(self.core.mixer.get_volume(), 70) + + def test_mute(self): + self.assertEqual(self.core.mixer.get_mute(), False) + + self.core.mixer.set_mute(True) + + self.assertEqual(self.core.mixer.get_mute(), True) diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index b9d19966..40741e23 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -426,21 +426,3 @@ class CorePlaybackTest(unittest.TestCase): self.assertFalse(self.playback2.get_time_position.called) # TODO Test on_tracklist_change - - def test_volume(self): - self.assertEqual(self.core.playback.volume, None) - - self.core.playback.volume = 30 - - self.assertEqual(self.core.playback.volume, 30) - - self.core.playback.volume = 70 - - self.assertEqual(self.core.playback.volume, 70) - - def test_mute(self): - self.assertEqual(self.core.playback.mute, False) - - self.core.playback.mute = True - - self.assertEqual(self.core.playback.mute, True) diff --git a/tests/mpd/protocol/test_audio_output.py b/tests/mpd/protocol/test_audio_output.py index 137ac029..a86f24f0 100644 --- a/tests/mpd/protocol/test_audio_output.py +++ b/tests/mpd/protocol/test_audio_output.py @@ -5,12 +5,12 @@ from tests.mpd import protocol class AudioOutputHandlerTest(protocol.BaseTestCase): def test_enableoutput(self): - self.core.playback.mute = False + self.core.mixer.set_mute(False) self.send_request('enableoutput "0"') self.assertInResponse('OK') - self.assertEqual(self.core.playback.mute.get(), True) + self.assertEqual(self.core.mixer.get_mute().get(), True) def test_enableoutput_unknown_outputid(self): self.send_request('enableoutput "7"') @@ -18,12 +18,12 @@ class AudioOutputHandlerTest(protocol.BaseTestCase): self.assertInResponse('ACK [50@0] {enableoutput} No such audio output') def test_disableoutput(self): - self.core.playback.mute = True + self.core.mixer.set_mute(True) self.send_request('disableoutput "0"') self.assertInResponse('OK') - self.assertEqual(self.core.playback.mute.get(), False) + self.assertEqual(self.core.mixer.get_mute().get(), False) def test_disableoutput_unknown_outputid(self): self.send_request('disableoutput "7"') @@ -32,7 +32,7 @@ class AudioOutputHandlerTest(protocol.BaseTestCase): 'ACK [50@0] {disableoutput} No such audio output') def test_outputs_when_unmuted(self): - self.core.playback.mute = False + self.core.mixer.set_mute(False) self.send_request('outputs') @@ -42,7 +42,7 @@ class AudioOutputHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_outputs_when_muted(self): - self.core.playback.mute = True + self.core.mixer.set_mute(True) self.send_request('outputs') diff --git a/tests/mpd/protocol/test_playback.py b/tests/mpd/protocol/test_playback.py index 1cd62bba..ea9c59ce 100644 --- a/tests/mpd/protocol/test_playback.py +++ b/tests/mpd/protocol/test_playback.py @@ -80,37 +80,37 @@ class PlaybackOptionsHandlerTest(protocol.BaseTestCase): def test_setvol_below_min(self): self.send_request('setvol "-10"') - self.assertEqual(0, self.core.playback.volume.get()) + self.assertEqual(0, self.core.mixer.get_volume().get()) self.assertInResponse('OK') def test_setvol_min(self): self.send_request('setvol "0"') - self.assertEqual(0, self.core.playback.volume.get()) + self.assertEqual(0, self.core.mixer.get_volume().get()) self.assertInResponse('OK') def test_setvol_middle(self): self.send_request('setvol "50"') - self.assertEqual(50, self.core.playback.volume.get()) + self.assertEqual(50, self.core.mixer.get_volume().get()) self.assertInResponse('OK') def test_setvol_max(self): self.send_request('setvol "100"') - self.assertEqual(100, self.core.playback.volume.get()) + self.assertEqual(100, self.core.mixer.get_volume().get()) self.assertInResponse('OK') def test_setvol_above_max(self): self.send_request('setvol "110"') - self.assertEqual(100, self.core.playback.volume.get()) + self.assertEqual(100, self.core.mixer.get_volume().get()) self.assertInResponse('OK') def test_setvol_plus_is_ignored(self): self.send_request('setvol "+10"') - self.assertEqual(10, self.core.playback.volume.get()) + self.assertEqual(10, self.core.mixer.get_volume().get()) self.assertInResponse('OK') def test_setvol_without_quotes(self): self.send_request('setvol 50') - self.assertEqual(50, self.core.playback.volume.get()) + self.assertEqual(50, self.core.mixer.get_volume().get()) self.assertInResponse('OK') def test_single_off(self): diff --git a/tests/mpd/test_status.py b/tests/mpd/test_status.py index 1015615c..75c10c94 100644 --- a/tests/mpd/test_status.py +++ b/tests/mpd/test_status.py @@ -52,7 +52,7 @@ class StatusHandlerTest(unittest.TestCase): self.assertEqual(int(result['volume']), -1) def test_status_method_contains_volume(self): - self.core.playback.volume = 17 + self.core.mixer.set_volume(17) result = dict(status.status(self.context)) self.assertIn('volume', result) self.assertEqual(int(result['volume']), 17) diff --git a/tests/utils/test_jsonrpc.py b/tests/utils/test_jsonrpc.py index d236469e..6e309c7c 100644 --- a/tests/utils/test_jsonrpc.py +++ b/tests/utils/test_jsonrpc.py @@ -49,7 +49,7 @@ class JsonRpcTestBase(unittest.TestCase): 'hello': lambda: 'Hello, world!', 'calc': Calculator(), 'core': self.core, - 'core.playback': self.core.playback, + 'core.mixer': self.core.mixer, 'core.tracklist': self.core.tracklist, 'get_uri_schemes': self.core.get_uri_schemes, }, @@ -188,7 +188,7 @@ class JsonRpcSingleCommandTest(JsonRpcTestBase): def test_call_method_on_actor_member(self): request = { 'jsonrpc': '2.0', - 'method': 'core.playback.get_volume', + 'method': 'core.mixer.get_volume', 'id': 1, } response = self.jrw.handle_data(request) @@ -215,26 +215,26 @@ class JsonRpcSingleCommandTest(JsonRpcTestBase): def test_call_method_with_positional_params(self): request = { 'jsonrpc': '2.0', - 'method': 'core.playback.set_volume', + 'method': 'core.mixer.set_volume', 'params': [37], 'id': 1, } response = self.jrw.handle_data(request) self.assertEqual(response['result'], None) - self.assertEqual(self.core.playback.get_volume().get(), 37) + self.assertEqual(self.core.mixer.get_volume().get(), 37) def test_call_methods_with_named_params(self): request = { 'jsonrpc': '2.0', - 'method': 'core.playback.set_volume', + 'method': 'core.mixer.set_volume', 'params': {'volume': 37}, 'id': 1, } response = self.jrw.handle_data(request) self.assertEqual(response['result'], None) - self.assertEqual(self.core.playback.get_volume().get(), 37) + self.assertEqual(self.core.mixer.get_volume().get(), 37) class JsonRpcSingleNotificationTest(JsonRpcTestBase): @@ -248,17 +248,17 @@ class JsonRpcSingleNotificationTest(JsonRpcTestBase): self.assertIsNone(response) def test_notification_makes_an_observable_change(self): - self.assertEqual(self.core.playback.get_volume().get(), None) + self.assertEqual(self.core.mixer.get_volume().get(), None) request = { 'jsonrpc': '2.0', - 'method': 'core.playback.set_volume', + 'method': 'core.mixer.set_volume', 'params': [37], } response = self.jrw.handle_data(request) self.assertIsNone(response) - self.assertEqual(self.core.playback.get_volume().get(), 37) + self.assertEqual(self.core.mixer.get_volume().get(), 37) def test_notification_unknown_method_returns_nothing(self): request = { @@ -526,7 +526,7 @@ class JsonRpcBatchErrorTest(JsonRpcTestBase): def test_batch_of_both_successfull_and_failing_requests(self): request = [ # Call with positional params - {'jsonrpc': '2.0', 'method': 'core.playback.set_volume', + {'jsonrpc': '2.0', 'method': 'core.mixer.set_volume', 'params': [47], 'id': '1'}, # Notification {'jsonrpc': '2.0', 'method': 'core.tracklist.set_consume',