Skip to content

Extract mixin from PV and EV component managers#1403

Open
simonvoelcker wants to merge 1 commit into
frequenz-floss:v1.x.xfrom
simonvoelcker:extract-set-power
Open

Extract mixin from PV and EV component managers#1403
simonvoelcker wants to merge 1 commit into
frequenz-floss:v1.x.xfrom
simonvoelcker:extract-set-power

Conversation

@simonvoelcker
Copy link
Copy Markdown
Contributor

PV inverter manager, EV charger manager and also battery manager have similar functionality for setting power levels in the microgrid API. These have drifted apart to different degrees, especially the battery manager implementation differs a lot. The other two were close enough to extract them into a mixin. This will be reusable when we introduce a similar manager for steam boilers.

@simonvoelcker simonvoelcker self-assigned this May 15, 2026
@github-actions github-actions Bot added the part:microgrid Affects the interactions with the microgrid label May 15, 2026
@simonvoelcker simonvoelcker added the cmd:skip-release-notes It is not necessary to update release notes for this PR label May 15, 2026
PV inverter manager, EV charger manager and also battery manager have similar functionality for setting power levels in the microgrid API. These have drifted apart to different degrees, especially the battery manager implementation differs a lot. The other two were close enough to extract them into a mixin. This will be reusable when we introduce a similar manager for steam boilers.

Signed-off-by: Simon Völcker <simon.voelcker@frequenz.com>
@simonvoelcker simonvoelcker marked this pull request as ready for review May 15, 2026 13:27
@simonvoelcker simonvoelcker requested a review from a team as a code owner May 15, 2026 13:27
@simonvoelcker simonvoelcker requested review from ela-kotulska-frequenz and removed request for a team May 15, 2026 13:27
Copy link
Copy Markdown
Contributor

@llucax llucax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I wonder if it makes more sense to just include it in ComponentManager and make BatteryManager override it if it needs something special, instead of introducing a separate mixin.

Will leave @shsms the final approval as he's more familiar with this code.

@llucax llucax requested a review from shsms May 19, 2026 07:32
@simonvoelcker
Copy link
Copy Markdown
Contributor Author

@llucax I was considering that, but the battery manager is structured very differently. The closest match to the extracted method, _set_api_power, is _set_distributed_power, and you will find that its signature already differs quite a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cmd:skip-release-notes It is not necessary to update release notes for this PR part:microgrid Affects the interactions with the microgrid

Projects

Status: To do

Development

Successfully merging this pull request may close these issues.

2 participants