rosidl icon indicating copy to clipboard operation
rosidl copied to clipboard

ROSIDL - Support generating files for fastddsgen without multiple definition errors

Open Ryanf55 opened this issue 2 years ago • 6 comments

Bug report

Required Info:

  • Operating System: -Ubuntu 22
  • Installation type:
    • Humble binaries
  • Version or commit hash:
    • 3.1.5-2jammy.20230718.201832 amd64
  • DDS implementation:
    • Fast-DDS
  • Client library (if applicable):
    • N/A

Steps to reproduce issue

mkdir -p IDL/std_msgs/msg/
# Same for others
cp /opt/ros/humble/share/std_msgs/msg/Header.idl IDL/std_msgs/msg/
cp /opt/ros/humble/share/builtin_interfaces/msg/Time.idl IDL/builtin_interfaces/msg/
cp /opt/ros/humble/share/geographic_msgs/msg/GeoPoint.idl IDL/geographic_msgs/msg/
cp /opt/ros/humble/share/vision_msgs/msg/BoundingBox2D.idl IDL/vision_msgs/msg/
cp /opt/ros/humble/share/vision_msgs/msg/Pose2D.idl IDL/vision_msgs/msg/
cp /opt/ros/humble/share/vision_msgs/msg/Point2D.idl IDL/vision_msgs/msg/

fastddsgen -cs -replace -typeros2 -d gen/builtin_interfaces/msg -I IDL IDL/builtin_interfaces/msg/Time.idl
# Repeat for all the other files

OR, use ros2 run rosidl_adapter msg2idl.py /opt/ros/humble/share/std_msgs/msg/Header.msg to generate the IDL file into a directory if you want to work off the ROS messages, you get the same idl files.

Expected behavior

The generated IDL files do not have naming conflicts.

Actual behavior

[build] /ws/IDL/builtin_interfaces/msg/Time.idl:8:16: error: Illegal identifier: builtin_interfaces::msg::Time is already defined (Definition: com.eprosima.idl.parser.tree.TypeDeclaration@5b239d7d)

Illegal identifier: std_msgs::msg::Header is already defined (Definition: com.eprosima.idl.parser.tree.TypeDeclaration@515aebb0)

Additional information

Causes this downstream issue because the IDL file has conflicts.

https://github.com/eProsima/Fast-DDS-Gen/issues/52

Ryanf55 avatar Sep 08 '23 00:09 Ryanf55

Just so you know, we don't use fastddsgen when generating code from the IDL files; the ROS 2 core does this itself. So this isn't functionality that is usually used.

That said, if you find a way to make this work we'd be happy to review a fix.

clalancette avatar Sep 08 '23 11:09 clalancette

Just so you know, we don't use fastddsgen when generating code from the IDL files; the ROS 2 core does this itself. So this isn't functionality that is usually used.

That said, if you find a way to make this work we'd be happy to review a fix.

In a quick test, looks like a simple #pragma once at the top of every single ROS-generated IDL solved the problem on the EProsima side. I know from some research RTI supports it. Other vendors will need to be tested.

What's interesting is there are notes about this issue in the OMG IDL spec, but no proposed solution. #pragma once is not anywhere in the OMG IDL DDS spec, so I'm concerned it's not standard.

Thoughts?

Ryanf55 avatar Sep 08 '23 15:09 Ryanf55

In a quick test, looks like a simple #pragma once at the top of every single ROS-generated IDL solved the problem on the EProsima side. I know from some research RTI supports it. Other vendors will need to be tested.

What's interesting is there are notes about this issue in the OMG IDL spec, but no proposed solution. #pragma once is not anywhere in the OMG IDL DDS spec, so I'm concerned it's not standard.

Yes, that's a definite concern. So we'll have to see what happens with other vendors, particularly CycloneDDS and GurumDDS.

I'll note that without additional work in rosidl, adding this won't have any effect for ROS 2 itself. But if this helps others use fastddsgen and the like outside of ROS 2, then it is definitely something we can consider adding.

clalancette avatar Sep 11 '23 12:09 clalancette

To be even more specific, the OMG IDL spec says

IDL shall be preprocessed according to the specification of the preprocessor in ISO/IEC 14882:2003.

That ISO is the C++03 spec, which says that

[A #pragma directive] causes the implementation to behave in an implementation-defined manner. Any pragma that is not recognized by the implementation is ignored.

That all to say, a spec-compliant IDL implementation is not required to implement #pragma once - which is why I never get to use this handy little doodad in my C++ code, "just in case" :)

emersonknapp avatar Sep 13 '23 06:09 emersonknapp

To be even more specific, the OMG IDL spec says

IDL shall be preprocessed according to the specification of the preprocessor in ISO/IEC 14882:2003.

That ISO is the C++03 spec, which says that

[A #pragma directive] causes the implementation to behave in an implementation-defined manner. Any pragma that is not recognized by the implementation is ignored.

That all to say, a spec-compliant IDL implementation is not required to implement #pragma once - which is why I never get to use this handy little doodad in my C++ code, "just in case" :)

As far as I can tell, all the compilers for ROS tier 1 platforms support it.

Ryanf55 avatar Sep 13 '23 07:09 Ryanf55

Up to now, the issue has not been resolved. just like Ryanf55 said: add #pragma once at the top of every idl, can ignore the issue.

com-server-ap avatar Aug 01 '24 08:08 com-server-ap