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

adjusting regulating when modifying phase tap changer #537

Merged
merged 19 commits into from
Oct 25, 2024

Conversation

EtienneLt
Copy link
Contributor

No description provided.

@EtienneLt EtienneLt self-assigned this Sep 24, 2024
@Mathieu-Deharbe Mathieu-Deharbe self-requested a review September 26, 2024 08:35
Copy link
Contributor

@Mathieu-Deharbe Mathieu-Deharbe left a comment

Choose a reason for hiding this comment

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

Doesn't compile anymore because of the recent merge of my refacto (that you reviewed) : #524

Also can you please comment the ticket with your answers to the questions asked by the PO at the end of the orignal ticket ? I cannot understand if the issue is solved just with your code.

@EtienneLt EtienneLt force-pushed the fix-regulating-phaseTapChanger branch from 6cad24e to 6db2232 Compare October 1, 2024 11:13
Copy link
Contributor

@Mathieu-Deharbe Mathieu-Deharbe left a comment

Choose a reason for hiding this comment

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

Small remark : I wonder how this "regulating" value is handled in this network modification now that we don't automatically set it to true. I didn't find how to modify it in the current interface.

Might be good to see this with a PO.

@thangqp
Copy link
Contributor

thangqp commented Oct 3, 2024

Small remark : I wonder how this "regulating" value is handled in this network modification now that we don't automatically set it to true. I didn't find how to modify it in the current interface.

Might be good to see this with a PO.

Share the same question???? In my opinion, we should have a PR for the frontend to compute isRegulating in certain case. It seems that isRegulating in TapChangerModificationInfos is never passed from the front-end

@EtienneLt EtienneLt force-pushed the fix-regulating-phaseTapChanger branch from 03ca6f8 to 5034152 Compare October 7, 2024 07:43
@EtienneLt EtienneLt requested a review from thangqp October 9, 2024 13:14
@EtienneLt EtienneLt requested a review from thangqp October 10, 2024 08:21
Copy link
Contributor

@Mathieu-Deharbe Mathieu-Deharbe left a comment

Choose a reason for hiding this comment

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

I wrote a lot about the regulation mode. If this is not clear just ask me.

@thangqp thangqp self-requested a review October 14, 2024 06:59
@thangqp thangqp closed this Oct 18, 2024
@thangqp thangqp deleted the fix-regulating-phaseTapChanger branch October 18, 2024 14:14
@thangqp thangqp restored the fix-regulating-phaseTapChanger branch October 18, 2024 14:15
@thangqp thangqp reopened this Oct 18, 2024
Copy link
Contributor

@thangqp thangqp left a comment

Choose a reason for hiding this comment

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

Could you add an additional test method for the creation of extension, pls? with name e.g. testPhaseTapChangerRegulationCreation(), I see in the code coverage, it does not yet cover some required modifications, e.g https://sonarcloud.io/code?id=org.gridsuite%3Anetwork-modification-server&pullRequest=537&selected=org.gridsuite%3Anetwork-modification-server%3Asrc%2Fmain%2Fjava%2Forg%2Fgridsuite%2Fmodification%2Fserver%2Fmodifications%2FTwoWindingsTransformerModification.java&line=293

Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
EtienneLt and others added 7 commits October 24, 2024 10:43
Signed-off-by: Etienne LESOT <[email protected]>
* Revert mistake in previous commit

* fix regulating when modifying phase tap changer (renamming proposition )
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
@EtienneLt EtienneLt force-pushed the fix-regulating-phaseTapChanger branch from decb2a4 to 2684fce Compare October 24, 2024 08:46
Signed-off-by: Etienne LESOT <[email protected]>
@thangqp thangqp self-requested a review October 24, 2024 10:20
Copy link
Contributor

@thangqp thangqp left a comment

Choose a reason for hiding this comment

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

Test OK, Code OK

@EtienneLt EtienneLt changed the title fix regulating when modifying phase tap changer adjusting regulating when modifying phase tap changer Oct 25, 2024
Signed-off-by: Etienne LESOT <[email protected]>
Copy link

@EtienneLt EtienneLt merged commit c2ece3e into main Oct 25, 2024
3 checks passed
@EtienneLt EtienneLt deleted the fix-regulating-phaseTapChanger branch October 25, 2024 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants