Detect offline hub children and fail fast on writes to them (#1830)
Some checks failed
CI / Perform Lint Checks (3.15) (push) Has been cancelled
CodeQL Checks / Analyze (python) (push) Has been cancelled
CI / Python 3.11 on macos-latest (push) Has been cancelled
CI / Python 3.12 on macos-latest (push) Has been cancelled
CI / Python 3.13 on macos-latest (push) Has been cancelled
CI / Python 3.14 on macos-latest (push) Has been cancelled
CI / Python 3.15 on macos-latest (push) Has been cancelled
CI / Python 3.11 on ubuntu-latest (push) Has been cancelled
CI / Python 3.12 on ubuntu-latest (push) Has been cancelled
CI / Python 3.13 on ubuntu-latest (push) Has been cancelled
CI / Python 3.14 on ubuntu-latest (push) Has been cancelled
CI / Python 3.15 on ubuntu-latest (push) Has been cancelled
CI / Python 3.11 on windows-latest (push) Has been cancelled
CI / Python 3.12 on windows-latest (push) Has been cancelled
CI / Python 3.13 on windows-latest (push) Has been cancelled
CI / Python 3.14 on windows-latest (push) Has been cancelled
CI / Python 3.15 on windows-latest (push) Has been cancelled
Stale / stale (push) Has been cancelled

Hubs keep reporting for children they have lost contact with, so callers
see stale values as if they were live. Trying to write to such devices
fail after timeouts with a generic error.
This PR exposes the offline/online information as reported by the hub,
and refuses writes to unreachable child devices without waiting for
device errors.

In details:
- Add new API: `Device.is_available`; hub children report unavailable
when the hub marks them offline or no longer lists them.
- New feature `available` for child devices to make it visible to the
CLI.
- Refuse writes to unavailable children with the new
`DeviceUnavailableError` instead of waiting for the hub to time out.
This commit is contained in:
Teemu R.
2026-11-01 18:18:03 +01:00
committed by GitHub
parent 81243ed87f
commit a68f2b3304
13 changed files with 229 additions and 16 deletions

View File

@@ -122,6 +122,12 @@
:undoc-members: :undoc-members:
``` ```
```{eval-rst}
.. autoclass:: kasa.exceptions.DeviceUnavailableError
:members:
:undoc-members:
```
```{eval-rst} ```{eval-rst}
.. autoclass:: kasa.exceptions.UnsupportedDeviceError .. autoclass:: kasa.exceptions.UnsupportedDeviceError
:members: :members:

View File

@@ -137,6 +137,8 @@ The base exception for all library errors is {class}`KasaException <kasa.excepti
- If the device returns an error the library raises a {class}`DeviceError <kasa.exceptions.DeviceError>` which will usually contain an ``error_code`` with the detail. - If the device returns an error the library raises a {class}`DeviceError <kasa.exceptions.DeviceError>` which will usually contain an ``error_code`` with the detail.
- If the device fails to authenticate the library raises an {class}`AuthenticationError <kasa.exceptions.AuthenticationError>` which is derived - If the device fails to authenticate the library raises an {class}`AuthenticationError <kasa.exceptions.AuthenticationError>` which is derived
from {class}`DeviceError <kasa.exceptions.DeviceError>` and could contain an ``error_code`` depending on the type of failure. from {class}`DeviceError <kasa.exceptions.DeviceError>` and could contain an ``error_code`` depending on the type of failure.
- If a command is sent to a hub child that is not available (see {attr}`Device.is_available <kasa.Device.is_available>`) the library raises a {class}`DeviceUnavailableError <kasa.exceptions.DeviceUnavailableError>`,
derived from {class}`DeviceError <kasa.exceptions.DeviceError>`, without contacting the device.
- If the library encounters and unsupported deviceit raises an {class}`UnsupportedDeviceError <kasa.exceptions.UnsupportedDeviceError>`. - If the library encounters and unsupported deviceit raises an {class}`UnsupportedDeviceError <kasa.exceptions.UnsupportedDeviceError>`.
- If the device fails to respond within a timeout the library raises a {class}`TimeoutError <kasa.exceptions.TimeoutError>`. - If the device fails to respond within a timeout the library raises a {class}`TimeoutError <kasa.exceptions.TimeoutError>`.
- All other failures will raise the base {class}`KasaException <kasa.exceptions.KasaException>` class. - All other failures will raise the base {class}`KasaException <kasa.exceptions.KasaException>` class.

View File

@@ -30,6 +30,7 @@ from kasa.emeterstatus import EmeterStatus
from kasa.exceptions import ( from kasa.exceptions import (
AuthenticationError, AuthenticationError,
DeviceError, DeviceError,
DeviceUnavailableError,
KasaException, KasaException,
TimeoutError, TimeoutError,
UnsupportedDeviceError, UnsupportedDeviceError,
@@ -68,6 +69,7 @@ __all__ = [
"KasaException", "KasaException",
"AuthenticationError", "AuthenticationError",
"DeviceError", "DeviceError",
"DeviceUnavailableError",
"UnsupportedDeviceError", "UnsupportedDeviceError",
"TimeoutError", "TimeoutError",
"Credentials", "Credentials",

View File

@@ -34,8 +34,12 @@ async def hub(dev: Device):
@pass_dev @pass_dev
async def hub_list(dev: Device): async def hub_list(dev: Device):
"""List hub paired child devices.""" """List hub paired child devices."""
for c in dev.children: echo(f"{'INDEX':<5} {'DEVICE ID':<40} {'MODEL':<9} {'AVAILABLE':<9} ALIAS")
echo(f"{c.device_id}: {c}") for index, child in enumerate(dev.children):
echo(
f"{index:<5} {child.device_id:<40} {child.model:<9} "
f"{child.is_available!s:<9} {child.alias}"
)
@hub.command(name="supported") @hub.command(name="supported")

View File

@@ -277,6 +277,11 @@ class Device(ABC):
"""Return True if device is off.""" """Return True if device is off."""
return not self.is_on return not self.is_on
@property
def is_available(self) -> bool:
"""Return False if the device is known to be unreachable."""
return True
@abstractmethod @abstractmethod
async def turn_on(self, **kwargs) -> dict: async def turn_on(self, **kwargs) -> dict:
"""Turn on the device.""" """Turn on the device."""

View File

@@ -55,6 +55,10 @@ class AuthenticationError(DeviceError):
"""Base exception for device authentication errors.""" """Base exception for device authentication errors."""
class DeviceUnavailableError(DeviceError):
"""Exception for commands to an unreachable device."""
class _RetryableError(DeviceError): class _RetryableError(DeviceError):
"""Retryable exception for device errors.""" """Retryable exception for device errors."""

View File

@@ -9,6 +9,8 @@ from typing import Any
from ..device import DeviceInfo from ..device import DeviceInfo
from ..device_type import DeviceType from ..device_type import DeviceType
from ..deviceconfig import DeviceConfig from ..deviceconfig import DeviceConfig
from ..exceptions import DeviceUnavailableError
from ..feature import Feature
from ..protocols.smartprotocol import SmartProtocol, _ChildProtocolWrapper from ..protocols.smartprotocol import SmartProtocol, _ChildProtocolWrapper
from .smartdevice import ComponentsRaw, SmartDevice from .smartdevice import ComponentsRaw, SmartDevice
from .smartmodule import SmartModule from .smartmodule import SmartModule
@@ -49,6 +51,7 @@ class SmartChildDevice(SmartDevice):
_protocol = protocol or _ChildProtocolWrapper(self._id, parent.protocol) _protocol = protocol or _ChildProtocolWrapper(self._id, parent.protocol)
super().__init__(parent.host, config=parent.config, protocol=_protocol) super().__init__(parent.host, config=parent.config, protocol=_protocol)
self._parent = parent self._parent = parent
self._was_available = True
self._update_internal_state(info) self._update_internal_state(info)
self._components_raw = component_info_raw self._components_raw = component_info_raw
self._components = self._parse_components(self._components_raw) self._components = self._parse_components(self._components_raw)
@@ -83,6 +86,10 @@ class SmartChildDevice(SmartDevice):
Internal implementation to allow patching of public update in the cli Internal implementation to allow patching of public update in the cli
or test framework. or test framework.
""" """
if (available := self.is_available) != self._was_available:
self._was_available = available
_LOGGER.debug("%s (%s) available: %s", self.alias, self._id, available)
now = time.monotonic() now = time.monotonic()
module_queries: list[SmartModule] = [] module_queries: list[SmartModule] = []
req: dict[str, Any] = {} req: dict[str, Any] = {}
@@ -160,6 +167,34 @@ class SmartChildDevice(SmartDevice):
return self._device_type return self._device_type
@property
def is_available(self) -> bool:
"""Return False if the hub reports the device offline or no longer lists it."""
listed = self._parent is None or self._id in self._parent._children
return listed and self._info.get("status") != "offline"
async def _initialize_features(self) -> None:
"""Initialize device features."""
await super()._initialize_features()
if "status" in self._info:
self._add_feature(
Feature(
self,
id="available",
name="Available",
attribute_getter="is_available",
category=Feature.Category.Info,
type=Feature.Type.BinarySensor,
)
)
async def _query_helper(self, method: str, params: dict | None = None) -> dict:
# Fail fast on writes, the hub rejects them only after a timeout.
if not method.startswith("get") and not self.is_available:
raise DeviceUnavailableError(f"{self.alias} is not available")
return await super()._query_helper(method, params)
def __repr__(self) -> str: def __repr__(self) -> str:
if not self._parent: if not self._parent:
return f"<{self.device_type}(child) without parent>" return f"<{self.device_type}(child) without parent>"

View File

@@ -730,7 +730,7 @@ class SmartDevice(Device):
See :meth:`is_on`. See :meth:`is_on`.
""" """
return await self.protocol.query({"set_device_info": {"device_on": on}}) return await self._query_helper("set_device_info", {"device_on": on})
async def turn_on(self, **kwargs: Any) -> dict: async def turn_on(self, **kwargs: Any) -> dict:
"""Turn on the device.""" """Turn on the device."""
@@ -833,8 +833,8 @@ class SmartDevice(Device):
async def set_alias(self, alias: str) -> dict: async def set_alias(self, alias: str) -> dict:
"""Set the device name (alias).""" """Set the device name (alias)."""
return await self.protocol.query( return await self._query_helper(
{"set_device_info": {"nickname": base64.b64encode(alias.encode()).decode()}} "set_device_info", {"nickname": base64.b64encode(alias.encode()).decode()}
) )
async def reboot(self, delay: int = 1) -> None: async def reboot(self, delay: int = 1) -> None:
@@ -843,14 +843,14 @@ class SmartDevice(Device):
Note that giving a delay of zero causes this to block, Note that giving a delay of zero causes this to block,
as the device reboots immediately without responding to the call. as the device reboots immediately without responding to the call.
""" """
await self.protocol.query({"device_reboot": {"delay": delay}}) await self._query_helper("device_reboot", {"delay": delay})
async def factory_reset(self) -> None: async def factory_reset(self) -> None:
"""Reset device back to factory settings. """Reset device back to factory settings.
Note, this does not downgrade the firmware. Note, this does not downgrade the firmware.
""" """
await self.protocol.query("device_reset") await self._query_helper("device_reset")
@property @property
def device_type(self) -> DeviceType: def device_type(self) -> DeviceType:

View File

@@ -85,6 +85,12 @@ class SmartCamChild(SmartChildDevice, SmartCamDevice):
# devices # devices
self._info = self._map_child_info_from_parent(info) self._info = self._map_child_info_from_parent(info)
@property
def is_available(self) -> bool:
"""Return False if the hub reports the camera offline or no longer lists it."""
# Camera children report "online" instead of an "offline" status.
return super().is_available and self._info.get("online") is not False
@property @property
def device_type(self) -> DeviceType: def device_type(self) -> DeviceType:
"""Return the device type.""" """Return the device type."""

View File

@@ -215,7 +215,7 @@ class SmartCamDevice(SmartDevice):
async def _query_setter_helper( async def _query_setter_helper(
self, method: str, module: str, section: str, params: dict | None = None self, method: str, module: str, section: str, params: dict | None = None
) -> dict: ) -> dict:
res = await self.protocol.query({method: {module: {section: params}}}) res = await self._query_helper(method, {module: {section: params}})
return res return res
@@ -293,10 +293,8 @@ class SmartCamDevice(SmartDevice):
async def set_alias(self, alias: str) -> dict: async def set_alias(self, alias: str) -> dict:
"""Set the device name (alias).""" """Set the device name (alias)."""
return await self.protocol.query( return await self._query_helper(
{ "setDeviceAlias", {"system": {"sys": {"dev_alias": alias}}}
"setDeviceAlias": {"system": {"sys": {"dev_alias": alias}}},
}
) )
@property @property

View File

@@ -5,7 +5,8 @@ from pytest_mock import MockerFixture
from kasa import Device, DeviceType, Module from kasa import Device, DeviceType, Module
from kasa.cli.hub import hub from kasa.cli.hub import hub
from ..device_fixtures import hubs, plug_iot from ..device_fixtures import hubs, hubs_smart, plug_iot
from ..fakeprotocol_smart import FakeSmartTransport
@hubs @hubs
@@ -51,6 +52,28 @@ async def test_hub_unpair(
assert res.exit_code == 0 assert res.exit_code == 0
@hubs_smart
@pytest.mark.requires_dummy
async def test_hub_list(dev: Device, runner: CliRunner) -> None:
"""Test that list shows each child with its index and availability."""
if not dev.children:
pytest.skip("Cannot test without child devices")
assert isinstance(dev.protocol._transport, FakeSmartTransport)
child_list = dev.protocol._transport.info["get_child_device_list"]
child_list["child_device_list"][0]["status"] = "offline"
await dev.update()
res = await runner.invoke(hub, ["list"], obj=dev, catch_exceptions=False)
rows = {line.split()[1]: line.split() for line in res.output.splitlines()[1:]}
for index, child in enumerate(dev.children):
row = rows[child.device_id]
assert row[0] == str(index)
assert row[3] == str(index != 0)
assert res.exit_code == 0
@plug_iot @plug_iot
async def test_non_hub(dev: Device, mocker: MockerFixture, runner: CliRunner) -> None: async def test_non_hub(dev: Device, mocker: MockerFixture, runner: CliRunner) -> None:
"""Test that hub commands return an error if executed on a non-hub.""" """Test that hub commands return an error if executed on a non-hub."""

View File

@@ -14,11 +14,17 @@ from freezegun.api import FrozenDateTimeFactory
from kasa import Credentials, Device, DeviceType, Module from kasa import Credentials, Device, DeviceType, Module
from kasa.credentials import DEFAULT_CREDENTIALS, get_default_credentials from kasa.credentials import DEFAULT_CREDENTIALS, get_default_credentials
from kasa.exceptions import AuthenticationError, DeviceError, KasaException from kasa.exceptions import (
from kasa.smartcam import SmartCamDevice AuthenticationError,
DeviceError,
DeviceUnavailableError,
KasaException,
)
from kasa.smartcam import SmartCamChild, SmartCamDevice
from kasa.smartcam.modules.time import Time from kasa.smartcam.modules.time import Time
from ..conftest import device_smartcam, hub_smartcam from ..conftest import device_smartcam, hub_smartcam
from ..fakeprotocol_smartcam import FakeSmartCamTransport
class _ChangeAdminPasswordPayload(TypedDict): class _ChangeAdminPasswordPayload(TypedDict):
@@ -96,6 +102,28 @@ async def test_hub(dev: Device) -> None:
assert child.device_id assert child.device_id
@hub_smartcam
@pytest.mark.requires_dummy
@pytest.mark.parametrize("online", [True, False])
async def test_camera_child_is_available(dev: Device, online: bool) -> None:
"""Test a camera child reports the reachability the hub reports for it."""
child = next((c for c in dev.children if isinstance(c, SmartCamChild)), None)
if child is None:
pytest.skip(f"Device {dev} fixture does not have camera children")
assert isinstance(dev.protocol._transport, FakeSmartCamTransport)
for info in dev.protocol._transport.info["getChildDeviceList"]["child_device_list"]:
if info["device_id"] == child.device_id:
info["online"] = online
await dev.update()
assert child.is_available is online
assert child.features["available"].value is online
if not online:
with pytest.raises(DeviceUnavailableError):
await child.set_alias("new")
@device_smartcam @device_smartcam
async def test_wifi_scan(dev: SmartCamDevice) -> None: async def test_wifi_scan(dev: SmartCamDevice) -> None:
fake_scan_data = { fake_scan_data = {

View File

@@ -1,11 +1,12 @@
import inspect import inspect
import logging
from datetime import UTC, datetime from datetime import UTC, datetime
import pytest import pytest
from freezegun.api import FrozenDateTimeFactory from freezegun.api import FrozenDateTimeFactory
from pytest_mock import MockerFixture from pytest_mock import MockerFixture
from kasa import Device from kasa import Device, DeviceUnavailableError
from kasa.device_type import DeviceType from kasa.device_type import DeviceType
from kasa.protocols.smartprotocol import SmartProtocol, _ChildProtocolWrapper from kasa.protocols.smartprotocol import SmartProtocol, _ChildProtocolWrapper
from kasa.smart.smartchilddevice import SmartChildDevice from kasa.smart.smartchilddevice import SmartChildDevice
@@ -18,6 +19,7 @@ from .conftest import (
strip_iot, strip_iot,
strip_smart, strip_smart,
) )
from .fakeprotocol_smart import FakeSmartTransport
has_children_smart = parametrize( has_children_smart = parametrize(
"has children", component_filter="control_child", protocol_filter={"SMART"} "has children", component_filter="control_child", protocol_filter={"SMART"}
@@ -126,6 +128,104 @@ async def test_parent_property(dev: Device) -> None:
assert child.parent == dev assert child.parent == dev
@hub_smart
@pytest.mark.requires_dummy
@pytest.mark.parametrize(
("status", "is_available"), [("online", True), ("offline", False)]
)
async def test_child_is_available(dev: Device, status: str, is_available: bool) -> None:
"""Test a hub child reports the reachability status from the hub."""
if not dev.children:
pytest.skip(f"Device {dev} fixture does not have any children")
assert isinstance(dev.protocol._transport, FakeSmartTransport)
child_list = dev.protocol._transport.info["get_child_device_list"]
child_list["child_device_list"][0]["status"] = status
await dev.update()
child = dev.children[0]
assert child.is_available is is_available
assert child.features["available"].value is is_available
assert dev.is_available is True
@hub_smart
@pytest.mark.requires_dummy
@pytest.mark.parametrize(
"write",
[
pytest.param(lambda child: child.set_alias("new"), id="set_alias"),
pytest.param(lambda child: child.reboot(), id="reboot"),
pytest.param(
lambda child: next(iter(child.modules.values())).call(
"set_device_info", {"a": 1}
),
id="module_call",
),
],
)
@pytest.mark.parametrize("removed", [False, True], ids=["offline", "removed"])
async def test_unavailable_child_refuses_writes(
dev: Device, write, removed: bool, mocker: MockerFixture
) -> None:
"""Test writes to an unreachable hub child fail without contacting the hub."""
if not dev.children:
pytest.skip(f"Device {dev} fixture does not have any children")
child = dev.children[0]
assert isinstance(child, SmartChildDevice)
assert isinstance(dev.protocol._transport, FakeSmartTransport)
info = dev.protocol._transport.info
child_list = info["get_child_device_list"]
if removed:
child_list["child_device_list"].pop(0)
child_list["sum"] -= 1
info["get_child_device_component_list"]["child_component_list"] = [
c
for c in info["get_child_device_component_list"]["child_component_list"]
if c["device_id"] != child.device_id
]
else:
child_list["child_device_list"][0]["status"] = "offline"
await dev.update()
query = mocker.spy(dev.protocol, "query")
assert child.is_available is False
with pytest.raises(DeviceUnavailableError):
await write(child)
query.assert_not_called()
await child._query_helper("get_device_info")
query.assert_called_once()
@hub_smart
@pytest.mark.requires_dummy
@pytest.mark.xdist_group(name="caplog")
async def test_child_availability_change_logged_once(
dev: Device, caplog: pytest.LogCaptureFixture
) -> None:
"""Test going offline and coming back are each logged once."""
if not dev.children:
pytest.skip(f"Device {dev} fixture does not have any children")
assert isinstance(dev.protocol._transport, FakeSmartTransport)
child_info = dev.protocol._transport.info["get_child_device_list"]
child_id = child_info["child_device_list"][0]["device_id"]
caplog.set_level(logging.DEBUG)
child_info["child_device_list"][0]["status"] = "offline"
await dev.update()
await dev.update()
assert caplog.text.count(f"({child_id}) available: False") == 1
child_info["child_device_list"][0]["status"] = "online"
await dev.update()
await dev.update()
assert caplog.text.count(f"({child_id}) available: True") == 1
@has_children_smart @has_children_smart
@pytest.mark.requires_dummy @pytest.mark.requires_dummy
async def test_child_time(dev: Device, freezer: FrozenDateTimeFactory) -> None: async def test_child_time(dev: Device, freezer: FrozenDateTimeFactory) -> None: