From 22264071e422b71671d7bc1f3903a9c53825a307 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 24 Sep 2015 22:35:59 +0200 Subject: [PATCH] core, mpd: Start tlid/songid counting at 1 The original MPD server starts at 1. upmpdcli has issues with Mopidy starting at 0 instead, as 0 is special in its context. As noone should care exactly what core's TLIDs are, I opted to start counting both core TLID and MPD songid from 1, instead of just increasing TLID with 1 in the MPD frontend to get a valid songid. This also keeps it easier to debug across the MPD/core boundary. --- docs/changelog.rst | 15 +++++++++++++-- mopidy/core/playback.py | 2 +- mopidy/core/tracklist.py | 4 ++-- tests/mpd/protocol/test_current_playlist.py | 18 +++++++++--------- tests/mpd/protocol/test_playback.py | 10 +++++----- tests/mpd/protocol/test_status.py | 2 +- tests/mpd/test_status.py | 2 +- 7 files changed, 32 insertions(+), 21 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 0d23a608..69e5279e 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -10,12 +10,23 @@ v1.2.0 (UNRELEASED) Feature release. -Local ------ +Core API +-------- + +- Start ``tlid`` counting at 1 instead of 0 to keep in sync with MPD's + ``songid``. + +Local backend +-------------- - Made :confval:`local/data_dir` really deprecated. This change breaks older versions of Mopidy-Local-SQLite and Mopidy-Local-Images. +MPD frontend +------------ + +- Start ``songid`` counting at 1 instead of 0 to match the original MPD server. + Zeroconf -------- diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 9a11066b..389e780f 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -297,7 +297,7 @@ class PlaybackController(object): raise ValueError('At most one of "tl_track" and "tlid" may be set') tl_track is None or validation.check_instance(tl_track, models.TlTrack) - tlid is None or validation.check_integer(tlid, min=0) + tlid is None or validation.check_integer(tlid, min=1) if tl_track: deprecation.warn('core.playback.play:tl_track_kwarg', pending=True) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 13efe322..dbe0c150 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -16,7 +16,7 @@ class TracklistController(object): def __init__(self, core): self.core = core - self._next_tlid = 0 + self._next_tlid = 1 self._tl_tracks = [] self._version = 0 @@ -218,7 +218,7 @@ class TracklistController(object): The *tlid* parameter """ tl_track is None or validation.check_instance(tl_track, TlTrack) - tlid is None or validation.check_integer(tlid, min=0) + tlid is None or validation.check_integer(tlid, min=1) if tl_track is None and tlid is None: tl_track = self.core.playback.get_current_tl_track() diff --git a/tests/mpd/protocol/test_current_playlist.py b/tests/mpd/protocol/test_current_playlist.py index 81bec5a4..8dd814e9 100644 --- a/tests/mpd/protocol/test_current_playlist.py +++ b/tests/mpd/protocol/test_current_playlist.py @@ -178,13 +178,13 @@ class MoveCommandsTest(BasePopulatedTracklistTestCase): self.assertInResponse('OK') def test_moveid(self): - self.send_request('moveid "4" "2"') + self.send_request('moveid "5" "2"') result = [t.name for t in self.core.tracklist.tracks.get()] self.assertEqual(result, ['a', 'b', 'e', 'c', 'd', 'f']) self.assertInResponse('OK') def test_moveid_with_tlid_not_found_in_tracklist_should_ack(self): - self.send_request('moveid "9" "0"') + self.send_request('moveid "10" "0"') self.assertEqualResponse( 'ACK [50@0] {moveid} No such song') @@ -210,7 +210,7 @@ class PlaylistFindCommandTest(protocol.BaseTestCase): self.send_request('playlistfind filename "dummy:///exists"') self.assertInResponse('file: dummy:///exists') - self.assertInResponse('Id: 0') + self.assertInResponse('Id: 1') self.assertInResponse('Pos: 0') self.assertInResponse('OK') @@ -224,11 +224,11 @@ class PlaylistIdCommandTest(BasePopulatedTracklistTestCase): self.assertInResponse('OK') def test_playlistid_with_songid(self): - self.send_request('playlistid "1"') + self.send_request('playlistid "2"') self.assertNotInResponse('Title: a') - self.assertNotInResponse('Id: 0') + self.assertNotInResponse('Id: 1') self.assertInResponse('Title: b') - self.assertInResponse('Id: 1') + self.assertInResponse('Id: 2') self.assertInResponse('OK') def test_playlistid_with_not_existing_songid_fails(self): @@ -445,18 +445,18 @@ class SwapCommandTest(BasePopulatedTracklistTestCase): self.assertInResponse('OK') def test_swapid(self): - self.send_request('swapid "1" "4"') + self.send_request('swapid "2" "5"') result = [t.name for t in self.core.tracklist.tracks.get()] self.assertEqual(result, ['a', 'e', 'c', 'd', 'b', 'f']) self.assertInResponse('OK') def test_swapid_with_first_id_unknown_should_ack(self): - self.send_request('swapid "0" "8"') + self.send_request('swapid "1" "8"') self.assertEqualResponse( 'ACK [50@0] {swapid} No such song') def test_swapid_with_second_id_unknown_should_ack(self): - self.send_request('swapid "8" "0"') + self.send_request('swapid "8" "1"') self.assertEqualResponse( 'ACK [50@0] {swapid} No such song') diff --git a/tests/mpd/protocol/test_playback.py b/tests/mpd/protocol/test_playback.py index b9adb646..6cfa30fc 100644 --- a/tests/mpd/protocol/test_playback.py +++ b/tests/mpd/protocol/test_playback.py @@ -292,12 +292,12 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_playid(self): - self.send_request('playid "0"') + self.send_request('playid "1"') self.assertEqual(PLAYING, self.core.playback.state.get()) self.assertInResponse('OK') def test_playid_without_quotes(self): - self.send_request('playid 0') + self.send_request('playid 1') self.assertEqual(PLAYING, self.core.playback.state.get()) self.assertInResponse('OK') @@ -398,7 +398,7 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): def test_seekid_in_current_track(self): self.core.playback.play() - self.send_request('seekid "0" "30"') + self.send_request('seekid "1" "30"') current_track = self.core.playback.current_track.get() self.assertEqual(current_track, self.tracks[0]) @@ -409,10 +409,10 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): def test_seekid_in_another_track(self): self.core.playback.play() - self.send_request('seekid "1" "30"') + self.send_request('seekid "2" "30"') current_tl_track = self.core.playback.current_tl_track.get() - self.assertEqual(current_tl_track.tlid, 1) + self.assertEqual(current_tl_track.tlid, 2) self.assertEqual(current_tl_track.track, self.tracks[1]) self.assertInResponse('OK') diff --git a/tests/mpd/protocol/test_status.py b/tests/mpd/protocol/test_status.py index fb448d8d..f65a7d3c 100644 --- a/tests/mpd/protocol/test_status.py +++ b/tests/mpd/protocol/test_status.py @@ -26,7 +26,7 @@ class StatusHandlerTest(protocol.BaseTestCase): self.assertNotInResponse('Track: 0') self.assertNotInResponse('Date: ') self.assertInResponse('Pos: 0') - self.assertInResponse('Id: 0') + self.assertInResponse('Id: 1') self.assertInResponse('OK') def test_currentsong_without_song(self): diff --git a/tests/mpd/test_status.py b/tests/mpd/test_status.py index 76fa9fcb..bce4d350 100644 --- a/tests/mpd/test_status.py +++ b/tests/mpd/test_status.py @@ -164,7 +164,7 @@ class StatusHandlerTest(unittest.TestCase): self.core.playback.play() result = dict(status.status(self.context)) self.assertIn('songid', result) - self.assertEqual(int(result['songid']), 0) + self.assertEqual(int(result['songid']), 1) def test_status_method_when_playing_contains_time_with_no_length(self): self.set_tracklist(Track(uri='dummy:/a', length=None))