Proposal: getter and setter methods on OpenEMS Natures

English Forum Software-Development
1

I created a proposal for changed getter and setter methods on OpenEMS Natures at Github:

OpenEMS Natures like SymmetricMeter declare the Channel-IDs that are to be provided by implementations of the Nature. The Nature interface also provides Java Getter methods to access the Channels, e.g. (Source)

/**
 * Gets the Active Power in [W]. Negative values for Consumption; positive for
 * Production
 * 
 * @return the Channel
 */
default Channel<Integer> getActivePower() {
	return this.channel(ChannelId.ACTIVE_POWER);
}

and (Source)

/**
 * Gets the Set Active Power Equals in [W].
 * 
 * @return the Channel
 */
default WriteChannel<Integer> getSetActivePowerEquals() {
	return this.channel(ChannelId.SET_ACTIVE_POWER_EQUALS);
}

This approach is useful for users of Components that implement this Natures, as they get access to all the metadata of the Channel, like current value, last values, unit, description, helper methods,… - but it also adds a lot of complexity. From my experince in most cases a user would prefer simple Getter and Setter methods, like:

default int getActivePower() throws InvalidValueException {
	Channel<Integer> activePowerChannel = this.channel(ChannelId.ACTIVE_POWER);
	return activePowerChannel.value().getOrError();
}

default void setActivePowerEquals(int value) throws OpenemsNamedException {
	WriteChannel<Integer> setActivePowerEqualsChannel = this.channel(ChannelId.SET_ACTIVE_POWER_EQUALS);
	setActivePowerEqualsChannel.setNextWriteValue(value);
}

Alternatively for the Getter, it might be feasible to return the Value-Object. This still allows access to a subset of helper methods like .orElse(...) and does not throw an Exception.

default Value<Integer> getActivePower() {
	Channel<Integer> activePowerChannel = this.channel(ChannelId.ACTIVE_POWER);
	return activePowerChannel.value();
}

Optionally we could still provide access to the Channels for cases where the user needs it, like:

default Channel<Integer> getActivePowerChannel() {
	return this.channel(ChannelId.ACTIVE_POWER);
}

This change will required touching a lot of files in the source code and will cause incompatibilities with forked repositories or non-open-source OpenEMS bundles.

What is your opinion?

2

I think it’s a good idea to reduce the complexity for an average API user. Learning OpenEMS takes some time and there are lot of things to learn. One of the many points to learn is how to read/write a channel and to understand the meaning of the various channel metadata. Just to recognize later that you do not need all the “complexity”. Because in most cases you simply want to set/get values.
I personally would prefer getter/setter methods with Value-Objects. A Java programmer will almost immediately understand it. Also the Value-Object helps improving the readability of the Code which works with the Nature interfaces.

As you already wrote, this change has a big impact on many other things. But on the other hand, it will help especially programmers which are starting with OpenEMS. So I would suggest doing it.

3

An advantage that I also see is that it would be easier to use a WriteChannel. The programmer only takes care of the setter and in case of a WriteChannel, only the write value will be set.

Because of that case: Is it necessary two have two separate getter/setter methods for the write value and the normal value of a WriteChannel or will we omit this in the first step?

Overall, I think that’s a good idea.