From 933bc6165bf35a0e39e4de2642a4d1c2f023caa4 Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Wed, 3 Jul 2024 15:44:07 +0200 Subject: [PATCH 1/4] Improve release notes for bounds class improvements Signed-off-by: Sahas Subramanian --- RELEASE_NOTES.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index eba4d5bb1..a751794d5 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -19,9 +19,7 @@ ## New Features - - -- Classes Bounds and SystemBounds now work with the `in` operator +- Classes `Bounds` and `SystemBounds` now implement the `__contains__` method, allowing the use of the `in` operator to check whether a value falls within the bounds or not. ## Bug Fixes From ff526d9bc2d90f98003836247df49f456a47941e Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Wed, 3 Jul 2024 12:56:32 +0200 Subject: [PATCH 2/4] Set api request timeout from `microgrid.initialize` Signed-off-by: Sahas Subramanian --- .../power_distribution/power_distributor.py | 1 + .../_component_managers/_battery_manager.py | 8 +++++-- .../_ev_charger_manager.py | 10 ++++---- .../_pv_inverter_manager.py | 6 ++++- .../power_distributing/power_distributing.py | 12 +++++++--- src/frequenz/sdk/microgrid/_data_pipeline.py | 24 +++++++++++++++---- src/frequenz/sdk/microgrid/_power_wrapper.py | 6 +++++ .../test_power_distributing.py | 20 ++++++++++++++++ 8 files changed, 72 insertions(+), 15 deletions(-) diff --git a/benchmarks/power_distribution/power_distributor.py b/benchmarks/power_distribution/power_distributor.py index 7d37d1cde..9511cc6f7 100644 --- a/benchmarks/power_distribution/power_distributor.py +++ b/benchmarks/power_distribution/power_distributor.py @@ -120,6 +120,7 @@ async def run_test( # pylint: disable=too-many-locals requests_receiver=power_request_channel.new_receiver(), results_sender=power_result_channel.new_sender(), component_pool_status_sender=battery_status_channel.new_sender(), + api_power_request_timeout=timedelta(seconds=5.0), ): tasks: list[Coroutine[Any, Any, list[Result]]] = [] tasks.append(send_requests(batteries, num_requests)) diff --git a/src/frequenz/sdk/actor/power_distributing/_component_managers/_battery_manager.py b/src/frequenz/sdk/actor/power_distributing/_component_managers/_battery_manager.py index ba9509b31..3bf7b10e3 100644 --- a/src/frequenz/sdk/actor/power_distributing/_component_managers/_battery_manager.py +++ b/src/frequenz/sdk/actor/power_distributing/_component_managers/_battery_manager.py @@ -125,7 +125,7 @@ def _add(key: str, value: dict[int, set[int]] | None) -> None: return mapping -class BatteryManager(ComponentManager): +class BatteryManager(ComponentManager): # pylint: disable=too-many-instance-attributes """Class to manage the data streams for batteries.""" @override @@ -133,6 +133,7 @@ def __init__( self, component_pool_status_sender: Sender[ComponentPoolStatus], results_sender: Sender[Result], + api_power_request_timeout: timedelta, ): """Initialize this instance. @@ -142,8 +143,11 @@ def __init__( streams, to dynamically adjust the values based on the health of the individual batteries. results_sender: Channel sender to send the power distribution results to. + api_power_request_timeout: Timeout to use when making power requests to + the microgrid API. """ self._results_sender = results_sender + self._api_power_request_timeout = api_power_request_timeout self._batteries = connection_manager.get().component_graph.components( component_categories={ComponentCategory.BATTERY} ) @@ -277,7 +281,7 @@ async def _distribute_power( ) failed_power, failed_batteries = await self._set_distributed_power( - distribution, request.request_timeout + distribution, self._api_power_request_timeout ) response: Success | PartialFailure diff --git a/src/frequenz/sdk/actor/power_distributing/_component_managers/_ev_charger_manager/_ev_charger_manager.py b/src/frequenz/sdk/actor/power_distributing/_component_managers/_ev_charger_manager/_ev_charger_manager.py index baf426200..e4c929097 100644 --- a/src/frequenz/sdk/actor/power_distributing/_component_managers/_ev_charger_manager/_ev_charger_manager.py +++ b/src/frequenz/sdk/actor/power_distributing/_component_managers/_ev_charger_manager/_ev_charger_manager.py @@ -32,8 +32,6 @@ _logger = logging.getLogger(__name__) -_DEFAULT_API_REQUEST_TIMEOUT = timedelta(seconds=5.0) - class EVChargerManager(ComponentManager): """Manage ev chargers for the power distributor.""" @@ -43,6 +41,7 @@ def __init__( self, component_pool_status_sender: Sender[ComponentPoolStatus], results_sender: Sender[Result], + api_power_request_timeout: timedelta, ): """Initialize the ev charger data manager. @@ -50,8 +49,11 @@ def __init__( component_pool_status_sender: Channel for sending information about which components are expected to be working. results_sender: Channel for sending results of power distribution. + api_power_request_timeout: Timeout to use when making power requests to + the microgrid API. """ self._results_sender = results_sender + self._api_power_request_timeout = api_power_request_timeout self._ev_charger_ids = self._get_ev_charger_ids() self._evc_states = EvcStates() self._voltage_cache: LatestValueCache[Sample3Phase[Voltage]] = LatestValueCache( @@ -226,7 +228,6 @@ async def _run(self) -> None: # pylint: disable=too-many-locals *[await api.ev_charger_data(evc_id) for evc_id in self._ev_charger_ids] ) target_power_rx = self._target_power_channel.new_receiver() - api_request_timeout = _DEFAULT_API_REQUEST_TIMEOUT latest_target_powers: dict[int, Power] = {} async for selected in select(ev_charger_data_rx, target_power_rx): target_power_changes = {} @@ -260,7 +261,6 @@ async def _run(self) -> None: # pylint: disable=too-many-locals elif selected_from(selected, target_power_rx): self._latest_request = selected.message - api_request_timeout = selected.message.request_timeout self._target_power = selected.message.power _logger.debug("New target power: %s", self._target_power) used_power = self._evc_states.get_ev_total_used_power() @@ -288,7 +288,7 @@ async def _run(self) -> None: # pylint: disable=too-many-locals latest_target_powers.update(target_power_changes) result = await self._set_api_power( - api, target_power_changes, api_request_timeout + api, target_power_changes, self._api_power_request_timeout ) await self._results_sender.send(result) diff --git a/src/frequenz/sdk/actor/power_distributing/_component_managers/_pv_inverter_manager/_pv_inverter_manager.py b/src/frequenz/sdk/actor/power_distributing/_component_managers/_pv_inverter_manager/_pv_inverter_manager.py index 570c79e37..b09690802 100644 --- a/src/frequenz/sdk/actor/power_distributing/_component_managers/_pv_inverter_manager/_pv_inverter_manager.py +++ b/src/frequenz/sdk/actor/power_distributing/_component_managers/_pv_inverter_manager/_pv_inverter_manager.py @@ -38,6 +38,7 @@ def __init__( self, component_pool_status_sender: Sender[ComponentPoolStatus], results_sender: Sender[Result], + api_power_request_timeout: timedelta, ) -> None: """Initialize this instance. @@ -45,8 +46,11 @@ def __init__( component_pool_status_sender: Channel for sending information about which components are expected to be working. results_sender: Channel for sending results of power distribution. + api_power_request_timeout: Timeout to use when making power requests to + the microgrid API. """ self._results_sender = results_sender + self._api_power_request_timeout = api_power_request_timeout self._pv_inverter_ids = self._get_pv_inverter_ids() self._component_pool_status_tracker = ( @@ -177,7 +181,7 @@ async def _set_api_power( # pylint: disable=too-many-locals ) _, pending = await asyncio.wait( tasks.values(), - timeout=request.request_timeout.total_seconds(), + timeout=self._api_power_request_timeout.total_seconds(), return_when=asyncio.ALL_COMPLETED, ) # collect the timed out tasks and cancel them while keeping the diff --git a/src/frequenz/sdk/actor/power_distributing/power_distributing.py b/src/frequenz/sdk/actor/power_distributing/power_distributing.py index 4c5d38152..d418d6eeb 100644 --- a/src/frequenz/sdk/actor/power_distributing/power_distributing.py +++ b/src/frequenz/sdk/actor/power_distributing/power_distributing.py @@ -12,6 +12,8 @@ """ +from datetime import timedelta + from frequenz.channels import Receiver, Sender from frequenz.client.microgrid import ComponentCategory, ComponentType, InverterType from typing_extensions import override @@ -60,6 +62,7 @@ def __init__( # pylint: disable=too-many-arguments results_sender: Sender[Result], component_pool_status_sender: Sender[ComponentPoolStatus], *, + api_power_request_timeout: timedelta, component_category: ComponentCategory, component_type: ComponentType | None = None, name: str | None = None, @@ -72,6 +75,8 @@ def __init__( # pylint: disable=too-many-arguments results_sender: Sender for sending results to the power manager. component_pool_status_sender: Channel for sending information about which components are expected to be working. + api_power_request_timeout: Timeout to use when making power requests to + the microgrid API. component_category: The category of the components that this actor is responsible for. component_type: The type of the component of the given category that this @@ -92,22 +97,23 @@ def __init__( # pylint: disable=too-many-arguments self._component_type = component_type self._requests_receiver = requests_receiver self._result_sender = results_sender + self._api_power_request_timeout = api_power_request_timeout self._component_manager: ComponentManager if component_category == ComponentCategory.BATTERY: self._component_manager = BatteryManager( - component_pool_status_sender, results_sender + component_pool_status_sender, results_sender, api_power_request_timeout ) elif component_category == ComponentCategory.EV_CHARGER: self._component_manager = EVChargerManager( - component_pool_status_sender, results_sender + component_pool_status_sender, results_sender, api_power_request_timeout ) elif ( component_category == ComponentCategory.INVERTER and component_type == InverterType.SOLAR ): self._component_manager = PVManager( - component_pool_status_sender, results_sender + component_pool_status_sender, results_sender, api_power_request_timeout ) else: raise ValueError( diff --git a/src/frequenz/sdk/microgrid/_data_pipeline.py b/src/frequenz/sdk/microgrid/_data_pipeline.py index ccdb3c9e0..010a9219b 100644 --- a/src/frequenz/sdk/microgrid/_data_pipeline.py +++ b/src/frequenz/sdk/microgrid/_data_pipeline.py @@ -14,6 +14,7 @@ import typing from collections import abc from dataclasses import dataclass +from datetime import timedelta from frequenz.channels import Broadcast, Sender from frequenz.client.microgrid import ComponentCategory, InverterType @@ -79,11 +80,14 @@ class _DataPipeline: # pylint: disable=too-many-instance-attributes def __init__( self, resampler_config: ResamplerConfig, + api_power_request_timeout: timedelta = timedelta(seconds=5.0), ) -> None: """Create a `DataPipeline` instance. Args: resampler_config: Config to pass on to the resampler. + api_power_request_timeout: Timeout to use when making power requests to + the microgrid API. """ from ..actor import ChannelRegistry @@ -97,13 +101,18 @@ def __init__( self._resampling_actor: _ActorInfo | None = None self._battery_power_wrapper = PowerWrapper( - self._channel_registry, component_category=ComponentCategory.BATTERY + self._channel_registry, + api_power_request_timeout=api_power_request_timeout, + component_category=ComponentCategory.BATTERY, ) self._ev_power_wrapper = PowerWrapper( - self._channel_registry, component_category=ComponentCategory.EV_CHARGER + self._channel_registry, + api_power_request_timeout=api_power_request_timeout, + component_category=ComponentCategory.EV_CHARGER, ) self._pv_power_wrapper = PowerWrapper( self._channel_registry, + api_power_request_timeout=api_power_request_timeout, component_category=ComponentCategory.INVERTER, component_type=InverterType.SOLAR, ) @@ -485,11 +494,18 @@ async def _stop(self) -> None: _DATA_PIPELINE: _DataPipeline | None = None -async def initialize(resampler_config: ResamplerConfig) -> None: +async def initialize( + resampler_config: ResamplerConfig, + api_power_request_timeout: timedelta = timedelta(seconds=5.0), +) -> None: """Initialize a `DataPipeline` instance. Args: resampler_config: Config to pass on to the resampler. + api_power_request_timeout: Timeout to use when making power requests to + the microgrid API. When requests to components timeout, they will + be marked as blocked for a short duration, during which time they + will be unavailable from the corresponding component pools. Raises: RuntimeError: if the DataPipeline is already initialized. @@ -498,7 +514,7 @@ async def initialize(resampler_config: ResamplerConfig) -> None: if _DATA_PIPELINE is not None: raise RuntimeError("DataPipeline is already initialized.") - _DATA_PIPELINE = _DataPipeline(resampler_config) + _DATA_PIPELINE = _DataPipeline(resampler_config, api_power_request_timeout) def frequency() -> GridFrequency: diff --git a/src/frequenz/sdk/microgrid/_power_wrapper.py b/src/frequenz/sdk/microgrid/_power_wrapper.py index fc13e47dd..480e24cfb 100644 --- a/src/frequenz/sdk/microgrid/_power_wrapper.py +++ b/src/frequenz/sdk/microgrid/_power_wrapper.py @@ -7,6 +7,7 @@ import logging import typing +from datetime import timedelta from frequenz.channels import Broadcast @@ -38,6 +39,7 @@ def __init__( self, channel_registry: ChannelRegistry, *, + api_power_request_timeout: timedelta, component_category: ComponentCategory, component_type: ComponentType | None = None, ): @@ -45,6 +47,8 @@ def __init__( Args: channel_registry: A channel registry for use in the actors. + api_power_request_timeout: Timeout to use when making power requests to + the microgrid API. component_category: The category of the components that actors started by this instance of the PowerWrapper will be responsible for. component_type: The type of the component of the given category that this @@ -58,6 +62,7 @@ def __init__( self._component_category = component_category self._component_type = component_type self._channel_registry = channel_registry + self._api_power_request_timeout = api_power_request_timeout self.status_channel: Broadcast[ComponentPoolStatus] = Broadcast( name="Component Status Channel", resend_latest=True @@ -145,6 +150,7 @@ def _start_power_distributing_actor(self) -> None: self._power_distributing_actor = PowerDistributingActor( component_category=self._component_category, component_type=self._component_type, + api_power_request_timeout=self._api_power_request_timeout, requests_receiver=self._power_distribution_requests_channel.new_receiver(), results_sender=self._power_distribution_results_channel.new_sender(), component_pool_status_sender=self.status_channel.new_sender(), diff --git a/tests/actor/power_distributing/test_power_distributing.py b/tests/actor/power_distributing/test_power_distributing.py index f87e5a502..8a721a0c6 100644 --- a/tests/actor/power_distributing/test_power_distributing.py +++ b/tests/actor/power_distributing/test_power_distributing.py @@ -112,6 +112,7 @@ async def test_constructor_with_grid_meter(self, mocker: MockerFixture) -> None: requests_receiver=requests_channel.new_receiver(), results_sender=results_channel.new_sender(), component_pool_status_sender=battery_status_channel.new_sender(), + api_power_request_timeout=SAFETY_TIMEOUT, ) as distributor: assert isinstance(distributor._component_manager, BatteryManager) assert distributor._component_manager._bat_invs_map == { @@ -143,6 +144,7 @@ async def test_constructor_without_grid_meter(self, mocker: MockerFixture) -> No requests_receiver=requests_channel.new_receiver(), results_sender=results_channel.new_sender(), component_pool_status_sender=battery_status_channel.new_sender(), + api_power_request_timeout=SAFETY_TIMEOUT, ) as distributor: assert isinstance(distributor._component_manager, BatteryManager) assert distributor._component_manager._bat_invs_map == { @@ -208,6 +210,7 @@ async def test_power_distributor_one_user(self, mocker: MockerFixture) -> None: requests_receiver=requests_channel.new_receiver(), results_sender=results_channel.new_sender(), component_pool_status_sender=battery_status_channel.new_sender(), + api_power_request_timeout=SAFETY_TIMEOUT, ): await asyncio.sleep(0.1) # wait for actor to collect data @@ -270,6 +273,7 @@ async def test_power_distributor_exclusion_bounds( requests_receiver=requests_channel.new_receiver(), results_sender=results_channel.new_sender(), component_pool_status_sender=battery_status_channel.new_sender(), + api_power_request_timeout=SAFETY_TIMEOUT, ): await asyncio.sleep(0.1) # wait for actor to collect data @@ -374,6 +378,7 @@ async def test_two_batteries_one_inverters(self, mocker: MockerFixture) -> None: requests_receiver=requests_channel.new_receiver(), component_pool_status_sender=battery_status_channel.new_sender(), results_sender=results_channel.new_sender(), + api_power_request_timeout=SAFETY_TIMEOUT, ): await asyncio.sleep(0.1) # wait for actor to collect data @@ -455,6 +460,7 @@ async def test_two_batteries_one_broken_one_inverters( requests_receiver=requests_channel.new_receiver(), component_pool_status_sender=battery_status_channel.new_sender(), results_sender=results_channel.new_sender(), + api_power_request_timeout=SAFETY_TIMEOUT, ): await requests_channel.new_sender().send(request) result_rx = results_channel.new_receiver() @@ -510,6 +516,7 @@ async def test_battery_two_inverters(self, mocker: MockerFixture) -> None: requests_receiver=requests_channel.new_receiver(), component_pool_status_sender=battery_status_channel.new_sender(), results_sender=results_channel.new_sender(), + api_power_request_timeout=SAFETY_TIMEOUT, ): await asyncio.sleep(0.1) # wait for actor to collect data @@ -563,6 +570,7 @@ async def test_two_batteries_three_inverters(self, mocker: MockerFixture) -> Non requests_receiver=requests_channel.new_receiver(), component_pool_status_sender=battery_status_channel.new_sender(), results_sender=results_channel.new_sender(), + api_power_request_timeout=SAFETY_TIMEOUT, ): await asyncio.sleep(0.1) # wait for actor to collect data @@ -650,6 +658,7 @@ async def test_two_batteries_one_inverter_different_exclusion_bounds_2( requests_receiver=requests_channel.new_receiver(), component_pool_status_sender=battery_status_channel.new_sender(), results_sender=results_channel.new_sender(), + api_power_request_timeout=SAFETY_TIMEOUT, ): await asyncio.sleep(0.1) # wait for actor to collect data @@ -738,6 +747,7 @@ async def test_two_batteries_one_inverter_different_exclusion_bounds( requests_receiver=requests_channel.new_receiver(), component_pool_status_sender=battery_status_channel.new_sender(), results_sender=results_channel.new_sender(), + api_power_request_timeout=SAFETY_TIMEOUT, ): await asyncio.sleep(0.1) # wait for actor to collect data @@ -805,6 +815,7 @@ async def test_connected_but_not_requested_batteries( requests_receiver=requests_channel.new_receiver(), component_pool_status_sender=battery_status_channel.new_sender(), results_sender=results_channel.new_sender(), + api_power_request_timeout=SAFETY_TIMEOUT, ): await requests_channel.new_sender().send(request) result_rx = results_channel.new_receiver() @@ -862,6 +873,7 @@ async def test_battery_soc_nan(self, mocker: MockerFixture) -> None: requests_receiver=requests_channel.new_receiver(), results_sender=results_channel.new_sender(), component_pool_status_sender=battery_status_channel.new_sender(), + api_power_request_timeout=SAFETY_TIMEOUT, ): await asyncio.sleep(0.1) # wait for actor to collect data @@ -918,6 +930,7 @@ async def test_battery_capacity_nan(self, mocker: MockerFixture) -> None: requests_receiver=requests_channel.new_receiver(), results_sender=results_channel.new_sender(), component_pool_status_sender=battery_status_channel.new_sender(), + api_power_request_timeout=SAFETY_TIMEOUT, ): await asyncio.sleep(0.1) # wait for actor to collect data @@ -993,6 +1006,7 @@ async def test_battery_power_bounds_nan(self, mocker: MockerFixture) -> None: requests_receiver=requests_channel.new_receiver(), results_sender=results_channel.new_sender(), component_pool_status_sender=battery_status_channel.new_sender(), + api_power_request_timeout=SAFETY_TIMEOUT, ): await asyncio.sleep(0.1) # wait for actor to collect data @@ -1040,6 +1054,7 @@ async def test_power_distributor_invalid_battery_id( requests_receiver=requests_channel.new_receiver(), results_sender=results_channel.new_sender(), component_pool_status_sender=battery_status_channel.new_sender(), + api_power_request_timeout=SAFETY_TIMEOUT, ): await requests_channel.new_sender().send(request) result_rx = results_channel.new_receiver() @@ -1084,6 +1099,7 @@ async def test_power_distributor_one_user_adjust_power_consume( requests_receiver=requests_channel.new_receiver(), results_sender=results_channel.new_sender(), component_pool_status_sender=battery_status_channel.new_sender(), + api_power_request_timeout=SAFETY_TIMEOUT, ): await asyncio.sleep(0.1) # wait for actor to collect data @@ -1132,6 +1148,7 @@ async def test_power_distributor_one_user_adjust_power_supply( requests_receiver=requests_channel.new_receiver(), results_sender=results_channel.new_sender(), component_pool_status_sender=battery_status_channel.new_sender(), + api_power_request_timeout=SAFETY_TIMEOUT, ): await asyncio.sleep(0.1) # wait for actor to collect data @@ -1180,6 +1197,7 @@ async def test_power_distributor_one_user_adjust_power_success( requests_receiver=requests_channel.new_receiver(), results_sender=results_channel.new_sender(), component_pool_status_sender=battery_status_channel.new_sender(), + api_power_request_timeout=SAFETY_TIMEOUT, ): await asyncio.sleep(0.1) # wait for actor to collect data @@ -1221,6 +1239,7 @@ async def test_not_all_batteries_are_working(self, mocker: MockerFixture) -> Non requests_receiver=requests_channel.new_receiver(), results_sender=results_channel.new_sender(), component_pool_status_sender=battery_status_channel.new_sender(), + api_power_request_timeout=SAFETY_TIMEOUT, ): await asyncio.sleep(0.1) # wait for actor to collect data @@ -1276,6 +1295,7 @@ async def test_partial_failure_result(self, mocker: MockerFixture) -> None: requests_receiver=requests_channel.new_receiver(), results_sender=results_channel.new_sender(), component_pool_status_sender=battery_status_channel.new_sender(), + api_power_request_timeout=SAFETY_TIMEOUT, ): await asyncio.sleep(0.1) # wait for actor to collect data From 82f6ac7d76ba73d3e1db9a65775325632f5f461a Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Wed, 3 Jul 2024 14:41:45 +0200 Subject: [PATCH 3/4] Remove support for passing `request_timeout` with each power proposal Signed-off-by: Sahas Subramanian --- .../actor/_power_managing/_base_classes.py | 4 ---- .../_power_managing/_power_managing_actor.py | 4 ---- .../sdk/actor/power_distributing/request.py | 4 ---- .../timeseries/battery_pool/_battery_pool.py | 22 ++----------------- .../ev_charger_pool/_ev_charger_pool.py | 4 ---- .../sdk/timeseries/pv_pool/_pv_pool.py | 4 ---- .../test_power_distributing.py | 19 ---------------- .../test_battery_pool_control_methods.py | 4 +--- 8 files changed, 3 insertions(+), 62 deletions(-) diff --git a/src/frequenz/sdk/actor/_power_managing/_base_classes.py b/src/frequenz/sdk/actor/_power_managing/_base_classes.py index d5588ab33..79f2d2af8 100644 --- a/src/frequenz/sdk/actor/_power_managing/_base_classes.py +++ b/src/frequenz/sdk/actor/_power_managing/_base_classes.py @@ -7,7 +7,6 @@ import abc import dataclasses -import datetime import enum import typing @@ -164,9 +163,6 @@ class Proposal: This is used by the power manager to determine the age of the proposal. """ - request_timeout: datetime.timedelta = datetime.timedelta(seconds=5.0) - """The maximum amount of time to wait for the request to be fulfilled.""" - set_operating_point: bool """Whether this proposal sets the operating point power or the normal power.""" diff --git a/src/frequenz/sdk/actor/_power_managing/_power_managing_actor.py b/src/frequenz/sdk/actor/_power_managing/_power_managing_actor.py index 48ac43332..4848b8a69 100644 --- a/src/frequenz/sdk/actor/_power_managing/_power_managing_actor.py +++ b/src/frequenz/sdk/actor/_power_managing/_power_managing_actor.py @@ -330,15 +330,11 @@ async def _send_updated_target_power( proposal, must_send, ) - request_timeout = ( - proposal.request_timeout if proposal else timedelta(seconds=5.0) - ) if target_power is not None: await self._power_distributing_requests_sender.send( power_distributing.Request( power=target_power, component_ids=component_ids, - request_timeout=request_timeout, adjust_power=True, ) ) diff --git a/src/frequenz/sdk/actor/power_distributing/request.py b/src/frequenz/sdk/actor/power_distributing/request.py index ca7266e4f..ccbd3d46e 100644 --- a/src/frequenz/sdk/actor/power_distributing/request.py +++ b/src/frequenz/sdk/actor/power_distributing/request.py @@ -5,7 +5,6 @@ import dataclasses from collections import abc -from datetime import timedelta from ...timeseries._quantities import Power @@ -20,9 +19,6 @@ class Request: component_ids: abc.Set[int] """The component ids of the components to be used for this request.""" - request_timeout: timedelta = timedelta(seconds=5.0) - """The maximum amount of time to wait for the request to be fulfilled.""" - adjust_power: bool = True """Whether to adjust the power to match the bounds. diff --git a/src/frequenz/sdk/timeseries/battery_pool/_battery_pool.py b/src/frequenz/sdk/timeseries/battery_pool/_battery_pool.py index 4cd0c50ab..f132ce335 100644 --- a/src/frequenz/sdk/timeseries/battery_pool/_battery_pool.py +++ b/src/frequenz/sdk/timeseries/battery_pool/_battery_pool.py @@ -11,7 +11,6 @@ import asyncio import uuid from collections import abc -from datetime import timedelta from ... import timeseries from ..._internal._channels import ReceiverFetcher @@ -85,7 +84,6 @@ async def propose_power( self, power: Power | None, *, - request_timeout: timedelta = timedelta(seconds=5.0), bounds: timeseries.Bounds[Power | None] = timeseries.Bounds(None, None), ) -> None: """Send a proposal to the power manager for the pool's set of batteries. @@ -117,7 +115,6 @@ async def propose_power( proposal will not have any effect on the target power, unless bounds are specified. If both are `None`, it is equivalent to not having a proposal or withdrawing a previous one. - request_timeout: The timeout for the request. bounds: The power bounds for the proposal. These bounds will apply to actors with a lower priority, and can be overridden by bounds from actors with a higher priority. If None, the power bounds will be set @@ -132,17 +129,11 @@ async def propose_power( component_ids=self._pool_ref_store._batteries, priority=self._priority, creation_time=asyncio.get_running_loop().time(), - request_timeout=request_timeout, set_operating_point=self._set_operating_point, ) ) - async def propose_charge( - self, - power: Power | None, - *, - request_timeout: timedelta = timedelta(seconds=5.0), - ) -> None: + async def propose_charge(self, power: Power | None) -> None: """Set the given charge power for the batteries in the pool. Power values need to be positive values, indicating charge power. @@ -164,7 +155,6 @@ async def propose_charge( power: The unsigned charge power to propose for the batteries in the pool. If None, the proposed power of higher priority actors will take precedence as the target power. - request_timeout: The timeout for the request. Raises: ValueError: If the given power is negative. @@ -179,17 +169,11 @@ async def propose_charge( component_ids=self._pool_ref_store._batteries, priority=self._priority, creation_time=asyncio.get_running_loop().time(), - request_timeout=request_timeout, set_operating_point=self._set_operating_point, ) ) - async def propose_discharge( - self, - power: Power | None, - *, - request_timeout: timedelta = timedelta(seconds=5.0), - ) -> None: + async def propose_discharge(self, power: Power | None) -> None: """Set the given discharge power for the batteries in the pool. Power values need to be positive values, indicating discharge power. @@ -211,7 +195,6 @@ async def propose_discharge( power: The unsigned discharge power to propose for the batteries in the pool. If None, the proposed power of higher priority actors will take precedence as the target power. - request_timeout: The timeout for the request. Raises: ValueError: If the given power is negative. @@ -228,7 +211,6 @@ async def propose_discharge( component_ids=self._pool_ref_store._batteries, priority=self._priority, creation_time=asyncio.get_running_loop().time(), - request_timeout=request_timeout, set_operating_point=self._set_operating_point, ) ) diff --git a/src/frequenz/sdk/timeseries/ev_charger_pool/_ev_charger_pool.py b/src/frequenz/sdk/timeseries/ev_charger_pool/_ev_charger_pool.py index b45144b31..1b44f4973 100644 --- a/src/frequenz/sdk/timeseries/ev_charger_pool/_ev_charger_pool.py +++ b/src/frequenz/sdk/timeseries/ev_charger_pool/_ev_charger_pool.py @@ -7,7 +7,6 @@ import asyncio import uuid from collections import abc -from datetime import timedelta from ..._internal._channels import ReceiverFetcher from ...actor import _power_managing @@ -73,7 +72,6 @@ async def propose_power( self, power: Power | None, *, - request_timeout: timedelta = timedelta(seconds=5.0), bounds: Bounds[Power | None] = Bounds(None, None), ) -> None: """Send a proposal to the power manager for the pool's set of EV chargers. @@ -110,7 +108,6 @@ async def propose_power( this proposal will not have any effect on the target power, unless bounds are specified. If both are `None`, it is equivalent to not having a proposal or withdrawing a previous one. - request_timeout: The timeout for the request. bounds: The power bounds for the proposal. These bounds will apply to actors with a lower priority, and can be overridden by bounds from actors with a higher priority. If None, the power bounds will be set to @@ -132,7 +129,6 @@ async def propose_power( component_ids=self._pool_ref_store.component_ids, priority=self._priority, creation_time=asyncio.get_running_loop().time(), - request_timeout=request_timeout, set_operating_point=self._set_operating_point, ) ) diff --git a/src/frequenz/sdk/timeseries/pv_pool/_pv_pool.py b/src/frequenz/sdk/timeseries/pv_pool/_pv_pool.py index 8f3ed23fb..7e7a24745 100644 --- a/src/frequenz/sdk/timeseries/pv_pool/_pv_pool.py +++ b/src/frequenz/sdk/timeseries/pv_pool/_pv_pool.py @@ -6,7 +6,6 @@ import asyncio import uuid from collections import abc -from datetime import timedelta from ..._internal._channels import ReceiverFetcher from ...actor import _power_managing @@ -63,7 +62,6 @@ async def propose_power( self, power: Power | None, *, - request_timeout: timedelta = timedelta(seconds=5.0), bounds: Bounds[Power | None] = Bounds(None, None), ) -> None: """Send a proposal to the power manager for the pool's set of PV inverters. @@ -99,7 +97,6 @@ async def propose_power( this proposal will not have any effect on the target power, unless bounds are specified. If both are `None`, it is equivalent to not having a proposal or withdrawing a previous one. - request_timeout: The timeout for the request. bounds: The power bounds for the proposal. These bounds will apply to actors with a lower priority, and can be overridden by bounds from actors with a higher priority. If None, the power bounds will be set to @@ -119,7 +116,6 @@ async def propose_power( component_ids=self._pool_ref_store.component_ids, priority=self._priority, creation_time=asyncio.get_running_loop().time(), - request_timeout=request_timeout, set_operating_point=self._set_operating_point, ) ) diff --git a/tests/actor/power_distributing/test_power_distributing.py b/tests/actor/power_distributing/test_power_distributing.py index 8a721a0c6..f8afcf7b5 100644 --- a/tests/actor/power_distributing/test_power_distributing.py +++ b/tests/actor/power_distributing/test_power_distributing.py @@ -197,7 +197,6 @@ async def test_power_distributor_one_user(self, mocker: MockerFixture) -> None: request = Request( power=Power.from_kilowatts(1.2), component_ids={9, 19}, - request_timeout=SAFETY_TIMEOUT, ) await self._patch_battery_pool_status(mocks, mocker, request.component_ids) @@ -281,7 +280,6 @@ async def test_power_distributor_exclusion_bounds( request = Request( power=Power.zero(), component_ids={9, 19}, - request_timeout=SAFETY_TIMEOUT, ) await requests_channel.new_sender().send(request) @@ -306,7 +304,6 @@ async def test_power_distributor_exclusion_bounds( request = Request( power=Power.from_watts(300.0), component_ids={9, 19}, - request_timeout=SAFETY_TIMEOUT, ) await requests_channel.new_sender().send(request) @@ -363,7 +360,6 @@ async def test_two_batteries_one_inverters(self, mocker: MockerFixture) -> None: bat_component1.component_id, bat_component2.component_id, }, - request_timeout=SAFETY_TIMEOUT, ) await self._patch_battery_pool_status(mocks, mocker, request.component_ids) @@ -446,7 +442,6 @@ async def test_two_batteries_one_broken_one_inverters( request = Request( power=Power.from_watts(1200.0), component_ids=set(battery.component_id for battery in bat_components), - request_timeout=SAFETY_TIMEOUT, ) await self._patch_battery_pool_status(mocks, mocker, request.component_ids) @@ -502,7 +497,6 @@ async def test_battery_two_inverters(self, mocker: MockerFixture) -> None: request = Request( power=Power.from_watts(1200.0), component_ids={bat_component.component_id}, - request_timeout=SAFETY_TIMEOUT, ) await self._patch_battery_pool_status(mocks, mocker, request.component_ids) @@ -556,7 +550,6 @@ async def test_two_batteries_three_inverters(self, mocker: MockerFixture) -> Non request = Request( power=Power.from_watts(1700.0), component_ids={batteries[0].component_id, batteries[1].component_id}, - request_timeout=SAFETY_TIMEOUT, ) await self._patch_battery_pool_status(mocks, mocker, request.component_ids) @@ -644,7 +637,6 @@ async def test_two_batteries_one_inverter_different_exclusion_bounds_2( request = Request( power=Power.from_watts(300.0), component_ids={batteries[0].component_id, batteries[1].component_id}, - request_timeout=SAFETY_TIMEOUT, ) await self._patch_battery_pool_status(mocks, mocker, request.component_ids) @@ -733,7 +725,6 @@ async def test_two_batteries_one_inverter_different_exclusion_bounds( request = Request( power=Power.from_watts(300.0), component_ids={batteries[0].component_id, batteries[1].component_id}, - request_timeout=SAFETY_TIMEOUT, ) await self._patch_battery_pool_status(mocks, mocker, request.component_ids) @@ -801,7 +792,6 @@ async def test_connected_but_not_requested_batteries( request = Request( power=Power.from_watts(600.0), component_ids={batteries[0].component_id}, - request_timeout=SAFETY_TIMEOUT, ) await self._patch_battery_pool_status(mocks, mocker, request.component_ids) @@ -860,7 +850,6 @@ async def test_battery_soc_nan(self, mocker: MockerFixture) -> None: request = Request( power=Power.from_kilowatts(1.2), component_ids={9, 19}, - request_timeout=SAFETY_TIMEOUT, ) await self._patch_battery_pool_status(mocks, mocker, request.component_ids) @@ -916,7 +905,6 @@ async def test_battery_capacity_nan(self, mocker: MockerFixture) -> None: request = Request( power=Power.from_kilowatts(1.2), component_ids={9, 19}, - request_timeout=SAFETY_TIMEOUT, ) await self._patch_battery_pool_status(mocks, mocker, request.component_ids) @@ -992,7 +980,6 @@ async def test_battery_power_bounds_nan(self, mocker: MockerFixture) -> None: request = Request( power=Power.from_kilowatts(1.2), component_ids={9, 19}, - request_timeout=SAFETY_TIMEOUT, ) await self._patch_battery_pool_status(mocks, mocker, request.component_ids) @@ -1040,7 +1027,6 @@ async def test_power_distributor_invalid_battery_id( request = Request( power=Power.from_kilowatts(1.2), component_ids={9, 100}, - request_timeout=SAFETY_TIMEOUT, ) await self._patch_battery_pool_status(mocks, mocker, request.component_ids) @@ -1084,7 +1070,6 @@ async def test_power_distributor_one_user_adjust_power_consume( request = Request( power=Power.from_kilowatts(1.2), component_ids={9, 19}, - request_timeout=SAFETY_TIMEOUT, adjust_power=False, ) @@ -1133,7 +1118,6 @@ async def test_power_distributor_one_user_adjust_power_supply( request = Request( power=-Power.from_kilowatts(1.2), component_ids={9, 19}, - request_timeout=SAFETY_TIMEOUT, adjust_power=False, ) @@ -1182,7 +1166,6 @@ async def test_power_distributor_one_user_adjust_power_success( request = Request( power=Power.from_kilowatts(1.0), component_ids={9, 19}, - request_timeout=SAFETY_TIMEOUT, adjust_power=False, ) @@ -1246,7 +1229,6 @@ async def test_not_all_batteries_are_working(self, mocker: MockerFixture) -> Non request = Request( power=Power.from_kilowatts(1.2), component_ids=batteries, - request_timeout=SAFETY_TIMEOUT, ) await requests_channel.new_sender().send(request) @@ -1302,7 +1284,6 @@ async def test_partial_failure_result(self, mocker: MockerFixture) -> None: request = Request( power=Power.from_kilowatts(1.70), component_ids=batteries, - request_timeout=SAFETY_TIMEOUT, ) await requests_channel.new_sender().send(request) diff --git a/tests/timeseries/_battery_pool/test_battery_pool_control_methods.py b/tests/timeseries/_battery_pool/test_battery_pool_control_methods.py index 3cd3a0416..27bffc6c0 100644 --- a/tests/timeseries/_battery_pool/test_battery_pool_control_methods.py +++ b/tests/timeseries/_battery_pool/test_battery_pool_control_methods.py @@ -230,9 +230,7 @@ async def side_effect(inv_id: int, _: float) -> None: await asyncio.sleep(1000.0) set_power.side_effect = side_effect - await battery_pool.propose_power( - Power.from_watts(100.0), request_timeout=timedelta(seconds=0.1) - ) + await battery_pool.propose_power(Power.from_watts(100.0)) self._assert_report( await bounds_rx.receive(), power=100.0, From e756de54fcf58e58b96a972ee3b2ece4ea503f57 Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Mon, 8 Jul 2024 10:37:29 +0200 Subject: [PATCH 4/4] Update release notes Signed-off-by: Sahas Subramanian --- RELEASE_NOTES.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index a751794d5..82f612b73 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -17,6 +17,8 @@ * `grid.current` -> `grid.current_per_phase` * `ev_charger_pool.current` -> `ev_charger_pool.current_per_phase` +* Passing a `request_timeout` in calls to `*_pool.propose_power` is no longer supported. It may be specified at application startup, through the new optional `api_power_request_timeout` parameter in the `microgrid.initialize()` method. + ## New Features - Classes `Bounds` and `SystemBounds` now implement the `__contains__` method, allowing the use of the `in` operator to check whether a value falls within the bounds or not.