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

Remote reactive power control extension #1732

Merged
merged 17 commits into from
Jun 15, 2021
Merged

Remote reactive power control extension #1732

merged 17 commits into from
Jun 15, 2021

Conversation

obrix
Copy link
Member

@obrix obrix commented May 12, 2021

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, ...)

This pull request introduce an extension to handle remote reactive power control. This is work in progress.

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

Remote reactive power control is not supported.

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

@annetill annetill changed the title [WIP] Remote reactive power control. [WIP] Remote reactive power control extension May 12, 2021
@obrix obrix force-pushed the regulation_extension branch from 4173c31 to a658f00 Compare May 21, 2021 15:16
@obrix
Copy link
Member Author

obrix commented May 21, 2021

@annetill Can you take a look on coverage for the class you modified ? (ie RegulatingControlMapping*)

@annetill annetill requested a review from miovd May 21, 2021 15:54
obrix and others added 8 commits May 31, 2021 14:03
@obrix @miovd
WIP RemoteReactivePowerControl.
acf9049 
Signed-off-by: Bertrand Rix <bertrand.rix@artelys.com>
@annetill @miovd
Add CGMES conversion.
cda8ba3 
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
@obrix @miovd
Continue implmentation of remote regulation extension, with a working…
ed39097 
… test.

Signed-off-by: Bertrand Rix <bertrand.rix@artelys.com>
@obrix @miovd
Fix typo in class name.
e99df5f 
Signed-off-by: Bertrand Rix <bertrand.rix@artelys.com>
@obrix @miovd
Add a non abstract test class for RemoteReactivePowerControlTest.
319a9bd 
Signed-off-by: Bertrand Rix <bertrand.rix@artelys.com>
@obrix @miovd
Remove wrongly named test class, fix package for the new class.
f8c2077 
Signed-off-by: Bertrand Rix <bertrand.rix@artelys.com>
@obrix @miovd
Remove useless setter on RemoteReactivePowerControl.
3298371 
Signed-off-by: Bertrand Rix <bertrand.rix@artelys.com>
@miovd
Add test
3da1f14 
Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
@miovd miovd force-pushed the regulation_extension branch from e179e40 to 3da1f14 Compare May 31, 2021 12:26
miovd
miovd approved these changes May 31, 2021
View reviewed changes
Copy link
Contributor

@miovd miovd left a comment

Choose a reason for hiding this comment

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

It is okay for me. The only thing missing is a javadoc for RemoteReactivePowerControl to explain what it is.

@miovd miovd changed the title [WIP] Remote reactive power control extension Remote reactive power control extension May 31, 2021
@obrix
Copy link
Member Author

obrix commented May 31, 2021

@MioRtia I'll add it, thanks for the review.

geofjamg
geofjamg previously requested changes May 31, 2021
View reviewed changes
Copy link
Member

@geofjamg geofjamg left a comment

Choose a reason for hiding this comment

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

Be careful, licences header are missing everywhere

annetill added 2 commits May 31, 2021 15:58
@annetill
Add headers and authors.
2f2cc5f 
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
@annetill
Add license header for tests.
1e5d797 
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
@geofjamg geofjamg dismissed their stale review May 31, 2021 14:27

why not

marqueslanauja
marqueslanauja reviewed Jun 7, 2021
View reviewed changes
Copy link
Contributor

@marqueslanauja marqueslanauja left a comment

Choose a reason for hiding this comment

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

Maybe in the method setRegulatingControlReactivePower of the class RegulatingControlMappingForGenerators we should consider the qPercent and eqControlEnabled attributes as in the voltage control (setRegulatingControlVoltage method) ?

@annetill
Copy link
Member

annetill commented Jun 7, 2021 •
edited
Loading

Maybe in the method setRegulatingControlReactivePower of the class RegulatingControlMappingForGenerators we should consider the qPercent and eqControlEnabled attributes as in the voltage control (setRegulatingControlVoltage method) ?

It means that for you, for a generator with reactive remote control extension, we will have an other extension to say that that remote reactive control is shared with an other generator? What the eqControlEnabled will introduced as change in IIDM modelling?

@annetill annetill mentioned this pull request Jun 8, 2021
Closed
5 tasks
@marqueslanauja
Copy link
Contributor

Maybe in the method setRegulatingControlReactivePower of the class RegulatingControlMappingForGenerators we should consider the qPercent and eqControlEnabled attributes as in the voltage control (setRegulatingControlVoltage method) ?

It means that for you, for a generator with reactive remote control extension, we will have an other extension to say that that remote reactive control is shared with an other generator? What the eqControlEnabled will introduced as change in IIDM modelling?

Since we record the reactive power control in an extension, we can use the same extension to record the qPercent by adding a new field.
The eqControlEnabled is used to determine if the IIDM control is enabled or not. The IIDM control is enabled if the enabled flag at the control and the enabled flag at the equipment are true.

@miovd
Consider qPercent and eqControlEnabled during reactive power control …
c5e2c1b 
…settings

Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
@miovd miovd requested a review from marqueslanauja June 15, 2021 07:37
@miovd
Copy link
Contributor

miovd commented Jun 15, 2021

@marqueslanauja Are the corrections okay for you?

marqueslanauja
marqueslanauja approved these changes Jun 15, 2021
View reviewed changes
Copy link
Contributor

@marqueslanauja marqueslanauja left a comment

Choose a reason for hiding this comment

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

It is ok.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

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

82.6% 82.6% Coverage
0.0% 0.0% Duplication

@miovd miovd merged commit 980f970 into master Jun 15, 2021
@miovd miovd deleted the regulation_extension branch June 15, 2021 09:14
miovd added a commit that referenced this pull request Jun 28, 2021
@obrix @annetill @miovd
Remote reactive power control extension (#1732)
13e5d35 
Signed-off-by: Bertrand Rix <bertrand.rix@artelys.com>

Co-authored-by: Anne Tilloy <anne.tilloy@rte-france.com>
Co-authored-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
@mathbagu mathbagu mentioned this pull request Sep 13, 2021
Closed
@annetill annetill mentioned this pull request Sep 11, 2023
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marqueslanauja marqueslanauja marqueslanauja approved these changes

@annetill annetill annetill approved these changes

@miovd miovd miovd approved these changes

@geofjamg geofjamg geofjamg left review comments

Assignees
No one assigned
Labels
CGMES Doc needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@obrix @annetill @marqueslanauja @miovd @geofjamg @zamarrenolm