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

Book remarks #2

Closed
jrutgeer opened this issue Oct 25, 2023 · 4 comments
Closed

Book remarks #2

jrutgeer opened this issue Oct 25, 2023 · 4 comments

Comments

@jrutgeer
Copy link

Thanks for the chapter on type adaptation.

Two small remarks wrt the book:

  • I think footnote 13 (in section 4.7.2 ) can be removed since the fix (Topic correct typeadapter deduction ros2/rclcpp#2294) was backported to Humble (and afaik type adaptation only exists since Humble).

  • Wrt. section 9.2.2 about 'named loggers' vs 'node loggers': imo. important to know: log messages using named loggers (i.e. rclcpp :: get_logger (" your_logger_name ")) are not published to /rosout, as opposed to log messages of node loggers.

@jrutgeer
Copy link
Author

Another remark about footnote 12:

It can be a good idea to define these
conversions directly within the same
ROS interface packages containing the
messages definitions.

This seems obvious, but it is not yet clear to me how to do this in practice:

  • The type adaptation code needs to include the custom message header files,
  • But the custom message headers are generated in the same package.

So you need some logic in the CMakeLists.txt to specify that custom messages must be processed first, and only then the type adaptation target compiled.

If you need inspiration for your next chapter: a chapter on Ament / CMake / making vendor packages for external libraries / etc would be nice... ;-)

@jrutgeer
Copy link
Author

Page 33, code example in section 4.4.2:

The code is incorrect. Line 5:

while(result.wait_for(1s) != rclcpp::FutureReturnCode::SUCCESS)
  • wait_for() returns an std::future_status
  • rclcpp::FutureReturnCode are the return codes of rclcpp::spin_until_future_complete()
  • There no == or != operator to compare these types.

I understand it is mentioned as being 'pseudo-code', yet this is confusing (it took me significant time to find out as I assumed something had changed from Humble to Iron or Rolling).

@MarcoMatteoBassa
Copy link
Contributor

MarcoMatteoBassa commented Nov 5, 2023

Hi @jrutgeer , thanks a lot for the very useful remarks and suggestions! I'm going through a busy time, but as soon as I can I will address them :)

For what concerns the type adapters compilation, what works for me is defining the adapters in an interface library after rosidl_generate_interfaces:

add_library(my_type_adapter INTERFACE)
target_include_directories(
  my_type_adapter
  INTERFACE $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/include>
            $<INSTALL_INTERFACE:include>)
ament_export_include_directories(include)
ament_export_targets(my_type_adapterTargets HAS_LIBRARY_TARGET)
install(DIRECTORY include/${PROJECT_NAME}/ DESTINATION include/${PROJECT_NAME})
install(TARGETS my_type_adapter EXPORT my_type_adapterTargets)

Let me know if this helps in your case :)

For the wait_for: sure, the correct thing to compare to is
std::future_status::ready
I probably messedup something copying the code to the book, sorry for the inconvenience!

@nirwester
Copy link
Owner

Fix published :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants