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.

kasa/cli/feature.py

@@ -6,10 +6,7 @@ import ast
 
 import asyncclick as click
 
-from kasa import (
-    Device,
-    Feature,
-)
+from kasa import Device, Feature
 
 from .common import (
     echo,
@@ -133,7 +130,22 @@ async def feature(
         echo(f"{feat.name} ({name}): {feat.value}{unit}")
         return feat.value
 
-    value = feat.parse_value(value, ast.literal_eval)
+    try:
+        # Attempt to parse as python literal.
+        value = ast.literal_eval(value)
+    except ValueError:
+        # The value is probably an unquoted string, so we'll raise an error,
+        # and tell the user to quote the string.
+        raise click.exceptions.BadParameter(
+            f'{repr(value)} for {name} (Perhaps you forgot to "quote" the value?)'
+        ) from SyntaxError
+    except SyntaxError:
+        # There are likely miss-matched quotes or odd characters in the input,
+        # so abort and complain to the user.
+        raise click.exceptions.BadParameter(
+            f"{repr(value)} for {name}"
+        ) from SyntaxError
+
     echo(f"Changing {name} from {feat.value} to {value}")
     response = await dev.features[name].set_value(value)
     await dev.update()

kasa/feature.py

@@ -163,9 +163,6 @@ class Feature:
     #: If set, this property will be used to get *choices*.
     choices_getter: str | Callable[[], list[str]] | None = None
 
-    #: Value converter, for when working with complex types.
-    value_parser: str | None = None
-
     def __post_init__(self) -> None:
         """Handle late-binding of members."""
         # Populate minimum & maximum values, if range_getter is given
@@ -258,7 +255,7 @@ class Feature:
         elif self.type == Feature.Type.Choice:  # noqa: SIM102
             if not self.choices or value not in self.choices:
                 raise ValueError(
-                    f"Unexpected value for {self.name}: {value}"
+                    f"Unexpected value for {self.name}: '{value}'"
                     f" - allowed: {self.choices}"
                 )
 
@@ -273,26 +270,6 @@ class Feature:
 
         return await attribute_setter(value)
 
-    def parse_value(
-        self, value: str, fallback: Callable[[str], Any | None] = lambda x: None
-    ) -> Any | None:
-        """Attempt to parse a given string into a value accepted by this feature."""
-        parser = self._get_property_value(self.value_parser)
-        parser = parser if parser else fallback
-        allowed = f"{self.choices}" if self.choices else "Unknown"
-        try:
-            parsed = parser(value)
-            if parsed is None:
-                raise ValueError(
-                    f"Unexpected value for {self.name}: {value}"
-                    f" - allowed: {allowed}"
-                )
-            return parsed
-        except SyntaxError as se:
-            raise ValueError(
-                f"{se.msg} for {self.name}: {value}" f" - allowed: {allowed}",
-            ) from se
-
     def __repr__(self) -> str:
         try:
             value = self.value

kasa/iot/modules/motion.py

@@ -5,7 +5,6 @@ from __future__ import annotations
 import logging
 import math
 from enum import Enum
-from typing import Literal, overload
 
 from ...exceptions import KasaException
 from ...feature import Feature
@@ -69,7 +68,6 @@ class Motion(IotModule):
                 attribute_setter="_set_range_cli",
                 type=Feature.Type.Choice,
                 choices_getter="ranges",
-                value_parser="parse_range_value",
                 category=Feature.Category.Config,
             )
         )
@@ -108,7 +106,7 @@ class Motion(IotModule):
                 device=self._device,
                 container=self,
                 id="pir_value",
-                name="PIR Reading",
+                name="PIR Value",
                 icon="mdi:motion-sensor",
                 attribute_getter="pir_value",
                 attribute_setter=None,
@@ -117,21 +115,6 @@ class Motion(IotModule):
             )
         )
 
-        self._add_feature(
-            Feature(
-                device=self._device,
-                container=self,
-                id="pir_percent",
-                name="PIR Percentage",
-                icon="mdi:motion-sensor",
-                attribute_getter="pir_percent",
-                attribute_setter=None,
-                type=Feature.Type.Sensor,
-                category=Feature.Category.Info,
-                unit_getter=lambda: "%",
-            )
-        )
-
         self._add_feature(
             Feature(
                 device=self._device,
@@ -188,6 +171,21 @@ class Motion(IotModule):
             )
         )
 
+        self._add_feature(
+            Feature(
+                device=self._device,
+                container=self,
+                id="pir_percent",
+                name="PIR Percentile",
+                icon="mdi:motion-sensor",
+                attribute_getter="pir_percent",
+                attribute_setter=None,
+                type=Feature.Type.Sensor,
+                category=Feature.Category.Debug,
+                unit_getter=lambda: "%",
+            )
+        )
+
     def query(self) -> dict:
         """Request PIR configuration."""
         req = merge(
@@ -233,33 +231,15 @@ class Motion(IotModule):
         """Enable/disable PIR."""
         return await self.call("set_enable", {"enable": int(state)})
 
-    def _parse_range_value(self, value: str) -> int | Range | None:
-        """Attempt to parse a range value from the given string."""
-        _LOGGER.debug("Parse Range Value: %s", value)
-        parsed: int | Range | None = None
-        try:
-            parsed = int(value)
-            _LOGGER.debug("Parse Range Value: %s is an integer.", value)
-            return parsed
-        except ValueError:
-            _LOGGER.debug("Parse Range Value: %s is not an integer.", value)
-        value = value.strip().upper()
-        if value in Range._member_names_:
-            _LOGGER.debug("Parse Range Value: %s is an enumeration.", value)
-            parsed = Range[value]
-            return parsed
-        _LOGGER.debug("Parse Range Value: %s is not a Range Value.", value)
-        return None
-
     @property
-    def ranges(self) -> list[Range]:
+    def ranges(self) -> list[str]:
         """Return set of supported range classes."""
         range_min = 0
         range_max = len(self.config["array"])
         valid_ranges = list()
         for r in Range:
             if (r.value >= range_min) and (r.value < range_max):
-                valid_ranges.append(r)
+                valid_ranges.append(r.name)
         return valid_ranges
 
     @property
@@ -267,45 +247,27 @@ class Motion(IotModule):
         """Return motion detection Range."""
         return Range(self.config["trigger_index"])
 
-    @overload
-    async def set_range(self, *, range: Range) -> dict: ...
-
-    @overload
-    async def set_range(self, *, range: Literal[Range.Custom], value: int) -> dict: ...
-
-    @overload
-    async def set_range(self, *, value: int) -> dict: ...
-
-    async def set_range(
-        self, *, range: Range | None = None, value: int | None = None
-    ) -> dict:
+    async def set_range(self, range: Range) -> dict:
         """Set the Range for the sensor.
 
-        :param Range: for using standard Ranges
-        :param custom_Range: Range in decimeters, overrides the Range parameter
+        :param Range: the range class to use.
         """
-        if value is not None:
-            if range is not None and range is not Range.Custom:
-                raise KasaException(
-                    f"Refusing to set non-custom range {range} to value {value}."
-                )
-            elif value is None:
-                raise KasaException("Custom range threshold may not be set to None.")
-            payload = {"index": Range.Custom.value, "value": value}
-        elif range is not None:
-            payload = {"index": range.value}
-        else:
-            raise KasaException("Either range or value needs to be defined")
-
+        payload = {"index": range.value}
         return await self.call("set_trigger_sens", payload)
 
-    async def _set_range_cli(self, input: Range | int) -> dict:
-        if isinstance(input, Range):
-            return await self.set_range(range=input)
-        elif isinstance(input, int):
-            return await self.set_range(value=input)
-        else:
-            raise KasaException(f"Invalid type: {type(input)} given to cli motion set.")
+    def _parse_range_value(self, value: str) -> Range:
+        """Attempt to parse a range value from the given string."""
+        value = value.strip().capitalize()
+        if value not in Range._member_names_:
+            raise KasaException(
+                f"Invalid range value: '{value}'."
+                f" Valid options are: {Range._member_names_}"
+            )
+        return Range[value]
+
+    async def _set_range_cli(self, input: str) -> dict:
+        value = self._parse_range_value(input)
+        return await self.set_range(range=value)
 
     def get_range_threshold(self, range_type: Range) -> int:
         """Get the distance threshold at which the PIR sensor is will trigger."""
@@ -322,7 +284,8 @@ class Motion(IotModule):
 
     async def set_threshold(self, value: int) -> dict:
         """Set the distance threshold at which the PIR sensor is will trigger."""
-        return await self.set_range(value=value)
+        payload = {"index": Range.Custom.value, "value": value}
+        return await self.call("set_trigger_sens", payload)
 
     @property
     def inactivity_timeout(self) -> int:

tests/iot/modules/test_motion.py

@@ -1,8 +1,6 @@
-import pytest
 from pytest_mock import MockerFixture
 
 from kasa import Module
-from kasa.exceptions import KasaException
 from kasa.iot import IotDimmer
 from kasa.iot.modules.motion import Motion, Range
 
@@ -38,32 +36,36 @@ async def test_motion_range(dev: IotDimmer, mocker: MockerFixture):
     motion: Motion = dev.modules[Module.IotMotion]
     query_helper = mocker.patch("kasa.iot.IotDimmer._query_helper")
 
-    await motion.set_range(value=123)
-    query_helper.assert_called_with(
-        "smartlife.iot.PIR",
-        "set_trigger_sens",
-        {"index": Range.Custom.value, "value": 123},
-    )
+    for range in Range:
+        await motion.set_range(range)
+        query_helper.assert_called_with(
+            "smartlife.iot.PIR",
+            "set_trigger_sens",
+            {"index": range.value},
+        )
 
-    await motion.set_range(range=Range.Custom, value=123)
-    query_helper.assert_called_with(
-        "smartlife.iot.PIR",
-        "set_trigger_sens",
-        {"index": Range.Custom.value, "value": 123},
-    )
 
-    await motion.set_range(range=Range.Far)
-    query_helper.assert_called_with(
-        "smartlife.iot.PIR", "set_trigger_sens", {"index": Range.Far.value}
-    )
+@dimmer_iot
+async def test_motion_threshold(dev: IotDimmer, mocker: MockerFixture):
+    motion: Motion = dev.modules[Module.IotMotion]
+    query_helper = mocker.patch("kasa.iot.IotDimmer._query_helper")
 
-    with pytest.raises(KasaException, match="Refusing to set non-custom range"):
-        await motion.set_range(range=Range.Near, value=100)  # type: ignore[call-overload]
+    for range in Range:
+        # Switch to a given range.
+        await motion.set_range(range)
+        query_helper.assert_called_with(
+            "smartlife.iot.PIR",
+            "set_trigger_sens",
+            {"index": range.value},
+        )
 
-    with pytest.raises(
-        KasaException, match="Either range or value needs to be defined"
-    ):
-        await motion.set_range()  # type: ignore[call-overload]
+        # Assert that the range always goes to custom, regardless of current range.
+        await motion.set_threshold(123)
+        query_helper.assert_called_with(
+            "smartlife.iot.PIR",
+            "set_trigger_sens",
+            {"index": Range.Custom.value, "value": 123},
+        )
 
 
 @dimmer_iot

tests/test_feature.py

@@ -140,7 +140,10 @@ async def test_feature_choice_list(dummy_feature, caplog, mocker: MockerFixture)
     mock_setter.assert_called_with("first")
     mock_setter.reset_mock()
 
-    with pytest.raises(ValueError, match="Unexpected value for dummy_feature: invalid"):  # noqa: PT012
+    with pytest.raises(  # noqa: PT012
+        ValueError,
+        match="Unexpected value for dummy_feature: 'invalid' (?: - allowed: .*)?",
+    ):
         await dummy_feature.set_value("invalid")
         assert "Unexpected value" in caplog.text