From 68714c2067c8f91f83ed14f52008f0e2e5be0d36 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" <nick@koston.org> Date: Wed, 2 Jun 2021 11:10:33 -0500 Subject: [PATCH] Update ping to use asyncio function in icmplib (#50808) --- homeassistant/components/ping/__init__.py | 23 +--------------- .../components/ping/binary_sensor.py | 24 +++++++---------- homeassistant/components/ping/const.py | 3 --- .../components/ping/device_tracker.py | 18 +++++-------- homeassistant/components/ping/manifest.json | 2 +- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- tests/components/ping/test_init.py | 26 ------------------- 8 files changed, 19 insertions(+), 81 deletions(-) diff --git a/homeassistant/components/ping/__init__.py b/homeassistant/components/ping/__init__.py index b9a9f6460db..70b90ccd886 100644 --- a/homeassistant/components/ping/__init__.py +++ b/homeassistant/components/ping/__init__.py @@ -5,10 +5,9 @@ import logging from icmplib import SocketPermissionError, ping as icmp_ping -from homeassistant.core import callback from homeassistant.helpers.reload import async_setup_reload_service -from .const import DEFAULT_START_ID, DOMAIN, MAX_PING_ID, PING_ID, PING_PRIVS, PLATFORMS +from .const import DOMAIN, PING_PRIVS, PLATFORMS _LOGGER = logging.getLogger(__name__) @@ -18,30 +17,10 @@ async def async_setup(hass, config): 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 -def async_get_next_ping_id(hass, count=1): - """Find the next id to use in the outbound ping. - - When using multiping, we increment the id - by the number of ids that multiping - will use. - - Must be called in async - """ - allocated_id = hass.data[DOMAIN][PING_ID] + 1 - if allocated_id > MAX_PING_ID: - allocated_id -= MAX_PING_ID - DEFAULT_START_ID - hass.data[DOMAIN][PING_ID] += count - if hass.data[DOMAIN][PING_ID] > MAX_PING_ID: - hass.data[DOMAIN][PING_ID] -= MAX_PING_ID - DEFAULT_START_ID - return allocated_id - - def _can_use_icmp_lib_with_privilege() -> None | bool: """Verify we can create a raw socket.""" try: diff --git a/homeassistant/components/ping/binary_sensor.py b/homeassistant/components/ping/binary_sensor.py index 9ae891d598b..cf2d8f7ed7a 100644 --- a/homeassistant/components/ping/binary_sensor.py +++ b/homeassistant/components/ping/binary_sensor.py @@ -4,13 +4,12 @@ from __future__ import annotations import asyncio from contextlib import suppress from datetime import timedelta -from functools import partial import logging import re import sys from typing import Any -from icmplib import NameLookupError, ping as icmp_ping +from icmplib import NameLookupError, async_ping import voluptuous as vol from homeassistant.components.binary_sensor import ( @@ -22,7 +21,6 @@ from homeassistant.const import CONF_HOST, CONF_NAME, STATE_ON import homeassistant.helpers.config_validation as cv from homeassistant.helpers.restore_state import RestoreEntity -from . import async_get_next_ping_id from .const import DOMAIN, ICMP_TIMEOUT, PING_PRIVS, PING_TIMEOUT _LOGGER = logging.getLogger(__name__) @@ -141,10 +139,10 @@ class PingBinarySensor(RestoreEntity, BinarySensorEntity): attributes = last_state.attributes self._ping.is_alive = True self._ping.data = { - "min": attributes[ATTR_ROUND_TRIP_TIME_AVG], + "min": attributes[ATTR_ROUND_TRIP_TIME_MIN], "max": attributes[ATTR_ROUND_TRIP_TIME_MAX], - "avg": attributes[ATTR_ROUND_TRIP_TIME_MDEV], - "mdev": attributes[ATTR_ROUND_TRIP_TIME_MIN], + "avg": attributes[ATTR_ROUND_TRIP_TIME_AVG], + "mdev": attributes[ATTR_ROUND_TRIP_TIME_MDEV], } @@ -172,15 +170,11 @@ class PingDataICMPLib(PingData): """Retrieve the latest details from the host.""" _LOGGER.debug("ping address: %s", self._ip_address) try: - data = await self.hass.async_add_executor_job( - partial( - icmp_ping, - self._ip_address, - count=self._count, - timeout=ICMP_TIMEOUT, - id=async_get_next_ping_id(self.hass), - privileged=self._privileged, - ) + data = await async_ping( + self._ip_address, + count=self._count, + timeout=ICMP_TIMEOUT, + privileged=self._privileged, ) except NameLookupError: self.is_alive = False diff --git a/homeassistant/components/ping/const.py b/homeassistant/components/ping/const.py index 62fca9123ba..9ca99db2419 100644 --- a/homeassistant/components/ping/const.py +++ b/homeassistant/components/ping/const.py @@ -15,7 +15,4 @@ 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 diff --git a/homeassistant/components/ping/device_tracker.py b/homeassistant/components/ping/device_tracker.py index d7d812d371d..b5acecf9314 100644 --- a/homeassistant/components/ping/device_tracker.py +++ b/homeassistant/components/ping/device_tracker.py @@ -1,12 +1,11 @@ """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 multiping +from icmplib import async_multiping import voluptuous as vol from homeassistant import const, util @@ -21,7 +20,6 @@ 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 DOMAIN, ICMP_TIMEOUT, PING_ATTEMPTS_COUNT, PING_PRIVS, PING_TIMEOUT _LOGGER = logging.getLogger(__name__) @@ -118,15 +116,11 @@ async def async_setup_scanner(hass, config, async_see, discovery_info=None): 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, len(ip_to_dev_id)), - ) + responses = await async_multiping( + list(ip_to_dev_id), + count=PING_ATTEMPTS_COUNT, + timeout=ICMP_TIMEOUT, + privileged=privileged, ) _LOGGER.debug("Multiping responses: %s", responses) await asyncio.gather( diff --git a/homeassistant/components/ping/manifest.json b/homeassistant/components/ping/manifest.json index 639a30a4fa0..d25d0fc731e 100644 --- a/homeassistant/components/ping/manifest.json +++ b/homeassistant/components/ping/manifest.json @@ -3,7 +3,7 @@ "name": "Ping (ICMP)", "documentation": "https://www.home-assistant.io/integrations/ping", "codeowners": [], - "requirements": ["icmplib==2.1.1"], + "requirements": ["icmplib==3.0"], "quality_scale": "internal", "iot_class": "local_polling" } diff --git a/requirements_all.txt b/requirements_all.txt index 0235499d0d9..447d4e238ce 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -816,7 +816,7 @@ ibm-watson==5.1.0 ibmiotf==0.3.4 # homeassistant.components.ping -icmplib==2.1.1 +icmplib==3.0 # homeassistant.components.network ifaddr==0.1.7 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 21733c61b71..14b103584be 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -460,7 +460,7 @@ hyperion-py==0.7.4 iaqualink==0.3.4 # homeassistant.components.ping -icmplib==2.1.1 +icmplib==3.0 # homeassistant.components.network ifaddr==0.1.7 diff --git a/tests/components/ping/test_init.py b/tests/components/ping/test_init.py index 3dfe193c4d5..05dc47e27d8 100644 --- a/tests/components/ping/test_init.py +++ b/tests/components/ping/test_init.py @@ -1,27 +1 @@ """Test ping id allocation.""" - -from homeassistant.components.ping import async_get_next_ping_id -from homeassistant.components.ping.const import ( - DEFAULT_START_ID, - DOMAIN, - MAX_PING_ID, - PING_ID, -) - - -async def test_async_get_next_ping_id(hass): - """Verify we allocate ping ids as expected.""" - hass.data[DOMAIN] = {PING_ID: DEFAULT_START_ID} - - assert async_get_next_ping_id(hass) == DEFAULT_START_ID + 1 - assert async_get_next_ping_id(hass) == DEFAULT_START_ID + 2 - assert async_get_next_ping_id(hass, 2) == DEFAULT_START_ID + 3 - assert async_get_next_ping_id(hass) == DEFAULT_START_ID + 5 - - hass.data[DOMAIN][PING_ID] = MAX_PING_ID - assert async_get_next_ping_id(hass) == DEFAULT_START_ID + 1 - assert async_get_next_ping_id(hass) == DEFAULT_START_ID + 2 - - hass.data[DOMAIN][PING_ID] = MAX_PING_ID - assert async_get_next_ping_id(hass, 2) == DEFAULT_START_ID + 1 - assert async_get_next_ping_id(hass) == DEFAULT_START_ID + 3 -- GitLab