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/08/02 09:20:12 UTC

[incubator-plc4x] 01/04: Added some javadoc to S7 communication path and several todos that may be helpful.

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

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

commit 75bf337d024698a9319dcb540fcb24fa3609c092
Author: julian <j....@pragmaticminds.de>
AuthorDate: Wed Aug 1 14:10:38 2018 +0200

    Added some javadoc to S7 communication path and several todos that may be helpful.
---
 .../apache/plc4x/java/api/messages/PlcReadRequest.java    | 15 +++++++++++++++
 .../apache/plc4x/java/api/messages/PlcReadResponse.java   | 10 ++++++++++
 .../plc4x/java/api/messages/items/ReadRequestItem.java    | 14 ++++++++++++++
 .../plc4x/java/api/messages/items/ReadResponseItem.java   |  7 +++++++
 .../apache/plc4x/java/api/messages/items/RequestItem.java |  4 ++++
 .../api/messages/specific/TypeSafePlcReadRequest.java     | 10 ++++++++++
 .../org/apache/plc4x/java/s7/netty/Plc4XS7Protocol.java   | 11 +++++++++++
 .../java/org/apache/plc4x/java/s7/netty/S7Protocol.java   |  9 +++++++++
 .../plc4x/java/s7/netty/model/messages/S7Message.java     | 10 ++++++++++
 .../java/s7/netty/model/messages/S7RequestMessage.java    |  4 ++++
 .../java/s7/netty/model/messages/S7ResponseMessage.java   |  3 +++
 .../plc4x/java/s7/netty/model/params/VarParameter.java    |  7 +++++++
 .../netty/model/params/items/S7AnyVarParameterItem.java   | 15 +++++++++++++++
 .../plc4x/java/s7/netty/model/payloads/VarPayload.java    |  5 +++++
 14 files changed, 124 insertions(+)

diff --git a/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/PlcReadRequest.java b/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/PlcReadRequest.java
index 96db08f..456f1f3 100644
--- a/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/PlcReadRequest.java
+++ b/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/PlcReadRequest.java
@@ -25,9 +25,24 @@ import org.apache.plc4x.java.api.model.Address;
 import java.util.List;
 import java.util.Objects;
 
+/**
+ * Request to read one or more values from a plc.
+ * The {@link PlcReadRequest} is a container for one or more {@link ReadRequestItem}s that are contained.
+ *
+ * By default this is NOT typesafe as it can contain {@link ReadRequestItem}s for different types.
+ * If there are only {@link ReadRequestItem}s of the same type one can use the {@link TypeSafePlcReadRequest} to enforce
+ * type safety.
+ *
+ * Provides a builder for object construction through {@link PlcReadRequest#builder()}.
+ *
+ * TODO 01.08.2018 jf: Can we make constructors private and force users to use the Builder to enforce immutability?
+ *
+ * @see TypeSafePlcReadRequest
+ */
 public class PlcReadRequest extends PlcRequest<ReadRequestItem<?>> {
 
     public PlcReadRequest() {
+        // Exists for construction of inherited classes
     }
 
     public PlcReadRequest(ReadRequestItem<?> requestItem) {
diff --git a/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/PlcReadResponse.java b/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/PlcReadResponse.java
index 78ebdea..136f478 100644
--- a/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/PlcReadResponse.java
+++ b/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/PlcReadResponse.java
@@ -25,6 +25,16 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 
+/**
+ * Response to a {@link PlcReadRequest}.
+ * Contains the values read from the PLC but untyped.
+ *
+ * Values are extracted using the {@link ReadRequestItem}s that were send in the read request.
+ *
+ * If only a variables of one type are requested it is better to use
+ * {@link org.apache.plc4x.java.api.messages.specific.TypeSafePlcReadRequest} which leads to a
+ * {@link org.apache.plc4x.java.api.messages.specific.TypeSafePlcReadResponse}.
+ */
 public class PlcReadResponse extends PlcResponse<PlcReadRequest, ReadResponseItem<?>, ReadRequestItem<?>> {
 
     public PlcReadResponse(PlcReadRequest request, ReadResponseItem<?> responseItems) {
diff --git a/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/items/ReadRequestItem.java b/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/items/ReadRequestItem.java
index bc77dc5..3f4f68e 100644
--- a/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/items/ReadRequestItem.java
+++ b/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/items/ReadRequestItem.java
@@ -22,6 +22,18 @@ import org.apache.plc4x.java.api.model.Address;
 
 import java.util.Objects;
 
+/**
+ * Encapsulats one {@link RequestItem} that could be read multiple times, i.e., from the given Address on
+ * {@link ReadRequestItem#size} number of Items with equal datatype are read.
+ *
+ * Thus,
+ * <pre>
+ *     new ReadRequestItem(Int.class, adress, 5)
+ * </pre>
+ * basically reads 5 consecutive integers starting at the given {@link Address}.
+ *
+ * @param <T> Generic Type of expected Datatype.
+ */
 public class ReadRequestItem<T> extends RequestItem<T> {
 
     private final int size;
@@ -52,6 +64,8 @@ public class ReadRequestItem<T> extends RequestItem<T> {
             return false;
         }
         ReadRequestItem<?> that = (ReadRequestItem<?>) o;
+        // TODO 01.18.18 jf: we should also call the comparison of super at least otherwise this can lead to unwanted behavior.
+        // Perhaps we should generate a UUID or something for each ReadRequest to have a good equality comparison
         return size == that.size;
     }
 
diff --git a/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/items/ReadResponseItem.java b/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/items/ReadResponseItem.java
index f7736e8..97cb36e 100644
--- a/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/items/ReadResponseItem.java
+++ b/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/items/ReadResponseItem.java
@@ -24,6 +24,12 @@ import java.util.Arrays;
 import java.util.List;
 import java.util.Objects;
 
+/**
+ * Response to a {@link ReadRequestItem}.
+ * Can contain a list of values if the size in {@link ReadRequestItem} is larger zero.
+ *
+ * @param <T>
+ */
 public class ReadResponseItem<T> extends ResponseItem<ReadRequestItem<T>> {
 
     private final List<T> values;
@@ -60,6 +66,7 @@ public class ReadResponseItem<T> extends ResponseItem<ReadRequestItem<T>> {
             return false;
         }
         ReadResponseItem<?> that = (ReadResponseItem<?>) o;
+        // TODO 01.08.18 jf: should we also use a UUID here or at least a check for equal request as this could lead to effects due to unwanted equality (same for hashCode).
         return Objects.equals(values, that.values);
     }
 
diff --git a/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/items/RequestItem.java b/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/items/RequestItem.java
index 39a3ec2..94b53cc 100644
--- a/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/items/RequestItem.java
+++ b/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/items/RequestItem.java
@@ -22,6 +22,10 @@ import org.apache.plc4x.java.api.model.Address;
 
 import java.util.Objects;
 
+/**
+ * Wrapper Object to encapsulate an {@link Address} and the expected datatype as {@link Class}.
+ * @param <DATA_TYPE> Generic Type of data at address
+ */
 public abstract class RequestItem<DATA_TYPE> {
 
     private final Class<DATA_TYPE> datatype;
diff --git a/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/specific/TypeSafePlcReadRequest.java b/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/specific/TypeSafePlcReadRequest.java
index e1d44ec..d1101d5 100644
--- a/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/specific/TypeSafePlcReadRequest.java
+++ b/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/specific/TypeSafePlcReadRequest.java
@@ -26,6 +26,16 @@ import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
 
+/**
+ * Type safe wrapper for {@link PlcReadRequest}.
+ * Can be used if requesting only values from the same type.
+ *
+ * Can be constructed using the {@link PlcReadRequest#builder()} method.
+ *
+ * TODO 01.08.18 jf: Could we hide constructors from users and enforce usage of the PlcReadRequest.builder?
+ *
+ * @param <T> Type of the {@link Class} of requested values
+ */
 public class TypeSafePlcReadRequest<T> extends PlcReadRequest {
 
     private final Class<T> dataType;
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 894a560..05163f8 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
@@ -54,6 +54,17 @@ import java.util.concurrent.atomic.AtomicInteger;
 import static org.apache.plc4x.java.s7.netty.util.S7TypeDecoder.decodeData;
 import static org.apache.plc4x.java.s7.netty.util.S7TypeEncoder.encodeData;
 
+/**
+ * This layer transforms between {@link PlcRequestContainer}s {@link S7Message}s.
+ * And stores all "in-flight" requests in an internal structure ({@link Plc4XS7Protocol#requests}).
+ *
+ * TODO 01.08.18, jf: is a Cleanup of the requests map necessary? Can it occur that for a request no response is generated that leads to success or failure?
+ *
+ * While sending a request, a {@link S7RequestMessage} is generated and send downstream (to the {@link S7Protocol}.
+ *
+ * When a {@link S7ResponseMessage} is received it takes the existing request container from its Map and finishes
+ * the {@link PlcRequestContainer}s future with the {@link PlcResponse}.
+ */
 public class Plc4XS7Protocol extends PlcMessageToMessageCodec<S7Message, PlcRequestContainer> {
 
     private static final AtomicInteger tpduGenerator = new AtomicInteger(1);
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 ba14d3a..902ab04 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
@@ -51,6 +51,15 @@ import org.slf4j.LoggerFactory;
 import java.lang.reflect.Field;
 import java.util.*;
 
+/**
+ * Communication Layer between the Application level ({@link Plc4XS7Protocol}) and the lower level (tcp) that sends and receives {@link S7Message}s.
+ * This layer also handles the control over the "wire", i.e., the queues of incoming and outgoing messages.
+ * Furthermore, here {@link S7Message}s are marshalled and unmarshalled to {@link ByteBuf}s to be send over wire.
+ *
+ * Before messages are send to the wire an optional {@link S7MessageProcessor} can be applied.
+ *
+ * @see S7MessageProcessor
+ */
 public class S7Protocol extends ChannelDuplexHandler {
 
     private static final byte S7_PROTOCOL_MAGIC_NUMBER = 0x32;
diff --git a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/messages/S7Message.java b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/messages/S7Message.java
index 7bd56c5..af39df6 100644
--- a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/messages/S7Message.java
+++ b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/messages/S7Message.java
@@ -27,6 +27,16 @@ import org.apache.plc4x.java.s7.netty.model.types.MessageType;
 import java.util.List;
 import java.util.Optional;
 
+/**
+ * Container for Request and Responses to and from S7.
+ * Contains the following information
+ * <ul>
+ *     <li>messageType - type of the message as {@link MessageType}</li>
+ *     <li>tpudReference - internal counter from {@link org.apache.plc4x.java.s7.netty.Plc4XS7Protocol} for tracking and correlation</li>
+ *     <li>parameters - description of command(s) to perform (read or write) and the exact address range</li>
+ *     <li>payloads - possible payload (for writes)</li>
+ * </ul>
+ */
 public abstract class S7Message extends RawMessage {
 
     private final MessageType messageType;
diff --git a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/messages/S7RequestMessage.java b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/messages/S7RequestMessage.java
index 7ed5ab6..b769ab4 100644
--- a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/messages/S7RequestMessage.java
+++ b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/messages/S7RequestMessage.java
@@ -25,6 +25,10 @@ import org.apache.plc4x.java.s7.netty.model.types.MessageType;
 
 import java.util.List;
 
+/**
+ * Container Object for Requests to S7 which additionally stores information if the request was acknowledged (by the PLC?).
+ * @see S7Message for the other attributes.
+ */
 public class S7RequestMessage extends S7Message {
 
     private boolean acknowledged;
diff --git a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/messages/S7ResponseMessage.java b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/messages/S7ResponseMessage.java
index 90b2270..b3e8922 100644
--- a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/messages/S7ResponseMessage.java
+++ b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/messages/S7ResponseMessage.java
@@ -24,6 +24,9 @@ import org.apache.plc4x.java.s7.netty.model.types.MessageType;
 
 import java.util.List;
 
+/**
+ * Response from S7 PLC that additionally contains error information.
+ */
 public class S7ResponseMessage extends S7Message {
 
     private final byte errorClass;
diff --git a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/params/VarParameter.java b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/params/VarParameter.java
index 00343f4..b76d57f 100644
--- a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/params/VarParameter.java
+++ b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/params/VarParameter.java
@@ -24,6 +24,13 @@ import org.apache.plc4x.java.s7.netty.model.types.ParameterType;
 
 import java.util.List;
 
+/**
+ * "Command" Message to inform PLC of wanted operation.
+ * Also contains {@link VarParameter#items} that hold detailed information on the "Command", e.g.,
+ * addresses to read or to write to.
+ *
+ * TODO 01.08.18 jf: Could this be renamed to Something like Command as this seems to be the command message pattern?
+ */
 public class VarParameter implements S7Parameter {
 
     private final ParameterType type;
diff --git a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/params/items/S7AnyVarParameterItem.java b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/params/items/S7AnyVarParameterItem.java
index d944a40..cea2c8c 100644
--- a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/params/items/S7AnyVarParameterItem.java
+++ b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/params/items/S7AnyVarParameterItem.java
@@ -23,6 +23,21 @@ import org.apache.plc4x.java.s7.netty.model.types.SpecificationType;
 import org.apache.plc4x.java.s7.netty.model.types.TransportSize;
 import org.apache.plc4x.java.s7.netty.model.types.VariableAddressingMode;
 
+/**
+ * "Low-level" description of S7 Address range and the necessary size for transportation of values.
+ * Is used as Arguments of {@link org.apache.plc4x.java.s7.netty.model.params.VarParameter} object.
+ *
+ * Contains the information to read one or more sequential values of the same datatype starting from given offset in a memory region
+ * and also contains the transportation size of the datatype read.
+ *
+ * In detail:
+ * <ul>
+ *     <li>transportSize - {@link TransportSize} of the datatype</li>
+ *     <li>numElements - number of consecutive elements to be read</li>
+ *     <li>dataBlockNumber - number of the datablock</li>
+ *     <li>bit / byteOffset where the adress starts</li>
+ * </ul>
+ */
 public class S7AnyVarParameterItem implements VarParameterItem {
 
     private final SpecificationType specificationType;
diff --git a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/payloads/VarPayload.java b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/payloads/VarPayload.java
index e560c8f..6631948 100644
--- a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/payloads/VarPayload.java
+++ b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/payloads/VarPayload.java
@@ -23,6 +23,11 @@ import org.apache.plc4x.java.s7.netty.model.types.ParameterType;
 
 import java.util.List;
 
+/**
+ * Used for writes to S7 as part of a Valid {@link org.apache.plc4x.java.s7.netty.model.messages.S7RequestMessage} together
+ * with a suitable {@link org.apache.plc4x.java.s7.netty.model.params.VarParameter} Object.
+ * Contains the values to write as {@link VarPayloadItem}s.
+ */
 public class VarPayload implements S7Payload {
 
     private final ParameterType parameterType;