Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


chrisdutz commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1115313507


##
plc4j/api/src/main/java/org/apache/plc4x/java/api/model/PlcTag.java:
##
@@ -63,6 +63,10 @@ default PlcValueType getPlcValueType() {
 return PlcValueType.NULL;
 }
 
+@JsonIgnore
+default void setPlcValueType(PlcValueType plcValueType){
+}

Review Comment:
   This seems to be related to the change in DefaultPlcWrite request, were the 
plc value type can be updated ... as I don't really like that change, I think 
we should also remove this change ... as both actually don't have anything to 
do with what we would like to address with this PR



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


chrisdutz commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1115312253


##
plc4j/tools/connection-cache/src/main/java/org/apache/plc4x/java/utils/cache/LeasedPlcConnection.java:
##
@@ -92,15 +102,139 @@ public PlcReadRequest.Builder readRequestBuilder() {
 if(connection == null) {
 throw new PlcRuntimeException("Error using leased connection after 
returning it to the cache.");
 }
-return connection.readRequestBuilder();
+final PlcReadRequest.Builder innerBuilder = 
connection.readRequestBuilder();
+return new PlcReadRequest.Builder(){
+
+@Override
+public PlcReadRequest build() {
+final PlcReadRequest innerPlcReadRequest = 
innerBuilder.build();
+return new PlcReadRequest(){
+
+@Override
+public CompletableFuture 
execute() {
+CompletableFuture future = 
innerPlcReadRequest.execute();
+final CompletableFuture 
responseFuture = new CompletableFuture<>();
+future.handle((plcReadResponse, throwable) -> {
+if (plcReadResponse != null) {
+responseFuture.complete(plcReadResponse);
+} else {
+try {
+destroy();
+} catch (Exception e) {
+}
+
responseFuture.completeExceptionally(throwable);

Review Comment:
   But I guess ... otherwise there's no way for the user to actually invalidate 
a connection ... I guess we should probably have different sets of Exceptions 
that we return. Some that indicate the connection is no longer usable and some 
that say: you did something wrong, please do it right next time (Like invalid 
addresses)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


chrisdutz commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1115310805


##
plc4j/tools/connection-cache/src/main/java/org/apache/plc4x/java/utils/cache/LeasedPlcConnection.java:
##
@@ -51,13 +51,23 @@ public void run() {
 
 @Override
 public synchronized void close() {
-// Make the connection unusable.
-connection = null;
-
+if(connectionContainer == null) {
+return;
+}

Review Comment:
   Why should the connectionContainer ever be null? The way I understand it, 
the only way this could happen, would be that a program got a lease ... 
executed a request, that had an issue and therefore it automatically closed the 
connection and destroy() called close from the inside and then the using 
application calls close regularly ... not too happy with this workflow in 
general ... we should really discuss things on the list.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


chrisdutz commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1115308098


##
plc4j/tools/connection-cache/src/main/java/org/apache/plc4x/java/utils/cache/LeasedPlcConnection.java:
##
@@ -92,15 +102,139 @@ public PlcReadRequest.Builder readRequestBuilder() {
 if(connection == null) {
 throw new PlcRuntimeException("Error using leased connection after 
returning it to the cache.");
 }
-return connection.readRequestBuilder();
+final PlcReadRequest.Builder innerBuilder = 
connection.readRequestBuilder();
+return new PlcReadRequest.Builder(){
+
+@Override
+public PlcReadRequest build() {
+final PlcReadRequest innerPlcReadRequest = 
innerBuilder.build();
+return new PlcReadRequest(){
+
+@Override
+public CompletableFuture 
execute() {
+CompletableFuture future = 
innerPlcReadRequest.execute();
+final CompletableFuture 
responseFuture = new CompletableFuture<>();
+future.handle((plcReadResponse, throwable) -> {
+if (plcReadResponse != null) {
+responseFuture.complete(plcReadResponse);
+} else {
+try {
+destroy();
+} catch (Exception e) {
+}
+
responseFuture.completeExceptionally(throwable);

Review Comment:
   So do I understand this block correctly? If anything goes wrong with the 
request, it terminates the connection, right? Should we really be doing this? I 
mean ... it's easy to ask for an invalid tag ... that doesn't really make it 
necessary to reconnect? For example with my KNX gateway it only allows 3 
connections in paralell and within a window of 2-3 minutes (when it closes 
inactive connections) ... not 100% sure we should be doing this.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


chrisdutz commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1115305690


##
plc4j/tools/connection-cache/src/main/java/org/apache/plc4x/java/utils/cache/LeasedPlcConnection.java:
##
@@ -92,15 +102,139 @@ public PlcReadRequest.Builder readRequestBuilder() {
 if(connection == null) {
 throw new PlcRuntimeException("Error using leased connection after 
returning it to the cache.");
 }
-return connection.readRequestBuilder();
+final PlcReadRequest.Builder innerBuilder = 
connection.readRequestBuilder();
+return new PlcReadRequest.Builder(){
+
+@Override
+public PlcReadRequest build() {
+final PlcReadRequest innerPlcReadRequest = 
innerBuilder.build();
+return new PlcReadRequest(){
+
+@Override
+public CompletableFuture 
execute() {
+CompletableFuture future = 
innerPlcReadRequest.execute();
+final CompletableFuture 
responseFuture = new CompletableFuture<>();
+future.handle((plcReadResponse, throwable) -> {
+if (plcReadResponse != null) {

Review Comment:
   Could we change this to "throwable == null"? ... it generally means the 
same, but it more sets the focus on "nothing went wrong".



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


chrisdutz commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1115303543


##
plc4j/tools/connection-cache/src/main/java/org/apache/plc4x/java/utils/cache/ConnectionContainer.java:
##
@@ -54,8 +62,19 @@ public synchronized Future lease() {
 }
 return connectionFuture;
 }
-
+public synchronized void close(){
+CompletableFuture leaseFuture;
+while((leaseFuture = queue.poll())!=null){
+leaseFuture.complete(null);

Review Comment:
   Wouldn't it make more sense to complete exceptionally instead of returning 
null? We are currently expecting the result of a getConnection() to be a usable 
connection or an exception ... returning "null" sort of doesn't match that 
contract. I would change it to "completeExceptionally"



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


chrisdutz commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1115301187


##
plc4j/tools/connection-cache/src/main/java/org/apache/plc4x/java/utils/cache/ConnectionContainer.java:
##
@@ -28,12 +28,20 @@
 import java.util.concurrent.Future;
 
 class ConnectionContainer {
-private final PlcConnection connection;
+private PlcConnection connection;
+private boolean closed = false;
 private final Duration maxLeaseTime;
 private final Queue> queue;
 
 private LeasedPlcConnection leasedConnection;
 
+public boolean isClosed() {
+return closed;
+}
+public PlcConnection getRawConnection() {

Review Comment:
   In contrast to your comment, this does actually return the Plc4x connection 
and not the Netty connection ... we shouldn't expose this, as this allows 
"stealing" the connection.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


chrisdutz commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1115297104


##
plc4j/spi/src/main/java/org/apache/plc4x/java/spi/messages/DefaultPlcWriteRequest.java:
##
@@ -194,6 +196,9 @@ public PlcWriteRequest build() {
 PlcTag tag = tagValues.getLeft().get();
 Object[] value = tagValues.getRight();
 PlcValue plcValue = valueHandler.newPlcValue(tag, value);
+if (tag.getPlcValueType() == PlcValueType.NULL && value!=null) 
{
+tag.setPlcValueType(plcValue.getPlcValueType());
+}

Review Comment:
   I still don't think we should have this in here ... if the tag says it's one 
type, but the PlcValue says it's something else, we shouldn't update this (I 
guess this is the reason you needed to add the "setPlcValue"). We should fire 
an exception, because in this case the user is doing something incorrectly.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


chrisdutz commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1115294277


##
plc4j/api/src/main/java/org/apache/plc4x/java/api/model/PlcTag.java:
##
@@ -63,6 +63,10 @@ default PlcValueType getPlcValueType() {
 return PlcValueType.NULL;
 }
 
+@JsonIgnore
+default void setPlcValueType(PlcValueType plcValueType){
+}

Review Comment:
   Hmm ... this adds a default implementation, which doesn't really do 
anything. Also do we usually implement all of the types in the API module, to 
create immutable types. Is there any reason for this addition? I think our tags 
should generally stay immutable types.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


chrisdutz commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1115294277


##
plc4j/api/src/main/java/org/apache/plc4x/java/api/model/PlcTag.java:
##
@@ -63,6 +63,10 @@ default PlcValueType getPlcValueType() {
 return PlcValueType.NULL;
 }
 
+@JsonIgnore
+default void setPlcValueType(PlcValueType plcValueType){
+}

Review Comment:
   Hmm ... this adds a default implementation, which doesn't really do 
anything. Also do we usually implement all of the types in the API module, to 
create immutable types. Is there any reason for this addition?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


chrisdutz commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1115292722


##
plc4j/tools/connection-cache/src/main/java/org/apache/plc4x/java/utils/cache/LeasedPlcConnection.java:
##
@@ -66,7 +76,7 @@ public void connect() throws PlcConnectionException {
 @Override
 public boolean isConnected() {
 if(connection == null) {
-throw new PlcRuntimeException("Error using leased connection after 
returning it to the cache.");
+return false;

Review Comment:
   I still don't quite understand the problem with this ... if you returned the 
connection to the pool, you should not do anything with it ... that's sort of 
the contract on using the pool. If there's some logic calling "isConnected()" 
after it has been returned, that's just a flaw in the program using it. I still 
think we should continue to throw an exception here.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


chrisdutz commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1115291492


##
plc4j/tools/connection-cache/src/main/java/org/apache/plc4x/java/utils/cache/CachedPlcConnectionManager.java:
##
@@ -81,14 +93,16 @@ public PlcConnection getConnection(String url) throws 
PlcConnectionException {
 try {
 return leaseFuture.get(this.maxWaitTime.toMillis(), 
TimeUnit.MILLISECONDS);
 } catch (ExecutionException | InterruptedException | TimeoutException 
e) {
+connectionContainer.close();
+connectionContainers.remove(url);
 throw new PlcConnectionException("Error acquiring lease for 
connection", e);
 }
 }
 
-public PlcConnection getConnection(String url, PlcAuthentication 
authentication) throws PlcConnectionException {
-throw new PlcConnectionException("the cached driver manager currently 
doesn't support authentication");

Review Comment:
   Well I'd rather simply do it right, right away ... shouldn't be too much 
additional work ... sort of like a "url + authentication.toString()"



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Translate README.md into Chinese (plc4x)

2023-02-22 Thread via GitHub 


Yeegsing closed pull request #508: Translate README.md into Chinese
URL: https://github.com/apache/plc4x/pull/508


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [plc4x-build-tools] dependabot[bot] opened a new pull request, #58: chore(deps): bump maven-assembly-plugin from 3.4.2 to 3.5.0

2023-02-22 Thread via GitHub 


dependabot[bot] opened a new pull request, #58:
URL: https://github.com/apache/plc4x-build-tools/pull/58

   Bumps 
[maven-assembly-plugin](https://github.com/apache/maven-assembly-plugin) from 
3.4.2 to 3.5.0.
   
   Commits
   
   https://github.com/apache/maven-assembly-plugin/commit/f73f48fc68d44ac2bc1ffd5e1241c427c2885522;>f73f48f
 [maven-release-plugin] prepare release maven-assembly-plugin-3.5.0
   https://github.com/apache/maven-assembly-plugin/commit/f5ccd4d5cc0de1f8a6be0347f53075b0d337f0e7;>f5ccd4d
 vuln-fix: Temporary File Information Disclosure
   https://github.com/apache/maven-assembly-plugin/commit/6b62453bdb9a2bfd6189eaac746e26df2049bea0;>6b62453
 improve javadoc
   https://github.com/apache/maven-assembly-plugin/commit/f42194b6c0e49bedad506821e524be076e839a79;>f42194b
 MASSEMBLY-941 keep file permission in Reproducible mode (https://github-redirect.dependabot.com/apache/maven-assembly-plugin/issues/96;>#96)
   https://github.com/apache/maven-assembly-plugin/commit/1336ea7989015d6976b5d409e74341ed80b2d85f;>1336ea7
 Revert [MASSEMBLY-941] keep file permission in Reproducible 
mode
   https://github.com/apache/maven-assembly-plugin/commit/b41a8eeb5f91399a330d2850917255403d567a1c;>b41a8ee
 [MASSEMBLY-941] keep file permission in Reproducible mode
   https://github.com/apache/maven-assembly-plugin/commit/1c6e07e1832903bb3e21c420943acf10a45fdf13;>1c6e07e
 update Reproducible Builds badge link
   https://github.com/apache/maven-assembly-plugin/commit/69c4f55efedceb0c7c41e1553e8c447b3d93ba9d;>69c4f55
 Fix site usage page
   https://github.com/apache/maven-assembly-plugin/commit/e01ed50f11e75f79abb2b5f2b11f3e36475201b7;>e01ed50
 [maven-release-plugin] prepare for next development iteration
   See full diff in https://github.com/apache/maven-assembly-plugin/compare/maven-assembly-plugin-3.4.2...maven-assembly-plugin-3.5.0;>compare
 view
   
   
   
   
   
   [![Dependabot compatibility 
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=org.apache.maven.plugins:maven-assembly-plugin=maven=3.4.2=3.5.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)
   
   Dependabot will resolve any conflicts with this PR as long as you don't 
alter it yourself. You can also trigger a rebase manually by commenting 
`@dependabot rebase`.
   
   [//]: # (dependabot-automerge-start)
   [//]: # (dependabot-automerge-end)
   
   ---
   
   
   Dependabot commands and options
   
   
   You can trigger Dependabot actions by commenting on this PR:
   - `@dependabot rebase` will rebase this PR
   - `@dependabot recreate` will recreate this PR, overwriting any edits that 
have been made to it
   - `@dependabot merge` will merge this PR after your CI passes on it
   - `@dependabot squash and merge` will squash and merge this PR after your CI 
passes on it
   - `@dependabot cancel merge` will cancel a previously requested merge and 
block automerging
   - `@dependabot reopen` will reopen this PR if it is closed
   - `@dependabot close` will close this PR and stop Dependabot recreating it. 
You can achieve the same result by closing it manually
   - `@dependabot ignore this major version` will close this PR and stop 
Dependabot creating any more for this major version (unless you reopen the PR 
or upgrade to it yourself)
   - `@dependabot ignore this minor version` will close this PR and stop 
Dependabot creating any more for this minor version (unless you reopen the PR 
or upgrade to it yourself)
   - `@dependabot ignore this dependency` will close this PR and stop 
Dependabot creating any more for this dependency (unless you reopen the PR or 
upgrade to it yourself)
   
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


hongjinlin commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1115166844


##
plc4j/tools/connection-cache/src/main/java/org/apache/plc4x/java/utils/cache/LeasedPlcConnection.java:
##
@@ -66,7 +76,7 @@ public void connect() throws PlcConnectionException {
 @Override
 public boolean isConnected() {
 if(connection == null) {
-throw new PlcRuntimeException("Error using leased connection after 
returning it to the cache.");
+return false;

Review Comment:
   > I have already explained that previous PR are the basis for subsequent 
PRs. Next I want to submit a PR for PL4x
   
   First of all, thank you for your PR. Secondly, I think it would be 
beneficial if you could discuss your entire design approach with everyone, 
rather than just submitting a basic PR and following up with subsequent PRs.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


hongjinlin commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1115163118


##
plc4j/tools/connection-cache/src/main/java/org/apache/plc4x/java/utils/cache/LeasedPlcConnection.java:
##
@@ -66,7 +76,7 @@ public void connect() throws PlcConnectionException {
 @Override
 public boolean isConnected() {
 if(connection == null) {
-throw new PlcRuntimeException("Error using leased connection after 
returning it to the cache.");
+return false;

Review Comment:
   > 
   
   
   
   > 
   
   I have encountered a scenario where the PLC connection appeared to be 
disconnected, but in reality it was still connected due to certain reasons. 
However, we kept trying to reconnect and as a result, the TCP estab value 
increased to tens of thousands. This caused the PLC to be disconnected and also 
resulted in server network issues. Therefore, I'm looking to see if there is a 
better solution available.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


spnettec commented on PR #818:
URL: https://github.com/apache/plc4x/pull/818#issuecomment-1440387183

   The raw conection is netty conection and if disconnected it always can
   reconnect
   
   Christofer Dutz ***@***.***> 于2023年2月22日周三 22:57写道：
   
   > ***@***. commented on this pull request.
   > --
   >
   > In
   > 
plc4j/tools/connection-cache/src/main/java/org/apache/plc4x/java/utils/cache/CachedPlcConnectionManager.java
   > :
   >
   > >  connectionContainers.put(url, connectionContainer);
   >  } else {
   >  LOG.debug("Reusing exising connection");
   > +if(connectionContainer.getRawConnection()!=null && 
!connectionContainer.getRawConnection().isConnected()){
   >
   > Also admittedly ... I'm not 100% sure if all drivers safely allow
   > reconnecting if they were somehow disconnected.
   >
   > —
   > Reply to this email directly, view it on GitHub
   > , or
   > unsubscribe
   > 

   > .
   > You are receiving this because you authored the thread.Message ID:
   > ***@***.***>
   >
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

need help with build and usage

2023-02-22 Thread Mihai DEMIAN 

Hi,
I tried to install the plc4x with the command sudo mvn -P 
with-c,with-sandbox install within the plc4x folder, where I cloned the 
repository from GitHub (if I try to start the installation in a 
different directory I get an error that the project is not found). I get 
the error message "Failed to execute goal 
org.jacoco:jacoco-maven-plugin:0.8.8:report (report) on project 
plc4j-driver-eip: An error has occurred in JaCoCo report generation.: 
Error while creating report: Unknown block type 0. -> [Help 1]".
I tried the command sudo mvn -P with-c,with-sandbox clean install. I get 
the error message "Failed to execute goal 
com.googlecode.cmake-maven-project:cmake-maven-plugin:3.23.2-b1:generate 
(cmake-generate-test-compile) on project plc4c: Return code: 1 -> [Help 1]".
I tried the command sudo mvn -P with-c clean install. The computer shut 
down...

What am I doing wrong?
I then tried the command sudo mvn install. The build succeeded. Can I 
use the "hello-world-s7.c"?

Thank you.

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


spnettec commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1114532155


##
plc4j/tools/connection-cache/src/main/java/org/apache/plc4x/java/utils/cache/LeasedPlcConnection.java:
##
@@ -66,7 +76,7 @@ public void connect() throws PlcConnectionException {
 @Override
 public boolean isConnected() {
 if(connection == null) {
-throw new PlcRuntimeException("Error using leased connection after 
returning it to the cache.");
+return false;

Review Comment:
   I have already explained that previous PR are the basis for subsequent PRs. 
Next I want to submit a PR for PL4x



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


spnettec commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1114529267


##
plc4j/spi/src/main/java/org/apache/plc4x/java/spi/messages/DefaultPlcWriteRequest.java:
##
@@ -194,6 +196,9 @@ public PlcWriteRequest build() {
 PlcTag tag = tagValues.getLeft().get();
 Object[] value = tagValues.getRight();
 PlcValue plcValue = valueHandler.newPlcValue(tag, value);
+if(tag.getPlcValueType()== PlcValueType.NULL){

Review Comment:
   You can checkout my repo and take look PL4x driver and PL4x server



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


spnettec commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1114527206


##
plc4j/spi/src/main/java/org/apache/plc4x/java/spi/messages/DefaultPlcWriteRequest.java:
##
@@ -194,6 +196,9 @@ public PlcWriteRequest build() {
 PlcTag tag = tagValues.getLeft().get();
 Object[] value = tagValues.getRight();
 PlcValue plcValue = valueHandler.newPlcValue(tag, value);
+if(tag.getPlcValueType()== PlcValueType.NULL){

Review Comment:
   By the way. The value can be set null not PlcValueType be set NULL. PLC4x 
driver write request must give explicit data type. This limits all the driver 
write tag format for the PLC4x



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


spnettec commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1114516266


##
plc4j/tools/connection-cache/src/main/java/org/apache/plc4x/java/utils/cache/LeasedPlcConnection.java:
##
@@ -66,7 +76,7 @@ public void connect() throws PlcConnectionException {
 @Override
 public boolean isConnected() {
 if(connection == null) {
-throw new PlcRuntimeException("Error using leased connection after 
returning it to the cache.");
+return false;

Review Comment:
   By the way. I'm working on eclipse kura. It require work uninterrupted. We 
must have explicit reconect signal to reconnect not a exception.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


spnettec commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1114516266


##
plc4j/tools/connection-cache/src/main/java/org/apache/plc4x/java/utils/cache/LeasedPlcConnection.java:
##
@@ -66,7 +76,7 @@ public void connect() throws PlcConnectionException {
 @Override
 public boolean isConnected() {
 if(connection == null) {
-throw new PlcRuntimeException("Error using leased connection after 
returning it to the cache.");
+return false;

Review Comment:
   By the way. I'm working on eclipse kura. It required working uninterrupted. 
We must have explicit reconect signal to reconnect not a exception.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


spnettec commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1114516266


##
plc4j/tools/connection-cache/src/main/java/org/apache/plc4x/java/utils/cache/LeasedPlcConnection.java:
##
@@ -66,7 +76,7 @@ public void connect() throws PlcConnectionException {
 @Override
 public boolean isConnected() {
 if(connection == null) {
-throw new PlcRuntimeException("Error using leased connection after 
returning it to the cache.");
+return false;

Review Comment:
   By the way. I'm working on eclipse kura. It required working uninterrupted. 
We must explicit reconect signal to reconnect not a exception.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


spnettec commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1114494692


##
plc4j/tools/connection-cache/src/main/java/org/apache/plc4x/java/utils/cache/CachedPlcConnectionManager.java:
##
@@ -81,14 +93,16 @@ public PlcConnection getConnection(String url) throws 
PlcConnectionException {
 try {
 return leaseFuture.get(this.maxWaitTime.toMillis(), 
TimeUnit.MILLISECONDS);
 } catch (ExecutionException | InterruptedException | TimeoutException 
e) {
+connectionContainer.close();
+connectionContainers.remove(url);
 throw new PlcConnectionException("Error acquiring lease for 
connection", e);
 }
 }
 
-public PlcConnection getConnection(String url, PlcAuthentication 
authentication) throws PlcConnectionException {
-throw new PlcConnectionException("the cached driver manager currently 
doesn't support authentication");

Review Comment:
   Just leave it for the future to authorize



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


spnettec commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1114492739


##
plc4j/tools/connection-cache/src/main/java/org/apache/plc4x/java/utils/cache/CachedPlcConnectionManager.java:
##
@@ -58,21 +58,33 @@ public CachedPlcConnectionManager(PlcConnectionManager 
connectionManager, Durati
 this.connectionContainers = new HashMap<>();
 }
 
+@Override
 public PlcConnection getConnection(String url) throws 
PlcConnectionException {
+return getConnection(url,null);
+}
+
+@Override
+public PlcConnection getConnection(String url, PlcAuthentication 
authentication) throws PlcConnectionException {
 ConnectionContainer connectionContainer;
 synchronized (connectionContainers) {
 connectionContainer = connectionContainers.get(url);
-if (connectionContainers.get(url) == null) {
+if (connectionContainer == null || connectionContainer.isClosed()) 
{
 LOG.debug("Creating new connection");
 
 // Establish the real connection to the plc
-PlcConnection connection = 
connectionManager.getConnection(url);
-
-// Crate a connection container to manage handling this 
connection
-connectionContainer = new ConnectionContainer(connection, 
maxLeaseTime);
+PlcConnection connection;
+if(authentication!=null) {
+connection = 
connectionManager.getConnection(url,authentication);
+} else{
+connection = connectionManager.getConnection(url);
+}
+connectionContainer = new 
ConnectionContainer(connection,maxLeaseTime);
 connectionContainers.put(url, connectionContainer);
 } else {
 LOG.debug("Reusing exising connection");
+if(connectionContainer.getRawConnection()!=null && 
!connectionContainer.getRawConnection().isConnected()){

Review Comment:
   Sometimes the row connection will die but we can't detected.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


spnettec commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1114490721


##
plc4j/tools/connection-cache/src/main/java/org/apache/plc4x/java/utils/cache/LeasedPlcConnection.java:
##
@@ -66,7 +76,7 @@ public void connect() throws PlcConnectionException {
 @Override
 public boolean isConnected() {
 if(connection == null) {
-throw new PlcRuntimeException("Error using leased connection after 
returning it to the cache.");
+return false;

Review Comment:
   If we lose the connection we can handle reconnection



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


spnettec commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1114488571


##
plc4j/tools/connection-cache/src/main/java/org/apache/plc4x/java/utils/cache/LeasedPlcConnection.java:
##
@@ -92,15 +102,139 @@ public PlcReadRequest.Builder readRequestBuilder() {
 if(connection == null) {
 throw new PlcRuntimeException("Error using leased connection after 
returning it to the cache.");
 }
-return connection.readRequestBuilder();
+final PlcReadRequest.Builder innerBuilder = 
connection.readRequestBuilder();

Review Comment:
   This is for handling  request failure and close the raw connection



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


spnettec commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1114486107


##
plc4j/spi/src/main/java/org/apache/plc4x/java/spi/messages/DefaultPlcWriteRequest.java:
##
@@ -194,6 +196,9 @@ public PlcWriteRequest build() {
 PlcTag tag = tagValues.getLeft().get();
 Object[] value = tagValues.getRight();
 PlcValue plcValue = valueHandler.newPlcValue(tag, value);
+if(tag.getPlcValueType()== PlcValueType.NULL){

Review Comment:
   This is ready for pl4x driver



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


chrisdutz commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r111171


##
plc4j/tools/connection-cache/src/main/java/org/apache/plc4x/java/utils/cache/CachedPlcConnectionManager.java:
##
@@ -58,21 +58,33 @@ public CachedPlcConnectionManager(PlcConnectionManager 
connectionManager, Durati
 this.connectionContainers = new HashMap<>();
 }
 
+@Override
 public PlcConnection getConnection(String url) throws 
PlcConnectionException {
+return getConnection(url,null);
+}
+
+@Override
+public PlcConnection getConnection(String url, PlcAuthentication 
authentication) throws PlcConnectionException {
 ConnectionContainer connectionContainer;
 synchronized (connectionContainers) {
 connectionContainer = connectionContainers.get(url);
-if (connectionContainers.get(url) == null) {
+if (connectionContainer == null || connectionContainer.isClosed()) 
{
 LOG.debug("Creating new connection");
 
 // Establish the real connection to the plc
-PlcConnection connection = 
connectionManager.getConnection(url);
-
-// Crate a connection container to manage handling this 
connection
-connectionContainer = new ConnectionContainer(connection, 
maxLeaseTime);
+PlcConnection connection;
+if(authentication!=null) {
+connection = 
connectionManager.getConnection(url,authentication);
+} else{
+connection = connectionManager.getConnection(url);
+}
+connectionContainer = new 
ConnectionContainer(connection,maxLeaseTime);
 connectionContainers.put(url, connectionContainer);
 } else {
 LOG.debug("Reusing exising connection");
+if(connectionContainer.getRawConnection()!=null && 
!connectionContainer.getRawConnection().isConnected()){

Review Comment:
   Also admittedly ... I'm not 100% sure if all drivers safely allow 
reconnecting if they were somehow disconnected.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


chrisdutz commented on code in PR #818:
URL: https://github.com/apache/plc4x/pull/818#discussion_r1114371581


##
plc4j/tools/connection-cache/src/main/java/org/apache/plc4x/java/utils/cache/CachedPlcConnectionManager.java:
##
@@ -81,14 +93,16 @@ public PlcConnection getConnection(String url) throws 
PlcConnectionException {
 try {
 return leaseFuture.get(this.maxWaitTime.toMillis(), 
TimeUnit.MILLISECONDS);
 } catch (ExecutionException | InterruptedException | TimeoutException 
e) {
+connectionContainer.close();
+connectionContainers.remove(url);
 throw new PlcConnectionException("Error acquiring lease for 
connection", e);
 }
 }
 
-public PlcConnection getConnection(String url, PlcAuthentication 
authentication) throws PlcConnectionException {
-throw new PlcConnectionException("the cached driver manager currently 
doesn't support authentication");

Review Comment:
   I intentionally left away the autherntication as in general we couldn't have 
two connections to the same plc with different authentication ... if we do 
this, we should probably also add the authentication information to the key of 
the map.



##
plc4j/tools/connection-cache/src/main/java/org/apache/plc4x/java/utils/cache/LeasedPlcConnection.java:
##
@@ -92,15 +102,139 @@ public PlcReadRequest.Builder readRequestBuilder() {
 if(connection == null) {
 throw new PlcRuntimeException("Error using leased connection after 
returning it to the cache.");
 }
-return connection.readRequestBuilder();
+final PlcReadRequest.Builder innerBuilder = 
connection.readRequestBuilder();

Review Comment:
   Could you please explain what this is needed for?



##
plc4j/tools/connection-cache/src/main/java/org/apache/plc4x/java/utils/cache/CachedPlcConnectionManager.java:
##
@@ -58,21 +58,33 @@ public CachedPlcConnectionManager(PlcConnectionManager 
connectionManager, Durati
 this.connectionContainers = new HashMap<>();
 }
 
+@Override
 public PlcConnection getConnection(String url) throws 
PlcConnectionException {
+return getConnection(url,null);
+}
+
+@Override
+public PlcConnection getConnection(String url, PlcAuthentication 
authentication) throws PlcConnectionException {
 ConnectionContainer connectionContainer;
 synchronized (connectionContainers) {
 connectionContainer = connectionContainers.get(url);
-if (connectionContainers.get(url) == null) {
+if (connectionContainer == null || connectionContainer.isClosed()) 
{
 LOG.debug("Creating new connection");
 
 // Establish the real connection to the plc
-PlcConnection connection = 
connectionManager.getConnection(url);
-
-// Crate a connection container to manage handling this 
connection
-connectionContainer = new ConnectionContainer(connection, 
maxLeaseTime);
+PlcConnection connection;
+if(authentication!=null) {
+connection = 
connectionManager.getConnection(url,authentication);
+} else{
+connection = connectionManager.getConnection(url);
+}
+connectionContainer = new 
ConnectionContainer(connection,maxLeaseTime);
 connectionContainers.put(url, connectionContainer);
 } else {
 LOG.debug("Reusing exising connection");
+if(connectionContainer.getRawConnection()!=null && 
!connectionContainer.getRawConnection().isConnected()){

Review Comment:
   Instead of exposing the raw connection, I think it might be better to create 
a new container and not expose the inner connection.



##
plc4j/tools/connection-cache/src/main/java/org/apache/plc4x/java/utils/cache/LeasedPlcConnection.java:
##
@@ -66,7 +76,7 @@ public void connect() throws PlcConnectionException {
 @Override
 public boolean isConnected() {
 if(connection == null) {
-throw new PlcRuntimeException("Error using leased connection after 
returning it to the cache.");
+return false;

Review Comment:
   Not quite sure this is the best way to do this. Because for me there's a 
difference, if we're not connected or if we already gave back the handle. The 
way I implemented it gives the user the explicit feedback, that he's doing 
something bad. If we return false, he might think he just needs to re-connect.



##
plc4j/spi/src/main/java/org/apache/plc4x/java/spi/messages/DefaultPlcWriteRequest.java:
##
@@ -194,6 +196,9 @@ public PlcWriteRequest build() {
 PlcTag tag = tagValues.getLeft().get();
 Object[] value = tagValues.getRight();
 PlcValue plcValue = valueHandler.newPlcValue(tag, value);
+

Re: Issue with CachedPlcConnectionManager in nifi integration

2023-02-22 Thread Łukasz Dywicki 
I think for that case we have connection state listener which might be 
help with such cases.


Best,
Łukasz

On 22.02.2023 11:58, youlin he wrote:

I have restored maxUserTime.

Sometimes we must hold the connection for a long time. For example, in the
loop retrieve PLC data.

If request timeout the base drive will throw a timeoutException and
cacheManage will close this real connection and set the isConnected of the
leased connection as false.
So we can determine whether the connection is closed in the loop and
automatically connect if it is closed.

If you can merge my New PR. We should remove maxUserTime again.

Christofer Dutz  于2023年2月22日周三 18:40写道：


Could you please summarize which changes these were exactly? Removing the
maxUserTime? Because if this is the case, we probably need to have a look
at how you are using the ConnectionCache.

Chris


From: Unai Leria 
Date: Wednesday, 22. February 2023 at 10:34
To: dev 
Subject: Re: Issue with CachedPlcConnectionManager in nifi integration
For the nifi integration to work I did only use the changes on
plc4x/plc4j/tools/connection-cache from spnettec/plc4x/tree/heyoulin.
Plus some minor changes in the nifi integration to add a timeout to all
processors.

Unai

- Mensaje original -
De: "youlin he" 
Para: "dev" , "Christofer Dutz" <
christofer.d...@c-ware.de>
Enviados: Miércoles, 22 de Febrero 2023 10:21:51
Asunto: Re: Issue with CachedPlcConnectionManager in nifi integration

But it should be based on driver base revision. Otherwise network
connection breaks would still happen.
@Christofer Dutz 

youlin he  于2023年2月22日周三 17:16写道：


Ok. I will create a PR

Christofer Dutz  于2023年2月22日周三 16:44写道：


Unfortunately, this is not a branch, but a fork … Would you be able to
pull the changes that made it work in a separate PR? Then we can more
quickly adopt the changes, because we definitely are not going to merge
that PR (At least I’m not going to do it)

Chris


From: Unai Leria 
Date: Wednesday, 22. February 2023 at 09:28
To: dev 
Subject: Re: Issue with CachedPlcConnectionManager in nifi integration
It does work in the heyoulin branch. Thanks you.

Unai

- Mensaje original -
De: "youlin he" 
Para: "dev" 
Enviados: Martes, 21 de Febrero 2023 14:54:14
Asunto: Re: Issue with CachedPlcConnectionManager in nifi integration

I removed the maxUseTime. I think it is unnecessary to add

request-timeout

feature in the next version.

youlin he  于2023年2月21日周二 21:48写道：


Can you test use https://github.com/spnettec/plc4x  heyoulin branch.

I

fixed this problem.

Unai Leria  于2023年2月21日周二 21:17写道：


Hi,

I've been working with the CachedPlcConnectionManager on the NiFi
integration and I have encountered a problem while looking at issue [
https://github.com/apache/plc4x/issues/623 | #623 ] :
When a successful connection is already stored in the cache and the
network connection breaks the connections in the cache are no longer
usable, but I have not been able to remove them.
This makes the processor not work until it is manually disabled and
enabled.

For the NiFi integration to work properly there should be a way of
removing a connection from the cache if it is invalid.

I would appreciate some guidance.

Unai

Re: [PR] Various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


spnettec closed pull request #812: Various Changes for #1
URL: https://github.com/apache/plc4x/pull/812


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: Issue with CachedPlcConnectionManager in nifi integration

2023-02-22 Thread youlin he 
@Override
public PlcReadRequest.Builder readRequestBuilder() {
if(connection == null) {
throw new PlcRuntimeException("Error using leased connection
after returning it to the cache.");
}
final PlcReadRequest.Builder innerBuilder = connection.readRequestBuilder();
return new PlcReadRequest.Builder(){

@Override
public PlcReadRequest build() {
final PlcReadRequest innerPlcReadRequest = innerBuilder.build();
return new PlcReadRequest(){

@Override
public CompletableFuture execute() {
CompletableFuture
future = innerPlcReadRequest.execute();
final CompletableFuture
responseFuture = new CompletableFuture<>();
future.handle((plcReadResponse, throwable) -> {
if (plcReadResponse != null) {
responseFuture.complete(plcReadResponse);
} else {

 *// Sometime request have some
problem maybe the connection died. *

*We must close the real connection.*
try {
destroy();

 *// In here auto re-connect?  I mean
lease a connection*
} catch (Exception e) {
}
responseFuture.completeExceptionally(throwable);
}
return null;
});
return responseFuture;
}

@Override
public int getNumberOfTags() {
return innerPlcReadRequest.getNumberOfTags();
}

@Override
public LinkedHashSet getTagNames() {
return innerPlcReadRequest.getTagNames();
}

@Override
public PlcTag getTag(String name) {
return innerPlcReadRequest.getTag(name);
}

@Override
public List getTags() {
return innerPlcReadRequest.getTags();
}
};
}

@Override
public PlcReadRequest.Builder addTagAddress(String name,
String tagAddress) {
return innerBuilder.addTagAddress(name, tagAddress);
}

@Override
public PlcReadRequest.Builder addTag(String name, PlcTag tag) {
return innerBuilder.addTag(name,tag);
}
};
}


youlin he  于2023年2月22日周三 18:58写道：

> I have restored maxUserTime.
>
> Sometimes we must hold the connection for a long time. For example, in the
> loop retrieve PLC data.
>
> If request timeout the base drive will throw a timeoutException and
> cacheManage will close this real connection and set the isConnected of the
> leased connection as false.
> So we can determine whether the connection is closed in the loop and
> automatically connect if it is closed.
>
> If you can merge my New PR. We should remove maxUserTime again.
>
> Christofer Dutz  于2023年2月22日周三 18:40写道：
>
>> Could you please summarize which changes these were exactly? Removing the
>> maxUserTime? Because if this is the case, we probably need to have a look
>> at how you are using the ConnectionCache.
>>
>> Chris
>>
>>
>> From: Unai Leria 
>> Date: Wednesday, 22. February 2023 at 10:34
>> To: dev 
>> Subject: Re: Issue with CachedPlcConnectionManager in nifi integration
>> For the nifi integration to work I did only use the changes on
>> plc4x/plc4j/tools/connection-cache from spnettec/plc4x/tree/heyoulin.
>> Plus some minor changes in the nifi integration to add a timeout to all
>> processors.
>>
>> Unai
>>
>> - Mensaje original -
>> De: "youlin he" 
>> Para: "dev" , "Christofer Dutz" <
>> christofer.d...@c-ware.de>
>> Enviados: Miércoles, 22 de Febrero 2023 10:21:51
>> Asunto: Re: Issue with CachedPlcConnectionManager in nifi integration
>>
>> But it should be based on driver base revision. Otherwise network
>> connection breaks would still happen.
>> @Christofer Dutz 
>>
>> youlin he  于2023年2月22日周三 17:16写道：
>>
>> > Ok. I will create a PR
>> >
>> > Christofer Dutz  于2023年2月22日周三 16:44写道：
>> >
>> >> Unfortunately, this is not a branch, but a fork … Would you be able to
>> >> pull the changes that made it work in a separate PR? Then we can more
>> >> quickly adopt the changes, because we definitely are not going to merge
>> >> that PR (At least I’m not going to do it)
>> >>
>> >> Chris
>> >>
>> >>
>> >> From: Unai Leria 
>> >> Date: Wednesday, 22. February 2023 at 09:28
>> >> To: dev 
>> >> Subject: Re: Issue with CachedPlcConnectionManager in nifi integration
>> >> It does work in the heyoulin branch. Thanks you.
>> >>
>> >> Unai
>> >>
>> >> - Mensaje original -
>> >> De: "youlin he" 
>> >> Para: "dev" 
>> >> Enviados: Martes, 21 de Febrero 2023 14:54:14
>> >> Asunto: Re: Issue

Re: Issue with CachedPlcConnectionManager in nifi integration

2023-02-22 Thread youlin he 
I have restored maxUserTime.

Sometimes we must hold the connection for a long time. For example, in the
loop retrieve PLC data.

If request timeout the base drive will throw a timeoutException and
cacheManage will close this real connection and set the isConnected of the
leased connection as false.
So we can determine whether the connection is closed in the loop and
automatically connect if it is closed.

If you can merge my New PR. We should remove maxUserTime again.

Christofer Dutz  于2023年2月22日周三 18:40写道：

> Could you please summarize which changes these were exactly? Removing the
> maxUserTime? Because if this is the case, we probably need to have a look
> at how you are using the ConnectionCache.
>
> Chris
>
>
> From: Unai Leria 
> Date: Wednesday, 22. February 2023 at 10:34
> To: dev 
> Subject: Re: Issue with CachedPlcConnectionManager in nifi integration
> For the nifi integration to work I did only use the changes on
> plc4x/plc4j/tools/connection-cache from spnettec/plc4x/tree/heyoulin.
> Plus some minor changes in the nifi integration to add a timeout to all
> processors.
>
> Unai
>
> - Mensaje original -
> De: "youlin he" 
> Para: "dev" , "Christofer Dutz" <
> christofer.d...@c-ware.de>
> Enviados: Miércoles, 22 de Febrero 2023 10:21:51
> Asunto: Re: Issue with CachedPlcConnectionManager in nifi integration
>
> But it should be based on driver base revision. Otherwise network
> connection breaks would still happen.
> @Christofer Dutz 
>
> youlin he  于2023年2月22日周三 17:16写道：
>
> > Ok. I will create a PR
> >
> > Christofer Dutz  于2023年2月22日周三 16:44写道：
> >
> >> Unfortunately, this is not a branch, but a fork … Would you be able to
> >> pull the changes that made it work in a separate PR? Then we can more
> >> quickly adopt the changes, because we definitely are not going to merge
> >> that PR (At least I’m not going to do it)
> >>
> >> Chris
> >>
> >>
> >> From: Unai Leria 
> >> Date: Wednesday, 22. February 2023 at 09:28
> >> To: dev 
> >> Subject: Re: Issue with CachedPlcConnectionManager in nifi integration
> >> It does work in the heyoulin branch. Thanks you.
> >>
> >> Unai
> >>
> >> - Mensaje original -
> >> De: "youlin he" 
> >> Para: "dev" 
> >> Enviados: Martes, 21 de Febrero 2023 14:54:14
> >> Asunto: Re: Issue with CachedPlcConnectionManager in nifi integration
> >>
> >> I removed the maxUseTime. I think it is unnecessary to add
> request-timeout
> >> feature in the next version.
> >>
> >> youlin he  于2023年2月21日周二 21:48写道：
> >>
> >> > Can you test use https://github.com/spnettec/plc4x  heyoulin branch.
> I
> >> > fixed this problem.
> >> >
> >> > Unai Leria  于2023年2月21日周二 21:17写道：
> >> >
> >> >> Hi,
> >> >>
> >> >> I've been working with the CachedPlcConnectionManager on the NiFi
> >> >> integration and I have encountered a problem while looking at issue [
> >> >> https://github.com/apache/plc4x/issues/623 | #623 ] :
> >> >> When a successful connection is already stored in the cache and the
> >> >> network connection breaks the connections in the cache are no longer
> >> >> usable, but I have not been able to remove them.
> >> >> This makes the processor not work until it is manually disabled and
> >> >> enabled.
> >> >>
> >> >> For the NiFi integration to work properly there should be a way of
> >> >> removing a connection from the cache if it is invalid.
> >> >>
> >> >> I would appreciate some guidance.
> >> >>
> >> >> Unai
> >> >>
> >> >
> >>
> >
>

Re: Issue with CachedPlcConnectionManager in nifi integration

2023-02-22 Thread Unai Leria 
The only change I did in the nifi integration is to add a timeout in the the 
CompletableFutures of write/read requests.
Not sure what the maxUseTime changes do.

Unai

- Mensaje original -
De: "Christofer Dutz" 
Para: "dev" 
Enviados: Miércoles, 22 de Febrero 2023 11:40:23
Asunto: Re: Issue with CachedPlcConnectionManager in nifi integration

Could you please summarize which changes these were exactly? Removing the 
maxUserTime? Because if this is the case, we probably need to have a look at 
how you are using the ConnectionCache.

Chris

Re: [PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


chrisdutz commented on PR #818:
URL: https://github.com/apache/plc4x/pull/818#issuecomment-1439797734

   YESS! ... that's much more something we can discuss ... I'll do that as soon 
as I can (Currently not allowed to do Apache code-work while on the clock :-( ) 
... but just wanted to say that it's greatly appreciated :-)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: Issue with CachedPlcConnectionManager in nifi integration

2023-02-22 Thread Christofer Dutz 
Could you please summarize which changes these were exactly? Removing the 
maxUserTime? Because if this is the case, we probably need to have a look at 
how you are using the ConnectionCache.

Chris


From: Unai Leria 
Date: Wednesday, 22. February 2023 at 10:34
To: dev 
Subject: Re: Issue with CachedPlcConnectionManager in nifi integration
For the nifi integration to work I did only use the changes on 
plc4x/plc4j/tools/connection-cache from spnettec/plc4x/tree/heyoulin.
Plus some minor changes in the nifi integration to add a timeout to all 
processors.

Unai

- Mensaje original -
De: "youlin he" 
Para: "dev" , "Christofer Dutz" 

Enviados: Miércoles, 22 de Febrero 2023 10:21:51
Asunto: Re: Issue with CachedPlcConnectionManager in nifi integration

But it should be based on driver base revision. Otherwise network
connection breaks would still happen.
@Christofer Dutz 

youlin he  于2023年2月22日周三 17:16写道：

> Ok. I will create a PR
>
> Christofer Dutz  于2023年2月22日周三 16:44写道：
>
>> Unfortunately, this is not a branch, but a fork … Would you be able to
>> pull the changes that made it work in a separate PR? Then we can more
>> quickly adopt the changes, because we definitely are not going to merge
>> that PR (At least I’m not going to do it)
>>
>> Chris
>>
>>
>> From: Unai Leria 
>> Date: Wednesday, 22. February 2023 at 09:28
>> To: dev 
>> Subject: Re: Issue with CachedPlcConnectionManager in nifi integration
>> It does work in the heyoulin branch. Thanks you.
>>
>> Unai
>>
>> - Mensaje original -
>> De: "youlin he" 
>> Para: "dev" 
>> Enviados: Martes, 21 de Febrero 2023 14:54:14
>> Asunto: Re: Issue with CachedPlcConnectionManager in nifi integration
>>
>> I removed the maxUseTime. I think it is unnecessary to add request-timeout
>> feature in the next version.
>>
>> youlin he  于2023年2月21日周二 21:48写道：
>>
>> > Can you test use https://github.com/spnettec/plc4x  heyoulin branch. I
>> > fixed this problem.
>> >
>> > Unai Leria  于2023年2月21日周二 21:17写道：
>> >
>> >> Hi,
>> >>
>> >> I've been working with the CachedPlcConnectionManager on the NiFi
>> >> integration and I have encountered a problem while looking at issue [
>> >> https://github.com/apache/plc4x/issues/623 | #623 ] :
>> >> When a successful connection is already stored in the cache and the
>> >> network connection breaks the connections in the cache are no longer
>> >> usable, but I have not been able to remove them.
>> >> This makes the processor not work until it is manually disabled and
>> >> enabled.
>> >>
>> >> For the NiFi integration to work properly there should be a way of
>> >> removing a connection from the cache if it is invalid.
>> >>
>> >> I would appreciate some guidance.
>> >>
>> >> Unai
>> >>
>> >
>>
>

Re: [PR] Various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


spnettec commented on PR #812:
URL: https://github.com/apache/plc4x/pull/812#issuecomment-1439764820

   I will close this PR


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[PR] New various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


spnettec opened a new pull request, #818:
URL: https://github.com/apache/plc4x/pull/818

   fix OutOfMemoryError
   add Timeout-Handling
   fix connection-cache


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: Issue with CachedPlcConnectionManager in nifi integration

2023-02-22 Thread Unai Leria 
For the nifi integration to work I did only use the changes on 
plc4x/plc4j/tools/connection-cache from spnettec/plc4x/tree/heyoulin.
Plus some minor changes in the nifi integration to add a timeout to all 
processors.

Unai

- Mensaje original -
De: "youlin he" 
Para: "dev" , "Christofer Dutz" 

Enviados: Miércoles, 22 de Febrero 2023 10:21:51
Asunto: Re: Issue with CachedPlcConnectionManager in nifi integration

But it should be based on driver base revision. Otherwise network
connection breaks would still happen.
@Christofer Dutz 

youlin he  于2023年2月22日周三 17:16写道：

> Ok. I will create a PR
>
> Christofer Dutz  于2023年2月22日周三 16:44写道：
>
>> Unfortunately, this is not a branch, but a fork … Would you be able to
>> pull the changes that made it work in a separate PR? Then we can more
>> quickly adopt the changes, because we definitely are not going to merge
>> that PR (At least I’m not going to do it)
>>
>> Chris
>>
>>
>> From: Unai Leria 
>> Date: Wednesday, 22. February 2023 at 09:28
>> To: dev 
>> Subject: Re: Issue with CachedPlcConnectionManager in nifi integration
>> It does work in the heyoulin branch. Thanks you.
>>
>> Unai
>>
>> - Mensaje original -
>> De: "youlin he" 
>> Para: "dev" 
>> Enviados: Martes, 21 de Febrero 2023 14:54:14
>> Asunto: Re: Issue with CachedPlcConnectionManager in nifi integration
>>
>> I removed the maxUseTime. I think it is unnecessary to add request-timeout
>> feature in the next version.
>>
>> youlin he  于2023年2月21日周二 21:48写道：
>>
>> > Can you test use https://github.com/spnettec/plc4x  heyoulin branch. I
>> > fixed this problem.
>> >
>> > Unai Leria  于2023年2月21日周二 21:17写道：
>> >
>> >> Hi,
>> >>
>> >> I've been working with the CachedPlcConnectionManager on the NiFi
>> >> integration and I have encountered a problem while looking at issue [
>> >> https://github.com/apache/plc4x/issues/623 | #623 ] :
>> >> When a successful connection is already stored in the cache and the
>> >> network connection breaks the connections in the cache are no longer
>> >> usable, but I have not been able to remove them.
>> >> This makes the processor not work until it is manually disabled and
>> >> enabled.
>> >>
>> >> For the NiFi integration to work properly there should be a way of
>> >> removing a connection from the cache if it is invalid.
>> >>
>> >> I would appreciate some guidance.
>> >>
>> >> Unai
>> >>
>> >
>>
>

Re: Issue with CachedPlcConnectionManager in nifi integration

2023-02-22 Thread youlin he 
But it should be based on driver base revision. Otherwise network
connection breaks would still happen.
@Christofer Dutz 

youlin he  于2023年2月22日周三 17:16写道：

> Ok. I will create a PR
>
> Christofer Dutz  于2023年2月22日周三 16:44写道：
>
>> Unfortunately, this is not a branch, but a fork … Would you be able to
>> pull the changes that made it work in a separate PR? Then we can more
>> quickly adopt the changes, because we definitely are not going to merge
>> that PR (At least I’m not going to do it)
>>
>> Chris
>>
>>
>> From: Unai Leria 
>> Date: Wednesday, 22. February 2023 at 09:28
>> To: dev 
>> Subject: Re: Issue with CachedPlcConnectionManager in nifi integration
>> It does work in the heyoulin branch. Thanks you.
>>
>> Unai
>>
>> - Mensaje original -
>> De: "youlin he" 
>> Para: "dev" 
>> Enviados: Martes, 21 de Febrero 2023 14:54:14
>> Asunto: Re: Issue with CachedPlcConnectionManager in nifi integration
>>
>> I removed the maxUseTime. I think it is unnecessary to add request-timeout
>> feature in the next version.
>>
>> youlin he  于2023年2月21日周二 21:48写道：
>>
>> > Can you test use https://github.com/spnettec/plc4x  heyoulin branch. I
>> > fixed this problem.
>> >
>> > Unai Leria  于2023年2月21日周二 21:17写道：
>> >
>> >> Hi,
>> >>
>> >> I've been working with the CachedPlcConnectionManager on the NiFi
>> >> integration and I have encountered a problem while looking at issue [
>> >> https://github.com/apache/plc4x/issues/623 | #623 ] :
>> >> When a successful connection is already stored in the cache and the
>> >> network connection breaks the connections in the cache are no longer
>> >> usable, but I have not been able to remove them.
>> >> This makes the processor not work until it is manually disabled and
>> >> enabled.
>> >>
>> >> For the NiFi integration to work properly there should be a way of
>> >> removing a connection from the cache if it is invalid.
>> >>
>> >> I would appreciate some guidance.
>> >>
>> >> Unai
>> >>
>> >
>>
>

Re: [PR] Various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


ottlukas commented on PR #812:
URL: https://github.com/apache/plc4x/pull/812#issuecomment-1439678584

   > I saw the discuss. I know the problem. But i think we should resolve user 
urgent problem. We can change that at next version. For me the additional 
encoding for string tags is important now.
   
   There is a balance between you specific urgent problem and a stable plc4x 
framework / open source in a foundation like the ASF. If the refactoring of 
code is extensive and can not forseen it is against the basic principle of 
Community over Code. If you can convince the community on the mailing list to 
accept your PR then it would work. To be realistic the changes are too big 
and it would be hard to get a majority with this amount of changes. So we like 
to pull in you really nice changes in a slower pace and decouple it by looking 
first and the non-breaking elements and for the string topic you already saw 
the dicussion on the mailing list. I hope that helps you to understand a little 
better as we are aiming to have a community around the code and a stable 
framework where developers are comfortable on supporting and evolving PLC4X.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: Issue with CachedPlcConnectionManager in nifi integration

2023-02-22 Thread youlin he 
Ok. I will create a PR

Christofer Dutz  于2023年2月22日周三 16:44写道：

> Unfortunately, this is not a branch, but a fork … Would you be able to
> pull the changes that made it work in a separate PR? Then we can more
> quickly adopt the changes, because we definitely are not going to merge
> that PR (At least I’m not going to do it)
>
> Chris
>
>
> From: Unai Leria 
> Date: Wednesday, 22. February 2023 at 09:28
> To: dev 
> Subject: Re: Issue with CachedPlcConnectionManager in nifi integration
> It does work in the heyoulin branch. Thanks you.
>
> Unai
>
> - Mensaje original -
> De: "youlin he" 
> Para: "dev" 
> Enviados: Martes, 21 de Febrero 2023 14:54:14
> Asunto: Re: Issue with CachedPlcConnectionManager in nifi integration
>
> I removed the maxUseTime. I think it is unnecessary to add request-timeout
> feature in the next version.
>
> youlin he  于2023年2月21日周二 21:48写道：
>
> > Can you test use https://github.com/spnettec/plc4x  heyoulin branch. I
> > fixed this problem.
> >
> > Unai Leria  于2023年2月21日周二 21:17写道：
> >
> >> Hi,
> >>
> >> I've been working with the CachedPlcConnectionManager on the NiFi
> >> integration and I have encountered a problem while looking at issue [
> >> https://github.com/apache/plc4x/issues/623 | #623 ] :
> >> When a successful connection is already stored in the cache and the
> >> network connection breaks the connections in the cache are no longer
> >> usable, but I have not been able to remove them.
> >> This makes the processor not work until it is manually disabled and
> >> enabled.
> >>
> >> For the NiFi integration to work properly there should be a way of
> >> removing a connection from the cache if it is invalid.
> >>
> >> I would appreciate some guidance.
> >>
> >> Unai
> >>
> >
>

Re: Issue with CachedPlcConnectionManager in nifi integration

2023-02-22 Thread Christofer Dutz 
Unfortunately, this is not a branch, but a fork … Would you be able to pull the 
changes that made it work in a separate PR? Then we can more quickly adopt the 
changes, because we definitely are not going to merge that PR (At least I’m not 
going to do it)

Chris


From: Unai Leria 
Date: Wednesday, 22. February 2023 at 09:28
To: dev 
Subject: Re: Issue with CachedPlcConnectionManager in nifi integration
It does work in the heyoulin branch. Thanks you.

Unai

- Mensaje original -
De: "youlin he" 
Para: "dev" 
Enviados: Martes, 21 de Febrero 2023 14:54:14
Asunto: Re: Issue with CachedPlcConnectionManager in nifi integration

I removed the maxUseTime. I think it is unnecessary to add request-timeout
feature in the next version.

youlin he  于2023年2月21日周二 21:48写道：

> Can you test use https://github.com/spnettec/plc4x  heyoulin branch. I
> fixed this problem.
>
> Unai Leria  于2023年2月21日周二 21:17写道：
>
>> Hi,
>>
>> I've been working with the CachedPlcConnectionManager on the NiFi
>> integration and I have encountered a problem while looking at issue [
>> https://github.com/apache/plc4x/issues/623 | #623 ] :
>> When a successful connection is already stored in the cache and the
>> network connection breaks the connections in the cache are no longer
>> usable, but I have not been able to remove them.
>> This makes the processor not work until it is manually disabled and
>> enabled.
>>
>> For the NiFi integration to work properly there should be a way of
>> removing a connection from the cache if it is invalid.
>>
>> I would appreciate some guidance.
>>
>> Unai
>>
>

Re: [PR] Various Changes for #1 (plc4x)

2023-02-22 Thread via GitHub 


chrisdutz commented on PR #812:
URL: https://github.com/apache/plc4x/pull/812#issuecomment-1439629013

   Well the thing is: I'm currently working most of my time, cleaning up 
things. So I will definitely not approve an approach like this. If the others 
do, they will have to do the house-keeping for it. I know how stuff like this 
happened in the past ... stuff was merged with the promise to clean up later, 
but then nothing happened and I'm currently working on cleaning it up.
   
   This is not the way Open-Source projects (at Apache work) ... we don't like 
huge code-drops like this. Even if the amount of code isn't that huge, but the 
number of changes are. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: Issue with CachedPlcConnectionManager in nifi integration

2023-02-22 Thread Unai Leria 
It does work in the heyoulin branch. Thanks you.

Unai

- Mensaje original -
De: "youlin he" 
Para: "dev" 
Enviados: Martes, 21 de Febrero 2023 14:54:14
Asunto: Re: Issue with CachedPlcConnectionManager in nifi integration

I removed the maxUseTime. I think it is unnecessary to add request-timeout
feature in the next version.

youlin he  于2023年2月21日周二 21:48写道：

> Can you test use https://github.com/spnettec/plc4x  heyoulin branch. I
> fixed this problem.
>
> Unai Leria  于2023年2月21日周二 21:17写道：
>
>> Hi,
>>
>> I've been working with the CachedPlcConnectionManager on the NiFi
>> integration and I have encountered a problem while looking at issue [
>> https://github.com/apache/plc4x/issues/623 | #623 ] :
>> When a successful connection is already stored in the cache and the
>> network connection breaks the connections in the cache are no longer
>> usable, but I have not been able to remove them.
>> This makes the processor not work until it is manually disabled and
>> enabled.
>>
>> For the NiFi integration to work properly there should be a way of
>> removing a connection from the cache if it is invalid.
>>
>> I would appreciate some guidance.
>>
>> Unai
>>
>

Re: [PR] Feature/nifi integration sink record processor and minor fixes (plc4x)

2023-02-22 Thread via GitHub 


QuanticPony commented on code in PR #809:
URL: https://github.com/apache/plc4x/pull/809#discussion_r1113982008


##
plc4j/integrations/apache-nifi/nifi-plc4x-processors/src/main/java/org/apache/plc4x/nifi/Plc4xSinkRecordProcessor.java:
##
@@ -0,0 +1,229 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied.  See the License for the
+ specific language governing permissions and limitations
+ under the License.
+ */
+package org.apache.plc4x.nifi;
+
+import java.io.InputStream;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
+
+import org.apache.nifi.annotation.behavior.InputRequirement;
+import org.apache.nifi.annotation.behavior.TriggerSerially;
+import org.apache.nifi.annotation.behavior.WritesAttribute;
+import org.apache.nifi.annotation.behavior.WritesAttributes;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.flowfile.FlowFile;
+import org.apache.nifi.logging.ComponentLog;
+import org.apache.nifi.processor.ProcessContext;
+import org.apache.nifi.processor.ProcessSession;
+import org.apache.nifi.processor.ProcessorInitializationContext;
+import org.apache.nifi.processor.Relationship;
+import org.apache.nifi.processor.exception.ProcessException;
+import org.apache.nifi.processor.util.StandardValidators;
+import org.apache.nifi.serialization.RecordReader;
+import org.apache.nifi.serialization.RecordReaderFactory;
+import org.apache.nifi.serialization.record.Record;
+import org.apache.nifi.util.StopWatch;
+import org.apache.plc4x.java.api.PlcConnection;
+import org.apache.plc4x.java.api.messages.PlcWriteRequest;
+import org.apache.plc4x.java.api.messages.PlcWriteResponse;
+import org.apache.plc4x.java.api.model.PlcTag;
+import org.apache.plc4x.java.api.types.PlcResponseCode;
+
+@TriggerSerially
+@Tags({"plc4x", "put", "sink", "record"})
+@InputRequirement(InputRequirement.Requirement.INPUT_REQUIRED)
+@CapabilityDescription("Processor able to write data to industrial PLCs using 
Apache PLC4X")
+@WritesAttributes({ @WritesAttribute(attribute = "value", description = "some 
value") })
+public class Plc4xSinkRecordProcessor extends BasePlc4xProcessor {
+
+   public static final String RESULT_ROW_COUNT = "plc4x.write.row.count";
+   public static final String RESULT_QUERY_DURATION = 
"plc4x.write.query.duration";
+   public static final String RESULT_QUERY_EXECUTION_TIME = 
"plc4x.write.query.executiontime";
+   public static final String RESULT_QUERY_FETCH_TIME = 
"plc4x.write.query.fetchtime";
+   public static final String INPUT_FLOWFILE_UUID = "input.flowfile.uuid";
+   public static final String RESULT_ERROR_MESSAGE = 
"plc4x.write.error.message";
+   
+   public static final PropertyDescriptor PLC_RECORD_READER_FACTORY = new 
PropertyDescriptor.Builder()
+   .name("record-reader").displayName("Record Reader")
+   .description(
+   "Specifies the Controller Service to 
use for reading record from a FlowFile. The Record Reader may use Inherit 
Schema to emulate the inferred schema behavior, i.e. "
+   + "an explicit schema 
need not be defined in the reader, and will be supplied by the same logic used 
to infer the schema from the column types.")
+   .identifiesControllerService(RecordReaderFactory.class)
+   .required(true)
+   .build();
+
+   public static final PropertyDescriptor 
PLC_WRITE_FUTURE_TIMEOUT_MILISECONDS = new 
PropertyDescriptor.Builder().name("plc4x-record-write-timeout").displayName("Write
 timeout (miliseconds)")
+   .description("Write timeout in miliseconds")
+   .defaultValue("1")
+   .required(true)
+   
.addValidator(StandardValidators.POSITIVE_INTEGER_VALIDATOR)
+   .build();
+   
+
+   public Plc4xSinkRecordProcessor() {
+   }
+
+

Re: [PR] Feature/nifi integration sink record processor and minor fixes (plc4x)

2023-02-22 Thread via GitHub 


QuanticPony commented on code in PR #809:
URL: https://github.com/apache/plc4x/pull/809#discussion_r1113977993


##
plc4j/integrations/apache-nifi/nifi-plc4x-processors/src/main/java/org/apache/plc4x/nifi/Plc4xSinkProcessor.java:
##
@@ -64,12 +67,13 @@ public void onTrigger(final ProcessContext context, final 
ProcessSession session
 
 if (tags != null){
 for (Map.Entry tag : tags.entrySet()){
-builder.addTag(tag.getKey(), tag.getValue());
+builder.addTag(tag.getKey(), tag.getValue(), 
flowFile.getAttribute(tag.getKey()));

Review Comment:
   This flowfile's attribute is the value we need to write into the Plc. I 
forgot including this on the previous PR.
   I have added a check to see if it is present. Similar to 
Plc4xSinkRecordProcessor and record fields.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org