Skip to content

Commit 7b50790

Browse files
committed
Mark a device online from any reachability source, not just mDNS
A device whose network blocks mDNS but answers ping and MQTT showed offline: the state monitor's priority gate dropped a lower-priority source's ONLINE when a higher-priority source owned the device, and the ICMP sweep only resolved the mDNS .local hostname, never the IP MQTT already supplied via apply_ip. apply() now lets a positive reachability from any source bring a not-online device online (the gate still governs offline/downgrades so a flaky low-priority source can't drop a higher one), and the ping sweep falls back to the device's known IP addresses when its hostname won't resolve. Restores the legacy dashboard's online-from-any-source behaviour.
1 parent a973add commit 7b50790

3 files changed

Lines changed: 77 additions & 1 deletion

File tree

esphome_device_builder/controllers/_device_state_monitor/controller.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,17 @@ def apply(self, name: str, state: DeviceState, source: str, *, claim: bool = Fal
231231
self.state.reachability.observe(name, source)
232232

233233
current_source = self.state.state_source.get(name, ReachabilitySource.UNKNOWN)
234-
if _SOURCE_PRIORITY.get(source, 0) < _SOURCE_PRIORITY.get(current_source, 0):
234+
# Online-wins: a positive reachability from any source brings a
235+
# not-online device online, even one a higher-priority source owns
236+
# (ping/MQTT reviving a device a stale mdns/mqtt OFFLINE owns). The
237+
# priority gate still governs OFFLINE/downgrades so a low-priority
238+
# flap can't drop a device a higher source confirmed online.
239+
online_takeover = state == DeviceState.ONLINE and any(
240+
d.state != DeviceState.ONLINE for d in devices
241+
)
242+
if not online_takeover and _SOURCE_PRIORITY.get(source, 0) < _SOURCE_PRIORITY.get(
243+
current_source, 0
244+
):
235245
return False
236246
# Dedupe must look at *every* matching device, not just the
237247
# first. Duplicate ``esphome.name`` entries (a config plus

esphome_device_builder/controllers/_device_state_monitor/ping.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,12 @@ def _select_ping_targets(self) -> tuple[list[Device], list[Device]]:
174174
monitor.apply_ip_addresses(device.name, cached)
175175
continue
176176
if monitor.state.dns_cache.has_cached_failure(device.address):
177+
if device.ip_addresses:
178+
# The ``.local`` won't resolve, but a prior MQTT/DNS
179+
# observation left a known IP — ping that instead of
180+
# marking the device offline (mDNS-less devices).
181+
pingable.append(device)
182+
continue
177183
# Don't hand the bare hostname to icmplib (it would
178184
# hammer the system resolver every sweep). Apply
179185
# OFFLINE under the ``ping`` source so a future
@@ -189,6 +195,11 @@ async def _resolve_and_ping(self, device: Device) -> None:
189195
monitor = self._monitor
190196
async with self._concurrency:
191197
addresses = await monitor.state.dns_cache.async_resolve(device.address)
198+
if not addresses:
199+
# mDNS-less devices: the ``.local`` won't resolve but a
200+
# prior MQTT/DNS observation left a usable IP. Ping that so
201+
# ping can confirm a device the network won't resolve.
202+
addresses = list(device.ip_addresses)
192203
if not addresses:
193204
monitor.apply(device.name, DeviceState.OFFLINE, "ping")
194205
return

tests/test_state_monitor_reachability.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,61 @@ def test_apply_with_no_tracker_does_not_raise() -> None:
191191
monitor.apply("kitchen", DeviceState.ONLINE, "mdns")
192192

193193

194+
def test_lower_priority_online_revives_higher_priority_offline() -> None:
195+
"""A ping ONLINE brings online a device a higher-priority mqtt OFFLINE owns."""
196+
devices = [_make_device(state=DeviceState.OFFLINE)]
197+
monitor = _make_monitor(devices)
198+
monitor.state.state_source["kitchen"] = "mqtt"
199+
200+
assert monitor.apply("kitchen", DeviceState.ONLINE, "ping") is True
201+
assert devices[0].state is DeviceState.ONLINE
202+
assert monitor.state.state_source["kitchen"] == "ping"
203+
204+
205+
def test_lower_priority_offline_does_not_drop_higher_priority_online() -> None:
206+
"""The gate still protects an mdns ONLINE from a ping OFFLINE flap."""
207+
devices = [_make_device(state=DeviceState.ONLINE)]
208+
monitor = _make_monitor(devices)
209+
monitor.state.state_source["kitchen"] = "mdns"
210+
211+
assert monitor.apply("kitchen", DeviceState.OFFLINE, "ping") is False
212+
assert devices[0].state is DeviceState.ONLINE
213+
assert monitor.state.state_source["kitchen"] == "mdns"
214+
215+
216+
def test_select_ping_targets_keeps_device_with_known_ip_when_dns_failed() -> None:
217+
"""A cached DNS failure with a known IP pings the IP, not OFFLINE+dns_failed."""
218+
devices = [_make_device(state=DeviceState.OFFLINE, ip_addresses=["10.0.0.5"])]
219+
monitor = _make_monitor(devices)
220+
monitor.get_cached_addresses = lambda _a: None
221+
monitor.state.dns_cache.has_cached_failure = MagicMock(return_value=True)
222+
223+
pingable, dns_failed = monitor._ping._select_ping_targets()
224+
225+
assert devices[0] in pingable
226+
assert dns_failed == []
227+
assert devices[0].state is DeviceState.OFFLINE
228+
229+
230+
async def test_resolve_and_ping_falls_back_to_known_ip() -> None:
231+
"""When the .local won't resolve, ping the MQTT/last-known IP and go ONLINE."""
232+
devices = [_make_device(state=DeviceState.OFFLINE, ip_addresses=["10.0.0.5"])]
233+
monitor = _make_monitor(devices)
234+
monitor.state.dns_cache.async_resolve = AsyncMock(return_value=[])
235+
236+
fake_result = MagicMock()
237+
fake_result.is_alive = True
238+
fake_result.min_rtt = 1.0
239+
with patch(
240+
"esphome_device_builder.controllers._device_state_monitor.ping.icmp_ping",
241+
AsyncMock(return_value=fake_result),
242+
) as mock_ping:
243+
await monitor._ping._resolve_and_ping(devices[0])
244+
245+
assert mock_ping.await_args.args[0] == "10.0.0.5"
246+
assert devices[0].state is DeviceState.ONLINE
247+
248+
194249
async def test_ping_success_records_rtt_and_observation() -> None:
195250
"""A successful ICMP probe captures ``min_rtt`` and stamps freshness."""
196251
devices = [_make_device()]

0 commit comments

Comments
 (0)