asfgit closed pull request #9: Added some javadoc to S7 communication path and 
several todos that ma…
URL: https://github.com/apache/incubator-plc4x/pull/9
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

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 96db08f0e..456f1f3d9 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 @@ Licensed to the Apache Software Foundation (ASF) under one
 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 78ebdea77..136f47848 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 @@ Licensed to the Apache Software Foundation (ASF) under one
 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 bc77dc518..3f4f68e0a 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 @@ Licensed to the Apache Software Foundation (ASF) under one
 
 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 boolean equals(Object o) {
             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 f7736e879..97cb36e7c 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 @@ Licensed to the Apache Software Foundation (ASF) under one
 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 boolean equals(Object o) {
             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 39a3ec238..94b53cc25 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 @@ Licensed to the Apache Software Foundation (ASF) under one
 
 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 e1d44ec69..d1101d5ac 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 @@ Licensed to the Apache Software Foundation (ASF) under one
 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/connection/S7PlcConnection.java
 
b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/connection/S7PlcConnection.java
index f17ec90b4..33c033540 100644
--- 
a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/connection/S7PlcConnection.java
+++ 
b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/connection/S7PlcConnection.java
@@ -53,6 +53,24 @@ Licensed to the Apache Software Foundation (ASF) under one
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+/**
+ * Class implementing the Connection handling for Siemens S7.
+ * The adressing of Values in S7 works as follows:
+ *
+ * For adressing values from Datablocks the following syntax is used:
+ * <pre>
+ *     DATA_BLOCKS/{blockNumer}/{byteOffset}
+ * </pre>
+ *
+ * For adressing data from other memory segments like I/O, Markers, ...
+ * <pre>
+ *     {memory area}/{byte offset}
+ *     or
+ *     {memory area}/{byte offset}/{bit offset}
+ * </pre>
+ * where the {bit-offset} is optional.
+ * All Available Memory Areas for this mode are defined in the {@link 
MemoryArea} enum.
+ */
 public class S7PlcConnection extends AbstractPlcConnection implements 
PlcReader, PlcWriter {
 
     private static final int ISO_ON_TCP_PORT = 102;
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 894a560eb..05163f87e 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 @@ Licensed to the Apache Software Foundation (ASF) under one
 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 ba14d3a6a..902ab0478 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 @@ Licensed to the Apache Software Foundation (ASF) under one
 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 7bd56c5fd..af39df6d3 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 @@ Licensed to the Apache Software Foundation (ASF) under one
 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 7ed5ab6fb..b769ab46e 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 @@ Licensed to the Apache Software Foundation (ASF) under one
 
 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 90b227033..b3e892211 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 @@ Licensed to the Apache Software Foundation (ASF) under one
 
 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 00343f4bd..b76d57fdc 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 @@ Licensed to the Apache Software Foundation (ASF) under one
 
 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 d944a4068..cea2c8cae 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 @@ Licensed to the Apache Software Foundation (ASF) under one
 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 e560c8fbb..663194859 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 @@ Licensed to the Apache Software Foundation (ASF) under one
 
 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;


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to