Skip to content

Commit 791c6cd

Browse files
Update power distributing Result to Power type
Using the Power type enhances code clarity and flexibility, as it supports various power units, power arithmetic operations, and precise logging, which improves code readability and error prevention. Signed-off-by: Daniel Zullo <[email protected]>
1 parent 6bbe2fb commit 791c6cd

File tree

3 files changed

+35
-31
lines changed

3 files changed

+35
-31
lines changed

src/frequenz/sdk/actor/power_distributing/power_distributing.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
import grpc
2727
from frequenz.channels import Peekable, Receiver, Sender
2828

29+
from frequenz.sdk.timeseries._quantities import Power
30+
2931
from ..._internal._math import is_close_to_zero
3032
from ...actor import ChannelRegistry
3133
from ...actor._actor import Actor
@@ -291,19 +293,19 @@ async def _run(self) -> None:
291293
succeed_batteries = set(battery_distribution.keys()) - failed_batteries
292294
response = PartialFailure(
293295
request=request,
294-
succeeded_power=distributed_power_value,
296+
succeeded_power=Power.from_watts(distributed_power_value),
295297
succeeded_batteries=succeed_batteries,
296-
failed_power=failed_power,
298+
failed_power=Power.from_watts(failed_power),
297299
failed_batteries=failed_batteries,
298-
excess_power=distribution.remaining_power,
300+
excess_power=Power.from_watts(distribution.remaining_power),
299301
)
300302
else:
301303
succeed_batteries = set(battery_distribution.keys())
302304
response = Success(
303305
request=request,
304-
succeeded_power=distributed_power_value,
306+
succeeded_power=Power.from_watts(distributed_power_value),
305307
succeeded_batteries=succeed_batteries,
306-
excess_power=distribution.remaining_power,
308+
excess_power=Power.from_watts(distribution.remaining_power),
307309
)
308310

309311
asyncio.gather(

src/frequenz/sdk/actor/power_distributing/result.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77

88
import dataclasses
99

10+
from frequenz.sdk.timeseries._quantities import Power
11+
1012
from .request import Request
1113

1214

@@ -34,13 +36,13 @@ class Result(_BaseResultMixin):
3436
class _BaseSuccessMixin:
3537
"""Result returned when setting the power succeed for all batteries."""
3638

37-
succeeded_power: float
39+
succeeded_power: Power
3840
"""The part of the requested power that was successfully set."""
3941

4042
succeeded_batteries: set[int]
4143
"""The subset of batteries for which power was set successfully."""
4244

43-
excess_power: float
45+
excess_power: Power
4446
"""The part of the requested power that could not be fulfilled.
4547
4648
This happens when the requested power is outside the available power bounds.
@@ -62,7 +64,7 @@ class Success(_BaseSuccessMixin, Result): # Order matters here. See above.
6264
class PartialFailure(_BaseSuccessMixin, Result):
6365
"""Result returned when any battery failed to perform the request."""
6466

65-
failed_power: float
67+
failed_power: Power
6668
"""The part of the requested power that failed to be set."""
6769

6870
failed_batteries: set[int]

tests/actor/power_distributing/test_power_distributing.py

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
from unittest.mock import AsyncMock, MagicMock
1313

1414
from frequenz.channels import Broadcast
15-
from pytest import approx
1615
from pytest_mock import MockerFixture
1716

1817
from frequenz.sdk import microgrid
@@ -31,6 +30,7 @@
3130
Success,
3231
)
3332
from frequenz.sdk.microgrid.component import ComponentCategory
33+
from frequenz.sdk.timeseries._quantities import Power
3434
from tests.timeseries.mock_microgrid import MockMicrogrid
3535

3636
from ...conftest import SAFETY_TIMEOUT
@@ -143,8 +143,8 @@ async def test_power_distributor_one_user(self, mocker: MockerFixture) -> None:
143143

144144
result: Result = done.pop().result()
145145
assert isinstance(result, Success)
146-
assert result.succeeded_power == approx(1000.0)
147-
assert result.excess_power == approx(200.0)
146+
assert result.succeeded_power.isclose(Power.from_kilowatts(1.0))
147+
assert result.excess_power.isclose(Power.from_watts(200.0))
148148
assert result.request == request
149149

150150
async def test_power_distributor_exclusion_bounds(
@@ -212,8 +212,8 @@ async def test_power_distributor_exclusion_bounds(
212212

213213
result: Result = done.pop().result()
214214
assert isinstance(result, Success)
215-
assert result.succeeded_power == approx(0.0)
216-
assert result.excess_power == approx(0.0)
215+
assert result.succeeded_power.isclose(Power.zero(), abs_tol=1e-9)
216+
assert result.excess_power.isclose(Power.zero(), abs_tol=1e-9)
217217
assert result.request == request
218218

219219
## non-zero power requests that fall within the exclusion bounds should be
@@ -300,8 +300,8 @@ async def test_battery_soc_nan(self, mocker: MockerFixture) -> None:
300300
result: Result = done.pop().result()
301301
assert isinstance(result, Success)
302302
assert result.succeeded_batteries == {19}
303-
assert result.succeeded_power == approx(500.0)
304-
assert result.excess_power == approx(700.0)
303+
assert result.succeeded_power.isclose(Power.from_watts(500.0))
304+
assert result.excess_power.isclose(Power.from_watts(700.0))
305305
assert result.request == request
306306

307307
async def test_battery_capacity_nan(self, mocker: MockerFixture) -> None:
@@ -356,8 +356,8 @@ async def test_battery_capacity_nan(self, mocker: MockerFixture) -> None:
356356
result: Result = done.pop().result()
357357
assert isinstance(result, Success)
358358
assert result.succeeded_batteries == {19}
359-
assert result.succeeded_power == approx(500.0)
360-
assert result.excess_power == approx(700.0)
359+
assert result.succeeded_power.isclose(Power.from_watts(500.0))
360+
assert result.excess_power.isclose(Power.from_watts(700.0))
361361
assert result.request == request
362362

363363
async def test_battery_power_bounds_nan(self, mocker: MockerFixture) -> None:
@@ -428,8 +428,8 @@ async def test_battery_power_bounds_nan(self, mocker: MockerFixture) -> None:
428428
result: Result = done.pop().result()
429429
assert isinstance(result, Success)
430430
assert result.succeeded_batteries == {19}
431-
assert result.succeeded_power == approx(1000.0)
432-
assert result.excess_power == approx(200.0)
431+
assert result.succeeded_power.isclose(Power.from_kilowatts(1.0))
432+
assert result.excess_power.isclose(Power.from_watts(200.0))
433433
assert result.request == request
434434

435435
async def test_power_distributor_invalid_battery_id(
@@ -627,8 +627,8 @@ async def test_power_distributor_one_user_adjust_power_success(
627627

628628
result = done.pop().result()
629629
assert isinstance(result, Success)
630-
assert result.succeeded_power == approx(1000.0)
631-
assert result.excess_power == approx(0.0)
630+
assert result.succeeded_power.isclose(Power.from_kilowatts(1.0))
631+
assert result.excess_power.isclose(Power.zero(), abs_tol=1e-9)
632632
assert result.request == request
633633

634634
async def test_not_all_batteries_are_working(self, mocker: MockerFixture) -> None:
@@ -676,8 +676,8 @@ async def test_not_all_batteries_are_working(self, mocker: MockerFixture) -> Non
676676
result = done.pop().result()
677677
assert isinstance(result, Success)
678678
assert result.succeeded_batteries == {19}
679-
assert result.excess_power == approx(700.0)
680-
assert result.succeeded_power == approx(500.0)
679+
assert result.excess_power.isclose(Power.from_watts(700.0))
680+
assert result.succeeded_power.isclose(Power.from_watts(500.0))
681681
assert result.request == request
682682

683683
async def test_use_all_batteries_none_is_working(
@@ -726,8 +726,8 @@ async def test_use_all_batteries_none_is_working(
726726
result = done.pop().result()
727727
assert isinstance(result, Success)
728728
assert result.succeeded_batteries == {9, 19}
729-
assert result.excess_power == approx(200.0)
730-
assert result.succeeded_power == approx(1000.0)
729+
assert result.excess_power.isclose(Power.from_watts(200.0))
730+
assert result.succeeded_power.isclose(Power.from_kilowatts(1.0))
731731
assert result.request == request
732732

733733
async def test_force_request_a_battery_is_not_working(
@@ -778,8 +778,8 @@ async def test_force_request_a_battery_is_not_working(
778778
result = done.pop().result()
779779
assert isinstance(result, Success)
780780
assert result.succeeded_batteries == {9, 19}
781-
assert result.excess_power == approx(200.0)
782-
assert result.succeeded_power == approx(1000.0)
781+
assert result.excess_power.isclose(Power.from_watts(200.0))
782+
assert result.succeeded_power.isclose(Power.from_kilowatts(1.0))
783783
assert result.request == request
784784

785785
async def test_force_request_battery_nan_value_non_cached(
@@ -849,8 +849,8 @@ async def test_force_request_battery_nan_value_non_cached(
849849
result: Result = done.pop().result()
850850
assert isinstance(result, Success)
851851
assert result.succeeded_batteries == batteries
852-
assert result.succeeded_power == approx(1199.9999)
853-
assert result.excess_power == approx(0.0)
852+
assert result.succeeded_power.isclose(Power.from_kilowatts(1.2))
853+
assert result.excess_power.isclose(Power.zero(), abs_tol=1e-9)
854854
assert result.request == request
855855

856856
async def test_force_request_batteries_nan_values_cached(
@@ -900,8 +900,8 @@ async def test_result() -> None:
900900
result: Result = done.pop().result()
901901
assert isinstance(result, Success)
902902
assert result.succeeded_batteries == batteries
903-
assert result.succeeded_power == approx(1199.9999)
904-
assert result.excess_power == approx(0.0)
903+
assert result.succeeded_power.isclose(Power.from_kilowatts(1.2))
904+
assert result.excess_power.isclose(Power.zero(), abs_tol=1e-9)
905905
assert result.request == request
906906

907907
batteries_data = (

0 commit comments

Comments
 (0)