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.

Sign up for GitHub

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

Jump to bottom

PLC4x-142 OPC-UA read variables #130

Merged
merged 3 commits into from
Mar 4, 2020
Merged

PLC4x-142 OPC-UA read variables #130

merged 3 commits into from
Mar 4, 2020

Conversation

nalim2
Copy link
Contributor

@nalim2 nalim2 commented Mar 3, 2020

[fix] Changed restircitive RegEx for OPC UA node id

@nalim2
[fix] Bug in the regular expression of the variable address which inc…
1775743 
…ludes the fix of the issue #PLC4X-142 OPC-UA read variables
@nalim2
[fix] Bug in the regular expression of the variable address which inc…
4c6e062 
…ludes the fix of the issue #PLC4X-142 OPC-UA read variables
JulianFeinauer
JulianFeinauer reviewed Mar 3, 2020
View reviewed changes
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.

LGTM, but I would like to have a Unit Test to see that it works. See e.g. S7FieldTest for an Example of Automated tests.

@nalim2
Copy link
Contributor Author

nalim2 commented Mar 3, 2020

I kind of like it to get feedback ;)

For my defense there is already an OpcuaFieldTest. But those tests are missing!
i will add them and come back

JulianFeinauer
JulianFeinauer approved these changes Mar 4, 2020
View reviewed changes
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.

LGTM

@nalim2 nalim2 merged commit 6471721 into rel/0.6 Mar 4, 2020
@asfgit asfgit deleted the fix-OPC-wrong-paramhandling branch February 3, 2021 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JulianFeinauer JulianFeinauer JulianFeinauer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nalim2 @JulianFeinauer