From 494109ce6fcf7d7940bb0df889161d0c36b06848 Mon Sep 17 00:00:00 2001 From: Teemu Rytilahti Date: Thu, 1 Feb 2024 00:42:05 +0100 Subject: [PATCH 1/2] Avoid crashing on childdevice property accesses --- kasa/emeterstatus.py | 7 ++++-- kasa/tapo/childdevice.py | 33 ++++++++++++++++----------- kasa/tapo/tapodevice.py | 8 +++++-- kasa/tests/test_childdevice.py | 41 ++++++++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 17 deletions(-) diff --git a/kasa/emeterstatus.py b/kasa/emeterstatus.py index 48d6e2410..9d3b3b571 100644 --- a/kasa/emeterstatus.py +++ b/kasa/emeterstatus.py @@ -79,8 +79,11 @@ def __getitem__(self, item): return super().__getitem__(item[: item.find("_")]) * 1000 else: # downscale for i in super().keys(): # noqa: SIM118 - if i.startswith(item): - return self.__getitem__(i) / 1000 + if ( + i.startswith(item) + and (value := self.__getitem__(i)) is not None + ): + return value / 1000 _LOGGER.debug(f"Unable to find value for '{item}'") return None diff --git a/kasa/tapo/childdevice.py b/kasa/tapo/childdevice.py index 7b66a79d3..5279983fb 100644 --- a/kasa/tapo/childdevice.py +++ b/kasa/tapo/childdevice.py @@ -1,6 +1,7 @@ """Child device implementation.""" from typing import Dict, Optional +from ..device_type import DeviceType from ..deviceconfig import DeviceConfig from ..exceptions import SmartDeviceException from ..smartprotocol import SmartProtocol, _ChildProtocolWrapper @@ -24,21 +25,27 @@ def __init__( self._parent = parent self._id = child_id self.protocol = _ChildProtocolWrapper(child_id, parent.protocol) + # TODO: remove the assignment after modularization is done, + # currently required to allow accessing time-related properties + self._time = parent._time + # TODO: cleanup the _last_update, _sys_info, _info, _data mess. + self._last_update = ( + self._sys_info + ) = self._info = self._data = self._get_child_info() + self._device_type = DeviceType.Plug + + def _get_child_info(self) -> Dict: + """Return the device specific information from the parent.""" + for child in self._parent._last_update["child_info"]["child_device_list"]: + if child["device_id"] == self._id: + return child + + raise SmartDeviceException(f"Unable to find child device with id {self._id}") async def update(self, update_children: bool = True): - """We just set the info here accordingly.""" - - def _get_child_info() -> Dict: - """Return the subdevice information for this device.""" - for child in self._parent._last_update["child_info"]["child_device_list"]: - if child["device_id"] == self._id: - return child - - raise SmartDeviceException( - f"Unable to find child device with id {self._id}" - ) - - self._last_update = self._sys_info = self._info = _get_child_info() + """Update the latest information from the parent.""" + # TODO: cleanup the _last_update, _sys_info, _info, _data mess. + self._last_update = self._sys_info = self._info = self._get_child_info() def __repr__(self): return f"" diff --git a/kasa/tapo/tapodevice.py b/kasa/tapo/tapodevice.py index a7e57a6d1..06fe8d533 100644 --- a/kasa/tapo/tapodevice.py +++ b/kasa/tapo/tapodevice.py @@ -32,7 +32,8 @@ def __init__( super().__init__(host=host, config=config, protocol=_protocol) self.protocol: SmartProtocol self._components_raw: Optional[Dict[str, Any]] = None - self._components: Dict[str, int] + self._components: Dict[str, int] = {} + self._energy: Dict[str, Any] = {} self._state_information: Dict[str, Any] = {} async def _initialize_children(self): @@ -204,10 +205,13 @@ async def _query_helper( @property def state_information(self) -> Dict[str, Any]: """Return the key state information.""" + ssid = self._info.get("ssid") + ssid = base64.b64decode(ssid).decode() if ssid else "No SSID" + return { "overheated": self._info.get("overheated"), "signal_level": self._info.get("signal_level"), - "SSID": base64.b64decode(str(self._info.get("ssid"))).decode(), + "SSID": ssid, } @property diff --git a/kasa/tests/test_childdevice.py b/kasa/tests/test_childdevice.py index 986f77b65..8ec9b9a18 100644 --- a/kasa/tests/test_childdevice.py +++ b/kasa/tests/test_childdevice.py @@ -1,4 +1,10 @@ +import inspect +import sys + +import pytest + from kasa.smartprotocol import _ChildProtocolWrapper +from kasa.tapo.childdevice import ChildDevice from .conftest import strip_smart @@ -28,3 +34,38 @@ async def test_childdevice_update(dev, dummy_protocol, mocker): assert dev._last_update != first._last_update assert dev._last_update["child_info"]["child_device_list"][0] == first._last_update + + +@strip_smart +@pytest.mark.skipif( + sys.version_info < (3, 11), + reason="exceptiongroup requires python3.11+", +) +async def test_childdevice_properties(dev: ChildDevice): + """Check that accessing childdevice properties do not raise exceptions.""" + assert len(dev.children) > 0 + + first = dev.children[0] + assert first.is_plug + + # children do not have children + assert not first.children + + def _test_property_getters(): + """Try accessing all properties and return a list of encountered exceptions.""" + exceptions = [] + properties = inspect.getmembers( + first.__class__, lambda o: isinstance(o, property) + ) + for prop in properties: + name, _ = prop + try: + _ = getattr(first, name) + except Exception as ex: + exceptions.append(ex) + + return exceptions + + exceptions = list(_test_property_getters()) + if exceptions: + raise ExceptionGroup("Accessing child properties caused exceptions", exceptions) From 773c0cdc3a35411854e64e501feed5a3f519763d Mon Sep 17 00:00:00 2001 From: Teemu Rytilahti Date: Thu, 1 Feb 2024 19:11:08 +0100 Subject: [PATCH 2/2] Push updates from parent to child --- kasa/tapo/childdevice.py | 24 ++++++------------- kasa/tapo/tapodevice.py | 42 ++++++++++++++++++++++++++-------- kasa/tests/test_childdevice.py | 11 +++++---- 3 files changed, 45 insertions(+), 32 deletions(-) diff --git a/kasa/tapo/childdevice.py b/kasa/tapo/childdevice.py index 5279983fb..43b748515 100644 --- a/kasa/tapo/childdevice.py +++ b/kasa/tapo/childdevice.py @@ -1,9 +1,8 @@ """Child device implementation.""" -from typing import Dict, Optional +from typing import Optional from ..device_type import DeviceType from ..deviceconfig import DeviceConfig -from ..exceptions import SmartDeviceException from ..smartprotocol import SmartProtocol, _ChildProtocolWrapper from .tapodevice import TapoDevice @@ -28,24 +27,15 @@ def __init__( # TODO: remove the assignment after modularization is done, # currently required to allow accessing time-related properties self._time = parent._time - # TODO: cleanup the _last_update, _sys_info, _info, _data mess. - self._last_update = ( - self._sys_info - ) = self._info = self._data = self._get_child_info() - self._device_type = DeviceType.Plug - - def _get_child_info(self) -> Dict: - """Return the device specific information from the parent.""" - for child in self._parent._last_update["child_info"]["child_device_list"]: - if child["device_id"] == self._id: - return child - - raise SmartDeviceException(f"Unable to find child device with id {self._id}") + self._device_type = DeviceType.StripSocket async def update(self, update_children: bool = True): - """Update the latest information from the parent.""" + """Noop update. The parent updates our internals.""" + + def update_internal_state(self, info): + """Set internal state for the child.""" # TODO: cleanup the _last_update, _sys_info, _info, _data mess. - self._last_update = self._sys_info = self._info = self._get_child_info() + self._last_update = self._sys_info = self._info = info def __repr__(self): return f"" diff --git a/kasa/tapo/tapodevice.py b/kasa/tapo/tapodevice.py index 06fe8d533..0ef28d071 100644 --- a/kasa/tapo/tapodevice.py +++ b/kasa/tapo/tapodevice.py @@ -2,7 +2,7 @@ import base64 import logging from datetime import datetime, timedelta, timezone -from typing import Any, Dict, List, Optional, Set, cast +from typing import TYPE_CHECKING, Any, Dict, List, Optional, Set, cast from ..aestransport import AesTransport from ..device_type import DeviceType @@ -15,6 +15,9 @@ _LOGGER = logging.getLogger(__name__) +if TYPE_CHECKING: + from .childdevice import ChildDevice + class TapoDevice(SmartDevice): """Base class to represent a TAPO device.""" @@ -33,20 +36,39 @@ def __init__( self.protocol: SmartProtocol self._components_raw: Optional[Dict[str, Any]] = None self._components: Dict[str, int] = {} + self._children: Dict[str, "ChildDevice"] = {} self._energy: Dict[str, Any] = {} self._state_information: Dict[str, Any] = {} async def _initialize_children(self): + """Initialize children for power strips.""" children = self._last_update["child_info"]["child_device_list"] # TODO: Use the type information to construct children, # as hubs can also have them. from .childdevice import ChildDevice - self.children = [ - ChildDevice(parent=self, child_id=child["device_id"]) for child in children - ] + self._children = { + child["device_id"]: ChildDevice(parent=self, child_id=child["device_id"]) + for child in children + } self._device_type = DeviceType.Strip + @property + def children(self): + """Return list of children. + + This is just to keep the existing SmartDevice API intact. + """ + return list(self._children.values()) + + @children.setter + def children(self, children): + """Initialize from a list of children. + + This is just to keep the existing SmartDevice API intact. + """ + self._children = {child["device_id"]: child for child in children} + async def update(self, update_children: bool = True): """Update the device.""" if self.credentials is None and self.credentials_hash is None: @@ -89,7 +111,7 @@ async def update(self, update_children: bool = True): self._energy = resp.get("get_energy_usage", {}) self._emeter = resp.get("get_current_power", {}) - self._last_update = self._data = { + self._last_update = { "components": self._components_raw, "info": self._info, "usage": self._usage, @@ -99,13 +121,13 @@ async def update(self, update_children: bool = True): "child_info": resp.get("get_child_device_list", {}), } - if self._last_update["child_info"]: + if child_info := self._last_update.get("child_info"): if not self.children: await self._initialize_children() - for child in self.children: - await child.update() + for info in child_info["child_device_list"]: + self._children[info["device_id"]].update_internal_state(info) - _LOGGER.debug("Got an update: %s", self._data) + _LOGGER.debug("Got an update: %s", self._last_update) async def _initialize_modules(self): """Initialize modules based on component negotiation response.""" @@ -193,7 +215,7 @@ def device_id(self) -> str: @property def internal_state(self) -> Any: """Return all the internal state data.""" - return self._data + return self._last_update async def _query_helper( self, target: str, cmd: str, arg: Optional[Dict] = None, child_ids=None diff --git a/kasa/tests/test_childdevice.py b/kasa/tests/test_childdevice.py index 8ec9b9a18..077a1f2dd 100644 --- a/kasa/tests/test_childdevice.py +++ b/kasa/tests/test_childdevice.py @@ -25,15 +25,16 @@ def test_childdevice_init(dev, dummy_protocol, mocker): @strip_smart async def test_childdevice_update(dev, dummy_protocol, mocker): """Test that parent update updates children.""" - assert len(dev.children) > 0 + child_info = dev._last_update["child_info"] + child_list = child_info["child_device_list"] + + assert len(dev.children) == child_info["sum"] first = dev.children[0] - child_update = mocker.patch.object(first, "update") await dev.update() - child_update.assert_called() assert dev._last_update != first._last_update - assert dev._last_update["child_info"]["child_device_list"][0] == first._last_update + assert child_list[0] == first._last_update @strip_smart @@ -46,7 +47,7 @@ async def test_childdevice_properties(dev: ChildDevice): assert len(dev.children) > 0 first = dev.children[0] - assert first.is_plug + assert first.is_strip_socket # children do not have children assert not first.children