From 042f04ef74b7799776250e41136a550c73c453f4 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Tue, 21 Jul 2026 15:15:06 +0000 Subject: [PATCH 1/3] Move `_bounds_set_from_proto` to the bounds proto module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_bounds_set_from_proto` converts a sequence of protobuf `Bounds` into a `BoundsSet | InvalidBoundsSet`. That is a bounds converter, so its natural home is `_bounds.py` next to `bounds_from_proto2` (which it calls), not `_sample.py`, where it only happened to be first needed. Relocating it lets other converters reuse it without reaching into the sample module — in particular the electrical-component metric config bounds converter, which is about to aggregate duplicate metrics through the same helper. Signed-off-by: Leandro Lucarella --- .../common/metrics/proto/v1alpha8/_bounds.py | 37 ++++++++++++++++- .../common/metrics/proto/v1alpha8/_sample.py | 40 +------------------ 2 files changed, 38 insertions(+), 39 deletions(-) diff --git a/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py b/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py index bce06af7..42797235 100644 --- a/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py +++ b/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py @@ -3,10 +3,13 @@ """Loading of Bounds objects from protobuf messages.""" +from collections.abc import Sequence +from typing import assert_never + from frequenz.api.common.v1alpha8.metrics import bounds_pb2 from typing_extensions import deprecated -from ..._bounds import Bounds, InvalidBounds +from ..._bounds import Bounds, BoundsSet, InvalidBounds, InvalidBoundsSet @deprecated( @@ -60,6 +63,38 @@ def bounds_from_proto2( return InvalidBounds(lower=lower, upper=upper) +def _bounds_set_from_proto( + messages: Sequence[bounds_pb2.Bounds], +) -> BoundsSet | InvalidBoundsSet: + """Convert a sequence of bounds messages to a bounds set. + + Args: + messages: The sequence of bounds messages. + + Returns: + A [`BoundsSet`][....BoundsSet] when every bound is well-formed, or an + [`InvalidBoundsSet`][....InvalidBoundsSet] preserving all the raw + bounds when any bound is malformed. + """ + valid: list[Bounds] = [] + raw: list[Bounds | InvalidBounds] = [] + has_invalid = False + for pb_bound in messages: + match bounds_from_proto2(pb_bound): + case Bounds() as bound: + valid.append(bound) + raw.append(bound) + case InvalidBounds() as bound: + has_invalid = True + raw.append(bound) + case unknown: + assert_never(unknown) + + if has_invalid: + return InvalidBoundsSet(bounds=tuple(raw)) + return BoundsSet(bounds=tuple(valid)) + + @deprecated( "`bounds_from_proto_with_issues` is deprecated; use " "`bounds_from_proto2` (returns `Bounds | InvalidBounds`) and inspect " diff --git a/src/frequenz/client/common/metrics/proto/v1alpha8/_sample.py b/src/frequenz/client/common/metrics/proto/v1alpha8/_sample.py index d86ffc07..0e7fe7e7 100644 --- a/src/frequenz/client/common/metrics/proto/v1alpha8/_sample.py +++ b/src/frequenz/client/common/metrics/proto/v1alpha8/_sample.py @@ -3,13 +3,9 @@ """Loading of MetricSample and AggregatedMetricValue objects from protobuf messages.""" -from collections.abc import Sequence -from typing import assert_never - -from frequenz.api.common.v1alpha8.metrics import bounds_pb2, metrics_pb2 +from frequenz.api.common.v1alpha8.metrics import metrics_pb2 from ....proto import datetime_from_proto -from ..._bounds import Bounds, BoundsSet, InvalidBounds, InvalidBoundsSet from ..._metric import Metric from ..._sample import ( AggregatedMetricValue, @@ -17,7 +13,7 @@ MetricConnectionCategory, MetricSample, ) -from ._bounds import bounds_from_proto2 +from ._bounds import _bounds_set_from_proto from ._metric import metric_from_proto from ._metric_connection_category import metric_connection_category_from_proto @@ -121,35 +117,3 @@ def metric_sample_from_proto_with_issues( bounds_set=bounds_set, connection=connection, ) - - -def _bounds_set_from_proto( - messages: Sequence[bounds_pb2.Bounds], -) -> BoundsSet | InvalidBoundsSet: - """Convert a sequence of bounds messages to a bounds set. - - Args: - messages: The sequence of bounds messages. - - Returns: - A [`BoundsSet`][....BoundsSet] when every bound is well-formed, or an - [`InvalidBoundsSet`][....InvalidBoundsSet] preserving all the raw - bounds when any bound is malformed. - """ - valid: list[Bounds] = [] - raw: list[Bounds | InvalidBounds] = [] - has_invalid = False - for pb_bound in messages: - match bounds_from_proto2(pb_bound): - case Bounds() as bound: - valid.append(bound) - raw.append(bound) - case InvalidBounds() as bound: - has_invalid = True - raw.append(bound) - case unknown: - assert_never(unknown) - - if has_invalid: - return InvalidBoundsSet(bounds=tuple(raw)) - return BoundsSet(bounds=tuple(valid)) From 9d0038c169158a436a48dd38b11767d25c4762d3 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Tue, 21 Jul 2026 15:29:41 +0000 Subject: [PATCH 2/3] Make `bounds_set_from_proto` public `bounds_set_from_proto` is the multi-bound counterpart of the public `bounds_from_proto2`: it converts a `repeated Bounds` field into a single `BoundsSet | InvalidBoundsSet`, surfacing malformed data at the type level. It was module-private, but any client reading a repeated bounds field needs the exact same conversion, so we better expose it in the public package. Signed-off-by: Leandro Lucarella --- .../common/metrics/proto/v1alpha8/__init__.py | 2 + .../common/metrics/proto/v1alpha8/_bounds.py | 15 ++++--- .../common/metrics/proto/v1alpha8/_sample.py | 4 +- tests/metrics/proto/v1alpha8/test_bounds.py | 39 ++++++++++++++++++- 4 files changed, 52 insertions(+), 8 deletions(-) diff --git a/src/frequenz/client/common/metrics/proto/v1alpha8/__init__.py b/src/frequenz/client/common/metrics/proto/v1alpha8/__init__.py index 757a0542..b7e31a3b 100644 --- a/src/frequenz/client/common/metrics/proto/v1alpha8/__init__.py +++ b/src/frequenz/client/common/metrics/proto/v1alpha8/__init__.py @@ -7,6 +7,7 @@ bounds_from_proto, bounds_from_proto2, bounds_from_proto_with_issues, + bounds_set_from_proto, ) from ._metric import metric_from_proto, metric_to_proto from ._metric_connection_category import ( @@ -24,6 +25,7 @@ "bounds_from_proto", "bounds_from_proto2", "bounds_from_proto_with_issues", + "bounds_set_from_proto", "metric_connection_category_from_proto", "metric_connection_category_to_proto", "metric_connection_from_proto_with_issues", diff --git a/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py b/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py index 42797235..33368eb4 100644 --- a/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py +++ b/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py @@ -63,18 +63,23 @@ def bounds_from_proto2( return InvalidBounds(lower=lower, upper=upper) -def _bounds_set_from_proto( +def bounds_set_from_proto( messages: Sequence[bounds_pb2.Bounds], ) -> BoundsSet | InvalidBoundsSet: - """Convert a sequence of bounds messages to a bounds set. + """Convert a sequence of bounds messages into a single bounds set. + + This is the multi-bound counterpart of + [`bounds_from_proto2`][..bounds_from_proto2]: it converts each message and + combines the results. Args: - messages: The sequence of bounds messages. + messages: The bounds messages to convert. Returns: - A [`BoundsSet`][....BoundsSet] when every bound is well-formed, or an + A [`BoundsSet`][....BoundsSet] (the union of the bounds) when every + message is well-formed, or an [`InvalidBoundsSet`][....InvalidBoundsSet] preserving all the raw - bounds when any bound is malformed. + bounds in order when any message is malformed. """ valid: list[Bounds] = [] raw: list[Bounds | InvalidBounds] = [] diff --git a/src/frequenz/client/common/metrics/proto/v1alpha8/_sample.py b/src/frequenz/client/common/metrics/proto/v1alpha8/_sample.py index 0e7fe7e7..d0104823 100644 --- a/src/frequenz/client/common/metrics/proto/v1alpha8/_sample.py +++ b/src/frequenz/client/common/metrics/proto/v1alpha8/_sample.py @@ -13,7 +13,7 @@ MetricConnectionCategory, MetricSample, ) -from ._bounds import _bounds_set_from_proto +from ._bounds import bounds_set_from_proto from ._metric import metric_from_proto from ._metric_connection_category import metric_connection_category_from_proto @@ -102,7 +102,7 @@ def metric_sample_from_proto_with_issues( message.value.aggregated_metric ) - bounds_set = _bounds_set_from_proto(message.bounds) + bounds_set = bounds_set_from_proto(message.bounds) connection = None if message.HasField("connection"): diff --git a/tests/metrics/proto/v1alpha8/test_bounds.py b/tests/metrics/proto/v1alpha8/test_bounds.py index b839b722..8a4037a2 100644 --- a/tests/metrics/proto/v1alpha8/test_bounds.py +++ b/tests/metrics/proto/v1alpha8/test_bounds.py @@ -8,11 +8,17 @@ import pytest from frequenz.api.common.v1alpha8.metrics import bounds_pb2 -from frequenz.client.common.metrics import Bounds, InvalidBounds +from frequenz.client.common.metrics import ( + Bounds, + BoundsSet, + InvalidBounds, + InvalidBoundsSet, +) from frequenz.client.common.metrics.proto.v1alpha8 import ( bounds_from_proto, bounds_from_proto2, bounds_from_proto_with_issues, + bounds_set_from_proto, ) @@ -218,3 +224,34 @@ def test_from_proto2_empty_message_is_unbounded_bounds() -> None: assert bounds == Bounds() assert bounds.lower is None assert bounds.upper is None + + +def test_bounds_set_from_proto_all_valid() -> None: + """`bounds_set_from_proto` unions well-formed bounds into a `BoundsSet`.""" + messages = [ + bounds_pb2.Bounds(lower=1.0, upper=5.0), + bounds_pb2.Bounds(lower=3.0, upper=10.0), + ] + + result = bounds_set_from_proto(messages) + + assert result == BoundsSet(bounds=(Bounds(lower=1.0, upper=10.0),)) + + +def test_bounds_set_from_proto_any_invalid_preserves_all() -> None: + """One malformed bound makes the whole set an `InvalidBoundsSet` keeping all bounds.""" + messages = [ + bounds_pb2.Bounds(lower=1.0, upper=5.0), + bounds_pb2.Bounds(lower=10.0, upper=-10.0), + ] + + result = bounds_set_from_proto(messages) + + assert result == InvalidBoundsSet( + bounds=(Bounds(lower=1.0, upper=5.0), InvalidBounds(lower=10.0, upper=-10.0)) + ) + + +def test_bounds_set_from_proto_empty_is_unbounded() -> None: + """No messages yield the empty, unbounded `BoundsSet`.""" + assert bounds_set_from_proto([]) == BoundsSet() From 86bb870f110a6e4b9fb46fd071bef1df67e58565 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Tue, 21 Jul 2026 15:32:23 +0000 Subject: [PATCH 3/3] Aggregate all metric config bounds into a `BoundsSet` `ElectricalComponent.metric_config_bounds` comes from the `repeated MetricConfigBounds metric_config_bounds` wire field, which is *not* a protobuf map. The converter nonetheless keyed a dict by metric and did `bounds[metric] = ...` on each entry, so when a metric appeared more than once the later entry silently overwrote the earlier one. Conflicting operational limits were lost with no diagnostic on the returned mapping or `electrical_component_from_proto_with_issues()`. Aggregate every entry for a metric instead, reusing `bounds_set_from_proto`. A metric whose entries are all well-formed maps to a `BoundsSet` (their union); a metric with any malformed entry maps to an `InvalidBoundsSet` preserving all the raw bounds in wire order, so the conflict stays inspectable. As a consequence the field is now typed `Mapping[Metric | int, BoundsSet | InvalidBoundsSet]`, and `get_metric_config_bounds()` resolves an entry to a valid `BoundsSet` (defaulting to the unbounded `BoundsSet()`) or raises `InvalidBoundsSetError`. Signed-off-by: Leandro Lucarella --- RELEASE_NOTES.md | 3 +- .../_electrical_component.py | 39 +++++------ .../proto/v1alpha8/_electrical_component.py | 54 ++++++++------- .../proto/v1alpha8/conftest.py | 29 +++++---- .../test_electrical_component_base.py | 65 ++++++++++++++----- .../test_electrical_component_base.py | 44 ++++++++----- 6 files changed, 144 insertions(+), 90 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 6f2a78d1..6a130cb7 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -137,6 +137,7 @@ * `frequenz.client.common.metrics.BoundsSet` — a normalized union of `Bounds` with an efficient `value in bounds_set` membership test. Overlapping and touching bounds are merged on construction, and the empty set is the unbounded set (it contains every value and is falsy). * `frequenz.client.common.metrics.InvalidBoundsSet` — a set built from bounds that included at least one `InvalidBounds`; it preserves all the raw bounds unmerged and provides no membership test. + * `frequenz.client.common.metrics.proto.v1alpha8.bounds_set_from_proto` conversion function returning `BoundsSet | InvalidBoundsSet`. It converts a `repeated Bounds` field into a single bounds set. * Added a new `frequenz.client.common.metrics.MetricSample.bounds_set` field, typed `BoundsSet | InvalidBoundsSet`, replacing the deprecated `bounds` list (see Upgrading). Malformed wire bounds are preserved as an `InvalidBoundsSet` instead of being dropped. Use `get_bounds_set()` to resolve it to a valid `BoundsSet` or a clear `InvalidBoundsSetError`. @@ -148,8 +149,6 @@ The class of a component is its identity; components don't carry category or type attributes. The only exceptions are the error-recovery classes `UnrecognizedElectricalComponent` and `MismatchedCategoryElectricalComponent` (with a raw protobuf `category` value) and `UnrecognizedBattery`, `UnrecognizedInverter` and `UnrecognizedEvCharger` (with a raw protobuf `type` value), which preserve the raw protobuf values received from the protocol version used to load them. - `ElectricalComponent.metric_config_bounds` is typed `Mapping[Metric | int, Bounds | InvalidBounds]`: malformed wire entries are preserved as `InvalidBounds` instead of being silently dropped, and entries that named a metric but carried no (or an empty) `config_bounds` submessage load as an unbounded `Bounds()` (a `Bounds` with neither bound set imposes no limit in either direction). Use `get_metric_config_bounds()` to resolve an entry to a valid `Bounds` or a clear `InvalidBoundsError`; it mimics `dict.get()`, returning an unbounded `Bounds()` (or a caller-supplied `default`) for absent metrics. - * Added a new `frequenz.client.common.microgrid.Microgrid` type with a raising `is_active()` method, together with the `frequenz.client.common.microgrid.proto.v1alpha8.microgrid_from_proto` conversion function. * Added `frequenz.client.common.FloatInt`, a type alias for `float | int`. 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 f2680ab1..4b3a5c2b 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py @@ -9,7 +9,7 @@ from typing import Any, Self, TypeVar, assert_never, overload from ..._exception import UnrecognizedEnumValueError, UnspecifiedEnumValueError -from ...metrics import Bounds, InvalidBounds, InvalidBoundsError, Metric +from ...metrics import BoundsSet, InvalidBoundsSet, InvalidBoundsSetError, Metric from .. import MicrogridId from .._lifetime import InvalidLifetime, InvalidLifetimeError, Lifetime from ._category_specific_info import CategorySpecificInfo @@ -76,7 +76,7 @@ class ElectricalComponent: # pylint: disable=too-many-instance-attributes ) """Internal guard allowing construction only via the `*_from_proto` converters.""" - metric_config_bounds: Mapping[Metric | int, Bounds | InvalidBounds] = ( + metric_config_bounds: Mapping[Metric | int, BoundsSet | InvalidBoundsSet] = ( dataclasses.field( default_factory=dict, # dict is not hashable, so we don't use this field to calculate the hash. @@ -91,9 +91,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. - Malformed bounds received from the wire are preserved as - [`InvalidBounds`][.....metrics.InvalidBounds] instances so callers can - inspect the raw values without accidentally using them for range checks. + Each metric maps to the aggregate of all the bounds configured for it: a + [`BoundsSet`][.....metrics.BoundsSet] when every one is well-formed, or an + [`InvalidBoundsSet`][.....metrics.InvalidBoundsSet] preserving all the raw + bounds when any is malformed. If an unspecified metric is received, it is stored as the plain `int` key `0` when loading from protobuf. Metrics unknown to this client version may also appear @@ -101,7 +102,7 @@ class ElectricalComponent: # pylint: disable=too-many-instance-attributes Tip: Prefer [`get_metric_config_bounds()`][..get_metric_config_bounds] - when a valid [`Bounds`][.....metrics.Bounds] is required. + when a valid [`BoundsSet`][.....metrics.BoundsSet] is required. """ category_specific_info: CategorySpecificInfo | None = None @@ -203,21 +204,21 @@ def accepts_control(self) -> bool: assert_never(unknown) @overload - def get_metric_config_bounds(self, metric: Metric) -> Bounds: ... + def get_metric_config_bounds(self, metric: Metric) -> BoundsSet: ... @overload def get_metric_config_bounds( self, metric: Metric, *, default: DefaultT - ) -> Bounds | DefaultT: ... + ) -> BoundsSet | DefaultT: ... def get_metric_config_bounds( - self, metric: Metric, *, default: object = Bounds() + self, metric: Metric, *, default: object = BoundsSet() ) -> object: - """Return the configured bounds for a metric as a valid `Bounds`. + """Return the configured bounds for a metric as a valid `BoundsSet`. An absent entry returns an unbounded metric, so when no bounds are configured for `metric` this returns an unbounded - [`Bounds`][frequenz.client.common.metrics.Bounds] by default. Pass + [`BoundsSet`][frequenz.client.common.metrics.BoundsSet] by default. Pass `default` to return a different value for absent entries instead, mimicking [`dict.get()`][dict.get]. @@ -241,25 +242,25 @@ def get_metric_config_bounds( `metric`. Returns: - The valid [`Bounds`][.....metrics.Bounds] configured for `metric`, - or `default` when there is no entry for `metric`. + The valid [`BoundsSet`][.....metrics.BoundsSet] configured for + `metric`, or `default` when there is no entry for `metric`. Raises: - InvalidBoundsError: If the bounds configured for `metric` are + InvalidBoundsSetError: If the bounds configured for `metric` are malformed. The offending instance is available on the - exception's `bounds` attribute. + exception's `bounds_set` attribute. """ match self.metric_config_bounds.get(metric): case None: return default - case InvalidBounds() as invalid: - raise InvalidBoundsError( + case InvalidBoundsSet() as invalid: + raise InvalidBoundsSetError( self, "metric_config_bounds", invalid, - f"invalid bounds {invalid} for metric {metric} in {self}", + f"invalid bounds set {invalid} for metric {metric} in {self}", ) - case Bounds() as valid: + case BoundsSet() as valid: return valid case unknown: assert_never(unknown) 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 bdb65ba3..b62f6905 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 @@ -8,13 +8,14 @@ from collections.abc import Mapping, Sequence from typing import Final, NamedTuple, TypeAlias, assert_never, overload +from frequenz.api.common.v1alpha8.metrics import bounds_pb2 from frequenz.api.common.v1alpha8.microgrid.electrical_components import ( electrical_components_pb2, ) from google.protobuf.json_format import MessageToDict -from .....metrics import Bounds, InvalidBounds, Metric -from .....metrics.proto.v1alpha8 import bounds_from_proto2 +from .....metrics import BoundsSet, InvalidBoundsSet, Metric +from .....metrics.proto.v1alpha8._bounds import bounds_set_from_proto from .....proto import enum_from_proto from ...._ids import MicrogridId from ...._lifetime import InvalidLifetime, Lifetime @@ -892,13 +893,16 @@ class _ElectricalComponentBaseData(NamedTuple): lifetime: Lifetime | InvalidLifetime """The operational lifetime of the electrical component.""" - metric_config_bounds: dict[Metric | int, Bounds | InvalidBounds] + metric_config_bounds: dict[Metric | int, BoundsSet | InvalidBoundsSet] """The metric configuration bounds extracted from the protobuf message. - Malformed entries are preserved as - [`InvalidBounds`][frequenz.client.common.metrics.InvalidBounds]; entries - whose `config_bounds` field was not set load as an unbounded - [`Bounds`][frequenz.client.common.metrics.Bounds]. + Each metric maps to the aggregate of every entry it had on the wire: a + [`BoundsSet`][frequenz.client.common.metrics.BoundsSet] when all are + well-formed, or an + [`InvalidBoundsSet`][frequenz.client.common.metrics.InvalidBoundsSet] + preserving all the raw bounds when any is malformed. A metric with no + configured limits maps to the empty, unbounded + [`BoundsSet`][frequenz.client.common.metrics.BoundsSet]. """ category_specific_info: CategorySpecificInfo | None @@ -1284,32 +1288,32 @@ def electrical_component_from_proto_with_issues( def _metric_config_bounds_from_proto( message: Sequence[electrical_components_pb2.MetricConfigBounds], -) -> dict[Metric | int, Bounds | InvalidBounds]: - """Convert a `MetricConfigBounds` message to a dictionary mapping `Metric` to bounds. +) -> dict[Metric | int, BoundsSet | InvalidBoundsSet]: + """Convert `MetricConfigBounds` messages to a mapping of metric to bounds set. The keys of the result map are [`Metric`][frequenz.client.common.metrics.Metric] enum members (or `int` for - unrecognized values). Values are - [`Bounds`][frequenz.client.common.metrics.Bounds] for well-formed entries and - [`InvalidBounds`][frequenz.client.common.metrics.InvalidBounds] for entries - that carried bound values violating `lower <= upper`. + unrecognized values, and `0` for the unspecified metric). Each value + aggregates *every* entry that named the metric into a single set: a + [`BoundsSet`][frequenz.client.common.metrics.BoundsSet] (their union) when + all of the metric's `config_bounds` are well-formed, or an + [`InvalidBoundsSet`][frequenz.client.common.metrics.InvalidBoundsSet] + preserving all the raw bounds in wire order when any is malformed. An entry with no configured limits — its `config_bounds` submessage absent, - or present but empty — loads as an unbounded - [`Bounds`][frequenz.client.common.metrics.Bounds]: a `Bounds` with neither - `lower` nor `upper` set imposes no limit in either direction. Absence and a + or present but empty — contributes an unbounded `Bounds()` (a `Bounds` with + neither `lower` nor `upper` set imposes no limit in either direction), which + normalizes into the empty, unbounded + [`BoundsSet`][frequenz.client.common.metrics.BoundsSet]. Absence and a present-but-empty submessage are intentionally treated the same. - Duplicated metrics on the wire follow proto3 map semantics: the last entry - wins silently. - Args: - message: The `MetricConfigBounds` message. + message: The `MetricConfigBounds` messages. Returns: - The resulting dictionary mapping metrics to their bounds. + A mapping from each metric to the set of its configured bounds. """ - bounds: dict[Metric | int, Bounds | InvalidBounds] = {} + grouped: dict[Metric | int, list[bounds_pb2.Bounds]] = {} for metric_bound in message: with warnings.catch_warnings(): warnings.filterwarnings("ignore", category=DeprecationWarning) @@ -1317,9 +1321,11 @@ def _metric_config_bounds_from_proto( if metric is Metric.UNSPECIFIED: metric = metric.value - bounds[metric] = bounds_from_proto2(metric_bound.config_bounds) + grouped.setdefault(metric, []).append(metric_bound.config_bounds) - return bounds + return { + metric: bounds_set_from_proto(configs) for metric, configs in grouped.items() + } def _get_operational_lifetime_from_proto( diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py b/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py index 10dcbddf..8f1346ae 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py @@ -13,7 +13,7 @@ ) from google.protobuf.timestamp_pb2 import Timestamp -from frequenz.client.common.metrics import Bounds, Metric +from frequenz.client.common.metrics import Bounds, BoundsSet, Metric from frequenz.client.common.microgrid import Lifetime, MicrogridId from frequenz.client.common.microgrid.electrical_components import ( ElectricalComponent, @@ -59,7 +59,9 @@ def default_component_base_data( model=DEFAULT_MODEL, category=ElectricalComponentCategory.UNSPECIFIED, lifetime=DEFAULT_LIFETIME, - metric_config_bounds={Metric.AC_ENERGY_ACTIVE: Bounds(lower=0, upper=100)}, + metric_config_bounds={ + Metric.AC_ENERGY_ACTIVE: BoundsSet(bounds=(Bounds(lower=0, upper=100),)) + }, category_specific_info=None, provides_telemetry=True, accepts_control=True, @@ -148,17 +150,18 @@ def base_data_as_proto( ) proto.operational_lifetime.CopyFrom(lifetime_pb2.Lifetime(**lifetime_dict)) if base_data.metric_config_bounds: - for metric, bounds in base_data.metric_config_bounds.items(): - bounds_dict: dict[str, float] = {} - if bounds.lower is not None: - bounds_dict["lower"] = bounds.lower - if bounds.upper is not None: - bounds_dict["upper"] = bounds.upper + for metric, bounds_set in base_data.metric_config_bounds.items(): metric_value = metric.value if isinstance(metric, Metric) else metric - proto.metric_config_bounds.append( - electrical_components_pb2.MetricConfigBounds( - metric=metrics_pb2.Metric.ValueType(metric_value), - config_bounds=bounds_pb2.Bounds(**bounds_dict), + for bounds in bounds_set.bounds: + bounds_dict: dict[str, float] = {} + if bounds.lower is not None: + bounds_dict["lower"] = bounds.lower + if bounds.upper is not None: + bounds_dict["upper"] = bounds.upper + proto.metric_config_bounds.append( + electrical_components_pb2.MetricConfigBounds( + metric=metrics_pb2.Metric.ValueType(metric_value), + config_bounds=bounds_pb2.Bounds(**bounds_dict), + ) ) - ) return proto 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 34bf6843..59f31a19 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 @@ -12,7 +12,13 @@ ) from google.protobuf.timestamp_pb2 import Timestamp -from frequenz.client.common.metrics import Bounds, InvalidBounds, Metric +from frequenz.client.common.metrics import ( + Bounds, + BoundsSet, + InvalidBounds, + InvalidBoundsSet, + Metric, +) from frequenz.client.common.microgrid import InvalidLifetime, Lifetime from frequenz.client.common.microgrid.electrical_components import ( CategorySpecificInfo, @@ -223,10 +229,16 @@ def test_metric_config_bounds_stores_unspecified_as_int() -> None: parsed = _metric_config_bounds_from_proto(message) - assert parsed[int(Metric.UNSPECIFIED.value)] == Bounds(lower=0.0, upper=1.0) + assert parsed[int(Metric.UNSPECIFIED.value)] == BoundsSet( + bounds=(Bounds(lower=0.0, upper=1.0),) + ) 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 parsed[_UNKNOWN_METRIC_INT] == BoundsSet( + bounds=(Bounds(lower=2.0, upper=3.0),) + ) + assert parsed[Metric.DC_VOLTAGE] == BoundsSet( + bounds=(Bounds(lower=4.0, upper=5.0),) + ) def test_metric_config_bounds_preserves_invalid_bounds() -> None: @@ -238,16 +250,16 @@ def test_metric_config_bounds_preserves_invalid_bounds() -> None: parsed = _metric_config_bounds_from_proto(message) - invalid = parsed[Metric.DC_VOLTAGE] - assert isinstance(invalid, InvalidBounds) - assert not isinstance(invalid, Bounds) - assert invalid.lower == 10.0 - assert invalid.upper == -10.0 - assert parsed[Metric.AC_POWER_ACTIVE] == Bounds(lower=-5.0, upper=5.0) + assert parsed[Metric.DC_VOLTAGE] == InvalidBoundsSet( + bounds=(InvalidBounds(lower=10.0, upper=-10.0),) + ) + assert parsed[Metric.AC_POWER_ACTIVE] == BoundsSet( + bounds=(Bounds(lower=-5.0, upper=5.0),) + ) def test_metric_config_bounds_absent_config_bounds_is_unbounded() -> None: - """An entry without a `config_bounds` field yields an unbounded `Bounds`.""" + """An entry without a `config_bounds` field yields an unbounded `BoundsSet`.""" entry = electrical_components_pb2.MetricConfigBounds( metric=metrics_pb2.Metric.ValueType(int(Metric.DC_VOLTAGE.value)) ) @@ -255,11 +267,11 @@ def test_metric_config_bounds_absent_config_bounds_is_unbounded() -> None: parsed = _metric_config_bounds_from_proto([entry]) - assert parsed[Metric.DC_VOLTAGE] == Bounds() + assert parsed[Metric.DC_VOLTAGE] == BoundsSet() -def test_metric_config_bounds_duplicated_metric_last_wins() -> None: - """A duplicated metric on the wire is kept as its last entry (proto3 map semantics).""" +def test_metric_config_bounds_duplicated_metric_unions_all() -> None: + """A duplicated metric aggregates all of its bounds into one `BoundsSet`.""" message = [ _metric_bound(int(Metric.DC_VOLTAGE.value), 0.0, 1.0), _metric_bound(int(Metric.DC_VOLTAGE.value), 2.0, 3.0), @@ -267,4 +279,27 @@ def test_metric_config_bounds_duplicated_metric_last_wins() -> None: parsed = _metric_config_bounds_from_proto(message) - assert parsed[Metric.DC_VOLTAGE] == Bounds(lower=2.0, upper=3.0) + assert parsed[Metric.DC_VOLTAGE] == BoundsSet( + bounds=(Bounds(lower=0.0, upper=1.0), Bounds(lower=2.0, upper=3.0)) + ) + + +def test_metric_config_bounds_duplicated_metric_valid_and_invalid() -> None: + """A metric mixing valid and invalid bounds becomes an `InvalidBoundsSet`. + + All the raw bounds are preserved in wire order so the conflict stays + inspectable. + """ + message = [ + _metric_bound(int(Metric.DC_VOLTAGE.value), -5.0, 5.0), + _metric_bound(int(Metric.DC_VOLTAGE.value), 10.0, -10.0), + ] + + parsed = _metric_config_bounds_from_proto(message) + + assert parsed[Metric.DC_VOLTAGE] == InvalidBoundsSet( + bounds=( + Bounds(lower=-5.0, upper=5.0), + InvalidBounds(lower=10.0, upper=-10.0), + ) + ) diff --git a/tests/microgrid/electrical_components/test_electrical_component_base.py b/tests/microgrid/electrical_components/test_electrical_component_base.py index c75edc0e..b3d611a3 100644 --- a/tests/microgrid/electrical_components/test_electrical_component_base.py +++ b/tests/microgrid/electrical_components/test_electrical_component_base.py @@ -14,8 +14,10 @@ ) from frequenz.client.common.metrics import ( Bounds, + BoundsSet, InvalidBounds, - InvalidBoundsError, + InvalidBoundsSet, + InvalidBoundsSetError, Metric, ) from frequenz.client.common.microgrid import ( @@ -38,7 +40,9 @@ class _TestElectricalComponent(ElectricalComponent): def _make_component( *, operational_lifetime: Lifetime | InvalidLifetime = Lifetime(), - metric_config_bounds: dict[Metric | int, Bounds | InvalidBounds] | None = None, + metric_config_bounds: ( + dict[Metric | int, BoundsSet | InvalidBoundsSet] | None + ) = None, ) -> _TestElectricalComponent: """Build a test component with the given operational lifetime.""" if metric_config_bounds is None: @@ -105,8 +109,10 @@ def test_creation_with_defaults() -> None: def test_creation_full() -> None: """Test electrical component creation with all attributes.""" - bounds = Bounds(lower=-100.0, upper=100.0) - metric_config_bounds: dict[Metric | int, Bounds] = {Metric.AC_POWER_ACTIVE: bounds} + bounds = BoundsSet(bounds=(Bounds(lower=-100.0, upper=100.0),)) + metric_config_bounds: dict[Metric | int, BoundsSet] = { + Metric.AC_POWER_ACTIVE: bounds + } info = CategorySpecificInfo(kind="battery", fields={"key1": "value1", "key2": 42}) component = _TestElectricalComponent( @@ -230,7 +236,7 @@ def test_is_operational_now_raises_for_invalid_lifetime() -> None: def test_get_metric_config_bounds_returns_valid_bounds() -> None: """`get_metric_config_bounds` returns the configured `Bounds` for a metric.""" - bounds = Bounds(lower=-10.0, upper=10.0) + bounds = BoundsSet(bounds=(Bounds(lower=-10.0, upper=10.0),)) component = _make_component(metric_config_bounds={Metric.AC_POWER_ACTIVE: bounds}) result = component.get_metric_config_bounds(Metric.AC_POWER_ACTIVE) @@ -239,13 +245,13 @@ def test_get_metric_config_bounds_returns_valid_bounds() -> None: def test_get_metric_config_bounds_absent_returns_unbounded() -> None: - """`get_metric_config_bounds` returns an unbounded `Bounds` for absent metrics.""" + """`get_metric_config_bounds` returns an unbounded `BoundsSet` for absent metrics.""" component = _make_component(metric_config_bounds={}) result = component.get_metric_config_bounds(Metric.AC_POWER_ACTIVE) - assert result == Bounds() - assert isinstance(result, Bounds) + assert result == BoundsSet() + assert isinstance(result, BoundsSet) def test_get_metric_config_bounds_absent_returns_default() -> None: @@ -264,7 +270,7 @@ def test_get_metric_config_bounds_absent_returns_default() -> None: def test_get_metric_config_bounds_present_ignores_default() -> None: """`get_metric_config_bounds` ignores `default` when the metric has bounds.""" - bounds = Bounds(lower=-10.0, upper=10.0) + bounds = BoundsSet(bounds=(Bounds(lower=-10.0, upper=10.0),)) component = _make_component(metric_config_bounds={Metric.AC_POWER_ACTIVE: bounds}) assert ( @@ -274,23 +280,23 @@ def test_get_metric_config_bounds_present_ignores_default() -> None: def test_get_metric_config_bounds_invalid_raises_error() -> None: - """`get_metric_config_bounds` raises `InvalidBoundsError` for malformed entries.""" - invalid = InvalidBounds(lower=10.0, upper=-10.0) + """`get_metric_config_bounds` raises `InvalidBoundsSetError` for malformed entries.""" + invalid = InvalidBoundsSet(bounds=(InvalidBounds(lower=10.0, upper=-10.0),)) component = _make_component(metric_config_bounds={Metric.AC_POWER_ACTIVE: invalid}) - with pytest.raises(InvalidBoundsError) as exc_info: + with pytest.raises(InvalidBoundsSetError) as exc_info: component.get_metric_config_bounds(Metric.AC_POWER_ACTIVE) - assert exc_info.value.bounds is invalid + assert exc_info.value.bounds_set is invalid assert "AC_POWER_ACTIVE" in str(exc_info.value) def test_get_metric_config_bounds_invalid_raises_despite_default() -> None: """`default` only applies to absent metrics, not malformed ones.""" - invalid = InvalidBounds(lower=10.0, upper=-10.0) + invalid = InvalidBoundsSet(bounds=(InvalidBounds(lower=10.0, upper=-10.0),)) component = _make_component(metric_config_bounds={Metric.AC_POWER_ACTIVE: invalid}) - with pytest.raises(InvalidBoundsError): + with pytest.raises(InvalidBoundsSetError): component.get_metric_config_bounds(Metric.AC_POWER_ACTIVE, default=None) @@ -372,7 +378,9 @@ def test_is_operational_now(mock_datetime: Mock) -> None: microgrid_id=MicrogridId(1), name="test", model="Test Model", - metric_config_bounds={Metric.AC_POWER_ACTIVE: Bounds(lower=-100.0, upper=100.0)}, + metric_config_bounds={ + Metric.AC_POWER_ACTIVE: BoundsSet(bounds=(Bounds(lower=-100.0, upper=100.0),)) + }, category_specific_info=CategorySpecificInfo( kind="battery", fields={"key": "value"} ), @@ -386,7 +394,9 @@ def test_is_operational_now(mock_datetime: Mock) -> None: microgrid_id=COMPONENT.microgrid_id, name=COMPONENT.name, model=COMPONENT.model, - metric_config_bounds={Metric.AC_POWER_ACTIVE: Bounds(lower=-200.0, upper=200.0)}, + metric_config_bounds={ + Metric.AC_POWER_ACTIVE: BoundsSet(bounds=(Bounds(lower=-200.0, upper=200.0),)) + }, category_specific_info=COMPONENT.category_specific_info, _provides_telemetry=True, _accepts_control=True,