Make class(es) more self-contained, reduce globals

[ferdnyc]

This PR contains a set of what I'm calling "quality of life" changes for the code. Nothing user-visible, this is all an attempt to make the MPDWrapper class and its helpers more self-contained, and importable/usable outside of the mpDris2 source file itself — for testing/debugging purposes. The user should see absolutely no difference in normal operation.

Specifically, this PR eliminates all references from inside the class code to symbols that are only defined if the if __name__== '__main__' block is run. As a result, it's now possible to import the class into an interactive session and call methods on it by hand, without it crashing due to things like mpd_wrapper and loop being undefined, or notification not being callable because its value is None.

Specific changes (as separate commits):

1. Move logger instantiation to top of module (the level is still configured in the __main__-only code)
2. Set up notification in the MPDWrapper.__init__() code as self._notification, since it's never used in the __main__ code but all of the class code assumes it's been properly initialized
3. Still define loop only in the __main__ code (I agree that's a good idea), but pass it in to the MPDWrapper() constructor and use self._loop to access it later.
4. Pass the MPDWrapper instance in to MPRISInterface() along with the params, and store it as MPRISInterface.__wrapper. Then, use that to make calls back to it, instead of trying to use mpd_wrapper which is undefined if the __main__ code wasn't executed

With these changes, it's possible to debug the code interactively via the REPL (after running ln -s mpDris2 mpDris2.py), all that's required is to set up the MainLoop and create a params dictionary, then pass them both to MPDWrapper() thusly:

>>> import mpDris2
>>> mpDris2.DBusGMainLoop(set_as_default=True); loop = mpDris2.GLib.MainLoop()
<dbus.mainloop.NativeMainLoop object at 0x7f6fc6a9e4f0>
>>> mpdw = mpDris2.MPDWrapper({'password': 'XXXXXXX', 'host': 'XXXXXXX', 'port': 6600,
... 'mmkeys': False, 'progname': 'mpdris2', 'bus_name': None, 'notify_urgency': 0,
... 'notify': True, 'cover_regex': None, 'music_dir': ''}, loop=loop)
>>> mpdw.my_connect()
False
>>> mpdw.connected
True
>>> status = mpdw.status()
>>> from pprint import pp
>>> pp(status)
{'volume': '58',
 'repeat': '0',
 'random': '0',
 'single': '0',
 'consume': '0',
 'partition': 'default',
 'playlist': '8',
 'playlistlength': '6',
 'mixrampdb': '0',
 'state': 'play',
 'xfade': '4',
 'song': '2',
 'songid': '3',
 'time': '4:2354',
 'elapsed': '3.995',
 'bitrate': '192',
 'duration': '2353.848',
 'audio': '48000:24:2',
 'nextsong': '3',
 'nextsongid': '4'}
>>> mpdw.pause()

(...and it does.)

[grawity]

3f4d8c4 - I'm not sure I understand this one; maybe it's just been too long since I looked at this code, but why is __wrapper being made a class property? If it's assigned in __init__(), then it should probably be an instance property (i.e. self._wrapper).

[ferdnyc]

3f4d8c4 - I'm not sure I understand this one; maybe it's just been too long since I looked at this code, but why is __wrapper being made a class property? If it's assigned in __init__(), then it should probably be an instance property (i.e. self._wrapper).

Normally, yes, I'd agree. But that didn't work, because the callback functions aren't methods — they don't have access to self. I assume it's something to do with how the DBus interface uses them? It was like that when I found it, and I didn't want to try and change it since that would've been much more invasive. So this was the simpler path towards making sure they could access the wrapper object.

[ferdnyc]

(Forgot the ping: @grawity )

[ferdnyc]

@grawity This is what I was referring to — the setters and getters aren't defined as methods of MPRISInterface, so they don't have access to self.

[ferdnyc]

@grawity I could use self.__wrapper for these methods, if you prefer. I decided to just be consistent, but I'm open to changing it if that was the wrong call.

[eonpatapon]

Shouldn't loop be a required argument ?

[ferdnyc]

@eonpatapon

Wow, ancient history, I don't know how I missed these! Sorry about that.

...I think I originally set these up as defaulted arguments because I didn't want to change the class constructors' semantics until I had my additions finalized. But they probably don't need to stay in that state now.

As far as loop goes, though... I mean, I'm not sure it really has to be required.

The only reason loop is passed in is so the relaunch code in _name_owner_changed_callback() can call self._loop.quit(). If self._loop is None (it checks), it just won't call quit() on it.

Which... seems like it would be fine? In situations where a loop isn't supplied (say, in testing).

[eonpatapon]

mpd_wrapper is required too

[ferdnyc]

@eonpatapon

mpd_wrapper, OTOH... yeah. I absolutely agree, it needs to be required. I'll fix that and rebase this PR to the current HEAD now.

[eonpatapon]

Pushed 6b246e4 to see what it needs to make the wrapper an instance of the class. Feels a bit more idiomatic python to me

Overall it's great to be able to run mpDris2 in the repl