Skip to content

Commit

Permalink
Improve handling of missing players:
Browse files Browse the repository at this point in the history
 * Tighten `SqueezeboxPlayerSettings` constructor to only allow valid objects (i.e. must find a `playerid` - there could be a parsing bug here somewhere, so let's find it early rather than via `__str__` later...
 * Die explicitly if there are no players found (we can't do anything much in the skill...). Fixes #56.
 * Check for missing DEFAULT_PLAYER and default to first (Fixes #59)
 * Improve log messages a bit
  • Loading branch information
declension committed Jun 24, 2018
1 parent ddf93f4 commit 9db14ef
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 7 deletions.
25 changes: 21 additions & 4 deletions squeezealexa/squeezebox/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

import time

from squeezealexa.utils import with_example, print_d, stronger
from squeezealexa.utils import with_example, print_d, stronger, print_w

import urllib.request as urllib

Expand All @@ -24,8 +24,14 @@ class SqueezeboxException(Exception):
class SqueezeboxPlayerSettings(dict):
"""Encapsulates player settings"""

def __init__(self, data: dict):
super().__init__(data)
if 'playerid' not in data:
raise SqueezeboxException(
"Couldn't find a playerid in {}".format(data))

@property
def id(self):
def id(self) -> str:
return self['playerid']

def __getattr__(self, key):
Expand Down Expand Up @@ -58,8 +64,19 @@ def __init__(self, transport, user=None, password=None,
print_d("Authenticated with %s!" % self)
self.players = {}
self.refresh_status()
self.cur_player_id = pid = cur_player_id or list(self.players)[0]
print_d("Default player is now %s" % (self.players[pid],))
players = list(self.players.values())
if not players:
raise SqueezeboxException("Uh-oh. No players found.")
if not cur_player_id:
self.cur_player_id = players[0].id
elif cur_player_id not in self.players:
print_w("Couldn't find player {id} (found: {all}). "
"Check your DEFAULT_PLAYER config.",
id=cur_player_id, all=", ".join(list(self.players.keys())))
self.cur_player_id = players[0].id
else:
self.cur_player_id = cur_player_id
print_d("Current player is now: {}", self.players[self.cur_player_id])
self.__genres = []
self.__playlists = []
self.__favorites = []
Expand Down
2 changes: 1 addition & 1 deletion squeezealexa/transport/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

class Error(Exception):

def __init__(self, msg, e):
def __init__(self, msg, e=None):
super(Error, self).__init__(msg)
self.message = msg
self.__cause__ = e
Expand Down
36 changes: 34 additions & 2 deletions tests/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,46 @@

from unittest import TestCase

from squeezealexa.squeezebox.server import Server
from pytest import raises

from squeezealexa.squeezebox.server import Server, SqueezeboxPlayerSettings, \
SqueezeboxException
from tests.fake_ssl import FakeTransport, FAKE_LENGTH, A_REAL_STATUS


class TestServer(TestCase):
class TestSqueezeboxPlayerSettings:
def test_raises_if_no_playerid_found(self):
with raises(SqueezeboxException) as e:
SqueezeboxPlayerSettings({})
assert "couldn't find a playerid" in str(e).lower()


class NoRefreshServer(Server):
"""A normal server, that has no transport never returns any players"""

def __init__(self, user=None, password=None, cur_player_id=None):
super().__init__(None, user, password, cur_player_id, False)

def refresh_status(self):
self.players = {}


class TestServerNoTransport:
def test_no_players_raises(self):
with raises(SqueezeboxException) as e:
NoRefreshServer()
assert "no players" in str(e).lower()


class TestServerWithTransport(TestCase):
def setUp(self):
self.server = Server(transport=FakeTransport())

def test_unknown_default_player(self):
transport = FakeTransport(fake_id="foo")
self.server = Server(transport=transport, cur_player_id="GONE")
assert self.server.cur_player_id == "foo"

def test_get_current(self):
assert self.server.get_status()['genre']

Expand Down

0 comments on commit 9db14ef

Please sign in to comment.