Skip to content

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.

@christophfroehlich christophfroehlich force-pushed the autodocs_parameters_nested branch from 9a3306c to 0d7bd68 Compare December 28, 2023 13:22
@codecov
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 force-pushed the autodocs_parameters_nested branch from fda15e6 to 7aa7b3c Compare January 9, 2024 15:14
@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
@mergify
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
Contributor

@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.

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
Contributor

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 ;)

Co-authored-by: Felix Exner (fexner) <[email protected]>
@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?

Felix Exner and others added 2 commits January 21, 2024 20:08
[JTC docs] Clarify what "joints and interfaces have to be used in com…
@mergify
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
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 Triggers PR backport to ROS 2 humble.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants