Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Power distributor's PartialFailure messages report full power as succeeded #751

Closed
shsms opened this issue Oct 30, 2023 · 0 comments · Fixed by #763
Closed

Power distributor's PartialFailure messages report full power as succeeded #751

shsms opened this issue Oct 30, 2023 · 0 comments · Fixed by #763
Assignees
@daniel-zullo-frequenz
Labels
part:power-management Affects the management of battery power and distribution type:bug Something isn't working
Milestone
v1.0.0-rc3

Comments

@shsms
Copy link
Contributor

shsms commented Oct 30, 2023

What happened?

The succeeded power in PartialFailure messages should exclude any failed power.

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

Line 353 in 862ebe4

succeeded_power=Power.from_watts(distributed_power_value),

This should likely be enough to fix it:

-    succeeded_power=Power.from_watts(distributed_power_value),
+    succeeded_power=Power.from_watts(distributed_power_value - failed_power),

Probably also needs adding/updating corresponding test cases.

What did you expect instead?

Report only the power value that got accepted by the microgrid API.

Affected version(s)

No response

Affected part(s)

Battery power distribution (part:power-distribution)

Extra information

No response
@shsms shsms added type:bug Something isn't working part:power-management Affects the management of battery power and distribution labels Oct 30, 2023
@shsms shsms added this to the v1.0.0-rc3 milestone Oct 30, 2023
@daniel-zullo-frequenz daniel-zullo-frequenz moved this from To do to In progress in Python SDK Roadmap Oct 31, 2023
Merged
@daniel-zullo-frequenz daniel-zullo-frequenz moved this from In progress to Review in progress in Python SDK Roadmap Nov 2, 2023
github-merge-queue bot pushed a commit that referenced this issue Nov 3, 2023
@daniel-zullo-frequenz
Fix power failure report (#763)
5cd8d9a 
The succeeded power in `PartialFailure` result should exclude any failed
power.

There was no unit tests covering `PartialFailure` results so this patch
adds a test to cover the case where the microgrid, for any reason, fails
to set power for one of the batteries in the power request.

Fixes #751
@github-project-automation github-project-automation bot moved this from Review in progress to Done in Python SDK Roadmap Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@daniel-zullo-frequenz daniel-zullo-frequenz

Labels
part:power-management Affects the management of battery power and distribution type:bug Something isn't working
Projects
Python SDK Roadmap
Status: Done
Milestone
v1.0.0-rc3
Development

Successfully merging a pull request may close this issue.

Fix power failure report daniel-zullo-frequenz/frequenz-sdk-python
2 participants
@shsms @daniel-zullo-frequenz