Refactor Channel-Source and Target information (OpenEMS#1595)

* Refactor ChannelSource - Use actual Channel object for storage, to avoid problems with static enums - Separate Read-Source and Write-Target * Soltaro Cluster C: fix duplicated source * Add Battery-Protection as source for Battery ChargeMaxCurrent/DischargeMaxCurrent * Improve error message for duplicated source/target * Weidmüller Meter: fix register mapping
clehne · Sep 12, 2021 · 58ef2c8 · 58ef2c8
1 parent d247ae5
commit 58ef2c8
Show file tree

Hide file tree

Showing 48 changed files with 397 additions and 327 deletions.
diff --git a/io.openems.edge.battery.api/src/io/openems/edge/battery/protection/BatteryProtection.java b/io.openems.edge.battery.api/src/io/openems/edge/battery/protection/BatteryProtection.java
@@ -300,6 +300,9 @@ protected BatteryProtection(Battery battery, ChargeMaxCurrentHandler chargeMaxCu
 		this.battery = battery;
 		this.chargeMaxCurrentHandler = chargeMaxCurrentHandler;
 		this.dischargeMaxCurrentHandler = dischargeMaxCurrentHandler;
+
+		this.battery.getChargeMaxCurrentChannel().setReadSource("Battery-Protection");
+		this.battery.getDischargeMaxCurrentChannel().setReadSource("Battery-Protection");
 	}
 
 	/**

diff --git a/...ery.soltaro/src/io/openems/edge/battery/soltaro/cluster/versionc/ClusterVersionCImpl.java b/...ery.soltaro/src/io/openems/edge/battery/soltaro/cluster/versionc/ClusterVersionCImpl.java
@@ -1,8 +1,8 @@
 package io.openems.edge.battery.soltaro.cluster.versionc;
 
-import java.util.HashSet;
 import java.util.Optional;
 import java.util.Set;
+import java.util.TreeSet;
 import java.util.concurrent.atomic.AtomicReference;
 
 import org.osgi.service.cm.ConfigurationAdmin;
@@ -31,8 +31,8 @@
 import io.openems.edge.battery.soltaro.cluster.versionc.statemachine.Context;
 import io.openems.edge.battery.soltaro.cluster.versionc.statemachine.StateMachine;
 import io.openems.edge.battery.soltaro.cluster.versionc.statemachine.StateMachine.State;
-import io.openems.edge.battery.soltaro.common.batteryprotection.BatteryProtectionDefinitionSoltaro3500Wh;
 import io.openems.edge.battery.soltaro.common.batteryprotection.BatteryProtectionDefinitionSoltaro3000Wh;
+import io.openems.edge.battery.soltaro.common.batteryprotection.BatteryProtectionDefinitionSoltaro3500Wh;
 import io.openems.edge.battery.soltaro.common.enums.ModuleType;
 import io.openems.edge.battery.soltaro.single.versionc.enums.PreChargeControl;
 import io.openems.edge.battery.soltaro.versionc.SoltaroBatteryVersionC;
@@ -89,7 +89,7 @@ public class ClusterVersionCImpl extends AbstractOpenemsModbusComponent implemen
 	private final StateMachine stateMachine = new StateMachine(State.UNDEFINED);
 
 	private Config config;
-	private Set<Rack> racks = new HashSet<>();
+	private TreeSet<Rack> racks = new TreeSet<>();
 	private BatteryProtection batteryProtection = null;
 
 	public ClusterVersionCImpl() {
@@ -381,18 +381,7 @@ protected ModbusProtocol defineModbusProtocol() throws OpenemsException {
 		// getModbusProtocol, and it is using racks...
 		for (Rack r : this.racks) {
 			protocol.addTasks(//
-					// Single Cluster Control Registers (running without Master BMS)
-					new FC6WriteRegisterTask(r.offset + 0x0010, //
-							m(this.rack(r, RackChannel.PRE_CHARGE_CONTROL), new UnsignedWordElement(r.offset + 0x0010)) //
-					), //
-					new FC16WriteRegistersTask(r.offset + 0x000B, //
-							m(this.rack(r, RackChannel.EMS_ADDRESS), new UnsignedWordElement(r.offset + 0x000B)), //
-							m(this.rack(r, RackChannel.EMS_BAUDRATE), new UnsignedWordElement(r.offset + 0x000C)) //
-					), //
-					new FC6WriteRegisterTask(r.offset + 0x00F4, //
-							m(this.rack(r, RackChannel.EMS_COMMUNICATION_TIMEOUT),
-									new UnsignedWordElement(r.offset + 0x00F4)) //
-					), //
+
 					new FC3ReadRegistersTask(r.offset + 0x000B, Priority.LOW, //
 							m(this.rack(r, RackChannel.EMS_ADDRESS), new UnsignedWordElement(r.offset + 0x000B)), //
 							m(this.rack(r, RackChannel.EMS_BAUDRATE), new UnsignedWordElement(r.offset + 0x000C)), //
@@ -407,6 +396,19 @@ protected ModbusProtocol defineModbusProtocol() throws OpenemsException {
 									new UnsignedWordElement(r.offset + 0x00F4)) //
 					),
 
+					// Single Cluster Control Registers (running without Master BMS)
+					new FC6WriteRegisterTask(r.offset + 0x0010, //
+							m(this.rack(r, RackChannel.PRE_CHARGE_CONTROL), new UnsignedWordElement(r.offset + 0x0010)) //
+					), //
+					new FC6WriteRegisterTask(r.offset + 0x00F4, //
+							m(this.rack(r, RackChannel.EMS_COMMUNICATION_TIMEOUT),
+									new UnsignedWordElement(r.offset + 0x00F4)) //
+					), //
+					new FC16WriteRegistersTask(r.offset + 0x000B, //
+							m(this.rack(r, RackChannel.EMS_ADDRESS), new UnsignedWordElement(r.offset + 0x000B)), //
+							m(this.rack(r, RackChannel.EMS_BAUDRATE), new UnsignedWordElement(r.offset + 0x000C)) //
+					), //
+
 					// Single Cluster Control Registers (General)
 					new FC6WriteRegisterTask(r.offset + 0x00CC, //
 							m(this.rack(r, RackChannel.SYSTEM_TOTAL_CAPACITY),
@@ -732,11 +734,8 @@ protected ModbusProtocol defineModbusProtocol() throws OpenemsException {
 								new UnsignedWordElement(r.offset + 0x400)), //
 						m(this.rack(r, RackChannel.LEVEL2_CELL_OVER_VOLTAGE_RECOVER),
 								new UnsignedWordElement(r.offset + 0x401)), //
-						m(new UnsignedWordElement(r.offset + 0x402)) //
-								.m(this.rack(r, RackChannel.LEVEL2_SYSTEM_OVER_VOLTAGE_PROTECTION),
-										ElementToChannelConverter.SCALE_FACTOR_2) // [mV]
-								.m(Battery.ChannelId.CHARGE_MAX_VOLTAGE, ElementToChannelConverter.SCALE_FACTOR_MINUS_1) // [V]
-								.build(), //
+						m(this.rack(r, RackChannel.LEVEL2_SYSTEM_OVER_VOLTAGE_PROTECTION),
+								new UnsignedWordElement(r.offset + 0x402)), //
 						m(this.rack(r, RackChannel.LEVEL2_SYSTEM_OVER_VOLTAGE_RECOVER),
 								new UnsignedWordElement(r.offset + 0x403), ElementToChannelConverter.SCALE_FACTOR_2), //
 						m(this.rack(r, RackChannel.LEVEL2_SYSTEM_CHARGE_OVER_CURRENT_PROTECTION),

diff --git a/...ry.soltaro/test/io/openems/edge/battery/soltaro/cluster/versionb/ClusterVersionBTest.java b/...ry.soltaro/test/io/openems/edge/battery/soltaro/cluster/versionb/ClusterVersionBTest.java
@@ -1,9 +1,7 @@
 package io.openems.edge.battery.soltaro.cluster.versionb;
 
-import org.junit.Before;
 import org.junit.Test;
 
-import io.openems.edge.battery.soltaro.cluster.versionc.ResetChannelSources;
 import io.openems.edge.battery.soltaro.common.enums.BatteryState;
 import io.openems.edge.battery.soltaro.common.enums.ModuleType;
 import io.openems.edge.bridge.modbus.test.DummyModbusBridge;
@@ -15,11 +13,6 @@ public class ClusterVersionBTest {
 	private static final String BATTERY_ID = "battery0";
 	private static final String MODBUS_ID = "modbus0";
 
-	@Before
-	public void before() {
-		ResetChannelSources.run();
-	}
-
 	@Test
 	public void test() throws Exception {
 		new ComponentTest(new ClusterVersionB()) //

diff --git a/...oltaro/test/io/openems/edge/battery/soltaro/cluster/versionc/ClusterVersionCImplTest.java b/...oltaro/test/io/openems/edge/battery/soltaro/cluster/versionc/ClusterVersionCImplTest.java
@@ -1,6 +1,5 @@
 package io.openems.edge.battery.soltaro.cluster.versionc;
 
-import org.junit.Before;
 import org.junit.Test;
 
 import io.openems.edge.battery.soltaro.common.enums.ModuleType;
@@ -14,11 +13,6 @@ public class ClusterVersionCImplTest {
 	private static final String BATTERY_ID = "battery0";
 	private static final String MODBUS_ID = "modbus0";
 
-	@Before
-	public void before() {
-		ResetChannelSources.run();
-	}
-
 	@Test
 	public void test() throws Exception {
 		new ComponentTest(new ClusterVersionCImpl()) //
@@ -30,9 +24,9 @@ public void test() throws Exception {
 						.setModuleType(ModuleType.MODULE_3_5_KWH) //
 						.setStartStop(StartStopConfig.AUTO) //
 						.setNumberOfSlaves(0) //
-						.setRack1Used(false) //
-						.setRack2Used(false) //
-						.setRack3Used(false) //
+						.setRack1Used(true) //
+						.setRack2Used(true) //
+						.setRack3Used(true) //
 						.setRack4Used(false) //
 						.setRack5Used(false) //
 						.build()) //

diff --git a/...ry.soltaro/test/io/openems/edge/battery/soltaro/cluster/versionc/ResetChannelSources.java b/...ry.soltaro/test/io/openems/edge/battery/soltaro/cluster/versionc/ResetChannelSources.java
diff --git a/....battery.soltaro/test/io/openems/edge/battery/soltaro/single/versiona/SingleRackTest.java b/....battery.soltaro/test/io/openems/edge/battery/soltaro/single/versiona/SingleRackTest.java
@@ -1,9 +1,7 @@
 package io.openems.edge.battery.soltaro.single.versiona;
 
-import org.junit.Before;
 import org.junit.Test;
 
-import io.openems.edge.battery.soltaro.cluster.versionc.ResetChannelSources;
 import io.openems.edge.battery.soltaro.common.enums.BatteryState;
 import io.openems.edge.bridge.modbus.test.DummyModbusBridge;
 import io.openems.edge.common.test.ComponentTest;
@@ -14,11 +12,6 @@ public class SingleRackTest {
 	private static final String BATTERY_ID = "battery0";
 	private static final String MODBUS_ID = "modbus0";
 
-	@Before
-	public void before() {
-		ResetChannelSources.run();
-	}
-
 	@Test
 	public void test() throws Exception {
 		new ComponentTest(new SingleRack()) //

diff --git a/...taro/test/io/openems/edge/battery/soltaro/single/versionb/SingleRackVersionBImplTest.java b/...taro/test/io/openems/edge/battery/soltaro/single/versionb/SingleRackVersionBImplTest.java
@@ -1,9 +1,7 @@
 package io.openems.edge.battery.soltaro.single.versionb;
 
-import org.junit.Before;
 import org.junit.Test;
 
-import io.openems.edge.battery.soltaro.cluster.versionc.ResetChannelSources;
 import io.openems.edge.battery.soltaro.common.enums.ModuleType;
 import io.openems.edge.bridge.modbus.test.DummyModbusBridge;
 import io.openems.edge.common.startstop.StartStopConfig;
@@ -16,11 +14,6 @@ public class SingleRackVersionBImplTest {
 	private static final String BATTERY_ID = "battery0";
 	private static final String MODBUS_ID = "modbus0";
 
-	@Before
-	public void before() {
-		ResetChannelSources.run();
-	}
-
 	@Test
 	public void test() throws Exception {
 		new ComponentTest(new SingleRackVersionBImpl()) //

diff --git a/...taro/test/io/openems/edge/battery/soltaro/single/versionc/SingleRackVersionCImplTest.java b/...taro/test/io/openems/edge/battery/soltaro/single/versionc/SingleRackVersionCImplTest.java
@@ -1,9 +1,7 @@
 package io.openems.edge.battery.soltaro.single.versionc;
 
-import org.junit.Before;
 import org.junit.Test;
 
-import io.openems.edge.battery.soltaro.cluster.versionc.ResetChannelSources;
 import io.openems.edge.battery.soltaro.common.enums.ModuleType;
 import io.openems.edge.bridge.modbus.test.DummyModbusBridge;
 import io.openems.edge.common.startstop.StartStopConfig;
@@ -15,11 +13,6 @@ public class SingleRackVersionCImplTest {
 	private static final String BATTERY_ID = "battery0";
 	private static final String MODBUS_ID = "modbus0";
 
-	@Before
-	public void before() {
-		ResetChannelSources.run();
-	}
-
 	@Test
 	public void test() throws Exception {
 		new ComponentTest(new SingleRackVersionCImpl()) //

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
@@ -266,6 +266,11 @@ public ChannelMapper(T element) {
 		public ChannelMapper<T> m(io.openems.edge.common.channel.ChannelId channelId,
 				ElementToChannelConverter converter) {
 			Channel<?> channel = channel(channelId);
+			if (channel instanceof WriteChannel<?>) {
+				((WriteChannel<?>) channel).setWriteTarget(new ModbusChannelMetaInfo(element.getStartAddress()));
+			} else {
+				channel.setReadSource(new ModbusChannelMetaInfo(element.getStartAddress()));
+			}
 			this.channelMaps.put(channel, converter);
 			return this;
 		}
@@ -363,7 +368,7 @@ public T build() {
 	 * Creates a ChannelMapper that can be used with builder pattern inside the
 	 * protocol definition.
 	 * 
-	 * @param <T>       the type of the {@link AbstractModbusElement}d
+	 * @param <T>     the type of the {@link AbstractModbusElement}d
 	 * @param element the ModbusElement
 	 * @return a {@link ChannelMapper}
 	 */
@@ -398,46 +403,17 @@ protected final <T extends AbstractModbusElement<?>> T m(io.openems.edge.common.
 	 * Maps the given element to the Channel identified by channelId, applying the
 	 * given @link{ElementToChannelConverter}.
 	 * 
-	 * @param <T>                    the type of the {@link AbstractModbusElement}d
-	 * @param channelId              the Channel-ID
-	 * @param element                the ModbusElement
-	 * @param converter              the ElementToChannelConverter
-	 * @param ignoreDuplicatedSource set to
-	 *                               {@link ModbusChannelSource#IGNORE_DUPLICATED_SOURCE}
-	 *                               to ignore the check for channels with multiple
-	 *                               mapped modbus registers
-	 * @return the element parameter
-	 */
-	protected final <T extends AbstractModbusElement<?>> T m(io.openems.edge.common.channel.ChannelId channelId,
-			T element, ElementToChannelConverter converter,
-			ModbusChannelSource.IgnoreDuplicatedSource ignoreDuplicatedSource) {
-		if (ignoreDuplicatedSource != null) {
-			try {
-				channelId.doc().source(new ModbusChannelSource(element.getStartAddress()));
-			} catch (IllegalArgumentException e) {
-				throw new IllegalArgumentException(
-						"Unable to add Modbus mapping for [" + channelId.id() + "]: " + e.getMessage());
-			}
-		}
-
-		return new ChannelMapper<T>(element) //
-				.m(channelId, converter) //
-				.build();
-	}
-
-	/**
-	 * Maps the given element to the Channel identified by channelId, applying the
-	 * given @link{ElementToChannelConverter}.
-	 * 
-	 * @param <T>       the type of the {@link AbstractModbusElement}
+	 * @param <T>       the type of the {@link AbstractModbusElement}d
 	 * @param channelId the Channel-ID
 	 * @param element   the ModbusElement
-	 * @param converter the {@link ElementToChannelConverter}
+	 * @param converter the ElementToChannelConverter
 	 * @return the element parameter
 	 */
 	protected final <T extends AbstractModbusElement<?>> T m(io.openems.edge.common.channel.ChannelId channelId,
 			T element, ElementToChannelConverter converter) {
-		return this.m(channelId, element, converter, null);
+		return new ChannelMapper<T>(element) //
+				.m(channelId, converter) //
+				.build();
 	}
 
 	public enum BitConverter {

diff --git a/...ridge/modbus/api/ModbusChannelSource.java → ...dge/modbus/api/ModbusChannelMetaInfo.java b/...ridge/modbus/api/ModbusChannelSource.java → ...dge/modbus/api/ModbusChannelMetaInfo.java
@@ -2,16 +2,7 @@
 
 import java.util.Objects;
 
-public class ModbusChannelSource {
-
-	/**
-	 * This can be used in {@link AbstractOpenemsModbusComponent} to ignore the
-	 * check for duplicated source mappings.
-	 */
-	public static final IgnoreDuplicatedSource IGNORE_DUPLICATED_SOURCE = new IgnoreDuplicatedSource();
-
-	protected static class IgnoreDuplicatedSource {
-	}
+public class ModbusChannelMetaInfo {
 
 	/**
 	 * Holds the Start-Address of the Modbus Register.
@@ -23,12 +14,12 @@ protected static class IgnoreDuplicatedSource {
 	 */
 	private final int bit;
 
-	public ModbusChannelSource(int address) {
+	public ModbusChannelMetaInfo(int address) {
 		this.address = address;
 		this.bit = -1;
 	}
 
-	public ModbusChannelSource(int address, int bit) {
+	public ModbusChannelMetaInfo(int address, int bit) {
 		this.address = address;
 		this.bit = bit;
 	}
@@ -59,7 +50,7 @@ public boolean equals(Object obj) {
 		if (getClass() != obj.getClass()) {
 			return false;
 		}
-		ModbusChannelSource other = (ModbusChannelSource) obj;
+		ModbusChannelMetaInfo other = (ModbusChannelMetaInfo) obj;
 		return this.address == other.address && this.bit == other.bit;
 	}
 

diff --git a/...ems.edge.bridge.modbus/src/io/openems/edge/bridge/modbus/api/element/BitsWordElement.java b/...ems.edge.bridge.modbus/src/io/openems/edge/bridge/modbus/api/element/BitsWordElement.java
@@ -19,7 +19,7 @@
 import io.openems.common.types.OpenemsType;
 import io.openems.edge.bridge.modbus.api.AbstractOpenemsModbusComponent;
 import io.openems.edge.bridge.modbus.api.AbstractOpenemsModbusComponent.BitConverter;
-import io.openems.edge.bridge.modbus.api.ModbusChannelSource;
+import io.openems.edge.bridge.modbus.api.ModbusChannelMetaInfo;
 import io.openems.edge.common.channel.Channel;
 import io.openems.edge.common.channel.ChannelId;
 import io.openems.edge.common.channel.WriteChannel;
@@ -69,17 +69,19 @@ public BitsWordElement bit(int bitIndex, ChannelId channelId, BitConverter conve
 		@SuppressWarnings("unchecked")
 		Channel<Boolean> booleanChannel = (Channel<Boolean>) channel;
 
-		// Add Modbus Address and Bit-Index to Channel Source
-		channelId.doc().source(new ModbusChannelSource(this.getStartAddress(), bitIndex));
-
-		// Handle Writes to Bit-Channels
 		ChannelWrapper channelWrapper = new ChannelWrapper(booleanChannel, converter);
+
+		// Add Modbus Address and Bit-Index to Channel Source
 		if (channel instanceof WriteChannel<?>) {
+			// Handle Writes to Bit-Channels
 			WriteChannel<Boolean> booleanWriteChannel = (WriteChannel<Boolean>) booleanChannel;
 			booleanWriteChannel.onSetNextWrite(value -> {
 				// Listen on Writes to the BooleanChannel and store the value
 				channelWrapper.setWriteValue(value);
 			});
+			booleanWriteChannel.setWriteTarget(new ModbusChannelMetaInfo(this.getStartAddress(), bitIndex));
+		} else {
+			channel.setReadSource(new ModbusChannelMetaInfo(this.getStartAddress(), bitIndex));
 		}
 
 		this.channels[bitIndex] = channelWrapper;