FreeOpcUa / opcua-asyncio

OPC UA library for python >= 3.7
GNU Lesser General Public License v3.0
1.15k stars 368 forks source link

read_data_type_as_variant_type returns wrong type for enums. #620

Closed Randelung closed 1 year ago

Randelung commented 3 years ago

EDIT: refuted. See history below.

Hey guys, I think this is a quick one.

import asyncio

from asyncua import Server, ua

async def main():
    server = Server()
    server.set_endpoint('opc.tcp://0.0.0.0:4840/')
    server.set_security_policy([ua.SecurityPolicyType.NoSecurity])
    await server.init()
    namespace = await server.register_namespace('http://ticos.ch')
    enum = await server.nodes.enum_data_type.add_data_type(namespace, 'testEnum')
    await enum.add_property(0, 'EnumStrings', [ua.LocalizedText(x) for x in ['Value 1', 'Value 2']])
    await server.load_data_type_definitions()
    await server.start()
    node = await server.nodes.objects.add_variable(namespace, 'variable', 0, datatype=enum.nodeid)
    print(await node.read_data_type_as_variant_type())

    await asyncio.Future()

if __name__ == '__main__':
    asyncio.run(main())

The script produces a server with an enum and a single variable using said enum. uaExpert shows Int64, as expected: image The script also prints out the variant type:

# python3 test.py 
VariantType.Int32

which is less expected. The line of code responsible is the last one here: https://github.com/FreeOpcUa/opcua-asyncio/blob/0a1741c806439d37f10abb0acc7a03febaf8890e/asyncua/common/ua_utils.py#L225-L234 and I believe it should be an Int64.

Cheers.

Randelung commented 3 years ago

There's a similar issue with Variants passed to method callbacks. After calling said method configured using an enum:

CallMethodRequest(ObjectId=NodeId(Identifier=UUID('97bb094c-d322-47cf-8274-ab6600aa254c'), NamespaceIndex=2, NodeIdType=<NodeIdType.Guid: 4>), MethodId=NodeId(Identifier=UUID('044a69f4-4559-48ad-9d2a-cae984bcad32'), NamespaceIndex=2, NodeIdType=<NodeIdType.Guid: 4>), InputArguments=[Variant(Value=0, VariantType=<VariantType.Int32: 6>, Dimensions=None, is_array=False)])

Edit: uaExpert is at least partially to blame, see below.

Randelung commented 3 years ago

Welp image The uaExpert method call sends an Int32, despite the input argument being an enum. So that's fun!

https://reference.opcfoundation.org/v104/Core/docs/Part3/8.40/

When it is used to define the string representation of an Enumeration DataType, the value range is limited to Int32, because the Enumeration DataType is a subtype of Int32. OPC 10000-8 specifies other usages where the actual value might be between 8 and 64 Bit.

So this means the RANGE of values can be limited, but the data type is still Int64, right? So uaExpert is wrong?

Internal enum datapoints ARE Int64, as confirmed by wireshark. image Which means my initial post still applies at least, but my comment above is wrong.

I'll give more context into the matter, too: I'm trying to fulfil a requirement that says most of the datapoints on the OPC UA server need to be Enums. Enum definition worked so far, they're Int64, which is confirmed by both uaExpert and wireshark. Another requirement says they have to be UInt16. I'm under the impression that's impossible, right? Now I'm also trying to translate types between WinCC OA and actual feature rich OPC UA (like this library), and with a recent change to the library I'm now required to know the target type of the node I'm writing to. (In the earlier days it would just write whatever, which was bound to lead to issues later on, so that's an improvement.) For that matter I'm querying the VariantType of the node-to-be-written-to, which is in almost all cases an enum. As outlined in the original post, it'll (in my opinion wrongfully) return an Int32. Then I get the Int32 from uaExpert, my comparison says it's already the correct type and tries to write to the node - only to fail, because it's actually an Int64. Monkey patching the line mentioned line in my first post will resove the issue... I'm trying to gauge if it's a correct fix or not, though.

In the end I'm thinking I'll leave it monkey patched (I froze the library version I'm using for deployment purposes). The server enum nodes are Int64 and it's stable with the change. Whether uaExpert sends an Int32, UInt32 or Int64 shouldn't matter, as both Int32 and UInt32 are contained in Int64.

oroulet commented 3 years ago

I was 100% sure enums were int32 but i just checked and they should be int64.... So maybe i wrote something wrong somewhere in library...

oroulet commented 3 years ago

well here they write enums sould be int32 https://reference.opcfoundation.org/Core/docs/Part6/5.2.4/

oroulet commented 3 years ago

and asyncua code we encode/decode in UInt32 and that has worked for years.... so I have no ideas what is happening here. Anyway I tried to fix it: https://github.com/FreeOpcUa/opcua-asyncio/pull/631

oroulet commented 3 years ago

so in conclusion. enums as Int32 but for some reasons that EnumValType structure uses Int64 to represent enum value

Randelung commented 3 years ago

So much for "this is a simple one". 😂 What you're saying makes sense to me. The choices encoded in the enum data type (together with the localized text and description) might be Int64, while the object instantiated from the type has type Int32. In my understanding that covers the little text below the EnumValueType. It doesn't make things easier, though.

The node created by a custom type would have to be Int32, then, so your PR also makes sense to me.

and asyncua code we encode/decode in UInt32 and that has worked for years.... I fully believe you, since enums are in amost all cases natural numbers plus 0, for which an unsigned number set is very much suited.

I fear it doesn't fully encompass the whole issue, though. Writing to the node with anything but an Int64 fails, no matter what, and the actual transmitted value is an Int64.

import asyncio

from asyncua import Server, ua

async def main():
    server = Server()
    server.set_endpoint('opc.tcp://0.0.0.0:4840/')
    server.set_security_policy([ua.SecurityPolicyType.NoSecurity])
    await server.init()
    namespace = await server.register_namespace('http://ticos.ch')
    enum = await server.nodes.enum_data_type.add_data_type(namespace, 'testEnum')
    await enum.add_property(0, 'EnumStrings', [ua.LocalizedText(x) for x in ['Value 1', 'Value 2']])
    await server.load_data_type_definitions()
    await server.start()
    node = await server.nodes.objects.add_variable(namespace, 'variable', 0, datatype=enum.nodeid)
    print(await node.read_data_type_as_variant_type())
    await node.write_value(ua.DataValue(ua.Variant(1, ua.VariantType.Int64))) # succeeds
    await node.write_value(ua.DataValue(ua.Variant(1, ua.VariantType.Int32))) # fails

    await asyncio.Future()

if __name__ == '__main__':
    asyncio.run(main())

I have no idea what the scope is of what I'm suggesting, but it sounds to me as if the node created by the add_variable method would need to be an Int32 instead of an Int64 when facing an enum data type, yes?

Edit: Small detail about why I'm being so insistent, which I forgot in my backstory: I have to transfer the StatusCode from the WinCC OA server, too, so I'm forced to work with DataValues and Variants. Also, writing nodes on the WinCC OA server requires knowledge of the target variant type in order not to corrupt the data set. Very fun experiment if you're inclined, you can send any Int to a WinCC OA write enabled Bool. Any dpConnect'ed callback will trigger with the actually sent data, while backup into the database fails. Not even 1 and 0 are being interpreted as Bool. WinCC OA - the usually so lenient type converting system - is unable to properly fit the data. It'll send you an error anyway, though.

oroulet commented 3 years ago

do yo uhave the same issue creating new enums using the new data_type_definiion?

enode = await new_enum(server, idx, "MyEnum", [
        "titi",
        "toto",
        "tutu",
    ])
oroulet commented 3 years ago

OK. I understand in the code over you create your enum by hand and add the defualt value as 0. if you had written the default value as ua.testEnum.Value_1 you would have gotten the correct variant type. The library could check that this is in reality an enum and that 0 should be converted to In32 and not Int64 but this is quite time consuming,,, I think it is not correct to use 0 as value when you shoud write an enum as ddefault value

swamper123 commented 3 years ago

@oroulet I am not sure how Errata 1.04.7 (Need to add the term "DefaultValue" for all datatypes) may influence this.

Since Erratas are not shown on the reference page, here the snippit: grafik

Randelung commented 3 years ago

OK. I understand in the code over you create your enum by hand and add the defualt value as 0. if you had written the default value as ua.testEnum.Value_1 you would have gotten the correct variant type.

That's good insight, thank you. The ua.enum.value approach didn't work when I tried it the first time, but I didn't try again after updating the library. I will try again and if everything is as you say I imagine I'll be able to close the ticket. After all, the initial node type is correctly Int32, the method call comes correctly as an Int32 - it basically came down to the node initialization that I did wrong. The UInt32 -> Int32 is a bonus. :)

Randelung commented 3 years ago
import asyncio

from asyncua import Server, ua

async def main():
    server = Server()
    server.set_endpoint('opc.tcp://0.0.0.0:4840/')
    server.set_security_policy([ua.SecurityPolicyType.NoSecurity])
    await server.init()
    namespace = await server.register_namespace('http://ticos.ch')
    enum = await server.nodes.enum_data_type.add_data_type(namespace, 'testEnum')
    await enum.add_property(0, 'EnumStrings', [ua.LocalizedText(x) for x in ['Value1', 'Value2']])
    await server.load_data_type_definitions()
    await server.start()
    node = await server.nodes.objects.add_variable(namespace, 'variable', ua.testEnum.Value1, datatype=enum.nodeid) # fails

    await asyncio.Future()

if __name__ == '__main__':
    asyncio.run(main())

I'm still getting the issue I had when I first tried... What am I doing wrong? The testEnum isn't present on the ua module.

  File "/home/randelung/Desktop/temp/script.py", line 16, in main
    node = await server.nodes.objects.add_variable(namespace, 'variable', ua.testEnum.Value1, datatype=enum.nodeid)
AttributeError: module 'asyncua.ua' has no attribute 'testEnum'
oroulet commented 3 years ago

call load_enums() since you are using the old protocol way of doing it

oroulet commented 3 years ago

load_data_type_definition() will only load custom enums/structs created the new way I showed above

oroulet commented 3 years ago

not all servers support that

Randelung commented 3 years ago

Oh shit, my bad. I thought the load_data_type_definitions was a replacement for the old load_enums. One more try.

Randelung commented 3 years ago
import asyncio

from asyncua import Server, ua

async def main():
    server = Server()
    server.set_endpoint('opc.tcp://0.0.0.0:4840/')
    server.set_security_policy([ua.SecurityPolicyType.NoSecurity])
    await server.init()
    namespace = await server.register_namespace('http://ticos.ch')
    enum = await server.nodes.enum_data_type.add_data_type(namespace, 'testEnum')
    await enum.add_property(0, 'EnumStrings', [ua.LocalizedText(x) for x in ['Value1', 'Value2']])
    await server.load_enums()
    await server.start()
    await server.nodes.objects.add_variable(namespace, 'variable0', 0, datatype=enum.nodeid)
    await server.nodes.objects.add_variable(namespace, 'testEnum', ua.testEnum.Value1, datatype=enum.nodeid)

    await asyncio.Future()

if __name__ == '__main__':
    asyncio.run(main())

This works! image

The new way starts alright, but uaExpert doesn't show the string representation anymore.

import asyncio

from asyncua import Server, ua
from asyncua.common.structures104 import new_enum

async def main():
    server = Server()
    server.set_endpoint('opc.tcp://0.0.0.0:4840/')
    server.set_security_policy([ua.SecurityPolicyType.NoSecurity])
    await server.init()
    namespace = await server.register_namespace('http://ticos.ch')
    await new_enum(server, namespace, 'testEnum', ['Value1', 'Value2'])
    await server.load_data_type_definitions()
    await server.start()
    await server.nodes.objects.add_variable(namespace, 'testEnum', ua.testEnum.Value1)

    await asyncio.Future()

if __name__ == '__main__':
    asyncio.run(main())

image

Randelung commented 3 years ago

For me, this solves the issue. I'll leave it up to you whether you want to close the issue or not, but it's probably cleaner to start a new ticket if you want to look into the load_data_type_definitions not showing up in uaExpert. As you said, it might just not be supported, or I may have made a mistake. I'll stick with the old style for now, though, as my counterpart will be using a custom library, too, and I don't know about their compatibility with 1.04.

Thanks for your help.