search

openscd / open-scd

A substation configuration description editor for projects using SCL IEC 61850-6 Edition 2 or greater
https://openscd.github.io
Apache License 2.0
101 stars 38 forks source link

OpenSCD incorrectly handles namespaces in new project with import #1060

Closed danyill closed 1 year ago

danyill commented 2 years ago

Describe the bug

I did the following:

  1. Create a new project
  2. Add an icd file
  3. Save and look at the output

The output looks like:

<SCL xmlns="http://www.iec.ch/61850/2003/SCL" version="2007" revision="B">
    <Header id="project"/>
    <Communication>
        <SubNetwork name="W01">
            <ConnectedAP iedName="XAT_SEL_411L_2" apName="S1">
                <Address>
                    <P type="IP">0.0.0.0</P>
                    <P type="IP-SUBNET">255.255.255.0</P>
                    <P type="IP-GATEWAY">0.0.0.0</P>
                    <P type="OSI-TSEL">0001</P>
                    <P type="OSI-PSEL">00000001</P>
                    <P type="OSI-SSEL">0001</P>
                </Address>
                <GSE ldInst="CFG" cbName="Ctl">
                    <Address>
                        <P type="MAC-Address">01-0C-CD-01-00-00</P>
                        <P type="APPID">0000</P>
                        <P type="VLAN-PRIORITY">4</P>
                        <P type="VLAN-ID">3E8</P>
                    </Address>
                    <MinTime unit="s" multiplier="m">4</MinTime>
                    <MaxTime unit="s" multiplier="m">1000</MaxTime>
                </GSE>
            </ConnectedAP>
        </SubNetwork>
    </Communication>
    <IED desc="ICD-411L-2S-R001-V0 for firmware R200-V0 or higher" name="XAT_SEL_411L_2" type="SEL_411L_2S" manufacturer="SEL" configVersion="ICD-411L-2S-R001-V0-Z200006-D20210701" originalSclVersion="2007" originalSclRevision="B">
        <Private type="SEL_IedInfo">
            <esel:ModelNumber xmlns:esel="http://www.selinc.com/2006/61850">SEL-411L-2S</esel:ModelNumber>
            <esel:ModelVersionMin xmlns:esel="http://www.selinc.com/2006/61850">R200</esel:ModelVersionMin>
            <esel:ClassFileVersion xmlns:esel="http://www.selinc.com/2006/61850">006</esel:ClassFileVersion>
            <esel:ClassFileDescription xmlns:esel="http://www.selinc.com/2006/61850" default="false">411L-2S - Sampled Values Subscriber R200-V0 or higher (Ed. 2 support, Controllable mod/beh)</esel:ClassFileDescription>
            <esel:IcdFilePath xmlns:esel="http://www.selinc.com/2006/61850">C:\Users\tilbrookm\AppData\Local\SEL\AcSELerator\Architect\Palette\SEL-411L-2_Template.CID</esel:IcdFilePath>
            <esel:CidSentTime xmlns:esel="http://www.selinc.com/2006/61850">07/20/2022 13:35:44</esel:CidSentTime>
        </Private>

Namespace declarations appear to be incorrectly.

This is on openscd.github.

I expect the namespace to be registered on the root element and not repeated.

The initial icd file appears correct:

<?xml version="1.0" encoding="UTF-8"?>
<SCL xmlns:esel="http://www.selinc.com/2006/61850" version="2007" revision="B" xmlns="http://www.iec.ch/61850/2003/SCL">
  <Header id="XAT_Prot1" version="32" revision="1.0" toolID="AcSELerator Architect 2.3.9.1954" nameStructure="IEDName">
    <History>
      <Hitem version="32" revision="1.0" when="08/19/2022 13:51:55" who="SANC\tilbrookm" what="IED exported from Project XAT_Prot1" />
    </History>
  </Header>
  <Communication>
    <SubNetwork name="W01">
      <ConnectedAP iedName="XAT_SEL_411L_2" apName="S1">
        <Address>
          <P type="IP">0.0.0.0</P>
          <P type="IP-SUBNET">255.255.255.0</P>
          <P type="IP-GATEWAY">0.0.0.0</P>
          <P type="OSI-TSEL">0001</P>
          <P type="OSI-PSEL">00000001</P>
          <P type="OSI-SSEL">0001</P>
        </Address>
        <GSE ldInst="CFG" cbName="Ctl">
          <Address>
            <P type="MAC-Address">01-0C-CD-01-00-00</P>
            <P type="APPID">0000</P>
            <P type="VLAN-PRIORITY">4</P>
            <P type="VLAN-ID">3E8</P>
          </Address>
          <MinTime unit="s" multiplier="m">4</MinTime>
          <MaxTime unit="s" multiplier="m">1000</MaxTime>
        </GSE>
      </ConnectedAP>
    </SubNetwork>
  </Communication>
  <IED desc="ICD-411L-2S-R001-V0 for firmware R200-V0 or higher" name="XAT_SEL_411L_2" type="SEL_411L_2S" manufacturer="SEL" configVersion="ICD-411L-2S-R001-V0-Z200006-D20210701" originalSclVersion="2007" originalSclRevision="B">
    <Private type="SEL_IedInfo">
      <esel:ModelNumber>SEL-411L-2S</esel:ModelNumber>
      <esel:ModelVersionMin>R200</esel:ModelVersionMin>
      <esel:ClassFileVersion>006</esel:ClassFileVersion>
      <esel:ClassFileDescription default="false">411L-2S - Sampled Values Subscriber R200-V0 or higher (Ed. 2 support, Controllable mod/beh)</esel:ClassFileDescription>
      <esel:IcdFilePath>C:\Users\tilbrookm\AppData\Local\SEL\AcSELerator\Architect\Palette\SEL-411L-2_Template.CID</esel:IcdFilePath>
      <esel:CidSentTime>07/20/2022 13:35:44</esel:CidSentTime>

To Reproduce

XAT_SEL-411L-2.ICD.zip

Steps to reproduce the behavior:

  1. Using the attached icd file.
  2. Create a new Edition 2 project.
  3. Import the ICD file.
  4. Save the project.
  5. Open the scd file and examined the first 40 lines.
  6. Observe that namespaces are incorrectly handled.

I think this must have happened recently...

danyill commented 2 years ago

No, not recently, at least 40 commits old.

danyill commented 2 years ago

FWIW I discovered this because xmldiff barfed at me when comparing some files:

14:47 $ xmldiff XAT_Prot1_MAC_Updated_SEL_Sub.scd Test_From_ICD_SEL_NR_Initial.scd
Traceback (most recent call last):
  File "/usr/bin/xmldiff", line 33, in <module>
    sys.exit(load_entry_point('xmldiff==2.4', 'console_scripts', 'xmldiff')())
  File "/usr/lib/python3/dist-packages/xmldiff/main.py", line 116, in diff_command
    result = diff_files(args.file1, args.file2, diff_options=diff_options,
  File "/usr/lib/python3/dist-packages/xmldiff/main.py", line 50, in diff_files
    return _diff(etree.parse, left, right,
  File "/usr/lib/python3/dist-packages/xmldiff/main.py", line 37, in _diff
    right_tree = parse_method(right, parser)
  File "src/lxml/etree.pyx", line 3536, in lxml.etree.parse
  File "src/lxml/parser.pxi", line 1897, in lxml.etree._parseDocument
  File "src/lxml/parser.pxi", line 1917, in lxml.etree._parseFilelikeDocument
  File "src/lxml/parser.pxi", line 1811, in lxml.etree._parseDocFromFilelike
  File "src/lxml/parser.pxi", line 1201, in lxml.etree._BaseParser._parseDocFromFilelike
  File "src/lxml/parser.pxi", line 615, in lxml.etree._ParserContext._handleParseResultDoc
  File "src/lxml/parser.pxi", line 725, in lxml.etree._handleParseResult
  File "src/lxml/parser.pxi", line 654, in lxml.etree._raiseParseError
  File "/home/mulhollandd/Documents/open-scd-testing/Prot1_ICD_Files/Test_From_ICD_SEL_NR_Initial.scd", line 51900
lxml.etree.XMLSyntaxError: Attribute xmlns:esel redefined, line 51900, column 146
danyill commented 2 years ago

Hmmmm I think this may just be a feature of the way OpenSCD serialises the XML and may be quite valid (?). We use XMLSerializer.serializeToString and MDN suggests that non-default namespaces are added on individual elements, not the root element:

For XML serializations, Element and Attr nodes are always serialized with their namespaceURI intact. This may mean that a previously-specified prefix or default namespace may be dropped or altered.

In which case perhaps this can/should be closed.

danyill commented 2 years ago

We discussed this today, and it was suggested that if the namespaces are transferred from the imported document first to the project document and then the nodes are cloned, the namespaces should not be repeated on the imported elements.

The behaviour at the moment is "safe" because it avoids namespace conflicts by only applying the namespace at the point at which it is required (thanks @dlabordus).

I thought I had tried this but was unsuccessful with this approach, but I will provide further information soon.

We agreed the ideally the namespaces would be declared on the root node so it seems the issue remains relevant.

danyill commented 2 years ago

@dlabordus thanks for the discussion this evening (and FYI @ca-d).

I have opened a PR (draft, not for merging) in #1080.

The output from running this with the attached is something like this, which is not what we hoped would occur. Did I get my order of operations incorrect? All gentle mockery gratefully accepted :smile_cat:

My steps are:

  1. Create new project.
  2. Import from the attached ICD file.
  3. Save project.
  4. Inspect XML in editor. Arrrgh! The worst of both worlds. :angry: :shrug: 
<SCL xmlns="http://www.iec.ch/61850/2003/SCL" version="2007" revision="B" release="4" xmlns:esel="http://www.selinc.com/2006/61850">
    <Header id="project"/>
    <Communication>
        <SubNetwork name="W01">
            <ConnectedAP iedName="XAT_SEL_411L_2" apName="S1">
                <Address>
                    <P type="IP">0.0.0.0</P>
                    <P type="IP-SUBNET">255.255.255.0</P>
                    <P type="IP-GATEWAY">0.0.0.0</P>
                    <P type="OSI-TSEL">0001</P>
                    <P type="OSI-PSEL">00000001</P>
                    <P type="OSI-SSEL">0001</P>
                </Address>
                <GSE ldInst="CFG" cbName="Ctl">
                    <Address>
                        <P type="MAC-Address">01-0C-CD-01-00-00</P>
                        <P type="APPID">0000</P>
                        <P type="VLAN-PRIORITY">4</P>
                        <P type="VLAN-ID">3E8</P>
                    </Address>
                    <MinTime unit="s" multiplier="m">4</MinTime>
                    <MaxTime unit="s" multiplier="m">1000</MaxTime>
                </GSE>
            </ConnectedAP>
        </SubNetwork>
    </Communication>
    <IED desc="ICD-411L-2S-R001-V0 for firmware R200-V0 or higher" name="XAT_SEL_411L_2" type="SEL_411L_2S" manufacturer="SEL" configVersion="ICD-411L-2S-R001-V0-Z200006-D20210701" originalSclVersion="2007" originalSclRevision="B">
        <Private type="SEL_IedInfo">
            <esel:ModelNumber xmlns:esel="http://www.selinc.com/2006/61850">SEL-411L-2S</esel:ModelNumber>
            <esel:ModelVersionMin xmlns:esel="http://www.selinc.com/2006/61850">R200</esel:ModelVersionMin>
            <esel:ClassFileVersion xmlns:esel="http://www.selinc.com/2006/61850">006</esel:ClassFileVersion>

See attached:

XAT_SEL-411L-2.ICD.zip

I notice we call cloneNode and not importNode as MDN suggests and I do wonder why but perhaps that is a distraction.

ca-d commented 2 years ago

Goot point, importNode may just be the fix for this.