-
Notifications
You must be signed in to change notification settings - Fork 0
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
Refactor default model factory #311
Conversation
Use the default model factory for EquipmentConnectionPoint Use MacroConnectionsAdder instead of AbstractBlackBoxModel methods for macro connections creation Signed-off-by: lisrte <[email protected]>
Signed-off-by: Lisrte <[email protected]>
Signed-off-by: lisrte <[email protected]>
Signed-off-by: lisrte <[email protected]>
Signed-off-by: lisrte <[email protected]>
Signed-off-by: lisrte <[email protected]>
Signed-off-by: lisrte <[email protected]>
Signed-off-by: lisrte <[email protected]>
Signed-off-by: Lisrte <[email protected]>
Signed-off-by: lisrte <[email protected]>
Signed-off-by: lisrte <[email protected]>
Signed-off-by: Florian Dupuy <[email protected]>
Signed-off-by: Florian Dupuy <[email protected]>
Signed-off-by: Florian Dupuy <[email protected]>
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.
Nice refactor! just a few comments
dynawaltz/src/main/java/com/powsybl/dynawaltz/DynaWaltzContext.java
Outdated
Show resolved
Hide resolved
...z/src/main/java/com/powsybl/dynawaltz/models/defaultmodels/DefaultModelFactoryInterface.java
Show resolved
Hide resolved
dynawaltz/src/main/java/com/powsybl/dynawaltz/models/buses/DefaultEquipmentConnectionPoint.java
Outdated
Show resolved
Hide resolved
|
||
private static final String EVENT_PREFIX = "Disconnect_"; | ||
private static final String DYNAMIC_MODEL_LIB = "EventSetPointBoolean"; | ||
private static final String DEFAULT_MODEL_LIB = "EventConnectedStatus"; | ||
protected static final String DISCONNECTION_VAR_CONNECT = "event_state1"; | ||
|
||
private final boolean disconnect; | ||
private final LateInitField<Boolean> equipmentHasDynamicModel = new LateInitField<>(); |
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.
Why didn't you use a Boolean
? Maybe I'm missing something, but I don't see what LateInitField has more
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.
Ok, so you want to emphasize the late initialized fields.
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.
Changed LateInitField so that it is final.
*/ | ||
public interface ContextDependentEvent { | ||
|
||
Identifiable<?> getEquipment(); |
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.
just realizing we could put getEquipment
in Event
also.
Shouldn't we put it in Event
and extends Event
here?
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.
Ko, EventModel is not in powsybl-dynawo. We might want to add getEquipment
in powsybl-dynamic-simulation API
...ain/java/com/powsybl/dynawaltz/models/frequencysynchronizers/FrequencySynchronizedModel.java
Outdated
Show resolved
Hide resolved
@Override | ||
public Bus getConnectesBus() { | ||
return BusUtils.getConnectableBus(equipment); | ||
} |
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.
We could put this in AbstractEquipmentBlackBoxModel
don't you think?
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.
Ko, equipment is an Identifiable there, not a Injection
Signed-off-by: Florian Dupuy <[email protected]>
Signed-off-by: Florian Dupuy <[email protected]>
Signed-off-by: Florian Dupuy <[email protected]>
Quality Gate passedThe SonarCloud Quality Gate passed, but some issues were introduced. 1 New issue |
Please check if the PR fulfills these requirements
Does this PR already have an issue describing the problem?
No
What kind of change does this PR introduce?
Refactor
What is the current behavior?
What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change or deprecate an API?
Other information: