eProsima / Fast-DDS-Gen

Fast-DDS IDL code generator tool. Looking for commercial support? Contact info@eprosima.com
Apache License 2.0
82 stars 62 forks source link

Error generating ROS2 idl messages [10690] #52

Closed Andrea2202 closed 1 year ago

Andrea2202 commented 3 years ago

Hello everybody, I am trying to generate structures to communicate using ros2 idl messages. I'm able to run fastddgen for all packages except for ros2 navigation ones

The error that appears for the nav_msgs / msg / OccupancyGrid.idl message is

builtin_interfaces :: msg :: Time is already defined

I enclose the structure here

// generated from rosidl_adapter / resource / msg.idl.em
// with input from nav_msgs / msg / OccupancyGrid.msg
// generated code does not contain a copyright notice

#include "nav_msgs / msg / MapMetaData.idl"
#include "std_msgs / msg / Header.idl"

module nav_msgs {
  module msg {
    @verbatim (language = "comment", text =
      "This represents a 2-D grid map, in which each cell represents the probability of occupancy.")
    struct OccupancyGrid {
      std_msgs :: msg :: Header header;

      @verbatim (language = "comment", text =
        "MetaData for the map")
      nav_msgs :: msg :: MapMetaData info;

      @verbatim (language = "comment", text =
        "The map data, in row-major order, starting with (0,0). Occupancy" "\ n"
        "probabilities are in the range [0,100]. Unknown is -1.")
      sequence <int8> data;
    };
  };
};

You can also tell me why, for some messages are only generated the structure Generating Type definition files ... Generating TypeObject files ... And not even pubsub files?

Thanks in advance

fwr-ml commented 3 years ago

Greetings,

For that particular error: Those IDL files have no include guards.

If you've got those files from installed ROS2 packages (i.e. from '/opt/ros/foxy/share/...'), next to those IDLs should be a directory 'ddsconnext'. I did ignore those for a while, since that directory unfortunately claims - by name - to be connext specific, until I realized that those seem to be THE ONES defining the DDS Topic Type realizations of higher layer ROS2 Topics, Services, and Actions. The structs there are defined in a namespace 'dds', also, just as the FQ-names of types communicated by ROS2 DDS participants suggest.

When you compare for instance the IDL files for turtlesim's rotate absolute action, you'll see that only the DDS-specific variant actually defines the topic types you see on the wire - the other one does not. The latter fact caused me to notice the error in my initial interpretation of what's what.

Regards

LakeFusion commented 2 years ago

Hello everybody, we are having the same issue while generating code from PolygonStamped.idl from ROS2. IDL file looks like this:

#include "Polygon.idl"
#include "Header.idl"

module geometry_msgs {
  module msg {
    @verbatim (language="comment", text=
      " This represents a Polygon with reference coordinate frame and timestamp")
    struct PolygonStamped {
      std_msgs::msg::Header header;

      geometry_msgs::msg::Polygon polygon;
    };
  };
};

The strange thing is, after changing the variable name from polygon to polygon_ in the IDL file, fast-dds-gen generates the code without any problems. So it looks like the variable name is colliding with type name but the same does not happen to header. Maybe because header type belongs to another namespace.

fwr-ml commented 2 years ago

What I said before, i.e. use the file ".../geometry_msgs/msg/ddsconnext/PolygonStamped.idl" instead to generate code from. The contents of those idl-files match the fully qualified type names ROS2 announces on the dds layer. The struct in the file you're using as input does not.

I don't recall everything, but there are also technical problems with the ROS2-provided non-"dds_connext" idl-files. At least that there are no header guards, though that problem kicks in only with a subset of topic types.

LakeFusion commented 2 years ago

Standard idl files in ROS2 are perfectly valid. Include guards are not needed (at least it is the case with the fast-dds-gen version I'm using).

Besides, PointCloud2.idl (not from dds_connext subfolder) generated by fast-dds-gen (with -typeros2 flag) generates fine and is fully compatible with ROS2.

Anyway, out of curiosity I've tried to generate with the idl files from the dds_connext subfolder. Still having the same error.

PolygonStamped_.idl:20:43: error: Illegal identifier: geometry_msgs::msg::dds_::polygon_ is already defined (Type: geometry_msgs::msg::dds_::Polygon_=com.eprosima.idl.parser.tree.TypeDeclaration@75329a49)

fwr-ml commented 2 years ago

I looked at my modifications of Fast-DDS-Gen, but don't see anything that might relate here. But I noticed, in a script I call fastddsgen with flag -cs. IIRC, it's case insensitive by default. Maybe that's causing your troubles. Though you said, it wouldn't complain in case of "header".

LakeFusion commented 2 years ago

Interesting, I would have expected it to do case sensitive by default. Thanks for pointing out.

I've found an additional yet similar issue while generating Odometry.idl. It occurs regardless of the -cs flag. This time, the error message claimed double was redefined. TwistWithCovariance.idl:9:12: error: 'double' was redefined

#include "Twist.idl"

module geometry_msgs {
  module msg {
    typedef double double__36[36];
    @verbatim (language="comment", text=
      " This expresses velocity in free space with uncertainty.")
    struct TwistWithCovariance {
      geometry_msgs::msg::Twist twist;

      @verbatim (language="comment", text=
        " Row-major representation of the 6x6 covariance matrix" "\n"
        " The orientation parameters use a fixed-axis representation." "\n"
        " In order, the parameters are:" "\n"
        " (x, y, z, rotation about X axis, rotation about Y axis, rotation about Z axis)")
      double__36 covariance;
    };
  };
};

in this case, renaming the typename from double__36 to doubl__36 was necessary to make code generation continue. It seems string comparison was done regardless of the string length.

fwr-ml commented 2 years ago

I know it's getting old, but that message type doesn't employ typedefs in its dds_connext variant. I must admit I'm rather coming from a I-need-something-that-works mindset. And as I recall, I found using the dds_connext IDLs less of a hastle. If you're more about debugging fastddsgen, keep at it. I just had the impression that there'd be much to do.

LakeFusion commented 2 years ago

As much as I'd love to, I won't be able to find the time to debug the tool itself. By mentioning strange behaviour I've encountered, I'm simply hoping to help the person who can fix the bug as well as others who are encountering similar issues while generating code from ROS2 idls.

Ryanf55 commented 1 year ago

It's odd, MicroXRCEDDSGen does not have this issue in the generation I wrote for Ardupilot: https://github.com/ArduPilot/ardupilot/blob/master/libraries/AP_DDS/wscript#L88C80-L88C96

However, I am facing it with fastddsgen. I don't understand why it's different. I'm already using -cs use the -typeros2 option in fastddsgen.

Ryanf55 commented 1 year ago

Here is a minimum reproducible example using the vulcanexus humble image.

docker run -it eprosima/vulcanexus:humble-tools bash
apt update && apt -y install ros-humble-vision-msgs
mkdir -p vision_msgs/msg
# Generate all
fastddsgen -cs -replace -typeros2 -d vision_msgs/msg -I /opt/ros/humble/share /opt/ros/humble/share/vision_msgs/msg/*.idl

You can even just try generating one message that fails

fastddsgen -cs -replace -typeros2 -d vision_msgs/msg -I /opt/ros/humble/share /opt/ros/humble/share/vision_msgs/msg/Detection2DArray.idl

Output:

fastddsgen -cs -replace -typeros2 -d vision_msgs/msg -I /opt/ros/humble/share /opt/ros/humble/share/vision_msgs/msg/*.idl
openjdk version "11.0.19" 2023-04-18
OpenJDK Runtime Environment (build 11.0.19+7-post-Ubuntu-0ubuntu122.04.1)
OpenJDK 64-Bit Server VM (build 11.0.19+7-post-Ubuntu-0ubuntu122.04.1, mixed mode, sharing)
Loading templates from /usr/bin/../share/fastddsgen/java/fastddsgen.jar
Template resource folders: com/eprosima/fastdds/idl/templates:com/eprosima/fastcdr/idl/templates
Processing the file /opt/ros/humble/share/vision_msgs/msg/BoundingBox2D.idl...
Generating Type definition files...
Generating TopicDataTypes files...
Adding project: /opt/ros/humble/share/vision_msgs/msg/BoundingBox2D.idl
Processing the file /opt/ros/humble/share/vision_msgs/msg/BoundingBox2DArray.idl...
Generating Type definition files...
Generating TopicDataTypes files...
Adding project: /opt/ros/humble/share/vision_msgs/msg/BoundingBox2DArray.idl
Processing the file /opt/ros/humble/share/vision_msgs/msg/BoundingBox3D.idl...
Generating Type definition files...
Generating TopicDataTypes files...
Adding project: /opt/ros/humble/share/vision_msgs/msg/BoundingBox3D.idl
Processing the file /opt/ros/humble/share/vision_msgs/msg/BoundingBox3DArray.idl...
Generating Type definition files...
Generating TopicDataTypes files...
Adding project: /opt/ros/humble/share/vision_msgs/msg/BoundingBox3DArray.idl
Processing the file /opt/ros/humble/share/vision_msgs/msg/Classification.idl...
WARNING (File Classification, Line 22): Annotation unit not supported. Ignoring...
Generating Type definition files...
Generating TopicDataTypes files...
Adding project: /opt/ros/humble/share/vision_msgs/msg/Classification.idl
Processing the file /opt/ros/humble/share/vision_msgs/msg/Detection2D.idl...
WARNING (File Detection2D, Line 22): Annotation unit not supported. Ignoring...
Generating Type definition files...
Generating TopicDataTypes files...
Adding project: /opt/ros/humble/share/vision_msgs/msg/Detection2D.idl
Processing the file /opt/ros/humble/share/vision_msgs/msg/Detection2DArray.idl...
In file included from opt/ros/humble/share/std_msgs/msg/Header.idl:5:0,
                      opt/ros/humble/share/vision_msgs/msg/Detection2D.idl:5:0,
                      opt/ros/humble/share/vision_msgs/msg/Detection2DArray.idl:6:0:
opt/ros/humble/share/builtin_interfaces/msg/Time.idl:11:16: error: Illegal identifier: builtin_interfaces::msg::Time is already defined (Definition: com.eprosima.idl.parser.tree.TypeDeclaration@37ddb69a)
In file included from opt/ros/humble/share/std_msgs/msg/Header.idl:5:0,
                      opt/ros/humble/share/vision_msgs/msg/Detection2D.idl:5:0,
                      opt/ros/humble/share/vision_msgs/msg/Detection2DArray.idl:6:0:
opt/ros/humble/share/builtin_interfaces/msg/Time.idl:14:6: error: no viable alternative at input 'int32'
In file included from opt/ros/humble/share/std_msgs/msg/Header.idl:5:0,
                      opt/ros/humble/share/vision_msgs/msg/Detection2D.idl:5:0,
                      opt/ros/humble/share/vision_msgs/msg/Detection2DArray.idl:6:0:
opt/ros/humble/share/builtin_interfaces/msg/Time.idl:18:6: error: no viable alternative at input 'uint32'
In file included from opt/ros/humble/share/std_msgs/msg/Header.idl:5:0,
                      opt/ros/humble/share/vision_msgs/msg/Detection2D.idl:5:0,
                      opt/ros/humble/share/vision_msgs/msg/Detection2DArray.idl:6:0:
opt/ros/humble/share/builtin_interfaces/msg/Time.idl:20:2: error: Unexpected input '}'
In file included from opt/ros/humble/share/vision_msgs/msg/Detection2D.idl:5:0,
                      opt/ros/humble/share/vision_msgs/msg/Detection2DArray.idl:6:0:
opt/ros/humble/share/std_msgs/msg/Header.idl:13:18: error: Illegal identifier: std_msgs::msg::Header is already defined (Definition: com.eprosima.idl.parser.tree.TypeDeclaration@2b5f4d54)
In file included from opt/ros/humble/share/vision_msgs/msg/Detection2D.idl:5:0,
                      opt/ros/humble/share/vision_msgs/msg/Detection2DArray.idl:6:0:
opt/ros/humble/share/std_msgs/msg/Header.idl:16:6: error: no viable alternative at input 'builtin_interfaces'
In file included from opt/ros/humble/share/vision_msgs/msg/Detection2D.idl:5:0,
                      opt/ros/humble/share/vision_msgs/msg/Detection2DArray.idl:6:0:
opt/ros/humble/share/std_msgs/msg/Header.idl:20:6: error: no viable alternative at input 'string'
In file included from opt/ros/humble/share/vision_msgs/msg/Detection2D.idl:5:0,
                      opt/ros/humble/share/vision_msgs/msg/Detection2DArray.idl:6:0:
opt/ros/humble/share/std_msgs/msg/Header.idl:22:2: error: Unexpected input '}'
WARNING (File Detection2DArray, Line 22): Annotation unit not supported. Ignoring...
Exception in thread "main" java.lang.NullPointerException
    at com.eprosima.fastdds.fastddsgen.execute(fastddsgen.java:441)
    at com.eprosima.fastdds.fastddsgen.main(fastddsgen.java:1558)

Theory: The failing message has two header fields:

root@56cc410e55f6:/# ros2 interface show vision_msgs/msg/Detection2DArray | grep header
std_msgs/Header header
    std_msgs/Header header
Ryanf55 commented 1 year ago

Hi @JLBuenoLopez-eProsima , I tried installing the FastDDSGen release 3 candidate, but couldn't compile it. Would you be able to test the steps above against the new version to see if this is present?

JLBuenoLopez commented 1 year ago

Hi @Ryanf55

I am afraid that Fast DDS-Gen and Micro XRCE DDS-Gen are two different and independent products. I understand your frustration when something works in one but not in the other.

Currently, Fast DDS-Gen requires that each IDL file (and its dependencies) define the types once and only once. For example, having two IDL files, one including the other and calling Fast DDS-Gen for both of them, it is fine, as each IDL file its passed to Fast DDS-Gen independently. The code will be regenerated the second time depending on the -replace option.

What it is not allowed is if you have three IDL files, the first one including the other two, and the second one including the third one. In that case, the third IDL file is included twice in the same generation unit and the redefined variable error is going to be thrown.

Regrettably this issue is not fixed in Fast DDS-Gen nor it is going to be fixed in the next release (Fast DDS-Gen v3.0.0). The release candidate you mention it is not yet ready and as it is a mayor version, it would require to upgrade other dependencies in order to work properly (surely the cause of your issues installing the release candidate).

Ryanf55 commented 1 year ago

Hi @Ryanf55

I am afraid that Fast DDS-Gen and Micro XRCE DDS-Gen are two different and independent products. I understand your frustration when something works in one but not in the other.

Currently, Fast DDS-Gen requires that each IDL file (and its dependencies) define the types once and only once. For example, having two IDL files, one including the other and calling Fast DDS-Gen for both of them, it is fine, as each IDL file its passed to Fast DDS-Gen independently. The code will be regenerated the second time depending on the -replace option.

What it is not allowed is if you have three IDL files, the first one including the other two, and the second one including the third one. In that case, the third IDL file is included twice in the same generation unit and the redefined variable error is going to be thrown.

Regrettably this issue is not fixed in Fast DDS-Gen nor it is going to be fixed in the next release (Fast DDS-Gen v3.0.0). The release candidate you mention it is not yet ready and as it is a mayor version, it would require to upgrade other dependencies in order to work properly (surely the cause of your issues installing the release candidate).

Many thanks for the response!

Are there any known workarounds other than changing our message definitions when using FastDDS to not use any messages that use the same type twice through includes? According to the RTI forums for a user facing the same problem in their software, you can modify the IDL's to add a #pragma once directive.

Changing the message definitions and code implementation to not have these kinds of includes would be significant work. Any alternatives are much appreciated!

Ryanf55 commented 1 year ago

I added #pragma once to the top of every single IDL file, before the #include statements. It solved the problem. fastddsgen has warnings on it, but I ignore them and the build compiles.

JLBuenoLopez commented 1 year ago

I have issued a PR to Fast DDS documentation concerning this issue:

As a C/C++ preprocessor is run by default, any preprocessor directive is applied. For instance, using compilation guards or the #pragma once directive to avoid the multiple inclusion of the same IDL file, which issues the variable already defined error reported in this issue. Consequently, I am going to close this issue.