-
Notifications
You must be signed in to change notification settings - Fork 423
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
Add encryption handler for OPC UA - Minor fix for Kafka Connector #225
Conversation
Outstanding issue with PLC4X comms failure when adding tag that isn't valid.
Fixed opcua server issue when polling errors occur. Now polling doesn't stop for other tags. Updated kafka conector based on Rankesh's comment about adding jitter to the poll return time.
@@ -1,11 +1,20 @@ | |||
/* | |||
* Copyright (c) 2019 the Eclipse Milo Authors |
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.
i am no expert on this license topic. But is this ok in that way? That we have the small Milo notice in the "Notice" file and change the license?
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.
Unfortunately not ... EPL is category B ... we can use (in form of use libaries) but we can't include that code. ... not even just a bit. If it's just a bit, it would be better to re-write it.
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.
The EPL headers for the Plc4xNamespace and OPCUAServer files were added in as I wasn't sure how much of the code reflected the Milo example.
I am comfortable that there is no code that needs to be licensed under EPL remaining.
...opcua-server/src/main/java/org/apache/plc4x/java/opcuaserver/backend/Plc4xCommunication.java
Show resolved
Hide resolved
@@ -195,93 +169,110 @@ public static NodeId getNodeId(String plcValue) { | |||
} | |||
|
|||
public DataValue getValue(AttributeFilterContext.GetAttributeContext ctx, String tag, String connectionString) { | |||
PlcConnection connection = null; | |||
DataValue resp = new DataValue(new Variant(null), StatusCode.BAD); |
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.
Maybe we should have a static constant, Datavalue resp = DataValue.BAD_STATUS
...opcua-server/src/main/java/org/apache/plc4x/java/opcuaserver/backend/Plc4xCommunication.java
Show resolved
Hide resolved
...ons/opcua-server/src/main/java/org/apache/plc4x/java/opcuaserver/backend/Plc4xNamespace.java
Show resolved
Hide resolved
import java.util.Calendar; | ||
import java.util.Date; | ||
import java.util.Locale; | ||
|
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.
Can we rename this for the context? Encryption seems a little too generic
Rename Encryption file to CertificateGenerator Updated license and notice files. Fix, if certificate store doesn't exist a new one wasn't created.
Otto, all good ideas, I have updated the code. |
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.
Just a question about making the BAD_RESPONSE static.
But that is a nit.
@@ -65,6 +65,7 @@ | |||
private final Logger logger = LoggerFactory.getLogger(getClass()); | |||
private final Integer DEFAULT_TIMEOUT = 1000000; | |||
private final Integer DEFAULT_RETRY_BACKOFF = 5000; | |||
private final DataValue BAD_RESPONSE = new DataValue(new Variant(null), StatusCode.BAD); |
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.
should this be static?
I've added the Encryption handler for the OPC UA server and re-wrote some of the logic. I have worked on a add just what we need to the logic, we can build from here.
If an error occurs, in some cases, while polling for the OPC UA server all the tags will stop. This has been fixed so that only the failed tag stops and shows a bad value.
Also a fix based on Rankesh's comment about adding jitter to the poll return value for the Kafka Source Connector. This distributes some of the load on the Kafka servers when a lot of connectors are all started at the same time.