Skip to content

Commit 75cb7d1

Browse files
authored
Allow ComponentDataSamples carrying only states (#1407)
If a `ComponentDataSample` doesn't have any `metric_samples` we raise an exception, but that's not correct, as it is perfectly valid for them to carry only states. This commits allows this case, using the timestamp of the last state (as we do with metrics). The only caveat is the data itself will be created in `*Data` object using defaults (zeros). We also add some regression tests, although the affected code should be gone soon. Fixes #1406.
2 parents 3a94ed0 + f2dd80b commit 75cb7d1

3 files changed

Lines changed: 127 additions & 22 deletions

File tree

RELEASE_NOTES.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,4 @@
1414

1515
## Bug Fixes
1616

17-
<!-- Here goes notable bug fixes that are worth a special mention or explanation -->
17+
* Accept `ComponentDataSamples` that only carry state changes (the state changes were dropped before).

src/frequenz/sdk/microgrid/_old_component_data.py

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -104,19 +104,26 @@ def to_samples(self: Self) -> ComponentDataSamples:
104104
@staticmethod
105105
def _from_samples(class_: type[T], /, samples: ComponentDataSamples) -> T:
106106
"""Create a new instance from a component data object."""
107-
if not samples.metric_samples:
108-
raise ValueError("No metrics in the samples.")
107+
if not samples.metric_samples and not samples.states:
108+
raise ValueError("No metrics and no states in the samples.")
109109

110110
# FIXME: This might not be true forever, but the service sends all metrics with
111111
# the same timestamp for now, and it is very convenient to map the received data
112112
# to the old component data metrics packets, which had only one timestamp.
113113
# When we move away frome these old_component_data wrappers, we should not
114114
# assume only one metric sample can come per telemetry message anymore.
115-
timestamp = samples.metric_samples[-1].sampled_at
116-
for sample in samples.metric_samples[:-1]:
117-
if sample.sampled_at != timestamp:
115+
# When there are no metric samples, fall back to the last state sample's
116+
# timestamp so that state-only messages (for example, a component error
117+
# state without any metric values) are still propagated downstream.
118+
source = samples.metric_samples or samples.states
119+
kind = "metric" if samples.metric_samples else "state"
120+
timestamp = source[-1].sampled_at
121+
for item in source[:-1]:
122+
if item.sampled_at != timestamp:
118123
_logger.warning(
119-
"ComponentData has multiple timestamps. Using the last one. Samples: %r",
124+
"ComponentData has multiple %s sample timestamps. Using the last"
125+
" one. Samples: %r",
126+
kind,
120127
samples,
121128
)
122129
break
@@ -298,9 +305,6 @@ class MeterData(ComponentData):
298305
# pylint: disable-next=too-many-branches
299306
def from_samples(cls, samples: ComponentDataSamples) -> Self:
300307
"""Create a new instance from a component data object."""
301-
if not samples.metric_samples:
302-
raise ValueError("No metrics in the samples.")
303-
304308
self = cls._from_samples(cls, samples)
305309

306310
active_power_per_phase: list[float] = [0.0, 0.0, 0.0]
@@ -486,9 +490,6 @@ class BatteryData(ComponentData): # pylint: disable=too-many-instance-attribute
486490
@classmethod
487491
def from_samples(cls, samples: ComponentDataSamples) -> Self:
488492
"""Create a new instance from a component data object."""
489-
if not samples.metric_samples:
490-
raise ValueError("No metrics in the samples.")
491-
492493
self = cls._from_samples(cls, samples)
493494

494495
for sample in samples.metric_samples:
@@ -704,9 +705,6 @@ class InverterData(ComponentData): # pylint: disable=too-many-instance-attribut
704705
# pylint: disable-next=too-many-branches
705706
def from_samples(cls, samples: ComponentDataSamples) -> Self:
706707
"""Create a new instance from a component data object."""
707-
if not samples.metric_samples:
708-
raise ValueError("No metrics in the samples.")
709-
710708
self = cls._from_samples(cls, samples)
711709

712710
active_power_per_phase: list[float] = [0.0, 0.0, 0.0]
@@ -966,9 +964,6 @@ class ChpData(ComponentData): # pylint: disable=too-many-instance-attributes
966964
# pylint: disable-next=too-many-branches
967965
def from_samples(cls, samples: ComponentDataSamples) -> Self:
968966
"""Create a new instance from a component data object."""
969-
if not samples.metric_samples:
970-
raise ValueError("No metrics in the samples.")
971-
972967
self = cls._from_samples(cls, samples)
973968

974969
active_power_per_phase: list[float] = [0.0, 0.0, 0.0]
@@ -1229,9 +1224,6 @@ class EVChargerData(ComponentData): # pylint: disable=too-many-instance-attribu
12291224
# pylint: disable-next=too-many-branches
12301225
def from_samples(cls, samples: ComponentDataSamples) -> Self:
12311226
"""Create a new instance from a component data object."""
1232-
if not samples.metric_samples:
1233-
raise ValueError("No metrics in the samples.")
1234-
12351227
self = cls._from_samples(cls, samples)
12361228

12371229
active_power_per_phase: list[float] = [0.0, 0.0, 0.0]
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
# License: MIT
2+
# Copyright © 2026 Frequenz Energy-as-a-Service GmbH
3+
4+
"""Tests for `frequenz.sdk.microgrid._old_component_data`."""
5+
6+
from __future__ import annotations
7+
8+
import logging
9+
from datetime import datetime, timedelta, timezone
10+
11+
import pytest
12+
from frequenz.client.common.microgrid.components import ComponentId
13+
from frequenz.client.microgrid.component import (
14+
ComponentDataSamples,
15+
ComponentStateCode,
16+
ComponentStateSample,
17+
)
18+
19+
from frequenz.sdk.microgrid._old_component_data import (
20+
BatteryData,
21+
ChpData,
22+
ComponentData,
23+
EVChargerData,
24+
InverterData,
25+
MeterData,
26+
)
27+
28+
29+
@pytest.mark.parametrize(
30+
"cls",
31+
[BatteryData, ChpData, EVChargerData, InverterData, MeterData],
32+
)
33+
def test_from_samples_preserves_state_without_metrics(
34+
cls: type[ComponentData],
35+
) -> None:
36+
"""`from_samples` must not drop a component state when no metrics are present.
37+
38+
Regression test for
39+
https://github.com/frequenz-floss/frequenz-sdk-python/issues/1406:
40+
a `ComponentDataSamples` message that carries only a state sample (for example
41+
`ComponentStateCode.ERROR`) and no metric samples must still be converted into
42+
a concrete `ComponentData` instance carrying that state, so downstream code
43+
learns about the component's error status as soon as it happens.
44+
45+
Previously, `from_samples` raised `ValueError("No metrics in the samples.")`
46+
and the state was silently swallowed by `_receive_logging_errors`.
47+
"""
48+
component_id = ComponentId(1503)
49+
sampled_at = datetime(2026, 5, 21, 12, 37, 40, 107899, tzinfo=timezone.utc)
50+
samples = ComponentDataSamples(
51+
component_id=component_id,
52+
metric_samples=[],
53+
states=[
54+
ComponentStateSample(
55+
sampled_at=sampled_at,
56+
states=frozenset({ComponentStateCode.ERROR}),
57+
warnings=frozenset(),
58+
errors=frozenset(),
59+
),
60+
],
61+
)
62+
63+
data = cls.from_samples(samples)
64+
65+
assert isinstance(data, cls)
66+
assert data.component_id == component_id
67+
assert data.timestamp == sampled_at
68+
assert ComponentStateCode.ERROR in data.states
69+
assert not data.warnings
70+
assert not data.errors
71+
72+
73+
def test_from_samples_warns_on_multiple_state_timestamps(
74+
caplog: pytest.LogCaptureFixture,
75+
) -> None:
76+
"""Multiple state samples with differing timestamps trigger a warning.
77+
78+
When falling back to a state sample's timestamp (because there are no metric
79+
samples), `_from_samples` should warn if the state samples disagree on
80+
timestamps, mirroring the existing behaviour for metric samples.
81+
"""
82+
component_id = ComponentId(1503)
83+
first = datetime(2026, 5, 21, 12, 37, 40, tzinfo=timezone.utc)
84+
second = first + timedelta(seconds=1)
85+
samples = ComponentDataSamples(
86+
component_id=component_id,
87+
metric_samples=[],
88+
states=[
89+
ComponentStateSample(
90+
sampled_at=first,
91+
states=frozenset({ComponentStateCode.ERROR}),
92+
warnings=frozenset(),
93+
errors=frozenset(),
94+
),
95+
ComponentStateSample(
96+
sampled_at=second,
97+
states=frozenset({ComponentStateCode.ERROR}),
98+
warnings=frozenset(),
99+
errors=frozenset(),
100+
),
101+
],
102+
)
103+
104+
with caplog.at_level(
105+
logging.WARNING, logger="frequenz.sdk.microgrid._old_component_data"
106+
):
107+
data = InverterData.from_samples(samples)
108+
109+
assert data.timestamp == second
110+
assert any(
111+
"multiple state sample timestamps" in record.message
112+
for record in caplog.records
113+
), caplog.records

0 commit comments

Comments
 (0)