Reading value from multiple bits in BitsWordElement

Hello,

I am writing a device in OpenEMS for a battery system that we are using in our project.

We are reading the status of the battery via Modbus TCP and I am currently implementing the Modbus protocol.

Some of the values are encoded by bits inside a Modbus register. For values where a single bit corresponds to a channel, this is no problem and is easily handled by the BitsWordElement.bit() method which maps directly to the channel.

Some of the status information is encoded across multiple bits, but should really map to a single channel within OpenEMS.
In one specific case, the status is encoded across the bits 0, 1 and 2 (representing numbers 00-07), with the following meaning:

  • 00: Sleep
  • 01: Charge
  • 02: Discharge
  • 03: Idle
    etc

The bit >4 are used to encode other status, e.g single bits for status and alarms etc so can be ignored for this question.

I am unsure what is the best way to read these values in OpenEMS. I suppose one approach would be to use bitshifting and bit masking to extract a number from the first 3 bits. Is this the right approach? How would this be implemented within OpenEMS?

Any help would be greatly appreciated :slight_smile:

Thomas

Hi Thomas,

First of all, I would create an Enum Status implementing OptionsEnum for decoding the status from the integer like that:

public enum Status implements OptionsEnum {
    UNDEFINED(-1, "Undefined"), //
    SLEEP(0, "Sleep"), //
...

Then you need a corresponding channel in your component:

STATUS(Doc.of(Status.values()))

Now if you don’t need the bits > 4, one could define an ElementToChannelConverter like

value -> value & 0b11100000

(or maybe something else)
If you do need the higher bits, maybe it’s best to define a dummy channel where you put the whole byte (or word) and use an onUpdate-callback on that channel, which “cuts the channel value into pieces” and maps everything correspondingly.

Now as I wrote this, I think this may be exactly your approach written down in a more detailed way… I don’t know of any more elegant ways, maybe someone else does?

Best regards,
Thomas

Hi,

there are a few examples in the code with similar requirements. One example is:

Looking at the code in detail, there are a few flaws that I would do differently if I had to write that code today:

  • Call a method from onUpdateCallback
m(new UnsignedWordElement(2729)).build().onUpdateCallback(value -> this.handle2729(value)))
  • In the method make sure to always set every Channel, even if the value is null. This makes sure that, e.g. if somebody pulls the cable, the values are reset to null or false, whatever is more suitable in the situation. (I would never set states to ‘null’, but only to false)
protected void handle2729(Integer value) {
	this.channel(BatteryFeneconCommercial.ChannelId.FAULT_STATUS)
			.setNextValue(value == null ? false : (value & 0x1) == 1);
	this.channel(BatteryFeneconCommercial.ChannelId.POWER_ON)
			.setNextValue(value == null ? false : (value & 0x100) >> 8);
	this.channel(BatteryFeneconCommercial.ChannelId.LOW_SELF_CONSUMPTION_STATUS)
			.setNextValue(value == null ? false : (value & 0x200) >> 9);
	this.channel(BatteryFeneconCommercial.ChannelId.FAULT)//
			.setNextValue(value == null ? false : (value & 0x400) >> 10);
	this.channel(BatteryFeneconCommercial.ChannelId.RUNNING)
			.setNextValue(value == null ? false : (value & 0x1000) >> 12);
	this.channel(BatteryFeneconCommercial.ChannelId.
			EXTERNAL_COMMUNICATION_ONLY_UNDER_STANDBY)
			.setNextValue(value == null ? false : (value & 0x2000) >> 13);
}
  • Add a JUnit test for this specific method
public class BatteryFeneconCommercialImplTest {

	@Test
	public void testHandle2729() {
		var sut = new BatteryFeneconCommercialImpl();
		var faultStatus = sut.channel(BatteryFeneconCommercial.ChannelId.FAULT_STATUS);
		...

		sut.handle2729(null);
		assertEquals(false, faultStatus.getNextValue().get());
		...

		sut.handle2729(0xFFFF);
		assertEquals(true, faultStatus.getNextValue().get());
		...
	}

}

@tsicking: the difference between this approach and an ElementToChannelConverter is, that I do not need a separate Channel that maps on the entire Element.

Regards,
Stefan

Ah, this is indeed quite beautiful. I wasn’t aware of that possibility, thanks for pointing that out :vulcan_salute:

Hi @stefan.feilmeier and @tsicking

Thank you both very much for your responses. I will try implementing this later on and will report back.

I think the approach presented is probably the most elegant way of doing it. One other idea I had would be to use two separate FC3ReadRegistersTask calls. They both access the same register, but one of them looks at the first 3 bits only using a similar logic to that presented above, and the other then uses BitsWordElement to access the subsequent bits directly.

I suppose this approach would be less efficient as it would require 2 modbus read operations. Is this a sufficient reason not to do it? Or is there another drawback I have not considered?

That would indeed work as well (didn’t even think of it). But yes, the major drawback is, that you have multiple read operations. I cannot think of any other drawback.

@stefan.feilmeier One question about your answer above.

Why is it important to set the values of the channels to null? I understand that we don’t want stale values in the event that we lose connectivity or another error occurs. My question is really, why do we not need this logic for every channel which involves reading from modbus?
I.e for every other channel I don’t write any logic to handle the case that the response is null - I am using a simple ElementToChannelConverter or reading bits to get the value.

Is this something that is handled automatically in OpenEMS? Or is there some reason why it is not necessary to set the null values?

Thomas

Hi Thomas,

yes, the ModbusBridge takes care of setting values to null whenever the connection gets lost. See

(Unfortunately I just found a bug where this invalidate does not work for bits in a BitsWordElement. I am about to fix this…)

Regards,
Stefan