diff --git a/docs/changes.rst b/docs/changes.rst index a0a4dad2..76f01610 100644 --- a/docs/changes.rst +++ b/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`. diff --git a/mopidy/backends/base/playback.py b/mopidy/backends/base/playback.py index f2b810e2..da951154 100644 --- a/mopidy/backends/base/playback.py +++ b/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 - self.state = self.PLAYING - elif self.next_cp_track is None: + + original_cp_track = self.current_cp_track + if self.next_cp_track: + 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 - - # TODO Do something sensible when _play() returns False, like calling - # next(). Adding this todo instead of just implementing it as I want a - # test case first. + if not self._play(cp_track[1]): + if at_error_step == 1: + self.next() + 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 - and self.state != self.STOPPED - and self._previous(self.previous_track)): - self.current_cp_track = self.previous_cp_track - self.state = self.PLAYING - - def _previous(self, track): - return self._play(track) + if self.previous_cp_track is None: + return + if self.state == self.STOPPED: + return + self.play(self.previous_cp_track, on_error_step=-1) def resume(self): """If paused, resume playing the current track.""" diff --git a/mopidy/backends/libspotify/playback.py b/mopidy/backends/libspotify/playback.py index 60a5d355..1195e9bc 100644 --- a/mopidy/backends/libspotify/playback.py +++ b/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: diff --git a/tests/backends/base.py b/tests/backends/base.py index 3aaa725f..2976f5cc 100644 --- a/tests/backends/base.py +++ b/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): - self.backend.current_playlist.load([Track(uri='foobar')]) - self.playback.play() - self.assertEqual(self.playback.state, self.playback.STOPPED) - + @populate_playlist 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)