Skip to content

Commit d0d3b3f

Browse files
authored
Minor improvements and cleanups (#1293)
This PR focuses on improving test resilience and code maintainability through refactoring, better logging, timeout additions, and various cleanups. The changes enhance test reliability by adding timeout mechanisms to prevent hanging tests and improve code readability through better imports, simplified logic, and consistent parameter ordering. **Key changes:** - Added timeout mechanisms (10 seconds) to test report receiving operations to prevent hanging tests - Refactored test fixtures to use shared `stop()` methods for cleaner cleanup - Improved type hints for NetworkX graphs and added type stubs to mypy configuration - Standardized parameter ordering (positional before keyword-only) in component status trackers
2 parents 3b08a0c + fe5ef13 commit d0d3b3f

File tree

16 files changed

+140
-111
lines changed

16 files changed

+140
-111
lines changed

docs/user-guide/glossary.md

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ article](https://en.wikipedia.org/wiki/State_of_charge) for more details.
5858

5959
A local electrical grid that connects a set of different [types of
6060
components](#component-category) together. It can be connected to the public
61-
[grid](#grid), or be completely isolated, in which case it is known as an
62-
island.
61+
[grid](#grid) (through a [grid connection point](#grid-connection-point)), or be completely isolated, in which case
62+
it is known as an island.
6363

6464
Components can be grouped into [assets](#assets) and [devices](#devices).
6565
Assets are core components like generators or storage systems that are crucial from a business perspective,
@@ -105,6 +105,15 @@ A station for charging [EVs](#ev).
105105

106106
A device that converts water into hydrogen and oxygen.
107107

108+
#### Grid Connection Point
109+
110+
The point where the local [microgrid](#microgrid) is connected to the public
111+
electricity [grid](#grid).
112+
113+
#### GCP
114+
115+
[Grid connection point](#grid-connection-point).
116+
108117
#### Grid
109118

110119
A point where the local [microgrid](#microgrid) is connected to the public

pyproject.toml

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ dev-mypy = [
7171
"types-Markdown == 3.9.0.20250906",
7272
"types-protobuf == 6.32.1.20250918",
7373
"types-setuptools == 80.9.0.20250822",
74+
"types-networkx == 3.5.0.20251106",
7475
# For checking the noxfile, docs/ script, and tests
7576
"frequenz-sdk[dev-mkdocs,dev-noxfile,dev-pytest]",
7677
]
@@ -204,13 +205,7 @@ files = ["src", "tests", "examples", "benchmarks", "docs", "noxfile.py"]
204205
strict = true
205206

206207
[[tool.mypy.overrides]]
207-
module = [
208-
"async_solipsism",
209-
"mkdocs_macros.*",
210-
# The available stubs packages are outdated or incomplete (WIP/experimental):
211-
# https://github.com/frequenz-floss/frequenz-sdk-python/issues/430
212-
"networkx",
213-
]
208+
module = ["async_solipsism", "mkdocs_macros.*"]
214209
ignore_missing_imports = true
215210

216211
[tool.setuptools_scm]

src/frequenz/sdk/microgrid/__init__.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
3333
subgraph Left[Measurements only]
3434
direction LR
35-
grid["Grid Connection"]
35+
grid["Grid Connection Point"]
3636
consumer["Consumer"]
3737
pv["PV Arrays"]
3838
chp["CHP"]
@@ -57,12 +57,12 @@
5757
5858
## Grid
5959
60-
This refers to a microgrid's connection to the external Grid. The power flowing through
61-
this connection can be streamed through
60+
This refers to a microgrid's {{glossary("grid-connection-point")}}. The power flowing
61+
through this connection can be streamed through
6262
[`grid_power`][frequenz.sdk.timeseries.grid.Grid.power].
6363
64-
In locations without a grid connection, this method remains accessible, and streams zero
65-
values.
64+
In locations without a grid connection point, this method remains accessible, and
65+
streams zero values.
6666
6767
## Consumer
6868

src/frequenz/sdk/microgrid/_power_distributing/_component_managers/_battery_manager.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -609,7 +609,7 @@ def _get_power_distribution(
609609
for inverter_ids in [
610610
self._bat_invs_map[battery_id_set] for battery_id_set in unavailable_bat_ids
611611
]:
612-
unavailable_inv_ids = unavailable_inv_ids.union(inverter_ids)
612+
unavailable_inv_ids = unavailable_inv_ids | inverter_ids
613613

614614
result = self._distribution_algorithm.distribute_power(
615615
request.power, inv_bat_pairs

src/frequenz/sdk/microgrid/_power_distributing/_component_status/_battery_status_tracker.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,25 +100,25 @@ class BatteryStatusTracker(ComponentStatusTracker, BackgroundService):
100100
@override
101101
def __init__( # pylint: disable=too-many-arguments
102102
self,
103-
*,
104103
component_id: ComponentId,
105-
max_data_age: timedelta,
106-
max_blocking_duration: timedelta,
107104
status_sender: Sender[ComponentStatus],
108105
set_power_result_receiver: Receiver[SetPowerResult],
106+
*,
107+
max_data_age: timedelta,
108+
max_blocking_duration: timedelta,
109109
) -> None:
110110
"""Create class instance.
111111
112112
Args:
113113
component_id: Id of this battery
114+
status_sender: Channel to send status updates.
115+
set_power_result_receiver: Channel to receive results of the requests to the
116+
components.
114117
max_data_age: If component stopped sending data, then this is the maximum
115118
time when its last message should be considered as valid. After that
116119
time, component won't be used until it starts sending data.
117120
max_blocking_duration: This value tell what should be the maximum
118121
timeout used for blocking failing component.
119-
status_sender: Channel to send status updates.
120-
set_power_result_receiver: Channel to receive results of the requests to the
121-
components.
122122
123123
Raises:
124124
RuntimeError: If battery has no adjacent inverter.

src/frequenz/sdk/microgrid/_power_distributing/_component_status/_component_status.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -88,23 +88,23 @@ class ComponentStatusTracker(BackgroundService, ABC):
8888
@abstractmethod
8989
def __init__( # pylint: disable=too-many-arguments,super-init-not-called
9090
self,
91-
*,
9291
component_id: ComponentId,
93-
max_data_age: timedelta,
94-
max_blocking_duration: timedelta,
9592
status_sender: Sender[ComponentStatus],
9693
set_power_result_receiver: Receiver[SetPowerResult],
94+
*,
95+
max_data_age: timedelta,
96+
max_blocking_duration: timedelta,
9797
) -> None:
9898
"""Create class instance.
9999
100100
Args:
101101
component_id: Id of this component
102+
status_sender: Channel to send status updates.
103+
set_power_result_receiver: Channel to receive results of the requests to the
104+
components.
102105
max_data_age: If component stopped sending data, then this is the maximum
103106
time when its last message should be considered as valid. After that
104107
time, component won't be used until it starts sending data.
105108
max_blocking_duration: This value tell what should be the maximum
106109
timeout used for blocking failing component.
107-
status_sender: Channel to send status updates.
108-
set_power_result_receiver: Channel to receive results of the requests to the
109-
components.
110110
"""

src/frequenz/sdk/microgrid/_power_distributing/_component_status/_ev_charger_status_tracker.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,25 +44,25 @@ class EVChargerStatusTracker(ComponentStatusTracker, BackgroundService):
4444
@override
4545
def __init__( # pylint: disable=too-many-arguments
4646
self,
47-
*,
4847
component_id: ComponentId,
49-
max_data_age: timedelta,
50-
max_blocking_duration: timedelta,
5148
status_sender: Sender[ComponentStatus],
5249
set_power_result_receiver: Receiver[SetPowerResult],
50+
*,
51+
max_data_age: timedelta,
52+
max_blocking_duration: timedelta,
5353
) -> None:
5454
"""Initialize this instance.
5555
5656
Args:
5757
component_id: ID of the EV charger to monitor the status of.
58-
max_data_age: max duration to wait for, before marking a component as
59-
NOT_WORKING, unless new data arrives.
60-
max_blocking_duration: duration for which the component status should be
61-
UNCERTAIN if a request to the component failed unexpectedly.
6258
status_sender: Channel sender to send status updates to.
6359
set_power_result_receiver: Receiver to fetch PowerDistributor responses
6460
from, to get the status of the most recent request made for an EV
6561
Charger.
62+
max_data_age: max duration to wait for, before marking a component as
63+
NOT_WORKING, unless new data arrives.
64+
max_blocking_duration: duration for which the component status should be
65+
UNCERTAIN if a request to the component failed unexpectedly.
6666
"""
6767
self._component_id = component_id
6868
self._max_data_age = max_data_age

src/frequenz/sdk/microgrid/component_graph.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ def __init__(
369369
InvalidGraphError: If `components` and `connections` are not both `None`
370370
and either of them is either `None` or empty.
371371
"""
372-
self._graph: nx.DiGraph = nx.DiGraph()
372+
self._graph: nx.DiGraph[ComponentId] = nx.DiGraph()
373373

374374
if components is None and connections is None:
375375
return
@@ -437,6 +437,7 @@ def connections(
437437
"""
438438
matching_sources = _comp_ids_to_iter(matching_sources)
439439
matching_destinations = _comp_ids_to_iter(matching_destinations)
440+
selection: Iterable[tuple[ComponentId, ComponentId]]
440441

441442
match (matching_sources, matching_destinations):
442443
case (None, None):
@@ -536,7 +537,7 @@ def refresh_from(
536537
if issues:
537538
raise InvalidGraphError(f"Invalid component data: {', '.join(issues)}")
538539

539-
new_graph = nx.DiGraph()
540+
new_graph: nx.DiGraph[ComponentId] = nx.DiGraph()
540541
new_graph.add_nodes_from(
541542
(component.id, {_DATA_KEY: component}) for component in components
542543
)
@@ -1120,6 +1121,11 @@ def _validate_leaf_components(self) -> None:
11201121
f"Leaf components with graph successors: {with_successors}"
11211122
)
11221123

1124+
@override
1125+
def __repr__(self) -> str:
1126+
"""Return a string representation of the component graph."""
1127+
return f"ComponentGraph({self._graph!r})"
1128+
11231129

11241130
def _comp_ids_to_iter(
11251131
ids: Iterable[ComponentId] | ComponentId | None,

src/frequenz/sdk/microgrid/connection_manager.py

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ def api_client(self) -> MicrogridApiClient:
4646
"""Get the MicrogridApiClient.
4747
4848
Returns:
49-
api client
49+
The microgrid API client used by this connection manager.
5050
"""
5151

5252
@property
@@ -172,28 +172,23 @@ async def initialize(server_url: str) -> None:
172172
where the port should be an int between `0` and `65535` (defaulting to
173173
`9090`) and ssl should be a boolean (defaulting to false). For example:
174174
`grpc://localhost:1090?ssl=true`.
175-
176-
Raises:
177-
AssertionError: If method was called more then once.
178175
"""
179176
# From Doc: pylint just try to discourage this usage.
180177
# That doesn't mean you cannot use it.
181178
global _CONNECTION_MANAGER # pylint: disable=global-statement
182179

183-
if _CONNECTION_MANAGER is not None:
184-
raise AssertionError("MicrogridApi was already initialized.")
180+
assert _CONNECTION_MANAGER is None, "MicrogridApi was already initialized."
185181

186182
_logger.info("Connecting to microgrid at %s", server_url)
187183

188-
microgrid_api = _InsecureConnectionManager(server_url)
189-
await microgrid_api._initialize() # pylint: disable=protected-access
184+
connection_manager = _InsecureConnectionManager(server_url)
185+
await connection_manager._initialize() # pylint: disable=protected-access
190186

191187
# Check again that _MICROGRID_API is None in case somebody had the great idea of
192188
# calling initialize() twice and in parallel.
193-
if _CONNECTION_MANAGER is not None:
194-
raise AssertionError("MicrogridApi was already initialized.")
189+
assert _CONNECTION_MANAGER is None, "MicrogridApi was already initialized."
195190

196-
_CONNECTION_MANAGER = microgrid_api
191+
_CONNECTION_MANAGER = connection_manager
197192

198193

199194
def get() -> ConnectionManager:

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -252,11 +252,8 @@ async def _send_on_update(self, min_update_interval: timedelta) -> None:
252252
latest_calculation_result = result
253253
await sender.send(result)
254254

255-
if result is None:
256-
sleep_for = min_update_interval.total_seconds()
257-
else:
258-
# Sleep for the rest of the time.
259-
# Then we won't send update more frequently then min_update_interval
260-
time_diff = datetime.now(tz=timezone.utc) - result.timestamp
261-
sleep_for = (min_update_interval - time_diff).total_seconds()
255+
# Sleep for the rest of the time.
256+
# Then we won't send update more frequently than min_update_interval
257+
time_diff = datetime.now(tz=timezone.utc) - result.timestamp
258+
sleep_for = (min_update_interval - time_diff).total_seconds()
262259
await asyncio.sleep(sleep_for)

0 commit comments

Comments
 (0)