Merge pull request #1397 from jodal/feature/mpd-volume-cmd

MPD: Add volume command

docs/changelog.rst

@@ -39,6 +39,10 @@ Bug fix release.
 - MPD: Don't return tracks with empty URIs. (Partly fixes: :issue:`1340`, PR:
   :issue:`1343`)
 
+- MPD: Add ``volume`` command that was reintroduced, though still as a
+  deprecated command, in MPD 0.18 and is in use by some clients like mpc.
+  (Fixes: :issue:`1393`, PR: :issue:`1397`)
+
 - Proxy: Handle case where :confval:`proxy/port` is either missing from config
   or set to an empty string. (PR: :issue:`1371`)
 

mopidy/mpd/protocol/playback.py

@@ -426,3 +426,27 @@ def stop(context):
         Stops playing.
     """
     context.core.playback.stop()
+
+
+@protocol.commands.add('volume', change=protocol.INT)
+def volume(context, change):
+    """
+    *musicpd.org, playback section:*
+
+        ``volume {CHANGE}``
+
+        Changes volume by amount ``CHANGE``.
+
+        Note: ``volume`` is deprecated, use ``setvol`` instead.
+    """
+    if change < -100 or change > 100:
+        raise exceptions.MpdArgError('Invalid volume value')
+
+    old_volume = context.core.mixer.get_volume().get()
+    if old_volume is None:
+        raise exceptions.MpdSystemError('problems setting volume')
+
+    new_volume = min(max(0, old_volume + change), 100)
+    success = context.core.mixer.set_volume(new_volume).get()
+    if not success:
+        raise exceptions.MpdSystemError('problems setting volume')

tests/mpd/protocol/test_playback.py

@@ -80,41 +80,6 @@ def test_repeat_on_without_quotes(self):
         self.assertTrue(self.core.tracklist.repeat.get())
         self.assertInResponse('OK')
 
-    def test_setvol_below_min(self):
-        self.send_request('setvol "-10"')
-        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.mixer.get_volume().get())
-        self.assertInResponse('OK')
-
-    def test_setvol_middle(self):
-        self.send_request('setvol "50"')
-        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.mixer.get_volume().get())
-        self.assertInResponse('OK')
-
-    def test_setvol_above_max(self):
-        self.send_request('setvol "110"')
-        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.mixer.get_volume().get())
-        self.assertInResponse('OK')
-
-    def test_setvol_without_quotes(self):
-        self.send_request('setvol 50')
-        self.assertEqual(50, self.core.mixer.get_volume().get())
-        self.assertInResponse('OK')
-
     def test_single_off(self):
         self.send_request('single "0"')
         self.assertFalse(self.core.tracklist.single.get())
@@ -451,9 +416,83 @@ def test_stop(self):
         self.assertInResponse('OK')
 
 
-class PlaybackOptionsHandlerNoneMixerTest(protocol.BaseTestCase):
+class VolumeTest(protocol.BaseTestCase):
+
+    def test_setvol_below_min(self):
+        self.send_request('setvol "-10"')
+        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.mixer.get_volume().get())
+        self.assertInResponse('OK')
+
+    def test_setvol_middle(self):
+        self.send_request('setvol "50"')
+        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.mixer.get_volume().get())
+        self.assertInResponse('OK')
+
+    def test_setvol_above_max(self):
+        self.send_request('setvol "110"')
+        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.mixer.get_volume().get())
+        self.assertInResponse('OK')
+
+    def test_setvol_without_quotes(self):
+        self.send_request('setvol 50')
+        self.assertEqual(50, self.core.mixer.get_volume().get())
+        self.assertInResponse('OK')
+
+    def test_volume_plus(self):
+        self.core.mixer.set_volume(50)
+
+        self.send_request('volume +20')
+
+        self.assertEqual(70, self.core.mixer.get_volume().get())
+        self.assertInResponse('OK')
+
+    def test_volume_minus(self):
+        self.core.mixer.set_volume(50)
+
+        self.send_request('volume -20')
+
+        self.assertEqual(30, self.core.mixer.get_volume().get())
+        self.assertInResponse('OK')
+
+    def test_volume_less_than_minus_100(self):
+        self.core.mixer.set_volume(50)
+
+        self.send_request('volume -110')
+
+        self.assertEqual(50, self.core.mixer.get_volume().get())
+        self.assertInResponse('ACK [2@0] {volume} Invalid volume value')
+
+    def test_volume_more_than_plus_100(self):
+        self.core.mixer.set_volume(50)
+
+        self.send_request('volume +110')
+
+        self.assertEqual(50, self.core.mixer.get_volume().get())
+        self.assertInResponse('ACK [2@0] {volume} Invalid volume value')
+
+
+class VolumeWithNoMixerTest(protocol.BaseTestCase):
     enable_mixer = False
 
-    def test_setvol_max_error(self):
+    def test_setvol_without_mixer_fails(self):
         self.send_request('setvol "100"')
         self.assertInResponse('ACK [52@0] {setvol} problems setting volume')
+
+    def test_volume_without_mixer_failes(self):
+        self.send_request('volume +100')
+        self.assertInResponse('ACK [52@0] {volume} problems setting volume')