FreeOpcUa / python-opcua

LGPL Pure Python OPC-UA Client and Server
http://freeopcua.github.io/
GNU Lesser General Public License v3.0
1.35k stars 658 forks source link

Problems with custom enumeration from xml import #426

Open ghost opened 7 years ago

ghost commented 7 years ago

Hey there. I import the following XML into my server:

...
    <UAObjectType BrowseName="0:CustomType" NodeId="i=20001">
        <DisplayName>CustomType</DisplayName>
        <Description>CustomType</Description>
        <References>
            <Reference IsForward="false" ReferenceType="HasSubtype">i=58</Reference>
            <Reference ReferenceType="HasProperty">i=20003</Reference>
            <Reference ReferenceType="HasProperty">i=20007</Reference>
        </References>
    </UAObjectType>
    <UADataType BrowseName="0:Prio" NodeId="i=20004">
        <DisplayName>Prio</DisplayName>
        <Description>Prio</Description>
        <References>
            <Reference IsForward="false" ReferenceType="HasSubtype">i=29</Reference>
            <Reference ReferenceType="HasProperty">i=20005</Reference>
        </References>
    </UADataType>
    <UAVariable ArrayDimensions="3" BrowseName="0:EnumStrings" DataType="LocalizedText" NodeId="i=20005" ParentNodeId="i=20004" ValueRank="-3">
        <DisplayName>EnumStrings</DisplayName>
        <Description>EnumStrings</Description>
        <References>
            <Reference IsForward="false" ReferenceType="HasProperty">i=20004</Reference>
            <Reference ReferenceType="HasTypeDefinition">i=68</Reference>
            <Reference ReferenceType="HasModellingRule">i=78</Reference>
        </References>
        <Value>
            <uax:ListOfLocalizedText>
                <uax:LocalizedText>
                    <uax:Text>Prio1</uax:Text>
                    <uax:Locale/>
                </uax:LocalizedText>
                <uax:LocalizedText>
                    <uax:Text>Prio2</uax:Text>
                    <uax:Locale/>
                </uax:LocalizedText>
                <uax:LocalizedText>
                    <uax:Text>Prio3</uax:Text>
                    <uax:Locale/>
                </uax:LocalizedText>
            </uax:ListOfLocalizedText>
        </Value>
    </UAVariable>
    <UAVariable BrowseName="0:prio" DataType="i=20004" NodeId="i=20007" ParentNodeId="i=20001">
        <DisplayName>prio</DisplayName>
        <Description>prio</Description>
        <References>
            <Reference IsForward="false" ReferenceType="HasProperty">i=20001</Reference>
            <Reference ReferenceType="HasTypeDefinition">i=68</Reference>
        </References>
        <Value>
            <uax:LocalizedText>
                <uax:Text>Prio3</uax:Text>
                <uax:Locale/>
            </uax:LocalizedText>
        </Value>
    </UAVariable>
...

I add the python object with the following code:

    def add_object(self, name: str, objecttype: str, val):
        nodeid = self.server.get_root_node().get_child(["0:Types", "0:ObjectTypes", "0:BaseObjectType", "0:{}".format(objecttype)]).nodeid
        myobject = self.myobj.add_object(self.idx, name, nodeid)

        for attr in myobject.get_children():
            name = attr.get_display_name().to_string()
            attr.set_value(getattr(val, name))

Everything works fine, but when I try to read the custom enum value prio the server crashes:

Exception raised while parsing message from client, closing
Traceback (most recent call last):
  File "C:\Program Files (x86)\Python36-32\lib\site-packages\opcua\server\binary_server_asyncio.py", line 85, in _process_data
    ret = self.processor.process(hdr, buf)
  File "C:\Program Files (x86)\Python36-32\lib\site-packages\opcua\server\uaprocessor.py", line 87, in process
    return self.process_message(msg.SecurityHeader(), msg.SequenceHeader(), msg.body())
  File "C:\Program Files (x86)\Python36-32\lib\site-packages\opcua\server\uaprocessor.py", line 107, in process_message
    return self._process_message(typeid, requesthdr, algohdr, seqhdr, body)
  File "C:\Program Files (x86)\Python36-32\lib\site-packages\opcua\server\uaprocessor.py", line 183, in _process_message
    self.send_response(requesthdr.RequestHandle, algohdr, seqhdr, response)
  File "C:\Program Files (x86)\Python36-32\lib\site-packages\opcua\server\uaprocessor.py", line 41, in send_response
    response.to_binary(), message_type=msgtype, request_id=seqhdr.RequestId, algohdr=algohdr)
  File "C:\Program Files (x86)\Python36-32\lib\site-packages\opcua\ua\uaprotocol_auto.py", line 8969, in to_binary
    packet.append(fieldname.to_binary())
  File "C:\Program Files (x86)\Python36-32\lib\site-packages\opcua\ua\uatypes.py", line 988, in to_binary
    packet.append(self.Value.to_binary())
  File "C:\Program Files (x86)\Python36-32\lib\site-packages\opcua\ua\uatypes.py", line 835, in to_binary
    b.append(uabin.pack_uatype(self.VariantType, self.Value))
  File "C:\Program Files (x86)\Python36-32\lib\site-packages\opcua\ua\ua_binary.py", line 267, in pack_uatype
    return extensionobject_to_binary(value)
  File "C:\Program Files (x86)\Python36-32\lib\site-packages\opcua\ua\uaprotocol_auto.py", line 16373, in extensionobject_to_binary
    TypeId = FourByteNodeId(getattr(ObjectIds, "{0}_Encoding_DefaultBinary".format(obj.__class__.__name__)))
AttributeError: type object 'ObjectIds' has no attribute 'Prio_Encoding_DefaultBinary'

Can someone please tell me the fault?

oroulet commented 7 years ago

do you used a released version or master? I just rewote that part in master. Can you test?

I do not think it will fix it anyway. I am not sure what is happening here. Are you following the procesdure in examples/server-custom-object.py ? mayne you are writting bad data using set_value(), we do not do any type check on server side...

ghost commented 7 years ago

I used a released version. Using the master I got the following Traceback:

Exception raised while parsing message from client, closing
Traceback (most recent call last):
...
  File "...\python-opcua\opcua\ua\uatypes.py", line 1172, in extensionobject_to_binary
    TypeId = extension_object_ids[obj.__class__.__name__]
KeyError: 'Prio'

The method _setvalue(value) is called with an enum Prio.Prio3. Thank you for your support!

oroulet commented 7 years ago

Enums in ua are not really enum, they are strings. Write a string there.

Maybe we should see if it is possible to raise a more understandable error for users

ghost commented 7 years ago

Thank you very much! Now it works. Maybe it would be helpful to document the usage of enums and other special datatypes.

bitkeeper commented 7 years ago

Hi guys, Little bit late for the discussion but I notice a few things:

1. @oroulet Can you explain what you mean with are enum are strings ?

Enums are coded over the wire as numbers (see Standard Part 6 5.2.3). The set and get of an enum is by this always an Int32. The EnumStrings are only used to make it more human to read/write, but are not required to get stuff working. Due this I doubt if de common test test_enum implementation is correct; I don't expect get_value to return an localized string.

2. @devel0peris Concerning custom types try keep away from namespace 0 in code. python-opcua treat stuff in namespace different.

In xml it is fine if you have a namespace declaration on top like, then on import al your definitions are remapped to the a new namespace index:

    <NamespaceUris>
        <Uri>http://www.yournamespace.nl/blabla</Uri>
    </NamespaceUris>

3. @devel0peris If you apply the above to your enum value xml file the value should be changed to:

<Value>
      <uax:Int32>2</uax:Int32>
 </Value>

By off course by joining after the discussion it can happen that I totally on the wrong track ;-)

oroulet commented 7 years ago

@bitkeeper I do not remember the details but there are two types of enum EnumStrings and EnumValues. one is a string, the other an extension object. The second type might be instanciated as uint32..

bitkeeper commented 7 years ago

@oroulet EnumStrings and EnumValues are both coded as Int32 (see Standard Part 6 5.2.3). Compare it with enums in C. Take a look at uaprotocol_auto.py it contains examples of both for example ApplicationType vs AttributeWriteMask. Think about what it would mean to use LocalizedText as values for clients ...

EnumStrings and EnumValues are only used giving a specific meaning to Int32 value by using label. And has nothing todo with the value transport itself.

The difference between EnumStrings and EnumValues is that EnumString is a fixed sequence and EnumValues allow gaps.

EnumStrings Example:

EnumValues Example:

EnumStrings can just be described as a list because it a sequence (no gaps in the numbers). That is why EnumValues are using a struct (extension object) to describe the relation between the Int32 number and the label.

Attached is a zip that contains a xml schema example for EnumStrings and EnumValues: ua_xmlschema_enumexample.zip

oroulet commented 7 years ago

@bitkeeper Interresting. What you say make sense. Do we have en error in implementation? I kind of remember having to write string to enums, but maybe I remember wrong....

bitkeeper commented 7 years ago

@oroulet ok did some enum testsing today, conclusie at the end:

Created opc xml schema with:

Test 1 - load xml schema and inspect the server

Ok: everything seems fine

Test 2 - change value of enum at server side

Nok: No label more present with the value

Expected wrong type due no type check at the server side. Indeed wrong type: Int64

Ok: fine now type ok and label keeps present.

Test 3 - Use value based on IntEnum (like in uaprotocol_auto.py)

Nok: The IntEnum also maps to Int64 instead of Int32

Test 4 - browse existing type I

Ok

Test 5 - browse existing type II

Nok: raise UaError("{0} could not be packed with value {1}".format(vtype, value))

The part that isn't going well is: standard_address_space_part8.py:953 attrs.Value = ua.Variant(['Linear', 'Log', 'Ln'], ua.VariantType.LocalizedText)

Reproducable by:

v=ua.Variant(['Linear', 'Log', 'Ln'], ua.VariantType.LocalizedText)
v.to_binary()

or even more simple:

v=ua.Variant('Linear', ua.VariantType.LocalizedText)
v.to_binary()

Preliminary Conclusion:

bitkeeper commented 7 years ago

@oroulet I also have doubts about the implementation ua_utils.py:data_type_to_variant_type regarding a enumeration. If now returns ua.VariantType.LocalizedText or ua.VariantType.ExtensionObject. I think it should be ua.VariantType.UInt32 ?

oroulet commented 7 years ago

I got a crash there on an enum last week so you are probably right

On Tue, Apr 25, 2017, 15:50 Marcel notifications@github.com wrote:

@oroulet https://github.com/oroulet I also have doubts about the implementation ua_utils.py:data_type_to_variant_type regarding a enumeration. If now returns ua.VariantType.LocalizedText or ua.VariantType.ExtensionObject. I think it should be ua.VariantType.UInt32 ?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/FreeOpcUa/python-opcua/issues/426#issuecomment-297036191, or mute the thread https://github.com/notifications/unsubscribe-auth/ACcfzp6mbS8sZlCuMd9oDd1TyxK3KsbLks5rzfofgaJpZM4Mxij4 .

oroulet commented 7 years ago

If you have 2 minutes, the best would be to write a small test showing the error

On Tue, Apr 25, 2017, 15:52 Olivier Roulet-Dubonnet < olivier.roulet@gmail.com> wrote:

I got a crash there on an enum last week so you are probably right

On Tue, Apr 25, 2017, 15:50 Marcel notifications@github.com wrote:

@oroulet https://github.com/oroulet I also have doubts about the implementation ua_utils.py:data_type_to_variant_type regarding a enumeration. If now returns ua.VariantType.LocalizedText or ua.VariantType.ExtensionObject. I think it should be ua.VariantType.UInt32 ?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/FreeOpcUa/python-opcua/issues/426#issuecomment-297036191, or mute the thread https://github.com/notifications/unsubscribe-auth/ACcfzp6mbS8sZlCuMd9oDd1TyxK3KsbLks5rzfofgaJpZM4Mxij4 .

bitkeeper commented 7 years ago

Haha decent tests in 2 minutes ... I will see if I find some time tomorrow.

oroulet commented 7 years ago

@bitkeeper I went to all you comment above and I could not see any error.

bitkeeper commented 7 years ago

@oroulet I don't have a problem with set_value at all, it was only a warning that with enums you shouldn't set_value with the default numbers, but explicit use use a Int32 else you end up with Int64.

I think we have 3 things that requires fixing:

  1. ua_utils.py:data_type_to_variant_type (already fixed see PR #439 )
  2. Using generated classes in uaprotocol_auto.py based on IntEnum as values result in Int64 instead of Int32.
  3. All the construction like ua.Variant(['Linear', ...], ua.VariantType.LocalizedText) in standard_address_space_part8 will give an exception when a client tries to get the enum labels. You can say it is wrong, but it is the generated code in the standard adress space that uses this construction. If ua.Variant('Linear', ua.VariantType.LocalizedText) is wrong what is correct ?Can we just replace it by ua.LocalizedText('Linear')? In that case the code generator should be updated.
oroulet commented 7 years ago
  1. Ok we might have a bug here.
  2. Yes

On Tue, Apr 25, 2017, 22:12 Marcel notifications@github.com wrote:

@oroulet https://github.com/oroulet I don't have a problem with set_value at all, it was only a warning that with enums you shouldn't set_value with the default numbers, but explicit use use a Int32 else you end up with Int64.

I think we have 3 things that requires fixing:

  1. ua_utils.py:data_type_to_variant_type (already fixed see PR #439 https://github.com/FreeOpcUa/python-opcua/pull/439 )
  2. Using generated classes in uaprotocol_auto.py based on IntEnum as values result in Int64 instead of Int32.
  3. All the construction like ua.Variant(['Linear', ...], ua.VariantType.LocalizedText) in standard_address_space_part8 will give an exception when a client tries to get the enum labels. You can say it is wrong, but it is the generated code in the standard adress space that uses this construction. If ua.Variant('Linear', ua.VariantType.LocalizedText) is wrong what is correct ?Can we just replace it by ua.LocalizedText('Linear')? In that case the code generator should be updated.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/FreeOpcUa/python-opcua/issues/426#issuecomment-297150840, or mute the thread https://github.com/notifications/unsubscribe-auth/ACcfzmlmv2Nf2EKFH5Y3Cqr8bGN1Tndgks5rzlOygaJpZM4Mxij4 .