FreeOpcUa / opcua-modeler

GUI to create OPC UA models and export them as XML
GNU General Public License v3.0
243 stars 89 forks source link

Multiple issues re-exporting xml #9

Open Pro opened 8 years ago

Pro commented 8 years ago

I loaded the "Devices Information Model" (from https://opcfoundation.org/UA/schemas/DI/1.00/Opc.Ua.Di.NodeSet2.xml) into the modeler and then just hit "Save As" (without any modification).

What I expected was to get the same XML back.

But unfortunately there are some issues here:

oroulet commented 8 years ago

In fact I am impressed by the short list of errors! I expected more.

Thank you for the tips about the "Devices Information Model". Maybe we could use it in tests for export/import

Great that someone using another stack is interested in this project. In fact it might be possible to use open62541 internally instead of python-opcua. But will probably require some work on open62541

I will have a look at the bugs when I get 2 minutes

Pro commented 8 years ago

I'm also a contributing member of the open62541 project, so if needed I can provide some help on the adaption. As far as I know open62541 doesn't has any XML exporter, so I don't know if it really makes sense to use it instead of python-opcua.

In fact it would also make sense to set up some unit testing and continuous integration (as we did in open62541). There you could then check if the XML output is the same.

Pro commented 8 years ago

Oh, and another issue:

Pro commented 8 years ago
    <Alias Alias="HasModellingRule">i=11508</Alias>
    <Alias Alias="HasProperty">ns=2;i=1003</Alias>
    <Alias Alias="HasTypeDefinition">ns=2;i=6029</Alias>
    <Alias Alias="HasComponent">ns=2;i=1005</Alias>
    <Alias Alias="HasSubtype">ns=2;i=1001</Alias>
    <Alias Alias="Organizes">ns=2;i=1005</Alias>

compared to the correct IDs:

<Alias Alias="HasEventSource">i=36</Alias>
    <Alias Alias="HasNotifier">i=48</Alias>
    <Alias Alias="HasSubtype">i=45</Alias>
    <Alias Alias="HasTypeDefinition">i=40</Alias>
    <Alias Alias="HasModellingRule">i=37</Alias>
    <Alias Alias="HasEncoding">i=38</Alias>
    <Alias Alias="HasDescription">i=39</Alias>
oroulet commented 8 years ago

In fact it would also make sense to set up some unit testing and continuous integration

xml import/export code is in python-opcua which has a lot of testing code. But the XML stuff is new so its is missing many tests. opcua-modeller is almost an empty shell.

I know open62541 doesn't has any XML exporter,

Yes this is a requirement :-) we can have a look when someone has implemented an exporter.

Help is welcome to fix these things, should not be difficult. As far as I could see all bugs are to be fixed in one file: https://github.com/FreeOpcUa/python-opcua/blob/master/opcua/common/xmlexporter.py

oroulet commented 8 years ago

https://github.com/FreeOpcUa/python-opcua/pull/302

oroulet commented 8 years ago

alias Ids were corrects in my tests....

Pro commented 8 years ago

Ok so it may have be related to an older version

oroulet commented 8 years ago

Do not think so. Must happen in some conditions..

On Tue, Sep 20, 2016, 21:13 Stefan Profanter notifications@github.com wrote:

Ok so it may have be related to an older version

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/FreeOpcUa/opcua-modeler/issues/9#issuecomment-248402437, or mute the thread https://github.com/notifications/unsubscribe-auth/ACcfzgdOfj356ZP82cBUddQD3RxH1GwWks5qsDA-gaJpZM4KBuJd .

zerox1212 commented 8 years ago

Thank you for documenting the issues. At the moment I don't have extra time for python opcua, but as soon as I have some free time I can continue working on the exporter. Like @oroulet said, any help is much appreciated!

zerox1212 commented 8 years ago

One note about XML element order. I think python element tree object (etree) is based on dicts, which do not maintain order. It's annoying but it might be difficult to solve.

oroulet commented 8 years ago

fixed most of them now. missing the alias and not exporting server uri.

oroulet commented 8 years ago

fixed all of them but order... Checked the exported Di file and it looks OK. We may have too many references. We do not need to keep all forward and backward references and ParentNodeId when they are the same

importing the Adi seems to generate a loop..... need to find where...

Pro commented 8 years ago

The generated XML is still not completely valid. E.g. according to the .xsd UAType (and its derived UAObjectType) aren't allowed to have a ParentNodeId attribute.

Also the Uses ReferenceType is wrongly exported as UAReference instead of UAReferenceType.

I'll see if I can find the corresponding code parts

Pro commented 8 years ago

Oh, and the NodeIDs are also wrong. It exports ns=2;... where it should be ns=1;... I think the referenced namespace index has to match the index in the NamespaceUris array.

oroulet commented 8 years ago

OK. do not be afraid to report everything you find... it helps a lot

I see some nodes are migrated to idx=2 when importing... there is a bug here. they should all be migrated to idx=2 when importing... or none. and I am wondering what to do when exporting....

oroulet commented 8 years ago

looked at it again. all nodeids are migrated to 2 when importing since namespace idx 1 is already used by server namespace. so the fixes should be to also migrate idx of browse names and move back to 1 when exporting again...

Pro commented 8 years ago

Yes, I think that would be the best way, also if you import multiple XMLs you then correctly handle the NS IDs.

Pro commented 8 years ago

I used the following python script to sort both xml files for diff: https://gist.github.com/Pro/0eb9a2082090883ec97df6fd842bb5c0

And then used this command line to start Meld with the diff: sed -i "s/ns=2/ns=1/g" ~/test/test.xml && python xmldiff.py meld ~/test/test.xml ~/opcua/schemas/DI/1.00/Opc.Ua.Di.NodeSet2.xml

Then you can see the difference more clearly

Pro commented 8 years ago

There's still a problem with the newer DI schema from here: https://opcfoundation.org/UADevices/1.1/Opc.Ua.Di.NodeSet2.xml

During Save As it fails with:

Traceback (most recent call last):
  File "/home/profanter/repos/opcua-modeler/uamodeler/uamodeler.py", line 281, in _save_as
    self._save()
  File "/home/profanter/repos/opcua-modeler/uamodeler/uamodeler.py", line 296, in _save
    exp.write_xml(self._current_path)
  File "/home/profanter/repos/python-opcua/opcua/common/xmlexporter.py", line 67, in write_xml
    xml_declaration=True
  File "/usr/lib/python3.5/xml/etree/ElementTree.py", line 775, in write
    short_empty_elements=short_empty_elements)
  File "/usr/lib/python3.5/xml/etree/ElementTree.py", line 940, in _serialize_xml
    short_empty_elements=short_empty_elements)
  File "/usr/lib/python3.5/xml/etree/ElementTree.py", line 932, in _serialize_xml
    v = _escape_attrib(v)
  File "/usr/lib/python3.5/xml/etree/ElementTree.py", line 1090, in _escape_attrib
    _raise_serialization_error(text)
  File "/usr/lib/python3.5/xml/etree/ElementTree.py", line 1056, in _raise_serialization_error
    "cannot serialize %r (type %s)" % (text, type(text).__name__)
TypeError: cannot serialize DataValue(Value:Variant(val:True,type:VariantType.Boolean), StatusCode:StatusCode(Good), SourceTimestamp:2016-09-22 09:29:18.327923, ServerTimestamp:2016-09-22 09:29:18.327922) (type DataValue)
oroulet commented 8 years ago

maybe put this in opcua-modeller repositoty in a directoy called dev-tools so we do not forget it?. I suppose there will be much work with xml diff in future....

That kind of bugs always happen if we write a python object instead of a string in etree. in this case we write a bool, should be easy to find where ...

oroulet commented 8 years ago

pushed a fix to the crash in python-opcua master. was a smal typo. Do you find other things? I did not manage to use you diff stuff The index stuff is much harder to fix. I need to understand the index import code in python-opcua and I am not the author of that...

oroulet commented 8 years ago

I removed the exposition of the freeopcua namespace in opcua-modeller. https://github.com/FreeOpcUa/opcua-modeler/commit/7c0ec26f193127a31fa0eb1702863e0cac00b6aa This is not necessary and as a side effect, it seems to solve all idx issues I could came up with. if this works, it was an easy fix. @Pro Can you test again?