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

Add network modification to action #2924

Merged
merged 16 commits into from
May 23, 2024
Merged

Add network modification to action #2924

merged 16 commits into from
May 23, 2024

Conversation

pjeanmarie
Copy link
Member

@pjeanmarie pjeanmarie commented Mar 11, 2024 •
edited by olperr1
Loading

Please check if the PR fulfills these requirements

  • 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)

Does this PR already have an issue describing the problem?

powsybl/powsybl-open-rao#880

What kind of change does this PR introduce?

Feature (first user will be open-rao)

What is the current behavior?

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

  • link network modifications to actions (create them if necessary)
  • add equals and hascode to action

Does this PR introduce a breaking change or deprecate an API?

  • Yes
  • No

If yes, please check if the following requirements are fulfilled

  • The Breaking Change or Deprecated label has been added
  • The migration steps are described in the following section

What changes might users need to make in their application due to this PR? (migration steps)

Custom IIDM implementation maintainers should define these new methods for Connectable implementations:

  • boolean connect(Predicate<Switch> isTypeSwitchToOperate, ThreeSides side);
  • boolean disconnect(Predicate<Switch> isSwitchOpenable, ThreeSides side)

They should act as their counterparts without the side parameter, but only on the specified side.

Also, for all users, the change of equals/hashcode could be considered as a breaking change but it will probably impact nobody.

Other information:

@pjeanmarie pjeanmarie requested a review from obrix March 11, 2024 13:51
@pjeanmarie pjeanmarie changed the title Add network modification to action WIP: Add network modification to action Mar 11, 2024
@pjeanmarie pjeanmarie mentioned this pull request Mar 11, 2024
Closed
@pjeanmarie pjeanmarie self-assigned this Mar 11, 2024
@pjeanmarie pjeanmarie force-pushed the add_network_modification_to_action branch 2 times, most recently from d333bca to abd3e88 Compare March 12, 2024 16:03
@annetill annetill changed the base branch from main to action_modules_rework March 13, 2024 10:04
@pjeanmarie pjeanmarie force-pushed the add_network_modification_to_action branch from abd3e88 to 22e088b Compare March 18, 2024 14:09
Base automatically changed from action_modules_rework to main March 18, 2024 15:16
@pjeanmarie pjeanmarie force-pushed the add_network_modification_to_action branch from 22e088b to 5ea2dc4 Compare March 19, 2024 10:13
@pjeanmarie pjeanmarie changed the title WIP: Add network modification to action Add network modification to action Mar 20, 2024
@pjeanmarie pjeanmarie force-pushed the add_network_modification_to_action branch from 5ea2dc4 to e0425e9 Compare March 20, 2024 09:25
@pjeanmarie pjeanmarie force-pushed the add_network_modification_to_action branch 2 times, most recently from c0362db to 075684f Compare April 4, 2024 16:10
@pjeanmarie pjeanmarie force-pushed the add_network_modification_to_action branch from 075684f to 5795638 Compare April 10, 2024 09:00
@annetill annetill requested a review from olperr1 May 7, 2024 07:29
@pjeanmarie pjeanmarie force-pushed the add_network_modification_to_action branch from 238e310 to 1554e86 Compare May 7, 2024 08:50
olperr1
olperr1 requested changes May 7, 2024
View reviewed changes
action-api/src/main/java/com/powsybl/action/MultipleActionsAction.java Outdated Show resolved Hide resolved
action-api/src/main/java/com/powsybl/action/AbstractLoadAction.java Outdated Show resolved Hide resolved
action-api/pom.xml Outdated Show resolved Hide resolved
action-api/src/test/java/com/powsybl/action/json/ApplyActionToNetworkTest.java Outdated Show resolved Hide resolved
.../iidm-modification/src/main/java/com/powsybl/iidm/modification/AbstractLoadModification.java Show resolved Hide resolved
action-api/src/main/java/com/powsybl/action/AbstractLoadAction.java Outdated Show resolved Hide resolved
action-api/src/main/java/com/powsybl/action/GeneratorAction.java Outdated Show resolved Hide resolved
action-api/src/main/java/com/powsybl/action/PhaseTapChangerTapPositionAction.java Outdated Show resolved Hide resolved
iidm/iidm-modification/src/main/java/com/powsybl/iidm/modification/GeneratorModification.java Outdated Show resolved Hide resolved
iidm/iidm-modification/src/main/java/com/powsybl/iidm/modification/GeneratorModification.java Outdated Show resolved Hide resolved
@pjeanmarie
Copy link
Member Author

Sonar raise coverage error mostly because of IDE generated equals and hashcode that I added for Rao (where they are tested) (where they are used by Set). Can we used some kind of automatic tests for these like https://mvnrepository.com/artifact/nl.jqno.equalsverifier/equalsverifier ?

@pjeanmarie pjeanmarie force-pushed the add_network_modification_to_action branch 2 times, most recently from 4d0a0ff to fd32cf9 Compare May 7, 2024 16:36
@olperr1
Copy link
Member

olperr1 commented May 13, 2024

Sonar raise coverage error mostly because of IDE generated equals and hashcode that I added for Rao (where they are tested) (where they are used by Set). Can we used some kind of automatic tests for these like https://mvnrepository.com/artifact/nl.jqno.equalsverifier/equalsverifier ?

EqualsTester from guava-testlib is already used in powsybl-core to test the equals() methods. You can find several examples in the code.
I think it would be better to use it instead. equalsverifier seems to be mostly maintained by one developer, which could be a problem.

@pjeanmarie pjeanmarie force-pushed the add_network_modification_to_action branch 4 times, most recently from 304e179 to b24a49c Compare May 13, 2024 15:28
Pauline Jean-Marie added 2 commits May 14, 2024 17:26
Add boolean to be able to remove the fact that when we change target …
459d3dd 
…P via a generator modification it eventually changes the target P if it is not within boundaries and it connects the terminal if disconnected

Signed-off-by: Pauline Jean-Marie <pauline.jean-marie@artelys.com>
Fix (the other builder return the specific type)
500f1ba 
Signed-off-by: Pauline Jean-Marie <pauline.jean-marie@artelys.com>
@pjeanmarie pjeanmarie force-pushed the add_network_modification_to_action branch from 26ea3b3 to efda0f8 Compare May 14, 2024 15:26
olperr1
olperr1 requested changes May 15, 2024
View reviewed changes
action-api/src/main/java/com/powsybl/action/GeneratorAction.java Outdated Show resolved Hide resolved
action-api/src/main/java/com/powsybl/action/HvdcAction.java Outdated Show resolved Hide resolved
action-api/src/main/java/com/powsybl/action/StaticVarCompensatorAction.java Outdated Show resolved Hide resolved
action-api/src/main/java/com/powsybl/action/AbstractAction.java Show resolved Hide resolved
action-api/src/test/java/com/powsybl/action/EqualsActionTest.java Show resolved Hide resolved
action-api/src/test/java/com/powsybl/action/EqualsActionTest.java Show resolved Hide resolved
action-api/src/test/java/com/powsybl/action/EqualsActionTest.java Show resolved Hide resolved
action-api/src/test/java/com/powsybl/action/EqualsActionTest.java Show resolved Hide resolved
action-api/src/test/java/com/powsybl/action/EqualsActionTest.java Show resolved Hide resolved
action-api/src/test/java/com/powsybl/action/EqualsActionTest.java Show resolved Hide resolved
Add equals and hashcode on all Action (Rao use some of them in a Set)
e01d3a7 
Signed-off-by: Pauline Jean-Marie <pauline.jean-marie@artelys.com>
@pjeanmarie pjeanmarie force-pushed the add_network_modification_to_action branch from efda0f8 to e01d3a7 Compare May 21, 2024 14:16
@annetill annetill requested a review from rolnico May 22, 2024 06:38
annetill and others added 7 commits May 22, 2024 12:54
@annetill
Remove terminal connection modification.
b645cc2 
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
@annetill
Add unit test.
2fec004 
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
@rolnico
replace Optional<ThreeSides> with ThreeSides
2913574 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@rolnico
remove unused imports
f62ab32 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@rolnico
fix test
94ca3b2 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@rolnico
remove import
592be20 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@rolnico
simplify TerminalsConnectionAction.toModification()
508e6c7 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@olperr1 olperr1 added the Breaking Change API is broken label May 23, 2024
@rolnico
fix message in reportnode
190dab6 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@rolnico
fix message in log
cf829aa 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented May 23, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
90.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@olperr1 olperr1 merged commit ed1a123 into main May 23, 2024
6 checks passed
@olperr1 olperr1 deleted the add_network_modification_to_action branch May 23, 2024 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@annetill annetill annetill left review comments

@olperr1 olperr1 olperr1 approved these changes

@rolnico rolnico rolnico approved these changes

@obrix obrix Awaiting requested review from obrix

Assignees

@pjeanmarie pjeanmarie

Labels
Breaking Change API is broken June release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pjeanmarie @olperr1 @annetill @rolnico