fix(plc4j/modbus): Cleanup of ModbusTag (#732)

* fix(plc4j/modbus): Cleanup of ModbusTag * fix(plc4j/modbus): ModbusTag address prefix to method
apache · Jan 8, 2023 · 8c49f4c · 8c49f4c
1 parent 8a793e2
commit 8c49f4c
Show file tree

Hide file tree

Showing 7 changed files with 165 additions and 100 deletions.
diff --git a/plc4j/drivers/modbus/src/main/java/org/apache/plc4x/java/modbus/base/tag/ModbusTag.java b/plc4j/drivers/modbus/src/main/java/org/apache/plc4x/java/modbus/base/tag/ModbusTag.java
@@ -40,7 +40,7 @@ public abstract class ModbusTag implements PlcTag, Serializable {
     public static final Pattern ADDRESS_PATTERN = Pattern.compile("(?<address>\\d+)(:(?<datatype>[a-zA-Z_]+))?(\\[(?<quantity>\\d+)])?");
     public static final Pattern FIXED_DIGIT_MODBUS_PATTERN = Pattern.compile("(?<address>\\d{4,5})?(:(?<datatype>[a-zA-Z_]+))?(\\[(?<quantity>\\d+)])?");
 
-    protected static final int PROTOCOL_ADDRESS_OFFSET = 1;
+    public static final int PROTOCOL_ADDRESS_OFFSET = 1;
 
     private final int address;
 
@@ -67,10 +67,30 @@ public static ModbusTag of(String addressString) {
         throw new PlcInvalidTagException("Unable to parse address: " + addressString);
     }
 
+    @Override
+    public String getAddressString() {
+        String address = String.format("%s%05d", getAddressStringPrefix(), getLogicalAddress());
+        if(getDataType() != null) {
+            address += ":" + getDataType().name();
+        }
+        if(!getArrayInfo().isEmpty()) {
+            address += "[" + getArrayInfo().get(0).getUpperBound() + "]";
+        }
+        return address;
+    }
+
+    protected abstract String getAddressStringPrefix();
+
+    /**
+     * Instantiate a new ModbusTag
+     * @param address The WIRE address that is to be used.
+     * @param quantity The number of registers
+     * @param dataType The type for the interpretation of the registers.
+     */
     protected ModbusTag(int address, Integer quantity, ModbusDataType dataType) {
         this.address = address;
-        if ((this.address + PROTOCOL_ADDRESS_OFFSET) <= 0) {
-            throw new IllegalArgumentException("address must be greater than zero. Was " + (this.address + PROTOCOL_ADDRESS_OFFSET));
+        if (getLogicalAddress() <= 0) {
+            throw new IllegalArgumentException("address must be greater than zero. Was " + getLogicalAddress());
         }
         this.quantity = quantity != null ? quantity : 1;
         if (this.quantity <= 0) {
@@ -79,10 +99,20 @@ protected ModbusTag(int address, Integer quantity, ModbusDataType dataType) {
         this.dataType = dataType != null ? dataType : ModbusDataType.INT;
     }
 
+    /**
+     * Get the technical address that must be used 'on the wire'
+     * @return The address that is to be used on the wire (shifted by 1 because of the modbus spec).
+     */
     public int getAddress() {
         return address;
     }
 
+    /**
+     * Get the logical (configured) address
+     * @return The address which was configured and is different from what is used on the wire.
+     */
+    public abstract int getLogicalAddress();
+
     public int getNumberOfElements() {
         return quantity;
     }
@@ -122,21 +152,24 @@ public boolean equals(Object o) {
             return false;
         }
         ModbusTag that = (ModbusTag) o;
-        return address == that.address;
+        return address == that.address &&
+            quantity == that.quantity &&
+            dataType == that.dataType &&
+            getClass() == that.getClass(); // MUST be identical
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(address);
+        return Objects.hash(this.getClass(), address, quantity, dataType);
     }
 
     @Override
     public String toString() {
-        return "ModbusTag{" +
+        return this.getClass().getSimpleName() + " {" +
             "address=" + address +
-            "datatype=" + dataType +
-            "quantity=" + quantity +
-            '}';
+            ", quantity=" + quantity +
+            ", dataType=" + dataType +
+            " }";
     }
 
     @Override

diff --git a/plc4j/drivers/modbus/src/main/java/org/apache/plc4x/java/modbus/base/tag/ModbusTagCoil.java b/plc4j/drivers/modbus/src/main/java/org/apache/plc4x/java/modbus/base/tag/ModbusTagCoil.java
@@ -26,6 +26,7 @@
 
 public class ModbusTagCoil extends ModbusTag {
 
+    public static final String ADDRESS_PREFIX = "0x";
     public static final Pattern ADDRESS_PATTERN = Pattern.compile("coil:" + ModbusTag.ADDRESS_PATTERN);
     public static final Pattern ADDRESS_SHORTER_PATTERN = Pattern.compile("0" + ModbusTag.FIXED_DIGIT_MODBUS_PATTERN);
     public static final Pattern ADDRESS_SHORT_PATTERN = Pattern.compile("0x" + ModbusTag.FIXED_DIGIT_MODBUS_PATTERN);
@@ -36,16 +37,13 @@ public ModbusTagCoil(int address, Integer quantity, ModbusDataType dataType) {
         super(address, quantity, dataType);
     }
 
+    protected String getAddressStringPrefix() {
+        return ADDRESS_PREFIX;
+    }
+
     @Override
-    public String getAddressString() {
-        String address = "0x" + getAddress();
-        if(getDataType() != null) {
-            address += ":" + getDataType().name();
-        }
-        if(getArrayInfo().size() > 0) {
-            address += "[" + getArrayInfo().get(0).getUpperBound() + "]";
-        }
-        return address;
+    public int getLogicalAddress() {
+        return getAddress() + PROTOCOL_ADDRESS_OFFSET;
     }
 
     public static boolean matches(String addressString) {

diff --git a/...rs/modbus/src/main/java/org/apache/plc4x/java/modbus/base/tag/ModbusTagDiscreteInput.java b/...rs/modbus/src/main/java/org/apache/plc4x/java/modbus/base/tag/ModbusTagDiscreteInput.java
@@ -26,6 +26,7 @@
 
 public class ModbusTagDiscreteInput extends ModbusTag {
 
+    public static final String ADDRESS_PREFIX = "1x";
     public static final Pattern ADDRESS_PATTERN = Pattern.compile("discrete-input:" + ModbusTag.ADDRESS_PATTERN);
     public static final Pattern ADDRESS_SHORTER_PATTERN = Pattern.compile("1" + ModbusTag.FIXED_DIGIT_MODBUS_PATTERN);
     public static final Pattern ADDRESS_SHORT_PATTERN = Pattern.compile("1x" + ModbusTag.FIXED_DIGIT_MODBUS_PATTERN);
@@ -36,16 +37,13 @@ public ModbusTagDiscreteInput(int address, Integer quantity, ModbusDataType data
         super(address, quantity, dataType);
     }
 
+    protected String getAddressStringPrefix() {
+        return ADDRESS_PREFIX;
+    }
+
     @Override
-    public String getAddressString() {
-        String address = "1x" + getAddress();
-        if(getDataType() != null) {
-            address += ":" + getDataType().name();
-        }
-        if(getArrayInfo().size() > 0) {
-            address += "[" + getArrayInfo().get(0).getUpperBound() + "]";
-        }
-        return address;
+    public int getLogicalAddress() {
+        return getAddress() + PROTOCOL_ADDRESS_OFFSET;
     }
 
     public static boolean matches(String addressString) {
@@ -57,7 +55,7 @@ public static boolean matches(String addressString) {
     public static Matcher getMatcher(String addressString) {
         Matcher matcher = ADDRESS_PATTERN.matcher(addressString);
         if (matcher.matches()) {
-          return matcher;
+            return matcher;
         }
         matcher = ADDRESS_SHORT_PATTERN.matcher(addressString);
         if (matcher.matches()) {

diff --git a/...modbus/src/main/java/org/apache/plc4x/java/modbus/base/tag/ModbusTagExtendedRegister.java b/...modbus/src/main/java/org/apache/plc4x/java/modbus/base/tag/ModbusTagExtendedRegister.java
@@ -26,6 +26,7 @@
 
 public class ModbusTagExtendedRegister extends ModbusTag {
 
+    public static final String ADDRESS_PREFIX = "6x";
     public static final Pattern ADDRESS_PATTERN = Pattern.compile("extended-register:" + ModbusTag.ADDRESS_PATTERN);
     public static final Pattern ADDRESS_SHORTER_PATTERN = Pattern.compile("6" + ModbusTag.FIXED_DIGIT_MODBUS_PATTERN);
     public static final Pattern ADDRESS_SHORT_PATTERN = Pattern.compile("6x" + ModbusTag.FIXED_DIGIT_MODBUS_PATTERN);
@@ -36,16 +37,14 @@ protected ModbusTagExtendedRegister(int address, Integer quantity, ModbusDataTyp
         super(address, quantity, dataType);
     }
 
+    protected String getAddressStringPrefix() {
+        return ADDRESS_PREFIX;
+    }
+
     @Override
-    public String getAddressString() {
-        String address = "6x" + getAddress();
-        if(getDataType() != null) {
-            address += ":" + getDataType().name();
-        }
-        if(getArrayInfo().size() > 0) {
-            address += "[" + getArrayInfo().get(0).getUpperBound() + "]";
-        }
-        return address;
+    public int getLogicalAddress() {
+        // Addresses for extended memory start at address 0 instead of 1
+        return getAddress();
     }
 
     public static boolean matches(String addressString) {

diff --git a/.../modbus/src/main/java/org/apache/plc4x/java/modbus/base/tag/ModbusTagHoldingRegister.java b/.../modbus/src/main/java/org/apache/plc4x/java/modbus/base/tag/ModbusTagHoldingRegister.java
@@ -26,6 +26,7 @@
 
 public class ModbusTagHoldingRegister extends ModbusTag {
 
+    public static final String ADDRESS_PREFIX = "4x";
     public static final Pattern ADDRESS_PATTERN = Pattern.compile("holding-register:" + ModbusTag.ADDRESS_PATTERN);
     public static final Pattern ADDRESS_SHORTER_PATTERN = Pattern.compile("4" + ModbusTag.FIXED_DIGIT_MODBUS_PATTERN);
     public static final Pattern ADDRESS_SHORT_PATTERN = Pattern.compile("4x" + ModbusTag.FIXED_DIGIT_MODBUS_PATTERN);
@@ -36,16 +37,13 @@ protected ModbusTagHoldingRegister(int address, Integer quantity, ModbusDataType
         super(address, quantity, dataType);
     }
 
+    protected String getAddressStringPrefix() {
+        return ADDRESS_PREFIX;
+    }
+
     @Override
-    public String getAddressString() {
-        String address = "4x" + getAddress();
-        if(getDataType() != null) {
-            address += ":" + getDataType().name();
-        }
-        if(getArrayInfo().size() > 0) {
-            address += "[" + getArrayInfo().get(0).getUpperBound() + "]";
-        }
-        return address;
+    public int getLogicalAddress() {
+        return getAddress() + PROTOCOL_ADDRESS_OFFSET;
     }
 
     public static boolean matches(String addressString) {

diff --git a/...rs/modbus/src/main/java/org/apache/plc4x/java/modbus/base/tag/ModbusTagInputRegister.java b/...rs/modbus/src/main/java/org/apache/plc4x/java/modbus/base/tag/ModbusTagInputRegister.java
@@ -26,6 +26,7 @@
 
 public class ModbusTagInputRegister extends ModbusTag {
 
+    public static final String ADDRESS_PREFIX = "3x";
     public static final Pattern ADDRESS_PATTERN = Pattern.compile("input-register:" + ModbusTag.ADDRESS_PATTERN);
     public static final Pattern ADDRESS_SHORTER_PATTERN = Pattern.compile("3" + ModbusTag.FIXED_DIGIT_MODBUS_PATTERN);
     public static final Pattern ADDRESS_SHORT_PATTERN = Pattern.compile("3x" + ModbusTag.FIXED_DIGIT_MODBUS_PATTERN);
@@ -36,16 +37,13 @@ protected ModbusTagInputRegister(int address, Integer quantity, ModbusDataType d
         super(address, quantity, dataType);
     }
 
+    protected String getAddressStringPrefix() {
+        return ADDRESS_PREFIX;
+    }
+
     @Override
-    public String getAddressString() {
-        String address = "3x" + getAddress();
-        if(getDataType() != null) {
-            address += ":" + getDataType().name();
-        }
-        if(getArrayInfo().size() > 0) {
-            address += "[" + getArrayInfo().get(0).getUpperBound() + "]";
-        }
-        return address;
+    public int getLogicalAddress() {
+        return getAddress() + PROTOCOL_ADDRESS_OFFSET;
     }
 
     public static boolean matches(String addressString) {