Play next track at play error

docs/changes.rst

@@ -35,6 +35,7 @@ greatly improved MPD client support.
 - Added new :mod:`mopidy.mixers.GStreamerSoftwareMixer` which now is the
   default mixer on all platforms.
 - New setting ``MIXER_MAX_VOLUME`` for capping the maximum output volume.
+- If failing to play a track, playback will skip to the next track.
 - MPD frontend:
 
   - Relocate from :mod:`mopidy.mpd` to :mod:`mopidy.frontends.mpd`.

mopidy/backends/base/playback.py

@@ -272,27 +272,24 @@ class BasePlaybackController(object):
 
     def next(self):
         """Play the next track."""
-        original_cp_track = self.current_cp_track
-
         if self.state == self.STOPPED:
             return
-        elif self.next_cp_track is not None and self._next(self.next_track):
+
-            self.current_cp_track = self.next_cp_track
+        original_cp_track = self.current_cp_track
-            self.state = self.PLAYING
+        if self.next_cp_track:
-        elif self.next_cp_track is None:
+            self.play(self.next_cp_track)
+        else:
             self.stop()
             self.current_cp_track = None
 
-        # FIXME handle in play aswell?
+        # FIXME This should only be applied when reaching end of track, and not
+        # when pressing "next"
         if self.consume:
             self.backend.current_playlist.remove(cpid=original_cp_track[0])
 
         if self.random and self.current_cp_track in self._shuffled:
             self._shuffled.remove(self.current_cp_track)
 
-    def _next(self, track):
-        return self._play(track)
-
     def pause(self):
         """Pause playback."""
         if self.state == self.PLAYING and self._pause():
@@ -301,13 +298,16 @@ class BasePlaybackController(object):
     def _pause(self):
         raise NotImplementedError
 
-    def play(self, cp_track=None):
+    def play(self, cp_track=None, on_error_step=1):
         """
         Play the given track or the currently active track.
 
         :param cp_track: track to play
         :type cp_track: two-tuple (CPID integer, :class:`mopidy.models.Track`)
             or :class:`None`
+        :param on_error_step: direction to step at play error, 1 for next
+            track (default), -1 for previous track
+        :type on_error_step: int, -1 or 1
         """
 
         if cp_track is not None:
@@ -317,13 +317,14 @@ class BasePlaybackController(object):
 
         if self.state == self.PAUSED and cp_track is None:
             self.resume()
-        elif cp_track is not None and self._play(cp_track[1]):
+        elif cp_track is not None:
             self.current_cp_track = cp_track
             self.state = self.PLAYING
-
+            if not self._play(cp_track[1]):
-        # TODO Do something sensible when _play() returns False, like calling
+                if at_error_step == 1:
-        # next(). Adding this todo instead of just implementing it as I want a
+                    self.next()
-        # test case first.
+                elif at_error_step == -1:
+                    self.previous()
 
         if self.random and self.current_cp_track in self._shuffled:
             self._shuffled.remove(self.current_cp_track)
@@ -333,14 +334,11 @@ class BasePlaybackController(object):
 
     def previous(self):
         """Play the previous track."""
-        if (self.previous_cp_track is not None
+        if self.previous_cp_track is None:
-                and self.state != self.STOPPED
+            return
-                and self._previous(self.previous_track)):
+        if self.state == self.STOPPED:
-            self.current_cp_track = self.previous_cp_track
+            return
-            self.state = self.PLAYING
+        self.play(self.previous_cp_track, on_error_step=-1)
-
-    def _previous(self, track):
-        return self._play(track)
 
     def resume(self):
         """If paused, resume playing the current track."""

mopidy/backends/libspotify/playback.py

@@ -26,7 +26,7 @@ class LibspotifyPlaybackController(BasePlaybackController):
     def _play(self, track):
         self._set_output_state('READY')
         if self.state == self.PLAYING:
-            self.stop()
+            self.backend.spotify.session.play(0)
         if track.uri is None:
             return False
         try:

tests/backends/base.py

@@ -345,6 +345,14 @@ class BasePlaybackControllerTest(object):
         self.playback.play(self.current_playlist.cp_tracks[-1])
         self.assertEqual(self.playback.current_track, self.tracks[-1])
 
+    @populate_playlist
+    def test_play_skips_to_next_track_on_failure(self):
+        # If _play() returns False, it is a failure.
+        self.playback._play = lambda track: track != self.tracks[0]
+        self.playback.play()
+        self.assertNotEqual(self.playback.current_track, self.tracks[0])
+        self.assertEqual(self.playback.current_track, self.tracks[1])
+
     @populate_playlist
     def test_current_track_after_completed_playlist(self):
         self.playback.play(self.current_playlist.cp_tracks[-1])
@@ -411,6 +419,16 @@ class BasePlaybackControllerTest(object):
         self.playback.next()
         self.assertEqual(self.playback.state, self.playback.STOPPED)
 
+    @populate_playlist
+    def test_next_skips_to_next_track_on_failure(self):
+        # If _play() returns False, it is a failure.
+        self.playback._play = lambda track: track != self.tracks[1]
+        self.playback.play()
+        self.assertEqual(self.playback.current_track, self.tracks[0])
+        self.playback.next()
+        self.assertNotEqual(self.playback.current_track, self.tracks[1])
+        self.assertEqual(self.playback.current_track, self.tracks[2])
+
     @populate_playlist
     def test_previous(self):
         self.playback.play()
@@ -451,6 +469,16 @@ class BasePlaybackControllerTest(object):
         self.assertEqual(self.playback.state, self.playback.STOPPED)
         self.assertEqual(self.playback.current_track, None)
 
+    @populate_playlist
+    def test_previous_skips_to_previous_track_on_failure(self):
+        # If _play() returns False, it is a failure.
+        self.playback._play = lambda track: track != self.tracks[1]
+        self.playback.play(self.current_playlist.cp_tracks[2])
+        self.assertEqual(self.playback.current_track, self.tracks[2])
+        self.playback.previous()
+        self.assertNotEqual(self.playback.current_track, self.tracks[1])
+        self.assertEqual(self.playback.current_track, self.tracks[0])
+
     @populate_playlist
     def test_next_track_before_play(self):
         self.assertEqual(self.playback.next_track, self.tracks[0])
@@ -906,13 +934,9 @@ class BasePlaybackControllerTest(object):
             played.append(self.playback.current_track)
             self.playback.next()
 
-    def test_playing_track_with_invalid_uri(self):
+    @populate_playlist
-        self.backend.current_playlist.load([Track(uri='foobar')])
-        self.playback.play()
-        self.assertEqual(self.playback.state, self.playback.STOPPED)
-
     def test_playing_track_that_isnt_in_playlist(self):
-        test = lambda: self.playback.play(self.tracks[0])
+        test = lambda: self.playback.play((17, Track()))
         self.assertRaises(AssertionError, test)