[modbus] BREAKING CHANGE in defineModbusProtocol method

this allows to not only have the modbus Unit-ID but the complete Component inside the Protocol and Tasks. One benefit of this are improved log messages
OpenEMS · Sep 13, 2018 · 5808942 · 5808942
1 parent af835d0
commit 5808942
Show file tree

Hide file tree

Showing 22 changed files with 120 additions and 108 deletions.
diff --git a/io.openems.edge.battery.soltaro/src/io/openems/edge/battery/soltaro/SoltaroRack.java b/io.openems.edge.battery.soltaro/src/io/openems/edge/battery/soltaro/SoltaroRack.java
@@ -636,8 +636,8 @@ public Doc doc() {
 	}
 
 	@Override
-	protected ModbusProtocol defineModbusProtocol(int unitId) {
-		return new ModbusProtocol(unitId, //
+	protected ModbusProtocol defineModbusProtocol() {
+		return new ModbusProtocol(this, //
 				new FC6WriteRegisterTask(0x2010,  //
 						m(SoltaroRack.ChannelId.BMS_CONTACTOR_CONTROL, new UnsignedWordElement(0x2010)) //
 				) , //

diff --git a/...e.bridge.modbus/src/io/openems/edge/bridge/modbus/api/AbstractOpenemsModbusComponent.java b/...e.bridge.modbus/src/io/openems/edge/bridge/modbus/api/AbstractOpenemsModbusComponent.java
@@ -41,26 +41,21 @@ protected void activate(String id) {
 	/**
 	 * Call this method from Component implementations activate().
 	 * 
-	 * @param context
-	 *            ComponentContext of this component. Receive it from parameter
-	 *            for @Activate
-	 * @param servicePid
-	 *            The service_pid of this Component. Typically
-	 *            'config.service_pid()'
-	 * @param id
-	 *            ID of this component. Typically 'config.id()'
-	 * @param enabled
-	 *            Whether the component should be enabled. Typically
-	 *            'config.enabled()'
-	 * @param unitId
-	 *            Unit-ID of the Modbus target
-	 * @param cm
-	 *            An instance of ConfigurationAdmin. Receive it using @Reference
-	 * @param modbusReference
-	 *            The name of the @Reference setter method for the Modbus bridge -
-	 *            e.g. 'Modbus' if you have a setModbus()-method
-	 * @param modbusId
-	 *            The ID of the Modbus brige. Typically 'config.modbus_id()'
+	 * @param context         ComponentContext of this component. Receive it from
+	 *                        parameter for @Activate
+	 * @param servicePid      The service_pid of this Component. Typically
+	 *                        'config.service_pid()'
+	 * @param id              ID of this component. Typically 'config.id()'
+	 * @param enabled         Whether the component should be enabled. Typically
+	 *                        'config.enabled()'
+	 * @param unitId          Unit-ID of the Modbus target
+	 * @param cm              An instance of ConfigurationAdmin. Receive it
+	 *                        using @Reference
+	 * @param modbusReference The name of the @Reference setter method for the
+	 *                        Modbus bridge - e.g. 'Modbus' if you have a
+	 *                        setModbus()-method
+	 * @param modbusId        The ID of the Modbus brige. Typically
+	 *                        'config.modbus_id()'
 	 */
 	protected void activate(ComponentContext context, String servicePid, String id, boolean enabled, int unitId,
 			ConfigurationAdmin cm, String modbusReference, String modbusId) {
@@ -113,18 +108,16 @@ private ModbusProtocol getModbusProtocol(int unitId) {
 		if (protocol != null) {
 			return protocol;
 		}
-		this.protocol = defineModbusProtocol(unitId);
+		this.protocol = this.defineModbusProtocol();
 		return this.protocol;
 	}
 
 	/**
 	 * Defines the Modbus protocol
 	 * 
-	 * @param unitId
-	 * 
 	 * @return
 	 */
-	protected abstract ModbusProtocol defineModbusProtocol(int unitId);
+	protected abstract ModbusProtocol defineModbusProtocol();
 
 	/**
 	 * Maps an Element to one or more ModbusChannels using converters, that convert

diff --git a/io.openems.edge.bridge.modbus/src/io/openems/edge/bridge/modbus/api/ModbusProtocol.java b/io.openems.edge.bridge.modbus/src/io/openems/edge/bridge/modbus/api/ModbusProtocol.java
@@ -17,9 +17,9 @@ public class ModbusProtocol {
 	private final Logger log = LoggerFactory.getLogger(ModbusProtocol.class);
 
 	/**
-	 * The Unit-ID for this Modbus connection
+	 * The Parent component
 	 */
-	private final int unitId;
+	private final AbstractOpenemsModbusComponent parent;
 
 	/**
 	 * TaskManager for ReadTasks
@@ -31,20 +31,16 @@ public class ModbusProtocol {
 	 */
 	private final TaskManager<WriteTask> writeTaskManager = new TaskManager<>();
 
-	public ModbusProtocol(int unitId, Task... tasks) {
-		this.unitId = unitId;
+	public ModbusProtocol(AbstractOpenemsModbusComponent parent, Task... tasks) {
+		this.parent = parent;
 		for (Task task : tasks) {
 			addTask(task);
 		}
 	}
 
-	public int getUnitId() {
-		return unitId;
-	}
-
 	public synchronized void addTask(Task task) {
-		// add the unitId to the abstractTask
-		task.setUnitId(this.unitId);
+		// add the the parent to the Task
+		task.setParent(this.parent);
 		// check abstractTask for plausibility
 		this.checkTask(task);
 		/*

diff --git a/...enems.edge.bridge.modbus/src/io/openems/edge/bridge/modbus/api/task/AbstractReadTask.java b/...enems.edge.bridge.modbus/src/io/openems/edge/bridge/modbus/api/task/AbstractReadTask.java
@@ -17,15 +17,15 @@
  * 
  * @author stefan.feilmeier
  */
-public abstract class AbstractReadTask<T> extends AbstractTask implements ReadTask{
-	
+public abstract class AbstractReadTask<T> extends AbstractTask implements ReadTask {
+
 	private final Priority priority;
 
 	public AbstractReadTask(int startAddress, Priority priority, AbstractModbusElement<?>... elements) {
 		super(startAddress, elements);
-		this.priority = priority;		
+		this.priority = priority;
 	}
-	
+
 	public void executeQuery(AbstractModbusBridge bridge) throws OpenemsException {
 		T[] response;
 		try {
@@ -44,21 +44,22 @@ public void executeQuery(AbstractModbusBridge bridge) throws OpenemsException {
 				throw new OpenemsException("Transaction failed: " + e.getMessage(), e2);
 			}
 		}
-	
+
 		// Verify response length
 		if (response.length < getLength()) {
-			throw new OpenemsException("Received message is too short. Expected [" + getLength() + "], got [" + response.length + "]");
+			throw new OpenemsException(
+					"Received message is too short. Expected [" + getLength() + "], got [" + response.length + "]");
 		}
-	
+
 		fillElements(response);
 	}
 
 	protected T[] readElements(AbstractModbusBridge bridge) throws OpenemsException, ModbusException {
 		ModbusRequest request = getRequest();
-		ModbusResponse response = Utils.getResponse(request, getUnitId(), bridge);		
+		ModbusResponse response = Utils.getResponse(request, this.getParent().getUnitId(), bridge);
 		return handleResponse(response);
 	}
-	
+
 	protected void fillElements(T[] response) {
 		int position = 0;
 		for (ModbusElement<?> modbusElement : this.getElements()) {
@@ -68,32 +69,39 @@ protected void fillElements(T[] response) {
 				try {
 					if (!modbusElement.isIgnored()) {
 						doElementSetInput(modbusElement, position, response);
-					} 
+					}
 				} catch (OpenemsException e) {
 					doWarnLog(e);
 				}
 			}
-			position = increasePosition(position, modbusElement);			
+			position = increasePosition(position, modbusElement);
 		}
 	}
 
 	public Priority getPriority() {
 		return priority;
 	}
-	
+
 	protected abstract int increasePosition(int position, ModbusElement<?> modbusElement);
-	protected abstract void doElementSetInput(ModbusElement<?> modbusElement, int position, T[] response) throws OpenemsException;
+
+	protected abstract void doElementSetInput(ModbusElement<?> modbusElement, int position, T[] response)
+			throws OpenemsException;
+
 	protected abstract String getRequiredElementName();
+
 	protected abstract boolean isCorrectElementInstance(ModbusElement<?> modbusElement);
+
 	protected abstract ModbusRequest getRequest();
+
 	protected abstract T[] handleResponse(ModbusResponse response) throws OpenemsException;
 
 	private void doWarnLog(OpenemsException e) {
-		log.warn("Unable to fill modbus element. UnitId [" + this.getUnitId() + "] Address [" + getStartAddress()
-				+ "] Length [" + getLength() + "]: " + e.getMessage());
+		log.warn("Unable to fill modbus element. UnitId [" + this.getParent().getUnitId() + "] Address ["
+				+ getStartAddress() + "] Length [" + getLength() + "]: " + e.getMessage());
 	}
 
 	private void doErrorLog(ModbusElement<?> modbusElement) {
-		log.error("A " + getRequiredElementName()  + " is required for a " + getActiondescription() + "Task! Element [" + modbusElement + "]");
+		log.error("A " + getRequiredElementName() + " is required for a " + getActiondescription() + "Task! Element ["
+				+ modbusElement + "]");
 	}
 }
diff --git a/io.openems.edge.bridge.modbus/src/io/openems/edge/bridge/modbus/api/task/AbstractTask.java b/io.openems.edge.bridge.modbus/src/io/openems/edge/bridge/modbus/api/task/AbstractTask.java
@@ -3,6 +3,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import io.openems.edge.bridge.modbus.api.AbstractOpenemsModbusComponent;
 import io.openems.edge.bridge.modbus.api.element.AbstractModbusElement;
 import io.openems.edge.bridge.modbus.api.element.ModbusElement;
 
@@ -20,7 +21,7 @@ public abstract class AbstractTask implements Task {
 	protected final Logger log;
 
 	private ModbusElement<?>[] elements;
-	private int unitId; // this is always set by ModbusProtocol.addTask()
+	private AbstractOpenemsModbusComponent parent = null; // this is always set by ModbusProtocol.addTask()
 
 	public AbstractTask(int startAddress, AbstractModbusElement<?>... elements) {
 		log = LoggerFactory.getLogger(getClass());
@@ -48,19 +49,23 @@ public int getStartAddress() {
 		return startAddress;
 	}
 
-	public void setUnitId(int unitId) {
-		this.unitId = unitId;
+	public void setParent(AbstractOpenemsModbusComponent parent) {
+		this.parent = parent;
 	}
-
-	public int getUnitId() {
-		return unitId;
+	
+	public AbstractOpenemsModbusComponent getParent() {
+		return parent;
 	}
 
 	@Override
 	public String toString() {
 		StringBuffer sb = new StringBuffer();
 		sb.append(getActiondescription());
 		sb.append(" [");
+		sb.append(this.parent.id());
+		sb.append(";unitid=");
+		sb.append(this.parent.getUnitId());
+		sb.append(";ref=");
 		sb.append(this.getStartAddress());
 		sb.append("/0x");
 		sb.append(Integer.toHexString(this.getStartAddress()));

diff --git a/...edge.bridge.modbus/src/io/openems/edge/bridge/modbus/api/task/FC16WriteRegistersTask.java b/...edge.bridge.modbus/src/io/openems/edge/bridge/modbus/api/task/FC16WriteRegistersTask.java
@@ -17,8 +17,8 @@
 import io.openems.edge.bridge.modbus.api.element.ModbusRegisterElement;
 
 /**
- * Implements a Write Holding Registers abstractTask, using Modbus function code 16
- * (http://www.simplymodbus.ca/FC16.htm)
+ * Implements a Write Holding Registers abstractTask, using Modbus function code
+ * 16 (http://www.simplymodbus.ca/FC16.htm)
  */
 public class FC16WriteRegistersTask extends AbstractTask implements WriteTask {
 
@@ -72,14 +72,16 @@ public void executeWrite(AbstractModbusBridge bridge) throws OpenemsException {
 				/*
 				 * First try
 				 */
-				this.writeMultipleRegisters(bridge, this.getUnitId(), write.startAddress, write.getRegisters());
+				this.writeMultipleRegisters(bridge, this.getParent().getUnitId(), write.startAddress,
+						write.getRegisters());
 			} catch (OpenemsException | ModbusException e) {
 				/*
 				 * Second try: with new connection
 				 */
 				bridge.closeModbusConnection();
 				try {
-					this.writeMultipleRegisters(bridge, this.getUnitId(), write.startAddress, write.getRegisters());
+					this.writeMultipleRegisters(bridge, this.getParent().getUnitId(), write.startAddress,
+							write.getRegisters());
 				} catch (ModbusException e2) {
 					throw new OpenemsException("Transaction failed: " + e.getMessage(), e2);
 				}
@@ -89,10 +91,10 @@ public void executeWrite(AbstractModbusBridge bridge) throws OpenemsException {
 
 	private void writeMultipleRegisters(AbstractModbusBridge bridge, int unitId, int startAddress, Register[] registers)
 			throws ModbusException, OpenemsException {
-		
-		WriteMultipleRegistersRequest request = new WriteMultipleRegistersRequest(startAddress, registers);			
-		 ModbusResponse response = Utils.getResponse(request, unitId, bridge);
-		
+
+		WriteMultipleRegistersRequest request = new WriteMultipleRegistersRequest(startAddress, registers);
+		ModbusResponse response = Utils.getResponse(request, unitId, bridge);
+
 		if (!(response instanceof WriteMultipleRegistersResponse)) {
 			throw new OpenemsException("Unexpected Modbus response. Expected [WriteMultipleRegistersResponse], got ["
 					+ response.getClass().getSimpleName() + "]");

diff --git a/...enems.edge.bridge.modbus/src/io/openems/edge/bridge/modbus/api/task/FC5WriteCoilTask.java b/...enems.edge.bridge.modbus/src/io/openems/edge/bridge/modbus/api/task/FC5WriteCoilTask.java
@@ -34,14 +34,14 @@ public void executeWrite(AbstractModbusBridge bridge) throws OpenemsException {
 					/*
 					 * First try
 					 */
-					this.writeCoil(bridge, this.getUnitId(), this.getStartAddress(), valueOpt.get());
+					this.writeCoil(bridge, this.getParent().getUnitId(), this.getStartAddress(), valueOpt.get());
 				} catch (OpenemsException | ModbusException e) {
 					/*
 					 * Second try: with new connection
 					 */
 					bridge.closeModbusConnection();
 					try {
-						this.writeCoil(bridge, this.getUnitId(), this.getStartAddress(), valueOpt.get());
+						this.writeCoil(bridge, this.getParent().getUnitId(), this.getStartAddress(), valueOpt.get());
 					} catch (ModbusException e2) {
 						throw new OpenemsException("Transaction failed: " + e.getMessage(), e2);
 					}
@@ -54,10 +54,10 @@ public void executeWrite(AbstractModbusBridge bridge) throws OpenemsException {
 
 	private void writeCoil(AbstractModbusBridge bridge, int unitId, int startAddress, boolean value)
 			throws OpenemsException, ModbusException {
-		
-		WriteCoilRequest request = new WriteCoilRequest(startAddress, value);	
+
+		WriteCoilRequest request = new WriteCoilRequest(startAddress, value);
 		ModbusResponse response = Utils.getResponse(request, unitId, bridge);
-		
+
 		if (!(response instanceof WriteCoilResponse)) {
 			throw new OpenemsException("Unexpected Modbus response. Expected [WriteCoilResponse], got ["
 					+ response.getClass().getSimpleName() + "]");

diff --git a/...s.edge.bridge.modbus/src/io/openems/edge/bridge/modbus/api/task/FC6WriteRegisterTask.java b/...s.edge.bridge.modbus/src/io/openems/edge/bridge/modbus/api/task/FC6WriteRegisterTask.java
@@ -17,7 +17,7 @@
 public class FC6WriteRegisterTask extends AbstractTask implements WriteTask {
 
 	public FC6WriteRegisterTask(int startAddress, AbstractModbusElement<?> element) {
-		super(startAddress,  element );
+		super(startAddress, element);
 	}
 
 	@Override
@@ -29,28 +29,30 @@ public void executeWrite(AbstractModbusBridge bridge) throws OpenemsException {
 			Optional<Register[]> valueOpt = ((AbstractWordElement<?>) element).getNextWriteValue();
 			if (valueOpt.isPresent()) {
 				Register[] registers = valueOpt.get();
-						
+
 				if (registers.length == 1 && registers[0] != null) {
 					// found value -> write
 					try {
 						/*
 						 * First try
 						 */
 
-						this.writeSingleRegister(bridge, this.getUnitId(), this.getStartAddress(), registers[0]);
+						this.writeSingleRegister(bridge, this.getParent().getUnitId(), this.getStartAddress(),
+								registers[0]);
 					} catch (OpenemsException | ModbusException e) {
 						/*
 						 * Second try: with new connection
 						 */
 						bridge.closeModbusConnection();
 						try {
-							this.writeSingleRegister(bridge, this.getUnitId(), this.getStartAddress(), registers[0]);
+							this.writeSingleRegister(bridge, this.getParent().getUnitId(), this.getStartAddress(),
+									registers[0]);
 						} catch (ModbusException e2) {
 							throw new OpenemsException("Transaction failed: " + e.getMessage(), e2);
 						}
 					}
 				} else {
-					log.warn("Expecting exactly one register. Got [" + registers.length + "]");		
+					log.warn("Expecting exactly one register. Got [" + registers.length + "]");
 				}
 			}
 		} else {