refactor: Fine tuning of the API and SPI to allow ILLEGAL_ADDRESS res…

…ponses for individual invalid addresses (Alongside a major cleanup)
apache · Sep 24, 2024 · 5236450 · 5236450
1 parent daee93f
commit 5236450
Show file tree

Hide file tree

Showing 47 changed files with 443 additions and 305 deletions.
diff --git a/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/PlcTagRequest.java b/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/PlcTagRequest.java
@@ -19,6 +19,7 @@
 package org.apache.plc4x.java.api.messages;
 
 import org.apache.plc4x.java.api.model.PlcTag;
+import org.apache.plc4x.java.api.types.PlcResponseCode;
 
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -33,7 +34,9 @@ public interface PlcTagRequest extends PlcRequest {
 
     LinkedHashSet<String> getTagNames();
 
-    PlcTag getTag(String name);
+    PlcResponseCode getTagResponseCode(String tagName);
+
+    PlcTag getTag(String tagName);
 
     List<PlcTag> getTags();
 

diff --git a/plc4j/drivers/ab-eth/src/main/java/org/apache/plc4x/java/abeth/AbEthDriver.java b/plc4j/drivers/ab-eth/src/main/java/org/apache/plc4x/java/abeth/AbEthDriver.java
@@ -28,7 +28,6 @@
 import org.apache.plc4x.java.spi.configuration.PlcConnectionConfiguration;
 import org.apache.plc4x.java.spi.configuration.PlcTransportConfiguration;
 import org.apache.plc4x.java.api.model.PlcTag;
-import org.apache.plc4x.java.spi.values.PlcValueHandler;
 import org.apache.plc4x.java.spi.connection.GeneratedDriverBase;
 import org.apache.plc4x.java.spi.connection.ProtocolStackConfigurer;
 import org.apache.plc4x.java.spi.connection.SingleProtocolStackConfigurer;
@@ -82,11 +81,6 @@ protected AbEthTagHandler getTagHandler() {
         return new AbEthTagHandler();
     }
 
-    @Override
-    protected org.apache.plc4x.java.api.value.PlcValueHandler getValueHandler() {
-        return new PlcValueHandler();
-    }
-
     /**
      * This protocol doesn't have a disconnect procedure, so there is no need to wait for a login to finish.
      * @return false

diff --git a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/AdsPlcDriver.java b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/AdsPlcDriver.java
@@ -29,7 +29,6 @@
 import org.apache.plc4x.java.spi.configuration.PlcTransportConfiguration;
 import org.apache.plc4x.java.api.messages.PlcDiscoveryRequest;
 import org.apache.plc4x.java.spi.messages.DefaultPlcDiscoveryRequest;
-import org.apache.plc4x.java.spi.values.PlcValueHandler;
 import org.apache.plc4x.java.spi.connection.GeneratedDriverBase;
 import org.apache.plc4x.java.spi.connection.ProtocolStackConfigurer;
 import org.apache.plc4x.java.spi.connection.SingleProtocolStackConfigurer;
@@ -121,11 +120,6 @@ protected AdsTagHandler getTagHandler() {
         return new AdsTagHandler();
     }
 
-    @Override
-    protected org.apache.plc4x.java.api.value.PlcValueHandler getValueHandler() {
-        return new PlcValueHandler();
-    }
-
     /**
      * This protocol doesn't have a disconnect procedure, so there is no need to wait for a login to finish.
      * @return false

diff --git a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/protocol/AdsProtocolLogic.java b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/protocol/AdsProtocolLogic.java
@@ -672,7 +672,7 @@ protected CompletableFuture<PlcReadResponse> singleRead(PlcReadRequest readReque
         if(directAdsTag == null) {
             return CompletableFuture.completedFuture(new DefaultPlcReadResponse(readRequest, Collections.singletonMap(
                 readRequest.getTagNames().stream().findFirst().orElseThrow(),
-                new ResponseItem<>(PlcResponseCode.INVALID_ADDRESS, null))));
+                new ResponseItem<>(PlcResponseCode.NOT_FOUND, null))));
         }
 
         CompletableFuture<PlcReadResponse> future = new CompletableFuture<>();

diff --git a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/tag/SymbolicAdsTag.java b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/tag/SymbolicAdsTag.java
@@ -37,8 +37,7 @@
  */
 public class SymbolicAdsTag implements AdsTag {
 
-    // TODO: Model the end of this address to allow usage of multi-dimensional arrays.
-    private static final Pattern SYMBOLIC_ADDRESS_PATTERN = Pattern.compile("^(?<symbolicAddress>.+)");
+    private static final Pattern SYMBOLIC_ADDRESS_PATTERN = Pattern.compile("^([\\w_]+)(\"[\"\\d*]\")*(\\.(\\w+)(\"[\"\\d*]\")*)*");
 
     private final String symbolicAddress;
 
@@ -57,9 +56,7 @@ public static SymbolicAdsTag of(String address) {
         if (!matcher.matches()) {
             throw new PlcInvalidTagException(address, SYMBOLIC_ADDRESS_PATTERN, "{address}");
         }
-        String symbolicAddress = matcher.group("symbolicAddress");
-
-        return new SymbolicAdsTag(symbolicAddress, null, Collections.emptyList());
+        return new SymbolicAdsTag(address, null, Collections.emptyList());
     }
 
     public static boolean matches(String address) {
@@ -120,4 +117,5 @@ public void serialize(WriteBuffer writeBuffer) throws SerializationException {
 
         writeBuffer.popContext(getClass().getSimpleName());
     }
+
 }
diff --git a/plc4j/drivers/ads/src/test/java/org/apache/plc4x/java/ads/tag/SymbolicAdsTagTest.java b/plc4j/drivers/ads/src/test/java/org/apache/plc4x/java/ads/tag/SymbolicAdsTagTest.java
@@ -0,0 +1,34 @@
+/*
+ * 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.java.ads.tag;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+class SymbolicAdsTagTest {
+
+    @Test
+    void of() {
+
+
+    }
+
+}
diff --git a/plc4j/drivers/ads/src/test/java/org/apache/plc4x/protocol/ads/ManualAdsDriverTest.java b/plc4j/drivers/ads/src/test/java/org/apache/plc4x/protocol/ads/ManualAdsDriverTest.java
@@ -18,6 +18,7 @@
  */
 package org.apache.plc4x.protocol.ads;
 
+import org.apache.plc4x.java.api.types.PlcResponseCode;
 import org.apache.plc4x.java.api.value.PlcValue;
 import org.apache.plc4x.java.spi.values.*;
 import org.apache.plc4x.test.manual.ManualTest;
@@ -131,6 +132,10 @@ public static void main(String[] args) throws Exception {
         children.put("hurz_TIME_OF_DAY", new PlcTIME_OF_DAY(LocalTime.parse("15:36:30.123")));
         children.put("hurz_DATE_AND_TIME", new PlcDATE_AND_TIME(LocalDateTime.parse("1996-05-06T15:36:30")));
         test.addTestCase("MAIN.hurz_Struct", new PlcStruct(children));
+        test.addTestCase("MAIN.thisVariableDoesntExist", PlcResponseCode.NOT_FOUND);
+        // TODO: Add some complex array path
+        test.addTestCase("...3323(/987", PlcResponseCode.INVALID_ADDRESS);
+        test.addTestCase("MAIN.hurz_UDT_array[4].hurz_INT_array[2].someProperty", PlcResponseCode.NOT_FOUND);
         test.run();
     }
 

diff --git a/plc4j/drivers/bacnet/src/main/java/org/apache/plc4x/java/bacnetip/BacNetIpDriver.java b/plc4j/drivers/bacnet/src/main/java/org/apache/plc4x/java/bacnetip/BacNetIpDriver.java
@@ -31,7 +31,6 @@
 import org.apache.plc4x.java.spi.connection.GeneratedDriverBase;
 import org.apache.plc4x.java.spi.connection.ProtocolStackConfigurer;
 import org.apache.plc4x.java.spi.connection.SingleProtocolStackConfigurer;
-import org.apache.plc4x.java.spi.values.PlcValueHandler;
 
 import java.util.Arrays;
 import java.util.List;
@@ -99,11 +98,6 @@ protected BacNetIpTagHandler getTagHandler() {
         return new BacNetIpTagHandler();
     }
 
-    @Override
-    protected org.apache.plc4x.java.api.value.PlcValueHandler getValueHandler() {
-        return new PlcValueHandler();
-    }
-
     @Override
     protected ProtocolStackConfigurer<BVLC> getStackConfigurer() {
         return SingleProtocolStackConfigurer.builder(BVLC.class, BVLC::staticParse)

diff --git a/plc4j/drivers/c-bus/src/main/java/org/apache/plc4x/java/cbus/CBusDriver.java b/plc4j/drivers/c-bus/src/main/java/org/apache/plc4x/java/cbus/CBusDriver.java
@@ -19,12 +19,9 @@
 package org.apache.plc4x.java.cbus;
 
 import io.netty.buffer.ByteBuf;
-import org.apache.plc4x.java.cbus.readwrite.CBusMessage;
 import org.apache.plc4x.java.cbus.readwrite.CBusOptions;
-import org.apache.plc4x.java.cbus.readwrite.RequestContext;
 import org.apache.plc4x.java.spi.configuration.PlcConnectionConfiguration;
 import org.apache.plc4x.java.spi.configuration.PlcTransportConfiguration;
-import org.apache.plc4x.java.api.value.PlcValueHandler;
 import org.apache.plc4x.java.cbus.configuration.CBusConfiguration;
 import org.apache.plc4x.java.cbus.configuration.CBusTcpTransportConfiguration;
 import org.apache.plc4x.java.cbus.context.CBusDriverContext;
@@ -88,11 +85,6 @@ protected PlcTagHandler getTagHandler() {
         return null;
     }
 
-    @Override
-    protected PlcValueHandler getValueHandler() {
-        return null;
-    }
-
     @Override
     protected ProtocolStackConfigurer<CBusCommand> getStackConfigurer() {
         return SingleProtocolStackConfigurer.builder(CBusCommand.class, io ->

diff --git a/plc4j/drivers/can/src/main/java/org/apache/plc4x/java/can/generic/GenericCANDriver.java b/plc4j/drivers/can/src/main/java/org/apache/plc4x/java/can/generic/GenericCANDriver.java
@@ -33,7 +33,6 @@
 import org.apache.plc4x.java.spi.generation.Message;
 import org.apache.plc4x.java.spi.optimizer.BaseOptimizer;
 import org.apache.plc4x.java.spi.transport.Transport;
-import org.apache.plc4x.java.spi.values.PlcValueHandler;
 import org.apache.plc4x.java.transport.can.CANTransport;
 
 import java.util.Optional;
@@ -85,11 +84,6 @@ protected GenericCANTagHandler getTagHandler() {
         return new GenericCANTagHandler();
     }
 
-    @Override
-    protected org.apache.plc4x.java.api.value.PlcValueHandler getValueHandler() {
-        return new PlcValueHandler();
-    }
-
     /**
      * This protocol doesn't have a disconnect procedure, so there is no need to wait for a login to finish.
      * @return false

diff --git a/plc4j/drivers/canopen/src/main/java/org/apache/plc4x/java/canopen/CANOpenPlcDriver.java b/plc4j/drivers/canopen/src/main/java/org/apache/plc4x/java/canopen/CANOpenPlcDriver.java
@@ -87,7 +87,7 @@ protected CANOpenTagHandler getTagHandler() {
     }
 
     @Override
-    protected org.apache.plc4x.java.api.value.PlcValueHandler getValueHandler() {
+    protected PlcValueHandler getValueHandler() {
         return new PlcValueHandler() {
             @Override
             public PlcValue newPlcValue(PlcTag tag, Object[] values) {

diff --git a/plc4j/drivers/eip/src/main/java/org/apache/plc4x/java/eip/base/EIPDriver.java b/plc4j/drivers/eip/src/main/java/org/apache/plc4x/java/eip/base/EIPDriver.java
@@ -38,8 +38,6 @@
 import org.apache.plc4x.java.spi.messages.DefaultPlcDiscoveryRequest;
 import org.apache.plc4x.java.spi.transport.Transport;
 
-import org.apache.plc4x.java.api.value.PlcValueHandler;
-
 import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
@@ -97,11 +95,6 @@ protected PlcTagHandler getTagHandler() {
         return new EipTagHandler();
     }
 
-    @Override
-    protected PlcValueHandler getValueHandler() {
-        return new org.apache.plc4x.java.spi.values.PlcValueHandler();
-    }
-
     @Override
     public PlcDiscoveryRequest.Builder discoveryRequestBuilder() {
         return new DefaultPlcDiscoveryRequest.Builder(new EipPlcDiscoverer());

diff --git a/plc4j/drivers/eip/src/main/java/org/apache/plc4x/java/eip/logix/LogixDriver.java b/plc4j/drivers/eip/src/main/java/org/apache/plc4x/java/eip/logix/LogixDriver.java
@@ -21,7 +21,6 @@
 import io.netty.buffer.ByteBuf;
 import org.apache.plc4x.java.spi.configuration.PlcConnectionConfiguration;
 import org.apache.plc4x.java.spi.configuration.PlcTransportConfiguration;
-import org.apache.plc4x.java.api.value.PlcValueHandler;
 import org.apache.plc4x.java.eip.base.tag.EipTag;
 import org.apache.plc4x.java.eip.base.protocol.EipProtocolLogic;
 import org.apache.plc4x.java.eip.logix.configuration.LogixConfiguration;
@@ -77,11 +76,6 @@ protected PlcTagHandler getTagHandler() {
         return new EipTagHandler();
     }
 
-    @Override
-    protected PlcValueHandler getValueHandler() {
-        return new org.apache.plc4x.java.spi.values.PlcValueHandler();
-    }
-
     @Override
     protected boolean awaitDisconnectComplete() {
         return true;

diff --git a/.../drivers/firmata/src/main/java/org/apache/plc4x/java/firmata/readwrite/FirmataDriver.java b/.../drivers/firmata/src/main/java/org/apache/plc4x/java/firmata/readwrite/FirmataDriver.java
@@ -26,7 +26,6 @@
 import org.apache.plc4x.java.firmata.readwrite.tag.FirmataTag;
 import org.apache.plc4x.java.firmata.readwrite.tag.FirmataTagHandler;
 import org.apache.plc4x.java.firmata.readwrite.protocol.FirmataProtocolLogic;
-import org.apache.plc4x.java.spi.values.PlcValueHandler;
 import org.apache.plc4x.java.spi.connection.GeneratedDriverBase;
 import org.apache.plc4x.java.spi.connection.PlcTagHandler;
 import org.apache.plc4x.java.spi.connection.ProtocolStackConfigurer;
@@ -80,11 +79,6 @@ protected PlcTagHandler getTagHandler() {
         return new FirmataTagHandler();
     }
 
-    @Override
-    protected org.apache.plc4x.java.api.value.PlcValueHandler getValueHandler() {
-        return new PlcValueHandler();
-    }
-
     /**
      * This protocol doesn't have a disconnect procedure, so there is no need to wait for a login to finish.
      * @return false

diff --git a/...0870/src/main/java/org/apache/plc4x/java/iec608705104/readwrite/Iec60870514PlcDriver.java b/...0870/src/main/java/org/apache/plc4x/java/iec608705104/readwrite/Iec60870514PlcDriver.java
@@ -28,7 +28,6 @@
 import org.apache.plc4x.java.spi.connection.GeneratedDriverBase;
 import org.apache.plc4x.java.spi.connection.ProtocolStackConfigurer;
 import org.apache.plc4x.java.spi.connection.SingleProtocolStackConfigurer;
-import org.apache.plc4x.java.spi.values.PlcValueHandler;
 
 import java.util.Collections;
 import java.util.List;
@@ -88,11 +87,6 @@ protected Iec608705104TagHandler getTagHandler() {
         return new Iec608705104TagHandler();
     }
 
-    @Override
-    protected org.apache.plc4x.java.api.value.PlcValueHandler getValueHandler() {
-        return new PlcValueHandler();
-    }
-
     /**
      * This protocol doesn't have a disconnect procedure, so there is no need to wait for a login to finish.
      * @return false

diff --git a/plc4j/drivers/knxnetip/src/main/java/org/apache/plc4x/java/knxnetip/KnxNetIpDriver.java b/plc4j/drivers/knxnetip/src/main/java/org/apache/plc4x/java/knxnetip/KnxNetIpDriver.java
@@ -33,7 +33,6 @@
 import org.apache.plc4x.java.knxnetip.tag.KnxNetIpTagHandler;
 import org.apache.plc4x.java.knxnetip.protocol.KnxNetIpProtocolLogic;
 import org.apache.plc4x.java.spi.messages.DefaultPlcDiscoveryRequest;
-import org.apache.plc4x.java.spi.values.PlcValueHandler;
 import org.apache.plc4x.java.spi.connection.GeneratedDriverBase;
 import org.apache.plc4x.java.spi.connection.PlcTagHandler;
 import org.apache.plc4x.java.spi.connection.ProtocolStackConfigurer;
@@ -128,11 +127,6 @@ protected PlcTagHandler getTagHandler() {
         return new KnxNetIpTagHandler();
     }
 
-    @Override
-    protected org.apache.plc4x.java.api.value.PlcValueHandler getValueHandler() {
-        return new PlcValueHandler();
-    }
-
     @Override
     protected boolean awaitDisconnectComplete() { return true; }
 

diff --git a/plc4j/drivers/modbus/src/main/java/org/apache/plc4x/java/modbus/ascii/ModbusAsciiDriver.java b/plc4j/drivers/modbus/src/main/java/org/apache/plc4x/java/modbus/ascii/ModbusAsciiDriver.java
@@ -22,8 +22,6 @@
 import org.apache.commons.codec.DecoderException;
 import org.apache.commons.codec.binary.Hex;
 import org.apache.plc4x.java.modbus.ascii.context.ModbusAsciiContext;
-import org.apache.plc4x.java.modbus.readwrite.ModbusADU;
-import org.apache.plc4x.java.modbus.readwrite.ModbusRtuADU;
 import org.apache.plc4x.java.spi.configuration.PlcConnectionConfiguration;
 import org.apache.plc4x.java.spi.configuration.PlcTransportConfiguration;
 import org.apache.plc4x.java.modbus.ascii.config.ModbusAsciiConfiguration;
@@ -39,7 +37,6 @@
 import org.apache.plc4x.java.spi.generation.*;
 import org.apache.plc4x.java.spi.optimizer.BaseOptimizer;
 import org.apache.plc4x.java.spi.optimizer.SingleTagOptimizer;
-import org.apache.plc4x.java.spi.values.PlcValueHandler;
 
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
@@ -127,11 +124,6 @@ protected ModbusTagHandler getTagHandler() {
         return new ModbusTagHandler();
     }
 
-    @Override
-    protected org.apache.plc4x.java.api.value.PlcValueHandler getValueHandler() {
-        return new PlcValueHandler();
-    }
-
     @Override
     protected ProtocolStackConfigurer<ModbusAsciiADU> getStackConfigurer() {
         return SingleProtocolStackConfigurer.builder(ModbusAsciiADU.class, new ModbusAsciiInput(), new ModbusAsciiOutput())