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

@@ -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):

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()
 

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