Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Weird cmake dependencies on rosidl_typesupport_introspection_c, rosidl_typesupport_introspection_cpp #159

Closed
jwang11 opened this issue Oct 19, 2017 · 7 comments
Labels
question Further information is requested

Comments

@jwang11
Copy link
Contributor

jwang11 commented Oct 19, 2017

In rmw_fastrtps_cpp/CMakeLists.txt, I see some weird dependencies as below. I don't see the reason that rmw implementation depend on rosidl_xxx packages. if or not I misunderstand sth?
ament_target_dependencies(rmw_fastrtps_cpp
"rcutils"
"rosidl_typesupport_introspection_c"
"rosidl_typesupport_introspection_cpp"
"rmw"
"rosidl_generator_c"
"rosidl_generator_cpp")

@dhood dhood added the question Further information is requested label Oct 19, 2017
@dhood
Copy link
Member

dhood commented Oct 19, 2017

ROSIDL is the message format of ROS messages, and the rosidl_* packages contain common code for working with those messages that is also used by other rmw implementations. For example the typesupport_introspection packages are also used by rmw_connext_dynamic_cpp as it's another rmw implementation that supports runtime interpretation of ROS msgs via "introspection" using msg metadata, as opposed to relying on pre-built support for each msg type.

Have a read of http://docs.ros2.org/beta3/developer_overview.html#internal-ros-interfaces which goes into more detail about typesupport and the roles the packages, poke around the codebase to see how the packages relate to each other, and feel free to post back with a more specific question if something's still unclear

@dhood dhood added the more-information-needed Further information is required label Oct 19, 2017
@jwang11
Copy link
Contributor Author

jwang11 commented Oct 19, 2017

Thank you for explanation. I basically understand what rosidl_XXX do, however, rmw_fastrpts not use .msg or .svr, thus no need to generate c/cpp code for them. I tried to drop those rosidl_xxx, there is no compile issue.

@dhood
Copy link
Member

dhood commented Oct 19, 2017

Things from rosidl_generator_c are used during serialisation/deserialisation:

rosidl_generator_c__String__assign(c_str, str.c_str());

so if you got it to build you maybe weren't using a clean, isolated workspace.

For rosidl_generator_cpp I'm not sure where the dependency comes from, it might be safe to remove it

@dhood dhood removed the more-information-needed Further information is required label Oct 19, 2017
@jwang11
Copy link
Contributor Author

jwang11 commented Oct 19, 2017

I searched the code, rosidl_generator_cpp did not used, however rosidl_typesupport_introspection_cpp was widely used. Is it the normal? should not they appear in pair?

@dhood
Copy link
Member

dhood commented Oct 20, 2017

It depends. rmw_fastrtps_cpp needs to know about the c data type that's being used for strings (what I linked to before) which is why it depends on rosidl_generator_c. For c++, it doesn't need to depend on rosidl_generator_cpp to get any info about the string data type because it's just an std::string (if it were a custom type then it would).

as far as I know there isn't any reason for the rosidl_generator_cpp dependency. I proposed its removal in #161

@mikaelarguedas
Copy link
Member

I tried to drop those rosidl_xxx, there is no compile issue.

Quick follow up on this. The reason it succeeds to build when removing rosidl_generator_c is because its headers are exported by rosidl_typesupport_introspection_c.
Though the dependency on rosidl_generator_c should be kept because we use it's headers and symbols directly in this package as pointed out by @dhood

@jwang11
Copy link
Contributor Author

jwang11 commented Oct 20, 2017

That is fair. thanks

@dhood dhood closed this as completed Oct 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants