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

Add parameters dedicated to Dynaflow (#118) #119

Merged
merged 6 commits into from
Oct 12, 2022
Merged

Add parameters dedicated to Dynaflow (#118) #119

merged 6 commits into from
Oct 12, 2022

Conversation

dimbdr
Copy link
Contributor

@dimbdr dimbdr commented Oct 7, 2022

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
Fixes #118

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Add new specific parameters to the Dynaflow API

What is the current behavior? (You can also link to an open issue here)
Currently, the Dynaflow API gives only 4 specific parameters.

What is the new behavior (if this is a feature change)?
Adding 4 more parameters to the Dynaflow API, for a total of 8 specific parameters.
Some were described in the PowSyBl documentation but were not available in the API. Documentation will be updated according to the current modifications.

Copy link

@sylvlecl sylvlecl left a comment

Choose a reason for hiding this comment

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

Thanks!
Implementation of loading from map still needs to be implemented :)

Note that we may implement at the same time that feature ? (but could be in another PR)
#104 (comment)

@@ -120,11 +188,14 @@ public static DynaFlowParameters load(Map<String, String> properties) {
Optional.ofNullable(properties.get(SVC_REGULATION_ON)).ifPresent(prop -> parameters.setSvcRegulationOn(Boolean.parseBoolean(prop)));
Optional.ofNullable(properties.get(SHUNT_REGULATION_ON)).ifPresent(prop -> parameters.setShuntRegulationOn(Boolean.parseBoolean(prop)));
Optional.ofNullable(properties.get(AUTOMATIC_SLACK_BUS_ON)).ifPresent(prop -> parameters.setAutomaticSlackBusOn(Boolean.parseBoolean(prop)));
Optional.ofNullable(properties.get(DSO_VOLTAGE_LEVEL)).ifPresent(prop -> parameters.setDsoVoltageLevel(Double.parseDouble(prop)));
Copy link

@sylvlecl sylvlecl Oct 10, 2022

Choose a reason for hiding this comment

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

We need to implement the loading of new parameters here too.

Maybe code could be shared with loading from the module ?

Choose a reason for hiding this comment

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

Note that we should also implement the method for update from a map, in DynaFlowProvider, not only the creation from a map.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to implement the loading of new parameters here too.

Maybe code could be shared with loading from the module ?

I made a little change, by providing a new signature called loading.
The load method is not broken, and I can use the method loading with the new implemented method updateSpecificParameters in the DynaFlowProvider.
Though, I'm not quite sure about the way I implemented this updateSpecificParameters.

Signed-off-by: BAUDRIER Dimitri <dimitri.baudrier@rte-france.com>
BAUDRIER Dimitri added 4 commits October 11, 2022 14:03
… properties. Also, fix typo on lccAsLoads.

Signed-off-by: BAUDRIER Dimitri <dimitri.baudrier@rte-france.com>
Signed-off-by: BAUDRIER Dimitri <dimitri.baudrier@rte-france.com>
…p of properties

Signed-off-by: BAUDRIER Dimitri <dimitri.baudrier@rte-france.com>
Signed-off-by: BAUDRIER Dimitri <dimitri.baudrier@rte-france.com>
Copy link

@sylvlecl sylvlecl left a comment

Choose a reason for hiding this comment

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

Ok for me, thank you for the update!
I added very minor comments, nothing blocking though

Signed-off-by: BAUDRIER Dimitri <dimitri.baudrier@rte-france.com>
@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 0 Code Smells

98.3% 98.3% Coverage
0.0% 0.0% Duplication

@sylvlecl sylvlecl merged commit b7b2fcd into main Oct 12, 2022
@sylvlecl sylvlecl deleted the issue_118 branch October 12, 2022 11:53
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.

[DynaFlow] Additional parameters
2 participants