Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The OPCUA Server sets the Servertimestamp in the moment of writing th… #1197

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mbay-ODW
Copy link

If you use an OPCUA Client such as UA Expert from Prosys and change values within the gui than the Servertimestamp is not set while writing the node, which is from our perspective not as specified by the OPCUA guidelines.
The Timestamp will be set to EPOCH Time which is on Windows in the year 1601.
The bug is contained in address_space.py in the write function. There the datavalue for Servertimestamp should be set before writing the node.

See: [(https://github.com//issues/1192)]

If the Sourcetimestamp is not set by the client, you can use the commented line with SourceTimestamp as well.

…e node, which is from our perspective as specified by the OPCUA guidline.

If the Sourcetimestamp is not set by the client, you can use the commented line with SourceTimestamp as well.
@@ -63,6 +63,9 @@ def write(self, params, user=UserManager.User.Admin):
if not ua.ua_binary.test_bit(al.Value.Value, ua.AccessLevel.CurrentWrite) or not ua.ua_binary.test_bit(ual.Value.Value, ua.AccessLevel.CurrentWrite):
res.append(ua.StatusCode(ua.StatusCodes.BadUserAccessDenied))
continue
tz = pytz.timezone('Europe/Berlin')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a fixed timezone "to Europe/Berlin" wouldn't be a good way. Either the servers local time should be passed or handled in another way.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally agree!

@@ -63,6 +63,9 @@ def write(self, params, user=UserManager.User.Admin):
if not ua.ua_binary.test_bit(al.Value.Value, ua.AccessLevel.CurrentWrite) or not ua.ua_binary.test_bit(ual.Value.Value, ua.AccessLevel.CurrentWrite):
res.append(ua.StatusCode(ua.StatusCodes.BadUserAccessDenied))
continue
tz = pytz.timezone('Europe/Berlin')
writevalue.Value.ServerTimestamp = datetime.now(tz)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

datetime.now(tz.utc) would be better, since every time thing on server side is based on UTC. Clients shall provide any conversions between UTC and local time.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

@@ -63,6 +63,9 @@ def write(self, params, user=UserManager.User.Admin):
if not ua.ua_binary.test_bit(al.Value.Value, ua.AccessLevel.CurrentWrite) or not ua.ua_binary.test_bit(ual.Value.Value, ua.AccessLevel.CurrentWrite):
res.append(ua.StatusCode(ua.StatusCodes.BadUserAccessDenied))
continue
tz = pytz.timezone('Europe/Berlin')
writevalue.Value.ServerTimestamp = datetime.now(tz)
#writevalue.Value.SourceTimestamp = datetime.now(tz)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No comment is left behind in a PR. ;) You can delte that line, if it is unused.

Copy link
Author

@mbay-ODW mbay-ODW left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with the comments!

@oroulet
Copy link
Member

oroulet commented Mar 3, 2021

That feature always comes back. I think it was removed due to performance. We should check if we have performance issues with that branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants