Skip to content

Commit d2a305a

Browse files
committed
Refactor battery_id validation to improve readability
Signed-off-by: Sahas Subramanian <[email protected]>
1 parent a8bf82d commit d2a305a

File tree

1 file changed

+34
-23
lines changed

1 file changed

+34
-23
lines changed

src/frequenz/sdk/actor/_power_managing/_matryoshka.py

Lines changed: 34 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,37 @@ def _calc_target_power(
9191

9292
return target_power
9393

94+
def _validate_battery_ids(
95+
self,
96+
battery_ids: frozenset[int],
97+
proposal: Proposal | None,
98+
system_bounds: PowerMetrics,
99+
) -> bool:
100+
if battery_ids not in self._battery_buckets:
101+
# if there are no previous proposals and there are no system bounds, then
102+
# don't calculate a target power and fail the validation.
103+
if (
104+
system_bounds.inclusion_bounds is None
105+
and system_bounds.exclusion_bounds is None
106+
):
107+
if proposal is not None:
108+
_logger.warning(
109+
"PowerManagingActor: No system bounds available for battery "
110+
"IDs %s, but a proposal was given. The proposal will be "
111+
"ignored.",
112+
battery_ids,
113+
)
114+
return False
115+
116+
for bucket in self._battery_buckets:
117+
if any(battery_id in bucket for battery_id in battery_ids):
118+
raise NotImplementedError(
119+
f"PowerManagingActor: Battery IDs {battery_ids} are already "
120+
"part of another bucket. Overlapping buckets are not "
121+
"yet supported."
122+
)
123+
return True
124+
94125
@override
95126
def get_target_power(
96127
self,
@@ -113,33 +144,13 @@ def get_target_power(
113144
The new target power for the batteries, or `None` if the target power
114145
didn't change.
115146
116-
Raises:
147+
Raises: # noqa: DOC502
117148
NotImplementedError: When the proposal contains battery IDs that are
118149
already part of another bucket.
119150
"""
120-
if battery_ids not in self._battery_buckets:
121-
# if there are no previous proposals and there are no system bounds, then
122-
# don't calculate a target power and just return `None`.
123-
if (
124-
system_bounds.inclusion_bounds is None
125-
and system_bounds.exclusion_bounds is None
126-
):
127-
if proposal is not None:
128-
_logger.warning(
129-
"PowerManagingActor: No system bounds available for battery "
130-
"IDs %s, but a proposal was given. The proposal will be "
131-
"ignored.",
132-
battery_ids,
133-
)
134-
return None
151+
if not self._validate_battery_ids(battery_ids, proposal, system_bounds):
152+
return None
135153

136-
for bucket in self._battery_buckets:
137-
if any(battery_id in bucket for battery_id in battery_ids):
138-
raise NotImplementedError(
139-
f"PowerManagingActor: Battery IDs {battery_ids} are already "
140-
"part of another bucket. Overlapping buckets are not "
141-
"yet supported."
142-
)
143154
if proposal is not None:
144155
self._battery_buckets.setdefault(battery_ids, SortedSet()).insert(proposal)
145156

0 commit comments

Comments
 (0)