Feat remove ied duplicates issue#146
Conversation
danyill
left a comment
danyill
left a comment
There was a problem hiding this comment.
Thank you for this contribution. It looks good to me but I wonder if it would be better if we didn't have an option to make the file schema invalid following ca-d comments in #121 (comment)
| )}`; | ||
| } | ||
|
|
||
| function updateIedNameToNone( |
There was a problem hiding this comment.
Should we call this something else? e.g. virtualiseIedOrRemoveLNode (too long) or ensureUniqueLNodes (doesn't quite capture it) or just updateLNodes (even if it can also remove them!).
Eek it's a bit of a mouthful but I think it's a bit misleading as is.
| return `${attr("lnInst")}|${attr("lnClass")}|${attr("ldInst")}|${attr( | ||
| "prefix", | ||
| )}`; |
There was a problem hiding this comment.
It is nit-picking but I suppose I'd prefer something a little more semantic:
return `${attr("ldInst")}|${attr("prefix")}|${attr("lnClass")}|${attr("lnInst")}`;| if (hasDuplicate) return { node: element } as Remove; | ||
| } | ||
| } | ||
| return { element, attributes: { iedName: "None" } } as SetAttributes; |
There was a problem hiding this comment.
I suppose I don't like that we have an API that can needlessly result in a schema invalid file. I think if we agree we should have only two options, "virtualise" or "remove LNodes" from the recent issue discussions then we should change this API.
How do you feel about making this small modification such that if we're not virtualising we just Remove the LNode rather than doing a SettAttributes?
| return { element, attributes: { iedName: "None" } } as SetAttributes; | |
| return { node: element } as Remove; |
I think then our options should reflect this terminology.
| * KDC, Association, ConnectedAP and IEDName | ||
| * 2. Remove subscriptions and supervisions | ||
| * 2. Update LNodes to an iedName of None | ||
| * 3. Update LNodes to an iedName of None (or remove duplicates) |
There was a problem hiding this comment.
Perhaps
| * 3. Update LNodes to an iedName of None (or remove duplicates) | |
| * 3. Update LNodes to an iedName of None (and remove any duplicates by default) |
3 participants
Resolves #121
Summary
When multiple IEDs reference the same LNode, removing those IEDs sequentially can result in duplicate LNode
entries with
iedName="None". This PR adds an option to detect and remove such duplicates instead ofcreating redundant entries, by default. Callers can use the options to prevent this happening:
Changes
tIED/removeIED.ts:RemoveIedOptionsinterface with aremoveLNodesflag (defaults totrue)lNodeKey()helper to compute a unique key per LNoderemoveLNodesistrue(default), duplicate LNodes already set to"None"are removed ratherthan duplicated
removeLNodesisfalse, LNodes are only updated to"None"(previous behaviour)index.ts: ExportedRemoveIedOptionstypetIED/removeIED.spec.ts: Added tests for duplicate removal and opt-out behaviourtIED/removeIED.testfile.ts: Added test fixture with duplicate LNode scenario.gitignore: Added IDE and build artifact patterns