Multimedia keys cause an MPD disconnect due to idling

[ikelos]

Having recently updated mpDris2 after many months, I found that it failed to control MPD using the multimedia keys, and key saying there was an error mid-connection, followed by "no connection" errors for any further media key presses.

It turns out this was due to the idle support that was added, and the multimedia key handling code not entering or leaving the idle state (causing python-mpd to send the play command directly after the idle command, rather than sending a noidle first).

I've created a patch at https://gist.github.com/ikelos/6952321 which solves the problem for me.

[grawity]

Thanks, but would be better if you sent it as a pull request (or at least a git format-patch) instead of just a pastebinned diff.

[grawity]

Also, I spent some time wondering how come it has been working perfectly for me... The explicit idle_leave() and idle_enter() shouldn't be necessary here – the code was written to do it automatically whenever any command is sent to mpd (lines 694-718 in mpDris2.in).

Then again, the whole business of overriding _execute()/_docommand() is really fragile, and I redid it in the python3 branch, but I don't know if anyone else has tested it yet, so I'm not sure about merging it yet.

[ikelos]

Sorry, I figured a git diff would be enough to patch it. I'll produce git format-patches in future.

The overriding of _execute() doesn't look like it's happening, because the various command functions get bound by the following bit of code:

    @classmethod
    def add_command(cls, name, callback):
        method = newFunction(cls._execute, key, callback)

That gets called in the main mpd module (in python-mpd-0.5.1) as follows:

for key, value in _commands.items():
    returnValue = None if value is None else MPDClient.__dict__[value]
    MPDClient.add_command(key, returnValue)

Which means the class passed to the class method will always be MPDClient rather than our MPDWrapper. As such, we'd either need to rebind all the functions ourself using a similar bit of code, or ask the python-mpd guys to make the extra function bindings part of the init or new functions, so they get applied to self rather than an explicit class. I've no idea how that's working on your version, unless you're using a different version of python-mpd, or a different commit of mpDris2 (I'm on commit 0d677e40a6)...

[grawity]

Sorry, I figured a git diff would be enough to patch it. I'll produce git format-patches in future.

It is enough, yes, but generally I like all commits to have the correct attribution.

While I can git apply and git commit --author=..., it's just more proper to send a pull request or post a full patch (with author information, &c).

The overriding of _execute() doesn't look like it's happening

Yes, different python-mpd versions do it differently, which is why there are two different overrides, and also why I want to get rid of the whole thing (commit grawity@207a0801bcba0d83be3462352e71d7833a8941f7) because there are even more python-mpd versions and some of them just make such overriding impossible.

Could you try the branches python3 and python3-gi in my repository (grawity/mpDris2), and check if either of them works well for you? (They should work with both Python 2 and Python 3.)

[ikelos]

Hiya, both the python3 and python3-gi branches worked fine (although Ctrl+C on the python3-gi branch times out rather than stopping mpDris2 immediately).