Don't return None from battery pool methods (#612)

In the case when no battery is available, the internal values of the
`Sample` or `PowerMetric` will be set to `None` instead.

Author: shsms

RELEASE_NOTES.md

@@ -8,6 +8,8 @@
 
 <!-- Here goes notes on how to upgrade from previous versions, including deprecations and what they should be replaced with -->
 
+- The battery pool metric methods no longer return `None` when no batteries are available. Instead, the value of the `Sample` or `PowerMetric` is set to `None`.
+
 ## New Features
 
 <!-- Here goes the main new features and examples or instructions on how to use them -->

src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py

@@ -124,7 +124,7 @@ def calculate(
         self,
         metrics_data: dict[int, ComponentMetricsData],
         working_batteries: set[int],
-    ) -> T | None:
+    ) -> T:
         """Aggregate the metrics_data and calculate high level metric.
 
         Missing components will be ignored. Formula will be calculated for all
@@ -191,7 +191,7 @@ def calculate(
         self,
         metrics_data: dict[int, ComponentMetricsData],
         working_batteries: set[int],
-    ) -> Sample[Energy] | None:
+    ) -> Sample[Energy]:
         """Aggregate the metrics_data and calculate high level metric.
 
         Missing components will be ignored. Formula will be calculated for all
@@ -229,7 +229,7 @@ def calculate(
             total_capacity += usable_capacity
 
         return (
-            None
+            Sample(datetime.now(tz=timezone.utc), None)
             if timestamp == _MIN_TIMESTAMP
             else Sample[Energy](timestamp, Energy.from_watt_hours(total_capacity))
         )
@@ -281,7 +281,7 @@ def calculate(
         self,
         metrics_data: dict[int, ComponentMetricsData],
         working_batteries: set[int],
-    ) -> Sample[Temperature] | None:
+    ) -> Sample[Temperature]:
         """Aggregate the metrics_data and calculate high level metric for temperature.
 
         Missing components will be ignored. Formula will be calculated for all
@@ -312,7 +312,7 @@ def calculate(
             temperature_sum += temperature
             temperature_count += 1
         if timestamp == _MIN_TIMESTAMP:
-            return None
+            return Sample(datetime.now(tz=timezone.utc), None)
 
         temperature_avg = temperature_sum / temperature_count
 
@@ -371,7 +371,7 @@ def calculate(
         self,
         metrics_data: dict[int, ComponentMetricsData],
         working_batteries: set[int],
-    ) -> Sample[Percentage] | None:
+    ) -> Sample[Percentage]:
         """Aggregate the metrics_data and calculate high level metric.
 
         Missing components will be ignored. Formula will be calculated for all
@@ -432,7 +432,7 @@ def calculate(
             total_capacity_x100 += usable_capacity_x100
 
         if timestamp == _MIN_TIMESTAMP:
-            return None
+            return Sample(datetime.now(tz=timezone.utc), None)
 
         # To avoid zero division error
         if total_capacity_x100 == 0:
@@ -598,7 +598,7 @@ def calculate(
         self,
         metrics_data: dict[int, ComponentMetricsData],
         working_batteries: set[int],
-    ) -> PowerMetrics | None:
+    ) -> PowerMetrics:
         """Aggregate the metrics_data and calculate high level metric.
 
         Missing components will be ignored. Formula will be calculated for all
@@ -642,7 +642,11 @@ def calculate(
                 exclusion_bounds_lower += min(exclusion_lower_bounds)
 
         if timestamp == _MIN_TIMESTAMP:
-            return None
+            return PowerMetrics(
+                timestamp=datetime.now(tz=timezone.utc),
+                inclusion_bounds=None,
+                exclusion_bounds=None,
+            )
 
         return PowerMetrics(
             timestamp=timestamp,

src/frequenz/sdk/timeseries/battery_pool/_result_types.py

@@ -29,7 +29,7 @@ class PowerMetrics:
     """Timestamp of the metrics."""
 
     # pylint: disable=line-too-long
-    inclusion_bounds: Bounds
+    inclusion_bounds: Bounds | None
     """Inclusion power bounds for all batteries in the battery pool instance.
 
     This is the range within which power requests are allowed by the battery pool.
@@ -42,7 +42,7 @@ class PowerMetrics:
     details.
     """
 
-    exclusion_bounds: Bounds
+    exclusion_bounds: Bounds | None
     """Exclusion power bounds for all batteries in the battery pool instance.
 
     This is the range within which power requests are NOT allowed by the battery pool.

tests/timeseries/_battery_pool/test_battery_pool.py

@@ -419,6 +419,9 @@ async def run_test_battery_status_channel(  # pylint: disable=too-many-arguments
         all_pool_result: result metric if all batteries in pool are working
         only_first_battery_result: result metric if only first battery in pool is
             working
+
+    Raises:
+        ValueError: If the received message is not an instance of PowerMetrics or Sample.
     """
     assert len(batteries_in_pool) == 2
 
@@ -438,7 +441,13 @@ async def run_test_battery_status_channel(  # pylint: disable=too-many-arguments
     msg = await asyncio.wait_for(
         battery_pool_metric_receiver.receive(), timeout=waiting_time_sec
     )
-    assert msg is None
+    if isinstance(msg, PowerMetrics):
+        assert msg.inclusion_bounds is None
+        assert msg.exclusion_bounds is None
+    elif isinstance(msg, Sample):
+        assert msg.value is None
+    else:
+        raise ValueError("Expected an instance of PowerMetrics or Sample")
 
     # One battery in uncertain state.
     await battery_status_sender.send(
@@ -662,7 +671,7 @@ async def run_capacity_test(setup_args: SetupArgs) -> None:
     await streamer.stop_streaming(batteries_in_pool[0])
     await asyncio.sleep(MAX_BATTERY_DATA_AGE_SEC + 0.2)
     msg = await asyncio.wait_for(capacity_receiver.receive(), timeout=waiting_time_sec)
-    assert msg is None
+    assert isinstance(msg, Sample) and msg.value is None
 
     # One battery started sending data.
     latest_data = streamer.get_current_component_data(batteries_in_pool[0])
@@ -740,7 +749,7 @@ async def run_soc_test(setup_args: SetupArgs) -> None:
         Scenario(
             batteries_in_pool[1],
             {"soc": math.nan},
-            None,
+            Sample(now, None),
         ),
         Scenario(
             batteries_in_pool[1],
@@ -751,7 +760,7 @@ async def run_soc_test(setup_args: SetupArgs) -> None:
         Scenario(
             batteries_in_pool[0],
             {"capacity": 0, "soc_lower_bound": 10.0, "soc_upper_bound": 100.0},
-            None,
+            Sample(now, None),
             wait_for_result=False,
         ),
         # Test zero division error
@@ -795,7 +804,7 @@ async def run_soc_test(setup_args: SetupArgs) -> None:
     await streamer.stop_streaming(batteries_in_pool[0])
     await asyncio.sleep(MAX_BATTERY_DATA_AGE_SEC + 0.2)
     msg = await asyncio.wait_for(receiver.receive(), timeout=waiting_time_sec)
-    assert msg is None
+    assert isinstance(msg, Sample) and msg.value is None
 
     # One battery started sending data.
     latest_data = streamer.get_current_component_data(batteries_in_pool[0])
@@ -885,7 +894,11 @@ async def run_power_bounds_test(  # pylint: disable=too-many-locals
                 "active_power_inclusion_upper_bound": 9000,
                 "active_power_exclusion_upper_bound": 250,
             },
-            expected_result=None,
+            expected_result=PowerMetrics(
+                now,
+                None,
+                None,
+            ),
             wait_for_result=False,
         ),
         Scenario(
@@ -995,7 +1008,11 @@ async def run_power_bounds_test(  # pylint: disable=too-many-locals
                 "power_inclusion_lower_bound": math.nan,
                 "power_inclusion_upper_bound": math.nan,
             },
-            None,
+            PowerMetrics(
+                now,
+                None,
+                None,
+            ),
         ),
         Scenario(
             batteries_in_pool[0],
@@ -1121,7 +1138,11 @@ async def run_power_bounds_test(  # pylint: disable=too-many-locals
     await streamer.stop_streaming(bat_inv_map[batteries_in_pool[1]])
     await asyncio.sleep(MAX_BATTERY_DATA_AGE_SEC + 0.2)
     msg = await asyncio.wait_for(receiver.receive(), timeout=waiting_time_sec)
-    assert msg is None
+    assert (
+        isinstance(msg, PowerMetrics)
+        and msg.inclusion_bounds is None
+        and msg.exclusion_bounds is None
+    )
 
     # One battery started sending data.
     latest_data = streamer.get_current_component_data(batteries_in_pool[0])
@@ -1201,7 +1222,7 @@ async def run_temperature_test(  # pylint: disable=too-many-locals
         Scenario(
             bat_1,
             {"temperature": math.nan},
-            None,
+            Sample(now, None),
         ),
         Scenario(
             bat_0,
@@ -1238,7 +1259,7 @@ async def run_temperature_test(  # pylint: disable=too-many-locals
     await streamer.stop_streaming(bat_0)
     await asyncio.sleep(MAX_BATTERY_DATA_AGE_SEC + 0.2)
     msg = await asyncio.wait_for(receiver.receive(), timeout=waiting_time_sec)
-    assert msg is None
+    assert isinstance(msg, Sample) and msg.value is None
 
     # one battery started sending data.
     latest_data = streamer.get_current_component_data(bat_1)