From b7ad83c65571b2a5f18f185f3030e924a83fe97d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 9 Sep 2020 15:19:14 -0500 Subject: [PATCH] Use a unique id for each icmplib ping to avoid mixing unrelated responses (#39830) --- homeassistant/components/ping/__init__.py | 24 +++++++++++++++++++ .../components/ping/binary_sensor.py | 23 ++++++++++++------ .../components/ping/device_tracker.py | 13 ++++++++-- homeassistant/components/ping/manifest.json | 2 +- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- 6 files changed, 54 insertions(+), 12 deletions(-) diff --git a/homeassistant/components/ping/__init__.py b/homeassistant/components/ping/__init__.py index d5ec35276cf..19207ada1b7 100644 --- a/homeassistant/components/ping/__init__.py +++ b/homeassistant/components/ping/__init__.py @@ -1,4 +1,28 @@ """The ping component.""" +from homeassistant.core import callback + DOMAIN = "ping" PLATFORMS = ["binary_sensor"] + +PING_ID = "ping_id" +DEFAULT_START_ID = 129 +MAX_PING_ID = 65534 + + +@callback +def async_get_next_ping_id(hass): + """Find the next id to use in the outbound ping. + + Must be called in async + """ + current_id = hass.data.setdefault(DOMAIN, {}).get(PING_ID, DEFAULT_START_ID) + + if current_id == MAX_PING_ID: + next_id = DEFAULT_START_ID + else: + next_id = current_id + 1 + + hass.data[DOMAIN][PING_ID] = next_id + + return next_id diff --git a/homeassistant/components/ping/binary_sensor.py b/homeassistant/components/ping/binary_sensor.py index cb0d025f65d..5befe0b7f3a 100644 --- a/homeassistant/components/ping/binary_sensor.py +++ b/homeassistant/components/ping/binary_sensor.py @@ -1,6 +1,7 @@ """Tracks the latency of a host by sending ICMP echo requests (ping).""" import asyncio from datetime import timedelta +from functools import partial import logging import re import sys @@ -14,7 +15,7 @@ from homeassistant.const import CONF_HOST, CONF_NAME import homeassistant.helpers.config_validation as cv from homeassistant.helpers.reload import setup_reload_service -from . import DOMAIN, PLATFORMS +from . import DOMAIN, PLATFORMS, async_get_next_ping_id from .const import PING_TIMEOUT _LOGGER = logging.getLogger(__name__) @@ -131,20 +132,28 @@ class PingData: class PingDataICMPLib(PingData): """The Class for handling the data retrieval using icmplib.""" - def ping(self): - """Send ICMP echo request and return details.""" - return icmp_ping(self._ip_address, count=self._count) - async def async_update(self) -> None: """Retrieve the latest details from the host.""" - data = await self.hass.async_add_executor_job(self.ping) + _LOGGER.warning("ping address: %s", self._ip_address) + data = await self.hass.async_add_executor_job( + partial( + icmp_ping, + self._ip_address, + count=self._count, + id=async_get_next_ping_id(self.hass), + ) + ) + self.available = data.is_alive + if not self.available: + self.data = False + return + self.data = { "min": data.min_rtt, "max": data.max_rtt, "avg": data.avg_rtt, "mdev": "", } - self.available = data.is_alive class PingDataSubProcess(PingData): diff --git a/homeassistant/components/ping/device_tracker.py b/homeassistant/components/ping/device_tracker.py index cbbce13171b..ea2d7a526e3 100644 --- a/homeassistant/components/ping/device_tracker.py +++ b/homeassistant/components/ping/device_tracker.py @@ -15,8 +15,10 @@ 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.util.process import kill_subprocess +from . import async_get_next_ping_id from .const import PING_ATTEMPTS_COUNT, PING_TIMEOUT _LOGGER = logging.getLogger(__name__) @@ -76,15 +78,22 @@ class HostSubProcess: class HostICMPLib: """Host object with ping detection.""" - def __init__(self, ip_address, dev_id, _, config): + 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.""" - return icmp_ping(self.ip_address, count=PING_ATTEMPTS_COUNT).is_alive + 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, id=next_id + ).is_alive def update(self, see): """Update device state by sending one or more ping messages.""" diff --git a/homeassistant/components/ping/manifest.json b/homeassistant/components/ping/manifest.json index f23697808a2..675ee1a9586 100644 --- a/homeassistant/components/ping/manifest.json +++ b/homeassistant/components/ping/manifest.json @@ -3,6 +3,6 @@ "name": "Ping (ICMP)", "documentation": "https://www.home-assistant.io/integrations/ping", "codeowners": [], - "requirements": ["icmplib==1.1.1"], + "requirements": ["icmplib==1.1.3"], "quality_scale": "internal" } diff --git a/requirements_all.txt b/requirements_all.txt index 7287f613fcc..c66acc032d4 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -789,7 +789,7 @@ ibm-watson==4.0.1 ibmiotf==0.3.4 # homeassistant.components.ping -icmplib==1.1.1 +icmplib==1.1.3 # homeassistant.components.iglo iglo==1.2.7 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 28906de633d..5e18738f556 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -392,7 +392,7 @@ huawei-lte-api==1.4.12 iaqualink==0.3.4 # homeassistant.components.ping -icmplib==1.1.1 +icmplib==1.1.3 # homeassistant.components.influxdb influxdb-client==1.8.0