package org.apache.plc4x.java.api.connection;

import org.apache.plc4x.java.api.exceptions.PlcConnectionException;
import org.apache.plc4x.java.api.exceptions.PlcInvalidAddressException;
import org.apache.plc4x.java.api.model.Address;

import java.util.Optional;

/**
 * Interface defining the most basic methods a PLC4X connection should support.
 * This generally handles the connection establishment itself and the parsing of
 * address strings to the platform dependent Address instances.
 * <p>
 * The individual operations are then defined by other interfaces within this package.
 */
public interface PlcConnection extends AutoCloseable {

    /**
     * Established the connection to the remote PLC.
     *
     * @throws PlcConnectionException an exception if the connection attempt failed.
     */
    void connect() throws PlcConnectionException;

    /**
     * Returns true if the PlcConnection is connected to a remote PLC.
     *
     * @return true, if connected, false, if not.
     */
    boolean isConnected();

    /**
     * Closes the connection to the remote PLC.
     *
     * @throws Exception an exception if shutting down the connection failed.
     */
    @Override
    void close() throws Exception;

    /**
     * Parses a PLC/protocol dependent query string into a PlcQuery object.
     *
     * @param queryString String representation of a query for the current type of PLC/protocol.
     * @return PlcQuery object containing all query data.
     * @throws PlcInvalidQueryException an exception if there was a problem parsing the query string.
     */
    PlcQuery prepareQuery(String queryString) throws PlcInvalidQueryException;

    Optional<PlcReader> getReader();

    Optional<PlcWriter> getWriter();

    Optional<PlcSubscriber> getSubscriber();
}

General

• I ensured all of our classes have the "Plc" prefix, as I did encounter situations in which very generic type names had collisions and I had to use the fully qualified class name (including the package name). The "Plc" prefix reduces this risk.
• This model is now simplified as it currently doesn't support reading of multi-values. There are multiple options to add Multi-Values, but I haven't decided on which option I like best:
  • Rename PlcRequest/ResponseItem to PlcSingleValueRequest/ResponseItem and add a Multi-Value counterpart, wich handles the multi-value.
  • Add a PlcMultiValueRequest/ResponseItem, which simply references multiple PlcRequest/ResponseItem objects and provides a "get(index)" method.
  • Add a getNumValues() (Not insisting on this name) method and give each of the accessors an "index" property (eventually additionally a no-args version which defaults to index 0) (I think I prefer this one but didn't want to blow up the class-diagram too much).

PlcQuery (Aka Address)

• I Renamed Address to PlcQuery
• I still think there is no immediate need to define any fields or methods in this interface as I haven't seen any common ground yet. Even if we are currently adding "TYPE" information to most queries, in Modbus the type is directly tied to the address you are reading. So If you are reading a "Register" or a "Coil", the datatype is fixed.

PlcConnection

...

• I removed the "getLister" method as I haven't seen any protocol or use-case where this actually made sense.

  I added the prefix "Plc" to the Query as I did notice, that when working with other sources or destinations the class Query/Address is a little vague and I did have to specify some "Address" types with their full package name due to Class name collisions at least once

• I changed the "parseparseAddress" to "prepareprepareQuery" as it sort of correlates to the "prepareStatement" of JDBC ... and some times I could imagine, that more work is involved than simple parsing of a string (For Beckhoff ADS and EtherNet/IP this method could actively connect to the corresponding resource so in future read operations only the connection-id has to be provided)

Code Block
language java
package org.apache.plc4x.java.api.model; /** * Base type for all query types. * Typically every driver provides an implementation of this interface in order * to be able to describe the query for a resource. As this is completely tied to * the implemented protocol, this base interface makes absolutely no assumption to * any information it should provide. * * In order to stay platform and protocol independent every driver connection implementation * provides a prepareQuery(String) method that is able to parse a string representation of * a query into it's individual query type. Manually constructing Query objects * manually makes the solution less independent from the protocol, but might be faster. */ public interface PlcQuery { }

Notes to the above class:

• I still think there is no immediate need to define any fields or methods in this interface as I haven't seen any common ground yet. Even if we are currently adding "TYPE" information to most queries, in Modbus the type is directly tied to the address you are reading. So If you are reading a "Register" or a "Coil", the datatype is fixed.

PlcRequest & PlcResponse

• I changed the List for referencing the items with a Map, so we can directly reference items.
• I removed the "single-item" convenience methods.
• I added getItem(name) methods for directly referencing a given item.

PlcRequestItem & PlcResponseItem

• I removed the datatype both from the properties as well as the generic type as I think type information - if needed - should be inside the query object.
• A request item now has a name, which allows to access it via it's name.
• PlcReadRequestItem no longer has a "size", as that's part of the query now. Some protocols ... such as Modbus (I think) ... don't support this concept.

• NOTE: I would suggest to not remove PlcReadRequestItem or PlcWriteResponseItem class entirely, even if we could simply make PlcRequestItem and PlcResponseItem non-abstract. For simplicity of the API I would opt for keeping it ... so we always have matching pairs of PlcXYZRequestItem and PlcXYZResponseItems.

Example Code

Here comes some example code to demonstrate how a Programm using this API could look like.

    public static void main(String[] args) throws Exception {
        PlcDriverManager manager = new PlcDriverManager

package org.apache.plc4x.java.api.messages;

import org.apache.plc4x.java.api.messages.items.RequestItem;

import java.util.*;

/**
 * Base type for all messages sent from the plc4x system to a connected plc.
 * @param <REQUEST_ITEM> 
 */
public abstract class PlcRequest<REQUEST_ITEM extends RequestItem> implements PlcMessage {

    protected final Map<String, REQUEST_ITEM> requestItemMap;

    public PlcRequest() {
        this.requestItemMap = new HashMap<>();
    }

    publictry PlcRequest(List<REQUEST_ITEM> requestItems) {
        this();
        Objects.requireNonNull(requestItems, "Request items must not be null");
        // Add all the items to the internal map.
        requestItems.stream().map(item -> requestItemMap.put(item.getName()));
    }

    public void addItem(REQUEST_ITEM requestItem) {
        Objects.requireNonNull(requestItem, "Request item must not be null");
PlcConnection conn = manager.getConnection("s7://192.168.167.211/0/0")) {
            Optional<PlcReader> optionalReader = conn.getReader();
            if requestItemMap.put(requestItem!optionalReader.getNameisPresent(), requestItem);
    } {
    
    public REQUEST_ITEM getItem(String name) {
        Objects.requireNonNull(name, "Name must not be null")return;
        return requestItemMap.get(name);
    }

    public Collection<REQUEST_ITEM> getRequestItems() {
     PlcReader Reader = return requestItemMapoptionalReader.valuesget();
    }

			PlcReadRequest request = new public int getNumberOfItems() {
        return getRequestItems().size();
    }
    
    public boolean isEmpty() {
        return requestItemMap.isEmpty();
    }
    
}

Notes to the above class:

• I changed the list with a Map, so we can directly reference items.
• I removed the "single-item" methods.
• For clarity reasons, I stripped the boilerplate: toString, equals, hashCode methods

package org.apache.plc4x.java.api.messages.items;

import org.apache.plc4x.java.api.model.PlcQuery;

import java.util.Objects;

/**
 * Wrapper Object to bind a name to a {@link PlcQuery} .
 */
public abstract class PlcRequestItem {
    
    private final String name;
    private final PlcQuery query;

    public RequestItem(String name, PlcQuery query) {
PlcReadRequest(
				new PlcReadRequestItem("parameterX", conn.prepareQuery("%DB8.DBX3:INT")),
				new PlcReadRequestItem("parameterY", conn.prepareQuery("%DB9.DBW4:DOUBLE")),
				new PlcReadRequestItem("others", conn.prepareQuery("%DB2.DBW2:BYTE[10]")));

            CompletableFuture<PlcReadResponse> future = reader.read(request);

            PlcReadResponse response = future.get();


			// Access an item by it's name.
            ObjectsSystem.out.requireNonNullprintln(name, "Name must not be null");
        Objects.requireNonNull(query, "Query type must not be null");
        this.name = name;
        this.query = query;
    }

    public String getName() {
        return null;
    }
    
    public PlcQuery getQuery() {
        return query;
    }

}


...


public class PlcReadRequestItem extends PlcRequestItem {

    public RequestItem(String name, PlcQuery query) {
		super(name, query);
	}

}

Notes to the above class:

• Added the "Plc" prefix to the classnames.
• I removed the datatype both from the properties as well as the generic type as I think type information - if needed - should be inside the query object.
• For clarity reasons, I stripped the boilerplate: toString, equals, hashCode methods
• PlcReadRequestItem no longer has a "size", as that's part of the query now. Some protocols ... such as Modbus (I think) ... don't support this concept.
• NOTE: I would suggest to not remove PlcReadRequestItem class entirely, even if we could simply make PlcRequestItem non-abstract. For simplicity of the API I would opt for keeping it ... so we always have matching pairs of PlcXYZRequestItem and PlcXYZResponseItems.
• (Eventually an Idea to define an Enum of operation types?!? ... then each item could return the operation-type)

package org.apache.plc4x.java.api.messages.items;

import org.apache.plc4x.java.api.types.ResponseCode;

import java.util.Objects;

public abstract class PlcResponseItem {

    private final PlcRequestItem requestItem;

    private final ResponseCode responseCode;

    public ResponseItem(PlcRequestItem requestItem, ResponseCode responseCode) {
        Objects.requireNonNull(requestItem,"Request item must not be null");
        Objects.requireNonNull(responseCode,"Response code must not be null");
        this.requestItem = requestItem;
        this.responseCode = responseCode;
    }

    public PlcRequestItem getRequestItem() {
        return requestItem;
    }

    public ResponseCode getResponseCode() {
        return responseCode;
    }

}

Notes to the above class:

• Added the "Plc" prefix to the classname.
• I removed the datatype both from the properties as well as the generic type as I think type information - if needed - should be inside the query object.
• For clarity reasons, I stripped the boilerplate: toString, equals, hashCode methods

...

Notes to the above class:

• Blah blah blah

...

Notes to the above class:

• Blah blah blah

...

Notes to the above class:

...

Parameter X: " + response.getInteger("parameterX"));


			// Iterate over all items
			for(PlcReadResponseItem item: response.getItems()) {
				// Conditionally handle an item by it's name.
				if("parameterY".equals(item.getName()) {
					// If using getInteger without an index, the index 0 is used.
					System.out.println("Parameter y: " + item.getDouble());
				}
				else if("others".equals(item.getName()) {
					System.out.println("Others:");
					// Handle multi-value items.
					for(int i = 0; i < item.getNumValues(); i++) {
						// Intentionally using short instead of byte 
						// In this case it will return Byte[i] but convert that into a Short.
						System.out.println(" - " + item.getShort(i));
					}
				}
			}
        }
    }