Fix empty message handling for constant-only messages (#33)

### Changelog
- Fixed a bug where messages containing only constants were not
correctly serialized/deserialized.

### Description

Follow-up to #16.
I found that this fix was only made for completely empty message
definitions, but it also needs to apply when a message has only constant
fields.

Related: FG-9334

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@foxglove/rosmsg2-serialization",
-  "version": "3.0.0",
+  "version": "3.0.1",
   "description": "ROS 2 (Robot Operating System) message serialization, for reading and writing bags and network messages",
   "license": "MIT",
   "keywords": [

src/MessageReader.test.ts

@@ -515,8 +515,23 @@ module builtin_interfaces {
     expect(read).toEqual({});
   });
 
-  it("should deserialize a custom msg with a std_msgs/msg/Empty field", () => {
-    const buffer = Uint8Array.from(Buffer.from("00010000000000007b000000", "hex"));
+  it("should deserialize a custom msg with a std_msgs/msg/Empty field followed by uint8", () => {
+    // Note: ROS/FastDDS seems to add 2 extra padding bytes at the end
+    const buffer = Uint8Array.from(Buffer.from("00010000007b0000", "hex"));
+    const msgDef = `
+    std_msgs/msg/Empty empty
+    uint8 uint_8_field
+    ================================================================================
+    MSG: std_msgs/msg/Empty
+    `;
+    const reader = new MessageReader(parseMessageDefinition(msgDef, { ros2: true }));
+    const read = reader.readMessage(buffer);
+
+    expect(read).toEqual({ empty: {}, uint_8_field: 123 });
+  });
+
+  it("should deserialize a custom msg with a std_msgs/msg/Empty field followed by int32", () => {
+    const buffer = Uint8Array.from(Buffer.from("00010000000000007b000001", "hex"));
     const msgDef = `
     std_msgs/msg/Empty empty
     int32 int_32_field
@@ -526,7 +541,22 @@ module builtin_interfaces {
     const reader = new MessageReader(parseMessageDefinition(msgDef, { ros2: true }));
     const read = reader.readMessage(buffer);
 
-    expect(read).toEqual({ empty: {}, int_32_field: 123 });
+    expect(read).toEqual({ empty: {}, int_32_field: 16777339 });
+  });
+
+  it("should deserialize a custom msg with an empty message (with constants) followed by int32", () => {
+    const buffer = Uint8Array.from(Buffer.from("00010000000000007b000001", "hex"));
+    const msgDef = `
+    custom_msgs/msg/Nothing empty
+    int32 int_32_field
+    ================================================================================
+    MSG: custom_msgs/msg/Nothing
+    int32 EXAMPLE=123
+    `;
+    const reader = new MessageReader(parseMessageDefinition(msgDef, { ros2: true }));
+    const read = reader.readMessage(buffer);
+
+    expect(read).toEqual({ empty: {}, int_32_field: 16777339 });
   });
 
   it("ros2idl should choose non-constant root definition", () => {

src/MessageReader.ts

@@ -2,6 +2,8 @@ import { CdrReader } from "@foxglove/cdr";
 import { MessageDefinition, MessageDefinitionField } from "@foxglove/message-definition";
 import { Time as Ros1Time } from "@foxglove/rostime";
 
+import { messageDefinitionHasDataFields } from "./messageDefinitionHasDataFields";
+
 type Ros2Time = {
   sec: number;
   nanosec: number;
@@ -73,7 +75,7 @@ export class MessageReader<T = unknown> {
   ): Record<string, unknown> {
     const msg: Record<string, unknown> = {};
 
-    if (definition.length === 0) {
+    if (!messageDefinitionHasDataFields(definition)) {
       // In case a message definition definition is empty, ROS 2 adds a
       // `uint8 structure_needs_at_least_one_member` field when converting to IDL,
       // to satisfy the requirement from IDL of not being empty.

src/MessageWriter.test.ts

@@ -495,16 +495,49 @@ module builtin_interfaces {
     expect(writer.calculateByteSize(message)).toEqual(expected.byteLength);
   });
 
-  it("should serialize a custom msg with a std_msgs/msg/Empty field", () => {
-    const expected = Uint8Array.from(Buffer.from("00010000000000007b000000", "hex"));
+  it("should serialize a custom msg with a std_msgs/msg/Empty field followed by uint8", () => {
+    const expected = Uint8Array.from(Buffer.from("00010000007b", "hex"));
+    const msgDef = `
+    std_msgs/msg/Empty empty
+    uint8 uint_8_field
+    ================================================================================
+    MSG: std_msgs/msg/Empty
+    `;
+    const writer = new MessageWriter(parseMessageDefinition(msgDef, { ros2: true }));
+    const message = { uint_8_field: 123 };
+    const written = writer.writeMessage(message);
+
+    expect(written).toBytesEqual(expected);
+    expect(writer.calculateByteSize(message)).toEqual(expected.byteLength);
+  });
+
+  it("should serialize a custom msg with a std_msgs/msg/Empty field followed by int32", () => {
+    const expected = Uint8Array.from(Buffer.from("00010000000000007b000001", "hex"));
     const msgDef = `
     std_msgs/msg/Empty empty
     int32 int_32_field
     ================================================================================
     MSG: std_msgs/msg/Empty
     `;
     const writer = new MessageWriter(parseMessageDefinition(msgDef, { ros2: true }));
-    const message = { int_32_field: 123 };
+    const message = { int_32_field: 16777339 };
+    const written = writer.writeMessage(message);
+
+    expect(written).toBytesEqual(expected);
+    expect(writer.calculateByteSize(message)).toEqual(expected.byteLength);
+  });
+
+  it("should serialize a custom msg withan empty message (with constants) followed by int32", () => {
+    const expected = Uint8Array.from(Buffer.from("00010000000000007b000001", "hex"));
+    const msgDef = `
+    custom_msgs/msg/Nothing empty
+    int32 int_32_field
+    ================================================================================
+    MSG: custom_msgs/msg/Nothing
+    int32 EXAMPLE=123
+    `;
+    const writer = new MessageWriter(parseMessageDefinition(msgDef, { ros2: true }));
+    const message = { int_32_field: 16777339 };
     const written = writer.writeMessage(message);
 
     expect(written).toBytesEqual(expected);

src/MessageWriter.ts

@@ -5,6 +5,8 @@ import {
   MessageDefinitionField,
 } from "@foxglove/message-definition";
 
+import { messageDefinitionHasDataFields } from "./messageDefinitionHasDataFields";
+
 type PrimitiveWriter = (
   value: unknown,
   defaultValue: DefaultValue,
@@ -119,7 +121,7 @@ export class MessageWriter {
     const messageObj = message as Record<string, unknown> | undefined;
     let newOffset = offset;
 
-    if (definition.length === 0) {
+    if (!messageDefinitionHasDataFields(definition)) {
       // In case a message definition definition is empty, ROS 2 adds a
       // `uint8 structure_needs_at_least_one_member` field when converting to IDL,
       // to satisfy the requirement from IDL of not being empty.
@@ -193,7 +195,7 @@ export class MessageWriter {
   #write(definition: MessageDefinitionField[], message: unknown, writer: CdrWriter): void {
     const messageObj = message as Record<string, unknown> | undefined;
 
-    if (definition.length === 0) {
+    if (!messageDefinitionHasDataFields(definition)) {
       // In case a message definition definition is empty, ROS 2 adds a
       // `uint8 structure_needs_at_least_one_member` field when converting to IDL,
       // to satisfy the requirement from IDL of not being empty.

src/messageDefinitionHasDataFields.ts

@@ -0,0 +1,5 @@
+import { MessageDefinitionField } from "@foxglove/message-definition";
+
+export function messageDefinitionHasDataFields(fields: MessageDefinitionField[]): boolean {
+  return fields.some((field) => field.isConstant !== true);
+}