Skip to content

Commit fb062dc

Browse files
chrisdutzchrisdutz
committed
feat: Corrected how the signed var-length integer is being handled.
1 parent 3cebcc1 commit fb062dc

File tree

7 files changed

+82
-57
lines changed

7 files changed

+82
-57
lines changed

‎code-generation/language-java/src/main/java/org/apache/plc4x/language/java/JavaLanguageTemplateHelper.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1420,4 +1420,13 @@ public boolean isVarduintField(Field field) {
14201420
return false;
14211421
}
14221422

1423+
public boolean isVardintField(Field field) {
1424+
Optional<Term> encoding = field.getEncoding();
1425+
if (encoding.isPresent()) {
1426+
String encodingName = encoding.get().asLiteral().orElseThrow().asStringLiteral().orElseThrow().getValue();
1427+
return encodingName.equalsIgnoreCase("VARDINT");
1428+
}
1429+
return false;
1430+
}
1431+
14231432
}

‎code-generation/language-java/src/main/resources/templates/java/complex-type-template.java.ftlh‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -549,7 +549,9 @@ public<#if type.isDiscriminatedParentTypeDefinition()> abstract</#if> class ${ty
549549
lengthInBits += ${helper.toSerializationExpression(simpleField, helper.intTypeReference, vstringTypeReference.getLengthExpression(), parserArguments)};
550550
<#else>
551551
<#if helper.isVarduintField(simpleField)>
552-
lengthInBits += GET_VARUDINT_LENGTH_IN_BITS(get${simpleField.getName()?cap_first}());
552+
lengthInBits += GET_VARDUINT_LENGTH_IN_BITS(get${simpleField.getName()?cap_first}());
553+
<#elseif helper.isVardintField(simpleField)>
554+
lengthInBits += GET_VARDINT_LENGTH_IN_BITS(get${simpleField.getName()?cap_first}());
553555
<#else>
554556
lengthInBits += ${simpleTypeReference.sizeInBits};
555557
</#if>

‎plc4j/spi/src/main/java/org/apache/plc4x/java/spi/generation/ReadBufferByteBased.java‎

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -454,49 +454,56 @@ public int readInt(String logicalName, int bitLength, WithReaderArgs... readerAr
454454
if (bitLength > 32) {
455455
throw new ParseException("int can only contain max 32 bits");
456456
}
457-
try {
458-
if (byteOrder == ByteOrder.LITTLE_ENDIAN) {
459-
return Integer.reverseBytes(bi.readInt(false, bitLength));
460-
}
461-
return bi.readInt(false, bitLength);
462-
} catch (IOException e) {
463-
throw new ParseException("Error reading signed int", e);
464-
}
465-
}
466-
467-
@Override
468-
public long readLong(String logicalName, int bitLength, WithReaderArgs... readerArgs) throws ParseException {
469-
bitPos+=bitLength;
470-
if (bitLength <= 0) {
471-
throw new ParseException("long must contain at least 1 bit");
472-
}
473-
if (bitLength > 64) {
474-
throw new ParseException("long can only contain max 64 bits");
475-
}
476457
try {
477458
String encoding = extractEncoding(readerArgs).orElse("default");
478459
switch (encoding) {
479460
case "VARDINT": {
480-
long result = 0;
461+
int result = 0;
481462
for (int i = 0; i < 4; i++) {
482463
short b = bi.readShort(true, 8);
483464

484465
// if this is the first byte, and it's negative (the 7th bit is true)
485466
// initialize the result with a value where all bits are 1
486467
if((i == 0) && ((b & SEVENTH_BIT) != 0)) {
487-
result = -1L;
468+
result = -1;
488469
}
489470

490471
// Add the lower 7 bits of b, shifted appropriately.
491472
result = result << 7;
492-
result |= ((long) b & LAST_SEVEN_BITS);
473+
result |= (int) (b & LAST_SEVEN_BITS);
493474
// If the most significant bit is 0, this is the last byte.
494475
if ((b & EIGHTH_BIT) == 0) {
495476
break;
496477
}
497478
}
498479
return result;
499480
}
481+
case "default":
482+
if (byteOrder == ByteOrder.LITTLE_ENDIAN) {
483+
return Integer.reverseBytes(bi.readInt(false, bitLength));
484+
}
485+
return bi.readInt(false, bitLength);
486+
487+
default:
488+
throw new ParseException("unsupported encoding '" + encoding + "'");
489+
}
490+
} catch (IOException e) {
491+
throw new ParseException("Error reading signed int", e);
492+
}
493+
}
494+
495+
@Override
496+
public long readLong(String logicalName, int bitLength, WithReaderArgs... readerArgs) throws ParseException {
497+
bitPos+=bitLength;
498+
if (bitLength <= 0) {
499+
throw new ParseException("long must contain at least 1 bit");
500+
}
501+
if (bitLength > 64) {
502+
throw new ParseException("long can only contain max 64 bits");
503+
}
504+
try {
505+
String encoding = extractEncoding(readerArgs).orElse("default");
506+
switch (encoding) {
500507
case "default":
501508
if (byteOrder == ByteOrder.LITTLE_ENDIAN) {
502509
return Long.reverseBytes(bi.readLong(false, bitLength));

‎plc4j/spi/src/main/java/org/apache/plc4x/java/spi/generation/StaticHelper.java‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ public static int PADCOUNT(Object obj, boolean hasNext) {
126126
return hasNext ? COUNT(obj) : 0;
127127
}
128128

129-
public static int GET_VARUDINT_LENGTH_IN_BITS(long value) {
129+
public static int GET_VARDUINT_LENGTH_IN_BITS(long value) {
130130
int curFieldLengthInBits = 0;
131131
long temp = value;
132132
do {
@@ -136,4 +136,15 @@ public static int GET_VARUDINT_LENGTH_IN_BITS(long value) {
136136
return curFieldLengthInBits;
137137
}
138138

139+
public static int GET_VARDINT_LENGTH_IN_BITS(long value) {
140+
int curFieldLengthInBits = 8;
141+
boolean positive = value >= 0;
142+
long tmpValue = value;
143+
while (tmpValue >> 6 != (positive ? 0 : -1)) {
144+
curFieldLengthInBits += 8;
145+
tmpValue >>= 7;
146+
}
147+
return curFieldLengthInBits;
148+
}
149+
139150
}

‎plc4j/spi/src/main/java/org/apache/plc4x/java/spi/generation/WriteBufferByteBased.java‎

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -401,24 +401,6 @@ public void writeInt(String logicalName, int bitLength, int value, WithWriterArg
401401
if (bitLength > 32) {
402402
throw new SerializationException("int can only contain max 32 bits");
403403
}
404-
try {
405-
if (byteOrder == ByteOrder.LITTLE_ENDIAN) {
406-
value = Integer.reverseBytes(value);
407-
}
408-
bo.writeInt(false, bitLength, value);
409-
} catch (Exception e) {
410-
throw new SerializationException("Error writing signed int", e);
411-
}
412-
}
413-
414-
@Override
415-
public void writeLong(String logicalName, int bitLength, long value, WithWriterArgs... writerArgs) throws SerializationException {
416-
if (bitLength <= 0) {
417-
throw new SerializationException("long must contain at least 1 bit");
418-
}
419-
if (bitLength > 64) {
420-
throw new SerializationException("long can only contain max 64 bits");
421-
}
422404
try {
423405
String encoding = extractEncoding(writerArgs).orElse("default");
424406
switch (encoding) {
@@ -449,6 +431,32 @@ public void writeLong(String logicalName, int bitLength, long value, WithWriterA
449431
}
450432
break;
451433
}
434+
case "default":
435+
if (byteOrder == ByteOrder.LITTLE_ENDIAN) {
436+
value = Integer.reverseBytes(value);
437+
}
438+
bo.writeInt(false, bitLength, value);
439+
break;
440+
default:
441+
throw new SerializationException("unsupported encoding '" + encoding + "'");
442+
}
443+
444+
} catch (Exception e) {
445+
throw new SerializationException("Error writing signed int", e);
446+
}
447+
}
448+
449+
@Override
450+
public void writeLong(String logicalName, int bitLength, long value, WithWriterArgs... writerArgs) throws SerializationException {
451+
if (bitLength <= 0) {
452+
throw new SerializationException("long must contain at least 1 bit");
453+
}
454+
if (bitLength > 64) {
455+
throw new SerializationException("long can only contain max 64 bits");
456+
}
457+
try {
458+
String encoding = extractEncoding(writerArgs).orElse("default");
459+
switch (encoding) {
452460
case "default":
453461
if (byteOrder == ByteOrder.LITTLE_ENDIAN) {
454462
value = Long.reverseBytes(value);

‎plc4j/spi/src/test/java/org/apache/plc4x/java/spi/generation/ReadBufferTest.java‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -53,16 +53,4 @@ void readStringUtf8() throws ParseException {
5353
assertEquals(value, answer);
5454
}
5555

56-
void readVarUint() throws ParseException {
57-
58-
}
59-
60-
@Test
61-
void readVarInt() throws Exception {
62-
byte[] serialized = Hex.decodeHex("8064");
63-
final ReadBuffer buffer = new ReadBufferByteBased(serialized);
64-
long answer = buffer.readLong("", 32, WithOption.WithEncoding("VARDINT"));
65-
assertEquals(100L, answer);
66-
}
67-
6856
}

‎plc4j/spi/src/test/java/org/apache/plc4x/java/spi/generation/VarIntTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,17 +75,17 @@ void writeVarUintRoundtrip(String hexString, long expectedValue, String descript
7575
"'BFFFFF7F', 134217727, 'Maximum positive value in 28 bits. (Groups: 63, 127, 127, 127.)'",
7676
"'C0808000', -134217728, 'Minimum negative value in 28 bits. (Groups: 64, 0, 0, 0 → 64<<21 = 134217728; then 134217728 – 268435456 = –134217728.)'",
7777
})
78-
void testVarIntRoundtrip(String hexString, long expectedValue, String description) throws Exception {
78+
void testVarIntRoundtrip(String hexString, int expectedValue, String description) throws Exception {
7979
byte[] serialized = Hex.decodeHex(hexString);
8080

8181
// Parse the given array into a value
8282
ReadBufferByteBased readBuffer = new ReadBufferByteBased(serialized);
83-
long value = readBuffer.readLong("", 32, WithOption.WithEncoding("VARDINT"));
83+
int value = readBuffer.readInt("", 32, WithOption.WithEncoding("VARDINT"));
8484
assertEquals(expectedValue, value);
8585

8686
// Serialize the given value into a byte array
8787
WriteBufferByteBased buffer = new WriteBufferByteBased(serialized.length);
88-
buffer.writeLong("", 32, expectedValue, WithOption.WithEncoding("VARDINT"));
88+
buffer.writeInt("", 32, expectedValue, WithOption.WithEncoding("VARDINT"));
8989
byte[] result = buffer.getBytes();
9090
assertArrayEquals(serialized, result, description);
9191
}

0 commit comments

Comments
 (0)
⚡