From 57777ef79a97ddb7877e03dc59a03b3958352d4c Mon Sep 17 00:00:00 2001 From: Eric Hagan Date: Sat, 22 Oct 2016 04:05:00 -0500 Subject: [PATCH] Adds support for Pioneer AVR interface port number (#3878) * Adds support for Pioneer AVR interface port number https://community.home-assistant.io/t/support-for-pioneer-avr/503 telnetlib supports a port number so adding port as an optional config element with a default of 23 resolves this. * Adds timeout to Pioneer AVR timeout in telnetlib defaults to socket._GLOBAL_DEFAULT_TIMEOUT which is not a value, but rather a bare Object used for comparison. telnetlib says the following about the timeout optional argument: "The optional timeout parameter specifies a timeout in seconds for blocking operations like the connection attempt (if not specified, the global default timeout setting will be used)." From the documentation for sockets: "Sockets are by default always created in blocking mode" Catching connect and timeout errors, logging to debug and continuing. * Catches timeout exceptions, logs and continues. --- .../components/media_player/pioneer.py | 47 +++++++++++++++---- homeassistant/helpers/config_validation.py | 19 ++++++++ tests/helpers/test_config_validation.py | 20 ++++++++ 3 files changed, 76 insertions(+), 10 deletions(-) diff --git a/homeassistant/components/media_player/pioneer.py b/homeassistant/components/media_player/pioneer.py index 599edf08b37..8930057857d 100644 --- a/homeassistant/components/media_player/pioneer.py +++ b/homeassistant/components/media_player/pioneer.py @@ -13,12 +13,15 @@ from homeassistant.components.media_player import ( SUPPORT_PAUSE, SUPPORT_SELECT_SOURCE, MediaPlayerDevice, PLATFORM_SCHEMA, SUPPORT_TURN_OFF, SUPPORT_TURN_ON, SUPPORT_VOLUME_MUTE, SUPPORT_VOLUME_SET) from homeassistant.const import ( - CONF_HOST, STATE_OFF, STATE_ON, STATE_UNKNOWN, CONF_NAME) + CONF_HOST, STATE_OFF, STATE_ON, STATE_UNKNOWN, CONF_NAME, CONF_PORT, + CONF_TIMEOUT) import homeassistant.helpers.config_validation as cv _LOGGER = logging.getLogger(__name__) DEFAULT_NAME = 'Pioneer AVR' +DEFAULT_PORT = 23 # telnet default. Some Pioneer AVRs use 8102 +DEFAULT_TIMEOUT = None SUPPORT_PIONEER = SUPPORT_PAUSE | SUPPORT_VOLUME_SET | SUPPORT_VOLUME_MUTE | \ SUPPORT_TURN_ON | SUPPORT_TURN_OFF | SUPPORT_SELECT_SOURCE @@ -29,12 +32,17 @@ MAX_SOURCE_NUMBERS = 60 PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({ vol.Required(CONF_HOST): cv.string, vol.Optional(CONF_NAME, default=DEFAULT_NAME): cv.string, + vol.Optional(CONF_PORT, default=DEFAULT_PORT): cv.port, + vol.Optional(CONF_TIMEOUT, default=DEFAULT_TIMEOUT): cv.socket_timeout, }) def setup_platform(hass, config, add_devices, discovery_info=None): """Setup the Pioneer platform.""" - pioneer = PioneerDevice(config.get(CONF_NAME), config.get(CONF_HOST)) + pioneer = PioneerDevice(config.get(CONF_NAME), + config.get(CONF_HOST), + config.get(CONF_PORT), + config.get(CONF_TIMEOUT)) if pioneer.update(): add_devices([pioneer]) @@ -48,10 +56,12 @@ class PioneerDevice(MediaPlayerDevice): # pylint: disable=too-many-public-methods, abstract-method # pylint: disable=too-many-instance-attributes - def __init__(self, name, host): + def __init__(self, name, host, port, timeout): """Initialize the Pioneer device.""" self._name = name self._host = host + self._port = port + self._timeout = timeout self._pwstate = 'PWR1' self._volume = 0 self._muted = False @@ -62,7 +72,11 @@ class PioneerDevice(MediaPlayerDevice): @classmethod def telnet_request(cls, telnet, command, expected_prefix): """Execute `command` and return the response.""" - telnet.write(command.encode("ASCII") + b"\r") + try: + telnet.write(command.encode("ASCII") + b"\r") + except telnetlib.socket.timeout: + _LOGGER.debug("Pioneer command %s timed out", command) + return None # The receiver will randomly send state change updates, make sure # we get the response we are looking for @@ -76,19 +90,32 @@ class PioneerDevice(MediaPlayerDevice): def telnet_command(self, command): """Establish a telnet connection and sends `command`.""" - telnet = telnetlib.Telnet(self._host) - telnet.write(command.encode("ASCII") + b"\r") - telnet.read_very_eager() # skip response - telnet.close() + try: + try: + telnet = telnetlib.Telnet(self._host, + self._port, + self._timeout) + except ConnectionRefusedError: + _LOGGER.debug("Pioneer %s refused connection", self._name) + return + telnet.write(command.encode("ASCII") + b"\r") + telnet.read_very_eager() # skip response + telnet.close() + except telnetlib.socket.timeout: + _LOGGER.debug( + "Pioneer %s command %s timed out", self._name, command) def update(self): """Get the latest details from the device.""" try: - telnet = telnetlib.Telnet(self._host) + telnet = telnetlib.Telnet(self._host, self._port, self._timeout) except ConnectionRefusedError: + _LOGGER.debug("Pioneer %s refused connection", self._name) return False - self._pwstate = self.telnet_request(telnet, "?P", "PWR") + pwstate = self.telnet_request(telnet, "?P", "PWR") + if pwstate: + self._pwstate = pwstate volume_str = self.telnet_request(telnet, "?V", "VOL") self._volume = int(volume_str[3:]) / MAX_VOLUME if volume_str else None diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index dcbbbb4b3db..4c6efe11001 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -3,6 +3,7 @@ from collections import OrderedDict from datetime import timedelta import os from urllib.parse import urlparse +from socket import _GLOBAL_DEFAULT_TIMEOUT from typing import Any, Union, TypeVar, Callable, Sequence, Dict @@ -306,6 +307,24 @@ def time_zone(value): weekdays = vol.All(ensure_list, [vol.In(WEEKDAYS)]) +def socket_timeout(value): + """Validate timeout float > 0.0. + + None coerced to socket._GLOBAL_DEFAULT_TIMEOUT bare object. + """ + if value is None: + return _GLOBAL_DEFAULT_TIMEOUT + else: + try: + float_value = float(value) + if float_value > 0.0: + return float_value + raise vol.Invalid('Invalid socket timeout value.' + ' float > 0.0 required.') + except Exception as _: + raise vol.Invalid('Invalid socket timeout: {err}'.format(err=_)) + + # pylint: disable=no-value-for-parameter def url(value: Any) -> str: """Validate an URL.""" diff --git a/tests/helpers/test_config_validation.py b/tests/helpers/test_config_validation.py index 287219aa669..3ff9755bba2 100644 --- a/tests/helpers/test_config_validation.py +++ b/tests/helpers/test_config_validation.py @@ -3,6 +3,7 @@ from collections import OrderedDict from datetime import timedelta import enum import os +from socket import _GLOBAL_DEFAULT_TIMEOUT import pytest import voluptuous as vol @@ -436,3 +437,22 @@ def test_enum(): schema('value3') TestEnum['value1'] + + +def test_socket_timeout(): + """Test socket timeout validator.""" + TEST_CONF_TIMEOUT = 'timeout' + + schema = vol.Schema( + {vol.Required(TEST_CONF_TIMEOUT, default=None): cv.socket_timeout}) + + with pytest.raises(vol.Invalid): + schema({TEST_CONF_TIMEOUT: 0.0}) + + with pytest.raises(vol.Invalid): + schema({TEST_CONF_TIMEOUT: -1}) + + assert _GLOBAL_DEFAULT_TIMEOUT == schema({TEST_CONF_TIMEOUT: + None})[TEST_CONF_TIMEOUT] + + assert 1.0 == schema({TEST_CONF_TIMEOUT: 1})[TEST_CONF_TIMEOUT]