Make ping binary_sensor update async (#35301)

* async_update

* isort and pylint

* @shenxn suggestions

Co-authored-by: Xiaonan Shen <s@sxn.dev>

* 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 <nick@koston.org>

* @bdraco review changes

* Update homeassistant/components/ping/binary_sensor.py

Co-authored-by: J. Nick Koston <nick@koston.org>

Co-authored-by: Xiaonan Shen <s@sxn.dev>
Co-authored-by: J. Nick Koston <nick@koston.org>

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_AVG: self._ping.data["avg"],
-                ATTR_ROUND_TRIP_TIME_MAX: self.ping.data["max"],
+                ATTR_ROUND_TRIP_TIME_MAX: self._ping.data["max"],
-                ATTR_ROUND_TRIP_TIME_MDEV: self.ping.data["mdev"],
+                ATTR_ROUND_TRIP_TIME_MDEV: self._ping.data["mdev"],
-                ATTR_ROUND_TRIP_TIME_MIN: self.ping.data["min"],
+                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(
+        pinger = await asyncio.create_subprocess_exec(
-            self._ping_cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE
+            *self._ping_cmd,
+            stdin=None,
+            stdout=asyncio.subprocess.PIPE,
+            stderr=asyncio.subprocess.PIPE,
         )
         try:
-            out = pinger.communicate(timeout=self._count + PING_TIMEOUT)
+            out_data, out_error = await asyncio.wait_for(
-            _LOGGER.debug("Output is %s", str(out))
+                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):
+            if "max/" not in str(out_data):
-                match = PING_MATCHER_BUSYBOX.search(str(out).split("\n")[-1])
+                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:
+        except asyncio.TimeoutError:
-            kill_subprocess(pinger)
+            _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)