diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 05fb4498c..1ecdb4847 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,5 +1,17 @@ # Frequenz Python SDK Release Notes -## Bug fixes +## Summary -- `FormulaEngine` and `FormulaEngine3Phase` are now type aliases to `Formula` and `Formula3Phase`, fixing a typing issue introduced in `v1.0.0-rc2202`. + + +## Upgrading + + + +## New Features + + + +## Bug Fixes + +- Component IDs are validated during creation of battery, pv and ev charger pools, so that errors are caught early and we don't end up getting cryptic failures from somewhere else. diff --git a/src/frequenz/sdk/timeseries/battery_pool/_battery_pool_reference_store.py b/src/frequenz/sdk/timeseries/battery_pool/_battery_pool_reference_store.py index 382809082..68a03f597 100644 --- a/src/frequenz/sdk/timeseries/battery_pool/_battery_pool_reference_store.py +++ b/src/frequenz/sdk/timeseries/battery_pool/_battery_pool_reference_store.py @@ -81,12 +81,24 @@ def __init__( # pylint: disable=too-many-arguments batteries_id: Subset of the batteries that should be included in the battery pool. If None or empty, then all batteries from the microgrid will be used. + + Raises: + ValueError: If any of the specified batteries is not present in the + microgrid. """ self._batteries: frozenset[ComponentId] + all_batteries = self._get_all_batteries() if batteries_id: self._batteries = frozenset(batteries_id) + if not self._batteries.issubset(all_batteries): + unknown_ids = self._batteries - all_batteries + raise ValueError( + "Unable to create a BatteryPool. These component IDs are either " + + "not batteries or are unknown: " + + f"{unknown_ids}" + ) else: - self._batteries = self._get_all_batteries() + self._batteries = all_batteries self._working_batteries: set[ComponentId] = set() diff --git a/src/frequenz/sdk/timeseries/ev_charger_pool/_ev_charger_pool_reference_store.py b/src/frequenz/sdk/timeseries/ev_charger_pool/_ev_charger_pool_reference_store.py index 0e4542aa7..7e4d4c27b 100644 --- a/src/frequenz/sdk/timeseries/ev_charger_pool/_ev_charger_pool_reference_store.py +++ b/src/frequenz/sdk/timeseries/ev_charger_pool/_ev_charger_pool_reference_store.py @@ -63,6 +63,10 @@ def __init__( # pylint: disable=too-many-arguments component_ids: An optional list of component_ids belonging to this pool. If not specified, IDs of all EV Chargers in the microgrid will be fetched from the component graph. + + Raises: + ValueError: If any of the specified component_ids are not EV chargers + or are unknown to the component graph. """ self.channel_registry = channel_registry self.resampler_subscription_sender = resampler_subscription_sender @@ -71,13 +75,21 @@ def __init__( # pylint: disable=too-many-arguments self.power_manager_bounds_subs_sender = power_manager_bounds_subs_sender self.power_distribution_results_fetcher = power_distribution_results_fetcher + graph = connection_manager.get().component_graph + all_ev_chargers = frozenset( + {evc.id for evc in graph.components(matching_types=EvCharger)} + ) + if component_ids is not None: self.component_ids: frozenset[ComponentId] = frozenset(component_ids) + if not self.component_ids.issubset(all_ev_chargers): + unknown_ids = self.component_ids - all_ev_chargers + raise ValueError( + "Unable to create an EVChargerPool. These component IDs are either " + + f"not EV chargers or are unknown: {unknown_ids}" + ) else: - graph = connection_manager.get().component_graph - self.component_ids = frozenset( - {evc.id for evc in graph.components(matching_types=EvCharger)} - ) + self.component_ids = all_ev_chargers self.power_bounds_subs: dict[str, asyncio.Task[None]] = {} diff --git a/src/frequenz/sdk/timeseries/pv_pool/_pv_pool_reference_store.py b/src/frequenz/sdk/timeseries/pv_pool/_pv_pool_reference_store.py index fbba10561..e893f9a36 100644 --- a/src/frequenz/sdk/timeseries/pv_pool/_pv_pool_reference_store.py +++ b/src/frequenz/sdk/timeseries/pv_pool/_pv_pool_reference_store.py @@ -64,6 +64,10 @@ def __init__( # pylint: disable=too-many-arguments component_ids: An optional list of component_ids belonging to this pool. If not specified, IDs of all PV inverters in the microgrid will be fetched from the component graph. + + Raises: + ValueError: If any of the provided component_ids are not PV inverters or + are unknown to the component graph. """ self.channel_registry = channel_registry self.resampler_subscription_sender = resampler_subscription_sender @@ -72,13 +76,22 @@ def __init__( # pylint: disable=too-many-arguments self.power_manager_bounds_subs_sender = power_manager_bounds_subs_sender self.power_distribution_results_fetcher = power_distribution_results_fetcher + graph = connection_manager.get().component_graph + all_solar_inverters = frozenset( + {inv.id for inv in graph.components(matching_types=SolarInverter)} + ) + if component_ids is not None: self.component_ids: frozenset[ComponentId] = frozenset(component_ids) + if not self.component_ids.issubset(all_solar_inverters): + unknown_ids = self.component_ids - all_solar_inverters + raise ValueError( + "Unable to create a PVPool. These component IDs are either " + + "not PV inverters or are unknown: " + + f"{unknown_ids}" + ) else: - graph = connection_manager.get().component_graph - self.component_ids = frozenset( - {inv.id for inv in graph.components(matching_types=SolarInverter)} - ) + self.component_ids = all_solar_inverters self.power_bounds_subs: dict[str, asyncio.Task[None]] = {} diff --git a/tests/microgrid/test_datapipeline.py b/tests/microgrid/test_datapipeline.py index 651a710d5..8142a0281 100644 --- a/tests/microgrid/test_datapipeline.py +++ b/tests/microgrid/test_datapipeline.py @@ -4,6 +4,7 @@ """Basic tests for the DataPipeline.""" import asyncio +import re from datetime import timedelta import async_solipsism @@ -12,13 +13,16 @@ from frequenz.client.common.microgrid import MicrogridId from frequenz.client.common.microgrid.components import ComponentId from frequenz.client.microgrid.component import ( + AcEvCharger, BatteryInverter, ComponentConnection, GridConnectionPoint, LiIonBattery, + SolarInverter, ) from pytest_mock import MockerFixture +from frequenz.sdk.microgrid import _data_pipeline from frequenz.sdk.microgrid._data_pipeline import _DataPipeline from frequenz.sdk.timeseries import ResamplerConfig2 @@ -68,16 +72,56 @@ async def test_actors_started( ) bat_inverter_4 = BatteryInverter(id=ComponentId(4), microgrid_id=_MICROGRID_ID) battery_15 = LiIonBattery(id=ComponentId(15), microgrid_id=_MICROGRID_ID) + pv_inverter_7 = SolarInverter(id=ComponentId(7), microgrid_id=_MICROGRID_ID) + ev_charger_9 = AcEvCharger(id=ComponentId(9), microgrid_id=_MICROGRID_ID) + mock_client = MockMicrogridClient( - components={grid_1, bat_inverter_4, battery_15}, + components={grid_1, bat_inverter_4, battery_15, pv_inverter_7, ev_charger_9}, connections={ ComponentConnection(source=grid_1.id, destination=bat_inverter_4.id), ComponentConnection(source=bat_inverter_4.id, destination=battery_15.id), + ComponentConnection(source=grid_1.id, destination=pv_inverter_7.id), + ComponentConnection(source=grid_1.id, destination=ev_charger_9.id), }, ) mock_client.initialize(mocker) + _data_pipeline._DATA_PIPELINE = datapipeline + datapipeline.new_battery_pool(priority=5) + datapipeline.new_pv_pool(priority=3) + datapipeline.new_ev_charger_pool(priority=4) + + datapipeline.new_battery_pool(priority=1, component_ids={ComponentId(15)}) + datapipeline.new_pv_pool(priority=2, component_ids={ComponentId(7)}) + datapipeline.new_ev_charger_pool(priority=2, component_ids={ComponentId(9)}) + + with pytest.raises( + ValueError, + match=re.escape( + "Unable to create a BatteryPool. These component IDs are either not " + + "batteries or are unknown: frozenset({ComponentId(4)})" + ), + ): + datapipeline.new_battery_pool(priority=2, component_ids={ComponentId(4)}) + + with pytest.raises( + ValueError, + match=re.escape( + "Unable to create a PVPool. These component IDs are either not PV " + + "inverters or are unknown: frozenset({ComponentId(1)})" + ), + ): + datapipeline.new_pv_pool(priority=4, component_ids={ComponentId(1)}) + + with pytest.raises( + ValueError, + match=re.escape( + "Unable to create an EVChargerPool. These component IDs are either " + + "not EV chargers or are unknown: frozenset({ComponentId(4)})" + ), + ): + datapipeline.new_ev_charger_pool(priority=5, component_ids={ComponentId(4)}) assert datapipeline._battery_power_wrapper._power_distributing_actor is not None await asyncio.sleep(1)