[NOT FOR MERGE] Add uint8 field when ROS 2 message definition is empty. #34
Conversation
Since IDL structs can't be empty, ROS adds a uint8 field in case the message definition is empty (e.g. std_msgs/Empty): See https://github.com/ros2/rosidl/blob/e3b71ece/rosidl_adapter/rosidl_adapter/resource/struct.idl.em#L101 This patch mirros this behavior.
| // Since IDL structs can't be empty, ROS adds a uint8 field in case the | ||
| // message definition is empty (e.g. std_msgs/Empty): | ||
| // https://github.com/ros2/rosidl/blob/e3b71ece/rosidl_adapter/rosidl_adapter/resource/struct.idl.em#L101 | ||
| if (lines.length === 0) { |
There was a problem hiding this comment.
This seems like a serialization time concern rather than something that would be in the MessageDefinition itself (which is more of an app description for the message fields). Is it possible to make this a rosmsg2-serialization thing rather than a message definition builder thing?
There was a problem hiding this comment.
Doing that in rosmsg2-serialization is definitely a possibility. However, since the IDL definition contains that extra member, I find it more correct to modify the MessageDefinition to be consistent with IDL parsing and how the field is displayed in Studio (e.g. RawMessage / Publish panels).
Content of Empty.idl:
// generated from rosidl_adapter/resource/msg.idl.em
// with input from std_msgs/msg/Empty.msg
// generated code does not contain a copyright notice
module std_msgs {
module msg {
struct Empty {
uint8 structure_needs_at_least_one_member;
};
};
};
There was a problem hiding this comment.
What happens in ROS tooling when working with these empty messages? Does ros topic echo show the field? Or are they special-casing the std_msgs/Empty (or even all empty messages?).
In practice empty messages are rare so this is an edge case with limited risk and impact but figuring out what the expectations are for a ROS user and following those seems defensible.
There was a problem hiding this comment.
What happens in ROS tooling when working with these empty messages? Does
ros topic echoshow the field? Or are they special-casing thestd_msgs/Empty(or even all empty messages?).
The python based ros2cli tools do not show the padding member anymore (since ros2/rosidl_python#73). And indeed, this applies for all empty message definitions, also empty service requests such as of std_srvs/Trigger (cfr. foxglove/ros-foxglove-bridge#229)
I have now implemented this in foxglove/rosmsg2-serialization#16, to avoid that the padding member is being shown.
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. See also https://design.ros2.org/articles/legacy_interface_definition.html This patch inserts or skips that extra member when serializing or deserializing respectively. Replaces foxglove/rosmsg#34, to avoid that the `structure_needs_at_least_one_member` is being displayed.
achim-k
changed the title
|
Superseded by foxglove/rosmsg2-serialization#16 |
### Public-Facing Changes Fix serde of empty message definition ### Description 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. See also https://design.ros2.org/articles/legacy_interface_definition.html This patch inserts or skips that extra member when serializing or deserializing respectively. Replaces foxglove/rosmsg#34, to avoid that the `structure_needs_at_least_one_member` is being displayed.
Public-Facing Changes
Description
Since IDL does not allow empty structs, ROS adds an extra uint8 field in case the message definition does not contain any fields (e.g.
std_msgs/Empty). See https://github.com/ros2/rosidl/blob/e3b71ece/rosidl_adapter/rosidl_adapter/resource/struct.idl.em#L101This patch mirrors this behavior.
Motivation:
Consider the following message definition
empty_test_msgs/msg/EmptyAndInt32:When publishing this type on a topic
Studio does not parse the message correctly, as it considers the empty field to have a size of 0.
This patch will fix foxglove/ros-foxglove-bridge#229