Skip to content
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import org.apache.plc4x.java.spi.messages.utils.PlcTagItem;
import org.apache.plc4x.java.spi.optimizer.SingleTagOptimizer;
import org.apache.plc4x.java.spi.values.PlcBOOL;
import org.apache.plc4x.java.spi.values.PlcList;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -208,11 +209,25 @@ protected PlcReadResponse processReadResponses(PlcReadRequest readRequest, Map<P

// Calculate the byte that contains the response for this Coil
byte[] responseData = response.getResponseData();
int bitPosition = modbusTag.getAddress() - response.startingAddress;
int bytePosition = bitPosition / 8;
int bitPositionInByte = bitPosition % 8;
boolean isBitSet = (responseData[bytePosition] & (1 << bitPositionInByte)) != 0;
values.put(tagName, new DefaultPlcResponseItem<>(PlcResponseCode.OK, new PlcBOOL(isBitSet)));

// Check if we're dealing with an array of Coils or Discrete Inputs
if (modbusTag.getNumberOfElements() > 1) {
PlcList bitValues = new PlcList();
for (int i = 0; i < modbusTag.getNumberOfElements(); i++) {
int bitPosition = modbusTag.getAddress() - response.startingAddress + i;
int bytePosition = bitPosition / 8;
int bitPositionInByte = bitPosition % 8;
boolean isBitSet = (responseData[bytePosition] & (1 << bitPositionInByte)) != 0;
bitValues.add(new PlcBOOL(isBitSet));
}
values.put(tagName, new DefaultPlcResponseItem<>(PlcResponseCode.OK, bitValues));
} else {
int bitPosition = modbusTag.getAddress() - response.startingAddress;
int bytePosition = bitPosition / 8;
int bitPositionInByte = bitPosition % 8;
boolean isBitSet = (responseData[bytePosition] & (1 << bitPositionInByte)) != 0;
values.put(tagName, new DefaultPlcResponseItem<>(PlcResponseCode.OK, new PlcBOOL(isBitSet)));
}
break;
}
}
Expand Down Expand Up @@ -436,6 +451,32 @@ private ReadBuffer getReadBuffer(byte[] data, ModbusByteOrder byteOrder) {
byte[] reordered = ModbusProtocolLogic.byteSwap(data);
return new ReadBufferByteBased(reordered, ByteOrder.LITTLE_ENDIAN);
}
case BIG_ENDIAN_WORD_SWAP: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sort of not entirely happy with the names. I asked chatgpt for help 😉 here's the response:

You're dealing with eight different byte/word permutations for Modbus data interpretation, which is common when mapping PLC registers to types like int32, float, or double. The current enum names are a bit ambiguous and repetitive.

🧠 Goal

Come up with more descriptive, unambiguous names that clearly convey:

Byte order within a word (e.g., big vs. little endian)

Word order (e.g., whether 16-bit words are swapped)

Whether both are swapped (combined swap)

🔢 First, mapping by example

Let's take the base array:
[1, 2, 3, 4, 5, 6, 7, 8]
Split into 16-bit words (2 bytes each):
[1,2], [3,4], [5,6], [7,8]

Then apply the transformations per enum:

EnumResultByte OrderWord OrderByte Swap?Word Swap?BIG_ENDIAN[1,2,3,4,5,6,7,8]BigNormal❌❌LITTLE_ENDIAN[8,7,6,5,4,3,2,1]LittleReversed✅✅BIG_ENDIAN_BYTE_SWAP[2,1,4,3,6,5,8,7]LittleNormal✅❌LITTLE_ENDIAN_BYTE_SWAP[7,8,5,6,3,4,1,2]BigReversed❌✅BIG_ENDIAN_WORD_SWAP[3,4,1,2,7,8,5,6]BigSwapped❌✅LITTLE_ENDIAN_WORD_SWAP[6,5,8,7,2,1,4,3]LittleSwapped✅✅BIG_ENDIAN_WORD_SWAP_BYTE_SWAP[4,3,2,1,8,7,6,5]ReversedSwapped✅✅LITTLE_ENDIAN_WORD_SWAP_BYTE_SWAP[5,6,7,8,1,2,3,4]NormalSwapped❌✅

✅ Proposed Enum Names

Old NameNew ProposalReasoningBIG_ENDIANBYTE_ORDER_BIG__WORD_ORDER_NORMALExplicitLITTLE_ENDIANBYTE_ORDER_LITTLE__WORD_ORDER_REVERSEDReflects full reversalBIG_ENDIAN_BYTE_SWAPBYTE_ORDER_LITTLE__WORD_ORDER_NORMALOnly byte swap = little endianLITTLE_ENDIAN_BYTE_SWAPBYTE_ORDER_BIG__WORD_ORDER_REVERSEDOnly word swap = word reversalBIG_ENDIAN_WORD_SWAPBYTE_ORDER_BIG__WORD_ORDER_SWAPPEDWords out of orderLITTLE_ENDIAN_WORD_SWAPBYTE_ORDER_LITTLE__WORD_ORDER_SWAPPEDByte+Word swapBIG_ENDIAN_WORD_SWAP_BYTE_SWAPBYTE_ORDER_REVERSED__WORD_ORDER_SWAPPEDBoth swapped, full reverseLITTLE_ENDIAN_WORD_SWAP_BYTE_SWAPBYTE_ORDER_NORMAL__WORD_ORDER_SWAPPEDJust word swap

💡 Optional Shorter Variant

If you want brevity but still clarity:

NORMAL, // [1,2,3,4] REVERSED, // [4,3,2,1] BYTE_SWAP, // [2,1,4,3] BYTE_SWAP_REVERSED, // [7,8,5,6,3,4,1,2] WORD_SWAP, // [3,4,1,2] BYTE_WORD_SWAP, // [6,5,8,7,2,1,4,3] FULL_REVERSED, // [4,3,2,1,8,7,6,5] WORD_SWAP_CONTIGUOUS // [5,6,7,8,1,2,3,4]

Would you prefer long descriptive or shorter but still clear names? I can adjust accordingly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Argh.. It fu**ed up the table 🙁

// [3, 4, 1, 2]
// [3, 4, 1, 2, 7, 8, 5, 6]
byte[] reordered = ModbusProtocolLogic.wordSwap(data);
return new ReadBufferByteBased(reordered, ByteOrder.BIG_ENDIAN);
}
case LITTLE_ENDIAN_WORD_SWAP: {
// [2, 1, 4, 3]
// [6, 5, 8, 7, 2, 1, 4, 3]
byte[] reordered = ModbusProtocolLogic.wordSwap(data);
return new ReadBufferByteBased(reordered, ByteOrder.LITTLE_ENDIAN);
}
case BIG_ENDIAN_WORD_SWAP_BYTE_SWAP: {
// [4, 3, 2, 1]
// [4, 3, 2, 1, 8, 7, 6, 5]
byte[] reordered = ModbusProtocolLogic.byteSwap(data);
reordered = ModbusProtocolLogic.wordSwap(reordered);
return new ReadBufferByteBased(reordered, ByteOrder.BIG_ENDIAN);
}
case LITTLE_ENDIAN_WORD_SWAP_BYTE_SWAP: {
// [1, 2, 3, 4]
// [5, 6, 7, 8, 1, 2, 3, 4]
byte[] reordered = ModbusProtocolLogic.byteSwap(data);
reordered = ModbusProtocolLogic.wordSwap(reordered);
return new ReadBufferByteBased(reordered, ByteOrder.LITTLE_ENDIAN);
}
default:
// 16909060
// [1, 2, 3, 4]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -418,4 +418,18 @@ public static byte[] byteSwap(byte[] in) {
return out;
}

public static byte[] wordSwap(byte[] in) {
if (in.length % 2 != 0) {
throw new PlcRuntimeException("Input byte array length must be a multiple of 2 for word swapping.");
}
byte[] out = new byte[in.length];
for (int i = 0; i < in.length; i += 4) {
out[i] = in[i + 2];
out[i + 1] = in[i + 3];
out[i + 2] = in[i];
out[i + 3] = in[i + 1];
}
return out;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,17 @@ public enum ModbusByteOrder {
BIG_ENDIAN_BYTE_SWAP,
// [3, 4, 1, 2]
// [7, 8, 5, 6, 3, 4, 1, 2]
LITTLE_ENDIAN_BYTE_SWAP
LITTLE_ENDIAN_BYTE_SWAP,
// [3, 4, 1, 2]
// [3, 4, 1, 2, 7, 8, 5, 6]
BIG_ENDIAN_WORD_SWAP,
// [2, 1, 4, 3]
// [6, 5, 8, 7, 2, 1, 4, 3]
LITTLE_ENDIAN_WORD_SWAP,
// [4, 3, 2, 1]
// [4, 3, 2, 1, 8, 7, 6, 5]
BIG_ENDIAN_WORD_SWAP_BYTE_SWAP,
// [1, 2, 3, 4]
// [5, 6, 7, 8, 1, 2, 3, 4]
LITTLE_ENDIAN_WORD_SWAP_BYTE_SWAP,
}