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

Voltage limit override new version #27

Merged
merged 17 commits into from
Oct 19, 2023
Merged

Conversation

p-arvy
Copy link
Member

@p-arvy p-arvy commented Oct 9, 2023

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?

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?

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

p-arvy added 2 commits October 9, 2023 14:00
Signed-off-by: parvy <pierre.arvy@artelys.com>
… voltage level has defined low/high voltage limits (taking into account voltage limit overrides)

Signed-off-by: parvy <pierre.arvy@artelys.com>
@p-arvy p-arvy requested a review from annetill October 9, 2023 15:09
@p-arvy p-arvy self-assigned this Oct 9, 2023
@p-arvy p-arvy changed the title Voltage limit override new version [WIP] Voltage limit override new version Oct 9, 2023
p-arvy added 3 commits October 9, 2023 17:36
Signed-off-by: parvy <pierre.arvy@artelys.com>
Signed-off-by: parvy <pierre.arvy@artelys.com>
@@ -165,7 +165,9 @@ public OpenReacParameters createOpenReacParameters(CommandLine line,
String voltageId = node.get("id").asText();
double lowerPercent = node.get("lower").asDouble();
double upperPercent = node.get("upper").asDouble();
openReacParameters.addSpecificVoltageLimits(Map.of(voltageId, new VoltageLimitOverride(lowerPercent, upperPercent)));
Copy link
Member

Choose a reason for hiding this comment

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

@geofjamg can we delete the tool now? If yes, I will dot that in a other PR.

p-arvy and others added 6 commits October 10, 2023 15:27
Signed-off-by: parvy <pierre.arvy@artelys.com>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: parvy <pierre.arvy@artelys.com>
Signed-off-by: parvy <pierre.arvy@artelys.com>
@p-arvy p-arvy changed the title [WIP] Voltage limit override new version Voltage limit override new version Oct 11, 2023
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
String voltageLevelId = null;
VoltageLimitOverride.VoltageLimitType type = null;
boolean isRelative = true;
double overrideValue = 0;
Copy link
Member

Choose a reason for hiding this comment

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

You might want to initialize to Double.NaN and then the check that the value is in the file will be done when calling the constructor.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but @p-arvy value must be defined ad so checked at construction.

break;
case "isRelative":
parser.nextToken();
isRelative = parser.readValueAs(boolean.class);
Copy link
Member

Choose a reason for hiding this comment

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

Use Boolean.class.

Copy link
Member

Choose a reason for hiding this comment

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

ok

double deltaHighVoltageLimit = 0;
String voltageLevelId = null;
VoltageLimitOverride.VoltageLimitType type = null;
boolean isRelative = true;
Copy link
Member

Choose a reason for hiding this comment

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

You might also want to initialize this boolean to null, for that you can use Boolean instead. Or do you want isRelative to have a default value to true if the field is not in the file?
Then you can add a check that isRelative is well defined and if not throw an exception because it's undefined.

Copy link
Member

Choose a reason for hiding this comment

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

ok great, you are right.

p-arvy and others added 5 commits October 18, 2023 09:56
Signed-off-by: parvy <pierre.arvy@artelys.com>
…ater than high limit.

Signed-off-by: parvy <pierre.arvy@artelys.com>
Signed-off-by: parvy <pierre.arvy@artelys.com>
Signed-off-by: parvy <pierre.arvy@artelys.com>
Signed-off-by: Anne Tilloy <anne.tilloy@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

84.7% 84.7% Coverage
0.0% 0.0% Duplication

@annetill annetill merged commit 6dd4d4c into main Oct 19, 2023
7 checks passed
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