From fce27e6fa950d1846ccc9ba99640d61c98299f62 Mon Sep 17 00:00:00 2001
From: Aidan Epstein <aidan@jmad.org>
Date: Sun, 9 Aug 2020 19:12:58 -0700
Subject: [PATCH] mpdstats: Don't record a skip when stopping MPD.

MPD keeps the current track in the queue when stopping, so it's not
really like a skip, and I use it so that I can stop the music, and later
start at the beginning of a track.

I do this by keeping track of the current song id, and then comparing
them when we receive a stop signal.
---
 beetsplug/mpdstats.py | 14 +++++++++-----
 docs/changelog.rst    |  4 ++++
 test/test_mpdstats.py |  3 ++-
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/beetsplug/mpdstats.py b/beetsplug/mpdstats.py
index f232d87e9..39b045f9b 100644
--- a/beetsplug/mpdstats.py
+++ b/beetsplug/mpdstats.py
@@ -108,8 +108,9 @@ class MPDClientWrapper(object):
         return self.get(command, retries=retries - 1)
 
     def currentsong(self):
-        """Return the path to the currently playing song.  Prefixes paths with the
-        music_directory, to get the absolute path.
+        """Return the path to the currently playing song, along with its
+        songid.  Prefixes paths with the music_directory, to get the absolute
+        path.
         """
         result = None
         entry = self.get('currentsong')
@@ -118,7 +119,7 @@ class MPDClientWrapper(object):
                 result = os.path.join(self.music_directory, entry['file'])
             else:
                 result = entry['file']
-        return result
+        return result, entry.get('id')
 
     def status(self):
         """Return the current status of the MPD.
@@ -240,7 +241,9 @@ class MPDStats(object):
     def on_stop(self, status):
         self._log.info(u'stop')
 
-        if self.now_playing:
+        # if the current song stays the same it means that we stopped on the
+        # current track and should not record a skip.
+        if self.now_playing and self.now_playing['id'] != status.get('songid'):
             self.handle_song_change(self.now_playing)
 
         self.now_playing = None
@@ -251,7 +254,7 @@ class MPDStats(object):
 
     def on_play(self, status):
 
-        path = self.mpd.currentsong()
+        path, songid = self.mpd.currentsong()
 
         if not path:
             return
@@ -286,6 +289,7 @@ class MPDStats(object):
             'started':    time.time(),
             'remaining':  remaining,
             'path':       path,
+            'id':         songid,
             'beets_item': self.get_item(path),
         }
 
diff --git a/docs/changelog.rst b/docs/changelog.rst
index 0dfa2c5f8..47159ddc2 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -245,6 +245,10 @@ Fixes:
 * Fix a bug that caused metadata starting with something resembling a drive
   letter to be incorrectly split into an extra directory after the colon.
   :bug:`3685`
+* :doc:`/plugins/mpdstats`: Don't record a skip when stopping MPD, as MPD keeps
+  the current track in the queue.
+  Thanks to :user:`aereaux`.
+  :bug:`3722`
 
 For plugin developers:
 
diff --git a/test/test_mpdstats.py b/test/test_mpdstats.py
index 0117e22aa..20226927f 100644
--- a/test/test_mpdstats.py
+++ b/test/test_mpdstats.py
@@ -62,10 +62,11 @@ class MPDStatsTest(unittest.TestCase, TestHelper):
                 {'state': u'stop'}]
     EVENTS = [["player"]] * (len(STATUSES) - 1) + [KeyboardInterrupt]
     item_path = util.normpath('/foo/bar.flac')
+    songid = 1
 
     @patch("beetsplug.mpdstats.MPDClientWrapper", return_value=Mock(**{
         "events.side_effect": EVENTS, "status.side_effect": STATUSES,
-        "currentsong.return_value": item_path}))
+        "currentsong.return_value": (item_path, songid)}))
     def test_run_mpdstats(self, mpd_mock):
         item = Item(title=u'title', path=self.item_path, id=1)
         item.add(self.lib)