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

Add macros to link to interface docs #5050

Draft
wants to merge 1 commit into
base: rolling
Choose a base branch
from

Conversation

christophebedard
Copy link
Member

@christophebedard christophebedard commented Feb 18, 2025

This is currently more of a proof of concept, so it's a draft while I request feedback.

While reviewing #5034, I realized/remembered that we now have interface (e.g., message) docs in ROS 2 like we had in ROS 1:

Having this for ROS 2 is quite useful, and we can use it in many places instead of linking to the source files, like we currently do in this tutorial (and others).

However, I also realized that the path was a bit different depending on the distro, see #5034 (comment). While that is probably just a result of a different version of rosdoc2 being used for different distros at possibly different times (see rosdoc2/verbs/build/generate_interface_docs.py), I think we could abstract the link away from the documentation writer using macros similar to the ones we currently have.


This PR introduces two macros: (1) interface_link() and (2) interface(). They each take the interface name as a parameter and expand to the (1) the link to the interface doc page and (2) the interface name hyperlinked to its interface doc page.

Macro Example Becomes (for Rolling)*
{interface_link(std_msgs/msg/String)} See: {interface_link(std_msgs/msg/String)}. See: https://docs.ros.org/en/rolling/p/std_msgs/interfaces/msg/String.html.
{interface(std_msgs/msg/String)} Publish a {interface(std_msgs/msg/String)}. Publish a std_msgs/msg/String.

(*) formatted as markdown in this PR description for demonstration purposes, but the result is the same

interface() exists for simples links with the interface name only, while interface_link() could be used anywhere, including more complex RST links, such as:

Publish `a geometry_msgs/msg/Twist message <{interface_link(geometry_msgs/msg/Twist)}>`_.

See the changes to source/Tutorials/Intermediate/Publishing-Messages-Using-YAML-Files.rst for a concrete example.

This is valid for messages, services, and actions:

Notes:

  1. This is implemented using regexes that are slightly more complex than the existing macros, but I believe it's still quite manageable.
  2. The link template (interface_link_templ) could be customized for each distro like we do for the existing macros.
    • TBD, will check if we expect this to be fixed to be all the same for all non-EOL distros.
  3. Downsides/limitations:
    1. Due to an RST limitation, we cannot easily make the interface name both a hyperlink and a literal, although we could find a way around it, especially if we hide away the ugly details.
    2. On its own, this doesn't actually check if the generated URL is valid, but it could be combined with a URL validation tool.

Future work:

  1. Move all the macro Python code to a separate .py file
  2. Add tests?
  3. This could also be extended to link to package docs, but that one is less complex for documentation writers to do manually.

Signed-off-by: Christophe Bedard <[email protected]>
@christophebedard christophebedard self-assigned this Feb 18, 2025
Copy link

HTML artifacts: https://github.com/ros2/ros2_documentation/actions/runs/13402426595/artifacts/2612970313.

To view the resulting site:

  1. Click on the above link to download the artifacts archive
  2. Extract it
  3. Open html-artifacts-5050/index.html in your favorite browser

Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this is good improvement that user can see the message definition instead of leaving the some_pkg/types/message text 👍

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

Successfully merging this pull request may close these issues.

2 participants