FreeOpcUa / opcua-asyncio

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

asyncua.Client discards tzinfo attribute when reading timezone aware variables from server #1414

Open ntbloom opened 1 year ago

ntbloom commented 1 year ago

Describe the bug
When adding a variable to an object of the uatypes.DateTime type, when we read the value from the server, the timezone is preserved. When reading from the client, the tzinfo attribute is discarded.

To Reproduce
The following code snippet should document the exact problem:

import asyncio
import logging

from asyncua import Server, Client
from asyncua.ua import uatypes

from datetime import timezone

async def main():
    _logger = logging.getLogger(__name__)
    server = Server()
    await server.init()
    url = "opc.tcp://0.0.0.0:4840/freeopcua/server/"
    server.set_endpoint(url)

    uri = "http://examples.freeopcua.github.io"
    idx = await server.register_namespace(uri)

    original_value = uatypes.DateTime.now(timezone.utc)
    myobj = await server.nodes.objects.add_object(idx, "MyObject")
    myvar = await myobj.add_variable(
        idx, "MyVariable", original_value, varianttype=uatypes.VariantType.DateTime
    )

    _logger.info("Starting server!")
    async with server:
        read_from_server = await myvar.get_value()
        async with Client(url=url) as client:
            client_node = client.get_node(myvar)
            read_from_client = await client_node.get_value()

        _logger.info(f"{original_value=}")
        _logger.info(f"{read_from_server=}")
        _logger.info(f"{read_from_client=}")

        assert read_from_server == original_value, "server read discards tzinfo"
        assert read_from_server == read_from_client, "client read discards tzinfo"

if __name__ == "__main__":
    logging.basicConfig(level=logging.INFO)
    logging.getLogger("asyncua").setLevel(logging.CRITICAL)
    asyncio.run(main(), debug=True)

Here are the results of running this:

...
INFO:__main__:original_value=DateTime(2023, 8, 23, 14, 47, 2, 319146, tzinfo=datetime.timezone.utc)
INFO:__main__:read_from_server=DateTime(2023, 8, 23, 14, 47, 2, 319146, tzinfo=datetime.timezone.utc)
INFO:__main__:read_from_client=datetime.datetime(2023, 8, 23, 14, 47, 2, 319146)
...
AssertionError: client read discards tzinfo

Expected behavior
I expect the client to return an equivalent value to what gets retrieved by the server.

Version
Python-Version:3.11.4
opcua-asyncio Version (e.g. master branch, 0.9): 1.0.3

schroeder- commented 1 year ago

This is normal, opc ua timestamps are always utc

ntbloom commented 1 year ago

I understand that the server handles timezones and always returns UTC time. The issue is that it makes it difficult to compare values if you pass in a timezone aware object and are returned a timezone naive object. I don't think it's reasonable to expect that all uatypes.DateTime objects should be returned with a tzinfo attribute of timezone.utc (that could break some users), but if the value that's passed into the server is timezone aware, shouldn't the returned value also be timezone aware?

AndreasHeine commented 1 year ago

i guess this could be an issue... https://github.com/FreeOpcUa/opcua-asyncio/blob/a8091a44dbf9959f37d84e39d15e3110acdd69e2/asyncua/ua/uatypes.py#L188

i am not sure if you can assume that a "Python DateTime" and a "OPC UA DateTime" are the same which means the native comparing might be not correct...

https://reference.opcfoundation.org/Core/Part6/v105/docs/5.2.2.5 https://reference.opcfoundation.org/Core/Part6/v105/docs/5.4.2.6 https://reference.opcfoundation.org/Core/Part6/v105/docs/5.3.1.6

i would convert the datetime to the 64bit signed int and compare those rather then the datetime itself!

ntbloom commented 1 year ago

Yes, there are different ways to workaround and this is not currently blocking me in any way. My guess is that inheriting DateTime from int instead of Datetime might be a longterm goal in order to be more compliant with the spec, but again that would almost certainly break a lot of existing code. I'm happy to close the issue if that's the sanest option.