Merge pull request #975 from jodal/feature/core-mixer

Add core.mixer replacing playback mixer methods.

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
 =============
 

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.

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

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.mixer = MixerController(mixer=mixer)
-        self.playback = PlaybackController(
+        self.playback = PlaybackController(backends=self.backends, core=self)
-            mixer=mixer, backends=self.backends, core=self)
+        self.playlists = PlaylistsController(backends=self.backends, core=self)
-
-        self.playlists = PlaylistsController(
-            backends=self.backends, core=self)
-
         self.tracklist = TracklistController(core=self)
 
         self.audio = audio

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

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):
+    def __init__(self, backends, core):
-        self.mixer = mixer
         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:
+        ... deprecated:: 0.20
-            return self.mixer.get_volume().get()
+            Use :meth:`core.mixer.get_volume()
-        else:
+            <mopidy.core.MixerController.get_volume>` instead.
-            # For testing
+        """
-            return self._volume
+        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:
+        ... deprecated:: 0.20
-            self.mixer.set_volume(volume)
+            Use :meth:`core.mixer.set_volume()
-        else:
+            <mopidy.core.MixerController.set_volume>` instead.
-            # For testing
+        """
-            self._volume = volume
+        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()
+        <mopidy.core.MixerController.get_volume>` and
+        :meth:`core.mixer.set_volume()
+        <mopidy.core.MixerController.set_volume>` instead.
     """
 
     def get_mute(self):
-        """Get mute state.
-
-        :class:`True` if muted, :class:`False` otherwise.
         """
-        if self.mixer:
+        ... deprecated:: 0.20
-            return self.mixer.get_mute().get()
+            Use :meth:`core.mixer.get_mute()
-        else:
+            <mopidy.core.MixerController.get_mute>` instead.
-            # For testing
-            return self._mute
-
-    def set_mute(self, value):
-        """Set mute state.
-
-        :class:`True` to mute, :class:`False` to unmute.
         """
-        value = bool(value)
+        warnings.warn('playback.get_mute() is deprecated', DeprecationWarning)
-        if self.mixer:
+        return self.core.mixer.get_mute()
-            self.mixer.set_mute(value)
+
-        else:
+    def set_mute(self, mute):
-            # For testing
+        """
-            self._mute = value
+        ... deprecated:: 0.20
+            Use :meth:`core.mixer.set_mute()
+            <mopidy.core.MixerController.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()
+        <mopidy.core.MixerController.get_mute>` and
+        :meth:`core.mixer.set_mute()
+        <mopidy.core.MixerController.set_mute>` instead.
     """
 
     # Methods

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,

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'),

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)

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:

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)

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)

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')
 

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):

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)

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',