8000 Remove Custom Parsers · python-kasa/python-kasa@45abfec · GitHub
[go: up one dir, main page]

Skip to content

Commit 45abfec

Browse files
committed
Remove Custom Parsers
- Fix: Back out custom value parser logic. - Fix: Remove overloaded range set functionality. - Fix: Improve error messages when user forgets quotes around cli arguments.
1 parent 9776e38 commit 45abfec

File tree

5 files changed

+85
-128
lines changed

5 files changed

+85
-128
lines changed

kasa/cli/feature.py

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,7 @@
66

77
import asyncclick as click
88

9-
from kasa import (
10-
Device,
11-
Feature,
12-
)
9+
from kasa import Device, Feature
1310

1411
from .common import (
1512
echo,
@@ -133,7 +130,22 @@ async def feature(
133130
echo(f"{feat.name} ({name}): {feat.value}{unit}")
134131
return feat.value
135132

136-
value = feat.parse_value(value, ast.literal_eval)
133+
try:
134+
# Attempt to parse as python literal.
135+
value = ast.literal_eval(value)
136+
except ValueError:
137+
# The value is probably an unquoted string, so we'll raise an error,
138+
# and tell the user to quote the string.
139+
raise click.exceptions.BadParameter(
140+
f'{repr(value)} for {name} (Perhaps you forgot to "quote" the value?)'
141+
) from SyntaxError
142+
except SyntaxError:
143+
# There are likely miss-matched quotes or odd characters in the input,
144+
# so abort and complain to the user.
145+
raise click.exceptions.BadParameter(
146+
f"{repr(value)} for {name}"
147+
) from SyntaxError
148+
137149
echo(f"Changing {name} from {feat.value} to {value}")
138150
response = await dev.features[name].set_value(value)
139151
await dev.update()

kasa/feature.py

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -163,9 +163,6 @@ class Category(Enum):
163163
#: If set, this property will be used to get *choices*.
164164
choices_getter: str | Callable[[], list[str]] | None = None
165165

166-
#: Value converter, for when working with complex types.
167-
value_parser: str | None = None
168-
169166
def __post_init__(self) -> None:
170167
"""Handle late-binding of members."""
171168
# Populate minimum & maximum values, if range_getter is given
@@ -258,7 +255,7 @@ async def set_value(self, value: int | float | bool | str | Enum | None) -> Any:
258255
elif self.type == Feature.Type.Choice: # noqa: SIM102
259256
if not self.choices or value not in self.choices:
260257
raise ValueError(
261-
f"Unexpected value for {self.name}: {value}"
258+
f"Unexpected value for {self.name}: '{value}'"
262259
f" - allowed: {self.choices}"
263260
)
264261

@@ -273,26 +270,6 @@ async def set_value(self, value: int | float | bool | str | Enum | None) -> Any:
273270

274271
return await attribute_setter(value)
275272

276-
def parse_value(
277-
self, value: str, fallback: Callable[[str], Any | None] = lambda x: None
278-
) -> Any | None:
279-
"""Attempt to parse a given string into a value accepted by this feature."""
280-
parser = self._get_property_value(self.value_parser)
281-
parser = parser if parser else fallback
282-
allowed = f"{self.choices}" if self.choices else "Unknown"
283-
try:
284-
parsed = parser(value)
285-
if parsed is None:
286-
raise ValueError(
287-
f"Unexpected value for {self.name}: {value}"
288-
f" - allowed: {allowed}"
289-
)
290-
return parsed
291-
except SyntaxError as se:
292-
raise ValueError(
293-
f"{se.msg} for {self.name}: {value}" f" - allowed: {allowed}",
294-
) from se
295-
296273
def __repr__(self) -> str:
297274
try:
298275
value = self.value

kasa/iot/modules/motion.py

Lines changed: 36 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
import logging
66
import math
77
from enum import Enum
8-
from typing import Literal, overload
98

109
from ...exceptions import KasaException
1110
from ...feature import Feature
@@ -69,7 +68,6 @@ def _initialize_features(self) -> None:
6968
attribute_setter="_set_range_cli",
7069
type=Feature.Type.Choice,
7170
choices_getter="ranges",
72-
value_parser="parse_range_value",
7371
category=Feature.Category.Config,
7472
)
7573
)
@@ -108,7 +106,7 @@ def _initialize_features(self) -> None:
108106
device=self._device,
109107
container=self,
110108
id="pir_value",
111-
name="PIR Reading",
109+
name="PIR Value",
112110
icon="mdi:motion-sensor",
113111
attribute_getter="pir_value",
114112
attribute_setter=None,
@@ -117,21 +115,6 @@ def _initialize_features(self) -> None:
117115
)
118116
)
119117

120-
self._add_feature(
121-
Feature(
122-
device=self._device,
123-
container=self,
124-
id="pir_percent",
125-
name="PIR Percentage",
126-
icon="mdi:motion-sensor",
127-
attribute_getter="pir_percent",
128-
attribute_setter=None,
129-
type=Feature.Type.Sensor,
130-
category=Feature.Category.Info,
131-
unit_getter=lambda: "%",
132-
)
133-
)
134-
135118
self._add_feature(
136119
Feature(
137120
device=self._device,
@@ -188,6 +171,21 @@ def _initialize_features(self) -> None:
188171
)
189172
)
190173

174+
self._add_feature(
175+
Feature(
176+
device=self._device,
177+
container=self,
178+
id="pir_percent",
179+
name="PIR Percentile",
180+
icon="mdi:motion-sensor",
181+
attribute_getter="pir_percent",
182+
attribute_setter=None,
183+
type=Feature.Type.Sensor,
184+
category=Feature.Category.Debug,
185+
unit_getter=lambda: "%",
186+
)
187+
)
188+
191189
def query(self) -> dict:
192190
"""Request PIR configuration."""
193191
req = merge(
@@ -233,79 +231,43 @@ async def set_enabled(self, state: bool) -> dict:
233231
"""Enable/disable PIR."""
234232
return await self.call("set_enable", {"enable": int(state)})
235233

236-
def _parse_range_value(self, value: str) -> int | Range | None:
237-
"""Attempt to parse a range value from the given string."""
238-
_LOGGER.debug("Parse Range Value: %s", value)
239-
parsed: int | Range | None = None
240-
try:
241-
parsed = int(value)
242-
_LOGGER.debug("Parse Range Value: %s is an integer.", value)
243-
return parsed
244-
except ValueError:
245-
_LOGGER.debug("Parse Range Value: %s is not an integer.", value)
246-
value = value.strip().upper()
247-
if value in Range._member_names_:
248-
_LOGGER.debug("Parse Range Value: %s is an enumeration.", value)
249-
parsed = Range[value]
250-
return parsed
251-
_LOGGER.debug("Parse Range Value: %s is not a Range Value.", value)
252-
return None
253-
254234
@property
255-
def ranges(self) -> list[Range]:
235+
def ranges(self) -> list[str]:
256236
"""Return set of supported range classes."""
257237
range_min = 0
258238
range_max = len(self.config["array"])
259239
valid_ranges = list()
260240
for r in Range:
261241
if (r.value >= range_min) and (r.value < range_max):
262-
valid_ranges.append(r)
242+
valid_ranges.append(r.name)
263243
return valid_ranges
264244

265245
@property
266246
def range(self) -> Range:
267247
"""Return motion detection Range."""
268248
return Range(self.config["trigger_index"])
269249

270-
@overload
271-
async def set_range(self, *, range: Range) -> dict: ...
272-
273-
@overload
274-
async def set_range(self, *, range: Literal[Range.Custom], value: int) -> dict: ...
275-
276-
@overload
277-
async def set_range(self, *, value: int) -> dict: ...
278-
279-
async def set_range(
280-
self, *, range: Range | None = None, value: int | None = None
281-
) -> dict:
250+
async def set_range(self, range: Range) -> dict:
282251
"""Set the Range for the sensor.
283252
284-
:param Range: for using standard Ranges
285-
:param custom_Range: Range in decimeters, overrides the Range parameter
253+
:param Range: the range class to use.
286254
"""
287-
if value is not None:
288-
if range is not None and range is not Range.Custom:
289-
raise KasaException(
290-
f"Refusing to set non-custom range {range} to value {value}."
291-
)
292-
elif value is None:
293-
raise KasaException("Custom range threshold may not be set to None.")
294-
payload = {"index": Range.Custom.value, "value": value}
295-
elif range is not None:
296-
payload = {"index": range.value}
297-
else:
298-
raise KasaException("Either range or value needs to be defined")
299-
255+
payload = {"index": range.value}
300256
return await self.call("set_trigger_sens", payload)
301257

302-
async def _set_range_cli(self, input: Range | int) -> dict:
303-
if isinstance(input, Range):
304-
return await self.set_range(range=input)
305-
elif isinstance(input, int):
306-
return await self.set_range(value=input)
307-
else:
308-
raise KasaException(f"Invalid type: {type(input)} given to cli motion set.")
258+
def _parse_range_value(self, value: str) -> Range:
259+
"""Attempt to parse a range value from the given string."""
260+
value = value.strip().capitalize()
261+
if value not in Range._member_names_:
262+
raise KasaException(
263+
f"Invalid range value: '{value}'."
264+
f" Valid options are: {Range._member_names_}"
265+
)
266+
return Range[value]
267+
268+
async def _set_range_cli(self, input: str) -> dict:
269+
value = self._parse_range_value(input)
270+
return await self.set_range(range=value)
309271

310272
def get_range_threshold(self, range_type: Range) -> int:
311273
"""Get the distance threshold at which the PIR sensor is will trigger."""
@@ -322,7 +284,8 @@ def threshold(self) -> int:
322284

323285
async def set_threshold(self, value: int) -> dict:
324286
"""Set the distance threshold at which the PIR sensor is will trigger."""
325-
return await self.set_range(value=value)
287+
payload = {"index": Range.Custom.value, "value": value}
288+
return await self.call("set_trigger_sens", payload)
326289

327290
@property
328291
def inactivity_timeout(self) -> int:

tests/iot/modules/test_motion.py

Lines changed: 27 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
1-
import pytest
21
from pytest_mock import MockerFixture
32

43
from kasa import Module
5-
from kasa.exceptions import KasaException
64
from kasa.iot import IotDimmer
75
from kasa.iot.modules.motion import Motion, Range
86

@@ -38,32 +36,36 @@ async def test_motion_range(dev: IotDimmer, mocker: MockerFixture):
3836
motion: Motion = dev.modules[Module.IotMotion]
3937
query_helper = mocker.patch("kasa.iot.IotDimmer._query_helper")
4038

41-
await motion.set_range(value=123)
42-
query_helper.assert_called_with(
43-
"smartlife.iot.PIR",
44-
"set_trigger_sens",
45-
{"index": Range.Custom.value, "value": 123},
46-
)
47-
48-
await motion.set_range(range=Range.Custom, value=123)
49-
query_helper.assert_called_with(
50-
"smartlife.iot.PIR",
51-
"set_trigger_sens",
52-
{"index": Range.Custom.value, "value": 123},
53-
)
39+
for range in Range:
40+
await motion.set_range(range)
41+
query_helper.assert_called_with(
42+
"smartlife.iot.PIR",
43+
"set_trigger_sens",
44+
{"index": range.value},
45+
)
5446

55-
await motion.set_range(range=Range.Far)
56-
query_helper.assert_called_with(
57-
"smartlife.iot.PIR", "set_trigger_sens", {"index": Range.Far.value}
58-
)
5947

60-
with pytest.raises(KasaException, match="Refusing to set non-custom range"):
61-
await motion.set_range(range=Range.Near, value=100) # type: ignore[call-overload]
48+
@dimmer_iot
49+
async def test_motion_threshold(dev: IotDimmer, mocker: MockerFixture):
50+
motion: Motion = dev.modules[Module.IotMotion]
51+
query_helper = mocker.patch("kasa.iot.IotDimmer._query_helper")
6252

63-
with pytest.raises(
64-
KasaException, match="Either range or value needs to be defined"
65-
):
66-
await motion.set_range() # type: ignore[call-overload]
53+
for range in Range:
54+
# Switch to a given range.
55+
await motion.set_range(range)
56+
query_helper.assert_called_with(
57+
"smartlife.iot.PIR",
58+
"set_trigger_sens",
59+
{"index": range.value},
60+
)
61+
62+
# Assert that the range always goes to custom, regardless of current range.
63+
await motion.set_threshold(123)
64+
query_helper.assert_called_with(
65+
"smartlife.iot.PIR",
66+
"set_trigger_sens",
67+
{"index": Range.Custom.value, "value": 123},
68+
)
6769

6870

6971
@dimmer_iot

tests/test_feature.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,10 @@ async def test_feature_choice_list(dummy_feature, caplog, mocker: MockerFixture)
140140
mock_setter.assert_called_with("first")
141141
mock_setter.reset_mock()
142142

143-
with pytest.raises(ValueError, match="Unexpected value for dummy_feature: invalid"): # noqa: PT012
143+
with pytest.raises( # noqa: PT012
144+
ValueError,
145+
match="Unexpected value for dummy_feature: 'invalid' (?: - allowed: .*)?",
146+
):
144147
await dummy_feature.set_value("invalid")
145148
assert "Unexpected value" in caplog.text
146149

0 commit comments

Comments
 (0)
0