8000 Improve feature setter robustness (#870) · python-kasa/python-kasa@5ef81f4 · GitHub
[go: up one dir, main page]
Skip to content

Commit 5ef81f4

Browse files
rytilahtirytilahti
authored
Improve feature setter robustness (#870)
This adds a test to check that all feature.set_value() calls will cause a query, i.e., that there are no self.call()s that are not awaited, and fixes existing code in this context. This also fixes an issue where it was not possible to print out the feature if the value threw an exception.
1 parent 9dcd8ec commit 5ef81f4

File tree

8 files changed

+90
-18
lines changed

8 files changed

+90
-18
lines changed

‎kasa/feature.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,9 +172,14 @@ async def set_value(self, value):
172172
return await getattr(container, self.attribute_setter)(value)
173173

174174
def __repr__(self):
175-
value = self.value
175+
try:
176+
value = self.value
177+
except Exception as ex:
178+
return f"Unable to read value ({self.id}): {ex}"
179+
176180
if self.precision_hint is not None and value is not None:
177181
value = round(self.value, self.precision_hint)
182+
178183
s = f"{self.name} ({self.id}): {value}"
179184
if self.unit is not None:
180185
s += f" {self.unit}"

‎kasa/iot/iotbulb.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@ async def _initialize_features(self):
233233
attribute_setter="set_color_temp",
234234
range_getter="valid_temperature_range",
235235
category=Feature.Category.Primary,
236+
type=Feature.Type.Number,
236237
)
237238
)
238239

‎kasa/smart/modules/autooffmodule.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,9 @@ def enabled(self) -> bool:
5555
"""Return True if enabled."""
5656
return self.data["enable"]
5757

58-
def set_enabled(self, enable: bool):
58+
async def set_enabled(self, enable: bool):
5959
"""Enable/disable auto off."""
60-
return self.call(
60+
return await self.call(
6161
"set_auto_off_config",
6262
{"enable": enable, "delay_min": self.data["delay_min"]},
6363
)
@@ -67,9 +67,9 @@ def delay(self) -> int:
6767
"""Return time until auto off."""
6868
return self.data["delay_min"]
6969

70-
def set_delay(self, delay: int):
70+
async def set_delay(self, delay: int):
7171
"""Set time until auto off."""
72-
return self.call(
72+
return await self.call(
7373
"set_auto_off_config", {"delay_min": delay, "enable": self.data["enable"]}
7474
)
7575

‎kasa/smart/modules/colortemp.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ def __init__(self, device: SmartDevice, module: str):
3434
attribute_setter="set_color_temp",
3535
range_getter="valid_temperature_range",
3636
category=Feature.Category.Primary,
37+
type=Feature.Type.Number,
3738
)
3839
)
3940

‎kasa/smart/modules/lighttransitionmodule.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,9 @@ def _turn_off(self):
8888

8989
return self.data["off_state"]
9090

91-
def set_enabled_v1(self, enable: bool):
91+
async def set_enabled_v1(self, enable: bool):
9292
"""Enable gradual on/off."""
93-
return self.call("set_on_off_gradually_info", {"enable": enable})
93+
return await self.call("set_on_off_gradually_info", {"enable": enable})
9494

9595
@property
9696
def enabled_v1(self) -> bool:

‎kasa/smart/modules/temperature.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ def __init__(self, device: SmartDevice, module: str):
4848
attribute_getter="temperature_unit",
4949
attribute_setter="set_temperature_unit",
5050
type=Feature.Type.Choice,
51+
choices=["celsius", "fahrenheit"],
5152
)
5253
)
5354
# TODO: use temperature_unit for feature creation

‎kasa/tests/device_fixtures.py

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
from __future__ import annotations
22

3+
from typing F438 import AsyncGenerator
4+
35
import pytest
46

57
from kasa import (
@@ -346,13 +348,13 @@ def device_for_fixture_name(model, protocol):
346348
raise Exception("Unable to find type for %s", model)
347349

348350

349-
async def _update_and_close(d):
351+
async def _update_and_close(d) -> Device:
350352
await d.update()
351353
await d.protocol.close()
352354
return d
353355

354356

355-
async def _discover_update_and_close(ip, username, password):
357+
async def _discover_update_and_close(ip, username, password) -> Device:
356358
if username and password:
357359
credentials = Credentials(username=username, password=password)
358360
else:
@@ -361,7 +363,7 @@ async def _discover_update_and_close(ip, username, password):
361363
return await _update_and_close(d)
362364

363365

364-
async def get_device_for_fixture(fixture_data: FixtureInfo):
366+
async def get_device_for_fixture(fixture_data: FixtureInfo) -> Device:
365367
# if the wanted file is not an absolute path, prepend the fixtures directory
366368

367369
d = device_for_fixture_name(fixture_data.name, fixture_data.protocol)(
@@ -395,13 +397,14 @@ async def get_device_for_fixture_protocol(fixture, protocol):
395397

396398

397399
@pytest.fixture(params=filter_fixtures("main devices"), ids=idgenerator)
398-
async def dev(request):
400+
async def dev(request) -> AsyncGenerator[Device, None]:
399401
"""Device fixture.
400402
401403
Provides a device (given --ip) or parametrized fixture for the supported devices.
402404
The initial update is called automatically before returning the device.
403405
"""
404406
fixture_data: FixtureInfo = request.param
407+
dev: Device
405408

406409
ip = request.config.getoption("--ip")
407410
username = request.config.getoption("--username")
@@ -412,13 +415,12 @@ async def dev(request):
412415
if not model:
413416
d = await _discover_update_and_close(ip, username, password)
414417
IP_MODEL_CACHE[ip] = model = d.model
418+
415419
if model not in fixture_data.name:
416420
pytest.skip(f"skipping file {fixture_data.name}")
417-
dev: Device = (
418-
d if d else await _discover_update_and_close(ip, username, password)
419-
)
421+
dev = d if d else await _discover_update_and_close(ip, username, password)
420422
else:
421-
dev: Device = await get_device_for_fixture(fixture_data)
423+
dev = await get_device_for_fixture(fixture_data)
422424

423425
yield dev
424426

‎kasa/tests/test_feature.py

Lines changed: 65 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
1+
import logging
2+
import sys
3+
14
import pytest
2-
from pytest_mock import MockFixture
5+
from pytest_mock import MockerFixture
6+
7+
from kasa import Device, Feature, KasaException
38

4-
from kasa import Feature
9+
_LOGGER = logging.getLogger(__name__)
510

611

712
class DummyDevice:
@@ -111,7 +116,7 @@ async def test_feature_action(mocker):
111116
mock_call_action.assert_called()
112117

113118

114-
async def test_feature_choice_list(dummy_feature, caplog, mocker: MockFixture):
119+
async def test_feature_choice_list(dummy_feature, caplog, mocker: MockerFixture):
115120
"""Test the choice feature type."""
116121
dummy_feature.type = Feature.Type.Choice
117122
dummy_feature.choices = ["first", "second"]
@@ -138,3 +143,60 @@ async def test_precision_hint(dummy_feature, precision_hint):
138143
dummy_feature.attribute_getter = lambda x: dummy_value
139144
assert dummy_feature.value == dummy_value
140145
assert f"{round(dummy_value, precision_hint)} dummyunit" in repr(dummy_feature)
146+
147+
148+
@pytest.mark.skipif(
149+
sys.version_info < (3, 11),
150+
reason="exceptiongroup requires python3.11+",
151+
)
152+
async def test_feature_setters(dev: Device, mocker: MockerFixture):
153+
"""Test that all feature setters query something."""
154+
155+
async def _test_feature(feat, query_mock):
156+
if feat.attribute_setter is None:
157+
return
158+
159+
expecting_call = True
160+
161+
if feat.type == Feature.Type.Number:
162+
await feat.set_value(feat.minimum_value)
163+
elif feat.type == Feature.Type.Switch:
164+
await feat.set_value(True)
165+
elif feat.type == Feature.Type.Action:
166+
await feat.set_value("dummyvalue")
167+
elif feat.type == Feature.Type.Choice:
168+
await feat.set_value(feat.choices[0])
169+
elif feat.type == Feature.Type.Unknown:
170+
_LOGGER.warning("Feature '%s' has no type, cannot test the setter", feat)
171+
expecting_call = False
172+
else:
173+
raise NotImplementedError(f"set_value not implemented for {feat.type}")
174+
175+
if expecting_call:
176+
query_mock.assert_called()
177+
178+
async def _test_features(dev):
179+
exceptions = []
180+
query = mocker.patch.object(dev.protocol, "query")
181+
for feat in dev.features.values():
182+
query.reset_mock()
183+
try:
184+
await _test_feature(feat, query)
185+
# we allow our own exceptions to avoid mocking valid responses
186+
except KasaException:
187+
pass
188+
except Exception as ex:
189+
ex.add_note(f"Exception when trying to set {feat} on {dev}")
190+
exceptions.append(ex)
191+
192+
return exceptions
193+
194+
exceptions = await _test_features(dev)
195+
196+
for child in dev.children:
197+
exceptions.extend(await _test_features(child))
198+
199+
if exceptions:
200+
raise ExceptionGroup(
201+
"Got exceptions while testing attribute_setters", exceptions
202+
)

0 commit comments

Comments
 (0)
0