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

Fix fork disconnection #3245

Merged
merged 10 commits into from
Dec 10, 2024
Merged

Fix fork disconnection #3245

merged 10 commits into from
Dec 10, 2024

Conversation

rolnico
Copy link
Member

@rolnico rolnico commented Dec 4, 2024

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?
Fixes #3236

What kind of change does this PR introduce?

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?

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

Other information:

Signed-off-by: Nicolas Rol <[email protected]>
@rolnico rolnico changed the title Fix fork disconnection WIP - Fix fork disconnection Dec 4, 2024
@rolnico rolnico self-assigned this Dec 9, 2024
@rolnico rolnico changed the title WIP - Fix fork disconnection Fix fork disconnection Dec 9, 2024
@rolnico rolnico marked this pull request as ready for review December 9, 2024 12:47
@rolnico rolnico requested a review from flo-dup December 9, 2024 12:53
@rolnico rolnico marked this pull request as draft December 9, 2024 12:59
@rolnico rolnico marked this pull request as ready for review December 9, 2024 13:28
@rolnico rolnico requested a review from alicecaron December 9, 2024 13:55
Copy link
Contributor

@flo-dup flo-dup left a comment

Choose a reason for hiding this comment

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

Great!

Switch breaker = network.getSwitch("B_L1_L2");

// In this case, B_L1_L2 is open
breaker.setOpen(true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add the assertFalse(line1.connect()) after?

Signed-off-by: Nicolas Rol <[email protected]>
Signed-off-by: Nicolas Rol <[email protected]>
Comment on lines +240 to +241
terminals.forEach(terminal -> assertNotNull(terminal.getBusView().getBus()));
terminals.forEach(terminal -> assertTrue(terminal.isConnected()));
Copy link
Contributor

Choose a reason for hiding this comment

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

these two lines are equivalent in node breaker topology (at least in the known implementations)

Copy link
Member Author

Choose a reason for hiding this comment

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

yes but I preferred to make it obvious and visible since it's part of the issue

Copy link
Contributor

@flo-dup flo-dup left a comment

Choose a reason for hiding this comment

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

I remove the direct link to the issue as it's only solving disconnection. The connection part might also need a change.

@flo-dup flo-dup merged commit f0482f1 into main Dec 10, 2024
8 checks passed
@flo-dup flo-dup deleted the nro/fix_disconnect_lines branch December 10, 2024 15:26
olperr1 pushed a commit that referenced this pull request Dec 11, 2024
Signed-off-by: Nicolas Rol <[email protected]>
(cherry picked from commit f0482f1)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Be able to disconnect a line from another one
4 participants