Fix bug in GridFrequency quantity conversion (#807)

This is a follow-up to #804.

The sample value can also be `None`, in which case we can't access to
`sample.value.base_value`. In that case we just cast the existing sample
as it doesn't matter which type the value has when it is `None`,
avoiding the extra copying.

I discovered this bug while implementing a solution for #806. Which kind
of proves it is a worthy pursue...

Also fixes #831.

Author: llucax

RELEASE_NOTES.md

@@ -57,7 +57,13 @@
 ## Bug Fixes
 
 - 0W power requests are now not adjusted to exclusion bounds by the `PowerManager` and `PowerDistributor`, and are sent over to the microgrid API directly.
-- Fixed that `microgrid.frequency()` was sending `Quantity` objects instead of `Frequency`.
+
+- `microgrid.frequency()` / `GridFrequency`:
+
+  * Fix sent samples to use `Frequency` objects instead of raw `Quantity`.
+  * Handle `None` values in the received samples properly.
+  * Convert `nan` values in the received samples to `None`.
+
 - The resampler now properly handles sending zero values.
 
   A bug made the resampler interpret zero values as `None` when generating new samples, so if the result of the resampling is zero, the resampler would just produce `None` values.

src/frequenz/sdk/timeseries/_grid_frequency.py

@@ -107,9 +107,9 @@ def new_receiver(self) -> Receiver[Sample[Frequency]]:
             )
 
         return receiver.map(
-            lambda sample: Sample(
-                sample.timestamp, Frequency.from_hertz(sample.value.base_value)
-            )
+            lambda sample: Sample[Frequency](sample.timestamp, None)
+            if sample.value is None or sample.value.isnan()
+            else Sample(sample.timestamp, Frequency.from_hertz(sample.value.base_value))
         )
 
     async def _send_request(self) -> None:

tests/timeseries/mock_resampler.py

@@ -5,6 +5,7 @@
 
 
 import asyncio
+import math
 from datetime import datetime
 
 from frequenz.channels import Broadcast, Receiver, Sender
@@ -187,58 +188,65 @@ async def _handle_resampling_requests(self) -> None:
                 )
             )
 
+    def make_sample(self, value: float | None) -> Sample[Quantity]:
+        """Create a sample with the given value."""
+        return Sample(
+            self._next_ts,
+            None if value is None or math.isnan(value) else Quantity(value),
+        )
+
     async def send_meter_power(self, values: list[float | None]) -> None:
         """Send the given values as resampler output for meter power."""
         assert len(values) == len(self._meter_power_senders)
         for chan, value in zip(self._meter_power_senders, values):
-            sample = Sample(self._next_ts, None if not value else Quantity(value))
+            sample = self.make_sample(value)
             await chan.send(sample)
 
     async def send_chp_power(self, values: list[float | None]) -> None:
         """Send the given values as resampler output for CHP power."""
         assert len(values) == len(self._chp_power_senders)
         for chan, value in zip(self._chp_power_senders, values):
-            sample = Sample(self._next_ts, None if not value else Quantity(value))
+            sample = self.make_sample(value)
             await chan.send(sample)
 
     async def send_pv_inverter_power(self, values: list[float | None]) -> None:
         """Send the given values as resampler output for PV Inverter power."""
         assert len(values) == len(self._pv_inverter_power_senders)
         for chan, value in zip(self._pv_inverter_power_senders, values):
-            sample = Sample(self._next_ts, None if not value else Quantity(value))
+            sample = self.make_sample(value)
             await chan.send(sample)
 
     async def send_meter_frequency(self, values: list[float | None]) -> None:
         """Send the given values as resampler output for meter frequency."""
         assert len(values) == len(self._meter_frequency_senders)
         for sender, value in zip(self._meter_frequency_senders, values):
-            sample = Sample(self._next_ts, None if not value else Quantity(value))
+            sample = self.make_sample(value)
             await sender.send(sample)
 
     async def send_bat_inverter_frequency(self, values: list[float | None]) -> None:
         """Send the given values as resampler output for battery inverter frequency."""
         assert len(values) == len(self._bat_inverter_frequency_senders)
         for chan, value in zip(self._bat_inverter_frequency_senders, values):
-            sample = Sample(self._next_ts, None if not value else Quantity(value))
+            sample = self.make_sample(value)
             await chan.send(sample)
 
     async def send_evc_power(self, values: list[float | None]) -> None:
         """Send the given values as resampler output for EV Charger power."""
         assert len(values) == len(self._ev_power_senders)
         for chan, value in zip(self._ev_power_senders, values):
-            sample = Sample(self._next_ts, None if not value else Quantity(value))
+            sample = self.make_sample(value)
             await chan.send(sample)
 
     async def send_bat_inverter_power(self, values: list[float | None]) -> None:
         """Send the given values as resampler output for battery inverter power."""
         assert len(values) == len(self._bat_inverter_power_senders)
         for chan, value in zip(self._bat_inverter_power_senders, values):
-            sample = Sample(self._next_ts, None if not value else Quantity(value))
+            sample = self.make_sample(value)
             await chan.send(sample)
 
     async def send_non_existing_component_value(self) -> None:
         """Send a value for a non existing component."""
-        sample = Sample[Quantity](self._next_ts, None)
+        sample = self.make_sample(None)
         await self._non_existing_component_sender.send(sample)
 
     async def send_evc_current(self, values: list[list[float | None]]) -> None:
@@ -247,7 +255,7 @@ async def send_evc_current(self, values: list[list[float | None]]) -> None:
         for chan, evc_values in zip(self._ev_current_senders, values):
             assert len(evc_values) == 3  # 3 values for phases
             for phase, value in enumerate(evc_values):
-                sample = Sample(self._next_ts, None if not value else Quantity(value))
+                sample = self.make_sample(value)
                 await chan[phase].send(sample)
 
     async def send_bat_inverter_current(self, values: list[list[float | None]]) -> None:
@@ -256,7 +264,7 @@ async def send_bat_inverter_current(self, values: list[list[float | None]]) -> N
         for chan, bat_values in zip(self._bat_inverter_current_senders, values):
             assert len(bat_values) == 3  # 3 values for phases
             for phase, value in enumerate(bat_values):
-                sample = Sample(self._next_ts, None if not value else Quantity(value))
+                sample = self.make_sample(value)
                 await chan[phase].send(sample)
 
     async def send_meter_current(self, values: list[list[float | None]]) -> None:
@@ -265,7 +273,7 @@ async def send_meter_current(self, values: list[list[float | None]]) -> None:
         for chan, meter_values in zip(self._meter_current_senders, values):
             assert len(meter_values) == 3  # 3 values for phases
             for phase, value in enumerate(meter_values):
-                sample = Sample(self._next_ts, None if not value else Quantity(value))
+                sample = self.make_sample(value)
                 await chan[phase].send(sample)
 
     async def send_meter_voltage(self, values: list[list[float | None]]) -> None:
@@ -274,5 +282,5 @@ async def send_meter_voltage(self, values: list[list[float | None]]) -> None:
         for chan, meter_values in zip(self._meter_voltage_senders, values):
             assert len(meter_values) == 3  # 3 values for phases
             for phase, value in enumerate(meter_values):
-                sample = Sample(self._next_ts, None if not value else Quantity(value))
+                sample = self.make_sample(value)
                 await chan[phase].send(sample)

tests/timeseries/test_frequency_streaming.py

@@ -19,6 +19,34 @@
 # pylint: disable=protected-access
 
 
+async def test_grid_frequency_none(mocker: MockerFixture) -> None:
+    """Test the grid frequency formula."""
+    mockgrid = MockMicrogrid(grid_meter=True)
+    mockgrid.add_batteries(2)
+    mockgrid.add_solar_inverters(1)
+    await mockgrid.start(mocker)
+
+    grid_freq = microgrid.frequency()
+    grid_freq_recv = grid_freq.new_receiver()
+
+    assert grid_freq._task is not None
+    # We have to wait for the metric request to be sent
+    await grid_freq._task
+    # And consumed
+    await asyncio.sleep(0)
+
+    await mockgrid.mock_client.send(
+        component_data_wrapper.MeterDataWrapper(
+            mockgrid.meter_ids[0], datetime.now(tz=timezone.utc)
+        )
+    )
+
+    val = await grid_freq_recv.receive()
+    assert val is not None
+    assert val.value is None
+    await mockgrid.cleanup()
+
+
 async def test_grid_frequency_1(mocker: MockerFixture) -> None:
     """Test the grid frequency formula."""
     mockgrid = MockMicrogrid(grid_meter=True)

tests/utils/component_data_wrapper.py

@@ -101,8 +101,8 @@ def __init__(  # pylint: disable=too-many-arguments
         component_id: int,
         timestamp: datetime,
         active_power: float = math.nan,
-        current_per_phase: tuple[float, float, float] | None = None,
-        voltage_per_phase: tuple[float, float, float] | None = None,
+        current_per_phase: tuple[float, float, float] = (math.nan, math.nan, math.nan),
+        voltage_per_phase: tuple[float, float, float] = (math.nan, math.nan, math.nan),
         active_power_inclusion_lower_bound: float = math.nan,
         active_power_exclusion_lower_bound: float = math.nan,
         active_power_inclusion_upper_bound: float = math.nan,
@@ -122,16 +122,8 @@ def __init__(  # pylint: disable=too-many-arguments
             component_id=component_id,
             timestamp=timestamp,
             active_power=active_power,
-            current_per_phase=(
-                current_per_phase
-                if current_per_phase
-                else (math.nan, math.nan, math.nan)
-            ),
-            voltage_per_phase=(
-                voltage_per_phase
-                if voltage_per_phase
-                else (math.nan, math.nan, math.nan)
-            ),
+            current_per_phase=current_per_phase,
+            voltage_per_phase=voltage_per_phase,
             active_power_inclusion_lower_bound=active_power_inclusion_lower_bound,
             active_power_exclusion_lower_bound=active_power_exclusion_lower_bound,
             active_power_inclusion_upper_bound=active_power_inclusion_upper_bound,
@@ -165,8 +157,8 @@ def __init__(  # pylint: disable=too-many-arguments
         component_id: int,
         timestamp: datetime,
         active_power: float = math.nan,
-        current_per_phase: tuple[float, float, float] | None = None,
-        voltage_per_phase: tuple[float, float, float] | None = None,
+        current_per_phase: tuple[float, float, float] = (math.nan, math.nan, math.nan),
+        voltage_per_phase: tuple[float, float, float] = (math.nan, math.nan, math.nan),
         frequency: float = 50.0,
         cable_state: EVChargerCableState = EVChargerCableState.UNSPECIFIED,
         component_state: EVChargerComponentState = EVChargerComponentState.UNSPECIFIED,
@@ -180,16 +172,8 @@ def __init__(  # pylint: disable=too-many-arguments
             component_id=component_id,
             timestamp=timestamp,
             active_power=active_power,
-            current_per_phase=(
-                current_per_phase
-                if current_per_phase
-                else (math.nan, math.nan, math.nan)
-            ),
-            voltage_per_phase=(
-                voltage_per_phase
-                if voltage_per_phase
-                else (math.nan, math.nan, math.nan)
-            ),
+            current_per_phase=current_per_phase,
+            voltage_per_phase=voltage_per_phase,
             frequency=frequency,
             cable_state=cable_state,
             component_state=component_state,
@@ -219,8 +203,8 @@ def __init__(  # pylint: disable=too-many-arguments
         component_id: int,
         timestamp: datetime,
         active_power: float = math.nan,
-        current_per_phase: tuple[float, float, float] | None = None,
-        voltage_per_phase: tuple[float, float, float] | None = None,
+        current_per_phase: tuple[float, float, float] = (math.nan, math.nan, math.nan),
+        voltage_per_phase: tuple[float, float, float] = (math.nan, math.nan, math.nan),
         frequency: float = math.nan,
     ) -> None:
         """Initialize the MeterDataWrapper.
@@ -232,16 +216,8 @@ def __init__(  # pylint: disable=too-many-arguments
             component_id=component_id,
             timestamp=timestamp,
             active_power=active_power,
-            current_per_phase=(
-                current_per_phase
-                if current_per_phase
-                else (math.nan, math.nan, math.nan)
-            ),
-            voltage_per_phase=(
-                voltage_per_phase
-                if voltage_per_phase
-                else (math.nan, math.nan, math.nan)
-            ),
+            current_per_phase=current_per_phase,
+            voltage_per_phase=voltage_per_phase,
             frequency=frequency,
         )