Add support active energy#53
Conversation
There was a problem hiding this comment.
Pull request overview
Adds broader default formula handling in microgrid component-type configuration so AC energy metrics get sensible defaults when not explicitly configured.
Changes:
- Extend
ComponentTypeConfig.__post_init__()to populate default summation formulas forAC_ENERGY_ACTIVE,AC_ENERGY_ACTIVE_CONSUMED, andAC_ENERGY_ACTIVE_DELIVERED(and keep backward-compat handling for deprecatedAC_ACTIVE_POWER). - Update release notes to document the new defaulting behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/frequenz/gridpool/_microgrid_config.py |
Adds default-formula initialization for additional AC energy metrics during config post-init. |
RELEASE_NOTES.md |
Documents the new default summation formulas for AC energy metrics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ddb36ba to
41a12cf
Compare
41a12cf to
ae7a9b8
Compare
Extend the default formula handling to also cover AC_ENERGY_ACTIVE, AC_ENERGY_ACTIVE_CONSUMED, and AC_ENERGY_ACTIVE_DELIVERED. Signed-off-by: Mohammad Tayyab <Mohammad.Tayyab@neustrom.de>
Signed-off-by: Mohammad Tayyab <Mohammad.Tayyab@neustrom.de>
Signed-off-by: Mohammad Tayyab <Mohammad.Tayyab@neustrom.de>
ae7a9b8 to
26de36e
Compare
|
|
||
| for metric in defaults: | ||
| if metric not in self.formula: | ||
| self.formula[metric] = "+".join( |
There was a problem hiding this comment.
Let's completely get rid of this, this is actually dangerous. If you prefer to break it slower can also be a separate PR.
There was a problem hiding this comment.
Wouldn't this remove the support for manual formulas then?
There was a problem hiding this comment.
This is adding a fallback so that if it has not been specified at this point it adds a default summing up the default CIDs. So this silently adds a formula. I think it's better to be explicit about this and fail in case we don't have a formula neither from the formula generator nor via config. Or do I misunderstand something?
There was a problem hiding this comment.
Oh, got it. I was thinking we might not want to fail in that case either. But yeah, it makes sense to throw an error instead of returning potentially incorrect interpreted formulas.
There was a problem hiding this comment.
I will make another PR for that then.
|
Oh that's interesting. I wonder if inverters can be used as fallback sources for energy metrics. Have you tried this already? If not, I'll do some tests. If it doesn't work, we might have to disable fallback for specific formulas. That's still part of the plan, just haven't gotten to it yet. |
|
@shsms Sure, I would merge this PR now but we can take the fallback discussion to the relevant slack channel. |
This PR adds default summation formulas for AC energy metrics in component type configuration, mirroring existing power behavior and deriving defaults from the configured component IDs.