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

Feature/opcua #66

Merged
merged 9 commits into from
May 19, 2019
Merged

Feature/opcua #66

merged 9 commits into from
May 19, 2019

Conversation

nalim2
Copy link
Contributor

@nalim2 nalim2 commented May 13, 2019

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.

@nalim2
Copy link
Contributor Author

nalim2 commented May 13, 2019

Added test class bodies and renamed classes

Copy link
Contributor

@JulianFeinauer JulianFeinauer left a 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?

@nalim2
Copy link
Contributor Author

nalim2 commented May 14, 2019

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?

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
Copy link
Contributor

@chrisdutz chrisdutz left a 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.

@nalim2
Copy link
Contributor Author

nalim2 commented May 14, 2019

It is on it's way i should have done that way earlier!

@JulianFeinauer JulianFeinauer merged commit 3c00fa3 into apache:develop May 19, 2019
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

3 participants