From 81b4c6956b30c492ab9b2748207aea488823f825 Mon Sep 17 00:00:00 2001 From: Tomasz Date: Wed, 19 Aug 2020 00:25:50 +0200 Subject: [PATCH] Make ping binary_sensor update async (#35301) * async_update * isort and pylint * @shenxn suggestions Co-authored-by: Xiaonan Shen * async_update * store the command from the beginning * command as string * f-string instead of str.format * create_subprocess_shell > create_subprocess_exec more logs * isort * types * use asyncio.wait_for * Update homeassistant/components/ping/binary_sensor.py Co-authored-by: J. Nick Koston * @bdraco review changes * Update homeassistant/components/ping/binary_sensor.py Co-authored-by: J. Nick Koston Co-authored-by: Xiaonan Shen Co-authored-by: J. Nick Koston --- .../components/ping/binary_sensor.py | 102 ++++++++++++------ 1 file changed, 70 insertions(+), 32 deletions(-) diff --git a/homeassistant/components/ping/binary_sensor.py b/homeassistant/components/ping/binary_sensor.py index 6db9d43eeda..0c568bb7892 100644 --- a/homeassistant/components/ping/binary_sensor.py +++ b/homeassistant/components/ping/binary_sensor.py @@ -1,16 +1,16 @@ """Tracks the latency of a host by sending ICMP echo requests (ping).""" +import asyncio from datetime import timedelta import logging import re -import subprocess import sys +from typing import Any, Dict 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 @@ -53,7 +53,7 @@ PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend( ) -def setup_platform(hass, config, add_entities, discovery_info=None): +def setup_platform(hass, config, add_entities, discovery_info=None) -> None: """Set up the Ping Binary sensor.""" host = config[CONF_HOST] count = config[CONF_PING_COUNT] @@ -65,46 +65,46 @@ def setup_platform(hass, config, add_entities, discovery_info=None): class PingBinarySensor(BinarySensorEntity): """Representation of a Ping Binary sensor.""" - def __init__(self, name, ping): + def __init__(self, name: str, ping) -> None: """Initialize the Ping Binary sensor.""" self._name = name - self.ping = ping + self._ping = ping @property - def name(self): + def name(self) -> str: """Return the name of the device.""" return self._name @property - def device_class(self): + def device_class(self) -> str: """Return the class of this sensor.""" return DEFAULT_DEVICE_CLASS @property - def is_on(self): + def is_on(self) -> bool: """Return true if the binary sensor is on.""" - return self.ping.available + return self._ping.available @property - def device_state_attributes(self): + def device_state_attributes(self) -> Dict[str, Any]: """Return the state attributes of the ICMP checo request.""" - if self.ping.data is not False: + if self._ping.data is not False: return { - ATTR_ROUND_TRIP_TIME_AVG: self.ping.data["avg"], - ATTR_ROUND_TRIP_TIME_MAX: self.ping.data["max"], - ATTR_ROUND_TRIP_TIME_MDEV: self.ping.data["mdev"], - ATTR_ROUND_TRIP_TIME_MIN: self.ping.data["min"], + ATTR_ROUND_TRIP_TIME_AVG: self._ping.data["avg"], + ATTR_ROUND_TRIP_TIME_MAX: self._ping.data["max"], + ATTR_ROUND_TRIP_TIME_MDEV: self._ping.data["mdev"], + ATTR_ROUND_TRIP_TIME_MIN: self._ping.data["min"], } - def update(self): + async def async_update(self) -> None: """Get the latest data.""" - self.ping.update() + await self._ping.async_update() class PingData: """The Class for handling the data retrieval.""" - def __init__(self, host, count): + def __init__(self, host, count) -> None: """Initialize the data object.""" self._ip_address = host self._count = count @@ -131,32 +131,70 @@ class PingData: self._ip_address, ] - def ping(self): + async def async_ping(self): """Send ICMP echo request and return details if success.""" - pinger = subprocess.Popen( - self._ping_cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE + pinger = await asyncio.create_subprocess_exec( + *self._ping_cmd, + stdin=None, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, ) try: - out = pinger.communicate(timeout=self._count + PING_TIMEOUT) - _LOGGER.debug("Output is %s", str(out)) + out_data, out_error = await asyncio.wait_for( + pinger.communicate(), self._count + PING_TIMEOUT + ) + + if out_data: + _LOGGER.debug( + "Output of command: `%s`, return code: %s:\n%s", + " ".join(self._ping_cmd), + pinger.returncode, + out_data, + ) + if out_error: + _LOGGER.debug( + "Error of command: `%s`, return code: %s:\n%s", + " ".join(self._ping_cmd), + pinger.returncode, + out_error, + ) + + if pinger.returncode != 0: + _LOGGER.exception( + "Error running command: `%s`, return code: %s", + " ".join(self._ping_cmd), + pinger.returncode, + ) + if sys.platform == "win32": - match = WIN32_PING_MATCHER.search(str(out).split("\n")[-1]) + match = WIN32_PING_MATCHER.search(str(out_data).split("\n")[-1]) rtt_min, rtt_avg, rtt_max = match.groups() return {"min": rtt_min, "avg": rtt_avg, "max": rtt_max, "mdev": ""} - if "max/" not in str(out): - match = PING_MATCHER_BUSYBOX.search(str(out).split("\n")[-1]) + if "max/" not in str(out_data): + match = PING_MATCHER_BUSYBOX.search(str(out_data).split("\n")[-1]) rtt_min, rtt_avg, rtt_max = match.groups() return {"min": rtt_min, "avg": rtt_avg, "max": rtt_max, "mdev": ""} - match = PING_MATCHER.search(str(out).split("\n")[-1]) + match = PING_MATCHER.search(str(out_data).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) + except asyncio.TimeoutError: + _LOGGER.exception( + "Timed out running command: `%s`, after: %ss", + self._ping_cmd, + self._count + PING_TIMEOUT, + ) + if pinger: + try: + await pinger.kill() + except TypeError: + pass + del pinger + return False - except (subprocess.CalledProcessError, AttributeError): + except AttributeError: return False - def update(self): + async def async_update(self) -> None: """Retrieve the latest details from the host.""" - self.data = self.ping() + self.data = await self.async_ping() self.available = bool(self.data)