samtherussell / squeezebox-controller

A python interface for controlling Logitech Squeezeboxes via the SqueezeboxServer
MIT License
14 stars 10 forks source link

query fails with player name or MAC address #1

Open davison opened 5 years ago

davison commented 5 years ago

If I use a player name with a query, the exception raised states that I should use a MAC address.

>>> params = {"player": "Office", "query": "NOW PLAYING"}
>>> controller.simple_query(params)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python2.7/dist-packages/squeezebox_controller/__init__.py", line 17, in cached_f
    return f(self, details, *args)
  File "/usr/local/lib/python2.7/dist-packages/squeezebox_controller/__init__.py", line 38, in needs_player_f
    return f(self, details, *args)
  File "/usr/local/lib/python2.7/dist-packages/squeezebox_controller/__init__.py", line 354, in simple_query
    player_info = self._get_player_info(self.player_macs[details['player']])
  File "/usr/local/lib/python2.7/dist-packages/squeezebox_controller/__init__.py", line 372, in _get_player_info
    return self._make_request(player, ["status","-"])["result"]
  File "/usr/local/lib/python2.7/dist-packages/squeezebox_controller/__init__.py", line 385, in _make_request
    raise Exception("Player must be a MAC string or list of MAC strings")
Exception: Player must be a MAC string or list of MAC strings

If I use a MAC address, the exception tells me I should use a name.

>>> params = {"player": "xx:xx:xx:xx:17:3d", "query": "NOW PLAYING"}
>>> controller.simple_query(params)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python2.7/dist-packages/squeezebox_controller/__init__.py", line 17, in cached_f
    return f(self, details, *args)
  File "/usr/local/lib/python2.7/dist-packages/squeezebox_controller/__init__.py", line 37, in needs_player_f
    raise Exception("%s must be one of: %s"%(field, ", ".join(self.player_macs.keys())))
Exception: player must be one of: ALL, PlayRoom, Office, Lounge, DiningRoom, Kitchen
samtherussell commented 5 years ago

haha, oh dear, that sounds a bit silly. Actually, the errors are coming from different levels inside the program. Using the name is correct, but it seems that the code is not picking up the corresponding MAC address to make the command. What does controller.player_macs give you?

davison commented 5 years ago
>>> controller.player_macs
{'ALL': [u'xx:xx:xx:af:40:80', u'xx:xx:xx:db:af:e4', u'xx:xx:xx:36:89:ad', u'xx:xx:xx:4a:17:3d', u'xx:xx:xx:77:bf:d6'], u'PlayRoom': u'xx:xx:xx:db:af:e4', u'Office': u'xx:xx:xx:4a:17:3d', u'Lounge': u'xx:xx:xx:af:40:80', u'DiningRoom': u'xx:xx:xx:36:89:ad', u'Kitchen': u'xx:xx:xx:77:bf:d6'}
davison commented 5 years ago

... is it python3 only (I was using it with 2.7)?

With 3.6.7 I get instead;

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/darren/.local/lib/python3.6/site-packages/squeezebox_controller/__init__.py", line 17, in cached_f
    return f(self, details, *args)
  File "/home/darren/.local/lib/python3.6/site-packages/squeezebox_controller/__init__.py", line 38, in needs_player_f
    return f(self, details, *args)
  File "/home/darren/.local/lib/python3.6/site-packages/squeezebox_controller/__init__.py", line 356, in simple_query
    return queries[details['query']](player_info)
  File "/home/darren/.local/lib/python3.6/site-packages/squeezebox_controller/__init__.py", line 74, in <lambda>
    if 'playlist_loop' in info and len(info['playlist_loop']) > 0 else "Nothing is playing"
KeyError: 'artist'

.. but other commands/queries work

samtherussell commented 5 years ago

Oooo, yeah sorry, I did only test it on python 3. The reason it was throwing an exception in python 2 was that unicode strings are not of type <class 'str'>

samtherussell commented 5 years ago

Wow, I did not test this enough. That is because what is playing does not have an artist - maybe a radio station or something. Should be a small fix.

samtherussell commented 5 years ago

I made a new release 0.15.4 with the updated code including a null check. Let me know if this sorts it out. Thanks

(install with pip install --upgrade squeezebox-controller)

davison commented 5 years ago

I'll check it out, but I managed to run the 0.15.3 version in a debugger (I'm trying to improve my python skills here from a very low base!) and noted similar that the player info which is returned did not include artist, album, genre etc. for any of the tracks in the playlist. i.e. the return value from self.request_lib.post(self.end_point_url, json=payload) did not include them. However, those fields are all visible in the LMS web interface and the playing track at the time certainly has that info available.

On a related note, it feels more natural to me, given that this is an API lib, those data would be better returned from controller.simple_query(params) as raw json so that the client can format the return values as desired. Currently your code makes decisions about how data should be used by its caller. What do you think?

davison commented 5 years ago

I just pulled the latest from git and ran that, so now there's no exception, but also no output from the command at all because the artist info is missing as described.

Testing with the CLI works as expected, so the data should be available via the json API, but I think you have to additionally query the playlist in order to get anything other than the title.

player name 2 ?
player name 2 Office
player id 2 ?
player id 2 xx%3Axx%3Axx%3Axx%3A17%3A3d
xx%3Axx%3Axx%3Axx%3A17%3A3d playlist title 0 ?
xx%3Axx%3Axx%3Axx%3A17%3A3d playlist title 0 Block%20Rockin'%20Beats
xx%3Axx%3Axx%3Axx%3A17%3A3d playlist artist 0 ?
xx%3Axx%3Axx%3Axx%3A17%3A3d playlist artist 0 The%20Chemical%20Brothers
xx%3Axx%3Axx%3Axx%3A17%3A3d playlist genre 0 ?
xx%3Axx%3Axx%3Axx%3A17%3A3d playlist genre 0 Electronica%2FDance

and in the debug console after the request is made to get the player info, the req.text contains

'{"params":["xx:xx:xx:xx:17:3d",["status","-"]],"method":"slim.request","result":{"player_name":"Office","player_connected":1,"player_ip":"192.168.21.100:60440","power":1,"signalstrength":0,"mode":"play","time":341.76535826683,"rate":1,"duration":498.755,"can_seek":1,"mixer volume":77,"playlist repeat":0,"playlist shuffle":0,"playlist mode":"off","seq_no":0,"playlist_cur_index":"2","playlist_timestamp":1554634618.79419,"playlist_tracks":10,"digital_volume_control":1,"playlist_loop":[{"id":14001,"title":"Elektrobank","playlist index":2},{"playlist index":3,"title":"Piku","id":14002},{"playlist index":4,"id":14003,"title":"Setting Sun"},{"title":"It Doesn\'t Matter","id":14004,"playlist index":5},{"playlist index":6,"title":"Don\'t Stop The Rock","id":14005},{"playlist index":7,"id":14006,"title":"Get Up On It Like This"},{"id":14007,"title":"Lost In The K-Hole","playlist index":8},{"playlist index":9,"title":"The Private Psychedelic Reel","id":14008}]}}'

so I would guess that a further call to the playlist API for the track with playlist_cur_index == 0 is maybe needed to make this work as intended for the "NOW PLAYING" query at least?

samtherussell commented 5 years ago

Yeah, true. Could maybe add a RAW option that just returned the whole thing.

Hmm, that is strange though - it works on my system. What squeeze server version are you on? I get an object for each song that includes all the data: Eg.

      "artist": "Robbie Williams",
      "genre": "Rock",
      "playlist index": 3,
      "id": 12408,
      "title": "Monsoon",
      "album": "Escapology",
      "duration": "226.6"
    }

Yes, it seems it will have to do a request to lookup the song's details, though I'm actually not sure how to do that with the jsonrpc api.

If you want to implement this to get it working on your system it'd be great to use it in here as well.

davison commented 5 years ago

I'm on LMS 7.9.1

miggland commented 5 years ago

Great thing you've made here, but you seem to have a small issue with Unicode vs normal strings.

Changing the line (in def _populate_player_macs) player_macs[name] = player['playerid'] to player_macs[name] = str(player['playerid'])

seems to fix the issue with MAC vs player name (otherwise, type(player) == str returns false since type(player) == unicode )