From 66e06ae234b2b5af0ef5bdacace938d85958d503 Mon Sep 17 00:00:00 2001 From: Matthew Steel Date: Wed, 10 Nov 2021 09:25:59 -0800 Subject: [PATCH] Add support for "ambiguous" message names (#106) The algorithm for resolving ROS type names inside message definitions that don't have a package specified is, - If it's a primitive, use the primitive value. - If it's the (reserved) type name `Header`, it's `std_msgs/Header`. - Otherwise, use the package of the containing message. (Note _containing_ message, not top-level message.) This unfortunately necessitates a breaking change in API. Message definitions can't be unambiguously parsed without knowing the package of the top-level message. (Definition MD5s are similarly ambiguous.) This felt like a good time to merge the named/unnamed definition types. Test plan: - Unit tests have been updated to use the new API. - Tested against a development version of webviz. Works on a bag I checked, and fixes the "ambiguous datatype" issue reported by a user. Co-authored-by: Matthew STEEL --- package.json | 2 +- src/BagReader.js | 4 +- src/MessageReader.js | 48 ++++++------------ src/MessageReader.test.js | 79 +++++++++++++++--------------- src/MessageWriter.js | 51 ++++++------------- src/MessageWriter.test.js | 77 ++++++++++++++++++++--------- src/bag.js | 4 +- src/header.js | 4 +- src/parseMessageDefinition.js | 70 +++++++++++++++----------- src/parseMessageDefinition.test.js | 67 ++++++++++++++++++------- src/record.js | 62 +++++++++-------------- src/types.js | 4 -- 12 files changed, 245 insertions(+), 227 deletions(-) diff --git a/package.json b/package.json index ea573f1..db4b903 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "rosbag", - "version": "2.6.4", + "version": "3.0.0", "license": "Apache-2.0", "repository": "cruise-automation/rosbag.js", "dependencies": { diff --git a/src/BagReader.js b/src/BagReader.js index 2181ddb..3646193 100644 --- a/src/BagReader.js +++ b/src/BagReader.js @@ -293,13 +293,13 @@ export default class BagReader { // read an individual record from a buffer readRecordFromBuffer(buffer: Buffer, fileOffset: number, cls: Class & { opcode: number }): T { const headerLength = buffer.readInt32LE(0); - const record = parseHeader(buffer.slice(4, 4 + headerLength), cls); + const headerFields = parseHeader(buffer.slice(4, 4 + headerLength), cls); const dataOffset = 4 + headerLength + 4; const dataLength = buffer.readInt32LE(4 + headerLength); const data = buffer.slice(dataOffset, dataOffset + dataLength); - record.parseData(data); + const record = new cls(headerFields, data); record.offset = fileOffset; record.dataOffset = record.offset + 4 + headerLength + 4; diff --git a/src/MessageReader.js b/src/MessageReader.js index 644c1f7..0ff8fc2 100644 --- a/src/MessageReader.js +++ b/src/MessageReader.js @@ -8,7 +8,7 @@ import int53 from "int53"; import { extractTime } from "./fields"; -import type { RosMsgDefinition, NamedRosMsgDefinition } from "./types"; +import type { RosMsgDefinition } from "./types"; import { parseMessageDefinition } from "./parseMessageDefinition"; type TypedArrayConstructor = ( @@ -179,42 +179,26 @@ class StandardTypeReader { } } -const findTypeByName = (types: RosMsgDefinition[], name = ""): NamedRosMsgDefinition => { - let foundName = ""; // track name separately in a non-null variable to appease Flow - const matches = types.filter((type) => { - const typeName = type.name || ""; - // if the search is empty, return unnamed types - if (!name) { - return !typeName; - } - // return if the search is in the type name - // or matches exactly if a fully-qualified name match is passed to us - const nameEnd = name.indexOf("/") > -1 ? name : `/${name}`; - if (typeName.endsWith(nameEnd)) { - foundName = typeName; - return true; - } - return false; - }); +const findTypeByName = (types: RosMsgDefinition[], name: string): RosMsgDefinition => { + const matches = types.filter((type) => type.name === name); if (matches.length !== 1) { throw new Error(`Expected 1 top level type definition for '${name}' but found ${matches.length}.`); } - return { ...matches[0], name: foundName }; + return matches[0]; }; const friendlyName = (name: string) => name.replace(/\//g, "_"); -const createParser = (types: RosMsgDefinition[], freeze: boolean) => { - const unnamedTypes = types.filter((type) => !type.name); - if (unnamedTypes.length !== 1) { - throw new Error("multiple unnamed types"); +const createParser = (types: RosMsgDefinition[], typeName: string, freeze: boolean) => { + const topLevelTypes = types.filter((type) => type.name === typeName); + if (topLevelTypes.length !== 1) { + throw new Error("multiple top-level types"); } + const [topLevelType] = topLevelTypes; - const [unnamedType] = unnamedTypes; - - const namedTypes: NamedRosMsgDefinition[] = (types.filter((type) => !!type.name): any[]); + const nestedTypes: RosMsgDefinition[] = types.filter((type) => type.name !== typeName); - const constructorBody = (type: RosMsgDefinition | NamedRosMsgDefinition) => { + const constructorBody = (type: RosMsgDefinition) => { const readerLines: string[] = []; type.definitions.forEach((def) => { if (def.isConstant) { @@ -264,10 +248,10 @@ const createParser = (types: RosMsgDefinition[], freeze: boolean) => { let js = ` var Record = function (reader) { - ${constructorBody(unnamedType)} + ${constructorBody(topLevelType)} };\n`; - namedTypes.forEach((t) => { + nestedTypes.forEach((t) => { js += ` Record.${friendlyName(t.name)} = function(reader) { ${constructorBody(t)} @@ -299,16 +283,16 @@ export class MessageReader { // takes an object message definition and returns // a message reader which can be used to read messages based // on the message definition - constructor(definitions: RosMsgDefinition[], options: { freeze?: ?boolean } = {}) { + constructor(definitions: RosMsgDefinition[], typeName: string, options: { freeze?: ?boolean } = {}) { let parsedDefinitions = definitions; if (typeof parsedDefinitions === "string") { // eslint-disable-next-line no-console console.warn( "Passing string message defintions to MessageReader is deprecated. Instead call `parseMessageDefinition` on it and pass in the resulting parsed message definition object." ); - parsedDefinitions = parseMessageDefinition(parsedDefinitions); + parsedDefinitions = parseMessageDefinition(parsedDefinitions, typeName); } - this.reader = createParser(parsedDefinitions, !!options.freeze); + this.reader = createParser(parsedDefinitions, typeName, !!options.freeze); } readMessage(buffer: Buffer) { diff --git a/src/MessageReader.test.js b/src/MessageReader.test.js index 3e4530e..e6f06d5 100644 --- a/src/MessageReader.test.js +++ b/src/MessageReader.test.js @@ -19,13 +19,19 @@ const getStringBuffer = (str: string) => { return Buffer.concat([len, data]); }; +const getMessageReader = (messageDefinitions, options = {}) => { + const typeName = options.typeName || "custom_type/CustomMessage"; + const parsedMessageDefinitions = parseMessageDefinition(messageDefinitions, typeName); + return new MessageReader(parsedMessageDefinitions, typeName, options.readerOptions); +}; + describe("MessageReader", () => { describe("simple type", () => { const testNum = (type: string, size: number, expected: any, cb: (buffer: Buffer) => any) => { const buffer = new Buffer(size); cb(buffer); it(`parses buffer ${JSON.stringify(buffer)} containing ${type}`, () => { - const reader = new MessageReader(parseMessageDefinition(`${type} foo`)); + const reader = getMessageReader(`${type} foo`); expect(reader.readMessage(buffer)).toEqual({ foo: expected, }); @@ -42,7 +48,7 @@ describe("MessageReader", () => { testNum("float64", 8, 0xdeadbeefcafebabe, (buffer) => buffer.writeDoubleLE(0xdeadbeefcafebabe, 0)); it("parses string", () => { - const reader = new MessageReader(parseMessageDefinition("string name")); + const reader = getMessageReader("string name"); const buff = getStringBuffer("test"); expect(reader.readMessage(buff)).toEqual({ name: "test", @@ -61,7 +67,7 @@ describe("MessageReader", () => { // $FlowFixMe flow doesn't like util.TextDecoder global.TextDecoder = util.TextDecoder; - const reader = new MessageReader(parseMessageDefinition("string name")); + const reader = getMessageReader("string name"); const string = range(0, 5000) .map(() => String.fromCharCode(Math.floor(Math.random() * 255))) .join(""); @@ -73,36 +79,30 @@ describe("MessageReader", () => { }); it("parses JSON", () => { - const reader = new MessageReader(parseMessageDefinition("#pragma rosbag_parse_json\nstring dummy")); + const reader = getMessageReader("#pragma rosbag_parse_json\nstring dummy"); const buff = getStringBuffer('{"foo":123,"bar":{"nestedFoo":456}}'); expect(reader.readMessage(buff)).toEqual({ dummy: { foo: 123, bar: { nestedFoo: 456 } }, }); - const readerWithSpaces = new MessageReader( - parseMessageDefinition(" #pragma rosbag_parse_json \n string dummy") - ); + const readerWithSpaces = getMessageReader(" #pragma rosbag_parse_json \n string dummy"); expect(readerWithSpaces.readMessage(buff)).toEqual({ dummy: { foo: 123, bar: { nestedFoo: 456 } }, }); - const readerWithNewlines = new MessageReader( - parseMessageDefinition("#pragma rosbag_parse_json\n\n\nstring dummy") - ); + const readerWithNewlines = getMessageReader("#pragma rosbag_parse_json\n\n\nstring dummy"); expect(readerWithNewlines.readMessage(buff)).toEqual({ dummy: { foo: 123, bar: { nestedFoo: 456 } }, }); - const readerWithNestedComplexType = new MessageReader( - parseMessageDefinition(`#pragma rosbag_parse_json + const readerWithNestedComplexType = getMessageReader(`#pragma rosbag_parse_json string dummy Account account ============ MSG: custom_type/Account string name uint16 id - `) - ); + `); expect( readerWithNestedComplexType.readMessage( Buffer.concat([buff, getStringBuffer('{"first":"First","last":"Last"}}'), new Buffer([100, 0x00])]) @@ -112,8 +112,7 @@ describe("MessageReader", () => { account: { name: '{"first":"First","last":"Last"}}', id: 100 }, }); - const readerWithTrailingPragmaComment = new MessageReader( - parseMessageDefinition(`#pragma rosbag_parse_json + const readerWithTrailingPragmaComment = getMessageReader(`#pragma rosbag_parse_json string dummy Account account #pragma rosbag_parse_json @@ -121,8 +120,7 @@ describe("MessageReader", () => { MSG: custom_type/Account string name uint16 id - `) - ); + `); expect( readerWithTrailingPragmaComment.readMessage( Buffer.concat([buff, getStringBuffer('{"first":"First","last":"Last"}}'), new Buffer([100, 0x00])]) @@ -134,7 +132,7 @@ describe("MessageReader", () => { }); it("parses time", () => { - const reader = new MessageReader(parseMessageDefinition("time right_now")); + const reader = getMessageReader("time right_now"); const buff = new Buffer(8); const now = new Date(); now.setSeconds(31); @@ -161,7 +159,7 @@ describe("MessageReader", () => { ### foo bar baz? string lastName `; - const reader = new MessageReader(parseMessageDefinition(messageDefinition)); + const reader = getMessageReader(messageDefinition); const buffer = Buffer.concat([getStringBuffer("foo"), getStringBuffer("bar")]); expect(reader.readMessage(buffer)).toEqual({ firstName: "foo", @@ -171,14 +169,14 @@ describe("MessageReader", () => { it("still works given string message definitions", () => { const messageDefinition = "string value"; - const reader = new MessageReader(parseMessageDefinition(messageDefinition)); + const reader = getMessageReader(messageDefinition); const buffer = getStringBuffer("foo"); expect(reader.readMessage(buffer)).toEqual({ value: "foo" }); }); describe("array", () => { it("parses variable length string array", () => { - const reader = new MessageReader(parseMessageDefinition("string[] names")); + const reader = getMessageReader("string[] names"); const buffer = Buffer.concat([ // variable length array has int32 as first entry new Buffer([0x03, 0x00, 0x00, 0x00]), @@ -192,9 +190,9 @@ describe("MessageReader", () => { }); it("parses fixed length arrays", () => { - const parser1 = new MessageReader(parseMessageDefinition("string[1] names")); - const parser2 = new MessageReader(parseMessageDefinition("string[2] names")); - const parser3 = new MessageReader(parseMessageDefinition("string[3] names")); + const parser1 = getMessageReader("string[1] names"); + const parser2 = getMessageReader("string[2] names"); + const parser3 = getMessageReader("string[3] names"); const buffer = Buffer.concat([getStringBuffer("foo"), getStringBuffer("bar"), getStringBuffer("baz")]); expect(parser1.readMessage(buffer)).toEqual({ names: ["foo"], @@ -208,7 +206,7 @@ describe("MessageReader", () => { }); it("uses an empty array for a 0 length array", () => { - const reader = new MessageReader(parseMessageDefinition("string[] names")); + const reader = getMessageReader("string[] names"); const buffer = Buffer.concat([ // variable length array has int32 as first entry new Buffer([0x00, 0x00, 0x00, 0x00]), @@ -220,7 +218,7 @@ describe("MessageReader", () => { describe("typed arrays", () => { it("uint8[] uses the same backing buffer", () => { - const reader = new MessageReader(parseMessageDefinition("uint8[] values\nuint8 after")); + const reader = getMessageReader("uint8[] values\nuint8 after"); const buffer = Buffer.from([0x03, 0x00, 0x00, 0x00, 0x01, 0x02, 0x03, 0x04]); const result = reader.readMessage(buffer); const { values, after } = result; @@ -237,7 +235,7 @@ describe("MessageReader", () => { }); it("parses uint8[] with a fixed length", () => { - const reader = new MessageReader(parseMessageDefinition("uint8[3] values\nuint8 after")); + const reader = getMessageReader("uint8[3] values\nuint8 after"); const buffer = Buffer.from([0x01, 0x02, 0x03, 0x04]); const result = reader.readMessage(buffer); const { values, after } = result; @@ -254,7 +252,7 @@ describe("MessageReader", () => { }); it("int8[] uses the same backing buffer", () => { - const reader = new MessageReader(parseMessageDefinition("int8[] values\nint8 after")); + const reader = getMessageReader("int8[] values\nint8 after"); const buffer = new Buffer([0x03, 0x00, 0x00, 0x00, 0x01, 0x02, 0x03, 0x04]); const result = reader.readMessage(buffer); const { values, after } = result; @@ -271,7 +269,7 @@ describe("MessageReader", () => { }); it("parses int8[] with a fixed length", () => { - const reader = new MessageReader(parseMessageDefinition("int8[3] values\nint8 after")); + const reader = getMessageReader("int8[3] values\nint8 after"); const buffer = new Buffer([0x01, 0x02, 0x03, 0x04]); const result = reader.readMessage(buffer); const { values, after } = result; @@ -288,7 +286,7 @@ describe("MessageReader", () => { }); it("parses combinations of typed arrays", () => { - const reader = new MessageReader(parseMessageDefinition("int8[] first\nuint8[2] second")); + const reader = getMessageReader("int8[] first\nuint8[2] second"); const buffer = new Buffer([0x02, 0x00, 0x00, 0x00, 0x01, 0x02, 0x03, 0x04]); const result = reader.readMessage(buffer); const { first, second } = result; @@ -310,7 +308,8 @@ describe("MessageReader", () => { describe("complex types", () => { it("parses single complex type", () => { - const reader = new MessageReader(parseMessageDefinition("string firstName \n string lastName\nuint16 age")); + const messageDefinition = "string firstName \n string lastName\nuint16 age"; + const reader = getMessageReader(messageDefinition); const buffer = Buffer.concat([getStringBuffer("foo"), getStringBuffer("bar"), new Buffer([0x05, 0x00])]); expect(reader.readMessage(buffer)).toEqual({ firstName: "foo", @@ -328,7 +327,7 @@ describe("MessageReader", () => { string name uint16 id `; - const reader = new MessageReader(parseMessageDefinition(messageDefinition)); + const reader = getMessageReader(messageDefinition); const buffer = Buffer.concat([getStringBuffer("foo"), getStringBuffer("bar"), new Buffer([100, 0x00])]); expect(reader.readMessage(buffer)).toEqual({ username: "foo", @@ -348,7 +347,7 @@ describe("MessageReader", () => { string name uint16 id `; - const reader = new MessageReader(parseMessageDefinition(messageDefinition)); + const reader = getMessageReader(messageDefinition); const buffer = Buffer.concat([ getStringBuffer("foo"), // uint32LE length of array (2) @@ -388,8 +387,7 @@ describe("MessageReader", () => { string url uint8 id `; - - const reader = new MessageReader(parseMessageDefinition(messageDefinition)); + const reader = getMessageReader(messageDefinition); const buffer = Buffer.concat([ getStringBuffer("foo"), // uint32LE length of Account array (2) @@ -479,7 +477,10 @@ describe("MessageReader", () => { string name # a description of the test/component reporting`; it("parses bytes and constants", () => { - const reader = new MessageReader(parseMessageDefinition(withBytesAndBools)); + const reader = getMessageReader(withBytesAndBools, { + typeName: "diagnostic_msgs/DiagnosticArray", + readerOptions: {}, + }); const buffer = Buffer.concat([Buffer.from([0x01]), Buffer.from([0x00]), getStringBuffer("foo")]); const message = reader.readMessage(buffer); @@ -493,8 +494,8 @@ describe("MessageReader", () => { }); it("freezes the resulting message if requested", () => { - const reader = new MessageReader(parseMessageDefinition("string firstName \n string lastName\nuint16 age"), { - freeze: true, + const reader = getMessageReader("string firstName \n string lastName\nuint16 age", { + readerOptions: { freeze: true }, }); const buffer = Buffer.concat([getStringBuffer("foo"), getStringBuffer("bar"), new Buffer([0x05, 0x00])]); const output = reader.readMessage(buffer); diff --git a/src/MessageWriter.js b/src/MessageWriter.js index fcb777d..442aab8 100644 --- a/src/MessageWriter.js +++ b/src/MessageWriter.js @@ -7,7 +7,7 @@ // @flow import int53 from "int53"; -import type { Time, RosMsgDefinition, NamedRosMsgDefinition } from "./types"; +import type { Time, RosMsgDefinition } from "./types"; // write a Time object to a buffer. function writeTime(time: Time, buffer: Buffer, offset: number) { @@ -167,27 +167,12 @@ class StandardTypeWriter { } } -const findTypeByName = (types: RosMsgDefinition[], name = ""): NamedRosMsgDefinition => { - let foundName = ""; // track name separately in a non-null variable to appease Flow - const matches = types.filter((type) => { - const typeName = type.name || ""; - // if the search is empty, return unnamed types - if (!name) { - return !typeName; - } - // return if the search is in the type name - // or matches exactly if a fully-qualified name match is passed to us - const nameEnd = name.indexOf("/") > -1 ? name : `/${name}`; - if (typeName.endsWith(nameEnd)) { - foundName = typeName; - return true; - } - return false; - }); - if (matches.length !== 1) { - throw new Error(`Expected 1 top level type definition for '${name}' but found ${matches.length}.`); +const findTypeByName = (types: RosMsgDefinition[], name: string): RosMsgDefinition => { + const ret = types.find((type) => type.name === name); + if (ret == null) { + throw new Error(`Type '${name}' but not found.`); } - return { ...matches[0], name: foundName }; + return ret; }; const friendlyName = (name: string) => name.replace(/\//g, "_"); @@ -196,17 +181,11 @@ type WriterAndSizeCalculator = {| bufferSizeCalculator: (message: any) => number, |}; -function createWriterAndSizeCalculator(types: RosMsgDefinition[]): WriterAndSizeCalculator { - const unnamedTypes = types.filter((type) => !type.name); - if (unnamedTypes.length !== 1) { - throw new Error("multiple unnamed types"); - } - - const [unnamedType] = unnamedTypes; - - const namedTypes: NamedRosMsgDefinition[] = (types.filter((type) => !!type.name): any[]); +function createWriterAndSizeCalculator(types: RosMsgDefinition[], typeName: string): WriterAndSizeCalculator { + const topLevelType = findTypeByName(types, typeName); + const nestedTypes = types.filter((type) => type.name !== typeName); - const constructorBody = (type: RosMsgDefinition | NamedRosMsgDefinition, argName: "offsetCalculator" | "writer") => { + const constructorBody = (type: RosMsgDefinition, argName: "offsetCalculator" | "writer") => { const lines: string[] = []; type.definitions.forEach((def) => { if (def.isConstant) { @@ -252,7 +231,7 @@ function createWriterAndSizeCalculator(types: RosMsgDefinition[]): WriterAndSize let writerJs = ""; let calculateSizeJs = ""; - namedTypes.forEach((t) => { + nestedTypes.forEach((t) => { writerJs += ` function ${friendlyName(t.name)}(writer, message) { ${constructorBody(t, "writer")} @@ -265,12 +244,12 @@ function createWriterAndSizeCalculator(types: RosMsgDefinition[]): WriterAndSize writerJs += ` return function write(writer, message) { - ${constructorBody(unnamedType, "writer")} + ${constructorBody(topLevelType, "writer")} return writer.buffer; };`; calculateSizeJs += ` return function calculateSize(offsetCalculator, message) { - ${constructorBody(unnamedType, "offsetCalculator")} + ${constructorBody(topLevelType, "offsetCalculator")} return offsetCalculator.offset; };`; @@ -308,8 +287,8 @@ export class MessageWriter { // takes an object string message definition and returns // a message writer which can be used to write messages based // on the message definition - constructor(definitions: RosMsgDefinition[]) { - const { writer, bufferSizeCalculator } = createWriterAndSizeCalculator(definitions); + constructor(definitions: RosMsgDefinition[], typeName: string) { + const { writer, bufferSizeCalculator } = createWriterAndSizeCalculator(definitions, typeName); this.writer = writer; this.bufferSizeCalculator = bufferSizeCalculator; } diff --git a/src/MessageWriter.test.js b/src/MessageWriter.test.js index b75e815..a4bb129 100644 --- a/src/MessageWriter.test.js +++ b/src/MessageWriter.test.js @@ -24,7 +24,8 @@ describe("MessageWriter", () => { const message = { foo: expected }; cb(buffer); it(`writes message ${JSON.stringify(message)} containing ${type}`, () => { - const writer = new MessageWriter(parseMessageDefinition(`${type} foo`)); + const typeName = "foo_msgs/Bar"; + const writer = new MessageWriter(parseMessageDefinition(`${type} foo`, typeName), typeName); expect( writer.writeMessage({ foo: expected, @@ -43,13 +44,18 @@ describe("MessageWriter", () => { testNum("float64", 8, 0xdeadbeefcafebabe, (buffer) => buffer.writeDoubleLE(0xdeadbeefcafebabe, 0)); it("writes strings", () => { - const writer = new MessageWriter(parseMessageDefinition("string name")); + const typeName = "foo_msgs/Bar"; + const writer = new MessageWriter(parseMessageDefinition("string name", typeName), typeName); const buff = getStringBuffer("test"); expect(writer.writeMessage({ name: "test" })).toEqual(buff); }); it("writes JSON", () => { - const writer = new MessageWriter(parseMessageDefinition("#pragma rosbag_parse_json\nstring dummy")); + const typeName = "custom_type/User"; + const writer = new MessageWriter( + parseMessageDefinition("#pragma rosbag_parse_json\nstring dummy", typeName), + typeName + ); const buff = getStringBuffer('{"foo":123,"bar":{"nestedFoo":456}}'); expect( writer.writeMessage({ @@ -58,14 +64,18 @@ describe("MessageWriter", () => { ).toEqual(buff); const writerWithNestedComplexType = new MessageWriter( - parseMessageDefinition(`#pragma rosbag_parse_json + parseMessageDefinition( + `#pragma rosbag_parse_json string dummy Account account ============ MSG: custom_type/Account string name uint16 id - `) + `, + typeName + ), + typeName ); expect( writerWithNestedComplexType.writeMessage({ @@ -75,7 +85,8 @@ describe("MessageWriter", () => { ).toEqual(Buffer.concat([buff, getStringBuffer('{"first":"First","last":"Last"}}'), new Buffer([100, 0x00])])); const writerWithTrailingPragmaComment = new MessageWriter( - parseMessageDefinition(`#pragma rosbag_parse_json + parseMessageDefinition( + `#pragma rosbag_parse_json string dummy Account account #pragma rosbag_parse_json @@ -83,7 +94,10 @@ describe("MessageWriter", () => { MSG: custom_type/Account string name uint16 id - `) + `, + typeName + ), + typeName ); expect( writerWithTrailingPragmaComment.writeMessage({ @@ -94,7 +108,8 @@ describe("MessageWriter", () => { }); it("writes time", () => { - const writer = new MessageWriter(parseMessageDefinition("time right_now")); + const typeName = "foo_msgs/Bar"; + const writer = new MessageWriter(parseMessageDefinition("time right_now", typeName), typeName); const buff = new Buffer(8); const now = new Date(); now.setSeconds(31); @@ -116,7 +131,8 @@ describe("MessageWriter", () => { describe("array", () => { it("writes variable length string array", () => { - const writer = new MessageWriter(parseMessageDefinition("string[] names")); + const typeName = "foo_msgs/Bar"; + const writer = new MessageWriter(parseMessageDefinition("string[] names", typeName), typeName); const buffer = Buffer.concat([ // variable length array has int32 as first entry new Buffer([0x03, 0x00, 0x00, 0x00]), @@ -132,9 +148,10 @@ describe("MessageWriter", () => { }); it("writes fixed length arrays", () => { - const writer1 = new MessageWriter(parseMessageDefinition("string[1] names")); - const writer2 = new MessageWriter(parseMessageDefinition("string[2] names")); - const writer3 = new MessageWriter(parseMessageDefinition("string[3] names")); + const typeName = "foo_msgs/Bar"; + const writer1 = new MessageWriter(parseMessageDefinition("string[1] names", typeName), typeName); + const writer2 = new MessageWriter(parseMessageDefinition("string[2] names", typeName), typeName); + const writer3 = new MessageWriter(parseMessageDefinition("string[3] names", typeName), typeName); expect( writer1.writeMessage({ names: ["foo", "bar", "baz"], @@ -153,7 +170,8 @@ describe("MessageWriter", () => { }); it("does not write any data for a zero length array", () => { - const writer = new MessageWriter(parseMessageDefinition("string[] names")); + const typeName = "foo_msgs/Bar"; + const writer = new MessageWriter(parseMessageDefinition("string[] names", typeName), typeName); const buffer = Buffer.concat([ // variable length array has int32 as first entry new Buffer([0x00, 0x00, 0x00, 0x00]), @@ -165,7 +183,8 @@ describe("MessageWriter", () => { describe("typed arrays", () => { it("writes a uint8[]", () => { - const writer = new MessageWriter(parseMessageDefinition("uint8[] values\nuint8 after")); + const typeName = "foo_msgs/Bar"; + const writer = new MessageWriter(parseMessageDefinition("uint8[] values\nuint8 after", typeName), typeName); const message = { values: Uint8Array.from([1, 2, 3]), after: 4 }; const result = writer.writeMessage(message); const buffer = Buffer.from([0x03, 0x00, 0x00, 0x00, 0x01, 0x02, 0x03, 0x04]); @@ -173,7 +192,8 @@ describe("MessageWriter", () => { }); it("writes a uint8[] with a fixed length", () => { - const writer = new MessageWriter(parseMessageDefinition("uint8[3] values\nuint8 after")); + const typeName = "foo_msgs/Bar"; + const writer = new MessageWriter(parseMessageDefinition("uint8[3] values\nuint8 after", typeName), typeName); const message = { values: Uint8Array.from([1, 2, 3]), after: 4 }; const result = writer.writeMessage(message); const buffer = Buffer.from([0x01, 0x02, 0x03, 0x04]); @@ -184,7 +204,11 @@ describe("MessageWriter", () => { describe("complex types", () => { it("writes single complex type", () => { - const writer = new MessageWriter(parseMessageDefinition("string firstName \n string lastName\nuint16 age")); + const typeName = "foo_msgs/Bar"; + const writer = new MessageWriter( + parseMessageDefinition("string firstName \n string lastName\nuint16 age", typeName), + typeName + ); const buffer = Buffer.concat([getStringBuffer("foo"), getStringBuffer("bar"), new Buffer([0x05, 0x00])]); const message = { firstName: "foo", @@ -203,7 +227,8 @@ describe("MessageWriter", () => { string name uint16 id `; - const writer = new MessageWriter(parseMessageDefinition(messageDefinition)); + const typeName = "custom_type/User"; + const writer = new MessageWriter(parseMessageDefinition(messageDefinition, typeName), typeName); const buffer = Buffer.concat([getStringBuffer("foo"), getStringBuffer("bar"), new Buffer([100, 0x00])]); const message = { username: "foo", @@ -224,7 +249,8 @@ describe("MessageWriter", () => { string name uint16 id `; - const writer = new MessageWriter(parseMessageDefinition(messageDefinition)); + const typeName = "custom_type/User"; + const writer = new MessageWriter(parseMessageDefinition(messageDefinition, typeName), typeName); const buffer = Buffer.concat([ getStringBuffer("foo"), // uint32LE length of array (2) @@ -266,7 +292,8 @@ describe("MessageWriter", () => { uint8 id `; - const writer = new MessageWriter(parseMessageDefinition(messageDefinition)); + const typeName = "custom_type/User"; + const writer = new MessageWriter(parseMessageDefinition(messageDefinition, typeName), typeName); const buffer = Buffer.concat([ getStringBuffer("foo"), // uint32LE length of Account array (2) @@ -358,7 +385,8 @@ describe("MessageWriter", () => { string name # a description of the test/component reporting`; it("writes bytes and constants", () => { - const writer = new MessageWriter(parseMessageDefinition(withBytesAndBools)); + const typeName = "diagnostic_msgs/DiagnosticArray"; + const writer = new MessageWriter(parseMessageDefinition(withBytesAndBools, typeName), typeName); const buffer = Buffer.concat([Buffer.from([0x01]), Buffer.from([0x00]), getStringBuffer("foo")]); const message = { @@ -421,7 +449,8 @@ describe("MessageWriter", () => { ], }; - const writer = new MessageWriter(parseMessageDefinition(messageDefinition)); + const typeName = "custom_type/User"; + const writer = new MessageWriter(parseMessageDefinition(messageDefinition, typeName), typeName); expect(writer.calculateBufferSize(message)).toEqual(108); }); }); @@ -475,8 +504,10 @@ describe("MessageWriter", () => { ], }; - const reader = new MessageReader(parseMessageDefinition(messageDefinition)); - const writer = new MessageWriter(parseMessageDefinition(messageDefinition)); + const typeName = "custom_type/User"; + const parsedDefinition = parseMessageDefinition(messageDefinition, typeName); + const reader = new MessageReader(parsedDefinition, typeName); + const writer = new MessageWriter(parsedDefinition, typeName); expect(reader.readMessage(writer.writeMessage(message))).toEqual(message); }); }); diff --git a/src/bag.js b/src/bag.js index 383a86a..9d66673 100644 --- a/src/bag.js +++ b/src/bag.js @@ -99,14 +99,14 @@ export default class Bag { function parseMsg(msg: MessageData, chunkOffset: number): ReadResult { const connection = connections[msg.conn]; - const { topic } = connection; + const { topic, type } = connection; const { data, time: timestamp } = msg; let message = null; if (!opts.noParse) { // lazily create a reader for this connection if it doesn't exist connection.reader = connection.reader || - new MessageReader(parseMessageDefinition(connection.messageDefinition), { freeze: opts.freeze }); + new MessageReader(parseMessageDefinition(connection.messageDefinition, type), type, { freeze: opts.freeze }); message = connection.reader.readMessage(data); } return new ReadResult(topic, message, timestamp, data, chunkOffset, chunkInfos.length, opts.freeze); diff --git a/src/header.js b/src/header.js index b09cb0d..aaaf8d8 100644 --- a/src/header.js +++ b/src/header.js @@ -11,7 +11,7 @@ import { Record } from "./record"; // given a buffer parses out the record within the buffer // based on the opcode type bit -export function parseHeader(buffer: Buffer, cls: Class & { opcode: number }): T { +export function parseHeader(buffer: Buffer, cls: Class & { opcode: number }): { [key: string]: Buffer } { const fields = extractFields(buffer); if (fields.op === undefined) { throw new Error("Header is missing 'op' field."); @@ -21,5 +21,5 @@ export function parseHeader(buffer: Buffer, cls: Class & { opcode: throw new Error(`Expected ${cls.name} (${cls.opcode}) but found ${opcode}`); } - return new cls(fields); + return fields; } diff --git a/src/parseMessageDefinition.js b/src/parseMessageDefinition.js index d18b682..b9ab448 100644 --- a/src/parseMessageDefinition.js +++ b/src/parseMessageDefinition.js @@ -61,24 +61,24 @@ function newDefinition(type: string, name: string): RosMsgField { }; } -const buildType = (lines: { isJson: boolean, line: string }[]): RosMsgDefinition => { +const tokenizeLine = (line: string) => + line + .replace(/#.*/gi, "") + .split(" ") + .filter((word) => word); + +const buildNamedType = (lines: { isJson: boolean, line: string }[], typeName: string): RosMsgDefinition => { const definitions: RosMsgField[] = []; - let complexTypeName: ?string; lines.forEach(({ isJson, line }) => { // remove comments and extra whitespace from each line - const splits = line - .replace(/#.*/gi, "") - .split(" ") - .filter((word) => word); + const splits = tokenizeLine(line); if (!splits[1]) { return; } // consume comments const type = splits[0].trim(); const name = splits[1].trim(); - if (type === "MSG:") { - complexTypeName = name; - } else if (name.indexOf("=") > -1 || splits.indexOf("=") > -1) { + if (name.indexOf("=") > -1 || splits.indexOf("=") > -1) { // constant type parsing const matches = line.match(/(\S+)\s*=\s*(.*)\s*/); if (!matches) { @@ -120,31 +120,40 @@ const buildType = (lines: { isJson: boolean, line: string }[]): RosMsgDefinition definitions.push(newDefinition(isJson ? "json" : type, name)); } }); - return { name: complexTypeName, definitions }; + return { name: typeName, definitions }; }; -const findTypeByName = (types: RosMsgDefinition[], name: string): RosMsgDefinition => { - const matches = types.filter((type) => { - const typeName = type.name || ""; - // if the search is empty, return unnamed types - if (!name) { - return !typeName; - } - // return if the search is in the type name - // or matches exactly if a fully-qualified name match is passed to us - const nameEnd = name.indexOf("/") > -1 ? name : `/${name}`; - return typeName.endsWith(nameEnd); - }); +const buildType = (lines: { isJson: boolean, line: string }[]): RosMsgDefinition => { + if (lines.length === 0) { + throw new Error("Empty message definition."); + } + if (!lines[0].line.startsWith("MSG: ")) { + throw new Error(`Malformed message definition name: ${lines[0].line}`); + } + const typeName = tokenizeLine(lines[0].line)[1].trim(); + return buildNamedType(lines.slice(1), typeName); +}; + +const findTypeByName = (types: RosMsgDefinition[], name: string, rosPackage: string): RosMsgDefinition => { + const fullName = name.includes("/") ? name : name === "Header" ? "std_msgs/Header" : `${rosPackage}/${name}`; + const matches = types.filter((type) => type.name === fullName); if (matches.length !== 1) { - throw new Error(`Expected 1 top level type definition for '${name}' but found ${matches.length}`); + throw new Error( + `Expected 1 top level type definition for '${name}' but found ${matches.length}, ${JSON.stringify({ + fullName, + k: types.map((type) => type.name), + })}` + ); } return matches[0]; }; // Given a raw message definition string, parse it into an object representation. +// Type names in all positions are always fully-qualified. +// // Example return value: // [{ -// name: undefined, +// name: "foo_msgs/Bar", // definitions: [ // { // arrayLength: undefined, @@ -157,7 +166,7 @@ const findTypeByName = (types: RosMsgDefinition[], name: string): RosMsgDefiniti // }, ... ] // // See unit tests for more examples. -export function parseMessageDefinition(messageDefinition: string) { +export function parseMessageDefinition(messageDefinition: string, typeName: string) { // read all the lines and remove empties const allLines = messageDefinition .split("\n") @@ -180,20 +189,23 @@ export function parseMessageDefinition(messageDefinition: string) { // definitions are split by equal signs if (line.startsWith("==")) { nextDefinitionIsJson = false; - types.push(buildType(definitionLines)); + const definition = types.length === 0 ? buildNamedType(definitionLines, typeName) : buildType(definitionLines); + types.push(definition); definitionLines = []; } else { definitionLines.push({ isJson: nextDefinitionIsJson, line }); nextDefinitionIsJson = false; } }); - types.push(buildType(definitionLines)); + const definition = types.length === 0 ? buildNamedType(definitionLines, typeName) : buildType(definitionLines); + types.push(definition); // Fix up complex type names - types.forEach(({ definitions }) => { + types.forEach(({ name, definitions }) => { + const typePackage = name.split("/")[0]; definitions.forEach((definition) => { if (definition.isComplex) { - const foundName = findTypeByName(types, definition.type).name; + const foundName = findTypeByName(types, definition.type, typePackage).name; if (foundName === undefined) { throw new Error(`Missing type definition for ${definition.type}`); } diff --git a/src/parseMessageDefinition.test.js b/src/parseMessageDefinition.test.js index 26142de..bdca734 100644 --- a/src/parseMessageDefinition.test.js +++ b/src/parseMessageDefinition.test.js @@ -10,7 +10,7 @@ import { parseMessageDefinition } from "./parseMessageDefinition"; describe("parseMessageDefinition", () => { it("parses a single field from a single message", () => { - const types = parseMessageDefinition("string name"); + const types = parseMessageDefinition("string name", "foo_msgs/Bar"); expect(types).toEqual([ { definitions: [ @@ -22,7 +22,7 @@ describe("parseMessageDefinition", () => { type: "string", }, ], - name: undefined, + name: "foo_msgs/Bar", }, ]); }); @@ -34,7 +34,7 @@ describe("parseMessageDefinition", () => { MSG: geometry_msgs/Point float64 x `; - const types = parseMessageDefinition(messageDefinition); + const types = parseMessageDefinition(messageDefinition, "geometry_msgs/Polygon"); expect(types).toEqual([ { definitions: [ @@ -46,7 +46,7 @@ describe("parseMessageDefinition", () => { type: "geometry_msgs/Point", }, ], - name: undefined, + name: "geometry_msgs/Polygon", }, { definitions: [ @@ -63,8 +63,39 @@ describe("parseMessageDefinition", () => { ]); }); + it("resolves seemingly-ambiguous unqualified names", () => { + const messageDefinition = ` + Header header # doesn't say std_msgs, but we special-case it. + Ambiguous m1 # refers to this_msgs/Ambiguous + other_msgs/Other + ============ + MSG: other_msgs/Other + Ambiguous # refers to other_msgs/Ambiguous + ============ + MSG: this_msgs/Ambiguous + ============ + MSG: other_msgs/Ambiguous + ============ + MSG: std_msgs/Header + `; + const types = parseMessageDefinition(messageDefinition, "this_msgs/Message"); + expect(types).toEqual([ + { + definitions: [ + { isArray: false, isComplex: true, name: "header", type: "std_msgs/Header" }, + { isArray: false, isComplex: true, name: "m1", type: "this_msgs/Ambiguous" }, + ], + name: "this_msgs/Message", + }, + { definitions: [], name: "other_msgs/Other" }, + { definitions: [], name: "this_msgs/Ambiguous" }, + { definitions: [], name: "other_msgs/Ambiguous" }, + { definitions: [], name: "std_msgs/Header" }, + ]); + }); + it("normalizes aliases", () => { - const types = parseMessageDefinition("char x\nbyte y"); + const types = parseMessageDefinition("char x\nbyte y", "foo_msgs/Bar"); expect(types).toEqual([ { definitions: [ @@ -83,7 +114,7 @@ describe("parseMessageDefinition", () => { type: "int8", }, ], - name: undefined, + name: "foo_msgs/Bar", }, ]); }); @@ -97,7 +128,7 @@ describe("parseMessageDefinition", () => { ### foo bar baz? string lastName `; - const types = parseMessageDefinition(messageDefinition); + const types = parseMessageDefinition(messageDefinition, "foo_msgs/Bar"); expect(types).toEqual([ { definitions: [ @@ -116,13 +147,13 @@ describe("parseMessageDefinition", () => { type: "string", }, ], - name: undefined, + name: "foo_msgs/Bar", }, ]); }); it("parses variable length string array", () => { - const types = parseMessageDefinition("string[] names"); + const types = parseMessageDefinition("string[] names", "foo_msgs/Bar"); expect(types).toEqual([ { definitions: [ @@ -134,13 +165,13 @@ describe("parseMessageDefinition", () => { type: "string", }, ], - name: undefined, + name: "foo_msgs/Bar", }, ]); }); it("parses fixed length string array", () => { - const types = parseMessageDefinition("string[3] names"); + const types = parseMessageDefinition("string[3] names", "foo_msgs/Bar"); expect(types).toEqual([ { definitions: [ @@ -152,7 +183,7 @@ describe("parseMessageDefinition", () => { type: "string", }, ], - name: undefined, + name: "foo_msgs/Bar", }, ]); }); @@ -166,7 +197,7 @@ describe("parseMessageDefinition", () => { string name uint16 id `; - const types = parseMessageDefinition(messageDefinition); + const types = parseMessageDefinition(messageDefinition, "custom_type/CustomMessage"); expect(types).toEqual([ { definitions: [ @@ -185,7 +216,7 @@ describe("parseMessageDefinition", () => { type: "custom_type/Account", }, ], - name: undefined, + name: "custom_type/CustomMessage", }, { definitions: [ @@ -218,7 +249,7 @@ describe("parseMessageDefinition", () => { string fooStr = Foo ${""} string EXAMPLE="#comments" are ignored, and leading and trailing whitespace removed `; - const types = parseMessageDefinition(messageDefinition); + const types = parseMessageDefinition(messageDefinition, "foo_msgs/Bar"); expect(types).toEqual([ { definitions: [ @@ -259,7 +290,7 @@ describe("parseMessageDefinition", () => { value: '"#comments" are ignored, and leading and trailing whitespace removed', }, ], - name: undefined, + name: "foo_msgs/Bar", }, ]); }); @@ -269,7 +300,7 @@ describe("parseMessageDefinition", () => { bool Alive=True bool Dead=False `; - const types = parseMessageDefinition(messageDefinition); + const types = parseMessageDefinition(messageDefinition, "foo_msgs/Bar"); expect(types).toEqual([ { definitions: [ @@ -286,7 +317,7 @@ describe("parseMessageDefinition", () => { value: false, }, ], - name: undefined, + name: "foo_msgs/Bar", }, ]); }); diff --git a/src/record.js b/src/record.js index 06a679c..bb583d9 100644 --- a/src/record.js +++ b/src/record.js @@ -22,9 +22,7 @@ export class Record { end: number; length: number; - constructor(_fields: { [key: string]: any }) {} - - parseData(_buffer: Buffer) {} + constructor(_fields: { [key: string]: any }, _buffer: Buffer) {} } export class BagHeader extends Record { @@ -33,8 +31,8 @@ export class BagHeader extends Record { connectionCount: number; chunkCount: number; - constructor(fields: { [key: string]: Buffer }) { - super(fields); + constructor(fields: { [key: string]: Buffer }, buffer: Buffer) { + super(fields, buffer); this.indexPosition = readUInt64LE(fields.index_pos); this.connectionCount = fields.conn_count.readInt32LE(0); this.chunkCount = fields.chunk_count.readInt32LE(0); @@ -47,13 +45,10 @@ export class Chunk extends Record { size: number; data: Buffer; - constructor(fields: { [key: string]: Buffer }) { - super(fields); + constructor(fields: { [key: string]: Buffer }, buffer: Buffer) { + super(fields, buffer); this.compression = fields.compression.toString(); this.size = fields.size.readUInt32LE(0); - } - - parseData(buffer: Buffer) { this.data = buffer; } } @@ -69,32 +64,28 @@ export class Connection extends Record { static opcode = 7; conn: number; topic: string; - type: ?string; - md5sum: ?string; + type: string; + md5sum: string; messageDefinition: string; callerid: ?string; latching: ?boolean; reader: ?MessageReader; - constructor(fields: { [key: string]: Buffer }) { - super(fields); + constructor(fields: { [key: string]: Buffer }, buffer: Buffer) { + super(fields, buffer); this.conn = fields.conn.readUInt32LE(0); this.topic = fields.topic.toString(); - this.type = undefined; - this.md5sum = undefined; this.messageDefinition = ""; - } - parseData(buffer: Buffer) { - const fields = extractFields(buffer); - this.type = getField(fields, "type"); - this.md5sum = getField(fields, "md5sum"); - this.messageDefinition = getField(fields, "message_definition"); - if (fields.callerid !== undefined) { - this.callerid = fields.callerid.toString(); + const bufferFields = extractFields(buffer); + this.type = getField(bufferFields, "type"); + this.md5sum = getField(bufferFields, "md5sum"); + this.messageDefinition = getField(bufferFields, "message_definition"); + if (bufferFields.callerid !== undefined) { + this.callerid = bufferFields.callerid.toString(); } - if (fields.latching !== undefined) { - this.latching = fields.latching.toString() === "1"; + if (bufferFields.latching !== undefined) { + this.latching = bufferFields.latching.toString() === "1"; } } } @@ -105,13 +96,10 @@ export class MessageData extends Record { time: Time; data: Buffer; - constructor(fields: { [key: string]: Buffer }) { - super(fields); + constructor(fields: { [key: string]: Buffer }, buffer: Buffer) { + super(fields, buffer); this.conn = fields.conn.readUInt32LE(0); this.time = extractTime(fields.time, 0); - } - - parseData(buffer: Buffer) { this.data = buffer; } } @@ -123,14 +111,12 @@ export class IndexData extends Record { count: number; indices: Array<{ time: Time, offset: number }>; - constructor(fields: { [key: string]: Buffer }) { - super(fields); + constructor(fields: { [key: string]: Buffer }, buffer: Buffer) { + super(fields, buffer); this.ver = fields.ver.readUInt32LE(0); this.conn = fields.conn.readUInt32LE(0); this.count = fields.count.readUInt32LE(0); - } - parseData(buffer: Buffer) { this.indices = []; for (let i = 0; i < this.count; i++) { this.indices.push({ @@ -151,16 +137,14 @@ export class ChunkInfo extends Record { connections: Array<{ conn: number, count: number }>; nextChunk: ?ChunkInfo; - constructor(fields: { [key: string]: Buffer }) { - super(fields); + constructor(fields: { [key: string]: Buffer }, buffer: Buffer) { + super(fields, buffer); this.ver = fields.ver.readUInt32LE(0); this.chunkPosition = readUInt64LE(fields.chunk_pos); this.startTime = extractTime(fields.start_time, 0); this.endTime = extractTime(fields.end_time, 0); this.count = fields.count.readUInt32LE(0); - } - parseData(buffer: Buffer) { this.connections = []; for (let i = 0; i < this.count; i++) { this.connections.push({ diff --git a/src/types.js b/src/types.js index 3a37215..73d2906 100644 --- a/src/types.js +++ b/src/types.js @@ -38,10 +38,6 @@ export type RosMsgField = {| |}; export type RosMsgDefinition = {| - name?: string, - definitions: RosMsgField[], -|}; -export type NamedRosMsgDefinition = {| name: string, definitions: RosMsgField[], |};