From b7030b127ad8adda2853d03ee6d75108dc0c2879 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 14 Aug 2010 21:58:39 +0200 Subject: [PATCH] MPD: Fix 'play[id] -1' behaviour when current track is set --- docs/changes.rst | 3 +- mopidy/frontends/mpd/protocol/playback.py | 23 ++++++++----- tests/frontends/mpd/playback_test.py | 40 ++++++++++++++++++----- 3 files changed, 48 insertions(+), 18 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 3856dfff..a0a4dad2 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -40,7 +40,8 @@ greatly improved MPD client support. - Relocate from :mod:`mopidy.mpd` to :mod:`mopidy.frontends.mpd`. - Split gigantic protocol implementation into eleven modules. - Search improvements, including support for multi-word search. - - Fixed ``play "-1"`` and ``playid "-1"`` behaviour when playlist is empty. + - Fixed ``play "-1"`` and ``playid "-1"`` behaviour when playlist is empty + or when a current track is set. - Support ``plchanges "-1"`` to work better with MPDroid. - Support ``pause`` without arguments to work better with MPDroid. - Support ``plchanges``, ``play``, ``consume``, ``random``, ``repeat``, and diff --git a/mopidy/frontends/mpd/protocol/playback.py b/mopidy/frontends/mpd/protocol/playback.py index bfff275e..7abc4509 100644 --- a/mopidy/frontends/mpd/protocol/playback.py +++ b/mopidy/frontends/mpd/protocol/playback.py @@ -139,9 +139,7 @@ def playid(frontend, cpid): cpid = int(cpid) try: if cpid == -1: - if not frontend.backend.current_playlist.cp_tracks: - return # Fail silently - cp_track = frontend.backend.current_playlist.cp_tracks[0] + cp_track = _get_cp_track_for_play_minus_one(frontend) else: cp_track = frontend.backend.current_playlist.get(cpid=cpid) return frontend.backend.playback.play(cp_track) @@ -158,10 +156,11 @@ def playpos(frontend, songpos): Begins playing the playlist at song number ``SONGPOS``. - *MPoD:* + *Many clients:* - - issues ``play "-1"`` after playlist replacement to start playback at - the first track. + - issue ``play "-1"`` after playlist replacement to start the current + track. If the current track is not set, start playback at the first + track. *BitMPC:* @@ -170,15 +169,21 @@ def playpos(frontend, songpos): songpos = int(songpos) try: if songpos == -1: - if not frontend.backend.current_playlist.cp_tracks: - return # Fail silently - cp_track = frontend.backend.current_playlist.cp_tracks[0] + cp_track = _get_cp_track_for_play_minus_one(frontend) else: cp_track = frontend.backend.current_playlist.cp_tracks[songpos] return frontend.backend.playback.play(cp_track) except IndexError: raise MpdArgError(u'Bad song index', command=u'play') +def _get_cp_track_for_play_minus_one(frontend): + if not frontend.backend.current_playlist.cp_tracks: + return # Fail silently + cp_track = frontend.backend.playback.current_cp_track + if cp_track is None: + cp_track = frontend.backend.current_playlist.cp_tracks[0] + return cp_track + @handle_pattern(r'^previous$') def previous(frontend): """ diff --git a/tests/frontends/mpd/playback_test.py b/tests/frontends/mpd/playback_test.py index a1331bb3..ce3130bf 100644 --- a/tests/frontends/mpd/playback_test.py +++ b/tests/frontends/mpd/playback_test.py @@ -225,13 +225,25 @@ class PlaybackControlHandlerTest(unittest.TestCase): self.assertEqual(result[0], u'ACK [2@0] {play} Bad song index') self.assertEqual(self.b.playback.STOPPED, self.b.playback.state) - def test_play_minus_one_plays_first_in_playlist(self): - track = Track() - self.b.current_playlist.load([track]) + def test_play_minus_one_plays_first_in_playlist_if_no_current_track(self): + self.assertEqual(self.b.playback.current_track, None) + self.b.current_playlist.load([Track(uri='a'), Track(uri='b')]) result = self.h.handle_request(u'play "-1"') self.assert_(u'OK' in result) self.assertEqual(self.b.playback.PLAYING, self.b.playback.state) - self.assertEqual(self.b.playback.current_track, track) + self.assertEqual(self.b.playback.current_track.uri, 'a') + + def test_play_minus_one_plays_current_track_if_current_track_is_set(self): + self.b.current_playlist.load([Track(uri='a'), Track(uri='b')]) + self.assertEqual(self.b.playback.current_track, None) + self.b.playback.play() + self.b.playback.next() + self.b.playback.stop() + self.assertNotEqual(self.b.playback.current_track, None) + result = self.h.handle_request(u'play "-1"') + self.assert_(u'OK' in result) + self.assertEqual(self.b.playback.PLAYING, self.b.playback.state) + self.assertEqual(self.b.playback.current_track.uri, 'b') def test_play_minus_one_on_empty_playlist_does_not_ack(self): self.b.current_playlist.clear() @@ -246,13 +258,25 @@ class PlaybackControlHandlerTest(unittest.TestCase): self.assert_(u'OK' in result) self.assertEqual(self.b.playback.PLAYING, self.b.playback.state) - def test_playid_minus_one_plays_first_in_playlist(self): - track = Track() - self.b.current_playlist.load([track]) + def test_playid_minus_one_plays_first_in_playlist_if_no_current_track(self): + self.assertEqual(self.b.playback.current_track, None) + self.b.current_playlist.load([Track(uri='a'), Track(uri='b')]) result = self.h.handle_request(u'playid "-1"') self.assert_(u'OK' in result) self.assertEqual(self.b.playback.PLAYING, self.b.playback.state) - self.assertEqual(self.b.playback.current_track, track) + self.assertEqual(self.b.playback.current_track.uri, 'a') + + def test_play_minus_one_plays_current_track_if_current_track_is_set(self): + self.b.current_playlist.load([Track(uri='a'), Track(uri='b')]) + self.assertEqual(self.b.playback.current_track, None) + self.b.playback.play() + self.b.playback.next() + self.b.playback.stop() + self.assertNotEqual(self.b.playback.current_track, None) + result = self.h.handle_request(u'playid "-1"') + self.assert_(u'OK' in result) + self.assertEqual(self.b.playback.PLAYING, self.b.playback.state) + self.assertEqual(self.b.playback.current_track.uri, 'b') def test_playid_minus_one_on_empty_playlist_does_not_ack(self): self.b.current_playlist.clear()