diff --git a/packages/omgidl-serialization/package.json b/packages/omgidl-serialization/package.json index c8f7344..f788321 100644 --- a/packages/omgidl-serialization/package.json +++ b/packages/omgidl-serialization/package.json @@ -50,7 +50,7 @@ "typescript": "4.9.5" }, "dependencies": { - "@foxglove/cdr": "3.1.0", + "@foxglove/cdr": "3.2.0", "@foxglove/message-definition": "^0.2.0" } } diff --git a/packages/omgidl-serialization/src/MessageReader.test.ts b/packages/omgidl-serialization/src/MessageReader.test.ts index 0b4058b..c9aac32 100644 --- a/packages/omgidl-serialization/src/MessageReader.test.ts +++ b/packages/omgidl-serialization/src/MessageReader.test.ts @@ -492,8 +492,8 @@ module builtin_interfaces { writer.emHeader(true, 1, 8); // heightMeters emHeader writer.float64(data.heightMeters); writer.emHeader(true, 2, 4 + 4 + 1); // address emHeader - // dHeader for inner object not written again because the object size is already specified in the emHeader - writer.emHeader(true, 1, 1); // pointer emHeader + // dHeader for inner object not written again because the object size is already specified in the emHeader and the lengthCode of 5 allows it to not be written again + writer.emHeader(true, 1, 1, 5); // pointer emHeader writer.uint8(data.address.pointer); writer.emHeader(true, 3, 1); // age emHeader writer.uint8(data.age); @@ -573,7 +573,8 @@ module builtin_interfaces { }); }); - it("reads mutable structs with arrays", () => { + it("reads mutable structs with arrays where size is in emHeader only", () => { + // size in only emheader means that lengthcode > 4 const msgDef = ` @mutable struct Plot { @@ -593,12 +594,14 @@ module builtin_interfaces { }; writer.dHeader(1); - writer.emHeader(true, 1, data.name.length + 1); // "name" field emHeader. add 1 for null terminator - writer.string(data.name, false); // don't write length again - writer.emHeader(true, 2, 3 * 8); // xValues emHeader - writer.float64Array(data.xValues, false); // do not write length of array again. Already included in emHeader + writer.emHeader(true, 1, data.name.length + 1, 2); // "name" field emHeader. add 1 for null terminator + writer.string(data.name, true); // need to write length because lengthCode < 5 + + writer.emHeader(true, 2, 3 * 8, 7); // xValues emHeader + writer.float64Array(data.xValues, false); // do not write length of array again. Already included in emHeader when lengthCode is 7 - writer.emHeader(true, 3, 3 * 8); // yValues emHeader + // size in only emheader means that lengthcode > 4 + writer.emHeader(true, 3, 3 * 8, 7); // yValues emHeader, lengthCode = 7 means we don't have to write sequenceLength writer.float64Array(data.yValues, false); // do not write length of array again. Already included in emHeader writer.emHeader(true, 4, 4); // count emHeader @@ -615,6 +618,48 @@ module builtin_interfaces { }); }); + it("reads mutable structs with arrays using length codes that cause sequenceLength to be written", () => { + const msgDef = ` + @mutable + struct Plot { + string name; + sequence xValues; + sequence yValues; + uint32 count; + }; + `; + + const writer = new CdrWriter({ size: 256, kind: EncapsulationKind.PL_CDR2_LE }); + const data = { + name: "MPG", + xValues: [1, 2, 3], + yValues: [4, 5, 6], + count: 3, + }; + + writer.dHeader(1); + writer.emHeader(true, 1, data.name.length + 1, 2); // "name" field emHeader. add 1 for null terminator + writer.string(data.name, true); // need to write length because lengthCode < 5 + writer.emHeader(true, 2, 3 * 8 + 1, 4); // xValues emHeader + writer.float64Array(data.xValues, /*writeLength:*/ true); // write length because lengthCode < 5 + + writer.emHeader(true, 3, 3 * 8 + 1, 4); // yValues emHeader + writer.float64Array(data.yValues, /*writeLength:*/ true); // write length because lengthCode < 5 + + writer.emHeader(true, 4, 4); // count emHeader + writer.uint32(data.count); + + const rootDef = "Plot"; + const reader = new MessageReader(rootDef, parseIDL(msgDef)); + + expect(reader.readMessage(writer.data)).toEqual({ + name: "MPG", + xValues: new Float64Array([1, 2, 3]), + yValues: new Float64Array([4, 5, 6]), + count: 3, + }); + }); + it("reads multi-dimensional fixed size arrays", () => { const msgDef = ` @mutable @@ -655,8 +700,9 @@ module builtin_interfaces { numbers: [], }; - writer.emHeader(true, 1, data.numbers.length + 4); // writes 4 because the sequence length is after it - writer.sequenceLength(0); + writer.dHeader(1); // dHeader of struct + writer.emHeader(true, 1, data.numbers.length + 4, 2); // writes 4 because the sequence length is after it + writer.sequenceLength(0); // Because its lengthCode < 5 const rootDef = "Array"; const reader = new MessageReader(rootDef, parseIDL(msgDef)); @@ -810,4 +856,56 @@ module builtin_interfaces { expect(msgout).toEqual(data); }); + + it("Reads mutable struct with string field that uses lengthCode = 4", () => { + const msgDef = ` + @mutable + struct Message { + @id(10) string text; + }; + `; + const data = { + text: "this is a string", + }; + const lengthCode = 4; + const writer = new CdrWriter({ kind: EncapsulationKind.PL_CDR2_LE }); + const cdrTextSizeBytes = data.text.length + 1; // +1 to include null terminator + writer.dHeader(8 + 4 + cdrTextSizeBytes); // emHeader 8 bytes header, 4 bytes sequenelength + nextint, then string length + writer.emHeader(true, 10, cdrTextSizeBytes, lengthCode); // emHeader does not take place of sequence length + writer.string(data.text, true); // write length because of lengthCode value 4 + + const rootDef = "Message"; + + const reader = new MessageReader(rootDef, parseIDL(msgDef)); + + const msgout = reader.readMessage(writer.data); + + expect(msgout).toEqual(data); + }); + + it("Reads mutable struct with string field that uses high lengthCode = 5", () => { + const msgDef = ` + @mutable + struct Message { + @id(10) string text; + }; + `; + const data = { + text: "this is a string", + }; + const lengthCode = 5; + const writer = new CdrWriter({ kind: EncapsulationKind.PL_CDR2_LE }); + const cdrTextSizeBytes = data.text.length + 1; // +1 to include null terminator + writer.dHeader(8 + cdrTextSizeBytes); // emHeader 8 bytes header + nextint, then string length + writer.emHeader(true, 10, cdrTextSizeBytes, lengthCode); // emHeader includes string length, because of high length code + writer.string(data.text, false); // no need to write length because of high length code + + const rootDef = "Message"; + + const reader = new MessageReader(rootDef, parseIDL(msgDef)); + + const msgout = reader.readMessage(writer.data); + + expect(msgout).toEqual(data); + }); }); diff --git a/packages/omgidl-serialization/src/MessageReader.ts b/packages/omgidl-serialization/src/MessageReader.ts index 74d5a78..13032d4 100644 --- a/packages/omgidl-serialization/src/MessageReader.ts +++ b/packages/omgidl-serialization/src/MessageReader.ts @@ -191,10 +191,11 @@ export class MessageReader { ): unknown { const { readMemberHeader, parentName } = emHeaderOptions; const definitionId = getDefinitionId(field); - let emHeaderSizeBytes = undefined; + let emHeaderSizeBytes; if (readMemberHeader) { - const { id, objectSize: objectSizeBytes } = reader.emHeader(); - emHeaderSizeBytes = objectSizeBytes; + const { id, objectSize: objectSizeBytes, lengthCode } = reader.emHeader(); + emHeaderSizeBytes = useEmHeaderAsLength(lengthCode) ? objectSizeBytes : undefined; + // this can help spot misalignments in reading the data if (definitionId != undefined && id !== definitionId) { throw Error( @@ -236,8 +237,6 @@ export class MessageReader { } const headerSpecifiedLength = emHeaderSizeBytes != undefined ? Math.floor(emHeaderSizeBytes / typeLength) : undefined; - const remainderBytes = - emHeaderSizeBytes != undefined ? emHeaderSizeBytes % typeLength : undefined; if (field.isArray === true) { const deser = typedArrayDeserializers.get(field.type); @@ -261,15 +260,7 @@ export class MessageReader { // last arrayLengths length is handled in deserializer. It returns an array return readNestedArray(typedArrayDeserializer, arrayLengths.slice(0, -1), 0); } else { - // Special case can happen where the emHeader is not divisible by the typeLength and there is a remainder of 4 bytes. - // 4 bytes of 0's are printed after the header length - // This can happen for 8 byte typeLength types. - if (arrayLengths[0] === 0 && remainderBytes === 4) { - reader.sequenceLength(); - return deser(reader, 0); - } else { - return deser(reader, arrayLengths[0]!); - } + return deser(reader, arrayLengths[0]!); } } else { const deser = deserializers.get(field.type); @@ -422,3 +413,8 @@ function getCaseForDiscriminator( } return unionDef.defaultCase; } + +/** Only length Codes >= 5 should allow for emHeader sizes to be used in place of other integer lengths */ +function useEmHeaderAsLength(lengthCode: number | undefined): boolean { + return lengthCode != undefined && lengthCode >= 5; +} diff --git a/yarn.lock b/yarn.lock index 8992946..a8eb201 100644 --- a/yarn.lock +++ b/yarn.lock @@ -452,10 +452,10 @@ __metadata: languageName: node linkType: hard -"@foxglove/cdr@npm:3.1.0": - version: 3.1.0 - resolution: "@foxglove/cdr@npm:3.1.0" - checksum: df27e4d1f2f6f4dab2c61a6a8ead7140ae1fbe4007e43b6c6690022f45db3dee838c8323b8aaf1660c8a9cc5da831f71f93bfbd2f18d94c12d0d2a8716420e47 +"@foxglove/cdr@npm:3.2.0": + version: 3.2.0 + resolution: "@foxglove/cdr@npm:3.2.0" + checksum: 1c8497debd32714bbe26724df1dbde783fddd6e8ef2e126df87f10221691419c1d8f1fdfb6dd9fd066408638aaa325d41aa6ccc9d1291c3d3774f2b844c9e546 languageName: node linkType: hard @@ -534,7 +534,7 @@ __metadata: version: 0.0.0-use.local resolution: "@foxglove/omgidl-serialization@workspace:packages/omgidl-serialization" dependencies: - "@foxglove/cdr": 3.1.0 + "@foxglove/cdr": 3.2.0 "@foxglove/message-definition": ^0.2.0 "@foxglove/omgidl-parser": "workspace:*" "@sounisi5011/jest-binary-data-matchers": 1.2.1