check null values for droop and P0

[EtienneLt]

check null values for droop and P0 for hvdcAngleDroopActivePowerControl in vsc modification

[Mathieu-Deharbe]

I may be wrong, you can ask to a PO but I think that you should check the hvcdc values as well, not juste the network modification values. Explication :
HVDC creation :

A you can see I have a k and P0 value but disables angle droop.

Now modifciation of this HVDC :

As you can see I activate angle droop but didn't change the values. And they are still there.

The result is a failure even if there are all the required values :

[Mathieu-Deharbe]

I think all of this may be included directly inside checkDroopModification.

Why would you do half of the checking inside the function and the other half outside of it ?

[thangqp]

YES

[thangqp]

I may be wrong, you can ask to a PO but I think that you should check the hvcdc values as well, not juste the network modification values. Explication : HVDC creation :

A you can see I have a k and P0 value but disables angle droop.

Now modifciation of this HVDC :

As you can see I activate angle droop but didn't change the values. And they are still there.

The result is a failure even if there are all the required values :

In the new commit 315a18b, the extension is in one of two cases:
1/ If already existed, P0 is required if Droop is filled (keep the same behaviour as before)
2/ If not yet existed, in case of activating power control, both P0 and Droop are required, otherwise the check is ignored

In addition, when applying the modification, if the extension not yet existed, we should check whether the power control is activated before creating a new extension, see https://github.com/gridsuite/network-modification-server/pull/512/files#diff-73327289c738f5205af82fc66dfecb066c50a235fdc0ae36feda05cbed59d8a4R255

In creation, the same rule 2 is used, see f3f5a71

[Mathieu-Deharbe]

maybe

Suggested change

	return modificationInfos.getAngleDroopActivePowerControl() != null &&
	modificationInfos.getAngleDroopActivePowerControl() &&
	return Boolean.TRUE.equals(modificationInfos.getAngleDroopActivePowerControl())) &&

[thangqp]

DONE

[Mathieu-Deharbe]

It may be good to send an error if (modificationInfos.getDroop() != null || modificationInfos.getP0() != null) { (because the network modification has weird ignored values). But not sure that it is required, see with a P0.

[thangqp]

These modification infos will be ignored if the control is not activated (i.e. no extension created). We can show a warning but not here because the implementation of check method is for a blocking issue, in other word we throw an exception that automatically captured and then created an error report.

For the simplicity, I do not implement a warning when do not create an extension while Droop or P0 is provided.

However, I modified the check as yours above comment:

        // extension is not enabled => ignore check inside fields
        if (!Boolean.TRUE.equals(modificationInfos.getAngleDroopActivePowerControl())) {
            return;
        }

[Mathieu-Deharbe]

Suggested change

	// the extension has already existed
	// the extension already exists

[thangqp]

DONE

[Mathieu-Deharbe]

Suggested change

	// the extension has not yet existed and the modification want to enable the extension =>
	// should verify whether all fields have been filled
	boolean isEnabledAngleDroopActivePowerControl = modificationInfos.getAngleDroopActivePowerControl() != null
	&& modificationInfos.getAngleDroopActivePowerControl().getValue();
	// the extension doesn't exist yet and the modification wants to enable the extension =>
	// should verify whether all fields have been filled
	boolean isEnabledAngleDroopActivePowerControl = Boolean.TRUE.equals(modificationInfos.getAngleDroopActivePowerControl()));

[thangqp]

modificationInfos.getAngleDroopActivePowerControl() is an AttributeModification and not a Boolean, so based on yours comment, I have changed to:

        boolean isEnabledAngleDroopActivePowerControl = modificationInfos.getAngleDroopActivePowerControl() != null
            && Boolean.TRUE.equals(modificationInfos.getAngleDroopActivePowerControl().getValue());

DONE

[Mathieu-Deharbe]

ACTIVE_POWER_CONTROL_EXTENSION_MODIFY_ERROR_MESSAGE variable name doesn't really fit (this is a modification).

It is also identical to the other ACTIVE_POWER_CONTROL_EXTENSION_MODIFY_ERROR_MESSAGE in the creation file so it may use it instead of cerating a new one.

[thangqp]

YES!
I have removed the last part of message in modification which contains the id of equipment, I think we do not need the id at this level because the report of the whole modification contains already the id.

Now we can share the message between creation and modification.

    public static final String DROOP_ACTIVE_POWER_CONTROL_P0_DROOP_REQUIRED_ERROR_MSG = "Both Droop and P0 are required when angle droop active power control is activated";

[thangqp]

DONE

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
86.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud