Skip to content

Add example of starting controllers with launch files#273

Closed
christophfroehlich wants to merge 1 commit intoros-controls:masterfrom
christophfroehlich:extend_example1
Closed

Add example of starting controllers with launch files#273
christophfroehlich wants to merge 1 commit intoros-controls:masterfrom
christophfroehlich:extend_example1

Conversation

@christophfroehlich
Copy link
Copy Markdown
Member

Rewrite of #66

Co-authored-by: Victor Lopez <victor.lopez@pal-robotics.com>
@destogl destogl force-pushed the extend_example1 branch from 87a0f42 to b9a0c76 Compare May 1, 2023 20:18
Copy link
Copy Markdown
Member

@destogl destogl left a comment

Choose a reason for hiding this comment

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

I like the docs extension and showing how controllers are managed. Nevertheless, I am not convinced that we need this launch file. We have great CLI options, and they can do everything required, so what is the added value into having this launch file.

@v-lopez if you are still there, please tell us.

Still, @bmagyar and @christophfroehlich, what do you think about his additional option? Is there something I don't see?

Comment thread example_1/doc/userdoc.rst
ros2 control load_controller joint_trajectory_position_controller

.. code-block:: shell
what should return ``Successfully loaded controller joint_trajectory_position_controller``. Check the status
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
what should return ``Successfully loaded controller joint_trajectory_position_controller``. Check the status
what should return ``Successfully loaded controller joint_trajectory_position_controller``.
Then check controllers' status

Comment thread example_1/doc/userdoc.rst
ros2 control list_controllers

.. code-block:: shell
what shows you that the controller is loaded but unconfigured.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
what shows you that the controller is loaded but unconfigured.
what shows you that the controller is loaded but unconfigured. Similar output is expected:

@christophfroehlich
Copy link
Copy Markdown
Member Author

christophfroehlich commented May 2, 2023

The diff view is broken here, this PR actually only adds literal c in line 167. The rest is just restructuring of the section.

I found the old PR interesting and merged it to the new structure. I thought it could be helpful for switching controller from some external interface like a GUI, or probably shows how to integrate controller_manager.launch_utils to a python state machine script. Due to fact that it only loads the controller but does not activate it, I agree that the benefit of this launch file is close to zero.

Copy link
Copy Markdown
Collaborator

@olivier-stasse olivier-stasse left a comment

Choose a reason for hiding this comment

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

For a first example I believe that the most simple example is preferable.
We can give a try but I think most of the feedback we had was that simpler is better.

@christophfroehlich
Copy link
Copy Markdown
Member Author

I don't have any hard feelings about this PR. If you think this is not adding any value to the demos but only making them more verbose -> I'll close it.

@christophfroehlich christophfroehlich deleted the extend_example1 branch July 1, 2023 11:53
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.

3 participants