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


##########
plc4j/drivers/eip/src/main/java/org/apache/plc4x/java/eip/base/EipTcpConnection.java:
##########
@@ -249,16 +249,13 @@ private CompletableFuture<Void> probeAttributes() {
         CipRRData eipWrapper = new CipRRData(sessionHandle, 
CIPStatus.Success.getValue(),
             DEFAULT_SENDER_CONTEXT, 0L, EMPTY_INTERFACE_HANDLE, 0, typeIds);
 
-        return sendRequest(eipWrapper).thenAccept(response -> {
-            if (!(response instanceof CipRRData rr) || rr.getStatus() != 
CIPStatus.Success.getValue()) {
-                return;
-            }
-            UnConnectedDataItem dataItem = (UnConnectedDataItem) 
rr.getTypeIds().get(1);
-            if (!(dataItem.getService() instanceof GetAttributeAllResponse 
gar)) {
-                return;
+        return sendRequest(eipWrapper).thenCompose(response -> {
+            if (!(response instanceof CipRRData rr) || rr.getStatus() != 
CIPStatus.Success.getValue() ||
+                !(rr.getTypeIds().get(1) instanceof UnConnectedDataItem di && 
di.getService() instanceof GetAttributeAllResponse gar)) {
+                return CompletableFuture.completedFuture(null);
             }
             if (gar.getStatus() == CIPStatus.ServiceNotSupported.getValue()) {
-                return;
+                return checkAttributesSingle();

Review Comment:
   I'm more trying to wrap my head around what the code should do ... 
   
   If it's (not a CipRRData) or (it is and it's status is not success) or (it 
is and it's first type id is an UnConnectedDataItem) or (it is, it's first type 
id is a UnConnectedDataItem's service is a GetAttributeAllResponse ....
   
   So much negation ... wouldn't it be an alternative to focus what we expect 
it to be?
   Something like this?
   
   ```
   if (response instanceof CipRRData rr
               && rr.getStatus() == CIPStatus.Success.getValue()
               && rr.getTypeIds().size() > 1
               && rr.getTypeIds().get(1) instanceof UnConnectedDataItem di
               && serviceType.isInstance(di.getService())) {
           return serviceType.cast(di.getService());
       }
       return null;
   ```
   
   That I would instantly understand ;-)



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to