From f8a2c22e8deb574144a27c6d1c3989171c10808b Mon Sep 17 00:00:00 2001
From: Carl Suster <carl@contraflo.ws>
Date: Thu, 4 Apr 2019 17:56:13 +1100
Subject: [PATCH 01/13] bpd: fix typo in comment

---
 beetsplug/bpd/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py
index 598e2971f3..4432ab5268 100644
--- a/beetsplug/bpd/__init__.py
+++ b/beetsplug/bpd/__init__.py
@@ -767,7 +767,7 @@ def run(self, conn):
         # by the client (so we can raise ERROR_ARG instead of ERROR_SYSTEM).
         # Maximum accepted arguments: argspec includes "self" and "conn".
         max_args = len(argspec.args) - 2
-        # Minimum accepted arguments: some arguments might be optional/
+        # Minimum accepted arguments: some arguments might be optional.
         min_args = max_args
         if argspec.defaults:
             min_args -= len(argspec.defaults)

From 5b0a02eb31dd804deeaf2c357522ab15b390a673 Mon Sep 17 00:00:00 2001
From: Carl Suster <carl@contraflo.ws>
Date: Fri, 5 Apr 2019 18:56:34 +1100
Subject: [PATCH 02/13] bpd: don't send volume if zero in status

---
 beetsplug/bpd/__init__.py | 4 +++-
 test/test_player.py       | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py
index 4432ab5268..dc7f64db7f 100644
--- a/beetsplug/bpd/__init__.py
+++ b/beetsplug/bpd/__init__.py
@@ -309,7 +309,6 @@ def cmd_status(self, conn):
         playlist, playlistlength, and xfade.
         """
         yield (
-            u'volume: ' + six.text_type(self.volume),
             u'repeat: ' + six.text_type(int(self.repeat)),
             u'random: ' + six.text_type(int(self.random)),
             u'consume: ' + six.text_type(int(self.consume)),
@@ -319,6 +318,9 @@ def cmd_status(self, conn):
             u'mixrampdb: ' + six.text_type(self.mixrampdb),
         )
 
+        if self.volume > 0:
+            yield u'volume: ' + six.text_type(self.volume)
+
         if not math.isnan(self.mixrampdelay):
             yield u'mixrampdelay: ' + six.text_type(self.mixrampdelay)
         if self.crossfade > 0:
diff --git a/test/test_player.py b/test/test_player.py
index 98fd13f636..aa3c3d6a8a 100644
--- a/test/test_player.py
+++ b/test/test_player.py
@@ -384,7 +384,7 @@ def test_cmd_status(self):
         fields_not_playing = {
             'repeat', 'random', 'single', 'consume', 'playlist',
             'playlistlength', 'mixrampdb', 'state',
-            'volume'  # not (always?) returned by MPD
+            'volume'
         }
         self.assertEqual(fields_not_playing, set(responses[0].data.keys()))
         fields_playing = fields_not_playing | {

From ee0c31ba6a3b00a671a8c2af1b5934af89d2f3df Mon Sep 17 00:00:00 2001
From: Carl Suster <carl@contraflo.ws>
Date: Thu, 4 Apr 2019 17:50:21 +1100
Subject: [PATCH 03/13] bpd: track and log client session details

Keep track of a list of currently-connected clients.

Use `socket.getpeername()` to get an identifier for each connection and
include this in each log message. This function is documented as not
being available on all systems, but it's unclear which systems this
involves.

Also log a message on client connect and disconnect events. If the
disconnection reason is because the client sent a blank line, match MPD
by returning a protocol error then hanging up. Escape curly braces.
---
 beetsplug/bpd/__init__.py | 39 +++++++++++++++++++++++++++++++++------
 test/test_player.py       |  5 +++++
 2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py
index dc7f64db7f..153fd675ed 100644
--- a/beetsplug/bpd/__init__.py
+++ b/beetsplug/bpd/__init__.py
@@ -187,9 +187,22 @@ def __init__(self, host, port, password, log):
         self.paused = False
         self.error = None
 
+        # Current connections
+        self.connections = set()
+
         # Object for random numbers generation
         self.random_obj = random.Random()
 
+    def connect(self, conn):
+        """A new client has connected.
+        """
+        self.connections.add(conn)
+
+    def disconnect(self, conn):
+        """Client has disconnected; clean up residual state.
+        """
+        self.connections.remove(conn)
+
     def run(self):
         """Block and start listening for connections from clients. An
         interrupt (^C) closes the server.
@@ -643,6 +656,7 @@ def __init__(self, server, sock):
         self.server = server
         self.sock = sock
         self.authenticated = False
+        self.address = u'{}:{}'.format(*sock.sock.getpeername())
 
     def send(self, lines):
         """Send lines, which which is either a single string or an
@@ -653,9 +667,9 @@ def send(self, lines):
         if isinstance(lines, six.string_types):
             lines = [lines]
         out = NEWLINE.join(lines) + NEWLINE
-        # Don't log trailing newline:
-        message = out[:-1].replace(u'\n', u'\n' + u' ' * 13)
-        self.server._log.debug('server: {}', message)
+        session = u'>[{}]: '.format(self.address)
+        for l in out.split(NEWLINE)[:-1]:
+            self.server._log.debug('{}', session + l)
         if isinstance(out, six.text_type):
             out = out.encode('utf-8')
         return self.sock.sendall(out)
@@ -672,24 +686,36 @@ def do_command(self, command):
             # Send success code.
             yield self.send(RESP_OK)
 
+    def disconnect(self):
+        """The connection has closed for any reason.
+        """
+        self.server.disconnect(self)
+        self.server._log.debug(u'*[{}]: disconnected', self.address)
+
     def run(self):
         """Send a greeting to the client and begin processing commands
         as they arrive.
         """
-        self.server._log.debug('New client connected')
+        self.server._log.debug(u'*[{}]: connected', self.address)
+        self.server.connect(self)
         yield self.send(HELLO)
 
+        session = u'<[{}]: '.format(self.address)
         clist = None  # Initially, no command list is being constructed.
         while True:
             line = yield self.sock.readline()
             if not line:
+                self.disconnect()  # Client disappeared.
                 break
             line = line.strip()
             if not line:
+                err = BPDError(ERROR_UNKNOWN, u'No command given')
+                yield self.send(err.response())
+                self.disconnect()  # Client sent a blank line.
                 break
             line = line.decode('utf8')  # MPD protocol uses UTF-8.
-            message = line.replace(u'\n', u'\n' + u' ' * 13)
-            self.server._log.debug(u'client: {}', message)
+            for l in line.split(NEWLINE):
+                self.server._log.debug('{}', session + l)
 
             if clist is not None:
                 # Command list already opened.
@@ -710,6 +736,7 @@ def run(self):
                 except BPDClose:
                     # Command indicates that the conn should close.
                     self.sock.close()
+                    self.disconnect()  # Client explicitly closed.
                     return
 
     @classmethod
diff --git a/test/test_player.py b/test/test_player.py
index aa3c3d6a8a..baf1ddfdb5 100644
--- a/test/test_player.py
+++ b/test/test_player.py
@@ -367,6 +367,11 @@ def test_system_error(self):
             response = client.send_command('crash_TypeError')
         self._assert_failed(response, bpd.ERROR_SYSTEM)
 
+    def test_empty_request(self):
+        with self.run_bpd() as client:
+            response = client.send_command('')
+        self._assert_failed(response, bpd.ERROR_UNKNOWN)
+
 
 class BPDQueryTest(BPDTestHelper):
     test_implements_query = implements({

From 7105c800aa3ba0da305110c77a44fd73606af9f4 Mon Sep 17 00:00:00 2001
From: Carl Suster <carl@contraflo.ws>
Date: Mon, 8 Apr 2019 11:36:32 +1000
Subject: [PATCH 04/13] bpd: implement the idle command

Getting this command puts the connection into a special mode where it
awaits MPD events (like the player changing state or the playlist
changing due to other clients interacting with the server.

The MPD specification states that events should queue while a client is
connected, and when it issues the `idle` command any matching events
should be sent immediately if there are any, or as soon as they happen
otherwise.
---
 beetsplug/bpd/__init__.py | 78 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py
index 153fd675ed..2cf0c22ce3 100644
--- a/beetsplug/bpd/__init__.py
+++ b/beetsplug/bpd/__init__.py
@@ -27,6 +27,7 @@
 import time
 import math
 import inspect
+import socket
 
 import beets
 from beets.plugins import BeetsPlugin
@@ -72,6 +73,10 @@
     u'close', u'commands', u'notcommands', u'password', u'ping',
 )
 
+# List of subsystems/events used by the `idle` command.
+SUBSYSTEMS = [
+]
+
 ITEM_KEYS_WRITABLE = set(MediaFile.fields()).intersection(Item._fields.keys())
 
 
@@ -147,6 +152,16 @@ class BPDClose(Exception):
     should be closed.
     """
 
+
+class BPDIdle(Exception):
+    """Raised by a command to indicate the client wants to enter the idle state
+    and should be notified when a relevant event happens.
+    """
+    def __init__(self, subsystems):
+        super(BPDIdle, self).__init__()
+        self.subsystems = set(subsystems)
+
+
 # Generic server infrastructure, implementing the basic protocol.
 
 
@@ -211,6 +226,11 @@ def run(self):
         bluelet.run(bluelet.server(self.host, self.port,
                                    Connection.handler(self)))
 
+    def _send_event(self, event):
+        """Notify subscribed connections of an event."""
+        for conn in self.connections:
+            conn.notify(event)
+
     def _item_info(self, item):
         """An abstract method that should response lines containing a
         single song's metadata.
@@ -271,6 +291,14 @@ def cmd_ping(self, conn):
         """Succeeds."""
         pass
 
+    def cmd_idle(self, conn, *subsystems):
+        subsystems = subsystems or SUBSYSTEMS
+        for system in subsystems:
+            if system not in SUBSYSTEMS:
+                raise BPDError(ERROR_ARG,
+                               u'Unrecognised idle event: {}'.format(system))
+        raise BPDIdle(subsystems)  # put the connection to sleep
+
     def cmd_kill(self, conn):
         """Exits the server process."""
         exit(0)
@@ -657,6 +685,7 @@ def __init__(self, server, sock):
         self.sock = sock
         self.authenticated = False
         self.address = u'{}:{}'.format(*sock.sock.getpeername())
+        self.notifications = set()
 
     def send(self, lines):
         """Send lines, which which is either a single string or an
@@ -692,6 +721,50 @@ def disconnect(self):
         self.server.disconnect(self)
         self.server._log.debug(u'*[{}]: disconnected', self.address)
 
+    def poll_notifications(self, subsystems):
+        """Sleep until we have some notifications from the subsystems given.
+        In order to promptly detect if the client has disconnected while
+        idling, try reading a single byte from the socket. According to the MPD
+        protocol the client can send the special command `noidle` to cancel
+        idle mode, otherwise we're expecting either a timeout or a zero-byte
+        reply. When we have notifications, send them to the client.
+        """
+        while True:
+            mpd_events = self.notifications.intersection(subsystems)
+            if mpd_events:
+                break
+            current_timeout = self.sock.sock.gettimeout()
+            try:
+                self.sock.sock.settimeout(0.01)
+                data = self.sock.sock.recv(1)
+                if data:  # Client sent data when it was meant to by idle.
+                    line = yield self.sock.readline()
+                    command = (data + line).rstrip()
+                    if command == b'noidle':
+                        self.server._log.debug(
+                                u'<[{}]: noidle'.format(self.address))
+                        break
+                    err = BPDError(
+                            ERROR_UNKNOWN,
+                            u'Got command while idle: {}'.format(
+                                command.decode('utf-8')))
+                    yield self.send(err.response())
+                    return
+                else:  # The socket has been closed.
+                    return
+            except socket.timeout:  # The socket is still alive.
+                pass
+            finally:
+                self.sock.sock.settimeout(current_timeout)
+            yield bluelet.sleep(0.02)
+        self.notifications = self.notifications.difference(subsystems)
+        for event in mpd_events:
+            yield self.send(u'changed: {}'.format(event))
+        yield self.send(RESP_OK)
+
+    def notify(self, event):
+        self.notifications.add(event)
+
     def run(self):
         """Send a greeting to the client and begin processing commands
         as they arrive.
@@ -738,6 +811,8 @@ def run(self):
                     self.sock.close()
                     self.disconnect()  # Client explicitly closed.
                     return
+                except BPDIdle as e:
+                    yield bluelet.call(self.poll_notifications(e.subsystems))
 
     @classmethod
     def handler(cls, server):
@@ -832,6 +907,9 @@ def run(self, conn):
             # it on the Connection.
             raise
 
+        except BPDIdle:
+            raise
+
         except Exception:
             # An "unintentional" error. Hide it from the client.
             conn.server._log.error('{}', traceback.format_exc())

From 699de94f4f7558a09cb33e0e11837b4468653026 Mon Sep 17 00:00:00 2001
From: Carl Suster <carl@contraflo.ws>
Date: Mon, 8 Apr 2019 11:37:05 +1000
Subject: [PATCH 05/13] bpd: send all relevant idle events

---
 beetsplug/bpd/__init__.py | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py
index 2cf0c22ce3..664eaa4bbe 100644
--- a/beetsplug/bpd/__init__.py
+++ b/beetsplug/bpd/__init__.py
@@ -75,6 +75,10 @@
 
 # List of subsystems/events used by the `idle` command.
 SUBSYSTEMS = [
+    u'update', u'player', u'mixer', u'options', u'playlist', u'database',
+    # Related to unsupported commands:
+    # u'stored_playlist', u'output', u'subscription', u'sticker', u'message',
+    # u'partition',
 ]
 
 ITEM_KEYS_WRITABLE = set(MediaFile.fields()).intersection(Item._fields.keys())
@@ -393,19 +397,23 @@ def cmd_clearerror(self, conn):
     def cmd_random(self, conn, state):
         """Set or unset random (shuffle) mode."""
         self.random = cast_arg('intbool', state)
+        self._send_event('options')
 
     def cmd_repeat(self, conn, state):
         """Set or unset repeat mode."""
         self.repeat = cast_arg('intbool', state)
+        self._send_event('options')
 
     def cmd_consume(self, conn, state):
         """Set or unset consume mode."""
         self.consume = cast_arg('intbool', state)
+        self._send_event('options')
 
     def cmd_single(self, conn, state):
         """Set or unset single mode."""
         # TODO support oneshot in addition to 0 and 1 [MPD 0.20]
         self.single = cast_arg('intbool', state)
+        self._send_event('options')
 
     def cmd_setvol(self, conn, vol):
         """Set the player's volume level (0-100)."""
@@ -413,6 +421,7 @@ def cmd_setvol(self, conn, vol):
         if vol < VOLUME_MIN or vol > VOLUME_MAX:
             raise BPDError(ERROR_ARG, u'volume out of range')
         self.volume = vol
+        self._send_event('mixer')
 
     def cmd_volume(self, conn, vol_delta):
         """Deprecated command to change the volume by a relative amount."""
@@ -425,6 +434,7 @@ def cmd_crossfade(self, conn, crossfade):
             raise BPDError(ERROR_ARG, u'crossfade time must be nonnegative')
         self._log.warning(u'crossfade is not implemented in bpd')
         self.crossfade = crossfade
+        self._send_event('options')
 
     def cmd_mixrampdb(self, conn, db):
         """Set the mixramp normalised max volume in dB."""
@@ -433,6 +443,7 @@ def cmd_mixrampdb(self, conn, db):
             raise BPDError(ERROR_ARG, u'mixrampdb time must be negative')
         self._log.warning('mixramp is not implemented in bpd')
         self.mixrampdb = db
+        self._send_event('options')
 
     def cmd_mixrampdelay(self, conn, delay):
         """Set the mixramp delay in seconds."""
@@ -441,6 +452,7 @@ def cmd_mixrampdelay(self, conn, delay):
             raise BPDError(ERROR_ARG, u'mixrampdelay time must be nonnegative')
         self._log.warning('mixramp is not implemented in bpd')
         self.mixrampdelay = delay
+        self._send_event('options')
 
     def cmd_replay_gain_mode(self, conn, mode):
         """Set the replay gain mode."""
@@ -448,6 +460,7 @@ def cmd_replay_gain_mode(self, conn, mode):
             raise BPDError(ERROR_ARG, u'Unrecognised replay gain mode')
         self._log.warning('replay gain is not implemented in bpd')
         self.replay_gain_mode = mode
+        self._send_event('options')
 
     def cmd_replay_gain_status(self, conn):
         """Get the replaygain mode."""
@@ -458,6 +471,7 @@ def cmd_clear(self, conn):
         self.playlist = []
         self.playlist_version += 1
         self.cmd_stop(conn)
+        self._send_event('playlist')
 
     def cmd_delete(self, conn, index):
         """Remove the song at index from the playlist."""
@@ -473,6 +487,7 @@ def cmd_delete(self, conn, index):
         elif index < self.current_index:  # Deleted before playing.
             # Shift playing index down.
             self.current_index -= 1
+        self._send_event('playlist')
 
     def cmd_deleteid(self, conn, track_id):
         self.cmd_delete(conn, self._id_to_index(track_id))
@@ -496,6 +511,7 @@ def cmd_move(self, conn, idx_from, idx_to):
             self.current_index += 1
 
         self.playlist_version += 1
+        self._send_event('playlist')
 
     def cmd_moveid(self, conn, idx_from, idx_to):
         idx_from = self._id_to_index(idx_from)
@@ -521,6 +537,7 @@ def cmd_swap(self, conn, i, j):
             self.current_index = i
 
         self.playlist_version += 1
+        self._send_event('playlist')
 
     def cmd_swapid(self, conn, i_id, j_id):
         i = self._id_to_index(i_id)
@@ -578,6 +595,7 @@ def cmd_next(self, conn):
         """Advance to the next song in the playlist."""
         old_index = self.current_index
         self.current_index = self._succ_idx()
+        self._send_event('playlist')
         if self.consume:
             # TODO how does consume interact with single+repeat?
             self.playlist.pop(old_index)
@@ -598,6 +616,7 @@ def cmd_previous(self, conn):
         """Step back to the last song."""
         old_index = self.current_index
         self.current_index = self._prev_idx()
+        self._send_event('playlist')
         if self.consume:
             self.playlist.pop(old_index)
         if self.current_index < 0:
@@ -613,6 +632,7 @@ def cmd_pause(self, conn, state=None):
             self.paused = not self.paused  # Toggle.
         else:
             self.paused = cast_arg('intbool', state)
+        self._send_event('player')
 
     def cmd_play(self, conn, index=-1):
         """Begin playback, possibly at a specified playlist index."""
@@ -632,6 +652,7 @@ def cmd_play(self, conn, index=-1):
             self.current_index = index
 
         self.paused = False
+        self._send_event('player')
 
     def cmd_playid(self, conn, track_id=0):
         track_id = cast_arg(int, track_id)
@@ -645,6 +666,7 @@ def cmd_stop(self, conn):
         """Stop playback."""
         self.current_index = -1
         self.paused = False
+        self._send_event('player')
 
     def cmd_seek(self, conn, index, pos):
         """Seek to a specified point in a specified song."""
@@ -652,6 +674,7 @@ def cmd_seek(self, conn, index, pos):
         if index < 0 or index >= len(self.playlist):
             raise ArgumentIndexError()
         self.current_index = index
+        self._send_event('player')
 
     def cmd_seekid(self, conn, track_id, pos):
         index = self._id_to_index(track_id)
@@ -1027,6 +1050,8 @@ def cmd_update(self, conn, path=u'/'):
         self.tree = vfs.libtree(self.lib)
         self._log.debug(u'Finished building directory tree.')
         self.updated_time = time.time()
+        self._send_event('update')
+        self._send_event('database')
 
     # Path (directory tree) browsing.
 
@@ -1136,6 +1161,7 @@ def _add(self, path, send_id=False):
             if send_id:
                 yield u'Id: ' + six.text_type(item.id)
         self.playlist_version += 1
+        self._send_event('playlist')
 
     def cmd_add(self, conn, path):
         """Adds a track or directory to the playlist, specified by a

From d05ca2c2b081336d992d13efb461a121a672cc2b Mon Sep 17 00:00:00 2001
From: Carl Suster <carl@contraflo.ws>
Date: Fri, 5 Apr 2019 11:18:42 +1100
Subject: [PATCH 06/13] bpd: add tests for idle command

---
 test/test_player.py | 58 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 51 insertions(+), 7 deletions(-)

diff --git a/test/test_player.py b/test/test_player.py
index baf1ddfdb5..7a29610e3a 100644
--- a/test/test_player.py
+++ b/test/test_player.py
@@ -23,6 +23,7 @@
 import os
 import sys
 import multiprocessing as mp
+import threading
 import socket
 import time
 import yaml
@@ -37,18 +38,19 @@
 import mock
 import imp
 gstplayer = imp.new_module("beetsplug.bpd.gstplayer")
-def _gstplayer_play(_):  # noqa: 42
+def _gstplayer_play(*_):  # noqa: 42
     bpd.gstplayer._GstPlayer.playing = True
     return mock.DEFAULT
 gstplayer._GstPlayer = mock.MagicMock(
     spec_set=[
         "time", "volume", "playing", "run", "play_file", "pause", "stop",
-        "seek"
+        "seek", "play"
     ], **{
         'playing': False,
         'volume': 0,
         'time.return_value': (0, 0),
         'play_file.side_effect': _gstplayer_play,
+        'play.side_effect': _gstplayer_play,
     })
 gstplayer.GstPlayer = lambda _: gstplayer._GstPlayer
 sys.modules["beetsplug.bpd.gstplayer"] = gstplayer
@@ -259,7 +261,7 @@ def tearDown(self):
 
     @contextmanager
     def run_bpd(self, host='localhost', port=9876, password=None,
-                do_hello=True):
+                do_hello=True, second_client=False):
         """ Runs BPD in another process, configured with the same library
         database as we created in the setUp method. Exposes a client that is
         connected to the server, and kills the server at the end.
@@ -290,7 +292,7 @@ def run_bpd(self, host='localhost', port=9876, password=None,
         server.start()
 
         # Wait until the socket is connected:
-        sock = None
+        sock, sock2 = None, None
         for _ in range(20):
             sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
             if sock.connect_ex((host, port)) == 0:
@@ -302,9 +304,16 @@ def run_bpd(self, host='localhost', port=9876, password=None,
             raise RuntimeError('Timed out waiting for the BPD server')
 
         try:
-            yield MPCClient(sock, do_hello)
+            if second_client:
+                sock2 = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+                sock2.connect((host, port))
+                yield MPCClient(sock, do_hello), MPCClient(sock2, do_hello)
+            else:
+                yield MPCClient(sock, do_hello)
         finally:
             sock.close()
+            if sock2:
+                sock2.close()
             server.terminate()
             server.join(timeout=0.2)
 
@@ -375,8 +384,8 @@ def test_empty_request(self):
 
 class BPDQueryTest(BPDTestHelper):
     test_implements_query = implements({
-            'clearerror', 'currentsong', 'idle', 'stats',
-            }, expectedFailure=True)
+            'clearerror', 'currentsong', 'stats',
+            })
 
     def test_cmd_status(self):
         with self.run_bpd() as client:
@@ -397,6 +406,41 @@ def test_cmd_status(self):
         }
         self.assertEqual(fields_playing, set(responses[2].data.keys()))
 
+    def test_cmd_idle(self):
+        def _toggle(c):
+            for _ in range(3):
+                rs = c.send_commands(('play',), ('pause',))
+                # time.sleep(0.05)  # uncomment if test is flaky
+                if any(not r.ok for r in rs):
+                    raise RuntimeError('Toggler failed')
+        with self.run_bpd(second_client=True) as (client, client2):
+            self._bpd_add(client, self.item1, self.item2)
+            toggler = threading.Thread(target=_toggle, args=(client2,))
+            toggler.start()
+            # Idling will hang until the toggler thread changes the play state.
+            # Since the client sockets have a 1s timeout set at worst this will
+            # raise a socket.timeout and fail the test if the toggler thread
+            # manages to finish before the idle command is sent here.
+            response = client.send_command('idle', 'player')
+            toggler.join()
+        self._assert_ok(response)
+
+    def test_cmd_idle_with_pending(self):
+        with self.run_bpd(second_client=True) as (client, client2):
+            response1 = client.send_command('random', '1')
+            response2 = client2.send_command('idle')
+        self._assert_ok(response1, response2)
+        self.assertEqual('options', response2.data['changed'])
+
+    def test_cmd_noidle(self):
+        with self.run_bpd() as client:
+            # Manually send a command without reading a response.
+            request = client.serialise_command('idle')
+            client.sock.sendall(request)
+            time.sleep(0.01)
+            response = client.send_command('noidle')
+        self._assert_ok(response)
+
 
 class BPDPlaybackTest(BPDTestHelper):
     test_implements_playback = implements({

From 6fbf3853f27e736f009343ff21f1251eaeaabc61 Mon Sep 17 00:00:00 2001
From: Carl Suster <carl@contraflo.ws>
Date: Fri, 5 Apr 2019 11:20:00 +1100
Subject: [PATCH 07/13] bpd: bump protocol version to 0.14.0

---
 beetsplug/bpd/__init__.py | 2 +-
 test/test_player.py       | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py
index 664eaa4bbe..fdd78a11fb 100644
--- a/beetsplug/bpd/__init__.py
+++ b/beetsplug/bpd/__init__.py
@@ -39,7 +39,7 @@
 from beets.mediafile import MediaFile
 import six
 
-PROTOCOL_VERSION = '0.13.0'
+PROTOCOL_VERSION = '0.14.0'
 BUFSIZE = 1024
 
 HELLO = u'OK MPD %s' % PROTOCOL_VERSION
diff --git a/test/test_player.py b/test/test_player.py
index 7a29610e3a..3fd5910b44 100644
--- a/test/test_player.py
+++ b/test/test_player.py
@@ -354,7 +354,7 @@ def _bpd_add(self, client, *items, **kwargs):
 class BPDTest(BPDTestHelper):
     def test_server_hello(self):
         with self.run_bpd(do_hello=False) as client:
-            self.assertEqual(client.readline(), b'OK MPD 0.13.0\n')
+            self.assertEqual(client.readline(), b'OK MPD 0.14.0\n')
 
     def test_unknown_cmd(self):
         with self.run_bpd() as client:

From 275301750a169d4f04981de70d822a961a6aed5a Mon Sep 17 00:00:00 2001
From: Carl Suster <carl@contraflo.ws>
Date: Fri, 5 Apr 2019 20:07:11 +1100
Subject: [PATCH 08/13] Changelog for #3205

---
 docs/changelog.rst | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/docs/changelog.rst b/docs/changelog.rst
index 43b6b20f69..e41d71bd29 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -86,6 +86,9 @@ New features:
   new fields for ``status``. The bpd server now understands and ignores some
   additional commands.
   :bug:`3200` :bug:`800`
+* :doc:`/plugins/bpd`: MPD protocol command ``idle`` is now supported, allowing
+  the MPD version to be bumped to 0.14.
+  :bug:`3205` :bug:`800`
 
 Changes:
 

From e70b2134e4537d63a502d63c58e44c013769fd17 Mon Sep 17 00:00:00 2001
From: Carl Suster <carl@contraflo.ws>
Date: Sat, 6 Apr 2019 13:34:06 +1100
Subject: [PATCH 09/13] bpd: update documentation

---
 docs/plugins/bpd.rst | 66 +++++++++++++++++++++++---------------------
 1 file changed, 34 insertions(+), 32 deletions(-)

diff --git a/docs/plugins/bpd.rst b/docs/plugins/bpd.rst
index fc22846de6..7757ba8933 100644
--- a/docs/plugins/bpd.rst
+++ b/docs/plugins/bpd.rst
@@ -95,40 +95,42 @@ on-disk directory structure can. (Note that an obvious solution to this is just
 string matching on items' destination, but this requires examining the entire
 library Python-side for every query.)
 
-We don't currently support versioned playlists. Many clients, however, use
-plchanges instead of playlistinfo to get the current playlist, so plchanges
-contains a dummy implementation that just calls playlistinfo.
+BPD plays music using GStreamer's ``playbin`` player, which has a simple API
+but doesn't support many advanced playback features.
 
-The ``stats`` command always send zero for ``playtime``, which is supposed to
-indicate the amount of time the server has spent playing music. BPD doesn't
-currently keep track of this.
+Differences from the real MPD
+-----------------------------
 
-The ``update`` command regenerates the directory tree from the beets database.
-
-Unimplemented Commands
-----------------------
-
-These are the commands from `the MPD protocol`_ that have not yet been
-implemented in BPD.
+BPD currently supports version 0.14 of `the MPD protocol`_, but several of the
+commands and features are "pretend" implementations or have slightly different
+behaviour to their MPD equivalents. BPD aims to look enough like MPD that it
+can interact with the ecosystem of clients, but doesn't try to be
+a fully-fledged MPD replacement in terms of its playback capabilities.
 
 .. _the MPD protocol: http://www.musicpd.org/doc/protocol/
 
-Saved playlists:
-
-* playlistclear
-* playlistdelete
-* playlistmove
-* playlistadd
-* playlistsearch
-* listplaylist
-* listplaylistinfo
-* playlistfind
-* rm
-* save
-* load
-* rename
-
-Deprecated:
-
-* playlist
-* volume
+These are some of the known differences between BPD and MPD:
+
+* BPD doesn't currently support versioned playlists. Many clients, however, use
+  plchanges instead of playlistinfo to get the current playlist, so plchanges
+  contains a dummy implementation that just calls playlistinfo.
+* Stored playlists aren't supported (BPD understands the commands though).
+* The ``stats`` command always send zero for ``playtime``, which is supposed to
+  indicate the amount of time the server has spent playing music. BPD doesn't
+  currently keep track of this.
+* The ``update`` command regenerates the directory tree from the beets database
+  synchronously, whereas MPD does this in the background.
+* Advanced playback features like cross-fade, ReplayGain and MixRamp are not
+  supported due to BPD's simple audio player backend.
+* Advanced query syntax is not currently supported.
+* Not all tags (fields) are currently exposed to BPD. Clients also can't use
+  the ``tagtypes`` mask to hide fields.
+* BPD's ``random`` mode is not deterministic and doesn't support priorities.
+* Mounts and streams are not supported. BPD can only play files from disk.
+* Stickers are not supported (although this is basically a flexattr in beets
+  nomenclature so this is feasible to add).
+* There is only a single password, and is enabled it grants access to all
+  features rather than having permissions-based granularity.
+* Partitions and alternative outputs are not supported; BPD can only play one
+  song at a time.
+* Client channels are not implemented.

From fa3813844c65199554756e03ec029614e3af5055 Mon Sep 17 00:00:00 2001
From: Carl Suster <carl@contraflo.ws>
Date: Mon, 8 Apr 2019 13:03:44 +1000
Subject: [PATCH 10/13] bpd: reimplement idle without polling

---
 beetsplug/bpd/__init__.py | 85 +++++++++++++++++++--------------------
 1 file changed, 41 insertions(+), 44 deletions(-)

diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py
index fdd78a11fb..229078e590 100644
--- a/beetsplug/bpd/__init__.py
+++ b/beetsplug/bpd/__init__.py
@@ -27,7 +27,6 @@
 import time
 import math
 import inspect
-import socket
 
 import beets
 from beets.plugins import BeetsPlugin
@@ -230,6 +229,14 @@ def run(self):
         bluelet.run(bluelet.server(self.host, self.port,
                                    Connection.handler(self)))
 
+    def dispatch_events(self):
+        """If any clients have idle events ready, send them.
+        """
+        # We need a copy of `self.connections` here since clients might
+        # disconnect once we try and send to them, changing `self.connections`.
+        for conn in list(self.connections):
+            yield bluelet.spawn(conn.send_notifications())
+
     def _send_event(self, event):
         """Notify subscribed connections of an event."""
         for conn in self.connections:
@@ -301,7 +308,7 @@ def cmd_idle(self, conn, *subsystems):
             if system not in SUBSYSTEMS:
                 raise BPDError(ERROR_ARG,
                                u'Unrecognised idle event: {}'.format(system))
-        raise BPDIdle(subsystems)  # put the connection to sleep
+        raise BPDIdle(subsystems)  # put the connection into idle mode
 
     def cmd_kill(self, conn):
         """Exits the server process."""
@@ -709,6 +716,7 @@ def __init__(self, server, sock):
         self.authenticated = False
         self.address = u'{}:{}'.format(*sock.sock.getpeername())
         self.notifications = set()
+        self.idle_subscriptions = set()
 
     def send(self, lines):
         """Send lines, which which is either a single string or an
@@ -744,50 +752,25 @@ def disconnect(self):
         self.server.disconnect(self)
         self.server._log.debug(u'*[{}]: disconnected', self.address)
 
-    def poll_notifications(self, subsystems):
-        """Sleep until we have some notifications from the subsystems given.
-        In order to promptly detect if the client has disconnected while
-        idling, try reading a single byte from the socket. According to the MPD
-        protocol the client can send the special command `noidle` to cancel
-        idle mode, otherwise we're expecting either a timeout or a zero-byte
-        reply. When we have notifications, send them to the client.
-        """
-        while True:
-            mpd_events = self.notifications.intersection(subsystems)
-            if mpd_events:
-                break
-            current_timeout = self.sock.sock.gettimeout()
-            try:
-                self.sock.sock.settimeout(0.01)
-                data = self.sock.sock.recv(1)
-                if data:  # Client sent data when it was meant to by idle.
-                    line = yield self.sock.readline()
-                    command = (data + line).rstrip()
-                    if command == b'noidle':
-                        self.server._log.debug(
-                                u'<[{}]: noidle'.format(self.address))
-                        break
-                    err = BPDError(
-                            ERROR_UNKNOWN,
-                            u'Got command while idle: {}'.format(
-                                command.decode('utf-8')))
-                    yield self.send(err.response())
-                    return
-                else:  # The socket has been closed.
-                    return
-            except socket.timeout:  # The socket is still alive.
-                pass
-            finally:
-                self.sock.sock.settimeout(current_timeout)
-            yield bluelet.sleep(0.02)
-        self.notifications = self.notifications.difference(subsystems)
-        for event in mpd_events:
-            yield self.send(u'changed: {}'.format(event))
-        yield self.send(RESP_OK)
-
     def notify(self, event):
+        """Queue up an event for sending to this client.
+        """
         self.notifications.add(event)
 
+    def send_notifications(self, force_close_idle=False):
+        """Send the client any queued events now.
+        """
+        pending = self.notifications.intersection(self.idle_subscriptions)
+        try:
+            for event in pending:
+                yield self.send(u'changed: {}'.format(event))
+            if pending or force_close_idle:
+                self.idle_subscriptions = set()
+                self.notifications = self.notifications.difference(pending)
+                yield self.send(RESP_OK)
+        except bluelet.SocketClosedError:
+            self.disconnect()  # Client disappeared.
+
     def run(self):
         """Send a greeting to the client and begin processing commands
         as they arrive.
@@ -813,6 +796,17 @@ def run(self):
             for l in line.split(NEWLINE):
                 self.server._log.debug('{}', session + l)
 
+            if self.idle_subscriptions:
+                # The connection is in idle mode.
+                if line == u'noidle':
+                    yield bluelet.call(self.send_notifications(True))
+                else:
+                    err = BPDError(ERROR_UNKNOWN,
+                                   u'Got command while idle: {}'.format(line))
+                    yield self.send(err.response())
+                    break
+                continue
+
             if clist is not None:
                 # Command list already opened.
                 if line == CLIST_END:
@@ -835,7 +829,10 @@ def run(self):
                     self.disconnect()  # Client explicitly closed.
                     return
                 except BPDIdle as e:
-                    yield bluelet.call(self.poll_notifications(e.subsystems))
+                    self.idle_subscriptions = e.subsystems
+                    self.server._log.debug('z[{}]: awaiting: {}', self.address,
+                                           ' '.join(e.subsystems))
+            yield bluelet.call(self.server.dispatch_events())
 
     @classmethod
     def handler(cls, server):

From d55f061f0b89919b6b641524f2cdf4571c16238f Mon Sep 17 00:00:00 2001
From: Carl Suster <carl@contraflo.ws>
Date: Tue, 9 Apr 2019 11:40:49 +1000
Subject: [PATCH 11/13] bpd: add control socket

A new `ControlConnection` is created each time a client connects over
a new control socket. This is used to forward events from the player,
and also for debugging utilities that are not part of the real MPD
protocol.

This new feature reuses as much infrastructure from the normal protocol
handling as possible (e.g. `Command` for parsing messages). While the
normal connection delegates to server `cmd_*` methods which are string
generators, the control connections delegate to `ctrl_*` methods defined
on the connection itself that are full coroutines.
---
 beetsplug/bpd/__init__.py | 193 ++++++++++++++++++++++++++++----------
 test/test_player.py       |   2 +-
 2 files changed, 146 insertions(+), 49 deletions(-)

diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py
index 229078e590..5ebee7e467 100644
--- a/beetsplug/bpd/__init__.py
+++ b/beetsplug/bpd/__init__.py
@@ -27,6 +27,7 @@
 import time
 import math
 import inspect
+import socket
 
 import beets
 from beets.plugins import BeetsPlugin
@@ -181,12 +182,14 @@ class BaseServer(object):
     This is a generic superclass and doesn't support many commands.
     """
 
-    def __init__(self, host, port, password, log):
+    def __init__(self, host, port, password, ctrl_port, log, ctrl_host=None):
         """Create a new server bound to address `host` and listening
         on port `port`. If `password` is given, it is required to do
         anything significant on the server.
         """
         self.host, self.port, self.password = host, port, password
+        self.ctrl_host, self.ctrl_port = ctrl_host or host, ctrl_port
+        self.ctrl_sock = None
         self._log = log
 
         # Default server values.
@@ -226,8 +229,14 @@ def run(self):
         interrupt (^C) closes the server.
         """
         self.startup_time = time.time()
-        bluelet.run(bluelet.server(self.host, self.port,
-                                   Connection.handler(self)))
+
+        def start():
+            yield bluelet.spawn(
+                    bluelet.server(self.ctrl_host, self.ctrl_port,
+                                   ControlConnection.handler(self)))
+            yield bluelet.server(self.host, self.port,
+                                 MPDConnection.handler(self))
+        bluelet.run(start())
 
     def dispatch_events(self):
         """If any clients have idle events ready, send them.
@@ -689,12 +698,6 @@ def cmd_seekid(self, conn, track_id, pos):
 
     # Debugging/testing commands that are not part of the MPD protocol.
 
-    def cmd_profile(self, conn):
-        """Memory profiling for debugging."""
-        from guppy import hpy
-        heap = hpy().heap()
-        print(heap)
-
     def cmd_crash_TypeError(self, conn):  # noqa: N802
         """Deliberately trigger a TypeError for testing purposes.
         We want to test that the server properly responds with ERROR_SYSTEM
@@ -705,18 +708,21 @@ def cmd_crash_TypeError(self, conn):  # noqa: N802
 
 
 class Connection(object):
-    """A connection between a client and the server. Handles input and
-    output from and to the client.
+    """A connection between a client and the server.
+
     """
     def __init__(self, server, sock):
         """Create a new connection for the accepted socket `client`.
         """
         self.server = server
         self.sock = sock
-        self.authenticated = False
         self.address = u'{}:{}'.format(*sock.sock.getpeername())
-        self.notifications = set()
-        self.idle_subscriptions = set()
+
+    def debug(self, message, kind=' '):
+        self.server._log.debug(u'{}[{}]: {}', kind, self.address, message)
+
+    def run(self):
+        pass
 
     def send(self, lines):
         """Send lines, which which is either a single string or an
@@ -727,13 +733,32 @@ def send(self, lines):
         if isinstance(lines, six.string_types):
             lines = [lines]
         out = NEWLINE.join(lines) + NEWLINE
-        session = u'>[{}]: '.format(self.address)
         for l in out.split(NEWLINE)[:-1]:
-            self.server._log.debug('{}', session + l)
+            self.debug(l, kind='>')
         if isinstance(out, six.text_type):
             out = out.encode('utf-8')
         return self.sock.sendall(out)
 
+    @classmethod
+    def handler(cls, server):
+        def _handle(sock):
+            """Creates a new `Connection` and runs it.
+            """
+            return cls(server, sock).run()
+        return _handle
+
+
+class MPDConnection(Connection):
+    """A connection that receives commands from an MPD-compatible client.
+    """
+    def __init__(self, server, sock):
+        """Create a new connection for the accepted socket `client`.
+        """
+        super(MPDConnection, self).__init__(server, sock)
+        self.authenticated = False
+        self.notifications = set()
+        self.idle_subscriptions = set()
+
     def do_command(self, command):
         """A coroutine that runs the given command and sends an
         appropriate response."""
@@ -750,7 +775,7 @@ def disconnect(self):
         """The connection has closed for any reason.
         """
         self.server.disconnect(self)
-        self.server._log.debug(u'*[{}]: disconnected', self.address)
+        self.debug('disconnected', kind='*')
 
     def notify(self, event):
         """Queue up an event for sending to this client.
@@ -775,11 +800,10 @@ def run(self):
         """Send a greeting to the client and begin processing commands
         as they arrive.
         """
-        self.server._log.debug(u'*[{}]: connected', self.address)
+        self.debug('connected', kind='*')
         self.server.connect(self)
         yield self.send(HELLO)
 
-        session = u'<[{}]: '.format(self.address)
         clist = None  # Initially, no command list is being constructed.
         while True:
             line = yield self.sock.readline()
@@ -794,7 +818,7 @@ def run(self):
                 break
             line = line.decode('utf8')  # MPD protocol uses UTF-8.
             for l in line.split(NEWLINE):
-                self.server._log.debug('{}', session + l)
+                self.debug(l, kind='<')
 
             if self.idle_subscriptions:
                 # The connection is in idle mode.
@@ -830,17 +854,67 @@ def run(self):
                     return
                 except BPDIdle as e:
                     self.idle_subscriptions = e.subsystems
-                    self.server._log.debug('z[{}]: awaiting: {}', self.address,
-                                           ' '.join(e.subsystems))
+                    self.debug('awaiting: {}'.format(' '.join(e.subsystems)),
+                               kind='z')
             yield bluelet.call(self.server.dispatch_events())
 
-    @classmethod
-    def handler(cls, server):
-        def _handle(sock):
-            """Creates a new `Connection` and runs it.
-            """
-            return cls(server, sock).run()
-        return _handle
+
+class ControlConnection(Connection):
+    """A connection used to control BPD for debugging and internal events.
+    """
+    def __init__(self, server, sock):
+        """Create a new connection for the accepted socket `client`.
+        """
+        super(ControlConnection, self).__init__(server, sock)
+
+    def debug(self, message, kind=' '):
+        self.server._log.debug(u'CTRL {}[{}]: {}', kind, self.address, message)
+
+    def run(self):
+        """Listen for control commands and delegate to `ctrl_*` methods.
+        """
+        self.debug('connected', kind='*')
+        while True:
+            line = yield self.sock.readline()
+            if not line:
+                break  # Client disappeared.
+            line = line.strip()
+            if not line:
+                break  # Client sent a blank line.
+            line = line.decode('utf8')  # Protocol uses UTF-8.
+            for l in line.split(NEWLINE):
+                self.debug(l, kind='<')
+            command = Command(line)
+            try:
+                func = command.delegate('ctrl_', self)
+                yield bluelet.call(func(*command.args))
+            except (AttributeError, TypeError) as e:
+                yield self.send('ERROR: {}'.format(e.args[0]))
+            except Exception:
+                yield self.send(['ERROR: server error',
+                                 traceback.format_exc().rstrip()])
+
+    def ctrl_play_finished(self):
+        """Callback from the player signalling a song finished playing.
+        """
+        yield bluelet.call(self.server.dispatch_events())
+
+    def ctrl_profile(self):
+        """Memory profiling for debugging.
+        """
+        from guppy import hpy
+        heap = hpy().heap()
+        yield self.send(heap)
+
+    def ctrl_nickname(self, oldlabel, newlabel):
+        """Rename a client in the log messages.
+        """
+        for c in self.server.connections:
+            if c.address == oldlabel:
+                c.address = newlabel
+                break
+        else:
+            yield self.send(u'ERROR: no such client: {}'.format(oldlabel))
 
 
 class Command(object):
@@ -869,16 +943,17 @@ def __init__(self, s):
                 arg = match[1]
             self.args.append(arg)
 
-    def run(self, conn):
-        """A coroutine that executes the command on the given
-        connection.
+    def delegate(self, prefix, target, extra_args=0):
+        """Get the target method that corresponds to this command.
+        The `prefix` is prepended to the command name and then the resulting
+        name is used to search `target` for a method with a compatible number
+        of arguments.
         """
         # Attempt to get correct command function.
-        func_name = 'cmd_' + self.name
-        if not hasattr(conn.server, func_name):
-            raise BPDError(ERROR_UNKNOWN,
-                           u'unknown command "{}"'.format(self.name))
-        func = getattr(conn.server, func_name)
+        func_name = prefix + self.name
+        if not hasattr(target, func_name):
+            raise AttributeError(u'unknown command "{}"'.format(self.name))
+        func = getattr(target, func_name)
 
         if six.PY2:
             # caution: the fields of the namedtuple are slightly different
@@ -889,8 +964,8 @@ def run(self, conn):
 
         # Check that `func` is able to handle the number of arguments sent
         # by the client (so we can raise ERROR_ARG instead of ERROR_SYSTEM).
-        # Maximum accepted arguments: argspec includes "self" and "conn".
-        max_args = len(argspec.args) - 2
+        # Maximum accepted arguments: argspec includes "self".
+        max_args = len(argspec.args) - 1 - extra_args
         # Minimum accepted arguments: some arguments might be optional.
         min_args = max_args
         if argspec.defaults:
@@ -898,10 +973,22 @@ def run(self, conn):
         wrong_num = (len(self.args) > max_args) or (len(self.args) < min_args)
         # If the command accepts a variable number of arguments skip the check.
         if wrong_num and not argspec.varargs:
-            raise BPDError(ERROR_ARG,
-                           u'wrong number of arguments for "{}"'
-                           .format(self.name),
-                           self.name)
+            raise TypeError(u'wrong number of arguments for "{}"'
+                            .format(self.name), self.name)
+
+        return func
+
+    def run(self, conn):
+        """A coroutine that executes the command on the given
+        connection.
+        """
+        try:
+            # `conn` is an extra argument to all cmd handlers.
+            func = self.delegate('cmd_', conn.server, extra_args=1)
+        except AttributeError as e:
+            raise BPDError(ERROR_UNKNOWN, e.args[0])
+        except TypeError as e:
+            raise BPDError(ERROR_ARG, e.args[0], self.name)
 
         # Ensure we have permission for this command.
         if conn.server.password and \
@@ -976,7 +1063,7 @@ class Server(BaseServer):
     to store its library.
     """
 
-    def __init__(self, library, host, port, password, log):
+    def __init__(self, library, host, port, password, ctrl_port, log):
         try:
             from beetsplug.bpd import gstplayer
         except ImportError as e:
@@ -986,7 +1073,7 @@ def __init__(self, library, host, port, password, log):
             else:
                 raise
         log.info(u'Starting server...')
-        super(Server, self).__init__(host, port, password, log)
+        super(Server, self).__init__(host, port, password, ctrl_port, log)
         self.lib = library
         self.player = gstplayer.GstPlayer(self.play_finished)
         self.cmd_update(None)
@@ -1001,7 +1088,11 @@ def play_finished(self):
         """A callback invoked every time our player finishes a
         track.
         """
+        if not self.ctrl_sock:
+            self.ctrl_sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+            self.ctrl_sock.connect((self.ctrl_host, self.ctrl_port))
         self.cmd_next(None)
+        self.ctrl_sock.sendall(u'play_finished\n'.encode('utf-8'))
 
     # Metadata helper functions.
 
@@ -1439,15 +1530,16 @@ def __init__(self):
         self.config.add({
             'host': u'',
             'port': 6600,
+            'control_port': 6601,
             'password': u'',
             'volume': VOLUME_MAX,
         })
         self.config['password'].redact = True
 
-    def start_bpd(self, lib, host, port, password, volume):
+    def start_bpd(self, lib, host, port, password, volume, ctrl_port):
         """Starts a BPD server."""
         try:
-            server = Server(lib, host, port, password, self._log)
+            server = Server(lib, host, port, password, ctrl_port, self._log)
             server.cmd_setvol(None, volume)
             server.run()
         except NoGstreamerError:
@@ -1464,11 +1556,16 @@ def func(lib, opts, args):
             host = self.config['host'].as_str()
             host = args.pop(0) if args else host
             port = args.pop(0) if args else self.config['port'].get(int)
+            if args:
+                ctrl_port = args.pop(0)
+            else:
+                ctrl_port = self.config['control_port'].get(int)
             if args:
                 raise beets.ui.UserError(u'too many arguments')
             password = self.config['password'].as_str()
             volume = self.config['volume'].get(int)
-            self.start_bpd(lib, host, int(port), password, volume)
+            self.start_bpd(lib, host, int(port), password, volume,
+                           int(ctrl_port))
 
         cmd.func = func
         return [cmd]
diff --git a/test/test_player.py b/test/test_player.py
index 3fd5910b44..6cc0869a71 100644
--- a/test/test_player.py
+++ b/test/test_player.py
@@ -270,7 +270,7 @@ def run_bpd(self, host='localhost', port=9876, password=None,
         config = {
                 'pluginpath': [py3_path(self.temp_dir)],
                 'plugins': 'bpd',
-                'bpd': {'host': host, 'port': port},
+                'bpd': {'host': host, 'port': port, 'control_port': port + 1},
         }
         if password:
             config['bpd']['password'] = password

From 826244777ee0d5f1143ffd9aeefd7ca605789a4d Mon Sep 17 00:00:00 2001
From: Carl Suster <carl@contraflo.ws>
Date: Wed, 10 Apr 2019 15:52:47 +1000
Subject: [PATCH 12/13] bpd: minor control socket refactor

---
 beetsplug/bpd/__init__.py | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py
index 5ebee7e467..11fec08906 100644
--- a/beetsplug/bpd/__init__.py
+++ b/beetsplug/bpd/__init__.py
@@ -186,6 +186,9 @@ def __init__(self, host, port, password, ctrl_port, log, ctrl_host=None):
         """Create a new server bound to address `host` and listening
         on port `port`. If `password` is given, it is required to do
         anything significant on the server.
+        A separate control socket is established listening to `ctrl_host` on
+        port `ctrl_port` which is used to forward notifications from the player
+        and can be sent debug commands (e.g. using netcat).
         """
         self.host, self.port, self.password = host, port, password
         self.ctrl_host, self.ctrl_port = ctrl_host or host, ctrl_port
@@ -246,6 +249,16 @@ def dispatch_events(self):
         for conn in list(self.connections):
             yield bluelet.spawn(conn.send_notifications())
 
+    def _ctrl_send(self, message):
+        """Send some data over the control socket.
+        If it's our first time, open the socket. The message should be a
+        string without a terminal newline.
+        """
+        if not self.ctrl_sock:
+            self.ctrl_sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+            self.ctrl_sock.connect((self.ctrl_host, self.ctrl_port))
+        self.ctrl_sock.sendall((message + u'\n').encode('utf-8'))
+
     def _send_event(self, event):
         """Notify subscribed connections of an event."""
         for conn in self.connections:
@@ -696,7 +709,7 @@ def cmd_seekid(self, conn, track_id, pos):
         index = self._id_to_index(track_id)
         return self.cmd_seek(conn, index, pos)
 
-    # Debugging/testing commands that are not part of the MPD protocol.
+    # Additions to the MPD protocol.
 
     def cmd_crash_TypeError(self, conn):  # noqa: N802
         """Deliberately trigger a TypeError for testing purposes.
@@ -709,7 +722,6 @@ def cmd_crash_TypeError(self, conn):  # noqa: N802
 
 class Connection(object):
     """A connection between a client and the server.
-
     """
     def __init__(self, server, sock):
         """Create a new connection for the accepted socket `client`.
@@ -719,6 +731,8 @@ def __init__(self, server, sock):
         self.address = u'{}:{}'.format(*sock.sock.getpeername())
 
     def debug(self, message, kind=' '):
+        """Log a debug message about this connection.
+        """
         self.server._log.debug(u'{}[{}]: {}', kind, self.address, message)
 
     def run(self):
@@ -836,6 +850,7 @@ def run(self):
                 if line == CLIST_END:
                     yield bluelet.call(self.do_command(clist))
                     clist = None  # Clear the command list.
+                    yield bluelet.call(self.server.dispatch_events())
                 else:
                     clist.append(Command(line))
 
@@ -856,7 +871,7 @@ def run(self):
                     self.idle_subscriptions = e.subsystems
                     self.debug('awaiting: {}'.format(' '.join(e.subsystems)),
                                kind='z')
-            yield bluelet.call(self.server.dispatch_events())
+                yield bluelet.call(self.server.dispatch_events())
 
 
 class ControlConnection(Connection):
@@ -1085,14 +1100,10 @@ def run(self):
         super(Server, self).run()
 
     def play_finished(self):
-        """A callback invoked every time our player finishes a
-        track.
+        """A callback invoked every time our player finishes a track.
         """
-        if not self.ctrl_sock:
-            self.ctrl_sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
-            self.ctrl_sock.connect((self.ctrl_host, self.ctrl_port))
         self.cmd_next(None)
-        self.ctrl_sock.sendall(u'play_finished\n'.encode('utf-8'))
+        self._ctrl_send(u'play_finished')
 
     # Metadata helper functions.
 

From 241e23eae8945eb45069b5b3587509ede4dd3f60 Mon Sep 17 00:00:00 2001
From: Carl Suster <carl@contraflo.ws>
Date: Wed, 10 Apr 2019 15:54:41 +1000
Subject: [PATCH 13/13] bpd: document new control_port config

---
 docs/plugins/bpd.rst | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/docs/plugins/bpd.rst b/docs/plugins/bpd.rst
index 7757ba8933..ee36c040ce 100644
--- a/docs/plugins/bpd.rst
+++ b/docs/plugins/bpd.rst
@@ -75,6 +75,8 @@ The available options are:
   Default: No password.
 - **volume**: Initial volume, as a percentage.
   Default: 100
+- **control_port**: Port for the internal control socket.
+  Default: 6601
 
 Here's an example::