feat(ByFilterAssignment): add reactive power measurement attributes for static var compensator#198
Open
dbraquart wants to merge 8 commits into
Open
feat(ByFilterAssignment): add reactive power measurement attributes for static var compensator#198dbraquart wants to merge 8 commits into
dbraquart wants to merge 8 commits into
Conversation
Signed-off-by: David BRAQUART <[email protected]>
📝 WalkthroughWalkthroughAdds read/write support for reactive power measurement value and validity on StaticVarCompensator via a new field handler, centralizes measurement upsert logic in MeasurementUtils, updates modification classes to use the utility, wires FieldUtils dispatcher, and adds an integration test. ChangesStatic Var Compensator Reactive Power Measurement Support
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: David BRAQUART <[email protected]>
Signed-off-by: David BRAQUART <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/main/java/org/gridsuite/modification/modifications/AbstractInjectionModification.java (1)
63-101: 🏗️ Heavy liftRefactor
upsertMeasurementto reduce branching complexity.Line 63 introduces a method that now mixes selection, update/create branching, and reporting, which is why complexity remains high. Please split into small helpers (e.g.,
updateMeasurement,createMeasurement,appendReportIfNeeded) to bring it under the quality threshold while keeping behavior identical.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/gridsuite/modification/modifications/AbstractInjectionModification.java` around lines 63 - 101, The upsertMeasurement method mixes selection, update/create branching, and reporting; split it into small helpers to reduce complexity while preserving behavior: extract an updateMeasurement(Measurement measurement, Double value, Boolean requestedValidity, List<ReportNode> reports) that handles the value change (uses measurement.getValue(), measurement.setValue()) and validity change (uses measurement.isValid() and ModificationUtils.updateMeasurementValidity()), a createMeasurement(Measurements<?> measurements, Measurement.Type type, Double value, Boolean requestedValidity, List<ReportNode> reports) that builds the new measurement via measurements.newMeasurement().setId(...).setType(...).setValue(...).setValid(...).add(), and an appendReportIfNeeded(Object oldVal, Object newVal, String label, List<ReportNode> reports) that wraps ModificationUtils.buildModificationReport(..., TypedValue.INFO_SEVERITY) and adds to reports when non-null; then make upsertMeasurement call getExistingMeasurement(...) and delegate to updateMeasurement or createMeasurement so all report creation and branching logic is localized to the new helpers and existing public behavior/labels (measurementType + VALUE / VALIDITY) is preserved.Source: Linters/SAST tools
src/test/java/org/gridsuite/modification/modifications/byfilter/assignment/StaticVarCompensatorModificationByAssignmentTest.java (1)
78-78: ⚡ Quick winRemove unnecessary casts to
Measurements<?>.The
getExtension(Measurements.class)method returns the correct type without requiring an explicit cast (same issue as inStaticVarCompensatorField).♻️ Proposed fix
- Measurements<?> ms1 = (Measurements<?>) svc1.getExtension(Measurements.class); + Measurements<?> ms1 = svc1.getExtension(Measurements.class);Apply the same change on line 82 for
ms2.Also applies to: 82-82
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/org/gridsuite/modification/modifications/byfilter/assignment/StaticVarCompensatorModificationByAssignmentTest.java` at line 78, In StaticVarCompensatorModificationByAssignmentTest remove the unnecessary explicit casts to Measurements<?>: change the assignments where ms1 and ms2 are obtained from svc1.getExtension(Measurements.class) and svc2.getExtension(Measurements.class) to rely on the method's generic return type (i.e., assign directly to Measurements<?> ms1 and Measurements<?> ms2 without the (Measurements<?>) cast) so the variables use the correct inferred type.src/main/java/org/gridsuite/modification/dto/byfilter/equipmentfield/StaticVarCompensatorField.java (1)
28-28: ⚡ Quick winRemove unnecessary casts to
Measurements<?>.The
getExtension(Class)method already returns the extension type matching the class parameter, making the explicit cast redundant.♻️ Proposed fix
- Measurements<?> measurements = (Measurements<?>) svc.getExtension(Measurements.class); + Measurements<?> measurements = svc.getExtension(Measurements.class);Apply the same change on line 44.
Also applies to: 44-44
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/gridsuite/modification/dto/byfilter/equipmentfield/StaticVarCompensatorField.java` at line 28, Remove the redundant explicit cast to Measurements<?> when calling svc.getExtension(Measurements.class) in StaticVarCompensatorField; update the expressions using svc.getExtension(Measurements.class) to rely on the method's generic return type (i.e., drop the "(Measurements<?>)" cast) and do the same for the other identical occurrence in this class (the second svc.getExtension(Measurements.class) usage).Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/org/gridsuite/modification/dto/byfilter/equipmentfield/StaticVarCompensatorField.java`:
- Line 51: In StaticVarCompensatorField where
REACTIVE_POWER_MEASUREMENT_VALIDITY calls upsertMeasurement(...,
Measurement.Type.REACTIVE_POWER, null, Boolean.parseBoolean(newValue), null),
replace the silent Boolean.parseBoolean behavior with explicit validation: check
newValue for a case-insensitive exact "true" or "false" (e.g., equalsIgnoreCase)
and only pass the corresponding boolean to upsertMeasurement; for any other
value, throw an IllegalArgumentException (or return/report an error) so invalid
inputs are rejected rather than being treated as false. Ensure the validation is
applied at the same call site in StaticVarCompensatorField and keep the
upsertMeasurement invocation signature unchanged.
---
Nitpick comments:
In
`@src/main/java/org/gridsuite/modification/dto/byfilter/equipmentfield/StaticVarCompensatorField.java`:
- Line 28: Remove the redundant explicit cast to Measurements<?> when calling
svc.getExtension(Measurements.class) in StaticVarCompensatorField; update the
expressions using svc.getExtension(Measurements.class) to rely on the method's
generic return type (i.e., drop the "(Measurements<?>)" cast) and do the same
for the other identical occurrence in this class (the second
svc.getExtension(Measurements.class) usage).
In
`@src/main/java/org/gridsuite/modification/modifications/AbstractInjectionModification.java`:
- Around line 63-101: The upsertMeasurement method mixes selection,
update/create branching, and reporting; split it into small helpers to reduce
complexity while preserving behavior: extract an updateMeasurement(Measurement
measurement, Double value, Boolean requestedValidity, List<ReportNode> reports)
that handles the value change (uses measurement.getValue(),
measurement.setValue()) and validity change (uses measurement.isValid() and
ModificationUtils.updateMeasurementValidity()), a
createMeasurement(Measurements<?> measurements, Measurement.Type type, Double
value, Boolean requestedValidity, List<ReportNode> reports) that builds the new
measurement via
measurements.newMeasurement().setId(...).setType(...).setValue(...).setValid(...).add(),
and an appendReportIfNeeded(Object oldVal, Object newVal, String label,
List<ReportNode> reports) that wraps
ModificationUtils.buildModificationReport(..., TypedValue.INFO_SEVERITY) and
adds to reports when non-null; then make upsertMeasurement call
getExistingMeasurement(...) and delegate to updateMeasurement or
createMeasurement so all report creation and branching logic is localized to the
new helpers and existing public behavior/labels (measurementType + VALUE /
VALIDITY) is preserved.
In
`@src/test/java/org/gridsuite/modification/modifications/byfilter/assignment/StaticVarCompensatorModificationByAssignmentTest.java`:
- Line 78: In StaticVarCompensatorModificationByAssignmentTest remove the
unnecessary explicit casts to Measurements<?>: change the assignments where ms1
and ms2 are obtained from svc1.getExtension(Measurements.class) and
svc2.getExtension(Measurements.class) to rely on the method's generic return
type (i.e., assign directly to Measurements<?> ms1 and Measurements<?> ms2
without the (Measurements<?>) cast) so the variables use the correct inferred
type.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 09ab7277-abc3-4616-afdc-2638392d5f6a
📒 Files selected for processing (4)
src/main/java/org/gridsuite/modification/dto/byfilter/equipmentfield/FieldUtils.javasrc/main/java/org/gridsuite/modification/dto/byfilter/equipmentfield/StaticVarCompensatorField.javasrc/main/java/org/gridsuite/modification/modifications/AbstractInjectionModification.javasrc/test/java/org/gridsuite/modification/modifications/byfilter/assignment/StaticVarCompensatorModificationByAssignmentTest.java
benrejebmoh
requested changes
Jun 12, 2026
|
|
||
| assertThat(svc1Measurement.getValue()).isEqualTo(REACTIVE_POWER_MEASUREMENT_VALUE_FOR_SVC1); | ||
| assertThat(svc2Measurement.isValid()).isEqualTo(REACTIVE_POWER_MEASUREMENT_VALIDITY_FOR_SVC2); | ||
| } |
Contributor
There was a problem hiding this comment.
assert all 4 combinations
Contributor
Author
There was a problem hiding this comment.
yes, I realize that the test updates both SVCs in the same way
…cation Signed-off-by: benrejebmoh <[email protected]>
dbraquart
force-pushed
the
dbraquart/add-measurement-for-cspr
branch
from
a27b0d7 to
94dad9c
Compare
Signed-off-by: David BRAQUART <[email protected]>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/java/org/gridsuite/modification/utils/MeasurementUtils.java (1)
81-98: Handle validity-only upsert explicitly (NaN value vs invalid/valid constraints)In
MeasurementUtils.upsertSideMeasurement, when adding a new measurement withrequestedValue == nullandrequestedValidity != null,setValue()is not called; the resulting measurement value is expected to beNaN(e.g.,LineModificationTestassertsDouble.isNaN(...getValue())for validity-only updates).
powsyblalso rejects the case whererequestedValidity == truewhile the value is undefined on extension creation (seetestCannotCreateMeasurementsExtensionWhenMissingWithOnlyTrueValidity, message: "Valid measurement cannot have an undefined value"), so this behavior depends onrequestedValidity:
falsevalidity: allowed withNaNvaluetruevalidity: fails unless a value is providedRecommendation: document this contract in the method (and/or add a small guard/message when creating new measurements with
requestedValidity == truebutrequestedValue == null) to avoid surprise failures.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/gridsuite/modification/utils/MeasurementUtils.java` around lines 81 - 98, In MeasurementUtils.upsertSideMeasurement the new-measurement path must explicitly handle the case where requestedValue == null but requestedValidity != null: if requestedValidity is false, call measurementAdder.setValue(Double.NaN) and add the corresponding ModificationUtils.buildModificationReport for the VALUE (same severity/logFieldPrefix usage) so the system records the NaN value; if requestedValidity is true, fail fast by throwing an IllegalArgumentException (or return an error/report) with a clear message ("Valid measurement cannot have an undefined value") instead of creating an invalid measurement; update the method comment to document this contract; use the existing symbols measurementAdder, requestedValue, requestedValidity, ModificationUtils.buildModificationReport and logFieldPrefix to locate changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/main/java/org/gridsuite/modification/utils/MeasurementUtils.java`:
- Around line 81-98: In MeasurementUtils.upsertSideMeasurement the
new-measurement path must explicitly handle the case where requestedValue ==
null but requestedValidity != null: if requestedValidity is false, call
measurementAdder.setValue(Double.NaN) and add the corresponding
ModificationUtils.buildModificationReport for the VALUE (same
severity/logFieldPrefix usage) so the system records the NaN value; if
requestedValidity is true, fail fast by throwing an IllegalArgumentException (or
return an error/report) with a clear message ("Valid measurement cannot have an
undefined value") instead of creating an invalid measurement; update the method
comment to document this contract; use the existing symbols measurementAdder,
requestedValue, requestedValidity, ModificationUtils.buildModificationReport and
logFieldPrefix to locate changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: be5d3c0b-4c84-4cd1-b186-95c982589ee3
📒 Files selected for processing (4)
src/main/java/org/gridsuite/modification/modifications/AbstractBranchModification.javasrc/main/java/org/gridsuite/modification/modifications/VoltageLevelModification.javasrc/main/java/org/gridsuite/modification/utils/MeasurementUtils.javasrc/main/java/org/gridsuite/modification/utils/ModificationUtils.java
💤 Files with no reviewable changes (1)
- src/main/java/org/gridsuite/modification/utils/ModificationUtils.java
Signed-off-by: David BRAQUART <[email protected]>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Summary
Because there is currently no static var compensator modification in GridSuite, we add the possibility to change the reactive power measurement value/validity with the "Modification by filter" exisiting modification.
To update a reactive power measurement, we just call existing function used in InjectionModification class.
In this PR we also refacto a little bit to regroup all "measurement" util functions in a new file, and we now use a single "upsertMeasurement" function for all cases (injections, branches, voltage level, by-filter assignement)