OpenEnergyTools / scl-lib

5 stars 4 forks source link

tIED: insertIED duplicates ConnectedAPs #71

Closed danyill closed 10 months ago

danyill commented 10 months ago

Currently, we duplicate ConnectedAPs during an Import under some circumstances.

https://github.com/OpenEnergyTools/scl-lib/blob/671b2fc11c7fece31a0fc22f38a05f3af78a6ffc/tIED/insertIED.ts#L9-L60

Consider the following file:

XAT_BusA_P1_Template.zip

Which I am using with: https://danyill.github.io/oscd-import-ieds/index.deploy.html or https://danyill.github.io/scl-editor/

If I start with a new project and import all IEDs in this file I get a schema validation error.

The cause is this:

danyill commented 10 months ago

Possible solution is to iterate across the SubNetworks and the ConnectedAPs separately.

danyill commented 10 months ago

Another approach could be to take the identity of each ConnectedAP and use a query/selector/find to see if it already exists in the SCL document and if it does we either replace or ignore it.

I am not sure of the best approach, but with a little direction I will be happy to prepare a PR.

danyill commented 10 months ago

Thinking about this a little more carefully, I think we should:

  1. Iterate across all matching ConnectedAPs with the iedName attribute which matches IED[name]
  2. For each ConnectedAP of these if the SubNetwork does not exist it should be created by a "deep copy" where we copy all Elements which are not ConnectedAP, thinking in particular of the tBitRateInMbPerSec but also any future additions the standard might add (WDYT? We could also do a shallow copy), I am of course thinking about the cloneNode API.
  3. The ConnectedAP should then be created by a deep copy if it does not exist (and if we are using the appropriate option).

There is a general need for some "helper" function so that any time we touch an element we can also scan for any used namespaces and ensure these are in the document header (as the standard recommends/requires) but that could be saved for another iteration.

If 1, 2 and 3 seem appropriate I shall prepare a PR.

JakobVogelsang commented 10 months ago

Sounds good. I would be particularly happy about additional test that make sure this will not happen in the future.

github-actions[bot] commented 4 days ago

:tada: This issue has been resolved in version 1.0.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: