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

fix decode NodeID #617

Closed
wants to merge 1 commit into from
Closed

fix decode NodeID #617

wants to merge 1 commit into from

Conversation

huskar-t
Copy link

When I use prosys OPC UA Simulation Server as server, I have the same problem as #550, I borrowed from python-opcua and fix the Decode method of NodeID, which seems to solve my problem.

@huskar-t
Copy link
Author

huskar-t commented Nov 14, 2022

@kung-foo @magiconair Please review, I really need this fix

@magiconair
Copy link
Member

Sure. Can you please remove the _ = since we don't do that. Can you also look up the relevant part of the spec and add a URL to the code.

@magiconair
Copy link
Member

Also, please add some tests. No merge without. Thank you.

@huskar-t
Copy link
Author

Sure. Can you please remove the _ = since we don't do that. Can you also look up the relevant part of the spec and add a URL to the code.

I can't find the client protocol specification on the web, can you give me some specifications or websites that you use?

@magiconair
Copy link
Member

@huskar-t You should find the NodeID specification somewhere in here: This is a link to the ExpandedNodeId but you should find the TCP/IP binary representation model somewhere in here as well.

https://reference.opcfoundation.org/v104/Core/docs/Part4/7.11/

@huskar-t
Copy link
Author

@magiconair Thank you very much, but after reading the code it seems that I made a mistake, I will solve this problem as soon as possible. Maybe close this pr first and create a new one after I solve it

@huskar-t huskar-t closed this Nov 14, 2022
@magiconair
Copy link
Member

Sounds good.

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.

None yet

2 participants