SunSpec: improvements (OpenEMS#2337)

- There are SunSpec Channels which have a constant scale factor, e.g. S305_LAT has a scale factor of -7. The current implementation of the AbstractOpenemsSunSpecComponent cannot handle these, but only those whose scale factor is read via modbus. This PR handles this issue.
- Moreover, a DummySunSpecComponent with all default models is added for JUnit testing, as well as a small JUnit test checking that the modbus tasks are not too long.
- DummySunSpecComponent: initialize SunSpec modals in ASC order
- Refactor addReadTasks to preprocessModbusElements to be able to reuse it for WriteTasks
- Create one FC16WriteRegistersTask for all writeable registers of one block This solves some perfomance problems we had with multiple WriteTasks being executed on every Cycle (e.g. write ActivePower and ReactivePower channels)
- Extract generateElementToChannelConverter logic was split to two places before; also now I am always applying the 'isDefined' check - even for Points with Scale-Factor
- ScaleFactor: get next value to win one cycle (NOTE: this is still too slow with one cycle, but that will be tackled in another PR)

---------

Co-authored-by: Stefan Feilmeier <[email protected]>

Author: tsicking

cnf/pom.xml

@@ -11,7 +11,7 @@
 			<!-- On update: also update gradle.properties file -->
 			<groupId>biz.aQute.bnd.workspace</groupId>
 			<artifactId>biz.aQute.bnd.workspace.gradle.plugin</artifactId>
-			<version>6.4.0</version>
+			<version>7.0.0</version>
 		</dependency>
 		<!-- com -->
 		<dependency>

io.openems.edge.bridge.modbus/src/io/openems/edge/bridge/modbus/api/ElementToChannelScaleFactorConverter.java

@@ -18,6 +18,16 @@
  */
 public class ElementToChannelScaleFactorConverter extends ElementToChannelConverter {
 
+	private static int getValueOrError(OpenemsComponent component, ChannelId channelId)
+			throws InvalidValueException, IllegalArgumentException {
+		var channel = (IntegerReadChannel) component.channel(channelId);
+		var value = channel.getNextValue().orElse(null);
+		if (value != null) {
+			return value;
+		}
+		return channel.value().getOrError();
+	}
+
 	public ElementToChannelScaleFactorConverter(OpenemsComponent component, SunSpecPoint point,
 			ChannelId scaleFactorChannel) {
 		super(//
@@ -27,8 +37,7 @@ public ElementToChannelScaleFactorConverter(OpenemsComponent component, SunSpecP
 						return null;
 					}
 					try {
-						return apply(value,
-								((IntegerReadChannel) component.channel(scaleFactorChannel)).value().getOrError() * -1);
+						return apply(value, getValueOrError(component, scaleFactorChannel) * -1);
 					} catch (InvalidValueException | IllegalArgumentException e) {
 						return null;
 					}
@@ -37,8 +46,7 @@ public ElementToChannelScaleFactorConverter(OpenemsComponent component, SunSpecP
 				// channel -> element
 				value -> {
 					try {
-						return apply(value,
-								((IntegerReadChannel) component.channel(scaleFactorChannel)).value().getOrError());
+						return apply(value, getValueOrError(component, scaleFactorChannel));
 					} catch (InvalidValueException | IllegalArgumentException e) {
 						return null;
 					}

io.openems.edge.bridge.modbus/src/io/openems/edge/bridge/modbus/sunspec/AbstractOpenemsSunSpecComponent.java

@@ -1,8 +1,6 @@
 package io.openems.edge.bridge.modbus.sunspec;
 
-import java.util.ArrayDeque;
 import java.util.ArrayList;
-import java.util.Deque;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -17,18 +15,20 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.collect.Lists;
+
 import io.openems.common.exceptions.OpenemsException;
 import io.openems.edge.bridge.modbus.api.AbstractOpenemsModbusComponent;
 import io.openems.edge.bridge.modbus.api.ElementToChannelConverter;
 import io.openems.edge.bridge.modbus.api.ElementToChannelScaleFactorConverter;
 import io.openems.edge.bridge.modbus.api.ModbusProtocol;
 import io.openems.edge.bridge.modbus.api.ModbusUtils;
-import io.openems.edge.bridge.modbus.api.element.AbstractModbusElement;
 import io.openems.edge.bridge.modbus.api.element.DummyRegisterElement;
 import io.openems.edge.bridge.modbus.api.element.ModbusElement;
 import io.openems.edge.bridge.modbus.api.element.ModbusRegisterElement;
 import io.openems.edge.bridge.modbus.api.element.UnsignedDoublewordElement;
 import io.openems.edge.bridge.modbus.api.element.UnsignedWordElement;
+import io.openems.edge.bridge.modbus.api.task.AbstractTask;
 import io.openems.edge.bridge.modbus.api.task.FC16WriteRegistersTask;
 import io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask;
 import io.openems.edge.bridge.modbus.api.task.Task;
@@ -291,102 +291,137 @@ public boolean isSunSpecInitializationCompleted() {
 	protected void addBlock(int startAddress, SunSpecModel model, Priority priority) throws OpenemsException {
 		this.logInfo(this.log, "Adding SunSpec-Model [" + model.getBlockId() + ":" + model.label() + "] starting at ["
 				+ startAddress + "]");
-		Deque<ModbusElement> elements = new ArrayDeque<>();
+		var readElements = new ArrayList<ModbusElement>();
+		var writeElements = new ArrayList<ModbusElement>();
 		startAddress += 2;
 		for (var i = 0; i < model.points().length; i++) {
 			var point = model.points()[i];
-			var element = point.get().generateModbusElement(startAddress);
-			startAddress += element.length;
-			elements.add(element);
+			final var element = point.get().generateModbusElement(startAddress);
 
+			// Handle AccessMode
+			switch (point.get().accessMode) {
+			case READ_ONLY -> {
+				readElements.add(element);
+			}
+			case READ_WRITE -> {
+				readElements.add(element);
+				writeElements.add(element);
+			}
+			case WRITE_ONLY -> {
+				readElements.add(new DummyRegisterElement(element.startAddress, element.length));
+				writeElements.add(element);
+			}
+			}
+
+			startAddress += element.length;
 			var channelId = point.getChannelId();
 			this.addChannel(channelId);
+			this.m(channelId, element, this.generateElementToChannelConverter(model, point));
+		}
 
-			if (point.get().scaleFactor.isPresent()) {
-				// This Point needs a ScaleFactor
-				// - find the ScaleFactor-Point
-				var scaleFactorName = SunSpecCodeGenerator.toUpperUnderscore(point.get().scaleFactor.get());
-				SunSpecPoint scaleFactorPoint = null;
-				for (var sfPoint : model.points()) {
-					if (sfPoint.name().equals(scaleFactorName)) {
-						scaleFactorPoint = sfPoint;
-						break;
-					}
-				}
-				if (scaleFactorPoint == null) {
-					// Unable to find ScaleFactor-Point
-					this.logError(this.log,
-							"Unable to find ScaleFactor [" + scaleFactorName + "] for Point [" + point.name() + "]");
-				}
+		// Create Tasks and add them to the ModbusProtocol
+		for (var elements : preprocessModbusElements(readElements)) {
+			this.modbusProtocol.addTask(//
+					new FC3ReadRegistersTask(//
+							elements.get(0).startAddress, priority, elements.toArray(ModbusElement[]::new)));
+		}
+		for (var elements : preprocessModbusElements(writeElements)) {
+			this.modbusProtocol.addTask(//
+					new FC16WriteRegistersTask(//
+							elements.get(0).startAddress, elements.toArray(ModbusElement[]::new)));
+		}
+	}
 
-				// Add a scale-factor mapping between Element and Channel
-				element = this.m(channelId, element,
-						new ElementToChannelScaleFactorConverter(this, point, scaleFactorPoint.getChannelId()));
+	/**
+	 * Converts a list of {@link ModbusElement}s to sublists, prepared for Modbus
+	 * {@link AbstractTask}s.
+	 * 
+	 * <ul>
+	 * <li>Sublists are without holes (i.e. nextStartAddress = currentStartAddress +
+	 * Length + 1)
+	 * <li>Length of sublist <= MAXIMUM_TASK_LENGTH
+	 * </ul>
+	 * 
+	 * @param elements the source elements
+	 * @return list of {@link ModbusElement} lists
+	 */
+	protected static List<List<ModbusElement>> preprocessModbusElements(List<ModbusElement> elements) {
+		var result = Lists.<List<ModbusElement>>newArrayList(Lists.<ModbusElement>newArrayList());
+		for (var element : elements) {
+			// Get last sublist in result
+			var l = result.get(result.size() - 1);
+			// Get last element of sublist
+			var e = l.isEmpty() ? null : l.get(l.size() - 1);
+			if ((
+			// Is first element of the sublist?
+			e == null
+					// Is element direct successor?
+					|| e.startAddress + e.length == element.startAddress) //
+					&& // Does element fit in task?
+					l.stream().mapToInt(m -> m.length).sum() + element.length <= MAXIMUM_TASK_LENGTH //
+			) {
+				l.add(element); // Add to existing sublist
 
 			} else {
-				// Add a direct mapping between Element and Channel
-				element = this.m(channelId, element, new ElementToChannelConverter(
-						// Element -> Channel
-						value -> {
-							if (!point.isDefined(value)) {
-								// This value is set to be 'UNDEFINED' for the given type by SunSpec
-								return null;
-							}
-							return value;
-						},
-						// Channel -> Element
-						value -> value));
-
-			}
-
-			// Evaluate Access-Mode of the Channel
-			switch (point.get().accessMode) {
-			case READ_ONLY:
-				// Read-Only -> replace element with dummy
-				element = new DummyRegisterElement(element.startAddress,
-						element.startAddress + point.get().type.length - 1);
-				break;
-			case READ_WRITE:
-			case WRITE_ONLY:
-				// Add a Write-Task
-				// TODO create one FC16WriteRegistersTask for entire block
-				final Task writeTask = new FC16WriteRegistersTask(element.startAddress, element);
-				this.modbusProtocol.addTask(writeTask);
-				break;
+				result.add(Lists.<ModbusElement>newArrayList(element)); // Create new sublist
 			}
 		}
 
-		this.addReadTasks(elements, priority);
+		// Avoid length check for sublist
+		if (result.get(0).isEmpty()) {
+			return List.of();
+		}
+		return result;
 	}
 
 	/**
-	 * Splits the task if it is too long and adds the read tasks.
+	 * Generates a {@link ElementToChannelConverter} for a Point.
 	 * 
-	 * @param elements	the Deque of {@link ModbusElement}s for one block.
-	 * @param priority	the reading priority
-	 * @throws OpenemsException on error
+	 * <ul>
+	 * <li>Check for UNDEFINED value as defined in SunSpec per Type specification
+	 * <li>If a Scale-Factor is defined, try to add it - either as other point of
+	 * model (e.g. "W_SF") or as static value converter
+	 * </ul>
+	 * 
+	 * @param model the {@link SunSpecModel}
+	 * @param point the {@link SunSpecPoint}
+	 * @return an {@link ElementToChannelConverter}, never null
 	 */
-	private void addReadTasks(Deque<ModbusElement> elements, Priority priority) throws OpenemsException {
-		var length = 0;
-		var taskElements = new ArrayDeque<ModbusElement>();
-		var element = elements.pollFirst();
-		while (element != null) {
-			if (length + element.length > MAXIMUM_TASK_LENGTH) {
-				this.modbusProtocol.addTask(//
-						new FC3ReadRegistersTask(//
-								taskElements.peekFirst().startAddress, priority, //
-								taskElements.toArray(new AbstractModbusElement[taskElements.size()])));
-				length = 0;
-				taskElements.clear();
-			}
-			taskElements.add(element);
-			length += element.length;
-			element = elements.pollFirst();
+	protected ElementToChannelConverter generateElementToChannelConverter(SunSpecModel model, SunSpecPoint point) {
+		// Create converter for 'defined' state
+		final var valueIsDefinedConverter = new ElementToChannelConverter(//
+				/* Element -> Channel */ value -> point.isDefined(value) ? value : null,
+				/* Channel -> Element */ value -> value);
+
+		// Generate Scale-Factor converter (possibly null)
+		ElementToChannelConverter scaleFactorConverter = null;
+		if (point.get().scaleFactor.isPresent()) {
+			final var scaleFactor = point.get().scaleFactor.get();
+			final var scaleFactorName = SunSpecCodeGenerator.toUpperUnderscore(scaleFactor);
+			scaleFactorConverter = Stream.of(model.points()) //
+					.filter(p -> p.name().equals(scaleFactorName)) //
+					.map(sfp -> new ElementToChannelScaleFactorConverter(this, point, sfp.getChannelId())) //
+					// Found matching Scale-Factor Point in SunSpec Modal
+					.findFirst()
+
+					// Else: try to parse constant Scale-Factor
+					.orElseGet(() -> {
+						try {
+							return new ElementToChannelScaleFactorConverter(Integer.parseInt(scaleFactor));
+						} catch (NumberFormatException e) {
+							// Unable to parse Scale-Factor to static value
+							this.logError(this.log, "Unable to parse Scale-Factor [" + scaleFactor + "] for Point ["
+									+ point.name() + "]");
+							return null;
+						}
+					}); //
+		}
+
+		if (scaleFactorConverter != null) {
+			return ElementToChannelConverter.chain(valueIsDefinedConverter, scaleFactorConverter);
+		} else {
+			return valueIsDefinedConverter;
 		}
-		this.modbusProtocol.addTask(//
-				new FC3ReadRegistersTask(//
-						taskElements.peekFirst().startAddress, priority, //
-						taskElements.toArray(new AbstractModbusElement[taskElements.size()])));
 	}
 
 	/**

io.openems.edge.bridge.modbus/test/io/openems/edge/bridge/modbus/sunspec/AbstractOpenemsSunSpecComponentTest.java

@@ -0,0 +1,32 @@
+package io.openems.edge.bridge.modbus.sunspec;
+
+import static org.junit.Assert.assertEquals;
+
+import java.util.ArrayList;
+
+import org.junit.Test;
+
+import io.openems.common.exceptions.OpenemsException;
+import io.openems.edge.bridge.modbus.api.element.ModbusElement;
+import io.openems.edge.bridge.modbus.api.element.StringWordElement;
+
+public class AbstractOpenemsSunSpecComponentTest {
+
+	@Test
+	public void testPreprocessModbusElements() throws OpenemsException {
+		var elements = new ArrayList<ModbusElement>();
+		var startAddress = 0;
+		for (var point : DefaultSunSpecModel.S_701.points()) {
+			var element = point.get().generateModbusElement(startAddress);
+			startAddress += element.length;
+			elements.add(element);
+		}
+
+		var sut = AbstractOpenemsSunSpecComponent.preprocessModbusElements(elements);
+		assertEquals(2, sut.size()); // two sublists
+		assertEquals(69, sut.get(0).size()); // first task
+		assertEquals(1, sut.get(1).size()); // second task
+		assertEquals(StringWordElement.class, sut.get(1).get(0).getClass()); // second task
+	}
+
+}

io.openems.edge.bridge.modbus/test/io/openems/edge/bridge/modbus/sunspec/DummySunSpecComponent.java

@@ -0,0 +1,57 @@
+package io.openems.edge.bridge.modbus.sunspec;
+
+import java.util.Map;
+import java.util.TreeMap;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import io.openems.common.exceptions.OpenemsException;
+import io.openems.edge.bridge.modbus.api.ModbusComponent;
+import io.openems.edge.bridge.modbus.api.task.Task;
+import io.openems.edge.common.component.OpenemsComponent;
+import io.openems.edge.common.taskmanager.Priority;
+
+public class DummySunSpecComponent extends AbstractOpenemsSunSpecComponent {
+
+	/**
+	 * All models are active with low priority.
+	 */
+	private static final Map<SunSpecModel, Priority> ACTIVE_MODELS = Stream.of(DefaultSunSpecModel.values())
+			.collect(Collectors.toMap(model -> model, model -> Priority.LOW, (a, b) -> a, TreeMap::new));
+
+	public DummySunSpecComponent() throws OpenemsException {
+		super(ACTIVE_MODELS, //
+				OpenemsComponent.ChannelId.values(), //
+				ModbusComponent.ChannelId.values()); //
+		this.addBlocks();
+	}
+
+	private void addBlocks() throws OpenemsException {
+		var startAddress = 40000;
+		for (var entry : ACTIVE_MODELS.keySet()) {
+			this.addBlock(startAddress, entry, ACTIVE_MODELS.get(entry));
+		}
+
+	}
+
+	@Override
+	protected void onSunSpecInitializationCompleted() {
+	}
+
+	/**
+	 * Gets the length of the longest modbus task.
+	 *
+	 * @return the maximum task length
+	 * @throws OpenemsException on error
+	 */
+	public int maximumTaskLenghth() throws OpenemsException {
+		return this.getModbusProtocol() //
+				.getTaskManager() //
+				.getTasks() //
+				.stream() //
+				.mapToInt(Task::getLength) //
+				.max().orElse(0);
+
+	}
+
+}