From 96ec7a2d2aef363596a5f0e5c99ea870f76fdf47 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 19 Jun 2026 13:15:52 +0200 Subject: [PATCH 01/10] Add new exception types Signed-off-by: Leandro Lucarella --- src/frequenz/client/common/__init__.py | 7 +++++++ src/frequenz/client/common/_exception.py | 15 +++++++++++++++ tests/test_exception.py | 11 +++++++++++ 3 files changed, 33 insertions(+) create mode 100644 src/frequenz/client/common/_exception.py create mode 100644 tests/test_exception.py diff --git a/src/frequenz/client/common/__init__.py b/src/frequenz/client/common/__init__.py index 01a78dc3..045cda0b 100644 --- a/src/frequenz/client/common/__init__.py +++ b/src/frequenz/client/common/__init__.py @@ -2,3 +2,10 @@ # Copyright © 2023 Frequenz Energy-as-a-Service GmbH """Common code and utilities for Frequenz API clients.""" + +from ._exception import ClientCommonError, UnspecifiedValueError + +__all__ = [ + "ClientCommonError", + "UnspecifiedValueError", +] diff --git a/src/frequenz/client/common/_exception.py b/src/frequenz/client/common/_exception.py new file mode 100644 index 00000000..6c198ef2 --- /dev/null +++ b/src/frequenz/client/common/_exception.py @@ -0,0 +1,15 @@ +# License: MIT +# Copyright © 2026 Frequenz Energy-as-a-Service GmbH + +"""Common exception types for Frequenz API clients.""" + + +class ClientCommonError(Exception): + """Base class for all errors raised by frequenz-client-common.""" + + +class UnspecifiedValueError(ClientCommonError, ValueError): + """Raised when a semantic accessor sees an unspecified or unknown protobuf value. + + This is also a ``ValueError`` for convenience. + """ diff --git a/tests/test_exception.py b/tests/test_exception.py new file mode 100644 index 00000000..b0ae7036 --- /dev/null +++ b/tests/test_exception.py @@ -0,0 +1,11 @@ +"""Tests for common exceptions.""" + +from frequenz.client.common import ClientCommonError, UnspecifiedValueError + + +def test_exceptions_exported_and_related() -> None: + """Given exception exports, then their hierarchy and string form are correct.""" + assert issubclass(UnspecifiedValueError, ClientCommonError) + assert issubclass(UnspecifiedValueError, ValueError) + assert not issubclass(ClientCommonError, ValueError) + assert str(UnspecifiedValueError("msg")) == "msg" From 2304d97fdbdcfe3378e578eae7961df6b92d221a Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 19 Jun 2026 14:14:06 +0200 Subject: [PATCH 02/10] Expose operational mode through individual properties Using the `ElectricalComponentOperationalMode` enum as an attribute is not very convenient. Users will need to check for multiple combinations when trying to find out if either telemetry or control is available, plus dealing with the possibility of an unspecified value, which should be rare, but possible. This new approach completely leaves the operational mode representation for the protobuf conversion layer, and maps different modes to new properties that are much safer to use. Signed-off-by: Leandro Lucarella --- .../_electrical_component.py | 47 +++++++++-- .../proto/v1alpha8/_electrical_component.py | 80 +++++++++++++++---- .../proto/v1alpha8/conftest.py | 39 +++++++-- .../test_electrical_component_base.py | 43 ++++++++++ .../electrical_components/test_battery.py | 6 ++ .../test_electrical_component_base.py | 57 ++++++++++++- .../electrical_components/test_ev_charger.py | 6 ++ .../test_grid_connection_point.py | 4 + .../electrical_components/test_inverter.py | 6 ++ .../test_power_transformer.py | 2 + .../electrical_components/test_problematic.py | 10 +++ .../test_simple_components.py | 2 + 12 files changed, 267 insertions(+), 35 deletions(-) diff --git a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py index 1dbcc8fa..246f69cd 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py @@ -8,12 +8,12 @@ from datetime import datetime, timezone from typing import Any, Self +from ..._exception import UnspecifiedValueError from ...metrics import Bounds, Metric from ...types import Lifetime from .. import MicrogridId from ._category import ElectricalComponentCategory from ._ids import ElectricalComponentId -from ._operational_mode import ElectricalComponentOperationalMode @dataclasses.dataclass(frozen=True, kw_only=True) @@ -51,14 +51,11 @@ class ElectricalComponent: # pylint: disable=too-many-instance-attributes operational_lifetime: Lifetime = dataclasses.field(default_factory=Lifetime) """The operational lifetime of this electrical component.""" - operational_mode: ElectricalComponentOperationalMode | int = ( - ElectricalComponentOperationalMode.UNSPECIFIED - ) - """The operational mode of this electrical component. + _provides_telemetry: bool | None + """Whether this component provides telemetry data, or `None` if unspecified.""" - This indicates whether the component is active and operational, and whether it - provides telemetry data, accepts control commands, or both. - """ + _accepts_control: bool | None + """Whether this component accepts control commands, or `None` if unspecified.""" metric_config_bounds: Mapping[Metric | int, Bounds] = dataclasses.field( default_factory=dict, @@ -97,6 +94,40 @@ def __new__(cls, *_: Any, **__: Any) -> Self: raise TypeError(f"Cannot instantiate {cls.__name__} directly") return super().__new__(cls) + def provides_telemetry(self) -> bool: + """Check whether this electrical component provides telemetry data. + + Returns: + Whether this electrical component provides telemetry data. + + Raises: + UnspecifiedValueError: If the operational mode is unspecified, so whether + telemetry is provided is unknown. + """ + if self._provides_telemetry is None: + raise UnspecifiedValueError( + f"operational mode of {self} is unspecified; " + "telemetry availability is unknown" + ) + return self._provides_telemetry + + def accepts_control(self) -> bool: + """Check whether this electrical component accepts control commands. + + Returns: + Whether this electrical component accepts control commands. + + Raises: + UnspecifiedValueError: If the operational mode is unspecified, so whether + control commands are accepted is unknown. + """ + if self._accepts_control is None: + raise UnspecifiedValueError( + f"operational mode of {self} is unspecified; " + "control availability is unknown" + ) + return self._accepts_control + def is_operational_at(self, timestamp: datetime) -> bool: """Check whether this electrical component is operational at a specific timestamp. diff --git a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py index 6d588999..f3dac828 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py @@ -29,7 +29,6 @@ DcEvCharger, ElectricalComponentCategory, ElectricalComponentId, - ElectricalComponentOperationalMode, ElectricalComponentTypes, Electrolyzer, EvChargerType, @@ -56,7 +55,6 @@ UnspecifiedInverter, WindTurbine, ) -from ._operational_mode import electrical_component_operational_mode_from_proto _logger = logging.getLogger(__name__) @@ -66,6 +64,40 @@ # pylint: disable=too-many-arguments +_BOOLS_BY_OPERATIONAL_MODE: dict[int, tuple[bool | None, bool | None]] = { + electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_INACTIVE: ( + False, + False, + ), + electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_TELEMETRY_ONLY: ( + True, + False, + ), + electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_CONTROL_ONLY: ( + False, + True, + ), + electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_CONTROL_AND_TELEMETRY: ( + True, + True, + ), +} + + +def _operational_mode_to_bools(value: int) -> tuple[bool | None, bool | None]: + """Map a protobuf operational mode to telemetry/control booleans. + + Args: + value: A protobuf operational-mode enum value (an + `ELECTRICAL_COMPONENT_OPERATIONAL_MODE_*` constant). + + Returns: + A `(provides_telemetry, accepts_control)` tuple, with both elements `None` + when the operational mode is unspecified or unrecognized. + """ + return _BOOLS_BY_OPERATIONAL_MODE.get(value, (None, None)) + + def electrical_component_from_proto( message: electrical_components_pb2.ElectricalComponent, ) -> ElectricalComponentTypes: @@ -111,7 +143,8 @@ class _ElectricalComponentBaseData(NamedTuple): lifetime: Lifetime metric_config_bounds: dict[Metric | int, Bounds] category_specific_info: dict[str, Any] - operational_mode: ElectricalComponentOperationalMode | int + provides_telemetry: bool | None + accepts_control: bool | None category_mismatched: bool = False @@ -143,7 +176,7 @@ def _electrical_component_base_from_proto_with_issues( if model is None: minor_issues.append("model is empty") - operational_mode = electrical_component_operational_mode_from_proto( + provides_telemetry, accepts_control = _operational_mode_to_bools( message.operational_mode ) @@ -192,7 +225,8 @@ def _electrical_component_base_from_proto_with_issues( lifetime, metric_config_bounds, category_specific_info, - operational_mode, + provides_telemetry, + accepts_control, category_mismatched, ) @@ -226,7 +260,8 @@ def electrical_component_from_proto_with_issues( model=base_data.model, category=base_data.category, operational_lifetime=base_data.lifetime, - operational_mode=base_data.operational_mode, + _provides_telemetry=base_data.provides_telemetry, + _accepts_control=base_data.accepts_control, category_specific_metadata=base_data.category_specific_info, metric_config_bounds=base_data.metric_config_bounds, ) @@ -240,7 +275,8 @@ def electrical_component_from_proto_with_issues( model=base_data.model, category=base_data.category, operational_lifetime=base_data.lifetime, - operational_mode=base_data.operational_mode, + _provides_telemetry=base_data.provides_telemetry, + _accepts_control=base_data.accepts_control, metric_config_bounds=base_data.metric_config_bounds, ) case ( @@ -262,7 +298,8 @@ def electrical_component_from_proto_with_issues( name=base_data.name, model=base_data.model, operational_lifetime=base_data.lifetime, - operational_mode=base_data.operational_mode, + _provides_telemetry=base_data.provides_telemetry, + _accepts_control=base_data.accepts_control, metric_config_bounds=base_data.metric_config_bounds, ) case ElectricalComponentCategory.BATTERY: @@ -286,7 +323,8 @@ def electrical_component_from_proto_with_issues( name=base_data.name, model=base_data.model, operational_lifetime=base_data.lifetime, - operational_mode=base_data.operational_mode, + _provides_telemetry=base_data.provides_telemetry, + _accepts_control=base_data.accepts_control, metric_config_bounds=base_data.metric_config_bounds, ) case int(): @@ -297,7 +335,8 @@ def electrical_component_from_proto_with_issues( name=base_data.name, model=base_data.model, operational_lifetime=base_data.lifetime, - operational_mode=base_data.operational_mode, + _provides_telemetry=base_data.provides_telemetry, + _accepts_control=base_data.accepts_control, metric_config_bounds=base_data.metric_config_bounds, type=battery_type, ) @@ -333,7 +372,8 @@ def electrical_component_from_proto_with_issues( name=base_data.name, model=base_data.model, operational_lifetime=base_data.lifetime, - operational_mode=base_data.operational_mode, + _provides_telemetry=base_data.provides_telemetry, + _accepts_control=base_data.accepts_control, metric_config_bounds=base_data.metric_config_bounds, ) case int(): @@ -346,7 +386,8 @@ def electrical_component_from_proto_with_issues( name=base_data.name, model=base_data.model, operational_lifetime=base_data.lifetime, - operational_mode=base_data.operational_mode, + _provides_telemetry=base_data.provides_telemetry, + _accepts_control=base_data.accepts_control, metric_config_bounds=base_data.metric_config_bounds, type=ev_charger_type, ) @@ -363,7 +404,8 @@ def electrical_component_from_proto_with_issues( name=base_data.name, model=base_data.model, operational_lifetime=base_data.lifetime, - operational_mode=base_data.operational_mode, + _provides_telemetry=base_data.provides_telemetry, + _accepts_control=base_data.accepts_control, metric_config_bounds=base_data.metric_config_bounds, rated_fuse_current=rated_fuse_current, ) @@ -397,7 +439,8 @@ def electrical_component_from_proto_with_issues( name=base_data.name, model=base_data.model, operational_lifetime=base_data.lifetime, - operational_mode=base_data.operational_mode, + _provides_telemetry=base_data.provides_telemetry, + _accepts_control=base_data.accepts_control, metric_config_bounds=base_data.metric_config_bounds, ) case int(): @@ -410,7 +453,8 @@ def electrical_component_from_proto_with_issues( name=base_data.name, model=base_data.model, operational_lifetime=base_data.lifetime, - operational_mode=base_data.operational_mode, + _provides_telemetry=base_data.provides_telemetry, + _accepts_control=base_data.accepts_control, metric_config_bounds=base_data.metric_config_bounds, type=inverter_type, ) @@ -423,7 +467,8 @@ def electrical_component_from_proto_with_issues( name=base_data.name, model=base_data.model, operational_lifetime=base_data.lifetime, - operational_mode=base_data.operational_mode, + _provides_telemetry=base_data.provides_telemetry, + _accepts_control=base_data.accepts_control, metric_config_bounds=base_data.metric_config_bounds, primary_voltage=message.category_specific_info.power_transformer.primary, secondary_voltage=message.category_specific_info.power_transformer.secondary, @@ -445,7 +490,8 @@ def electrical_component_from_proto_with_issues( model=base_data.model, category=base_data.category.value, operational_lifetime=base_data.lifetime, - operational_mode=base_data.operational_mode, + _provides_telemetry=base_data.provides_telemetry, + _accepts_control=base_data.accepts_control, metric_config_bounds=base_data.metric_config_bounds, ) case unexpected_category: diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py b/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py index 14852e80..1875f891 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py @@ -19,7 +19,6 @@ ElectricalComponent, ElectricalComponentCategory, ElectricalComponentId, - ElectricalComponentOperationalMode, ) from frequenz.client.common.microgrid.electrical_components.proto.v1alpha8._electrical_component import ( # noqa: E501 _ElectricalComponentBaseData, @@ -63,7 +62,8 @@ def default_component_base_data( lifetime=DEFAULT_LIFETIME, metric_config_bounds={Metric.AC_ENERGY_ACTIVE: Bounds(lower=0, upper=100)}, category_specific_info={}, - operational_mode=ElectricalComponentOperationalMode.CONTROL_AND_TELEMETRY, + provides_telemetry=True, + accepts_control=True, category_mismatched=False, ) @@ -78,11 +78,36 @@ def assert_base_data( assert base_data.model == other.model assert base_data.category == other.category assert base_data.lifetime == other.operational_lifetime - assert base_data.operational_mode == other.operational_mode + # pylint: disable=protected-access + assert base_data.provides_telemetry == other._provides_telemetry + assert base_data.accepts_control == other._accepts_control + # pylint: enable=protected-access assert base_data.metric_config_bounds == other.metric_config_bounds assert base_data.category_specific_info == other.category_specific_metadata +_OPERATIONAL_MODE_BY_BOOLS: dict[ + tuple[bool | None, bool | None], + electrical_components_pb2.ElectricalComponentOperationalMode.ValueType, +] = { + (None, None): ( + electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_UNSPECIFIED + ), + (False, False): ( + electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_INACTIVE + ), + (True, False): ( + electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_TELEMETRY_ONLY + ), + (False, True): ( + electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_CONTROL_ONLY + ), + (True, True): ( + electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_CONTROL_AND_TELEMETRY + ), +} + + def base_data_as_proto( base_data: _ElectricalComponentBaseData, ) -> electrical_components_pb2.ElectricalComponent: @@ -97,11 +122,9 @@ def base_data_as_proto( if isinstance(base_data.category, int) else int(base_data.category.value) # type: ignore[arg-type] ), - operational_mode=( - base_data.operational_mode - if isinstance(base_data.operational_mode, int) - else int(base_data.operational_mode.value) # type: ignore[arg-type] - ), + operational_mode=_OPERATIONAL_MODE_BY_BOOLS[ + (base_data.provides_telemetry, base_data.accepts_control) + ], ) if base_data.lifetime: lifetime_dict: dict[str, Timestamp] = {} diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py index 5acae128..9ec77f87 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py @@ -3,6 +3,7 @@ """Tests for protobuf conversion of the base/common part of electrical components.""" +import pytest from frequenz.api.common.v1alpha8.microgrid.electrical_components import ( electrical_components_pb2, ) @@ -14,12 +15,54 @@ from frequenz.client.common.microgrid.electrical_components.proto.v1alpha8._electrical_component import ( # noqa: E501 _electrical_component_base_from_proto_with_issues, _ElectricalComponentBaseData, + _operational_mode_to_bools, ) from frequenz.client.common.types import Lifetime from .conftest import base_data_as_proto +@pytest.mark.parametrize( + "proto_value, expected", + [ + ( + electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_UNSPECIFIED, + (None, None), + ), + ( + electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_INACTIVE, + (False, False), + ), + ( + electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_TELEMETRY_ONLY, + (True, False), + ), + ( + electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_CONTROL_ONLY, + (False, True), + ), + ( + electrical_components_pb2.ELECTRICAL_COMPONENT_OPERATIONAL_MODE_CONTROL_AND_TELEMETRY, + (True, True), + ), + (999, (None, None)), + ], + ids=[ + "unspecified", + "inactive", + "telemetry-only", + "control-only", + "control-and-telemetry", + "unknown-int", + ], +) +def test_operational_mode_to_bools( + proto_value: int, expected: tuple[bool | None, bool | None] +) -> None: + """Test that proto operational-mode values map to (provides_telemetry, accepts_control).""" + assert _operational_mode_to_bools(proto_value) == expected + + def test_complete(default_component_base_data: _ElectricalComponentBaseData) -> None: """Test parsing of a complete base component proto.""" major_issues: list[str] = [] diff --git a/tests/microgrid/electrical_components/test_battery.py b/tests/microgrid/electrical_components/test_battery.py index 4fdbea4b..3261401d 100644 --- a/tests/microgrid/electrical_components/test_battery.py +++ b/tests/microgrid/electrical_components/test_battery.py @@ -51,6 +51,8 @@ def test_abstract_battery_cannot_be_instantiated( microgrid_id=microgrid_id, name="test_battery", type=BatteryType.LI_ION, + _provides_telemetry=True, + _accepts_control=True, ) @@ -81,6 +83,8 @@ def test_recognized_battery_types( id=component_id, microgrid_id=microgrid_id, name=case.name, + _provides_telemetry=True, + _accepts_control=True, ) assert battery.id == component_id @@ -99,6 +103,8 @@ def test_unrecognized_battery_type( microgrid_id=microgrid_id, name="unrecognized_battery", type=999, + _provides_telemetry=True, + _accepts_control=True, ) assert battery.id == component_id diff --git a/tests/microgrid/electrical_components/test_electrical_component_base.py b/tests/microgrid/electrical_components/test_electrical_component_base.py index 67d3e56c..485407fc 100644 --- a/tests/microgrid/electrical_components/test_electrical_component_base.py +++ b/tests/microgrid/electrical_components/test_electrical_component_base.py @@ -9,13 +9,13 @@ import pytest +from frequenz.client.common import UnspecifiedValueError from frequenz.client.common.metrics import Bounds, Metric from frequenz.client.common.microgrid import MicrogridId from frequenz.client.common.microgrid.electrical_components import ( ElectricalComponent, ElectricalComponentCategory, ElectricalComponentId, - ElectricalComponentOperationalMode, ) from frequenz.client.common.types import Lifetime @@ -37,6 +37,8 @@ def test_base_creation_fails() -> None: id=ElectricalComponentId(1), microgrid_id=MicrogridId(1), category=ElectricalComponentCategory.UNSPECIFIED, + _provides_telemetry=True, + _accepts_control=True, ) @@ -46,12 +48,13 @@ def test_creation_with_defaults() -> None: id=ElectricalComponentId(1), microgrid_id=MicrogridId(2), category=ElectricalComponentCategory.UNSPECIFIED, + _provides_telemetry=True, + _accepts_control=True, ) assert component.name is None assert component.model is None assert component.operational_lifetime == Lifetime() - assert component.operational_mode == ElectricalComponentOperationalMode.UNSPECIFIED assert component.metric_config_bounds == {} assert component.category_specific_metadata == {} @@ -70,6 +73,8 @@ def test_creation_full() -> None: model="Test Manufacturer Test Model", metric_config_bounds=metric_config_bounds, category_specific_metadata=metadata, + _provides_telemetry=True, + _accepts_control=True, ) assert component.name == "test-component" @@ -78,6 +83,36 @@ def test_creation_full() -> None: assert component.category_specific_metadata == metadata +def test_accessors_return_values_when_set() -> None: + """Test that telemetry/control accessors return the stored booleans.""" + component = _TestElectricalComponent( + id=ElectricalComponentId(1), + microgrid_id=MicrogridId(2), + category=ElectricalComponentCategory.UNSPECIFIED, + _provides_telemetry=True, + _accepts_control=False, + ) + + assert component.provides_telemetry() is True + assert component.accepts_control() is False + + +def test_accessors_raise_when_unspecified() -> None: + """Test that accessors raise UnspecifiedValueError when the value is unknown.""" + component = _TestElectricalComponent( + id=ElectricalComponentId(1), + microgrid_id=MicrogridId(2), + category=ElectricalComponentCategory.UNSPECIFIED, + _provides_telemetry=None, + _accepts_control=None, + ) + + with pytest.raises(UnspecifiedValueError): + component.provides_telemetry() + with pytest.raises(UnspecifiedValueError): + component.accepts_control() + + @pytest.mark.parametrize( "name,expected_str", [ @@ -93,6 +128,8 @@ def test_str(name: str | None, expected_str: str) -> None: microgrid_id=MicrogridId(2), category=ElectricalComponentCategory.UNSPECIFIED, name=name, + _provides_telemetry=True, + _accepts_control=True, ) assert str(component) == expected_str @@ -110,6 +147,8 @@ def test_operational_at(is_operational: bool) -> None: microgrid_id=MicrogridId(1), category=ElectricalComponentCategory.UNSPECIFIED, operational_lifetime=mock_lifetime, + _provides_telemetry=True, + _accepts_control=True, ) test_time = datetime.now(timezone.utc) @@ -133,6 +172,8 @@ def test_is_operational_now(mock_datetime: Mock) -> None: microgrid_id=MicrogridId(1), category=ElectricalComponentCategory.UNSPECIFIED, operational_lifetime=mock_lifetime, + _provides_telemetry=True, + _accepts_control=True, ) assert component.is_operational_now() is True @@ -147,6 +188,8 @@ def test_is_operational_now(mock_datetime: Mock) -> None: name="test", metric_config_bounds={Metric.AC_POWER_ACTIVE: Bounds(lower=-100.0, upper=100.0)}, category_specific_metadata={"key": "value"}, + _provides_telemetry=True, + _accepts_control=True, ) DIFFERENT_NONHASHABLE = _TestElectricalComponent( @@ -156,6 +199,8 @@ def test_is_operational_now(mock_datetime: Mock) -> None: name=COMPONENT.name, metric_config_bounds={Metric.AC_POWER_ACTIVE: Bounds(lower=-200.0, upper=200.0)}, category_specific_metadata={"different": "metadata"}, + _provides_telemetry=True, + _accepts_control=True, ) DIFFERENT_NAME = _TestElectricalComponent( @@ -165,6 +210,8 @@ def test_is_operational_now(mock_datetime: Mock) -> None: name="different", metric_config_bounds=COMPONENT.metric_config_bounds, category_specific_metadata=COMPONENT.category_specific_metadata, + _provides_telemetry=True, + _accepts_control=True, ) DIFFERENT_ID = _TestElectricalComponent( @@ -174,6 +221,8 @@ def test_is_operational_now(mock_datetime: Mock) -> None: name=COMPONENT.name, metric_config_bounds=COMPONENT.metric_config_bounds, category_specific_metadata=COMPONENT.category_specific_metadata, + _provides_telemetry=True, + _accepts_control=True, ) DIFFERENT_MICROGRID_ID = _TestElectricalComponent( @@ -183,6 +232,8 @@ def test_is_operational_now(mock_datetime: Mock) -> None: name=COMPONENT.name, metric_config_bounds=COMPONENT.metric_config_bounds, category_specific_metadata=COMPONENT.category_specific_metadata, + _provides_telemetry=True, + _accepts_control=True, ) DIFFERENT_BOTH_ID = _TestElectricalComponent( @@ -192,6 +243,8 @@ def test_is_operational_now(mock_datetime: Mock) -> None: name=COMPONENT.name, metric_config_bounds=COMPONENT.metric_config_bounds, category_specific_metadata=COMPONENT.category_specific_metadata, + _provides_telemetry=True, + _accepts_control=True, ) diff --git a/tests/microgrid/electrical_components/test_ev_charger.py b/tests/microgrid/electrical_components/test_ev_charger.py index ef30e68f..6a398d40 100644 --- a/tests/microgrid/electrical_components/test_ev_charger.py +++ b/tests/microgrid/electrical_components/test_ev_charger.py @@ -52,6 +52,8 @@ def test_abstract_ev_charger_cannot_be_instantiated( microgrid_id=microgrid_id, name="test_charger", type=EvChargerType.AC, + _provides_telemetry=True, + _accepts_control=True, ) @@ -83,6 +85,8 @@ def test_recognized_ev_charger_types( # Renamed from test_ev_charger_types id=component_id, microgrid_id=microgrid_id, name=case.name, + _provides_telemetry=True, + _accepts_control=True, ) assert charger.id == component_id @@ -101,6 +105,8 @@ def test_unrecognized_ev_charger_type( microgrid_id=microgrid_id, name="unrecognized_charger", type=999, # type is passed here for UnrecognizedEvCharger + _provides_telemetry=True, + _accepts_control=True, ) assert charger.id == component_id diff --git a/tests/microgrid/electrical_components/test_grid_connection_point.py b/tests/microgrid/electrical_components/test_grid_connection_point.py index c4f3e39f..1521ac8f 100644 --- a/tests/microgrid/electrical_components/test_grid_connection_point.py +++ b/tests/microgrid/electrical_components/test_grid_connection_point.py @@ -37,6 +37,8 @@ def test_creation_ok( microgrid_id=microgrid_id, name="test_grid_point", rated_fuse_current=rated_fuse_current, + _provides_telemetry=True, + _accepts_control=True, ) assert grid_point.id == component_id @@ -58,4 +60,6 @@ def test_creation_invalid_rated_fuse_current( microgrid_id=microgrid_id, name="test_grid_point", rated_fuse_current=-1, + _provides_telemetry=True, + _accepts_control=True, ) diff --git a/tests/microgrid/electrical_components/test_inverter.py b/tests/microgrid/electrical_components/test_inverter.py index 0981c827..eb66ef4c 100644 --- a/tests/microgrid/electrical_components/test_inverter.py +++ b/tests/microgrid/electrical_components/test_inverter.py @@ -52,6 +52,8 @@ def test_abstract_inverter_cannot_be_instantiated( microgrid_id=microgrid_id, name="test_inverter", type=InverterType.BATTERY, + _provides_telemetry=True, + _accepts_control=True, ) @@ -83,6 +85,8 @@ def test_recognized_inverter_types( id=component_id, microgrid_id=microgrid_id, name=case.name, + _provides_telemetry=True, + _accepts_control=True, ) assert inverter.id == component_id @@ -101,6 +105,8 @@ def test_unrecognized_inverter_type( microgrid_id=microgrid_id, name="unrecognized_inverter", type=999, # type is passed here for UnrecognizedInverter + _provides_telemetry=True, + _accepts_control=True, ) assert inverter.id == component_id diff --git a/tests/microgrid/electrical_components/test_power_transformer.py b/tests/microgrid/electrical_components/test_power_transformer.py index b524fb48..7751cbed 100644 --- a/tests/microgrid/electrical_components/test_power_transformer.py +++ b/tests/microgrid/electrical_components/test_power_transformer.py @@ -41,6 +41,8 @@ def test_creation_ok( name="test_power_transformer", primary_voltage=primary, secondary_voltage=secondary, + _provides_telemetry=True, + _accepts_control=True, ) assert power_transformer.id == component_id diff --git a/tests/microgrid/electrical_components/test_problematic.py b/tests/microgrid/electrical_components/test_problematic.py index 62fed103..d885aa72 100644 --- a/tests/microgrid/electrical_components/test_problematic.py +++ b/tests/microgrid/electrical_components/test_problematic.py @@ -40,6 +40,8 @@ def test_abstract_problematic_electrical_component_cannot_be_instantiated( microgrid_id=microgrid_id, name="test_problematic", category=ElectricalComponentCategory.UNSPECIFIED, + _provides_telemetry=True, + _accepts_control=True, ) @@ -51,6 +53,8 @@ def test_unspecified_component( id=component_id, microgrid_id=microgrid_id, name="unspecified_component", + _provides_telemetry=True, + _accepts_control=True, ) assert component.id == component_id @@ -69,6 +73,8 @@ def test_mismatched_category_component_with_known_category( microgrid_id=microgrid_id, name="mismatched_battery", category=expected_category, + _provides_telemetry=True, + _accepts_control=True, ) assert component.id == component_id @@ -87,6 +93,8 @@ def test_mismatched_category_component_with_unrecognized_category( microgrid_id=microgrid_id, name="mismatched_unrecognized", category=expected_category, + _provides_telemetry=True, + _accepts_control=True, ) assert component.id == component_id @@ -104,6 +112,8 @@ def test_unrecognized_component_type( microgrid_id=microgrid_id, name="unrecognized_component", category=999, + _provides_telemetry=True, + _accepts_control=True, ) assert component.id == component_id diff --git a/tests/microgrid/electrical_components/test_simple_components.py b/tests/microgrid/electrical_components/test_simple_components.py index 70668f8a..7f98f9a3 100644 --- a/tests/microgrid/electrical_components/test_simple_components.py +++ b/tests/microgrid/electrical_components/test_simple_components.py @@ -71,6 +71,8 @@ def test_init( id=component_id, microgrid_id=microgrid_id, name="test_component", + _provides_telemetry=True, + _accepts_control=True, ) assert component.id == component_id From dadbf110b7fd4775986d2f0cba3bd0f4aeafa081 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 19 Jun 2026 14:41:03 +0200 Subject: [PATCH 03/10] Remove `ElectricalComponentOperationalMode` Signed-off-by: Leandro Lucarella --- .../electrical_components/__init__.py | 2 - .../proto/v1alpha8/__init__.py | 6 --- .../proto/v1alpha8/_operational_mode.py | 42 ------------------- .../proto/v1alpha8/test_operational_mode.py | 27 ------------ 4 files changed, 77 deletions(-) delete mode 100644 src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_operational_mode.py delete mode 100644 tests/microgrid/electrical_components/proto/v1alpha8/test_operational_mode.py diff --git a/src/frequenz/client/common/microgrid/electrical_components/__init__.py b/src/frequenz/client/common/microgrid/electrical_components/__init__.py index 6bc0c0e3..d1e75c50 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/__init__.py +++ b/src/frequenz/client/common/microgrid/electrical_components/__init__.py @@ -45,7 +45,6 @@ UnspecifiedInverter, ) from ._meter import Meter -from ._operational_mode import ElectricalComponentOperationalMode from ._power_transformer import PowerTransformer from ._precharger import Precharger from ._problematic import ( @@ -80,7 +79,6 @@ "ElectricalComponentConnection", "ElectricalComponentDiagnosticCode", "ElectricalComponentId", - "ElectricalComponentOperationalMode", "ElectricalComponentStateCode", "ElectricalComponentTypes", "Electrolyzer", diff --git a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/__init__.py b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/__init__.py index 357e068a..21a5bd1f 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/__init__.py +++ b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/__init__.py @@ -22,10 +22,6 @@ ) from ._ev_charger import ev_charger_type_from_proto, ev_charger_type_to_proto from ._inverter import inverter_type_from_proto, inverter_type_to_proto -from ._operational_mode import ( - electrical_component_operational_mode_from_proto, - electrical_component_operational_mode_to_proto, -) from ._state_code import ( electrical_component_state_code_from_proto, electrical_component_state_code_to_proto, @@ -42,8 +38,6 @@ "electrical_component_diagnostic_code_to_proto", "electrical_component_from_proto", "electrical_component_from_proto_with_issues", - "electrical_component_operational_mode_from_proto", - "electrical_component_operational_mode_to_proto", "electrical_component_state_code_from_proto", "electrical_component_state_code_to_proto", "ev_charger_type_from_proto", diff --git a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_operational_mode.py b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_operational_mode.py deleted file mode 100644 index 49d8fac3..00000000 --- a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_operational_mode.py +++ /dev/null @@ -1,42 +0,0 @@ -# License: MIT -# Copyright © 2026 Frequenz Energy-as-a-Service GmbH - -"""Conversion of electrical component operational modes to/from protobuf v1alpha8.""" - -from frequenz.api.common.v1alpha8.microgrid.electrical_components import ( - electrical_components_pb2, -) - -from .....proto import enum_from_proto -from ... import ElectricalComponentOperationalMode - - -def electrical_component_operational_mode_from_proto( - message: electrical_components_pb2.ElectricalComponentOperationalMode.ValueType, -) -> ElectricalComponentOperationalMode | int: - """Convert a protobuf ElectricalComponentOperationalMode enum value to an enum member. - - Args: - message: A protobuf ElectricalComponentOperationalMode enum value. - - Returns: - The corresponding ElectricalComponentOperationalMode enum member, or the raw - `int` if the protobuf value is not recognized. - """ - return enum_from_proto(message, ElectricalComponentOperationalMode) - - -def electrical_component_operational_mode_to_proto( - operational_mode: ElectricalComponentOperationalMode, -) -> electrical_components_pb2.ElectricalComponentOperationalMode.ValueType: - """Convert an ElectricalComponentOperationalMode enum member to a protobuf enum value. - - Args: - operational_mode: An ElectricalComponentOperationalMode enum member. - - Returns: - The corresponding protobuf ElectricalComponentOperationalMode enum value. - """ - return electrical_components_pb2.ElectricalComponentOperationalMode.ValueType( - operational_mode.value - ) diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/test_operational_mode.py b/tests/microgrid/electrical_components/proto/v1alpha8/test_operational_mode.py deleted file mode 100644 index 1d741efd..00000000 --- a/tests/microgrid/electrical_components/proto/v1alpha8/test_operational_mode.py +++ /dev/null @@ -1,27 +0,0 @@ -# License: MIT -# Copyright © 2026 Frequenz Energy-as-a-Service GmbH - -"""Tests for electrical component operational mode to/from protobuf v1alpha8 conversion.""" - -from frequenz.api.common.v1alpha8.microgrid.electrical_components import ( - electrical_components_pb2, -) - -from frequenz.client.common.microgrid.electrical_components import ( - ElectricalComponentOperationalMode, -) -from frequenz.client.common.microgrid.electrical_components.proto.v1alpha8 import ( - electrical_component_operational_mode_from_proto, - electrical_component_operational_mode_to_proto, -) -from frequenz.client.common.test.enum_parity import EnumParityTest - - -class TestElectricalComponentOperationalModeParity(EnumParityTest): - """Parity tests for the `ElectricalComponentOperationalMode` enum.""" - - python_enum = ElectricalComponentOperationalMode - proto_enum = electrical_components_pb2.ElectricalComponentOperationalMode - name_prefix = "ELECTRICAL_COMPONENT_OPERATIONAL_MODE_" - from_proto = staticmethod(electrical_component_operational_mode_from_proto) - to_proto = staticmethod(electrical_component_operational_mode_to_proto) From 85115f193111e25830434f7a3eaae26937fd85ba Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 19 Jun 2026 14:33:19 +0200 Subject: [PATCH 04/10] Forbid constructing electrical components directly Electrical components are normally loaded from protobuf, and Python code should never create them, but get them from API services. We forbid constructing electrical components, but add a hidden escape hatch to be able to create them in `_from_proto()` functions. Signed-off-by: Leandro Lucarella --- .../_electrical_component.py | 18 +++++++++++++ .../_grid_connection_point.py | 3 ++- .../proto/v1alpha8/_electrical_component.py | 12 +++++++++ .../electrical_components/test_battery.py | 2 ++ .../test_electrical_component_base.py | 25 +++++++++++++++++++ .../electrical_components/test_ev_charger.py | 2 ++ .../test_grid_connection_point.py | 16 ++++++++++++ .../electrical_components/test_inverter.py | 2 ++ .../test_power_transformer.py | 1 + .../electrical_components/test_problematic.py | 4 +++ .../test_simple_components.py | 1 + 11 files changed, 85 insertions(+), 1 deletion(-) diff --git a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py index 246f69cd..93e81585 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py @@ -57,6 +57,11 @@ class ElectricalComponent: # pylint: disable=too-many-instance-attributes _accepts_control: bool | None """Whether this component accepts control commands, or `None` if unspecified.""" + _allow_construction: bool = dataclasses.field( + default=False, repr=False, compare=False, hash=False + ) + """Internal guard allowing construction only via the `*_from_proto` converters.""" + metric_config_bounds: Mapping[Metric | int, Bounds] = dataclasses.field( default_factory=dict, # dict is not hashable, so we don't use this field to calculate the hash. This @@ -94,6 +99,19 @@ def __new__(cls, *_: Any, **__: Any) -> Self: raise TypeError(f"Cannot instantiate {cls.__name__} directly") return super().__new__(cls) + def __post_init__(self) -> None: + """Reject direct construction of this read-only type. + + Raises: + TypeError: If the instance was not created via the corresponding + `*_from_proto` converter. + """ + if not self._allow_construction: + raise TypeError( + f"{type(self).__name__} cannot be constructed directly; obtain " + "instances via the corresponding *_from_proto converter." + ) + def provides_telemetry(self) -> bool: """Check whether this electrical component provides telemetry data. diff --git a/src/frequenz/client/common/microgrid/electrical_components/_grid_connection_point.py b/src/frequenz/client/common/microgrid/electrical_components/_grid_connection_point.py index ef6ee413..cfc8606a 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/_grid_connection_point.py +++ b/src/frequenz/client/common/microgrid/electrical_components/_grid_connection_point.py @@ -54,7 +54,8 @@ class GridConnectionPoint(ElectricalComponent): """ def __post_init__(self) -> None: - """Validate the fuse's rated current.""" + """Run the base construction gate and validate the fuse's rated current.""" + super().__post_init__() if self.rated_fuse_current < 0: raise ValueError( f"rated_fuse_current must be a non-negative integer, not {self.rated_fuse_current}" diff --git a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py index f3dac828..0f3c61e9 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py @@ -262,6 +262,7 @@ def electrical_component_from_proto_with_issues( operational_lifetime=base_data.lifetime, _provides_telemetry=base_data.provides_telemetry, _accepts_control=base_data.accepts_control, + _allow_construction=True, category_specific_metadata=base_data.category_specific_info, metric_config_bounds=base_data.metric_config_bounds, ) @@ -277,6 +278,7 @@ def electrical_component_from_proto_with_issues( operational_lifetime=base_data.lifetime, _provides_telemetry=base_data.provides_telemetry, _accepts_control=base_data.accepts_control, + _allow_construction=True, metric_config_bounds=base_data.metric_config_bounds, ) case ( @@ -300,6 +302,7 @@ def electrical_component_from_proto_with_issues( operational_lifetime=base_data.lifetime, _provides_telemetry=base_data.provides_telemetry, _accepts_control=base_data.accepts_control, + _allow_construction=True, metric_config_bounds=base_data.metric_config_bounds, ) case ElectricalComponentCategory.BATTERY: @@ -325,6 +328,7 @@ def electrical_component_from_proto_with_issues( operational_lifetime=base_data.lifetime, _provides_telemetry=base_data.provides_telemetry, _accepts_control=base_data.accepts_control, + _allow_construction=True, metric_config_bounds=base_data.metric_config_bounds, ) case int(): @@ -337,6 +341,7 @@ def electrical_component_from_proto_with_issues( operational_lifetime=base_data.lifetime, _provides_telemetry=base_data.provides_telemetry, _accepts_control=base_data.accepts_control, + _allow_construction=True, metric_config_bounds=base_data.metric_config_bounds, type=battery_type, ) @@ -374,6 +379,7 @@ def electrical_component_from_proto_with_issues( operational_lifetime=base_data.lifetime, _provides_telemetry=base_data.provides_telemetry, _accepts_control=base_data.accepts_control, + _allow_construction=True, metric_config_bounds=base_data.metric_config_bounds, ) case int(): @@ -388,6 +394,7 @@ def electrical_component_from_proto_with_issues( operational_lifetime=base_data.lifetime, _provides_telemetry=base_data.provides_telemetry, _accepts_control=base_data.accepts_control, + _allow_construction=True, metric_config_bounds=base_data.metric_config_bounds, type=ev_charger_type, ) @@ -406,6 +413,7 @@ def electrical_component_from_proto_with_issues( operational_lifetime=base_data.lifetime, _provides_telemetry=base_data.provides_telemetry, _accepts_control=base_data.accepts_control, + _allow_construction=True, metric_config_bounds=base_data.metric_config_bounds, rated_fuse_current=rated_fuse_current, ) @@ -441,6 +449,7 @@ def electrical_component_from_proto_with_issues( operational_lifetime=base_data.lifetime, _provides_telemetry=base_data.provides_telemetry, _accepts_control=base_data.accepts_control, + _allow_construction=True, metric_config_bounds=base_data.metric_config_bounds, ) case int(): @@ -455,6 +464,7 @@ def electrical_component_from_proto_with_issues( operational_lifetime=base_data.lifetime, _provides_telemetry=base_data.provides_telemetry, _accepts_control=base_data.accepts_control, + _allow_construction=True, metric_config_bounds=base_data.metric_config_bounds, type=inverter_type, ) @@ -469,6 +479,7 @@ def electrical_component_from_proto_with_issues( operational_lifetime=base_data.lifetime, _provides_telemetry=base_data.provides_telemetry, _accepts_control=base_data.accepts_control, + _allow_construction=True, metric_config_bounds=base_data.metric_config_bounds, primary_voltage=message.category_specific_info.power_transformer.primary, secondary_voltage=message.category_specific_info.power_transformer.secondary, @@ -492,6 +503,7 @@ def electrical_component_from_proto_with_issues( operational_lifetime=base_data.lifetime, _provides_telemetry=base_data.provides_telemetry, _accepts_control=base_data.accepts_control, + _allow_construction=True, metric_config_bounds=base_data.metric_config_bounds, ) case unexpected_category: diff --git a/tests/microgrid/electrical_components/test_battery.py b/tests/microgrid/electrical_components/test_battery.py index 3261401d..86600aee 100644 --- a/tests/microgrid/electrical_components/test_battery.py +++ b/tests/microgrid/electrical_components/test_battery.py @@ -85,6 +85,7 @@ def test_recognized_battery_types( name=case.name, _provides_telemetry=True, _accepts_control=True, + _allow_construction=True, ) assert battery.id == component_id @@ -105,6 +106,7 @@ def test_unrecognized_battery_type( type=999, _provides_telemetry=True, _accepts_control=True, + _allow_construction=True, ) assert battery.id == component_id diff --git a/tests/microgrid/electrical_components/test_electrical_component_base.py b/tests/microgrid/electrical_components/test_electrical_component_base.py index 485407fc..e7fb7e88 100644 --- a/tests/microgrid/electrical_components/test_electrical_component_base.py +++ b/tests/microgrid/electrical_components/test_electrical_component_base.py @@ -42,6 +42,18 @@ def test_base_creation_fails() -> None: ) +def test_direct_construction_without_flag_raises() -> None: + """Test that a concrete component cannot be built without the construction flag.""" + with pytest.raises(TypeError, match="cannot be constructed directly"): + _TestElectricalComponent( + id=ElectricalComponentId(1), + microgrid_id=MicrogridId(2), + category=ElectricalComponentCategory.UNSPECIFIED, + _provides_telemetry=True, + _accepts_control=True, + ) + + def test_creation_with_defaults() -> None: """Test electrical component default values.""" component = _TestElectricalComponent( @@ -50,6 +62,7 @@ def test_creation_with_defaults() -> None: category=ElectricalComponentCategory.UNSPECIFIED, _provides_telemetry=True, _accepts_control=True, + _allow_construction=True, ) assert component.name is None @@ -75,6 +88,7 @@ def test_creation_full() -> None: category_specific_metadata=metadata, _provides_telemetry=True, _accepts_control=True, + _allow_construction=True, ) assert component.name == "test-component" @@ -91,6 +105,7 @@ def test_accessors_return_values_when_set() -> None: category=ElectricalComponentCategory.UNSPECIFIED, _provides_telemetry=True, _accepts_control=False, + _allow_construction=True, ) assert component.provides_telemetry() is True @@ -105,6 +120,7 @@ def test_accessors_raise_when_unspecified() -> None: category=ElectricalComponentCategory.UNSPECIFIED, _provides_telemetry=None, _accepts_control=None, + _allow_construction=True, ) with pytest.raises(UnspecifiedValueError): @@ -130,6 +146,7 @@ def test_str(name: str | None, expected_str: str) -> None: name=name, _provides_telemetry=True, _accepts_control=True, + _allow_construction=True, ) assert str(component) == expected_str @@ -149,6 +166,7 @@ def test_operational_at(is_operational: bool) -> None: operational_lifetime=mock_lifetime, _provides_telemetry=True, _accepts_control=True, + _allow_construction=True, ) test_time = datetime.now(timezone.utc) @@ -174,6 +192,7 @@ def test_is_operational_now(mock_datetime: Mock) -> None: operational_lifetime=mock_lifetime, _provides_telemetry=True, _accepts_control=True, + _allow_construction=True, ) assert component.is_operational_now() is True @@ -190,6 +209,7 @@ def test_is_operational_now(mock_datetime: Mock) -> None: category_specific_metadata={"key": "value"}, _provides_telemetry=True, _accepts_control=True, + _allow_construction=True, ) DIFFERENT_NONHASHABLE = _TestElectricalComponent( @@ -201,6 +221,7 @@ def test_is_operational_now(mock_datetime: Mock) -> None: category_specific_metadata={"different": "metadata"}, _provides_telemetry=True, _accepts_control=True, + _allow_construction=True, ) DIFFERENT_NAME = _TestElectricalComponent( @@ -212,6 +233,7 @@ def test_is_operational_now(mock_datetime: Mock) -> None: category_specific_metadata=COMPONENT.category_specific_metadata, _provides_telemetry=True, _accepts_control=True, + _allow_construction=True, ) DIFFERENT_ID = _TestElectricalComponent( @@ -223,6 +245,7 @@ def test_is_operational_now(mock_datetime: Mock) -> None: category_specific_metadata=COMPONENT.category_specific_metadata, _provides_telemetry=True, _accepts_control=True, + _allow_construction=True, ) DIFFERENT_MICROGRID_ID = _TestElectricalComponent( @@ -234,6 +257,7 @@ def test_is_operational_now(mock_datetime: Mock) -> None: category_specific_metadata=COMPONENT.category_specific_metadata, _provides_telemetry=True, _accepts_control=True, + _allow_construction=True, ) DIFFERENT_BOTH_ID = _TestElectricalComponent( @@ -245,6 +269,7 @@ def test_is_operational_now(mock_datetime: Mock) -> None: category_specific_metadata=COMPONENT.category_specific_metadata, _provides_telemetry=True, _accepts_control=True, + _allow_construction=True, ) diff --git a/tests/microgrid/electrical_components/test_ev_charger.py b/tests/microgrid/electrical_components/test_ev_charger.py index 6a398d40..48c45d96 100644 --- a/tests/microgrid/electrical_components/test_ev_charger.py +++ b/tests/microgrid/electrical_components/test_ev_charger.py @@ -87,6 +87,7 @@ def test_recognized_ev_charger_types( # Renamed from test_ev_charger_types name=case.name, _provides_telemetry=True, _accepts_control=True, + _allow_construction=True, ) assert charger.id == component_id @@ -107,6 +108,7 @@ def test_unrecognized_ev_charger_type( type=999, # type is passed here for UnrecognizedEvCharger _provides_telemetry=True, _accepts_control=True, + _allow_construction=True, ) assert charger.id == component_id diff --git a/tests/microgrid/electrical_components/test_grid_connection_point.py b/tests/microgrid/electrical_components/test_grid_connection_point.py index 1521ac8f..8fea6cf1 100644 --- a/tests/microgrid/electrical_components/test_grid_connection_point.py +++ b/tests/microgrid/electrical_components/test_grid_connection_point.py @@ -39,6 +39,7 @@ def test_creation_ok( rated_fuse_current=rated_fuse_current, _provides_telemetry=True, _accepts_control=True, + _allow_construction=True, ) assert grid_point.id == component_id @@ -62,4 +63,19 @@ def test_creation_invalid_rated_fuse_current( rated_fuse_current=-1, _provides_telemetry=True, _accepts_control=True, + _allow_construction=True, + ) + + +def test_creation_without_flag_raises( + component_id: ElectricalComponentId, microgrid_id: MicrogridId +) -> None: + """Test that a GridConnectionPoint cannot be built without the construction flag.""" + with pytest.raises(TypeError, match="cannot be constructed directly"): + GridConnectionPoint( + id=component_id, + microgrid_id=microgrid_id, + rated_fuse_current=0, + _provides_telemetry=True, + _accepts_control=True, ) diff --git a/tests/microgrid/electrical_components/test_inverter.py b/tests/microgrid/electrical_components/test_inverter.py index eb66ef4c..4e03eb66 100644 --- a/tests/microgrid/electrical_components/test_inverter.py +++ b/tests/microgrid/electrical_components/test_inverter.py @@ -87,6 +87,7 @@ def test_recognized_inverter_types( name=case.name, _provides_telemetry=True, _accepts_control=True, + _allow_construction=True, ) assert inverter.id == component_id @@ -107,6 +108,7 @@ def test_unrecognized_inverter_type( type=999, # type is passed here for UnrecognizedInverter _provides_telemetry=True, _accepts_control=True, + _allow_construction=True, ) assert inverter.id == component_id diff --git a/tests/microgrid/electrical_components/test_power_transformer.py b/tests/microgrid/electrical_components/test_power_transformer.py index 7751cbed..1f221718 100644 --- a/tests/microgrid/electrical_components/test_power_transformer.py +++ b/tests/microgrid/electrical_components/test_power_transformer.py @@ -43,6 +43,7 @@ def test_creation_ok( secondary_voltage=secondary, _provides_telemetry=True, _accepts_control=True, + _allow_construction=True, ) assert power_transformer.id == component_id diff --git a/tests/microgrid/electrical_components/test_problematic.py b/tests/microgrid/electrical_components/test_problematic.py index d885aa72..2a06c0cc 100644 --- a/tests/microgrid/electrical_components/test_problematic.py +++ b/tests/microgrid/electrical_components/test_problematic.py @@ -55,6 +55,7 @@ def test_unspecified_component( name="unspecified_component", _provides_telemetry=True, _accepts_control=True, + _allow_construction=True, ) assert component.id == component_id @@ -75,6 +76,7 @@ def test_mismatched_category_component_with_known_category( category=expected_category, _provides_telemetry=True, _accepts_control=True, + _allow_construction=True, ) assert component.id == component_id @@ -95,6 +97,7 @@ def test_mismatched_category_component_with_unrecognized_category( category=expected_category, _provides_telemetry=True, _accepts_control=True, + _allow_construction=True, ) assert component.id == component_id @@ -114,6 +117,7 @@ def test_unrecognized_component_type( category=999, _provides_telemetry=True, _accepts_control=True, + _allow_construction=True, ) assert component.id == component_id diff --git a/tests/microgrid/electrical_components/test_simple_components.py b/tests/microgrid/electrical_components/test_simple_components.py index 7f98f9a3..8a9a505d 100644 --- a/tests/microgrid/electrical_components/test_simple_components.py +++ b/tests/microgrid/electrical_components/test_simple_components.py @@ -71,6 +71,7 @@ def test_init( id=component_id, microgrid_id=microgrid_id, name="test_component", + _allow_construction=True, _provides_telemetry=True, _accepts_control=True, ) From b2e83fc681f18458e43b61f1a58cb52c90c13a8c Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 19 Jun 2026 14:46:50 +0200 Subject: [PATCH 05/10] Drop unspecified metrics `from metric_config_bounds` The idea is to get rid of all unspecified values in Python code, so this is a step towards that direction. Having a UNSPECIFIED value in that map is really not helpful, there is not much a user can do with that value. Signed-off-by: Leandro Lucarella --- .../_electrical_component.py | 4 ++ .../proto/v1alpha8/_electrical_component.py | 5 ++- .../test_electrical_component_base.py | 40 +++++++++++++++++++ 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py index 93e81585..3cc32696 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py @@ -74,6 +74,10 @@ class ElectricalComponent: # pylint: disable=too-many-instance-attributes These bounds may be derived from the component configuration, manufacturer limits, or limits of other devices. + + The keys never include `Metric.UNSPECIFIED`: such entries are dropped when + loading from protobuf. Metrics unknown to this client version may still appear + as plain `int` keys for forward-compatibility. """ category_specific_metadata: Mapping[str, Any] = dataclasses.field( diff --git a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py index 0f3c61e9..c13322d6 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py @@ -562,7 +562,10 @@ def _metric_config_bounds_from_proto( metric = enum_from_proto(metric_bound.metric, Metric) match metric: case Metric.UNSPECIFIED: - major_issues.append("metric_config_bounds has an UNSPECIFIED metric") + major_issues.append( + "metric_config_bounds has an UNSPECIFIED metric, dropping it" + ) + continue case int(): minor_issues.append( f"metric_config_bounds has an unrecognized metric {metric}" diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py index 9ec77f87..7ca41472 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py @@ -4,17 +4,20 @@ """Tests for protobuf conversion of the base/common part of electrical components.""" import pytest +from frequenz.api.common.v1alpha8.metrics import bounds_pb2, metrics_pb2 from frequenz.api.common.v1alpha8.microgrid.electrical_components import ( electrical_components_pb2, ) from google.protobuf.timestamp_pb2 import Timestamp +from frequenz.client.common.metrics import Bounds, Metric from frequenz.client.common.microgrid.electrical_components import ( ElectricalComponentCategory, ) from frequenz.client.common.microgrid.electrical_components.proto.v1alpha8._electrical_component import ( # noqa: E501 _electrical_component_base_from_proto_with_issues, _ElectricalComponentBaseData, + _metric_config_bounds_from_proto, _operational_mode_to_bools, ) from frequenz.client.common.types import Lifetime @@ -166,3 +169,40 @@ def test_invalid_lifetime( ] assert not minor_issues assert parsed == base_data + + +_UNKNOWN_METRIC_INT = 9999 +"""A metric int with no corresponding `Metric` member (forward-compat case).""" + + +def _metric_bound( + metric_value: int, lower: float, upper: float +) -> electrical_components_pb2.MetricConfigBounds: + """Build a `MetricConfigBounds` proto for the given raw metric int and bounds.""" + return electrical_components_pb2.MetricConfigBounds( + metric=metrics_pb2.Metric.ValueType(metric_value), + config_bounds=bounds_pb2.Bounds(lower=lower, upper=upper), + ) + + +def test_metric_config_bounds_drops_unspecified() -> None: + """Test UNSPECIFIED keys drop on load while unknown-int and real metrics survive.""" + major_issues: list[str] = [] + minor_issues: list[str] = [] + message = [ + _metric_bound(int(Metric.UNSPECIFIED.value), 0.0, 1.0), + _metric_bound(_UNKNOWN_METRIC_INT, 2.0, 3.0), + _metric_bound(int(Metric.DC_VOLTAGE.value), 4.0, 5.0), + ] + + parsed = _metric_config_bounds_from_proto( + message, major_issues=major_issues, minor_issues=minor_issues + ) + + assert Metric.UNSPECIFIED not in parsed + assert parsed[_UNKNOWN_METRIC_INT] == Bounds(lower=2.0, upper=3.0) + assert parsed[Metric.DC_VOLTAGE] == Bounds(lower=4.0, upper=5.0) + assert any( + "UNSPECIFIED" in issue and "drop" in issue.lower() for issue in major_issues + ) + assert any(str(_UNKNOWN_METRIC_INT) in issue for issue in minor_issues) From 1659427fad9bf624ee30a0b61b87bd743d6f3c0a Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 19 Jun 2026 14:43:17 +0200 Subject: [PATCH 06/10] Remove test hack to avoid passing the category The problem was we were using a type too broad that required passing the category explicitly. If we use a type union of only the types we are going to test we can get also mypy validation for the ctor we are calling. Signed-off-by: Leandro Lucarella --- .../test_simple_components.py | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/tests/microgrid/electrical_components/test_simple_components.py b/tests/microgrid/electrical_components/test_simple_components.py index 8a9a505d..f4117393 100644 --- a/tests/microgrid/electrical_components/test_simple_components.py +++ b/tests/microgrid/electrical_components/test_simple_components.py @@ -16,7 +16,6 @@ Chp, Converter, CryptoMiner, - ElectricalComponent, ElectricalComponentCategory, ElectricalComponentId, Electrolyzer, @@ -57,17 +56,24 @@ def microgrid_id() -> MicrogridId: ids=lambda value: value.__name__ if isinstance(value, type) else value.name, ) def test_init( - cls: type[ElectricalComponent], + cls: type[ + Breaker + | Chp + | Converter + | CryptoMiner + | Electrolyzer + | Hvac + | Meter + | Precharger + | SteamBoiler + | WindTurbine + ], expected_category: ElectricalComponentCategory, component_id: ElectricalComponentId, microgrid_id: MicrogridId, ) -> None: """Test initialization and category of a simple leaf electrical component.""" - # We need to ignore call-arg because otherwise mypy complains about a missing - # category argument. It seems by doing this `cls` trick, mypy can't figure out - # the concrete class we are instantiating has a default category specified, so - # we don't really need to specify the category explicitly. - component = cls( # type: ignore[call-arg] + component = cls( id=component_id, microgrid_id=microgrid_id, name="test_component", From 879cc19d1378188ca00a498557ebae0cfe01c1f1 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 19 Jun 2026 14:59:41 +0200 Subject: [PATCH 07/10] Expose microgrid status only through the `is_active()` method Similar to operational status in electrical components, exposing an enum with the microgrid status is too low level and inconvenient to use. This class already exposed a `is_active` property, but an unspecified value produced a log warning, which is unsafer than it should, not allowing end users decide how to deal with it, and forcing a default that might not be suitable for all downstream users. This commit changes the `is_active` property to a function that raises if the value is unspecified. Signed-off-by: Leandro Lucarella --- .../client/common/microgrid/_microgrid.py | 31 ++++++----- .../microgrid/proto/v1alpha8/_microgrid.py | 28 ++++++++-- .../proto/v1alpha8/test_microgrid.py | 55 ++++++++++++------- tests/microgrid/test_microgrid.py | 51 ++++++++++------- 4 files changed, 106 insertions(+), 59 deletions(-) diff --git a/src/frequenz/client/common/microgrid/_microgrid.py b/src/frequenz/client/common/microgrid/_microgrid.py index d6e49be4..d07e68c6 100644 --- a/src/frequenz/client/common/microgrid/_microgrid.py +++ b/src/frequenz/client/common/microgrid/_microgrid.py @@ -5,16 +5,13 @@ import datetime import enum -import logging from dataclasses import dataclass -from functools import cached_property +from .._exception import UnspecifiedValueError from ..grid._delivery_area import DeliveryArea from ..types._location import Location from ._ids import EnterpriseId, MicrogridId -_logger = logging.getLogger(__name__) - @enum.unique class MicrogridStatus(enum.Enum): @@ -63,21 +60,27 @@ class Microgrid: location: Location | None """Physical location of the microgrid, in geographical co-ordinates.""" - status: MicrogridStatus | int - """The current status of the microgrid.""" - create_time: datetime.datetime """The UTC timestamp indicating when the microgrid was initially created.""" - @cached_property + _active: bool | None + """Whether the microgrid is active, or `None` if its status is unspecified.""" + def is_active(self) -> bool: - """Whether the microgrid is active.""" - if self.status is MicrogridStatus.UNSPECIFIED: - # Because this is a cached property, the warning will only be logged once. - _logger.warning( - "Microgrid %s has an unspecified status. Assuming it is active.", self + """Check whether the microgrid is active. + + Returns: + Whether the microgrid is active. + + Raises: + UnspecifiedValueError: If the status is unspecified, so whether the + microgrid is active is unknown. + """ + if self._active is None: + raise UnspecifiedValueError( + f"status of microgrid {self} is unspecified; active state is unknown" ) - return self.status in (MicrogridStatus.ACTIVE, MicrogridStatus.UNSPECIFIED) + return self._active def __str__(self) -> str: """Return the ID of this microgrid as a string.""" diff --git a/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py b/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py index e3b8aa59..9c5a4d46 100644 --- a/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py +++ b/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py @@ -47,6 +47,26 @@ def microgrid_status_to_proto( return microgrid_pb2.MicrogridStatus.ValueType(status.value) +_ACTIVE_BY_STATUS: dict[int, bool] = { + microgrid_pb2.MICROGRID_STATUS_ACTIVE: True, + microgrid_pb2.MICROGRID_STATUS_INACTIVE: False, +} + + +def _microgrid_status_to_active(value: int) -> bool | None: + """Map a protobuf microgrid status to an active boolean. + + Args: + value: A protobuf microgrid-status enum value (a `MICROGRID_STATUS_*` + constant). + + Returns: + `True` if active, `False` if inactive, or `None` when the status is + unspecified or unrecognized. + """ + return _ACTIVE_BY_STATUS.get(value) + + def microgrid_from_proto(message: microgrid_pb2.Microgrid) -> Microgrid: """Convert a protobuf microgrid message to a microgrid object. @@ -75,10 +95,10 @@ def microgrid_from_proto(message: microgrid_pb2.Microgrid) -> Microgrid: if name is None: minor_issues.append("name is empty") - status = microgrid_status_from_proto(message.status) - if status is MicrogridStatus.UNSPECIFIED: + active = _microgrid_status_to_active(message.status) + if message.status == microgrid_pb2.MICROGRID_STATUS_UNSPECIFIED: major_issues.append("status is unspecified") - elif isinstance(status, int): + elif active is None: major_issues.append("status is unrecognized") if major_issues: @@ -102,6 +122,6 @@ def microgrid_from_proto(message: microgrid_pb2.Microgrid) -> Microgrid: name=message.name or None, delivery_area=delivery_area, location=location, - status=status, create_time=datetime_from_proto(message.create_timestamp), + _active=active, ) diff --git a/tests/microgrid/proto/v1alpha8/test_microgrid.py b/tests/microgrid/proto/v1alpha8/test_microgrid.py index 84fb64bf..18b66af4 100644 --- a/tests/microgrid/proto/v1alpha8/test_microgrid.py +++ b/tests/microgrid/proto/v1alpha8/test_microgrid.py @@ -11,6 +11,7 @@ from frequenz.api.common.v1alpha8.grid import delivery_area_pb2 from frequenz.api.common.v1alpha8.microgrid import microgrid_pb2 +from frequenz.client.common import UnspecifiedValueError from frequenz.client.common.grid import DeliveryArea, EnergyMarketCodeType from frequenz.client.common.microgrid import EnterpriseId, MicrogridId, MicrogridStatus from frequenz.client.common.microgrid.proto.v1alpha8 import ( @@ -48,8 +49,11 @@ class _ProtoConversionTestCase: has_name: bool """Whether to include name in the protobuf message.""" - status: MicrogridStatus | int - """The status to set in the protobuf message.""" + status: int + """The raw protobuf `MICROGRID_STATUS_*` value to set in the message.""" + + expected_active: bool | None + """The expected `_active` value after conversion (`None` if unspecified/unknown).""" expected_log: tuple[str, str] | None = None """Whether to expect a log during conversion (level, message).""" @@ -59,18 +63,28 @@ class _ProtoConversionTestCase: "case", [ _ProtoConversionTestCase( - name="full", + name="active", has_delivery_area=True, has_location=True, has_name=True, - status=MicrogridStatus.ACTIVE, + status=microgrid_pb2.MICROGRID_STATUS_ACTIVE, + expected_active=True, + ), + _ProtoConversionTestCase( + name="inactive", + has_delivery_area=True, + has_location=True, + has_name=True, + status=microgrid_pb2.MICROGRID_STATUS_INACTIVE, + expected_active=False, ), _ProtoConversionTestCase( name="no_delivery_area", has_delivery_area=False, has_location=True, has_name=True, - status=MicrogridStatus.ACTIVE, + status=microgrid_pb2.MICROGRID_STATUS_ACTIVE, + expected_active=True, expected_log=( "WARNING", "Found issues in microgrid: delivery_area is missing", @@ -81,7 +95,8 @@ class _ProtoConversionTestCase: has_delivery_area=True, has_location=False, has_name=True, - status=MicrogridStatus.ACTIVE, + status=microgrid_pb2.MICROGRID_STATUS_ACTIVE, + expected_active=True, expected_log=("WARNING", "Found issues in microgrid: location is missing"), ), _ProtoConversionTestCase( @@ -89,7 +104,8 @@ class _ProtoConversionTestCase: has_delivery_area=True, has_location=True, has_name=False, - status=MicrogridStatus.ACTIVE, + status=microgrid_pb2.MICROGRID_STATUS_ACTIVE, + expected_active=True, expected_log=("DEBUG", "Found minor issues in microgrid: name is empty"), ), _ProtoConversionTestCase( @@ -97,7 +113,8 @@ class _ProtoConversionTestCase: has_delivery_area=True, has_location=True, has_name=True, - status=MicrogridStatus.UNSPECIFIED, + status=microgrid_pb2.MICROGRID_STATUS_UNSPECIFIED, + expected_active=None, expected_log=( "WARNING", "Found issues in microgrid: status is unspecified", @@ -109,6 +126,7 @@ class _ProtoConversionTestCase: has_location=True, has_name=True, status=999, # Unknown status value + expected_active=None, expected_log=( "WARNING", "Found issues in microgrid: status is unrecognized", @@ -121,14 +139,10 @@ class _ProtoConversionTestCase: "frequenz.client.common.microgrid.proto.v1alpha8._microgrid.delivery_area_from_proto" ) @patch("frequenz.client.common.microgrid.proto.v1alpha8._microgrid.location_from_proto") -@patch( - "frequenz.client.common.microgrid.proto.v1alpha8._microgrid.microgrid_status_from_proto" -) @patch("frequenz.client.common.microgrid.proto.v1alpha8._microgrid.datetime_from_proto") # pylint: disable-next=too-many-arguments,too-many-positional-arguments,too-many-branches def test_from_proto( mock_datetime_from_proto: Mock, - mock_microgrid_status_from_proto: Mock, mock_location_from_proto: Mock, mock_delivery_area_from_proto: Mock, caplog: pytest.LogCaptureFixture, @@ -138,8 +152,6 @@ def test_from_proto( now = datetime.now(timezone.utc) mock_datetime_from_proto.return_value = now - mock_microgrid_status_from_proto.return_value = case.status - mock_location = ( Location( latitude=52.52, @@ -161,15 +173,11 @@ def test_from_proto( ) mock_delivery_area_from_proto.return_value = mock_delivery_area - proto_status = microgrid_pb2.MicrogridStatus.ValueType( - case.status.value if isinstance(case.status, MicrogridStatus) else case.status - ) - proto = microgrid_pb2.Microgrid( id=1234, enterprise_id=5678, name="Test Grid" if case.has_name else "", - status=proto_status, + status=microgrid_pb2.MicrogridStatus.ValueType(case.status), ) # Add optional fields if needed @@ -198,9 +206,16 @@ def test_from_proto( else: assert info.name is None + # Verify the active state mapping and the raising accessor. + assert info._active == case.expected_active # pylint: disable=protected-access + if case.expected_active is None: + with pytest.raises(UnspecifiedValueError): + info.is_active() + else: + assert info.is_active() is case.expected_active + # Verify mock calls mock_datetime_from_proto.assert_called_once_with(proto.create_timestamp) - mock_microgrid_status_from_proto.assert_called_once_with(proto.status) if case.has_delivery_area: mock_delivery_area_from_proto.assert_called_once_with(proto.delivery_area) diff --git a/tests/microgrid/test_microgrid.py b/tests/microgrid/test_microgrid.py index afca27bd..afb396d3 100644 --- a/tests/microgrid/test_microgrid.py +++ b/tests/microgrid/test_microgrid.py @@ -7,13 +7,9 @@ import pytest +from frequenz.client.common import UnspecifiedValueError from frequenz.client.common.grid import DeliveryArea, EnergyMarketCodeType -from frequenz.client.common.microgrid import ( - EnterpriseId, - Microgrid, - MicrogridId, - MicrogridStatus, -) +from frequenz.client.common.microgrid import EnterpriseId, Microgrid, MicrogridId from frequenz.client.common.types import Location @@ -28,8 +24,8 @@ def test_creation() -> None: code="DE123", code_type=EnergyMarketCodeType.EUROPE_EIC ), location=Location(latitude=52.52, longitude=13.405, country_code="DE"), - status=MicrogridStatus.ACTIVE, create_time=now, + _active=True, ) assert info.id == MicrogridId(1234) @@ -44,9 +40,8 @@ def test_creation() -> None: assert info.location.longitude is not None assert info.location.longitude == pytest.approx(13.405) assert info.location.country_code == "DE" - assert info.status == MicrogridStatus.ACTIVE assert info.create_time == now - assert info.is_active is True + assert info.is_active() is True def test_creation_without_optionals() -> None: @@ -58,8 +53,8 @@ def test_creation_without_optionals() -> None: name=None, delivery_area=None, location=None, - status=MicrogridStatus.ACTIVE, create_time=now, + _active=True, ) assert info.id == MicrogridId(1234) @@ -67,21 +62,34 @@ def test_creation_without_optionals() -> None: assert info.name is None assert info.delivery_area is None assert info.location is None - assert info.status == MicrogridStatus.ACTIVE assert info.create_time == now - assert info.is_active is True + assert info.is_active() is True @pytest.mark.parametrize( - "status,expected_active", + "active", [ - pytest.param(MicrogridStatus.ACTIVE, True, id="ACTIVE"), - pytest.param(MicrogridStatus.INACTIVE, False, id="INACTIVE"), - pytest.param(MicrogridStatus.UNSPECIFIED, True, id="UNSPECIFIED"), + pytest.param(True, id="active"), + pytest.param(False, id="inactive"), ], ) -def test_is_active_property(status: MicrogridStatus, expected_active: bool) -> None: - """Test the is_active property for different status values.""" +def test_is_active(active: bool) -> None: + """Test the is_active method for known active states.""" + now = datetime.now(timezone.utc) + info = Microgrid( + id=MicrogridId(1234), + enterprise_id=EnterpriseId(5678), + name=None, + delivery_area=None, + location=None, + create_time=now, + _active=active, + ) + assert info.is_active() is active + + +def test_is_active_unspecified() -> None: + """Test that is_active raises when the active state is unspecified.""" now = datetime.now(timezone.utc) info = Microgrid( id=MicrogridId(1234), @@ -89,10 +97,11 @@ def test_is_active_property(status: MicrogridStatus, expected_active: bool) -> N name=None, delivery_area=None, location=None, - status=status, create_time=now, + _active=None, ) - assert info.is_active is expected_active + with pytest.raises(UnspecifiedValueError): + info.is_active() @pytest.mark.parametrize( @@ -112,7 +121,7 @@ def test_str(name: str | None, expected_str: str) -> None: name=name, delivery_area=None, location=None, - status=MicrogridStatus.ACTIVE, create_time=now, + _active=True, ) assert str(info) == expected_str From 48d6a544348659e0f5b0d05f6c7ef1c55d768c91 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 19 Jun 2026 15:18:30 +0200 Subject: [PATCH 08/10] Remove `MicrogridStatus` Signed-off-by: Leandro Lucarella --- .../client/common/microgrid/__init__.py | 3 +- .../client/common/microgrid/_microgrid.py | 15 --------- .../microgrid/proto/v1alpha8/__init__.py | 8 +---- .../microgrid/proto/v1alpha8/_microgrid.py | 33 ++----------------- .../proto/v1alpha8/test_microgrid.py | 19 ++--------- 5 files changed, 6 insertions(+), 72 deletions(-) diff --git a/src/frequenz/client/common/microgrid/__init__.py b/src/frequenz/client/common/microgrid/__init__.py index b010f6a4..50f3b60f 100644 --- a/src/frequenz/client/common/microgrid/__init__.py +++ b/src/frequenz/client/common/microgrid/__init__.py @@ -4,11 +4,10 @@ """Frequenz microgrid definition.""" from ._ids import EnterpriseId, MicrogridId -from ._microgrid import Microgrid, MicrogridStatus +from ._microgrid import Microgrid __all__ = [ "EnterpriseId", "Microgrid", "MicrogridId", - "MicrogridStatus", ] diff --git a/src/frequenz/client/common/microgrid/_microgrid.py b/src/frequenz/client/common/microgrid/_microgrid.py index d07e68c6..269f0fa3 100644 --- a/src/frequenz/client/common/microgrid/_microgrid.py +++ b/src/frequenz/client/common/microgrid/_microgrid.py @@ -4,7 +4,6 @@ """Definition of a microgrid.""" import datetime -import enum from dataclasses import dataclass from .._exception import UnspecifiedValueError @@ -13,20 +12,6 @@ from ._ids import EnterpriseId, MicrogridId -@enum.unique -class MicrogridStatus(enum.Enum): - """The possible statuses for a microgrid.""" - - UNSPECIFIED = 0 - """The status is unspecified. This should not be used.""" - - ACTIVE = 1 - """The microgrid is active.""" - - INACTIVE = 2 - """The microgrid is inactive.""" - - @dataclass(frozen=True, kw_only=True) class Microgrid: """A localized grouping of electricity generation, energy storage, and loads. diff --git a/src/frequenz/client/common/microgrid/proto/v1alpha8/__init__.py b/src/frequenz/client/common/microgrid/proto/v1alpha8/__init__.py index 0d563dda..9c55d6c8 100644 --- a/src/frequenz/client/common/microgrid/proto/v1alpha8/__init__.py +++ b/src/frequenz/client/common/microgrid/proto/v1alpha8/__init__.py @@ -3,14 +3,8 @@ """Conversion of microgrid objects from/to protobuf v1alpha8.""" -from ._microgrid import ( - microgrid_from_proto, - microgrid_status_from_proto, - microgrid_status_to_proto, -) +from ._microgrid import microgrid_from_proto __all__ = [ "microgrid_from_proto", - "microgrid_status_from_proto", - "microgrid_status_to_proto", ] diff --git a/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py b/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py index 9c5a4d46..8ad7045c 100644 --- a/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py +++ b/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py @@ -9,44 +9,15 @@ from ....grid import DeliveryArea from ....grid.proto.v1alpha8 import delivery_area_from_proto -from ....proto import datetime_from_proto, enum_from_proto +from ....proto import datetime_from_proto from ....types import Location from ....types.proto.v1alpha8 import location_from_proto from ..._ids import EnterpriseId, MicrogridId -from ..._microgrid import Microgrid, MicrogridStatus +from ..._microgrid import Microgrid _logger = logging.getLogger(__name__) -def microgrid_status_from_proto( - message: microgrid_pb2.MicrogridStatus.ValueType, -) -> MicrogridStatus | int: - """Convert a protobuf MicrogridStatus enum value to a MicrogridStatus enum member. - - Args: - message: A protobuf MicrogridStatus enum value. - - Returns: - The corresponding MicrogridStatus enum member, or the raw `int` if the protobuf - value is not recognized. - """ - return enum_from_proto(message, MicrogridStatus) - - -def microgrid_status_to_proto( - status: MicrogridStatus, -) -> microgrid_pb2.MicrogridStatus.ValueType: - """Convert a MicrogridStatus enum member to a protobuf MicrogridStatus enum value. - - Args: - status: A MicrogridStatus enum member. - - Returns: - The corresponding protobuf MicrogridStatus enum value. - """ - return microgrid_pb2.MicrogridStatus.ValueType(status.value) - - _ACTIVE_BY_STATUS: dict[int, bool] = { microgrid_pb2.MICROGRID_STATUS_ACTIVE: True, microgrid_pb2.MICROGRID_STATUS_INACTIVE: False, diff --git a/tests/microgrid/proto/v1alpha8/test_microgrid.py b/tests/microgrid/proto/v1alpha8/test_microgrid.py index 18b66af4..266e2c11 100644 --- a/tests/microgrid/proto/v1alpha8/test_microgrid.py +++ b/tests/microgrid/proto/v1alpha8/test_microgrid.py @@ -13,26 +13,11 @@ from frequenz.client.common import UnspecifiedValueError from frequenz.client.common.grid import DeliveryArea, EnergyMarketCodeType -from frequenz.client.common.microgrid import EnterpriseId, MicrogridId, MicrogridStatus -from frequenz.client.common.microgrid.proto.v1alpha8 import ( - microgrid_from_proto, - microgrid_status_from_proto, - microgrid_status_to_proto, -) -from frequenz.client.common.test.enum_parity import EnumParityTest +from frequenz.client.common.microgrid import EnterpriseId, MicrogridId +from frequenz.client.common.microgrid.proto.v1alpha8 import microgrid_from_proto from frequenz.client.common.types import Location -class TestMicrogridStatusParity(EnumParityTest): - """Parity tests for the `MicrogridStatus` enum.""" - - python_enum = MicrogridStatus - proto_enum = microgrid_pb2.MicrogridStatus - name_prefix = "MICROGRID_STATUS_" - from_proto = staticmethod(microgrid_status_from_proto) - to_proto = staticmethod(microgrid_status_to_proto) - - @dataclass(frozen=True, kw_only=True) class _ProtoConversionTestCase: """Test case for protobuf conversion.""" From 80690f58e0bd74ddad47caf9f673a864f6a0d37d Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 19 Jun 2026 15:23:37 +0200 Subject: [PATCH 09/10] Forbid constructing `Microgrid` directly Like electrical components, this object is normally loaded from protobuf, and Python code should never create it, but get it from API services. We forbid microgrid objects, but add a hidden escape hatch to be able to create them in the `microgrid_from_proto()` function. Signed-off-by: Leandro Lucarella --- .../client/common/microgrid/_microgrid.py | 22 ++++++++++- .../microgrid/proto/v1alpha8/_microgrid.py | 1 + tests/microgrid/test_microgrid.py | 39 +++++++++++++++++++ 3 files changed, 60 insertions(+), 2 deletions(-) diff --git a/src/frequenz/client/common/microgrid/_microgrid.py b/src/frequenz/client/common/microgrid/_microgrid.py index 269f0fa3..74cf71b6 100644 --- a/src/frequenz/client/common/microgrid/_microgrid.py +++ b/src/frequenz/client/common/microgrid/_microgrid.py @@ -4,7 +4,7 @@ """Definition of a microgrid.""" import datetime -from dataclasses import dataclass +from dataclasses import dataclass, field from .._exception import UnspecifiedValueError from ..grid._delivery_area import DeliveryArea @@ -13,7 +13,7 @@ @dataclass(frozen=True, kw_only=True) -class Microgrid: +class Microgrid: # pylint: disable=too-many-instance-attributes """A localized grouping of electricity generation, energy storage, and loads. A microgrid is a localized grouping of electricity generation, energy storage, and @@ -51,6 +51,24 @@ class Microgrid: _active: bool | None """Whether the microgrid is active, or `None` if its status is unspecified.""" + _allow_construction: bool = field( + default=False, repr=False, compare=False, hash=False + ) + """Internal guard allowing construction only via the `microgrid_from_proto` converter.""" + + def __post_init__(self) -> None: + """Reject direct construction of this read-only type. + + Raises: + TypeError: If the instance was not created via the `microgrid_from_proto` + converter. + """ + if not self._allow_construction: + raise TypeError( + f"{type(self).__name__} cannot be constructed directly; obtain " + "instances via the microgrid_from_proto converter." + ) + def is_active(self) -> bool: """Check whether the microgrid is active. diff --git a/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py b/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py index 8ad7045c..04935df7 100644 --- a/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py +++ b/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py @@ -95,4 +95,5 @@ def microgrid_from_proto(message: microgrid_pb2.Microgrid) -> Microgrid: location=location, create_time=datetime_from_proto(message.create_timestamp), _active=active, + _allow_construction=True, ) diff --git a/tests/microgrid/test_microgrid.py b/tests/microgrid/test_microgrid.py index afb396d3..95d9b472 100644 --- a/tests/microgrid/test_microgrid.py +++ b/tests/microgrid/test_microgrid.py @@ -3,6 +3,7 @@ """Tests for the Microgrid type.""" +import dataclasses from datetime import datetime, timezone import pytest @@ -26,6 +27,7 @@ def test_creation() -> None: location=Location(latitude=52.52, longitude=13.405, country_code="DE"), create_time=now, _active=True, + _allow_construction=True, ) assert info.id == MicrogridId(1234) @@ -55,6 +57,7 @@ def test_creation_without_optionals() -> None: location=None, create_time=now, _active=True, + _allow_construction=True, ) assert info.id == MicrogridId(1234) @@ -84,6 +87,7 @@ def test_is_active(active: bool) -> None: location=None, create_time=now, _active=active, + _allow_construction=True, ) assert info.is_active() is active @@ -99,6 +103,7 @@ def test_is_active_unspecified() -> None: location=None, create_time=now, _active=None, + _allow_construction=True, ) with pytest.raises(UnspecifiedValueError): info.is_active() @@ -123,5 +128,39 @@ def test_str(name: str | None, expected_str: str) -> None: location=None, create_time=now, _active=True, + _allow_construction=True, ) assert str(info) == expected_str + + +def test_direct_construction_raises() -> None: + """Test that constructing a Microgrid without the gate flag raises TypeError.""" + now = datetime.now(timezone.utc) + with pytest.raises(TypeError): + Microgrid( + id=MicrogridId(1234), + enterprise_id=EnterpriseId(5678), + name=None, + delivery_area=None, + location=None, + create_time=now, + _active=True, + ) + + +def test_replace_preserves_construction() -> None: + """Test that dataclasses.replace on a gated instance works and keeps is_active().""" + now = datetime.now(timezone.utc) + info = Microgrid( + id=MicrogridId(1234), + enterprise_id=EnterpriseId(5678), + name=None, + delivery_area=None, + location=None, + create_time=now, + _active=True, + _allow_construction=True, + ) + replaced = dataclasses.replace(info, name="renamed") + assert replaced.name == "renamed" + assert replaced.is_active() is True From ccf5a4ba1d7100173ee47ebc060629b219d44d34 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 19 Jun 2026 15:28:15 +0200 Subject: [PATCH 10/10] Update release notes Add also a release note about adding the new `electrical_components` package that was missing. Signed-off-by: Leandro Lucarella --- RELEASE_NOTES.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index d384c60d..f57cb03b 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -12,7 +12,9 @@ * Added a new `frequenz.client.common.types.Lifetime` type together with the `frequenz.client.common.types.proto.v1alpha8.lifetime_from_proto` conversion function. * Added a new `frequenz.client.common.types.Location` type together with the `frequenz.client.common.types.proto.v1alpha8.location_from_proto` conversion function. -* Added a new `frequenz.client.common.microgrid.Microgrid` type and `MicrogridStatus` enum together with the `frequenz.client.common.microgrid.proto.v1alpha8.microgrid_from_proto`, `microgrid_status_from_proto`, and `microgrid_status_to_proto` conversion functions. +* Added a new `frequenz.client.common.microgrid.Microgrid` type, together with the `frequenz.client.common.microgrid.proto.v1alpha8.microgrid_from_proto` conversion function. +* Added a new `frequenz.client.common.ClientCommonError` base exception and `UnspecifiedValueError` at the package root. +* Added a new `frequenz.client.common.microgrid.electrical_components` package, featuring a `ElectricalComponent` class hierarchy and its families (battery, inverter, EV charger, etc.), and `ElectricalComponentConnection`, including `v1alpha8` proto conversion functions. ## Bug Fixes