Merge pull request #84 from graphras-com/fix/75-normalize-range-validation

fix: normalize range validation for light, cover, and valve domains

Author: Faerkeren

src/haclient/domains/_utils.py

@@ -0,0 +1,42 @@
+"""Shared validation helpers for domain action methods."""
+
+from __future__ import annotations
+
+
+def validate_range(value: int, lo: int, hi: int, name: str) -> int:
+    """Coerce *value* to ``int`` and raise if it falls outside ``[lo, hi]``.
+
+    Parameters
+    ----------
+    value : int
+        The raw value supplied by the caller.
+    lo : int
+        Inclusive lower bound.
+    hi : int
+        Inclusive upper bound.
+    name : str
+        Human-readable parameter name used in the error message.
+
+    Returns
+    -------
+    int
+        The coerced integer, guaranteed to satisfy ``lo <= result <= hi``.
+
+    Raises
+    ------
+    ValueError
+        If the coerced value is less than *lo* or greater than *hi*.
+
+    Examples
+    --------
+    >>> validate_range(128, 0, 255, "brightness")
+    128
+    >>> validate_range(300, 0, 255, "brightness")
+    Traceback (most recent call last):
+        ...
+    ValueError: brightness must be between 0 and 255, got 300
+    """
+    coerced = int(value)
+    if not lo <= coerced <= hi:
+        raise ValueError(f"{name} must be between {lo} and {hi}, got {coerced}")
+    return coerced

src/haclient/domains/cover.py

@@ -3,6 +3,7 @@
 from __future__ import annotations
 
 from haclient.core.plugins import DomainSpec, register_domain
+from haclient.domains._utils import validate_range
 from haclient.entity.base import Entity, ValueChangeHandler
 
 
@@ -103,10 +104,18 @@ async def set_position(self, position: int) -> None:
         Parameters
         ----------
         position : int
-            Target position, clamped/coerced to ``int``. ``0`` is fully
+            Target position in the range 0--100. ``0`` is fully
             closed; ``100`` is fully open.
+
+        Raises
+        ------
+        ValueError
+            If *position* is outside the 0--100 range.
         """
-        await self._call_service("set_cover_position", {"position": int(position)})
+        await self._call_service(
+            "set_cover_position",
+            {"position": validate_range(position, 0, 100, "position")},
+        )
 
     async def toggle(self) -> None:
         """Toggle open/close state."""

src/haclient/domains/light.py

@@ -5,6 +5,7 @@
 from typing import Any
 
 from haclient.core.plugins import DomainSpec, register_domain
+from haclient.domains._utils import validate_range
 from haclient.entity.base import Entity, ValueChangeHandler
 
 
@@ -151,12 +152,17 @@ async def set_brightness(self, brightness: int, *, transition: float | None = No
         Parameters
         ----------
         brightness : int
-            New brightness value, coerced to ``int``. ``0`` turns the
+            New brightness value in the range 0--255. ``0`` turns the
             light off; ``255`` is full brightness.
         transition : float or None, optional
             Seconds for the transition. Forwarded to HA when set.
+
+        Raises
+        ------
+        ValueError
+            If *brightness* is outside the 0--255 range.
         """
-        data: dict[str, Any] = {"brightness": int(brightness)}
+        data: dict[str, Any] = {"brightness": validate_range(brightness, 0, 255, "brightness")}
         if transition is not None:
             data["transition"] = transition
         await self._call_service("turn_on", data)
@@ -189,15 +195,25 @@ async def set_rgb(
         Parameters
         ----------
         r : int
-            Red component (0-255).
+            Red component (0--255).
         g : int
-            Green component (0-255).
+            Green component (0--255).
         b : int
-            Blue component (0-255).
+            Blue component (0--255).
         transition : float or None, optional
             Seconds for the transition. Forwarded to HA when set.
+
+        Raises
+        ------
+        ValueError
+            If any component is outside the 0--255 range.
         """
-        data: dict[str, Any] = {"rgb_color": [r, g, b]}
+        rgb = [
+            validate_range(r, 0, 255, "r"),
+            validate_range(g, 0, 255, "g"),
+            validate_range(b, 0, 255, "b"),
+        ]
+        data: dict[str, Any] = {"rgb_color": rgb}
         if transition is not None:
             data["transition"] = transition
         await self._call_service("turn_on", data)

src/haclient/domains/valve.py

@@ -5,6 +5,7 @@
 import logging
 
 from haclient.core.plugins import DomainSpec, register_domain
+from haclient.domains._utils import validate_range
 from haclient.entity.base import Entity, ValueChangeHandler
 
 _LOGGER = logging.getLogger(__name__)
@@ -176,23 +177,31 @@ async def set_position(self, position: int) -> None:
         Parameters
         ----------
         position : int
-            Target position (``0`` = fully closed, ``100`` = fully
-            open), coerced to ``int``.
+            Target position in the range 0--100 (``0`` = fully closed,
+            ``100`` = fully open).
+
+        Raises
+        ------
+        ValueError
+            If *position* is outside the 0--100 range.
 
         Notes
         -----
         Degrades safely: if the valve does not advertise the
         ``SET_POSITION`` feature (e.g. a binary water shutoff), this
         method logs a debug message and returns without raising.
         Callers can pre-check with `supports_set_position`.
+        Range validation always runs, even when the feature is absent,
+        so callers receive an immediate error for clearly invalid input.
         """
+        value = validate_range(position, 0, 100, "position")
         if not self.supports_set_position:
             _LOGGER.debug(
                 "set_position() unsupported for %s; skipping (no ValveEntityFeature.SET_POSITION)",
                 self.entity_id,
             )
             return
-        await self._call_service("set_valve_position", {"position": int(position)})
+        await self._call_service("set_valve_position", {"position": value})
 
     async def toggle(self) -> None:
         """Toggle open/close state."""

tests/test_domains.py

@@ -1581,3 +1581,89 @@ def _dir(old: Any, new: Any) -> None:
     assert turned_off == [("on", "off")]
     assert speed == [(0, 50), (50, 75), (75, 0)]
     assert direction == [("forward", "reverse"), ("reverse", "forward")]
+
+
+# ---------------------------------------------------------------------------
+# Range validation — issue #75
+# ---------------------------------------------------------------------------
+
+
+async def test_light_set_brightness_validation(client: HAClient, fake_ha: FakeHA) -> None:
+    """``set_brightness`` enforces the 0-255 range."""
+    light = client.light("kitchen")
+
+    with pytest.raises(ValueError, match="brightness"):
+        await light.set_brightness(-1)
+    with pytest.raises(ValueError, match="brightness"):
+        await light.set_brightness(256)
+
+    # Boundary values are accepted.
+    await light.set_brightness(0)
+    await light.set_brightness(255)
+    services = [c["service"] for c in fake_ha.ws_service_calls]
+    assert services == ["turn_on", "turn_on"]
+    assert fake_ha.ws_service_calls[0]["service_data"]["brightness"] == 0
+    assert fake_ha.ws_service_calls[1]["service_data"]["brightness"] == 255
+
+
+async def test_light_set_rgb_validation(client: HAClient, fake_ha: FakeHA) -> None:
+    """``set_rgb`` enforces the 0-255 range on each component."""
+    light = client.light("kitchen")
+
+    with pytest.raises(ValueError, match="r"):
+        await light.set_rgb(-1, 0, 0)
+    with pytest.raises(ValueError, match="r"):
+        await light.set_rgb(256, 0, 0)
+    with pytest.raises(ValueError, match="g"):
+        await light.set_rgb(0, -1, 0)
+    with pytest.raises(ValueError, match="g"):
+        await light.set_rgb(0, 256, 0)
+    with pytest.raises(ValueError, match="b"):
+        await light.set_rgb(0, 0, -1)
+    with pytest.raises(ValueError, match="b"):
+        await light.set_rgb(0, 0, 256)
+
+    # Boundary values are accepted.
+    await light.set_rgb(0, 0, 0)
+    await light.set_rgb(255, 255, 255)
+    assert fake_ha.ws_service_calls[0]["service_data"]["rgb_color"] == [0, 0, 0]
+    assert fake_ha.ws_service_calls[1]["service_data"]["rgb_color"] == [255, 255, 255]
+
+
+async def test_cover_set_position_validation(client: HAClient, fake_ha: FakeHA) -> None:
+    """``set_position`` enforces the 0-100 range."""
+    cv = client.cover("garage")
+
+    with pytest.raises(ValueError, match="position"):
+        await cv.set_position(-1)
+    with pytest.raises(ValueError, match="position"):
+        await cv.set_position(101)
+
+    # Boundary values are accepted.
+    await cv.set_position(0)
+    await cv.set_position(100)
+    services = [c["service"] for c in fake_ha.ws_service_calls]
+    assert services == ["set_cover_position", "set_cover_position"]
+    assert fake_ha.ws_service_calls[0]["service_data"]["position"] == 0
+    assert fake_ha.ws_service_calls[1]["service_data"]["position"] == 100
+
+
+async def test_valve_set_position_validation(client: HAClient, fake_ha: FakeHA) -> None:
+    """``set_position`` enforces the 0-100 range even when the feature is absent."""
+    valve = client.valve("main_water")
+
+    # Range validation fires regardless of feature support.
+    valve._apply_state({"state": "open", "attributes": {"supported_features": 0}})
+    with pytest.raises(ValueError, match="position"):
+        await valve.set_position(-1)
+    with pytest.raises(ValueError, match="position"):
+        await valve.set_position(101)
+
+    # With feature support, boundary values are accepted and dispatched.
+    valve._apply_state({"state": "open", "attributes": {"supported_features": 15}})
+    await valve.set_position(0)
+    await valve.set_position(100)
+    services = [c["service"] for c in fake_ha.ws_service_calls]
+    assert services == ["set_valve_position", "set_valve_position"]
+    assert fake_ha.ws_service_calls[0]["service_data"]["position"] == 0
+    assert fake_ha.ws_service_calls[1]["service_data"]["position"] == 100