equinor / iec63131

Other
8 stars 8 forks source link

Change FallbackFunction to SubstituteFunction? #91

Open AshokaK0 opened 7 months ago

AshokaK0 commented 7 months ago

Reference is made to GitHub issue #58 and conclusion from workshop held on 21.06.2023 (refer to item 12 in “MOM workshop AML Library v0.0.11” - Substitute Value and Fall back values are found across the IEC63131 standard, but it should be standardize on “Substitute”.) As decided in the workshop, change the “FallbackFunction” attribute in “In {Class: NorsokSignalClass}” to “SubstituteFunction” after the update on term “FallbackValue” to “SubstituteValue” in V0.0.12.

Status from NorsokSCDLibrary.aml V0.0.10: FallbackFunction and FallbackValue image

Status from NorsokSCDLibrary.aml V0.0.12: FallbackFunction and SubstituteValue image

Erik0x42 commented 7 months ago

There is no clear question, claim or proposed action in this issue, only presentation of a set of observations with an implied conflict, I would recommend to include at least one clear summary problem statement when raising an issue.

I believe your statement to change 'FallbackFunction' is inaccurate/incomplete, the planned change for 'FallbackFunction' was not of it's name, but of one of the legal values in the constraints collection (corresponding with the attribute name change of FallbackValue -> SubstituteValue). and this has been implemented in accordance with the decision on record (screenshot from v0.0.12):

image

code permalink: https://github.com/equinor/iec63131/blob/0.0.12/NorsokSCDLibrary.aml#L60

AshokaK0 commented 7 months ago

No questions about what was implemented in V0.0.12 with respect to the decision from the workshop. Ideally, this issue should have been a query around uniform naming. I raised issue https://github.com/equinor/iec63131/issues/58, after noticing “Substitute” in the legal values for FallbackFunction with other references (FallbackFunction, FallbackValue) being “Fallback” in V0.0.10.

Hence my query is - Should the “FallbackFunction” attribute in NorsokSignalClass also be changed to “SubstituteFunction” after the attribute name change of FallbackValue to SubstituteValue?

Erik0x42 commented 7 months ago

Ok, could you please update the present issue title to succinctly reflect your intention then? Perhaps simply: "Change FallbackFunction to SubstituteFunction?"

Note that SubstituteValue is solidly founded in IEC PAS 63131:2017. There is no equivalent underpinning in the official standard for the present broad-arching implementation of FallbackFunction (not to be mixed up with FaultAction for CA, CS, OA and QA). By "broad-arching" I mean how FallbackFunction is implemented on terminal types' super class In (instead of as a block level parameter for the MA block, which is all that is mandated by the standard, as far as I can tell).