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.

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

Do not allow switches with the same node or bus at both ends. #1991

Merged

Conversation

marqueslanauja
Copy link
Contributor

@marqueslanauja marqueslanauja commented Mar 1, 2022

Signed-off-by: José Antonio Marqués marquesja@aia.es

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)

Does this PR already have an issue describing the problem ? If so, link to this issue using '#XXX' and skip the rest

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)
Switches with the same node or bus at both ends are allowed.

What is the new behavior (if this is a feature change)?
An exception is thrown when a switch with the same node or bus at both ends is added.

Does this PR introduce a breaking change or deprecate an API? If yes, check the following:

  • The Breaking Change or Deprecated label has been added
  • The migration guide has been updated in the github wiki (What changes might users need to make in their application due to this PR?)

Other information:

(if any of the questions/checkboxes don't apply, please delete them entirely)

marqueslanauja and others added 4 commits March 1, 2022 12:31
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
@zamarrenolm zamarrenolm marked this pull request as ready for review March 2, 2022 10:20
@annetill annetill requested a review from flo-dup March 4, 2022 14:29
@miovd
Copy link
Contributor

miovd commented Mar 4, 2022

Could you please add something clever (no idea right now) to handle compatibility with old XIIDM files that can allow this configuration?

… same node or bus.

Signed-off-by: José Antonio Marqués <marquesja@aia.es>
@marqueslanauja
Copy link
Contributor Author

marqueslanauja commented Mar 9, 2022

We decided to handle compatibility with old XIIDM files by discarding switches with the same node or bus. This verification is always done independently of the XIIDM version.
Another alternative is to add a new attribute in the SwitchAdder to configure the check, throw an exception (by default) or discard the switch (when importing XIIDM files). We selected the first alternative to avoid adding new attributes in the adder.

marqueslanauja and others added 3 commits March 9, 2022 10:48
…discard switches with same ends during writing (for example for calculated views)

Signed-off-by: VEDELAGO MIORA <miora.ralambotiana@rte-france.com>
@miovd
Copy link
Contributor

miovd commented Mar 10, 2022

Hello @marqueslanauja @zamarrenolm I made some modifications to add to your work:

  • check that the identifiable is not null before reading sub-elements to prevent NPE
  • discard switches with same ends during writing (for example while writing a calculated view) to allow round trip

Do you agree? If yes, it is good for me!

Signed-off-by: VEDELAGO MIORA <miora.ralambotiana@rte-france.com>
@marqueslanauja
Copy link
Contributor Author

Hello @marqueslanauja @zamarrenolm I made some modifications to add to your work:

  • check that the identifiable is not null before reading sub-elements to prevent NPE
  • discard switches with same ends during writing (for example while writing a calculated view) to allow round trip

Do you agree? If yes, it is good for me!

It is ok for me. Thanks.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

81.4% 81.4% Coverage
0.0% 0.0% Duplication

@miovd miovd merged commit f5c3c7a into main Mar 10, 2022
@miovd miovd deleted the iidm_do_not_allow_switches_with_both_ends_in_the_same_node_or_bus branch March 10, 2022 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants