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

Create doc file for chained controllers #985

Merged
merged 6 commits into from
Jul 2, 2023

Conversation

destogl
Copy link
Member

@destogl destogl commented Apr 10, 2023

No description provided.

@destogl destogl self-assigned this Apr 10, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #985 (c2e5006) into master (925f5f3) will decrease coverage by 1.93%.
The diff coverage is 35.61%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master     #985      +/-   ##
==========================================
- Coverage   34.61%   32.69%   -1.93%     
==========================================
  Files          52       91      +39     
  Lines        2981     9551    +6570     
  Branches     1855     6432    +4577     
==========================================
+ Hits         1032     3123    +2091     
- Misses        310      704     +394     
- Partials     1639     5724    +4085     
Flag Coverage Δ
unittests 32.69% <35.61%> (-1.93%) ⬇️

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

Impacted Files Coverage Δ
controller_manager/src/controller_manager.cpp 38.27% <ø> (-1.44%) ⬇️
controller_manager/src/ros2_control_node.cpp 0.00% <0.00%> (ø)
..._interface/include/hardware_interface/actuator.hpp 100.00% <ø> (ø)
...re_interface/include/hardware_interface/sensor.hpp 100.00% <ø> (ø)
...re_interface/include/hardware_interface/system.hpp 100.00% <ø> (ø)
hardware_interface/src/resource_manager.cpp 49.91% <ø> (-3.72%) ⬇️
hardware_interface/src/sensor.cpp 50.52% <ø> (ø)
hardware_interface/src/system.cpp 55.45% <ø> (ø)
...rface/test/mock_components/test_generic_system.cpp 9.14% <ø> (ø)
...dware_interface/test/test_component_interfaces.cpp 32.44% <ø> (+4.25%) ⬆️
... and 68 more

... and 19 files with indirect coverage changes

@bmagyar bmagyar changed the title Create doc fiel for chained controllers Create doc field for chained controllers Apr 11, 2023
@destogl destogl changed the title Create doc field for chained controllers Create doc file for chained controllers Apr 11, 2023
@bmagyar
Copy link
Member

bmagyar commented Jun 12, 2023

Are you happy with me merging this? People want some docs, this is better than nothing!

controller_manager/doc/controller_chaining.rst Outdated Show resolved Hide resolved
controller_manager/doc/controller_chaining.rst Outdated Show resolved Hide resolved
controller_manager/doc/controller_chaining.rst Outdated Show resolved Hide resolved
controller_manager/doc/controller_chaining.rst Outdated Show resolved Hide resolved
controller_manager/doc/controller_chaining.rst Outdated Show resolved Hide resolved
controller_manager/doc/controller_chaining.rst Outdated Show resolved Hide resolved
controller_manager/doc/controller_chaining.rst Outdated Show resolved Hide resolved
controller_manager/doc/controller_chaining.rst Outdated Show resolved Hide resolved
controller_manager/doc/controller_chaining.rst Outdated Show resolved Hide resolved
controller_manager/doc/controller_chaining.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

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

I hope this gets merged soon ;)

At least a proper rst markup + pictures and link the file to the docs is needed. (Silently advertising ros-controls/control.ros.org#125).
Or: Push this file as .md file where the suggested changes are accepted, and I could do the translation to rst then.

controller_manager/doc/controller_chaining.rst Outdated Show resolved Hide resolved
@bmagyar
Copy link
Member

bmagyar commented Jun 30, 2023

@christophfroehlich could we try the docs on this PR?

@christophfroehlich
Copy link
Contributor

this is no valid rst syntax. i'll make an PR against this PR with the changes this weekend

@bmagyar
Copy link
Member

bmagyar commented Jun 30, 2023

Ah yeah I'm aware of that :D was just wondering how far it is. But agree that this should be merged asap

@christophfroehlich
Copy link
Contributor

I refactored it with PR #1068

christophfroehlich and others added 2 commits July 1, 2023 13:09
* Refactor doc for rst syntax

* Add chaining controllers to toc

* fix typo

---------

Co-authored-by: Bence Magyar <[email protected]>
@bmagyar bmagyar marked this pull request as ready for review July 1, 2023 16:58
@bmagyar bmagyar merged commit 185fbea into master Jul 2, 2023
1 of 2 checks passed
@christophfroehlich
Copy link
Contributor

christophfroehlich commented Sep 6, 2023

@bmagyar this hasn't been backported to humble, which caused the conflict with #1106. The image and the link in doc/index.rst is missing, see this error.

@christophfroehlich
Copy link
Contributor

@destogl can this be backported to humble to fix the build error with the docs for humble?

@christophfroehlich christophfroehlich added the backport-humble This label should be used by maintainers only! Label triggers PR backport to ROS2 humble. label Oct 10, 2023
mergify bot pushed a commit that referenced this pull request Oct 10, 2023
Co-authored-by: Bence Magyar <[email protected]>
Co-authored-by: Christoph Fröhlich <[email protected]>
(cherry picked from commit 185fbea)

# Conflicts:
#	controller_manager/doc/controller_chaining.rst
@destogl destogl deleted the docs-about-chained-controllers branch October 15, 2023 12:34
@destogl
Copy link
Member Author

destogl commented Oct 15, 2023

We have to fix this. The docs is very wrong and misleading in few places...

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 maintainers only! Label triggers PR backport to ROS2 humble.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants