-
Notifications
You must be signed in to change notification settings - Fork 426
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
Feature/opcua #66
Feature/opcua #66
Conversation
…out any tests or refactoring in a first fully functional state.
…] the wrong handle function "encodeDateTime" it become changed to "encodeByteArray"
[Added] requestTimeout field for variable timeouts
Added test class bodies and renamed classes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @nalim2 thank you very much and congratz to your first PLC4X PR 🎉
I found some (minor) issues which I think are debatable, so feel free to comment on them (or ignore them? :D).
I did not test it against a OPC UA device, so I would like to do a manual test against a device. Did you do a manual check?
...rivers/opcua/src/main/java/org/apache/plc4x/java/opcua/connection/OpcuaTcpPlcConnection.java
Show resolved
Hide resolved
...rivers/opcua/src/main/java/org/apache/plc4x/java/opcua/connection/OpcuaTcpPlcConnection.java
Show resolved
Hide resolved
...rivers/opcua/src/main/java/org/apache/plc4x/java/opcua/connection/OpcuaTcpPlcConnection.java
Show resolved
Hide resolved
...rivers/opcua/src/main/java/org/apache/plc4x/java/opcua/connection/OpcuaTcpPlcConnection.java
Show resolved
Hide resolved
...rivers/opcua/src/main/java/org/apache/plc4x/java/opcua/connection/OpcuaTcpPlcConnection.java
Show resolved
Hide resolved
plc4j/drivers/opcua/src/main/java/org/apache/plc4x/java/opcua/OpcuaPlcDriver.java
Outdated
Show resolved
Hide resolved
...j/drivers/opcua/src/main/java/org/apache/plc4x/java/opcua/protocol/model/OpcuaDataTypes.java
Show resolved
Hide resolved
...vers/opcua/src/main/java/org/apache/plc4x/java/opcua/protocol/model/OpcuaIdentifierType.java
Outdated
Show resolved
Hide resolved
plc4j/drivers/opcua/src/test/java/org/apache/plc4x/java/opcua/ManualPLC4XOpcua.java
Show resolved
Hide resolved
Yeah now after one year i found this 2 days where a was able to programm something beside the PPEE. It is a bit ad hoc tested. Those tests are in the ManualPLC4XOpcua.java but they access a public test server and the variables are not writable and i wanted to avoid automated tests against a third partie public OPC UA server (even if it is a public test server). So the read and subscribe works for all the primitives and for the error behavior of the write method. I will integrate later a OPC UA Server over milo as test environment. |
[Added] Warning messages at exception points with the consideration of adding later some run-time exceptions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks valid from my point of view.
So +1 for merging as soon as we have ICLA on file.
It is on it's way i should have done that way earlier! |
initial PR for the branche feature/opcua. This should be feature complete but in a bad structure as well just ad hoc tested. So there will be a major refactoring, code commenting and testing.