You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@plc4x.apache.org by cd...@apache.org on 2018/03/11 10:09:29 UTC

[incubator-plc4x] branch master updated (1110324 -> a5b123e)

This is an automated email from the ASF dual-hosted git repository.

cdutz pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-plc4x.git.


    from 1110324  added missing ASF header
     new ad38796  - Renamed the S7 encoders and decoders
     new e46a01e  - Updated the "implementing drivers" to recent changes
     new 691f3b8  - Changed the modbus driver to be a single driver with multiple sub-types
     new e926898  - Added the "tcp:" suffix to the tcp-variant of the ads driver
     new a5b123e  - Updated some links in the s7 documentation - Updated the festures table - Added a link to some more wireshark dumps

The 5 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../org/apache/plc4x/java/ads/AdsPlcDriver.java    |  2 +-
 ...usSerialPlcDriver.java => ModbusPlcDriver.java} | 28 +++++---
 .../plc4x/java/modbus/ModbusTcpPlcDriver.java      | 76 ----------------------
 .../services/org.apache.plc4x.java.api.PlcDriver   |  3 +-
 .../plc4x/java/s7/netty/Plc4XS7Protocol.java       |  4 +-
 .../org/apache/plc4x/java/s7/netty/S7Protocol.java | 50 +++++++++++---
 .../{BigEndianDecoder.java => S7TypeDecoder.java}  | 24 ++++---
 .../{BigEndianEncoder.java => S7TypeEncoder.java}  |  4 +-
 .../asciidoc/developers/implementing-drivers.adoc  |  5 --
 src/site/asciidoc/protocols/features.adoc          | 38 ++++++-----
 src/site/asciidoc/protocols/index.adoc             |  2 +
 src/site/asciidoc/protocols/s7/index.adoc          |  4 +-
 12 files changed, 109 insertions(+), 131 deletions(-)
 rename plc4j/protocols/modbus/src/main/java/org/apache/plc4x/java/modbus/{ModbusSerialPlcDriver.java => ModbusPlcDriver.java} (70%)
 delete mode 100644 plc4j/protocols/modbus/src/main/java/org/apache/plc4x/java/modbus/ModbusTcpPlcDriver.java
 rename plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/util/{BigEndianDecoder.java => S7TypeDecoder.java} (76%)
 rename plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/util/{BigEndianEncoder.java => S7TypeEncoder.java} (98%)

-- 
To stop receiving notification emails like this one, please contact
cdutz@apache.org.

[incubator-plc4x] 02/05: - Updated the "implementing drivers" to recent changes

Posted by cd...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

cdutz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-plc4x.git

commit e46a01ec11ffba774a7a862743c9ad0cb09102af
Author: Christofer Dutz <ch...@c-ware.de>
AuthorDate: Sun Mar 11 11:06:06 2018 +0100

    - Updated the "implementing drivers" to recent changes
---
 .../protocols/src/site/asciidoc/developers/implementing-drivers.adoc | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/plc4j/protocols/src/site/asciidoc/developers/implementing-drivers.adoc b/plc4j/protocols/src/site/asciidoc/developers/implementing-drivers.adoc
index 87830fc..99af2a1 100644
--- a/plc4j/protocols/src/site/asciidoc/developers/implementing-drivers.adoc
+++ b/plc4j/protocols/src/site/asciidoc/developers/implementing-drivers.adoc
@@ -178,11 +178,6 @@ For the S7 driver for example all it contains is this line:
 
     org.apache.plc4x.java.s7.S7PlcDriver
 
-As the Modbus driver contains two variants, it contains two lines - one for each variant:
-
-    org.apache.plc4x.java.modbus.ModbusTcpPlcDriver
-    org.apache.plc4x.java.modbus.ModbusSerialPlcDriver
-
 A driver implementation must implement the `org.apache.plc4x.java.api.PlcDriver` interface.
 This defines the necessary methods for the `DriverManager` to find the correct implementation and create a new connection instance.
 

-- 
To stop receiving notification emails like this one, please contact
cdutz@apache.org.

[incubator-plc4x] 01/05: - Renamed the S7 encoders and decoders

Posted by cd...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

cdutz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-plc4x.git

commit ad387967f4aa8cfbd034a78637ecf120e746b4c5
Author: Christofer Dutz <ch...@c-ware.de>
AuthorDate: Sun Mar 11 11:05:41 2018 +0100

    - Renamed the S7 encoders and decoders
---
 .../plc4x/java/s7/netty/Plc4XS7Protocol.java       |  4 +-
 .../org/apache/plc4x/java/s7/netty/S7Protocol.java | 50 ++++++++++++++++++----
 .../{BigEndianDecoder.java => S7TypeDecoder.java}  | 24 +++++++----
 .../{BigEndianEncoder.java => S7TypeEncoder.java}  |  4 +-
 4 files changed, 61 insertions(+), 21 deletions(-)

diff --git a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/Plc4XS7Protocol.java b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/Plc4XS7Protocol.java
index 9fcec13..750ed62 100644
--- a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/Plc4XS7Protocol.java
+++ b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/Plc4XS7Protocol.java
@@ -49,8 +49,8 @@ import org.apache.plc4x.java.s7.netty.model.types.*;
 import java.util.*;
 import java.util.concurrent.atomic.AtomicInteger;
 
-import static org.apache.plc4x.java.s7.netty.util.BigEndianDecoder.decodeData;
-import static org.apache.plc4x.java.s7.netty.util.BigEndianEncoder.encodeData;
+import static org.apache.plc4x.java.s7.netty.util.S7TypeDecoder.decodeData;
+import static org.apache.plc4x.java.s7.netty.util.S7TypeEncoder.encodeData;
 
 public class Plc4XS7Protocol extends MessageToMessageCodec<S7Message, PlcRequestContainer> {
 
diff --git a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/S7Protocol.java b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/S7Protocol.java
index 2aa4a75..40398d0 100644
--- a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/S7Protocol.java
+++ b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/S7Protocol.java
@@ -43,16 +43,16 @@ import org.apache.plc4x.java.s7.netty.model.types.*;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.util.Collections;
-import java.util.LinkedList;
-import java.util.List;
+import java.util.*;
 
 public class S7Protocol extends MessageToMessageCodec<IsoTPMessage, S7Message> {
 
-    public static final byte S7_PROTOCOL_MAGIC_NUMBER = 0x32;
+    private static final byte S7_PROTOCOL_MAGIC_NUMBER = 0x32;
 
     private static final Logger logger = LoggerFactory.getLogger(S7Protocol.class);
 
+    private Map<Short, List<S7Message>> messageFragments = new HashMap<>();
+
     private short maxAmqCaller;
     private short maxAmqCallee;
     private short pduSize;
@@ -81,13 +81,45 @@ public class S7Protocol extends MessageToMessageCodec<IsoTPMessage, S7Message> {
     protected void encode(ChannelHandlerContext ctx, S7Message in, List<Object> out) {
         logger.debug("S7 Message sent");
 
-        ByteBuf buf = Unpooled.buffer();
+        List<S7Message> messages = splitMessage(in);
+        messageFragments.put(in.getTpduReference(), messages);
+
+        for (S7Message message : messages) {
+            ByteBuf buf = Unpooled.buffer();
+
+            encodeHeader(message, buf);
+            encodeParameters(message, buf);
+            encodePayloads(message, buf);
 
-        encodeHeader(in, buf);
-        encodeParameters(in, buf);
-        encodePayloads(in, buf);
+            out.add(new DataTpdu(true, (byte) 1, Collections.emptyList(), buf));
+        }
+    }
 
-        out.add(new DataTpdu(true, (byte) 1, Collections.emptyList(), buf));
+    /**
+     * While a SetupCommunication message is no problem, when reading multiple
+     * addresses in one request, the size of the PDU could be exceeded, therefore we need
+     * to split up one incoming message. When writing, the S7 PLCs only seem to accept
+     * writing of single elements, so we have to break up writing of multiple values into
+     * multiple single-value write operations.
+     *
+     * @param message incoming message
+     * @return List of outgoing messages
+     */
+    private List<S7Message> splitMessage(S7Message message) {
+        // The following considerations have to be taken into account:
+        // - The size of all parameters and payloads of a message cannot exceed the negotiated PDU size
+        // - When reading data, the size of the returned data cannot exceed the negotiated PDU size
+        //
+        // Examples:
+        // - Size of the request exceeds the maximum
+        //  When having a negotiated max PDU size of 256, the maximum size of individual addresses can be at most 18
+        //  If more are sent, the S7 will respond with a frame error.
+        // - Size of the response exceeds the maximum
+        //  When reading two Strings of each 200 bytes length, the size of the request is ok, however the PLC would
+        //  have to send back 400 bytes of String data, which would exceed the PDU size. In this case the first String
+        //  is correctly returned, but for the second item the PLC will return a code of 0x03 = Access Denied
+
+        return Collections.singletonList(message);
     }
 
     private void encodePayloads(S7Message in, ByteBuf buf) {
diff --git a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/util/BigEndianDecoder.java b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/util/S7TypeDecoder.java
similarity index 76%
rename from plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/util/BigEndianDecoder.java
rename to plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/util/S7TypeDecoder.java
index 6e22bcd..ac67863 100644
--- a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/util/BigEndianDecoder.java
+++ b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/util/S7TypeDecoder.java
@@ -20,12 +20,13 @@ package org.apache.plc4x.java.s7.netty.util;
 
 import org.apache.plc4x.java.api.exceptions.PlcProtocolException;
 
+import java.io.UnsupportedEncodingException;
 import java.util.LinkedList;
 import java.util.List;
 
-public class BigEndianDecoder {
+public class S7TypeDecoder {
 
-    private BigEndianDecoder() {
+    private S7TypeDecoder() {
         // Utility class
     }
 
@@ -58,13 +59,20 @@ public class BigEndianDecoder {
                 result.add(Float.intBitsToFloat(intValue));
                 i += 4;
             } else if (datatype == String.class) {
-                StringBuilder builder = new StringBuilder();
-                while (s7Data[i] != (byte) 0x0 && i < length) {
-                    builder.append((char) s7Data[i]);
-                    i++;
+                // Every string value had a prefix of two bytes for which I have no idea, what the meaning is.
+                // This code assumes the string values doesn't contain UTF-8 values with a code of 0x00 as it
+                // uses this as termination char.
+                try {
+                    int j = 0;
+                    for(; j < s7Data.length; j++) {
+                        if(s7Data[j] == 0) {
+                            break;
+                        }
+                    }
+                    result.add(new String(s7Data, 2, j - 2, "UTF-8"));
+                } catch (UnsupportedEncodingException e) {
+                    throw new PlcProtocolException("Error decoding String value");
                 }
-                i++; // skip terminating character
-                result.add(builder.toString());
             } else {
                 throw new PlcProtocolException("Unsupported datatype " + datatype.getSimpleName());
             }
diff --git a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/util/BigEndianEncoder.java b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/util/S7TypeEncoder.java
similarity index 98%
rename from plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/util/BigEndianEncoder.java
rename to plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/util/S7TypeEncoder.java
index f9d1159..5c50e50 100644
--- a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/util/BigEndianEncoder.java
+++ b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/util/S7TypeEncoder.java
@@ -20,9 +20,9 @@ package org.apache.plc4x.java.s7.netty.util;
 
 import java.util.Calendar;
 
-public class BigEndianEncoder {
+public class S7TypeEncoder {
 
-    private BigEndianEncoder() {
+    private S7TypeEncoder() {
         // Utility class
     }
 

-- 
To stop receiving notification emails like this one, please contact
cdutz@apache.org.

[incubator-plc4x] 05/05: - Updated some links in the s7 documentation - Updated the festures table - Added a link to some more wireshark dumps

Posted by cd...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

cdutz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-plc4x.git

commit a5b123e601cd0dc70dc93d7363e86dbaedbb3f0f
Author: Christofer Dutz <ch...@c-ware.de>
AuthorDate: Sun Mar 11 11:09:25 2018 +0100

    - Updated some links in the s7 documentation
    - Updated the festures table
    - Added a link to some more wireshark dumps
---
 src/site/asciidoc/protocols/features.adoc | 38 ++++++++++++++++++-------------
 src/site/asciidoc/protocols/index.adoc    |  2 ++
 src/site/asciidoc/protocols/s7/index.adoc |  4 ++--
 3 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/src/site/asciidoc/protocols/features.adoc b/src/site/asciidoc/protocols/features.adoc
index ffcf018..e2c114e 100644
--- a/src/site/asciidoc/protocols/features.adoc
+++ b/src/site/asciidoc/protocols/features.adoc
@@ -21,35 +21,41 @@
 The following table contains a list of operations and the protocols that support them:
 
 |===
-|Protocol |Read Address Value |Write Address Value |List Resources |Telegrams/Broadcased Values
+|Protocol |Read Single Address Value |Read Multiple Address Values |Write Single Address Value |Write Multiple Address Value
 
 |S7
 |icon:check[role="green"]
 |icon:check[role="green"]
-|icon:remove[role="red"]
 |icon:check[role="green"]
+|icon:exclamation[role="yellow"]
 
 |Beckhoff ADS
 |icon:check[role="green"]
 |icon:check[role="green"]
-|icon:question[role="yellow"]
-|icon:question[role="yellow"]
-
-|Modbus
 |icon:check[role="green"]
 |icon:check[role="green"]
-|icon:check[role="green"]
-|icon:question[role="yellow"]
+
+|Modbus
+|icon:question[role="red"]
+|icon:question[role="red"]
+|icon:question[role="red"]
+|icon:question[role="red"]
 
 |OPC-UA
-|icon:check[role="green"]
-|icon:check[role="green"]
-|icon:check[role="green"]
-|icon:check[role="green"]
+|icon:question[role="red"]
+|icon:question[role="red"]
+|icon:question[role="red"]
+|icon:question[role="red"]
 
 |IEC 60870-5-104
-|icon:check[role="green"]
-|icon:check[role="green"]
-|icon:question[role="yellow"]
-|icon:question[role="yellow"]
+|icon:question[role="red"]
+|icon:question[role="red"]
+|icon:question[role="red"]
+|icon:question[role="red"]
 |===
+
+Legend:
+
+- icon:check[role="green"] Implemented and fully supported
+- icon:exclamation[role="yellow"] Implemented and supported by simulation
+- icon:question[role="red"] Not implemented yet
\ No newline at end of file
diff --git a/src/site/asciidoc/protocols/index.adoc b/src/site/asciidoc/protocols/index.adoc
index 08fa0fc..3400043 100644
--- a/src/site/asciidoc/protocols/index.adoc
+++ b/src/site/asciidoc/protocols/index.adoc
@@ -26,4 +26,6 @@
 Apache 2.0 licensed JNI library for accessing raw IPv4 and IPv6 sockets. Might be the ideal starting point for implementing protocols below TCP & UDP.
 https://www.savarese.org/software/rocksaw/
 
+Links to different Wireshark captures: https://github.com/automayt/ICS-pcap
+
 
diff --git a/src/site/asciidoc/protocols/s7/index.adoc b/src/site/asciidoc/protocols/s7/index.adoc
index fd259c1..ed0f54b 100644
--- a/src/site/asciidoc/protocols/s7/index.adoc
+++ b/src/site/asciidoc/protocols/s7/index.adoc
@@ -88,9 +88,9 @@ The `S7 Comm` protocols are generally split up into to flavours: The classic `S7
 |===
 |Name |ISO |RFC |Link
 |Transmission Control Protocol (TCP) |- | RFC 793 |https://tools.ietf.org/html/rfc793
-|ISO Transport Protocol (Class 4) |ISO DP 8073 | RFC 905 |https://tools.ietf.org/html/rfc905
 |ISO on TCP |- | RFC 1006| https://tools.ietf.org/html/rfc1006
-|S7 Comm (0x32) |- |- |http://gmiru.com/article/s7comm/ http://gmiru.com/article/s7comm-part2/
+|ISO Transport Protocol (Class 4) |ISO DP 8073 | RFC 905 |https://tools.ietf.org/html/rfc905
+|S7 Comm (0x32) |- |- |http://gmiru.com/article/s7comm/ http://gmiru.com/article/s7comm-part2/ https://www.eng.tau.ac.il/~yash/jdsfl2014.pdf
 |S7 Comm Plus (0x72) |- |- |https://opensource-security.de/thesis/MA_Maik_Brueggemann.pdf
 |RPC |- | RFC 1057 & RFC 5531 |https://tools.ietf.org/html/rfc1057 https://tools.ietf.org/html/rfc5531
 |DCOM |- |- | https://msdn.microsoft.com/library/cc201989.aspx

-- 
To stop receiving notification emails like this one, please contact
cdutz@apache.org.

[incubator-plc4x] 04/05: - Added the "tcp:" suffix to the tcp-variant of the ads driver

Posted by cd...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

cdutz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-plc4x.git

commit e92689843d5d9f2d14c48437e1f828813bd18b08
Author: Christofer Dutz <ch...@c-ware.de>
AuthorDate: Sun Mar 11 11:07:29 2018 +0100

    - Added the "tcp:" suffix to the tcp-variant of the ads driver
---
 .../ads/src/main/java/org/apache/plc4x/java/ads/AdsPlcDriver.java       | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/AdsPlcDriver.java b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/AdsPlcDriver.java
index dde9936..36284f3 100644
--- a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/AdsPlcDriver.java
+++ b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/AdsPlcDriver.java
@@ -45,7 +45,7 @@ public class AdsPlcDriver implements PlcDriver {
             + "(/"
             + "(?<sourceAmsNetId>" + AmsNetId.AMS_NET_ID_PATTERN + "):(?<sourceAmsPort>" + AmsPort.AMS_PORT_PATTERN + ")"
             + ")?");
-    public static final Pattern INET_ADDRESS_PATTERN = Pattern.compile("(?<host>[\\w.]+)(:(?<port>\\d*))?");
+    public static final Pattern INET_ADDRESS_PATTERN = Pattern.compile("(?<host>tcp:[\\w.]+)(:(?<port>\\d*))?");
     public static final Pattern SERIAL_PATTERN = Pattern.compile("(?<serialDefinition>serial:.*)");
     public static final Pattern ADS_URI_PATTERN = Pattern.compile("^ads:/?/?(" + INET_ADDRESS_PATTERN + "|" + SERIAL_PATTERN + ")/" + ADS_ADDRESS_PATTERN);
 

-- 
To stop receiving notification emails like this one, please contact
cdutz@apache.org.

[incubator-plc4x] 03/05: - Changed the modbus driver to be a single driver with multiple sub-types

Posted by cd...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

cdutz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-plc4x.git

commit 691f3b843582b53622df166cc374394b8cc81335
Author: Christofer Dutz <ch...@c-ware.de>
AuthorDate: Sun Mar 11 11:06:37 2018 +0100

    - Changed the modbus driver to be a single driver with multiple sub-types
---
 ...usSerialPlcDriver.java => ModbusPlcDriver.java} | 28 +++++---
 .../plc4x/java/modbus/ModbusTcpPlcDriver.java      | 76 ----------------------
 .../services/org.apache.plc4x.java.api.PlcDriver   |  3 +-
 3 files changed, 21 insertions(+), 86 deletions(-)

diff --git a/plc4j/protocols/modbus/src/main/java/org/apache/plc4x/java/modbus/ModbusSerialPlcDriver.java b/plc4j/protocols/modbus/src/main/java/org/apache/plc4x/java/modbus/ModbusPlcDriver.java
similarity index 70%
rename from plc4j/protocols/modbus/src/main/java/org/apache/plc4x/java/modbus/ModbusSerialPlcDriver.java
rename to plc4j/protocols/modbus/src/main/java/org/apache/plc4x/java/modbus/ModbusPlcDriver.java
index 739ebb1..7c211be 100644
--- a/plc4j/protocols/modbus/src/main/java/org/apache/plc4x/java/modbus/ModbusSerialPlcDriver.java
+++ b/plc4j/protocols/modbus/src/main/java/org/apache/plc4x/java/modbus/ModbusPlcDriver.java
@@ -33,20 +33,19 @@ import java.util.regex.Pattern;
 /**
  * Implementation of the Modbus protocol, based on:
  * - Modbus Protocol (http://www.modbus.org/docs/Modbus_Application_Protocol_V1_1b3.pdf)
- * - TCP
  */
-public class ModbusSerialPlcDriver implements PlcDriver {
+public class ModbusPlcDriver implements PlcDriver {
 
-    private static final Pattern MODBUS_SERIAL_URI_PATTERN = Pattern.compile("^modbus-serial://(?<port>.*)(?<params>\\?.*)?");
+    private static final Pattern MODBUS_SERIAL_URI_PATTERN = Pattern.compile("^modbus:(?<subType>.*)://(?<port>.*)|(?<host>.*)(?<params>\\?.*)?");
 
     @Override
     public String getProtocolCode() {
-        return "modbus-serial";
+        return "modbus";
     }
 
     @Override
     public String getProtocolName() {
-        return "Modbus (Serial)";
+        return "Modbus (TCP / Serial)";
     }
 
     @Override
@@ -54,12 +53,25 @@ public class ModbusSerialPlcDriver implements PlcDriver {
         Matcher matcher = MODBUS_SERIAL_URI_PATTERN.matcher(url);
         if (!matcher.matches()) {
             throw new PlcConnectionException(
-                "Connection url doesn't match the format 'modbus-serial://{port}'");
+                "Connection url doesn't match the format 'modbus:{type}//{port|host}'");
         }
 
-        String port = matcher.group("port");
+        String subType = matcher.group("subType");
         String params = matcher.group("params") != null ? matcher.group("params").substring(1) : null;
-        return new ModbusSerialPlcConnection(port, params);
+        if("tcp".equalsIgnoreCase(subType)) {
+            String hostName = matcher.group("host");
+            try {
+                InetAddress host = InetAddress.getByName(hostName);
+                return new ModbusTcpPlcConnection(host, params);
+            } catch (UnknownHostException e) {
+                throw new PlcConnectionException("Unknown host" + hostName, e);
+            }
+        } else if("serial".equalsIgnoreCase(subType)) {
+            String port = matcher.group("port");
+            return new ModbusSerialPlcConnection(port, params);
+        } else {
+            throw new PlcConnectionException("Unknown sub-type " + subType);
+        }
     }
 
     @Override
diff --git a/plc4j/protocols/modbus/src/main/java/org/apache/plc4x/java/modbus/ModbusTcpPlcDriver.java b/plc4j/protocols/modbus/src/main/java/org/apache/plc4x/java/modbus/ModbusTcpPlcDriver.java
deleted file mode 100644
index 225429d..0000000
--- a/plc4j/protocols/modbus/src/main/java/org/apache/plc4x/java/modbus/ModbusTcpPlcDriver.java
+++ /dev/null
@@ -1,76 +0,0 @@
-/*
-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.modbus;
-
-import org.apache.plc4x.java.api.PlcDriver;
-import org.apache.plc4x.java.api.authentication.PlcAuthentication;
-import org.apache.plc4x.java.api.connection.PlcConnection;
-import org.apache.plc4x.java.api.exceptions.PlcConnectionException;
-import org.apache.plc4x.java.modbus.connection.ModbusSerialPlcConnection;
-import org.apache.plc4x.java.modbus.connection.ModbusTcpPlcConnection;
-
-import java.net.InetAddress;
-import java.net.UnknownHostException;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-
-/**
- * Implementation of the Modbus protocol, based on:
- * - Modbus Protocol (http://www.modbus.org/docs/Modbus_Application_Protocol_V1_1b3.pdf)
- * - Modbus TCP Protocol (http://www.modbus.org/docs/Modbus_Messaging_Implementation_Guide_V1_0b.pdf)
- * - TCP
- */
-public class ModbusTcpPlcDriver implements PlcDriver {
-
-    private static final Pattern MODBUS_TCP_URI_PATTERN = Pattern.compile("^modbus-tcp://(?<host>.*)(?<params>\\?.*)?");
-
-    @Override
-    public String getProtocolCode() {
-        return "modbus-tcp";
-    }
-
-    @Override
-    public String getProtocolName() {
-        return "Modbus (TCP)";
-    }
-
-    @Override
-    public PlcConnection connect(String url) throws PlcConnectionException {
-        Matcher matcher = MODBUS_TCP_URI_PATTERN.matcher(url);
-        if (!matcher.matches()) {
-            throw new PlcConnectionException(
-                "Connection url doesn't match the format 'modbus-tcp://{host|ip}'");
-        }
-
-        String host = matcher.group("host");
-        String params = matcher.group("params") != null ? matcher.group("params").substring(1) : null;
-        try {
-            InetAddress serverInetAddress = InetAddress.getByName(host);
-            return new ModbusTcpPlcConnection(serverInetAddress, params);
-        } catch (UnknownHostException e) {
-            throw new PlcConnectionException("Error parsing address", e);
-        }
-    }
-
-    @Override
-    public PlcConnection connect(String url, PlcAuthentication authentication) throws PlcConnectionException {
-        throw new PlcConnectionException("Modbus connections don't support authentication.");
-    }
-
-}
diff --git a/plc4j/protocols/modbus/src/main/resources/META-INF/services/org.apache.plc4x.java.api.PlcDriver b/plc4j/protocols/modbus/src/main/resources/META-INF/services/org.apache.plc4x.java.api.PlcDriver
index a199527..8df2ee4 100644
--- a/plc4j/protocols/modbus/src/main/resources/META-INF/services/org.apache.plc4x.java.api.PlcDriver
+++ b/plc4j/protocols/modbus/src/main/resources/META-INF/services/org.apache.plc4x.java.api.PlcDriver
@@ -16,5 +16,4 @@
 # specific language governing permissions and limitations
 # under the License.
 #
-org.apache.plc4x.java.modbus.ModbusTcpPlcDriver
-org.apache.plc4x.java.modbus.ModbusSerialPlcDriver
+org.apache.plc4x.java.modbus.ModbusPlcDriver

-- 
To stop receiving notification emails like this one, please contact
cdutz@apache.org.