diff --git a/.coveragerc b/.coveragerc index 6978958a3f0..0b72c81a5dd 100644 --- a/.coveragerc +++ b/.coveragerc @@ -636,6 +636,7 @@ omit = homeassistant/components/picotts/tts.py homeassistant/components/piglow/light.py homeassistant/components/pilight/* + homeassistant/components/ping/const.py homeassistant/components/ping/binary_sensor.py homeassistant/components/ping/device_tracker.py homeassistant/components/pioneer/media_player.py diff --git a/homeassistant/components/ping/binary_sensor.py b/homeassistant/components/ping/binary_sensor.py index a9c69f4ddad..6db9d43eeda 100644 --- a/homeassistant/components/ping/binary_sensor.py +++ b/homeassistant/components/ping/binary_sensor.py @@ -10,9 +10,13 @@ import voluptuous as vol from homeassistant.components.binary_sensor import PLATFORM_SCHEMA, BinarySensorEntity from homeassistant.const import CONF_HOST, CONF_NAME import homeassistant.helpers.config_validation as cv +from homeassistant.util.process import kill_subprocess + +from .const import PING_TIMEOUT _LOGGER = logging.getLogger(__name__) + ATTR_ROUND_TRIP_TIME_AVG = "round_trip_time_avg" ATTR_ROUND_TRIP_TIME_MAX = "round_trip_time_max" ATTR_ROUND_TRIP_TIME_MDEV = "round_trip_time_mdev" @@ -20,12 +24,14 @@ ATTR_ROUND_TRIP_TIME_MIN = "round_trip_time_min" CONF_PING_COUNT = "count" -DEFAULT_NAME = "Ping Binary sensor" +DEFAULT_NAME = "Ping" DEFAULT_PING_COUNT = 5 DEFAULT_DEVICE_CLASS = "connectivity" SCAN_INTERVAL = timedelta(minutes=5) +PARALLEL_UPDATES = 0 + PING_MATCHER = re.compile( r"(?P\d+.\d+)\/(?P\d+.\d+)\/(?P\d+.\d+)\/(?P\d+.\d+)" ) @@ -39,17 +45,19 @@ WIN32_PING_MATCHER = re.compile(r"(?P\d+)ms.+(?P\d+)ms.+(?P\d+)ms PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend( { vol.Required(CONF_HOST): cv.string, - vol.Optional(CONF_NAME, default=DEFAULT_NAME): cv.string, - vol.Optional(CONF_PING_COUNT, default=DEFAULT_PING_COUNT): cv.positive_int, + vol.Optional(CONF_NAME): cv.string, + vol.Optional(CONF_PING_COUNT, default=DEFAULT_PING_COUNT): vol.Range( + min=1, max=100 + ), } ) def setup_platform(hass, config, add_entities, discovery_info=None): """Set up the Ping Binary sensor.""" - name = config.get(CONF_NAME) - host = config.get(CONF_HOST) - count = config.get(CONF_PING_COUNT) + host = config[CONF_HOST] + count = config[CONF_PING_COUNT] + name = config.get(CONF_NAME, f"{DEFAULT_NAME} {host}") add_entities([PingBinarySensor(name, PingData(host, count))], True) @@ -129,7 +137,7 @@ class PingData: self._ping_cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE ) try: - out = pinger.communicate() + out = pinger.communicate(timeout=self._count + PING_TIMEOUT) _LOGGER.debug("Output is %s", str(out)) if sys.platform == "win32": match = WIN32_PING_MATCHER.search(str(out).split("\n")[-1]) @@ -142,6 +150,9 @@ class PingData: match = PING_MATCHER.search(str(out).split("\n")[-1]) rtt_min, rtt_avg, rtt_max, rtt_mdev = match.groups() return {"min": rtt_min, "avg": rtt_avg, "max": rtt_max, "mdev": rtt_mdev} + except subprocess.TimeoutExpired: + kill_subprocess(pinger) + return False except (subprocess.CalledProcessError, AttributeError): return False diff --git a/homeassistant/components/ping/const.py b/homeassistant/components/ping/const.py new file mode 100644 index 00000000000..8be8c1bdaa3 --- /dev/null +++ b/homeassistant/components/ping/const.py @@ -0,0 +1,3 @@ +"""Tracks devices by sending a ICMP echo request (ping).""" + +PING_TIMEOUT = 3 diff --git a/homeassistant/components/ping/device_tracker.py b/homeassistant/components/ping/device_tracker.py index c0effda7a55..f4e2e806143 100644 --- a/homeassistant/components/ping/device_tracker.py +++ b/homeassistant/components/ping/device_tracker.py @@ -14,9 +14,13 @@ from homeassistant.components.device_tracker.const import ( SOURCE_TYPE_ROUTER, ) import homeassistant.helpers.config_validation as cv +from homeassistant.util.process import kill_subprocess + +from .const import PING_TIMEOUT _LOGGER = logging.getLogger(__name__) +PARALLEL_UPDATES = 0 CONF_PING_COUNT = "count" PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend( @@ -47,8 +51,12 @@ class Host: self._ping_cmd, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL ) try: - pinger.communicate() + pinger.communicate(timeout=1 + PING_TIMEOUT) return pinger.returncode == 0 + except subprocess.TimeoutExpired: + kill_subprocess(pinger) + return False + except subprocess.CalledProcessError: return False diff --git a/homeassistant/util/process.py b/homeassistant/util/process.py new file mode 100644 index 00000000000..fb2d6dec58e --- /dev/null +++ b/homeassistant/util/process.py @@ -0,0 +1,12 @@ +"""Util to handle processes.""" + +import subprocess + + +def kill_subprocess(process: subprocess.Popen) -> None: + """Force kill a subprocess and wait for it to exit.""" + process.kill() + process.communicate() + process.wait() + + del process diff --git a/tests/util/test_process.py b/tests/util/test_process.py new file mode 100644 index 00000000000..a82df0dbb99 --- /dev/null +++ b/tests/util/test_process.py @@ -0,0 +1,26 @@ +"""Test process util.""" + +import os +import subprocess + +import pytest + +from homeassistant.util import process + + +async def test_kill_process(): + """Test killing a process.""" + sleeper = subprocess.Popen( + "sleep 1000", + shell=True, # nosec # shell by design + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) + pid = sleeper.pid + + assert os.kill(pid, 0) is None + + process.kill_subprocess(sleeper) + + with pytest.raises(OSError): + os.kill(pid, 0)