From f1f223bba85d43ee485fb3813f4cbe314739c744 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 20 Oct 2013 21:43:19 +0200 Subject: [PATCH 1/3] local: Fix handling of single in eot_track (fixes #496) - Adds test cases for code paths that caused bug - Short circuits EOT next track handling when in single mode. --- mopidy/core/tracklist.py | 10 +++++---- tests/backends/local/playback_test.py | 30 +++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 5d85e190..8e2789c4 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -161,6 +161,11 @@ class TracklistController(object): if not self.tl_tracks: return None + if self.single and self.repeat: + return tl_track + elif self.single: + return None + if self.random and not self._shuffled: if self.repeat or self._first_shuffle: logger.debug('Shuffling tracks') @@ -176,10 +181,7 @@ class TracklistController(object): position = self.index(tl_track) - if self.repeat and self.single: - return self.tl_tracks[position] - - if self.repeat and not self.single: + if self.repeat: return self.tl_tracks[(position + 1) % len(self.tl_tracks)] try: diff --git a/tests/backends/local/playback_test.py b/tests/backends/local/playback_test.py index cd67a0e6..5543b3ab 100644 --- a/tests/backends/local/playback_test.py +++ b/tests/backends/local/playback_test.py @@ -339,6 +339,7 @@ class LocalPlaybackProviderTest(unittest.TestCase): self.tracklist.single = True self.tracklist.repeat = True self.playback.play() + self.assertEqual(self.playback.current_track, self.tracks[0]) self.playback.next() self.assertEqual(self.playback.current_track, self.tracks[1]) @@ -899,9 +900,38 @@ class LocalPlaybackProviderTest(unittest.TestCase): self.tracklist.single = True self.tracklist.repeat = True self.playback.play() + self.assertEqual(self.playback.current_track, self.tracks[0]) self.playback.on_end_of_track() self.assertEqual(self.playback.current_track, self.tracks[0]) + @populate_tracklist + def test_end_of_song_with_single_random_and_repeat_starts_same(self): + self.tracklist.single = True + self.tracklist.repeat = True + self.tracklist.random = True + self.playback.play() + current_track = self.playback.current_track + self.playback.on_end_of_track() + self.assertEqual(self.playback.current_track, current_track) + + @populate_tracklist + def test_end_of_song_with_single_stops(self): + self.tracklist.single = True + self.playback.play() + self.assertEqual(self.playback.current_track, self.tracks[0]) + self.playback.on_end_of_track() + self.assertEqual(self.playback.current_track, None) + self.assertEqual(self.playback.state, PlaybackState.STOPPED) + + @populate_tracklist + def test_end_of_song_with_single_and_random_stops(self): + self.tracklist.single = True + self.tracklist.random = True + self.playback.play() + self.playback.on_end_of_track() + self.assertEqual(self.playback.current_track, None) + self.assertEqual(self.playback.state, PlaybackState.STOPPED) + @populate_tracklist def test_end_of_playlist_stops(self): self.playback.play(self.tracklist.tl_tracks[-1]) From ba55181bc1ddab4760fd0a2a27b08dc2d5f66bf0 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 20 Oct 2013 21:48:42 +0200 Subject: [PATCH 2/3] core: Reduce duplication between next and eot track handling. --- mopidy/core/tracklist.py | 36 +++++++----------------------------- 1 file changed, 7 insertions(+), 29 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 8e2789c4..d995029c 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -158,36 +158,15 @@ class TracklistController(object): :type tl_track: :class:`mopidy.models.TlTrack` or :class:`None` :rtype: :class:`mopidy.models.TlTrack` or :class:`None` """ - if not self.tl_tracks: - return None - if self.single and self.repeat: return tl_track elif self.single: return None - if self.random and not self._shuffled: - if self.repeat or self._first_shuffle: - logger.debug('Shuffling tracks') - self._shuffled = self.tl_tracks - random.shuffle(self._shuffled) - self._first_shuffle = False - - if self.random and self._shuffled: - return self._shuffled[0] - - if tl_track is None: - return self.tl_tracks[0] - - position = self.index(tl_track) - - if self.repeat: - return self.tl_tracks[(position + 1) % len(self.tl_tracks)] - - try: - return self.tl_tracks[position + 1] - except IndexError: - return None + # Current differnce between next and eot handling is that eot needs to + # handle single, with that out of the way the rest of the logic is + # shared. + return self.next_track(tl_track) def next_track(self, tl_track): """ @@ -220,13 +199,12 @@ class TracklistController(object): if tl_track is None: return self.tl_tracks[0] - position = self.index(tl_track) - + next_index = self.index(tl_track) + 1 if self.repeat: - return self.tl_tracks[(position + 1) % len(self.tl_tracks)] + next_index %= len(self.tl_tracks) try: - return self.tl_tracks[position + 1] + return self.tl_tracks[next_index] except IndexError: return None From bfddfab15a3fd57d59e9d9a6f26c15c021b3dbc1 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 20 Oct 2013 22:47:54 +0200 Subject: [PATCH 3/3] core: Fix typos pointed out in PR#542 --- mopidy/core/tracklist.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 57da7901..dbc81945 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -165,8 +165,8 @@ class TracklistController(object): elif self.single: return None - # Current differnce between next and eot handling is that eot needs to - # handle single, with that out of the way the rest of the logic is + # Current difference between next and EOT handling is that EOT needs to + # handle "single", with that out of the way the rest of the logic is # shared. return self.next_track(tl_track)