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