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

Let sphinx add parameter description with nested structures to documentation #652

Merged

Conversation

christophfroehlich
Copy link
Contributor

@christophfroehlich christophfroehlich commented Jun 5, 2023

This adds automatic documentation generation from generate_parameter_library yamls, where nested structures or __map_ parameters are used and completes #651

Binary builds will fail until generate_parameter_library is released: the multiline description generates invalid C++ code.

Copy link

codecov bot commented Dec 28, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9f7e9e9) 47.71% compared to head (c0b8a83) 47.81%.
Report is 6 commits behind head on master.

❗ Current head c0b8a83 differs from pull request most recent head 62b6c05. Consider uploading reports for the commit 62b6c05 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #652      +/-   ##
==========================================
+ Coverage   47.71%   47.81%   +0.10%     
==========================================
  Files          41       41              
  Lines        3871     3871              
  Branches     1833     1827       -6     
==========================================
+ Hits         1847     1851       +4     
  Misses        751      751              
+ Partials     1273     1269       -4     
Flag Coverage Δ
unittests 47.81% <ø> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 4 files with indirect coverage changes

@christophfroehlich christophfroehlich added backport-humble This label should be used by maintaines only! Label triggers PR backport to ROS2 humble. backport-iron This label should be used by maintaines only! Label triggers PR backport to ROS2 Iron. labels Dec 28, 2023
@christophfroehlich christophfroehlich added the hold Holding off merging this PR until some condition label Jan 10, 2024
@christophfroehlich christophfroehlich marked this pull request as ready for review January 10, 2024 10:44
Copy link
Contributor

mergify bot commented Jan 11, 2024

This pull request is in conflict. Could you fix it @christophfroehlich?

@bmagyar bmagyar removed the hold Holding off merging this PR until some condition label Jan 12, 2024
Copy link

@fmauch fmauch left a comment

Choose a reason for hiding this comment

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

I checked that all links point to the correct files and briefly went over the texts. I left a couple of comments on the way that in my opinion improve readability, but since I'm not a native speaker I wouldn't consider them required, hence approving.

force_torque_sensor_broadcaster/doc/userdoc.rst Outdated Show resolved Hide resolved
Comment on lines 22 to 23
description: "Parameter to support broadcasting of only specific joints and interfaces.
It has to be used in combination with the ``joints`` parameter."
Copy link

Choose a reason for hiding this comment

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

To be honest (maybe out of scope), I think having an example on what "has to be used in combination with" would be beneficial. Especially in terms of different scenarios to using map_interface_to_joint_state.

Also, explaining what leaving the lists empty means would be beneficial in my opinion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you have a text suggestion: feel free to add one. In the meantime, I opened an issue from that ;)

joint_trajectory_controller/doc/parameters_context.yaml Outdated Show resolved Hide resolved
Co-authored-by: Felix Exner (fexner) <felix_mauch@web.de>
@christophfroehlich
Copy link
Contributor Author

I checked that all links point to the correct files and briefly went over the texts. I left a couple of comments on the way that in my opinion improve readability, but since I'm not a native speaker I wouldn't consider them required, hence approving.

Thanks for the proof reading. I basically merged the infos from the documentation files with them of the yaml -> adding new information is actually out of scope -> I opened an issue for that.

@bmagyar can we merge and release that with the same sync than 3.7.0 of generate_parameter_library? I guess we should wait for the next one?

fmauch and others added 2 commits January 21, 2024 20:08
[JTC docs] Clarify what "joints and interfaces have to be used in com…
Copy link
Contributor

mergify bot commented Jan 25, 2024

This pull request is in conflict. Could you fix it @christophfroehlich?

@bmagyar bmagyar merged commit f375c69 into ros-controls:master Jan 31, 2024
11 of 15 checks passed
mergify bot pushed a commit that referenced this pull request Jan 31, 2024
…ntation (#652)

(cherry picked from commit f375c69)

# Conflicts:
#	joint_trajectory_controller/doc/parameters.rst
mergify bot pushed a commit that referenced this pull request Jan 31, 2024
@christophfroehlich christophfroehlich deleted the autodocs_parameters_nested branch January 31, 2024 21:55
bmagyar pushed a commit that referenced this pull request Feb 1, 2024
pac48 pushed a commit to pac48/ros2_controllers that referenced this pull request Feb 2, 2024
bmagyar pushed a commit that referenced this pull request Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-humble This label should be used by maintaines only! Label triggers PR backport to ROS2 humble. backport-iron This label should be used by maintaines only! Label triggers PR backport to ROS2 Iron.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants