Skip to content

Commit 8b36e6d

Browse files
llucaxllucax
committed
Expose operational mode through individual properties
Using the operational mode protobuf enum as a component attribute is not very convenient. Users would need to check for multiple combinations to find out whether telemetry or control is available, plus handle the unspecified value, which should be rare but is possible. Instead, keep the operational mode representation entirely in the protobuf conversion layer and expose it through two Component accessors that are much safer to use: - `provides_telemetry()`: whether the component provides telemetry data - `accepts_control()`: whether the component accepts control commands Both raise `ValueError` when the operational mode is unspecified or unrecognized, so callers get an explicit failure instead of a silently wrong boolean. The mode is stored internally as two private `bool | None` fields that default to `None` (unspecified), so existing component construction sites remain unaffected. This is a port of commit [5c729ba][] from `frequenz-client-common`, so we upgrade to it the migration should be smooth. [5c729ba]: frequenz-floss/frequenz-client-common-python@5c729ba Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
1 parent 73d008b commit 8b36e6d

5 files changed

Lines changed: 203 additions & 0 deletions

File tree

‎src/frequenz/client/microgrid/component/_component.py‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,12 @@ class Component: # pylint: disable=too-many-instance-attributes
5252
operational_lifetime: Lifetime = dataclasses.field(default_factory=Lifetime)
5353
"""The operational lifetime of this component."""
5454

55+
_provides_telemetry: bool | None = None
56+
"""Whether this component provides telemetry data, or `None` if unspecified."""
57+
58+
_accepts_control: bool | None = None
59+
"""Whether this component accepts control commands, or `None` if unspecified."""
60+
5561
rated_bounds: Mapping[Metric | int, Bounds] = dataclasses.field(
5662
default_factory=dict,
5763
# dict is not hashable, so we don't use this field to calculate the hash. This
@@ -85,6 +91,40 @@ def __new__(cls, *_: Any, **__: Any) -> Self:
8591
raise TypeError(f"Cannot instantiate {cls.__name__} directly")
8692
return super().__new__(cls)
8793

94+
def provides_telemetry(self) -> bool:
95+
"""Check whether this component provides telemetry data.
96+
97+
Returns:
98+
Whether this component provides telemetry data.
99+
100+
Raises:
101+
ValueError: If the operational mode is unspecified, so whether telemetry
102+
is provided is unknown.
103+
"""
104+
if self._provides_telemetry is None:
105+
raise ValueError(
106+
f"operational mode of {self} is unspecified; "
107+
"telemetry availability is unknown"
108+
)
109+
return self._provides_telemetry
110+
111+
def accepts_control(self) -> bool:
112+
"""Check whether this component accepts control commands.
113+
114+
Returns:
115+
Whether this component accepts control commands.
116+
117+
Raises:
118+
ValueError: If the operational mode is unspecified, so whether control
119+
commands are accepted is unknown.
120+
"""
121+
if self._accepts_control is None:
122+
raise ValueError(
123+
f"operational mode of {self} is unspecified; "
124+
"control availability is unknown"
125+
)
126+
return self._accepts_control
127+
88128
def is_operational_at(self, timestamp: datetime) -> bool:
89129
"""Check whether this component is operational at a specific timestamp.
90130

‎src/frequenz/client/microgrid/component/_component_proto.py‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,40 @@
7171
# pylint: disable=too-many-arguments
7272

7373

74+
_BOOLS_BY_OPERATIONAL_MODE: dict[int, tuple[bool | None, bool | None]] = {
75+
electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_INACTIVE: (
76+
False,
77+
False,
78+
),
79+
electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_TELEMETRY_ONLY: (
80+
True,
81+
False,
82+
),
83+
electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_CONTROL_ONLY: (
84+
False,
85+
True,
86+
),
87+
electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_CONTROL_AND_TELEMETRY: (
88+
True,
89+
True,
90+
),
91+
}
92+
93+
94+
def _operational_mode_to_bools(value: int) -> tuple[bool | None, bool | None]:
95+
"""Map a protobuf operational mode to telemetry/control booleans.
96+
97+
Args:
98+
value: A protobuf operational-mode enum value (an
99+
`ELECTRICAL_COMPONENT_OPERATIONAL_MODE_*` constant).
100+
101+
Returns:
102+
A `(provides_telemetry, accepts_control)` tuple, with both elements `None`
103+
when the operational mode is unspecified or unrecognized.
104+
"""
105+
return _BOOLS_BY_OPERATIONAL_MODE.get(value, (None, None))
106+
107+
74108
def component_from_proto(
75109
message: electrical_components_pb2.ElectricalComponent,
76110
) -> ComponentTypes:
@@ -117,6 +151,8 @@ class ComponentBaseData(NamedTuple):
117151
lifetime: Lifetime
118152
rated_bounds: dict[Metric | int, Bounds]
119153
category_specific_info: dict[str, Any]
154+
provides_telemetry: bool | None
155+
accepts_control: bool | None
120156
category_mismatched: bool = False
121157

122158

@@ -197,6 +233,7 @@ def component_base_from_proto_with_issues(
197233
lifetime,
198234
rated_bounds,
199235
category_specific_info,
236+
*_operational_mode_to_bools(message.operational_mode),
200237
category_mismatched,
201238
)
202239

@@ -231,6 +268,8 @@ def component_from_proto_with_issues(
231268
model_name=base_data.model_name,
232269
category=base_data.category,
233270
operational_lifetime=base_data.lifetime,
271+
_provides_telemetry=base_data.provides_telemetry,
272+
_accepts_control=base_data.accepts_control,
234273
category_specific_metadata=base_data.category_specific_info,
235274
rated_bounds=base_data.rated_bounds,
236275
)
@@ -245,6 +284,8 @@ def component_from_proto_with_issues(
245284
model_name=base_data.model_name,
246285
category=base_data.category,
247286
operational_lifetime=base_data.lifetime,
287+
_provides_telemetry=base_data.provides_telemetry,
288+
_accepts_control=base_data.accepts_control,
248289
rated_bounds=base_data.rated_bounds,
249290
)
250291
case (
@@ -267,6 +308,8 @@ def component_from_proto_with_issues(
267308
manufacturer=base_data.manufacturer,
268309
model_name=base_data.model_name,
269310
operational_lifetime=base_data.lifetime,
311+
_provides_telemetry=base_data.provides_telemetry,
312+
_accepts_control=base_data.accepts_control,
270313
rated_bounds=base_data.rated_bounds,
271314
)
272315
case ComponentCategory.BATTERY:
@@ -291,6 +334,8 @@ def component_from_proto_with_issues(
291334
manufacturer=base_data.manufacturer,
292335
model_name=base_data.model_name,
293336
operational_lifetime=base_data.lifetime,
337+
_provides_telemetry=base_data.provides_telemetry,
338+
_accepts_control=base_data.accepts_control,
294339
rated_bounds=base_data.rated_bounds,
295340
)
296341
case int():
@@ -302,6 +347,8 @@ def component_from_proto_with_issues(
302347
manufacturer=base_data.manufacturer,
303348
model_name=base_data.model_name,
304349
operational_lifetime=base_data.lifetime,
350+
_provides_telemetry=base_data.provides_telemetry,
351+
_accepts_control=base_data.accepts_control,
305352
rated_bounds=base_data.rated_bounds,
306353
type=battery_type,
307354
)
@@ -338,6 +385,8 @@ def component_from_proto_with_issues(
338385
manufacturer=base_data.manufacturer,
339386
model_name=base_data.model_name,
340387
operational_lifetime=base_data.lifetime,
388+
_provides_telemetry=base_data.provides_telemetry,
389+
_accepts_control=base_data.accepts_control,
341390
rated_bounds=base_data.rated_bounds,
342391
)
343392
case int():
@@ -351,6 +400,8 @@ def component_from_proto_with_issues(
351400
manufacturer=base_data.manufacturer,
352401
model_name=base_data.model_name,
353402
operational_lifetime=base_data.lifetime,
403+
_provides_telemetry=base_data.provides_telemetry,
404+
_accepts_control=base_data.accepts_control,
354405
rated_bounds=base_data.rated_bounds,
355406
type=ev_charger_type,
356407
)
@@ -368,6 +419,8 @@ def component_from_proto_with_issues(
368419
manufacturer=base_data.manufacturer,
369420
model_name=base_data.model_name,
370421
operational_lifetime=base_data.lifetime,
422+
_provides_telemetry=base_data.provides_telemetry,
423+
_accepts_control=base_data.accepts_control,
371424
rated_bounds=base_data.rated_bounds,
372425
rated_fuse_current=rated_fuse_current,
373426
)
@@ -405,6 +458,8 @@ def component_from_proto_with_issues(
405458
manufacturer=base_data.manufacturer,
406459
model_name=base_data.model_name,
407460
operational_lifetime=base_data.lifetime,
461+
_provides_telemetry=base_data.provides_telemetry,
462+
_accepts_control=base_data.accepts_control,
408463
rated_bounds=base_data.rated_bounds,
409464
)
410465
case int():
@@ -418,6 +473,8 @@ def component_from_proto_with_issues(
418473
manufacturer=base_data.manufacturer,
419474
model_name=base_data.model_name,
420475
operational_lifetime=base_data.lifetime,
476+
_provides_telemetry=base_data.provides_telemetry,
477+
_accepts_control=base_data.accepts_control,
421478
rated_bounds=base_data.rated_bounds,
422479
type=inverter_type,
423480
)
@@ -433,6 +490,8 @@ def component_from_proto_with_issues(
433490
manufacturer=base_data.manufacturer,
434491
model_name=base_data.model_name,
435492
operational_lifetime=base_data.lifetime,
493+
_provides_telemetry=base_data.provides_telemetry,
494+
_accepts_control=base_data.accepts_control,
436495
rated_bounds=base_data.rated_bounds,
437496
primary_voltage=message.category_specific_info.power_transformer.primary,
438497
secondary_voltage=message.category_specific_info.power_transformer.secondary,

‎tests/component/component_proto/conftest.py‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ def default_component_base_data(
5959
lifetime=DEFAULT_LIFETIME,
6060
rated_bounds={Metric.AC_ENERGY_ACTIVE: Bounds(lower=0, upper=100)},
6161
category_specific_info={},
62+
provides_telemetry=True,
63+
accepts_control=True,
6264
category_mismatched=False,
6365
)
6466

@@ -74,6 +76,32 @@ def assert_base_data(base_data: ComponentBaseData, other: Component) -> None:
7476
assert base_data.lifetime == other.operational_lifetime
7577
assert base_data.rated_bounds == other.rated_bounds
7678
assert base_data.category_specific_info == other.category_specific_metadata
79+
# pylint: disable=protected-access
80+
assert base_data.provides_telemetry == other._provides_telemetry
81+
assert base_data.accepts_control == other._accepts_control
82+
# pylint: enable=protected-access
83+
84+
85+
_OPERATIONAL_MODE_BY_BOOLS: dict[
86+
tuple[bool | None, bool | None],
87+
electrical_components_pb2.ElectricalComponentOperationalMode.ValueType,
88+
] = {
89+
(None, None): (
90+
electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_UNSPECIFIED
91+
),
92+
(False, False): (
93+
electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_INACTIVE
94+
),
95+
(True, False): (
96+
electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_TELEMETRY_ONLY
97+
),
98+
(False, True): (
99+
electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_CONTROL_ONLY
100+
),
101+
(True, True): (
102+
electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_CONTROL_AND_TELEMETRY
103+
),
104+
}
77105

78106

79107
def base_data_as_proto(
@@ -91,6 +119,9 @@ def base_data_as_proto(
91119
if isinstance(base_data.category, int)
92120
else int(base_data.category.value) # type: ignore[arg-type]
93121
),
122+
operational_mode=_OPERATIONAL_MODE_BY_BOOLS[
123+
(base_data.provides_telemetry, base_data.accepts_control)
124+
],
94125
)
95126
if base_data.lifetime:
96127
lifetime_dict: dict[str, Timestamp] = {}

‎tests/component/component_proto/test_base.py‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
"""Tests for protobuf conversion of the base/common part of Component objects."""
55

6+
import pytest
67
from frequenz.api.common.v1alpha8.microgrid.electrical_components import (
78
electrical_components_pb2,
89
)
@@ -12,12 +13,54 @@
1213
from frequenz.client.microgrid.component import ComponentCategory
1314
from frequenz.client.microgrid.component._component_proto import (
1415
ComponentBaseData,
16+
_operational_mode_to_bools,
1517
component_base_from_proto_with_issues,
1618
)
1719

1820
from .conftest import base_data_as_proto
1921

2022

23+
@pytest.mark.parametrize(
24+
"proto_value, expected",
25+
[
26+
(
27+
electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_UNSPECIFIED,
28+
(None, None),
29+
),
30+
(
31+
electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_INACTIVE,
32+
(False, False),
33+
),
34+
(
35+
electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_TELEMETRY_ONLY,
36+
(True, False),
37+
),
38+
(
39+
electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_CONTROL_ONLY,
40+
(False, True),
41+
),
42+
(
43+
electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_CONTROL_AND_TELEMETRY,
44+
(True, True),
45+
),
46+
(999, (None, None)),
47+
],
48+
ids=[
49+
"unspecified",
50+
"inactive",
51+
"telemetry-only",
52+
"control-only",
53+
"control-and-telemetry",
54+
"unknown-int",
55+
],
56+
)
57+
def test_operational_mode_to_bools(
58+
proto_value: int, expected: tuple[bool | None, bool | None]
59+
) -> None:
60+
"""Test proto operational-mode maps to (provides_telemetry, accepts_control)."""
61+
assert _operational_mode_to_bools(proto_value) == expected
62+
63+
2164
def test_complete(default_component_base_data: ComponentBaseData) -> None:
2265
"""Test parsing of a complete base component proto."""
2366
major_issues: list[str] = []

‎tests/component/test_component.py‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,36 @@ def test_creation_full() -> None:
7474
assert component.category_specific_metadata == metadata
7575

7676

77+
def test_accessors_return_values_when_set() -> None:
78+
"""Test that telemetry/control accessors return the stored booleans."""
79+
component = _TestComponent(
80+
id=ComponentId(1),
81+
microgrid_id=MicrogridId(2),
82+
category=ComponentCategory.UNSPECIFIED,
83+
_provides_telemetry=True,
84+
_accepts_control=False,
85+
)
86+
87+
assert component.provides_telemetry() is True
88+
assert component.accepts_control() is False
89+
90+
91+
def test_accessors_raise_when_unspecified() -> None:
92+
"""Test that accessors raise ValueError when the value is unknown."""
93+
component = _TestComponent(
94+
id=ComponentId(1),
95+
microgrid_id=MicrogridId(2),
96+
category=ComponentCategory.UNSPECIFIED,
97+
_provides_telemetry=None,
98+
_accepts_control=None,
99+
)
100+
101+
with pytest.raises(ValueError):
102+
component.provides_telemetry()
103+
with pytest.raises(ValueError):
104+
component.accepts_control()
105+
106+
77107
@pytest.mark.parametrize(
78108
"name,expected_str",
79109
[

0 commit comments

Comments
 (0)