From 3a57a5792bd8da519c513a27fea316d35c27475a Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 21 Nov 2015 22:43:40 +0100 Subject: [PATCH] core: Make sure we always emit state_changed between tracks Gapless broke this, so this change makes sure that next/prev/play and gapless track changes all correctly emit events. Note that this only ensures we get PLAYING -> PLAYING events. Not the old STOPPED -> PLAYING and then PLAYING -> STOPPED. --- mopidy/core/playback.py | 8 ++++---- tests/core/test_playback.py | 32 +++++++++++++++++++++++--------- tests/local/test_playback.py | 2 +- 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 300a6a89..45e1b4ba 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -217,6 +217,7 @@ class PlaybackController(object): if self._pending_tl_track: self._set_current_tl_track(self._pending_tl_track) self._pending_tl_track = None + self.set_state(PlaybackState.PLAYING) self._trigger_track_playback_started() def _on_about_to_finish_callback(self): @@ -233,6 +234,9 @@ class PlaybackController(object): }) def _on_about_to_finish(self): + if self._state == PlaybackState.STOPPED: + return + # TODO: check that we always have a current track original_tl_track = self.get_current_tl_track() next_tl_track = self.core.tracklist.eot_track(original_tl_track) @@ -328,10 +332,6 @@ class PlaybackController(object): current = self._pending_tl_track or self._current_tl_track pending = tl_track or current or self.core.tracklist.next_track(None) - if pending: - # TODO: remove? - self.set_state(PlaybackState.PLAYING) - while pending: # TODO: should we consume unplayable tracks in this loop? if self._change(pending, PlaybackState.PLAYING): diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index 0a86881c..4ae3b4ef 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -314,6 +314,8 @@ class TestCurrentAndPendingTlTrack(BaseTest): 'mopidy.core.playback.listener.CoreListener', spec=core.CoreListener) class EventEmissionTest(BaseTest): + maxDiff = None + def test_play_when_stopped_emits_events(self, listener_mock): tl_tracks = self.core.tracklist.get_tl_tracks() @@ -345,12 +347,12 @@ class EventEmissionTest(BaseTest): self.assertListEqual( [ - mock.call( - 'playback_state_changed', - old_state='paused', new_state='playing'), mock.call( 'track_playback_ended', tl_track=tl_tracks[0], time_position=mock.ANY), + mock.call( + 'playback_state_changed', + old_state='paused', new_state='playing'), mock.call( 'track_playback_started', tl_track=tl_tracks[1]), ], @@ -366,15 +368,14 @@ class EventEmissionTest(BaseTest): self.core.playback.play(tl_tracks[2]) self.replay_events() - # TODO: Do we want to emit playing->playing for this case? self.assertListEqual( [ - mock.call( - 'playback_state_changed', old_state='playing', - new_state='playing'), mock.call( 'track_playback_ended', tl_track=tl_tracks[0], time_position=mock.ANY), + mock.call( + 'playback_state_changed', old_state='playing', + new_state='playing'), mock.call( 'track_playback_started', tl_track=tl_tracks[2]), ], @@ -458,18 +459,20 @@ class EventEmissionTest(BaseTest): self.core.playback.next() self.replay_events() - # TODO: should we be emitting playing -> playing? self.assertListEqual( [ mock.call( 'track_playback_ended', tl_track=tl_tracks[0], time_position=mock.ANY), + mock.call( + 'playback_state_changed', + old_state='playing', new_state='playing'), mock.call( 'track_playback_started', tl_track=tl_tracks[1]), ], listener_mock.send.mock_calls) - def test_on_end_of_track_emits_events(self, listener_mock): + def test_gapless_track_change_emits_events(self, listener_mock): tl_tracks = self.core.tracklist.get_tl_tracks() self.core.playback.play(tl_tracks[0]) @@ -483,6 +486,9 @@ class EventEmissionTest(BaseTest): mock.call( 'track_playback_ended', tl_track=tl_tracks[0], time_position=mock.ANY), + mock.call( + 'playback_state_changed', + old_state='playing', new_state='playing'), mock.call( 'track_playback_started', tl_track=tl_tracks[1]), ], @@ -515,6 +521,9 @@ class EventEmissionTest(BaseTest): mock.call( 'track_playback_ended', tl_track=tl_tracks[0], time_position=mock.ANY), + mock.call( + 'playback_state_changed', + old_state='playing', new_state='playing'), mock.call( 'track_playback_started', tl_track=tl_tracks[1]), ], @@ -535,6 +544,9 @@ class EventEmissionTest(BaseTest): mock.call( 'track_playback_ended', tl_track=tl_tracks[1], time_position=mock.ANY), + mock.call( + 'playback_state_changed', + old_state='playing', new_state='playing'), mock.call( 'track_playback_started', tl_track=tl_tracks[0]), ], @@ -612,6 +624,8 @@ class SeekTest(BaseTest): tl_tracks = self.core.tracklist.get_tl_tracks() self.core.playback.play(tl_tracks[0]) + self.replay_events() + self.core.playback.pause() self.replay_events() diff --git a/tests/local/test_playback.py b/tests/local/test_playback.py index 92fbe5b9..f0dcf20b 100644 --- a/tests/local/test_playback.py +++ b/tests/local/test_playback.py @@ -998,7 +998,7 @@ class LocalPlaybackProviderTest(unittest.TestCase): self.playback.next().get() self.assert_next_tl_track_is_not(None) self.assert_state_is(PlaybackState.STOPPED) - self.playback.play() + self.playback.play().get() self.assert_state_is(PlaybackState.PLAYING) @populate_tracklist