-
Notifications
You must be signed in to change notification settings - Fork 39
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.
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
Add apparent power attribute (ratedS) on transformers in IIDM #1199
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After a discussion with our experts, it seems more logical to have ratedS
linked to ends of transformers for ThreeWindingsTransformer
Hence, instead of being in the ThreeWindingsTransformer
, please put the ratedS
attribute on ThreeWindingsTransformer.Leg
More general remarks:
- don't forget default implementation when adding methods in interfaces to prevent breaking changes
e.g.
default double getRatedS1() {
throw new UnsupportedOperationException();
}
- contrary to
ratedS
in generators, apparent power is optional for transformers: it can beNaN
. The only check should be to ensure it is strictly positive if notNaN
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
…gsTransformerXML Signed-off-by: Thomas ADAM <tadam@silicom.fr>
…ormer to Leg Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
@@ -24,6 +24,8 @@ | |||
|
|||
TwoWindingsTransformerAdder setRatedU2(double ratedU2); | |||
|
|||
TwoWindingsTransformerAdder setRatedS(double ratedS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add default implementation
|
||
protected static void readRatedS(String name, NetworkXmlReaderContext context, RatedSConsumer consumer) { | ||
double ratedS = XmlUtil.readOptionalDoubleAttribute(context.getReader(), name); | ||
consumer.accept(ratedS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than using a custom interface, you can use DoubleConsumer
here and delete RatedSConsumer
…oubleConsumer Signed-off-by: Thomas ADAM <tadam@silicom.fr>
@@ -137,15 +146,15 @@ public ThreeWindingsTransformerAdderImpl add() { | |||
if (legNumber == 1) { | |||
voltageLevel1 = checkAndGetVoltageLevel(); | |||
terminal1 = checkAndGetTerminal(); | |||
leg1 = new LegImpl(r, x, g, b, ratedU, legNumber); | |||
leg1 = new LegImpl(r, x, g, b, ratedU, legNumber, ratedS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should put ratedS
close to ratedU
?
public void invalidRatedS() { | ||
ThreeWindingsTransformer transformer = createThreeWindingsTransformer(); | ||
ThreeWindingsTransformer.Leg leg1 = transformer.getLeg1(); | ||
leg1.setRatedS(Double.NaN); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems useless to set ratedS to NaN.
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Kudos, SonarCloud Quality Gate passed! 0 Bugs |
Please check if the PR fulfills these requirements (please use
'[x]'
to check the checkboxes, or submit the PR and then click the checkboxes)Does this PR already have an issue describing the problem ? If so, link to this issue using
'#XXX'
and skip the restfix #1144
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Feature
Other information: