Skip to content

powsybl_4.0.1 #183

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
Merged
merged 4 commits into from
Apr 15, 2021
Merged

powsybl_4.0.1 #183

merged 4 commits into from
Apr 15, 2021

Conversation

AbdelHedhili
Copy link
Member

Signed-off-by: AbdelHedhili [email protected]

Please check if the PR fulfills these requirements (please use '[x]' to check the checkboxes, or submit the PR and then click the checkboxes)

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
feature

What is the current behavior? (You can also link to an open issue here)
uses powsybl 3.8.0

What is the new behavior (if this is a feature change)?
uses powsybl 4.0.1

@AbdelHedhili
powsybl_4.0.1
5b131cd 
Signed-off-by: AbdelHedhili <[email protected]>
@AbdelHedhili AbdelHedhili requested a review from jonenst April 8, 2021 18:18
@AbdelHedhili AbdelHedhili self-assigned this Apr 8, 2021
@AbdelHedhili AbdelHedhili force-pushed the powsybl_4.0.1 branch 2 times, most recently from a88ca58 to bed3dd1 Compare April 9, 2021 14:17
@AbdelHedhili
fixes and add tests
26b24cb 
Signed-off-by: AbdelHedhili <[email protected]>
jonenst
jonenst reviewed Apr 9, 2021
View reviewed changes
...-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/ActivePowerLimitsAdderImpl.java
}

private void checkTemporaryLimits() {
// check temporary limits are ok and are consistents with permanent
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this copy pasted from powsybl-core ? Can we avoid it (maybe in another PR ?)

Copy link
Member Author

Choose a reason for hiding this comment

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

It's from powsybl-core indeed but why should I remove it, it makes sense to have the same check no ?

jonenst
jonenst reviewed Apr 9, 2021
View reviewed changes
...-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/ActivePowerLimitsAdderImpl.java
.temporaryLimits(temporaryLimits)
.build();
owner.setActivePowerLimits(side, attributes);
return new ActivePowerLimitsImpl(owner, attributes);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This object should also be the same as the one returned from the owner

jonenst
jonenst reviewed Apr 9, 2021
View reviewed changes
...idm-impl/src/main/java/com/powsybl/network/store/iidm/impl/ApparentPowerLimitsAdderImpl.java
}

private void checkTemporaryLimits() {
// check temporary limits are ok and are consistents with permanent
Copy link
Collaborator

Choose a reason for hiding this comment

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

same remark

jonenst
jonenst reviewed Apr 9, 2021
View reviewed changes
network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/BatteryImpl.java
public <E extends Extension<Battery>> E getExtension(Class<? super E> type) {
E extension = super.getExtension(type);
if (type == ActivePowerControl.class) {
extension = createActivePowerControlExtension();
Copy link
Collaborator

Choose a reason for hiding this comment

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

is it ok to create everytime ? Also the extesnion is already assigned in createActivePowerControlExtension, only if not null... why the two places ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't get the problem here. What do you mean with the 2 places ?

jonenst
jonenst reviewed Apr 9, 2021
View reviewed changes
network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/BatteryImpl.java
public <E extends Extension<Battery>> E getExtensionByName(String name) {
E extension = super.getExtensionByName(name);
if (name.equals("activePowerControl")) {
extension = createActivePowerControlExtension();
Copy link
Collaborator

Choose a reason for hiding this comment

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

same remark

jonenst
jonenst reviewed Apr 9, 2021
View reviewed changes
network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/BatteryImpl.java
@Override
public <E extends Extension<Battery>> Collection<E> getExtensions() {
Collection<E> extensions = super.getExtensions();
E extension = createActivePowerControlExtension();
Copy link
Collaborator

Choose a reason for hiding this comment

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

same remark

jonenst
jonenst reviewed Apr 9, 2021
View reviewed changes
network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/DanglingLineImpl.java
@Override
public CurrentLimitsAdder newCurrentLimits() {
return new CurrentLimitsAdderImpl<>(null, this);
}

@Override
public ApparentPowerLimitsAdder newApparentPowerLimits() {
return new ApparentPowerLimitsAdderImpl<>(null, this);
Copy link
Collaborator

Choose a reason for hiding this comment

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

same remark

jonenst
jonenst reviewed Apr 9, 2021
View reviewed changes
network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/DanglingLineImpl.java

@Override
public ActivePowerLimitsAdder newActivePowerLimits() {
return new ActivePowerLimitsAdderImpl<>(null, this);
Copy link
Collaborator

Choose a reason for hiding this comment

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

same remark.. I think I'm going to stop here for these kind of remarks ^^

@AbdelHedhili
review fixes
8d971f9 
Signed-off-by: AbdelHedhili <[email protected]>
@geofjamg
Copy link
Member

@AbdelHedhili there is missing licence and authorship, could you add it?

@AbdelHedhili AbdelHedhili force-pushed the powsybl_4.0.1 branch 2 times, most recently from 3355f90 to 2080732 Compare April 14, 2021 09:41
@AbdelHedhili
review fixes
47afdb2 
Signed-off-by: AbdelHedhili <[email protected]>
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 47 Code Smells

81.8% 81.8% Coverage
9.3% 9.3% Duplication

@AbdelHedhili AbdelHedhili requested a review from geofjamg April 14, 2021 10:52
@jonenst jonenst merged commit 37f00d9 into master Apr 15, 2021
@jonenst jonenst deleted the powsybl_4.0.1 branch April 15, 2021 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonenst jonenst jonenst approved these changes

@geofjamg geofjamg geofjamg approved these changes

Assignees

@AbdelHedhili AbdelHedhili

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AbdelHedhili @geofjamg @jonenst