openems icon indicating copy to clipboard operation
openems copied to clipboard

Modbus TCP bridge skip cycle function

Open cvabc opened this issue 1 year ago • 2 comments

Related PR https://github.com/OpenEMS/openems/pull/2569

OpenEMS's core cycle time is usually 1 second, but some modbus devices can be very slow. It's possible to lower the core cycle time but the whole system is slowed down.

The new configuable paraemter intervalBetweenAccesses can be configured to slow down the modbus bridge's access. Specifically, it defines the minimum time (in milliseconds) between 2 modbus accesses, which normally equals to the core cycle time (usually 1000ms).

intervalBetweenAccesses is aligned to and calculated from the core cycle time. It's set to be 0 by default and any value between 0 and core cycle time will not affect the original modbus bridge logic. The right amount of core cycles to be skipped will be calculated and applied when intervalBetweenAccesses is set to be longer than core cycle time.

Note that the real interval may be longer than configured due to the round off of core cycle time. For example, we set intervalBetweenAccesses to 1.5 seconds but core time is 1 second and in this case the final interval is 2 seconds.

cvabc avatar Apr 11 '24 07:04 cvabc

Code Coverage

github-actions[bot] avatar Apr 11 '24 07:04 github-actions[bot]

I like the feature 👍

How about this alternative approach for implementation?

  • Drop the calculation of noSkipIdx in applyConfig()
  • Instead calculate the passed time manually in handleEvent():
public void handleEvent(Event event) {
    [...]
    if (this.skipCycle(event)) {
        return;
    }
    [...]
}

private Instant lastBeforeProcessImageEvent = Instant.MIN;
private boolean _skipCycle;
private boolean skipCycle(Event event) {
	final var now = Instant.now();
    if (event.getTopic() == EdgeEventConstants.TOPIC_CYCLE_BEFORE_PROCESS_IMAGE) {
        if (Duration.between(this.lastBeforeProcessImageEvent, now).toMillis() > this.minIntervalBetweenAccesses) {
        	this._skipCycle = true;
        }
        this.lastBeforeProcessImageEvent = now;
    }
    return this._skipCycle;
}

Actually the same idea was used in the initial commit https://github.com/OpenEMS/openems/pull/2615/commits/940fa15b9c2942d652db1b9cd239af3e559311f1 but was later replaced by https://github.com/OpenEMS/openems/pull/2615/commits/4035fb225672ddaab2afc9315418ca5dc964dc65, for edge case reasons. The test script was written with the concept of cycle, instead of passed time, as can be found in the use of next() function: https://github.com/OpenEMS/openems/blob/76f519cf550ad2a03f717dee11b70d76ade6d55c/io.openems.edge.bridge.modbus/test/io/openems/edge/bridge/modbus/BridgeModbusTcpImplTest.java#L77 The problem of using passed time shows when repeating the test script for dozens of time, little imprecision of cycle time will accumulate to large error (edge case), which could potentially invalidate the test. Therefore I tried another way, not rely on time passed, but time relative to core cycle and hence counting how many cycles should be skipped. Actually current implementation would fail in tests in some conditions as well. For example, the following change of the parameter would invalidate the test

--- a/io.openems.edge.bridge.modbus/test/io/openems/edge/bridge/modbus/BridgeModbusTcpImplTest.java
+++ b/io.openems.edge.bridge.modbus/test/io/openems/edge/bridge/modbus/BridgeModbusTcpImplTest.java
@@ -177,7 +177,7 @@ public class BridgeModbusTcpImplTest {
                        }
                        // 0 <= interval < maxInterval
                        numTests = 7;
-                       int maxInterval = CYCLE_TIME * 3;
+                       int maxInterval = CYCLE_TIME * 4;
                        for (int i = 0; i < numTests; i++) {
                                var sut = new BridgeModbusTcpImpl();
                                var device = new MyModbusComponent(DEVICE_ID, sut, UNIT_ID);

But I could not explain the error and now it seems to me the error is not related to the implementation of skip cycle function, but the test script itself. Maybe I misunderstand the test script somehow? Also I noticed the original test script would also fail around here: https://github.com/OpenEMS/openems/blob/76f519cf550ad2a03f717dee11b70d76ade6d55c/io.openems.edge.bridge.modbus/test/io/openems/edge/bridge/modbus/BridgeModbusTcpImplTest.java#L96 Is it expected?

cvabc avatar Apr 15 '24 09:04 cvabc