From 6ced9c473f81b72e9b51a662bbbbf9be1405e7de Mon Sep 17 00:00:00 2001 From: Andrew Sayre Date: Fri, 24 May 2019 08:39:29 -0500 Subject: [PATCH 01/10] Wrap command errors and improve logging --- pyheos/__init__.py | 6 +++-- pyheos/connection.py | 46 +++++++++++++++++++++------------ pyheos/error.py | 60 ++++++++++++++++++++++++++++++++++++++++++++ pyheos/response.py | 30 +++------------------- pytest.ini | 2 ++ tests/test_heos.py | 38 ++++++++++++++++++++++------ 6 files changed, 130 insertions(+), 52 deletions(-) create mode 100644 pyheos/error.py create mode 100644 pytest.ini diff --git a/pyheos/__init__.py b/pyheos/__init__.py index 1eb01df..3a66b86 100644 --- a/pyheos/__init__.py +++ b/pyheos/__init__.py @@ -1,20 +1,22 @@ """pyheos - a library for interacting with HEOS devices.""" from . import const from .dispatch import Dispatcher +from .error import CommandError, CommandFailedError, HeosError from .group import HeosGroup from .heos import Heos from .player import HeosNowPlayingMedia, HeosPlayer -from .response import CommandError from .source import HeosSource, InputSource __all__ = [ 'const', 'CommandError', + 'CommandFailedError', 'Dispatcher', 'Heos', + 'HeosError', 'HeosGroup', 'HeosPlayer', 'HeosNowPlayingMedia', 'HeosSource', - 'InputSource' + 'InputSource', ] diff --git a/pyheos/connection.py b/pyheos/connection.py index 2690de6..986da5b 100644 --- a/pyheos/connection.py +++ b/pyheos/connection.py @@ -9,6 +9,7 @@ from . import const from .command import HeosCommands +from .error import CommandError, format_error_message from .response import HeosResponse SEPARATOR = '\r\n' @@ -23,17 +24,24 @@ '%': '%25' } +_MASKED_PARAMS = { + 'pw', +} + +_MASK = "********" + def _quote(string: str) -> str: """Quote a string per the CLI specification.""" return ''.join([_QUOTE_MAP.get(char, char) for char in str(string)]) -def _encode_query(items: dict) -> str: +def _encode_query(items: dict, *, mask=False) -> str: """Encode a dict to query string per CLI specifications.""" pairs = [] - for key, value in items.items(): - item = key + "=" + _quote(value) + for key in sorted(items.keys()): + value = _MASK if mask and key in _MASKED_PARAMS else items[key] + item = "{}={}".format(key, _quote(value)) # Ensure 'url' goes last per CLI spec if key == 'url': pairs.append(item) @@ -223,29 +231,30 @@ async def _heart_beat(self): if last_activity > threshold: try: await self.commands.heart_beat() - except (ConnectionError, asyncio.IncompleteReadError, - asyncio.TimeoutError): + except CommandError: pass await asyncio.sleep(self._heart_beat_interval / 2) async def command( self, command: str, params: Dict[str, Any] = None) -> HeosResponse: - """Run a command and get it's response.""" - if self._state != const.STATE_CONNECTED: - raise ValueError - - # append sequence number + """Execute a command and get it's response.""" + # Build command URI sequence = self._sequence self._sequence += 1 params = params or {} params['sequence'] = sequence - command_name = command - uri = const.BASE_URI + command + '?' + _encode_query(params) + uri = "{}{}?{}".format(const.BASE_URI, command, _encode_query(params)) + masked_uri = "{}{}?{}".format( + const.BASE_URI, command, _encode_query(params, mask=True)) + + if self._state != const.STATE_CONNECTED: + _LOGGER.debug("Command failed '%s': %s", masked_uri, + "Not connected to device") + raise CommandError(command, "Not connected to device") # Add reservation event = ResponseEvent(sequence) - pending_commands = self._pending_commands[command_name] - pending_commands.append(event) + self._pending_commands[command].append(event) # Send command try: self._writer.write((uri + SEPARATOR).encode()) @@ -254,9 +263,14 @@ async def command( except (ConnectionError, asyncio.TimeoutError) as error: # Occurs when the connection breaks asyncio.ensure_future(self._handle_connection_error(error)) - raise + message = format_error_message(error) + _LOGGER.debug("Command failed '%s': %s", masked_uri, message) + raise CommandError(command, message) from error - _LOGGER.debug("Executed command '%s': '%s'", command, response) + if response.result: + _LOGGER.debug("Command executed '%s': '%s'", masked_uri, response) + else: + _LOGGER.debug("Command failed '%s': %s", masked_uri, response) response.raise_for_result() return response diff --git a/pyheos/error.py b/pyheos/error.py new file mode 100644 index 0000000..32198a9 --- /dev/null +++ b/pyheos/error.py @@ -0,0 +1,60 @@ +"""Define the error module for HEOS.""" +import asyncio + +DEFAULT_ERROR_MESSAGES = { + asyncio.TimeoutError: "Command timed out", + ConnectionError: "Connection error", + BrokenPipeError: "Broken pipe", + ConnectionAbortedError: "Connection aborted", + ConnectionRefusedError: "Connection refused", + ConnectionResetError: "Connection reset" +} + + +def format_error_message(error: Exception) -> str: + """Format the error message based on a base error.""" + error_message = str(error) + if not error_message: + error_message = DEFAULT_ERROR_MESSAGES.get(type(error)) + return error_message + + +class HeosError(Exception): + """Define an error from the heos library.""" + + pass + + +class CommandError(HeosError): + """Define an error command response.""" + + def __init__(self, command: str, message: str): + """Create a new instance of the error.""" + self._command = command + super().__init__(message) + + @property + def command(self) -> str: + """Get the command that raised the error.""" + return self._command + + +class CommandFailedError(CommandError): + """Define an error when a HEOS command fails.""" + + def __init__(self, command: str, text: str, error_id: int): + """Create a new instance of the error.""" + self._command = command + self._error_text = text + self._error_id = error_id + super().__init__(command, "{} ({})".format(text, error_id)) + + @property + def error_text(self) -> str: + """Get the error text from the response.""" + return self._error_text + + @property + def error_id(self) -> int: + """Return the error id.""" + return self._error_id diff --git a/pyheos/response.py b/pyheos/response.py index b573052..0537e39 100644 --- a/pyheos/response.py +++ b/pyheos/response.py @@ -2,6 +2,8 @@ from typing import Any, Dict, Optional from urllib.parse import parse_qsl +from .error import CommandFailedError + class HeosResponse: """Define a HEOS response representation.""" @@ -87,30 +89,4 @@ def raise_for_result(self): text += " " + system_error_number error_id = self.get_message('eid') error_id = int(error_id) if error_id else error_id - raise CommandError(self._command, text, error_id) - - -class CommandError(Exception): - """Define an error command response.""" - - def __init__(self, command: str, text: str, error_id: int): - """Create a new instance of the error.""" - self._command = command - self._error_text = text - self._error_id = error_id - super().__init__("{} ({})".format(text, error_id)) - - @property - def command(self) -> str: - """Get the command that raised the error.""" - return self._command - - @property - def error_text(self) -> str: - """Get the error text from the response.""" - return self._error_text - - @property - def error_id(self) -> int: - """Return the error id.""" - return self._error_id + raise CommandFailedError(self._command, text, error_id) diff --git a/pytest.ini b/pytest.ini new file mode 100644 index 0000000..cb22dc4 --- /dev/null +++ b/pytest.ini @@ -0,0 +1,2 @@ +[pytest] +log_level=DEBUG \ No newline at end of file diff --git a/tests/test_heos.py b/tests/test_heos.py index 26b1b19..8f272fe 100644 --- a/tests/test_heos.py +++ b/tests/test_heos.py @@ -5,8 +5,8 @@ from pyheos import const from pyheos.dispatch import Dispatcher +from pyheos.error import CommandError, CommandFailedError from pyheos.heos import Heos -from pyheos.response import CommandError from . import connect_handler, get_fixture @@ -97,6 +97,21 @@ async def test_disconnect(mock_device, heos): assert heos.connection_state == const.STATE_DISCONNECTED +@pytest.mark.asyncio +async def test_commands_fail_when_disconnected(mock_device, heos, caplog): + """Test calling commands fail when disconnected.""" + # Fixture automatically connects + await heos.disconnect() + assert heos.connection_state == const.STATE_DISCONNECTED + + with pytest.raises(CommandError) as e_info: + await heos.load_players() + assert str(e_info.value) == "Not connected to device" + assert e_info.value.command == const.COMMAND_GET_PLAYERS + assert "Command failed 'heos://player/get_players?sequence=0': " \ + "Not connected to device" in caplog.text + + @pytest.mark.asyncio async def test_connection_error_during_event(mock_device, heos): """Test connection error during event results in disconnected.""" @@ -117,8 +132,10 @@ async def test_connection_error_during_command(mock_device, heos): # Assert transitions to reconnecting and fires disconnect await mock_device.stop() - with pytest.raises(asyncio.TimeoutError): + with pytest.raises(CommandError) as e_info: await heos.get_players() + assert str(e_info.value) == "Command timed out" + assert isinstance(e_info.value.__cause__, asyncio.TimeoutError) await disconnect_signal.wait() assert heos.connection_state == const.STATE_DISCONNECTED @@ -173,8 +190,10 @@ async def test_reconnect_during_command(mock_device): # Act await mock_device.stop() await mock_device.start() - with pytest.raises(asyncio.TimeoutError): + with pytest.raises(CommandError) as e_info: await heos.get_players() + assert str(e_info.value) == "Command timed out" + assert isinstance(e_info.value.__cause__, asyncio.TimeoutError) # Assert signals set await disconnect_signal.wait() @@ -239,7 +258,7 @@ async def test_get_players_error(mock_device, heos): mock_device.register(const.COMMAND_GET_PLAYERS, None, "player.get_players_error", replace=True) - with pytest.raises(CommandError) as e_info: + with pytest.raises(CommandFailedError) as e_info: await heos.get_players() assert str(e_info.value) == "System error -519 (12)" assert e_info.value.command == const.COMMAND_GET_PLAYERS @@ -770,19 +789,24 @@ async def test_get_playlists(mock_device, heos): @pytest.mark.asyncio -async def test_sign_in_and_out(mock_device, heos): - """Test the sign in and sign out methods.""" +async def test_sign_in_and_out(mock_device, heos, caplog): + """Test the sign in and sign out methods and ensure log is masked.""" data = {'un': "example@example.com", 'pw': 'example'} + # Test sign-in failure mock_device.register(const.COMMAND_SIGN_IN, data, 'system.sign_in_failure') - with pytest.raises(CommandError) as e_info: + with pytest.raises(CommandFailedError) as e_info: await heos.sign_in("example@example.com", "example") assert str(e_info.value.error_text) == "User not found" + assert "Command failed 'heos://system/sign_in" \ + "?un=example@example.com&sequence=2&pw=********':" in caplog.text # Test sign-in success mock_device.register(const.COMMAND_SIGN_IN, data, 'system.sign_in', replace=True) await heos.sign_in("example@example.com", "example") + assert "Command executed 'heos://system/sign_in" \ + "?un=example@example.com&sequence=3&pw=********':" in caplog.text # Test sign-out mock_device.register(const.COMMAND_SIGN_OUT, None, 'system.sign_out') From d84a7b1719f6dd78bb2cdffd973f828e23400e01 Mon Sep 17 00:00:00 2001 From: Andrew Sayre Date: Sun, 26 May 2019 08:59:53 -0500 Subject: [PATCH 02/10] Update inputs for CLI 1.14 --- pyheos/const.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pyheos/const.py b/pyheos/const.py index 69c9a6a..4debe56 100644 --- a/pyheos/const.py +++ b/pyheos/const.py @@ -160,7 +160,9 @@ INPUT_TV_AUDIO = "inputs/tvaudio" INPUT_PHONO = "inputs/phono" INPUT_USB_AC = "inputs/usbdac" -INPUT_ANALOG = "inputs/analog" +INPUT_ANALOG_IN_1 = "inputs/analog_in_1" +INPUT_ANALOG_IN_2 = "inputs/analog_in_2" +INPUT_RECORDER_IN_1 = "inputs/recorder_in_1" VALID_INPUTS = ( INPUT_AUX_IN_1, INPUT_AUX_IN_2, INPUT_AUX_IN_3, INPUT_AUX_IN_4, @@ -171,7 +173,7 @@ INPUT_HDMI_IN_3, INPUT_HDMI_IN_4, INPUT_HDMI_ARC_1, INPUT_CABLE_SAT, INPUT_DVD, INPUT_BLURAY, INPUT_GAME, INPUT_MEDIA_PLAYER, INPUT_CD, INPUT_TUNER, INPUT_HD_RADIO, INPUT_TV_AUDIO, INPUT_PHONO, INPUT_USB_AC, - INPUT_ANALOG) + INPUT_ANALOG_IN_1, INPUT_ANALOG_IN_2, INPUT_RECORDER_IN_1) # Add to Queue Options ADD_QUEUE_PLAY_NOW = 1 From eafc96e4827aa40a7101b93fd1a8f724b56d63a8 Mon Sep 17 00:00:00 2001 From: Andrew Sayre Date: Sun, 26 May 2019 09:01:02 -0500 Subject: [PATCH 03/10] Update readme for 1.14 --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index d72b439..aa92709 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ [![image](https://img.shields.io/pypi/l/pyheos.svg)](https://pypi.org/project/pyheos/) [![image](https://img.shields.io/badge/Reviewed_by-Hound-8E64B0.svg)](https://houndci.com) -An async python library for controlling HEOS devices through the HEOS CLI Protocol (version 1.13 for players with firmware 1.481.130 or newer). +An async python library for controlling HEOS devices through the HEOS CLI Protocol (version 1.14 for players with firmware 1.505.140 or newer). ## Installation ```bash From 62c65736da003b58ee33085d5655d7af9befc342 Mon Sep 17 00:00:00 2001 From: Andrew Sayre Date: Sat, 24 Aug 2019 08:48:00 -0500 Subject: [PATCH 04/10] Fix missing source_id from playlists --- pyheos/heos.py | 7 +++++-- tests/test_heos.py | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/pyheos/heos.py b/pyheos/heos.py index 900a66d..152c79e 100644 --- a/pyheos/heos.py +++ b/pyheos/heos.py @@ -191,8 +191,11 @@ async def get_playlists(self) -> Sequence[HeosSource]: """Get available playlists.""" payload = await self._connection.commands.browse( const.MUSIC_SOURCE_PLAYLISTS) - return [HeosSource(self._connection.commands, item) - for item in payload] + playlists = [] + for item in payload: + item['sid'] = const.MUSIC_SOURCE_PLAYLISTS + playlists.append(HeosSource(self._connection.commands, item)) + return playlists @property def dispatcher(self) -> Dispatcher: diff --git a/tests/test_heos.py b/tests/test_heos.py index 8f272fe..cc0d3aa 100644 --- a/tests/test_heos.py +++ b/tests/test_heos.py @@ -786,6 +786,7 @@ async def test_get_playlists(mock_device, heos): assert playlist.container_id == "171566" assert playlist.name == "Rockin Songs" assert playlist.type == const.TYPE_PLAYLIST + assert playlist.source_id == const.MUSIC_SOURCE_PLAYLISTS @pytest.mark.asyncio From 9c5d60a21263dd3f68b304798e6c6c07013032fb Mon Sep 17 00:00:00 2001 From: Andrew Sayre Date: Sat, 24 Aug 2019 08:48:29 -0500 Subject: [PATCH 05/10] Bump version 0.5.3 --- pyheos/const.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyheos/const.py b/pyheos/const.py index 4debe56..342c8df 100644 --- a/pyheos/const.py +++ b/pyheos/const.py @@ -1,7 +1,7 @@ """Define consts for the pyheos package.""" __title__ = "pyheos" -__version__ = "0.5.2" +__version__ = "0.5.3" CLI_PORT = 1255 DEFAULT_TIMEOUT = 10.0 From 4c670ec461969d8dd4267b2cf61b4e2f0691b4e6 Mon Sep 17 00:00:00 2001 From: Andrew Sayre Date: Sat, 24 Aug 2019 09:07:43 -0500 Subject: [PATCH 06/10] Handle OSError --- pyheos/connection.py | 6 +++--- pyheos/error.py | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/pyheos/connection.py b/pyheos/connection.py index 986da5b..b159905 100644 --- a/pyheos/connection.py +++ b/pyheos/connection.py @@ -172,7 +172,7 @@ async def _reconnect(self): await self._connect() self._reconnect_task = None return - except (ConnectionError, asyncio.TimeoutError) as err: + except (ConnectionError, asyncio.TimeoutError, OSError) as err: # Occurs when we could not reconnect _LOGGER.debug("Failed to reconnect to %s: %s", self.host, err) await self._disconnect() @@ -219,7 +219,7 @@ async def _response_handler(self): # Occurs when the task is being killed return except (ConnectionError, asyncio.IncompleteReadError, - RuntimeError) as error: + RuntimeError, OSError) as error: # Occurs when the connection breaks asyncio.ensure_future(self._handle_connection_error(error)) return @@ -260,7 +260,7 @@ async def command( self._writer.write((uri + SEPARATOR).encode()) await self._writer.drain() response = await asyncio.wait_for(event.wait(), self.timeout) - except (ConnectionError, asyncio.TimeoutError) as error: + except (ConnectionError, asyncio.TimeoutError, OSError) as error: # Occurs when the connection breaks asyncio.ensure_future(self._handle_connection_error(error)) message = format_error_message(error) diff --git a/pyheos/error.py b/pyheos/error.py index 32198a9..6005c3a 100644 --- a/pyheos/error.py +++ b/pyheos/error.py @@ -7,7 +7,8 @@ BrokenPipeError: "Broken pipe", ConnectionAbortedError: "Connection aborted", ConnectionRefusedError: "Connection refused", - ConnectionResetError: "Connection reset" + ConnectionResetError: "Connection reset", + OSError: "OS I/O error" } From 5847483cb6c8aaa05b6ccae04ae7bdb82469e330 Mon Sep 17 00:00:00 2001 From: Andrew Sayre Date: Sat, 24 Aug 2019 20:10:51 -0500 Subject: [PATCH 07/10] Wrap errors in connection. --- pyheos/connection.py | 15 +++++++++------ tests/test_heos.py | 14 +++++++++----- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/pyheos/connection.py b/pyheos/connection.py index b159905..bac6271 100644 --- a/pyheos/connection.py +++ b/pyheos/connection.py @@ -9,7 +9,7 @@ from . import const from .command import HeosCommands -from .error import CommandError, format_error_message +from .error import CommandError, HeosError, format_error_message from .response import HeosResponse SEPARATOR = '\r\n' @@ -90,10 +90,13 @@ async def connect(self, *, auto_reconnect: bool = False, async def _connect(self): """Perform core connection logic.""" - open_future = asyncio.open_connection( - self.host, const.CLI_PORT) - self._reader, self._writer = await asyncio.wait_for( - open_future, self.timeout) + try: + open_future = asyncio.open_connection( + self.host, const.CLI_PORT) + self._reader, self._writer = await asyncio.wait_for( + open_future, self.timeout) + except (OSError, ConnectionError, asyncio.TimeoutError) as err: + raise HeosError(format_error_message(err)) from err # Start response handler self._response_handler_task = asyncio.ensure_future( self._response_handler()) @@ -172,7 +175,7 @@ async def _reconnect(self): await self._connect() self._reconnect_task = None return - except (ConnectionError, asyncio.TimeoutError, OSError) as err: + except HeosError as err: # Occurs when we could not reconnect _LOGGER.debug("Failed to reconnect to %s: %s", self.host, err) await self._disconnect() diff --git a/tests/test_heos.py b/tests/test_heos.py index cc0d3aa..3aa089b 100644 --- a/tests/test_heos.py +++ b/tests/test_heos.py @@ -5,7 +5,7 @@ from pyheos import const from pyheos.dispatch import Dispatcher -from pyheos.error import CommandError, CommandFailedError +from pyheos.error import CommandError, CommandFailedError, HeosError from pyheos.heos import Heos from . import connect_handler, get_fixture @@ -66,11 +66,13 @@ async def test_heart_beat(mock_device): async def test_connect_fails(): """Test connect fails when host not available.""" heos = Heos('127.0.0.1') - with pytest.raises(ConnectionRefusedError): + with pytest.raises(HeosError) as e_info: await heos.connect() + assert isinstance(e_info.value.__cause__, ConnectionRefusedError) # Also fails for initial connection even with reconnect. - with pytest.raises(ConnectionRefusedError): + with pytest.raises(HeosError) as e_info: await heos.connect(auto_reconnect=True) + assert isinstance(e_info.value.__cause__, ConnectionRefusedError) await heos.disconnect() @@ -78,11 +80,13 @@ async def test_connect_fails(): async def test_connect_timeout(): """Test connect fails when host not available.""" heos = Heos('www.google.com', timeout=1) - with pytest.raises(asyncio.TimeoutError): + with pytest.raises(HeosError) as e_info: await heos.connect() + assert isinstance(e_info.value.__cause__, asyncio.TimeoutError) # Also fails for initial connection even with reconnect. - with pytest.raises(asyncio.TimeoutError): + with pytest.raises(HeosError) as e_info: await heos.connect(auto_reconnect=True) + assert isinstance(e_info.value.__cause__, asyncio.TimeoutError) await heos.disconnect() From 3ab0db33a3c0854ecfbef68c16149e22cca3e133 Mon Sep 17 00:00:00 2001 From: Andrew Sayre Date: Sat, 24 Aug 2019 20:13:47 -0500 Subject: [PATCH 08/10] Update test requirements --- test-requirements.txt | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test-requirements.txt b/test-requirements.txt index 410b3bc..d1fe5f7 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,9 +1,9 @@ -coveralls==1.7.0 -flake8==3.7.7 -flake8-docstrings==1.3.0 -pydocstyle==3.0.0 +coveralls==1.8.2 +flake8==3.7.8 +flake8-docstrings==1.3.1 +pydocstyle==4.0.1 pylint==2.3.1 -pytest==4.3.1 +pytest==5.1.1 pytest-asyncio==0.10.0 -pytest-cov==2.6.1 +pytest-cov==2.7.1 pytest-timeout==1.3.3 \ No newline at end of file From ca9cd5348490870e9aa5a7dcb78a877e8c595fca Mon Sep 17 00:00:00 2001 From: Andrew Sayre Date: Sat, 24 Aug 2019 20:14:11 -0500 Subject: [PATCH 09/10] Bump v0.6.0 --- pyheos/const.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyheos/const.py b/pyheos/const.py index 342c8df..b50defa 100644 --- a/pyheos/const.py +++ b/pyheos/const.py @@ -1,7 +1,7 @@ """Define consts for the pyheos package.""" __title__ = "pyheos" -__version__ = "0.5.3" +__version__ = "0.6.0" CLI_PORT = 1255 DEFAULT_TIMEOUT = 10.0 From a50ed004d95b33fe0966bfaec78478d07ec42ce0 Mon Sep 17 00:00:00 2001 From: Andrew Sayre Date: Sat, 24 Aug 2019 20:19:51 -0500 Subject: [PATCH 10/10] Update tox config --- tox.ini | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tox.ini b/tox.ini index d056de8..8ebcf6e 100644 --- a/tox.ini +++ b/tox.ini @@ -9,7 +9,7 @@ setenv = deps = -r{toxinidir}/test-requirements.txt commands = - pytest tests --timeout=30 --duration=10 {posargs} + pytest tests --timeout=30 {posargs} [testenv:lint] setenv = @@ -29,4 +29,4 @@ setenv = deps = -r{toxinidir}/test-requirements.txt commands = - pytest tests --timeout=30 --duration=10 --cov {posargs} \ No newline at end of file + pytest tests --timeout=30 --cov {posargs} \ No newline at end of file