Add wrappers for microgrid and electrical components #73
Conversation
Signed-off-by: Flora <[email protected]>
github-actions
bot
added
part:tooling
There was a problem hiding this comment.
Pull Request Overview
This PR adds common wrappers for microgrid and electrical components by extracting identifier classes and converting protobuf messages into strongly-typed Python objects. Key changes include:
- Creation of a new module for strongly-typed unique identifiers.
- Introduction of electrical components classes (including conversion functions from protobuf messages) and a dedicated connection class.
- Addition of utility functions and microgrid info conversion, along with updated dependency requirements.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/frequenz/client/common/microgrid/id.py | Adds a strongly-typed identifier framework for microgrid entities. |
| src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py | Introduces electrical components, enums, and conversion functions from protobuf. |
| src/frequenz/client/common/microgrid/electrical_components/_connection.py | Defines connection metadata between components. |
| src/frequenz/client/common/microgrid/init.py | Removes deprecated electrical component category definitions. |
| Other files | Add utility functions, location/microgrid info conversion, lifetime and delivery area handling. |
| pyproject.toml | Updates dependency versions for timezonefinder and the client base. |
| @@ -0,0 +1,35 @@ | |||
| # License: MIT | |||
| # Copyright © 2022 Frequenz Energy-as-a-Service GmbH | |||
There was a problem hiding this comment.
The copyright year in this header (2022) is inconsistent with the rest of the repository (2025). Please update it to maintain consistency.
Copilot uses AI. Check for mistakes.
| source_component_id: int | ||
| """The component ID that represents the source component of the connection.""" | ||
|
|
||
| destination_component_id: int |
There was a problem hiding this comment.
There are different types being used for component identifiers (int for source/destination IDs and ComponentId for start/end). Consider unifying these types for consistency and to avoid potential confusion.
| source_component_id: int | |
| """The component ID that represents the source component of the connection.""" | |
| destination_component_id: int | |
| source_component_id: ComponentId | |
| """The component ID that represents the source component of the connection.""" | |
| destination_component_id: ComponentId |
Copilot uses AI. Check for mistakes.
|
This is also being reviewed at: And upcoming PRs. I guess you need this more or less urgently and can't wait until it is merged first in the microgrid API? I think splitting the discussion/review in 2 different places could get problematic 😟 |
I am happy to wait until this one is through, maybe you can just give me a head up once its done and I'll change it! Just FYI @cwasicki these are the wrappers we want to use when we move all Microgrid etc classes to common. Maybe we can check if we can take in the first "inital" client as it was and then add the proper wrappers a little later. @llucax Super sorry about deleting your tests comment - somehow I completely messed up my comment, wanted to delete it and then got the wrong one :-D |
This comment was marked as outdated.
This comment was marked as outdated.
Haha, no worries. |
Then I'd say please do review the wrappers in frequenz-floss/frequenz-client-microgrid-python#94 and mention any changes you might need for other clients there, so we can already think about an approach that is good in general. Then I can see if I apply the changes there already or if I keep it as it is, but already knowing which changes are coming. After the review is done there, we can move the reviewed wrappers here already. |
A first attempt to move create wrappers for microgrid and electrical components in common rather than assets-client. Most wrappers are from Luca's microgrid client, but the electrical component & connection still needs to change a bit. Any feedback is appreciated.