Prevent ping integration from delaying startup (#43869)

.coveragerc

@@ -741,6 +741,7 @@ omit =
     homeassistant/components/picotts/tts.py
     homeassistant/components/piglow/light.py
     homeassistant/components/pilight/*
+    homeassistant/components/ping/__init__.py
     homeassistant/components/ping/const.py
     homeassistant/components/ping/binary_sensor.py
     homeassistant/components/ping/device_tracker.py

homeassistant/components/ping/__init__.py

@@ -1,13 +1,26 @@
 """The ping component."""
+from __future__ import annotations
+
+import logging
+
+from icmplib import SocketPermissionError, ping as icmp_ping
 
 from homeassistant.core import callback
+from homeassistant.helpers.reload import async_setup_reload_service
 
-DOMAIN = "ping"
+from .const import DEFAULT_START_ID, DOMAIN, MAX_PING_ID, PING_ID, PING_PRIVS, PLATFORMS
-PLATFORMS = ["binary_sensor"]
 
-PING_ID = "ping_id"
+_LOGGER = logging.getLogger(__name__)
-DEFAULT_START_ID = 129
+
-MAX_PING_ID = 65534
+
+async def async_setup(hass, config):
+    """Set up the template integration."""
+    await async_setup_reload_service(hass, DOMAIN, PLATFORMS)
+    hass.data[DOMAIN] = {
+        PING_PRIVS: await hass.async_add_executor_job(_can_use_icmp_lib_with_privilege),
+        PING_ID: DEFAULT_START_ID,
+    }
+    return True
 
 
 @callback
@@ -16,8 +29,7 @@ def async_get_next_ping_id(hass):
 
     Must be called in async
     """
-    current_id = hass.data.setdefault(DOMAIN, {}).get(PING_ID, DEFAULT_START_ID)
+    current_id = hass.data[DOMAIN][PING_ID]
-
     if current_id == MAX_PING_ID:
         next_id = DEFAULT_START_ID
     else:
@@ -26,3 +38,23 @@ def async_get_next_ping_id(hass):
     hass.data[DOMAIN][PING_ID] = next_id
 
     return next_id
+
+
+def _can_use_icmp_lib_with_privilege() -> None | bool:
+    """Verify we can create a raw socket."""
+    try:
+        icmp_ping("127.0.0.1", count=0, timeout=0, privileged=True)
+    except SocketPermissionError:
+        try:
+            icmp_ping("127.0.0.1", count=0, timeout=0, privileged=False)
+        except SocketPermissionError:
+            _LOGGER.debug(
+                "Cannot use icmplib because privileges are insufficient to create the socket"
+            )
+            return None
+        else:
+            _LOGGER.debug("Using icmplib in privileged=False mode")
+            return False
+    else:
+        _LOGGER.debug("Using icmplib in privileged=True mode")
+        return True

homeassistant/components/ping/binary_sensor.py

@@ -10,7 +10,7 @@ import re
 import sys
 from typing import Any
 
-from icmplib import SocketPermissionError, ping as icmp_ping
+from icmplib import NameLookupError, ping as icmp_ping
 import voluptuous as vol
 
 from homeassistant.components.binary_sensor import (
@@ -18,12 +18,12 @@ from homeassistant.components.binary_sensor import (
     PLATFORM_SCHEMA,
     BinarySensorEntity,
 )
-from homeassistant.const import CONF_HOST, CONF_NAME
+from homeassistant.const import CONF_HOST, CONF_NAME, STATE_ON
 import homeassistant.helpers.config_validation as cv
-from homeassistant.helpers.reload import setup_reload_service
+from homeassistant.helpers.restore_state import RestoreEntity
 
-from . import DOMAIN, PLATFORMS, async_get_next_ping_id
+from . import async_get_next_ping_id
-from .const import PING_TIMEOUT
+from .const import DOMAIN, ICMP_TIMEOUT, PING_PRIVS, PING_TIMEOUT
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -63,32 +63,30 @@ PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend(
 )
 
 
-def setup_platform(hass, config, add_entities, discovery_info=None) -> None:
+async def async_setup_platform(
+    hass, config, async_add_entities, discovery_info=None
+) -> None:
     """Set up the Ping Binary sensor."""
-    setup_reload_service(hass, DOMAIN, PLATFORMS)
-
     host = config[CONF_HOST]
     count = config[CONF_PING_COUNT]
     name = config.get(CONF_NAME, f"{DEFAULT_NAME} {host}")
-
+    privileged = hass.data[DOMAIN][PING_PRIVS]
-    try:
+    if privileged is None:
-        # Verify we can create a raw socket, or
-        # fallback to using a subprocess
-        icmp_ping("127.0.0.1", count=0, timeout=0)
-        ping_cls = PingDataICMPLib
-    except SocketPermissionError:
         ping_cls = PingDataSubProcess
+    else:
+        ping_cls = PingDataICMPLib
 
-    ping_data = ping_cls(hass, host, count)
+    async_add_entities(
-
+        [PingBinarySensor(name, ping_cls(hass, host, count, privileged))]
-    add_entities([PingBinarySensor(name, ping_data)], True)
+    )
 
 
-class PingBinarySensor(BinarySensorEntity):
+class PingBinarySensor(RestoreEntity, BinarySensorEntity):
     """Representation of a Ping Binary sensor."""
 
     def __init__(self, name: str, ping) -> None:
         """Initialize the Ping Binary sensor."""
+        self._available = False
         self._name = name
         self._ping = ping
 
@@ -97,6 +95,11 @@ class PingBinarySensor(BinarySensorEntity):
         """Return the name of the device."""
         return self._name
 
+    @property
+    def available(self) -> str:
+        """Return if we have done the first ping."""
+        return self._available
+
     @property
     def device_class(self) -> str:
         """Return the class of this sensor."""
@@ -105,7 +108,7 @@ class PingBinarySensor(BinarySensorEntity):
     @property
     def is_on(self) -> bool:
         """Return true if the binary sensor is on."""
-        return self._ping.available
+        return self._ping.is_alive
 
     @property
     def extra_state_attributes(self) -> dict[str, Any]:
@@ -121,6 +124,28 @@ class PingBinarySensor(BinarySensorEntity):
     async def async_update(self) -> None:
         """Get the latest data."""
         await self._ping.async_update()
+        self._available = True
+
+    async def async_added_to_hass(self):
+        """Restore previous state on restart to avoid blocking startup."""
+        await super().async_added_to_hass()
+
+        last_state = await self.async_get_last_state()
+        if last_state is not None:
+            self._available = True
+
+        if last_state is None or last_state.state != STATE_ON:
+            self._ping.data = False
+            return
+
+        attributes = last_state.attributes
+        self._ping.is_alive = True
+        self._ping.data = {
+            "min": attributes[ATTR_ROUND_TRIP_TIME_AVG],
+            "max": attributes[ATTR_ROUND_TRIP_TIME_MAX],
+            "avg": attributes[ATTR_ROUND_TRIP_TIME_MDEV],
+            "mdev": attributes[ATTR_ROUND_TRIP_TIME_MIN],
+        }
 
 
 class PingData:
@@ -132,26 +157,37 @@ class PingData:
         self._ip_address = host
         self._count = count
         self.data = {}
-        self.available = False
+        self.is_alive = False
 
 
 class PingDataICMPLib(PingData):
     """The Class for handling the data retrieval using icmplib."""
 
+    def __init__(self, hass, host, count, privileged) -> None:
+        """Initialize the data object."""
+        super().__init__(hass, host, count)
+        self._privileged = privileged
+
     async def async_update(self) -> None:
         """Retrieve the latest details from the host."""
         _LOGGER.debug("ping address: %s", self._ip_address)
-        data = await self.hass.async_add_executor_job(
+        try:
-            partial(
+            data = await self.hass.async_add_executor_job(
-                icmp_ping,
+                partial(
-                self._ip_address,
+                    icmp_ping,
-                count=self._count,
+                    self._ip_address,
-                timeout=1,
+                    count=self._count,
-                id=async_get_next_ping_id(self.hass),
+                    timeout=ICMP_TIMEOUT,
+                    id=async_get_next_ping_id(self.hass),
+                    privileged=self._privileged,
+                )
             )
-        )
+        except NameLookupError:
-        self.available = data.is_alive
+            self.is_alive = False
-        if not self.available:
+            return
+
+        self.is_alive = data.is_alive
+        if not self.is_alive:
             self.data = False
             return
 
@@ -166,7 +202,7 @@ class PingDataICMPLib(PingData):
 class PingDataSubProcess(PingData):
     """The Class for handling the data retrieval using the ping binary."""
 
-    def __init__(self, hass, host, count) -> None:
+    def __init__(self, hass, host, count, privileged) -> None:
         """Initialize the data object."""
         super().__init__(hass, host, count)
         if sys.platform == "win32":
@@ -254,4 +290,4 @@ class PingDataSubProcess(PingData):
     async def async_update(self) -> None:
         """Retrieve the latest details from the host."""
         self.data = await self.async_ping()
-        self.available = bool(self.data)
+        self.is_alive = bool(self.data)

homeassistant/components/ping/const.py

@@ -1,4 +1,21 @@
 """Tracks devices by sending a ICMP echo request (ping)."""
 
+# The ping binary and icmplib timeouts are not the same
+# timeout. ping is an overall timeout, icmplib is the
+# time since the data was sent.
+
+# ping binary
 PING_TIMEOUT = 3
+
+# icmplib timeout
+ICMP_TIMEOUT = 1
+
 PING_ATTEMPTS_COUNT = 3
+
+DOMAIN = "ping"
+PLATFORMS = ["binary_sensor"]
+
+PING_ID = "ping_id"
+PING_PRIVS = "ping_privs"
+DEFAULT_START_ID = 129
+MAX_PING_ID = 65534

homeassistant/components/ping/device_tracker.py

@@ -1,10 +1,12 @@
 """Tracks devices by sending a ICMP echo request (ping)."""
+import asyncio
 from datetime import timedelta
+from functools import partial
 import logging
 import subprocess
 import sys
 
-from icmplib import SocketPermissionError, ping as icmp_ping
+from icmplib import multiping
 import voluptuous as vol
 
 from homeassistant import const, util
@@ -15,16 +17,18 @@ from homeassistant.components.device_tracker.const import (
     SOURCE_TYPE_ROUTER,
 )
 import homeassistant.helpers.config_validation as cv
-from homeassistant.util.async_ import run_callback_threadsafe
+from homeassistant.helpers.event import async_track_point_in_utc_time
+from homeassistant.util.async_ import gather_with_concurrency
 from homeassistant.util.process import kill_subprocess
 
 from . import async_get_next_ping_id
-from .const import PING_ATTEMPTS_COUNT, PING_TIMEOUT
+from .const import DOMAIN, ICMP_TIMEOUT, PING_ATTEMPTS_COUNT, PING_PRIVS, PING_TIMEOUT
 
 _LOGGER = logging.getLogger(__name__)
 
 PARALLEL_UPDATES = 0
 CONF_PING_COUNT = "count"
+CONCURRENT_PING_LIMIT = 6
 
 PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend(
     {
@@ -37,16 +41,16 @@ PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend(
 class HostSubProcess:
     """Host object with ping detection."""
 
-    def __init__(self, ip_address, dev_id, hass, config):
+    def __init__(self, ip_address, dev_id, hass, config, privileged):
         """Initialize the Host pinger."""
         self.hass = hass
         self.ip_address = ip_address
         self.dev_id = dev_id
         self._count = config[CONF_PING_COUNT]
         if sys.platform == "win32":
-            self._ping_cmd = ["ping", "-n", "1", "-w", "1000", self.ip_address]
+            self._ping_cmd = ["ping", "-n", "1", "-w", "1000", ip_address]
         else:
-            self._ping_cmd = ["ping", "-n", "-q", "-c1", "-W1", self.ip_address]
+            self._ping_cmd = ["ping", "-n", "-q", "-c1", "-W1", ip_address]
 
     def ping(self):
         """Send an ICMP echo request and return True if success."""
@@ -63,86 +67,83 @@ class HostSubProcess:
         except subprocess.CalledProcessError:
             return False
 
-    def update(self, see):
+    def update(self) -> bool:
         """Update device state by sending one or more ping messages."""
         failed = 0
         while failed < self._count:  # check more times if host is unreachable
             if self.ping():
-                see(dev_id=self.dev_id, source_type=SOURCE_TYPE_ROUTER)
                 return True
             failed += 1
 
         _LOGGER.debug("No response from %s failed=%d", self.ip_address, failed)
+        return False
 
 
-class HostICMPLib:
+async def async_setup_scanner(hass, config, async_see, discovery_info=None):
-    """Host object with ping detection."""
-
-    def __init__(self, ip_address, dev_id, hass, config):
-        """Initialize the Host pinger."""
-        self.hass = hass
-        self.ip_address = ip_address
-        self.dev_id = dev_id
-        self._count = config[CONF_PING_COUNT]
-
-    def ping(self):
-        """Send an ICMP echo request and return True if success."""
-        next_id = run_callback_threadsafe(
-            self.hass.loop, async_get_next_ping_id, self.hass
-        ).result()
-
-        return icmp_ping(
-            self.ip_address, count=PING_ATTEMPTS_COUNT, timeout=1, id=next_id
-        ).is_alive
-
-    def update(self, see):
-        """Update device state by sending one or more ping messages."""
-        if self.ping():
-            see(dev_id=self.dev_id, source_type=SOURCE_TYPE_ROUTER)
-            return True
-
-        _LOGGER.debug(
-            "No response from %s (%s) failed=%d",
-            self.ip_address,
-            self.dev_id,
-            PING_ATTEMPTS_COUNT,
-        )
-
-
-def setup_scanner(hass, config, see, discovery_info=None):
     """Set up the Host objects and return the update function."""
 
-    try:
+    privileged = hass.data[DOMAIN][PING_PRIVS]
-        # Verify we can create a raw socket, or
+    ip_to_dev_id = {ip: dev_id for (dev_id, ip) in config[const.CONF_HOSTS].items()}
-        # fallback to using a subprocess
-        icmp_ping("127.0.0.1", count=0, timeout=0)
-        host_cls = HostICMPLib
-    except SocketPermissionError:
-        host_cls = HostSubProcess
-
-    hosts = [
-        host_cls(ip, dev_id, hass, config)
-        for (dev_id, ip) in config[const.CONF_HOSTS].items()
-    ]
     interval = config.get(
         CONF_SCAN_INTERVAL,
-        timedelta(seconds=len(hosts) * config[CONF_PING_COUNT]) + SCAN_INTERVAL,
+        timedelta(seconds=len(ip_to_dev_id) * config[CONF_PING_COUNT]) + SCAN_INTERVAL,
     )
     _LOGGER.debug(
         "Started ping tracker with interval=%s on hosts: %s",
         interval,
-        ",".join([host.ip_address for host in hosts]),
+        ",".join(ip_to_dev_id.keys()),
     )
 
-    def update_interval(now):
+    if privileged is None:
-        """Update all the hosts on every interval time."""
+        hosts = [
-        try:
+            HostSubProcess(ip, dev_id, hass, config, privileged)
-            for host in hosts:
+            for (dev_id, ip) in config[const.CONF_HOSTS].items()
-                host.update(see)
+        ]
-        finally:
+
-            hass.helpers.event.track_point_in_utc_time(
+        async def async_update(now):
-                update_interval, util.dt.utcnow() + interval
+            """Update all the hosts on every interval time."""
+            results = await gather_with_concurrency(
+                CONCURRENT_PING_LIMIT,
+                *[hass.async_add_executor_job(host.update) for host in hosts],
+            )
+            await asyncio.gather(
+                *[
+                    async_see(dev_id=host.dev_id, source_type=SOURCE_TYPE_ROUTER)
+                    for idx, host in enumerate(hosts)
+                    if results[idx]
+                ]
             )
 
-    update_interval(None)
+    else:
+
+        async def async_update(now):
+            """Update all the hosts on every interval time."""
+            responses = await hass.async_add_executor_job(
+                partial(
+                    multiping,
+                    ip_to_dev_id.keys(),
+                    count=PING_ATTEMPTS_COUNT,
+                    timeout=ICMP_TIMEOUT,
+                    privileged=privileged,
+                    id=async_get_next_ping_id(hass),
+                )
+            )
+            _LOGGER.debug("Multiping responses: %s", responses)
+            await asyncio.gather(
+                *[
+                    async_see(dev_id=dev_id, source_type=SOURCE_TYPE_ROUTER)
+                    for idx, dev_id in enumerate(ip_to_dev_id.values())
+                    if responses[idx].is_alive
+                ]
+            )
+
+    async def _async_update_interval(now):
+        try:
+            await async_update(now)
+        finally:
+            async_track_point_in_utc_time(
+                hass, _async_update_interval, util.dt.utcnow() + interval
+            )
+
+    await _async_update_interval(None)
     return True

homeassistant/components/ping/manifest.json

@@ -3,6 +3,6 @@
   "name": "Ping (ICMP)",
   "documentation": "https://www.home-assistant.io/integrations/ping",
   "codeowners": [],
-  "requirements": ["icmplib==2.0.2"],
+  "requirements": ["icmplib==2.1.1"],
   "quality_scale": "internal"
 }

requirements_all.txt

@@ -814,7 +814,7 @@ ibm-watson==4.0.1
 ibmiotf==0.3.4
 
 # homeassistant.components.ping
-icmplib==2.0.2
+icmplib==2.1.1
 
 # homeassistant.components.iglo
 iglo==1.2.7

requirements_test_all.txt

@@ -443,7 +443,7 @@ hyperion-py==0.7.0
 iaqualink==0.3.4
 
 # homeassistant.components.ping
-icmplib==2.0.2
+icmplib==2.1.1
 
 # homeassistant.components.influxdb
 influxdb-client==1.14.0