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

Bugfix: Node.set_value() does not work with enumerations #888

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Bugfix: Node.set_value() does not work with enumerations #888

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Sep 9, 2019

If a client tries to set a enum variable without explicitly force the data type it becomes implicit converted to a ua.Variant.Int64. The server expects an Int32 for an enum value. This is why set_value() crashes if you try to set an enum value using a integer literal like "set_value(2)".

The enumeration seems to need a special treadment at this point. See ua_utils.py def data_type_to_variant_type().

If no data type was given in method set_value(), this change checks the data type of the node object and automatically converts the value argument to the correct target data type of the server.

If a client tries to set a enum variable without explicitly force the data type it becomes implicit converted to a ua.Variant.Int64. The server expects an Int32 for an enum value. This is why set_value() crashes if you try to set an enum value using a integer literal like "set_value(2)".

The enumeration seems to need a special treadment at this point. See ua_utils.py def data_type_to_variant_type().

If no data type was given in method set_value(), this change checks the data type of the node object and automatically converts the value argument to the correct target data type of the server.
@ghost
Copy link
Author

ghost commented Sep 9, 2019

Can anyone please review this PL? My client is doing fine with this change. The unit tests have also gone through successfully (on my machine). However, in my opinion, a test for setting an enumerarion unsing an integer literal lacks. Can someone help with implementing a test for the change?

@ghost
Copy link
Author

ghost commented Sep 9, 2019

The tests that fail here also fail with the main branch without any changes. Does anyone have an explanation for this?

The negative results of the tests seem to habe another cause the my changes.

@oroulet
Copy link
Member

oroulet commented Sep 23, 2019

Thanks for the PR but I am a bit afraid of that one. you are introusing a second network operation on a write call...

@ghost
Copy link
Author

ghost commented Nov 28, 2019

Thanks for the PR but I am a bit afraid of that one. you are introusing a second network operation on a write call...

Oh dear, I did not know that. How could this be done without a second network operation?

@oroulet
Copy link
Member

oroulet commented Dec 1, 2019

Maybe it is impossible.... But maybe we should write numbers with unspecified data types as 4 bytes by default. 32 seems to be the default in opcua...

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.

1 participant