Skip to content

Commit ddf09c0

Browse files
authored
refactor(models)!: clarify States value accessors and widen accessor typing (#2108)
## Summary Two related, breaking v2 ergonomics fixes to the device accessor surface, before the 2.0 freeze. ### #2105 — `States.has()`/`has_any()` conflate existence with value `has()` returned `True` only when the state existed **and** had a non-`None` value, which pushed call sites toward a double lookup (`has(x)` then `get_value(x)`). The two concepts are now separated: - `has()` → **`has_value()`**, `has_any()` → **`has_any_value()`** — explicit about the "present AND non-None value" semantic. - Pure existence is `name in states` (already provided by `Mapping.__contains__`). Many consumers don't even need `has_value` anymore: `get_value(x) == ON` already returns `False` for a missing state, collapsing the double lookup to one call. ### #2106 — Inconsistent accessor typing State accessors were typed `str`-only even though `OverkizState`/`OverkizAttribute` are `StrEnum` and worked at runtime. Signatures now advertise enum support, matching the command helpers: - `States.{get_value, first, first_value, has_value, has_any_value}` → `str | OverkizState | OverkizAttribute` (the `States` class backs both `device.states` and `device.attributes`). - `StateDefinitions.{first, has_any}` → `str | OverkizState`. - `CommandDefinitions` and `device.supports_command()` already accepted enums — unchanged. Scope deliberately excludes new device-level wrappers (e.g. `device.has_state()`): `device.states` is a first-class container read constantly in consumers, while `device.supports_command()` only exists as a shortcut to the buried `device.definition.commands`. Mirroring would duplicate the container API for no gain. ## Changes - `pyoverkiz/models.py`: rename + widen typing (new `StateName` alias). - `docs/device-control.md`, `docs/migration-v2.md`: updated to new names; document `in` for pure existence. - `tests/test_models.py`: renamed tests + new test exercising enum keys. ## Breaking changes - `States.has()` → `States.has_value()` - `States.has_any()` → `States.has_any_value()` ## Test plan - [x] `ruff check` / `ruff format` / `mypy` / `ty` clean - [x] Full suite green (502 passed) Closes #2105 Closes #2106
1 parent ac8e458 commit ddf09c0

4 files changed

Lines changed: 84 additions & 37 deletions

File tree

docs/device-control.md

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,17 @@ print(f"Orientation: {slats_orientation}")
5353
slats_orientation = device.states.first_value([OverkizState.CORE_SLATS_ORIENTATION, OverkizState.CORE_SLATE_ORIENTATION])
5454
print(f"Orientation: {slats_orientation}")
5555

56-
# Check if a single state has a non-None value
57-
if device.states.has(OverkizState.CORE_SLATS_ORIENTATION):
56+
# Check if a single state exists with a non-None value
57+
if device.states.has_value(OverkizState.CORE_SLATS_ORIENTATION):
5858
print("Device has a slats orientation")
5959

60-
# Check if any of the states have non-None values
61-
if device.states.has_any([OverkizState.CORE_SLATS_ORIENTATION, OverkizState.CORE_SLATE_ORIENTATION]):
60+
# Check if any of the states exist with a non-None value
61+
if device.states.has_any_value([OverkizState.CORE_SLATS_ORIENTATION, OverkizState.CORE_SLATE_ORIENTATION]):
6262
print("Device has a slats orientation")
63+
64+
# For a pure existence check (ignoring the value), use `in`
65+
if OverkizState.CORE_SLATS_ORIENTATION in device.states:
66+
print("Device reports a slats orientation state")
6367
```
6468

6569
#### Get state definition

docs/migration-v2.md

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -188,8 +188,9 @@ v2 provides a consistent API on `device.states` and `device.attributes` (both ar
188188
| `.get_value(name)` | `StateType` | Single-key value (None if missing) |
189189
| `.first(names)` | `State \| None` | First non-None match from fallback list |
190190
| `.first_value(names)` | `StateType` | First non-None value from fallback list |
191-
| `.has(name)` | `bool` | Check single state exists with value |
192-
| `.has_any(names)` | `bool` | Check any state exists with value |
191+
| `.has_value(name)` | `bool` | Check single state exists with a non-None value |
192+
| `.has_any_value(names)` | `bool` | Check any state exists with a non-None value |
193+
| `name in states` | `bool` | Pure existence check (ignores the value) |
193194

194195
```python
195196
# Get a single state value
@@ -198,8 +199,12 @@ temp = device.states.get_value("core:TemperatureState")
198199
# Fallback chain — try multiple state names, return first hit
199200
temp = device.states.first_value(["core:TemperatureState", "core:TemperatureInCelsiusState"])
200201

201-
# Existence check
202-
if device.states.has("core:ClosureState"):
202+
# Value check — state exists and has a non-None value
203+
if device.states.has_value("core:ClosureState"):
204+
...
205+
206+
# Pure existence check (ignores the value)
207+
if "core:ClosureState" in device.states:
203208
...
204209

205210
# Same API works for attributes

pyoverkiz/models.py

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
from pyoverkiz.enums.command import OverkizCommand
3131
from pyoverkiz.enums.protocol import Protocol
3232
from pyoverkiz.enums.server import APIType, Server
33+
from pyoverkiz.enums.state import OverkizAttribute, OverkizState
3334
from pyoverkiz.exceptions import OverkizError
3435
from pyoverkiz.obfuscate import obfuscate_email, obfuscate_id, obfuscate_string
3536
from pyoverkiz.types import DATA_TYPE_TO_PYTHON, CommandParameterValue, StateType
@@ -132,6 +133,12 @@ def _cast_json_value(self, raw_value: str) -> StateType:
132133
) from err
133134

134135

136+
# States are keyed by core state names (OverkizState) for device.states and by
137+
# attribute names (OverkizAttribute) for device.attributes; both are StrEnum, so
138+
# plain str keys work too.
139+
StateName = str | OverkizState | OverkizAttribute
140+
141+
135142
@define(init=False)
136143
class States(Mapping[str, State]):
137144
"""Container of State objects implementing Mapping[str, State]."""
@@ -176,34 +183,37 @@ def __len__(self) -> int:
176183
"""Return number of states in the container."""
177184
return len(self._states)
178185

179-
def get_value(self, name: str) -> StateType:
186+
def get_value(self, name: StateName) -> StateType:
180187
"""Return the value of a state by name, or None if missing."""
181188
state = self._index.get(name)
182189
if state is not None:
183190
return state.value
184191
return None
185192

186-
def first(self, names: list[str]) -> State | None:
193+
def first(self, names: list[StateName]) -> State | None:
187194
"""Return the first State that exists and has a non-None value, or None."""
188195
for name in names:
189196
state = self._index.get(name)
190197
if state is not None and state.value is not None:
191198
return state
192199
return None
193200

194-
def first_value(self, names: list[str]) -> StateType:
201+
def first_value(self, names: list[StateName]) -> StateType:
195202
"""Return the value of the first State with a non-None value, or None."""
196203
if state := self.first(names):
197204
return state.value
198205
return None
199206

200-
def has(self, name: str) -> bool:
201-
"""Return True if the state exists with a non-None value."""
207+
def has_value(self, name: StateName) -> bool:
208+
"""Return True if the state exists and has a non-None value.
209+
210+
For a pure existence check (ignoring the value), use ``name in states``.
211+
"""
202212
state = self._index.get(name)
203213
return state is not None and state.value is not None
204214

205-
def has_any(self, names: list[str]) -> bool:
206-
"""Return True if any of the given state names exist with a non-None value."""
215+
def has_any_value(self, names: list[StateName]) -> bool:
216+
"""Return True if any of the given states exist with a non-None value."""
207217
return self.first(names) is not None
208218

209219

@@ -317,14 +327,14 @@ def __len__(self) -> int:
317327
"""Return number of state definitions."""
318328
return len(self._definitions)
319329

320-
def first(self, names: list[str]) -> StateDefinition | None:
330+
def first(self, names: list[str | OverkizState]) -> StateDefinition | None:
321331
"""Return the first StateDefinition whose qualified_name matches, or None."""
322332
for name in names:
323333
if name in self._index:
324334
return self._index[name]
325335
return None
326336

327-
def has_any(self, names: list[str]) -> bool:
337+
def has_any(self, names: list[str | OverkizState]) -> bool:
328338
"""Return True if any of the given state definitions exist."""
329339
return self.first(names) is not None
330340

tests/test_models.py

Lines changed: 48 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
UIProfile,
2222
)
2323
from pyoverkiz.enums.command import OverkizCommand
24+
from pyoverkiz.enums.state import OverkizAttribute, OverkizState
2425
from pyoverkiz.models import (
2526
Action,
2627
ActionGroup,
@@ -348,17 +349,44 @@ def test_states_first_value(self):
348349
value = device.states.first_value(["nonexistent", "core:ClosureState"])
349350
assert value == 100
350351

351-
def test_states_has(self):
352-
"""device.states.has() checks if a single state exists with non-None value."""
352+
def test_states_has_value(self):
353+
"""device.states.has_value() checks if a single state exists with non-None value."""
353354
device = _make_device()
354-
assert device.states.has("core:ClosureState")
355-
assert not device.states.has("nonexistent")
355+
assert device.states.has_value("core:ClosureState")
356+
assert not device.states.has_value("nonexistent")
356357

357-
def test_states_has_any(self):
358-
"""device.states.has_any() checks if any state exists with non-None value."""
358+
def test_states_has_any_value(self):
359+
"""device.states.has_any_value() checks if any state exists with non-None value."""
359360
device = _make_device()
360-
assert device.states.has_any(["nonexistent", "core:ClosureState"])
361-
assert not device.states.has_any(["nonexistent"])
361+
assert device.states.has_any_value(["nonexistent", "core:ClosureState"])
362+
assert not device.states.has_any_value(["nonexistent"])
363+
364+
def test_states_accept_enum_keys(self):
365+
"""States accessors accept OverkizState enums, not just plain strings."""
366+
device = _make_device()
367+
assert OverkizState.CORE_CLOSURE in device.states
368+
assert device.states.has_value(OverkizState.CORE_CLOSURE)
369+
assert device.states.get_value(OverkizState.CORE_CLOSURE) == 100
370+
assert device.states.first([OverkizState.CORE_CLOSURE]) is not None
371+
assert device.states.has_any_value([OverkizState.CORE_CLOSURE])
372+
373+
def test_attributes_accept_enum_keys(self):
374+
"""Attribute accessors accept OverkizAttribute enums, not just plain strings."""
375+
device = _make_device(
376+
{
377+
**RAW_DEVICES,
378+
"attributes": [
379+
{"name": "core:Manufacturer", "type": 3, "value": "VELUX"},
380+
],
381+
}
382+
)
383+
assert OverkizAttribute.CORE_MANUFACTURER in device.attributes
384+
assert device.attributes.has_value(OverkizAttribute.CORE_MANUFACTURER)
385+
assert (
386+
device.attributes.get_value(OverkizAttribute.CORE_MANUFACTURER) == "VELUX"
387+
)
388+
assert device.attributes.first([OverkizAttribute.CORE_MANUFACTURER]) is not None
389+
assert device.attributes.has_any_value([OverkizAttribute.CORE_MANUFACTURER])
362390

363391
def test_definition_states_get(self):
364392
"""device.definition.states.get() returns StateDefinition for a single state."""
@@ -500,25 +528,25 @@ def test_first_value_returns_none_when_no_match(self):
500528
states = self._make_states(RAW_STATES)
501529
assert states.first_value(["nonexistent"]) is None
502530

503-
def test_has_returns_true(self):
504-
"""has() returns True when the state exists with a non-None value."""
531+
def test_has_value_returns_true(self):
532+
"""has_value() returns True when the state exists with a non-None value."""
505533
states = self._make_states(RAW_STATES)
506-
assert states.has("core:NameState")
534+
assert states.has_value("core:NameState")
507535

508-
def test_has_returns_false(self):
509-
"""has() returns False when the state does not exist."""
536+
def test_has_value_returns_false(self):
537+
"""has_value() returns False when the state does not exist."""
510538
states = self._make_states(RAW_STATES)
511-
assert not states.has("nonexistent")
539+
assert not states.has_value("nonexistent")
512540

513-
def test_has_any_true(self):
514-
"""has_any() returns True when at least one state exists."""
541+
def test_has_any_value_true(self):
542+
"""has_any_value() returns True when at least one state exists."""
515543
states = self._make_states(RAW_STATES)
516-
assert states.has_any(["nonexistent", "core:NameState"])
544+
assert states.has_any_value(["nonexistent", "core:NameState"])
517545

518-
def test_has_any_false(self):
519-
"""has_any() returns False when no state exists."""
546+
def test_has_any_value_false(self):
547+
"""has_any_value() returns False when no state exists."""
520548
states = self._make_states(RAW_STATES)
521-
assert not states.has_any(["nonexistent", "also_nonexistent"])
549+
assert not states.has_any_value(["nonexistent", "also_nonexistent"])
522550

523551
def test_getitem_raises_keyerror_on_missing(self):
524552
"""Subscript access raises KeyError for missing states."""

0 commit comments

Comments
 (0)