[Bug]: Modbus: ModbusOptimizer doesn't check tag specific endianness

[IsmoLeszczynski]

What happened?

ModbusOptimizer.processReadResponses(...) always uses the connection default endianness when processing the results, instead of checking for tag specific configurations:

byte[] responseData = response.getResponseDataForTag(modbusTag);
ReadBuffer readBuffer = getReadBuffer(responseData, modbusContext.getByteOrder());
try {
    PlcValue plcValue = DataItem.staticParse(readBuffer, modbusTag.getDataType(),
        modbusTag.getNumberOfElements(),
        modbusContext.getByteOrder() == ModbusByteOrder.BIG_ENDIAN);
    values.put(tagName, new DefaultPlcResponseItem<>(PlcResponseCode.OK, plcValue));
} catch (ParseException e) {
    values.put(tagName, new DefaultPlcResponseItem<>(PlcResponseCode.INTERNAL_ERROR, null));
}

I believe the byte-order should be taken from the modbusTag.getByteOrder() if available, and fallback to the default? I have a fix for this that I can PR, but it's built on top of #2055, which is still under review.

Version

v0.13.0-SNAPSHOT

Programming Languages

• plc4c
• plc4go
• plc4j
• plc4net
• plc4py

Protocols

• AB-Ethernet
• ADS /AMS
• BACnet/IP
• C-Bus
• CANopen
• EtherNet/IP
• Firmata
• IEC-69870
• KNXnet/IP
• Modbus
• OPC-UA
• Profinet
• S7
• S7-light

[chrisdutz]

Good catch ... yeah ... I think I didn't imeplement that when I implemented the optimizer. But sort of wierd use-case ... could you explain why this is an issue? Do you have a system where some tags are big endian and some little endian? (Or one of the odd modbus cases)?

[IsmoLeszczynski]

I don't have a current use-case, but I ran in to it when implementing the Web of Things Modbus binding in our application, which allows defining the endianness and swapping per property. The only use-case I could imagine is a Modbus Gateway that aggregates different Modbus devices via UnitId - with PLC4X you could have one connection to the gateway and the modbusTags would define the specific UnitId + possible differing endiannes per underlying device.

[chrisdutz]

Aaah ... ok .. yeah ... that makes sense ... will look into this especially your PR. Thanks for this.

I guess this whole "sharing of connections" is really something that we would need the SPI 3.0 for, but that's definitely not going to happen any time soon.

[IsmoLeszczynski]

The fix was so minor that I just added it to the earlier PR already.