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

Make sure that controllers are properly sorted and executed in a chain #853

Closed
destogl opened this issue Nov 11, 2022 · 4 comments
Closed

Comments

@destogl
Copy link
Member

destogl commented Nov 11, 2022

Is your feature request related to a problem? Please describe.

When loading controllers that are part of the chain, they have to be loaded in a certain order. Otherwise, there could be a lag in execution.

Describe the solution you'd like
Controllers are resorted in a proper order and calculations are correct.

Additional context

Check doc about controllers' chaining here: https://github.com/ros-controls/roadmap/blob/08d1454cd0d75e4f88cf16a05dad824c69caa302/design_drafts/cascade_control.md

  1. Start writing a test for it that shows wrong calculation of controller, i.e., in the current state the test should fail.
  2. Adjust controller manager when controller is getting loaded so that controllers are reordered in the controller's lists:
@destogl destogl changed the title Make sure that controllers are properly sorted in a chain Make sure that controllers are properly sorted and executed in a chain Nov 11, 2022
@sgmurray
Copy link
Contributor

I will work on this.

@saikishor
Copy link
Member

Hello @destogl and @sgmurray!

I would like to contribute to this. Shall I continue based on yours or shall I create a new one?
I would like to have the controllers with only state interface update first and then one of the chainable controllers and then comes the controllers with command interfaces. What do you think?.

Best Regards,
Sai

@bmagyar
Copy link
Member

bmagyar commented Jun 19, 2023

Please do! I think it's a sensible approach.
It is up to you whether to pick up the existing work or not. Please don't forget adding tests ;)

@saikishor
Copy link
Member

Please do! I think it's a sensible approach. It is up to you whether to pick up the existing work or not. Please don't forget adding tests ;)

Sure!. Thank you!

saikishor added a commit to saikishor/ros2_control that referenced this issue Nov 6, 2023
…s#853) (ros-controls#1063)

* Added test for the reordering controllers case

* Perform controller sorting at the end of switch_controller

* fix the list_chained_controllers_srv test case for the new controller sorting

* move the logic to a separate function

* Apply suggestions from code review

Co-authored-by: Christoph Fröhlich <christophfroehlich@users.noreply.github.com>

* remove obsolete TODO comments in the controller chaining tests

* Add a test case to sort 6 controllers in chained mode

* Added a method to retrieve the following controller names given a controller name

* Update the controller_sorting method to support progressive chaining ability

* Added test case to sort chained controllers with branching

* Added a method to retrieve the preceding controller names given a controller name

* Added logic to controller_sorting to support sorting branched controller chains

* Added some documentation to the newly added functions

* Add debug print of reordered controllers list once they are sorted

* Add the condition to skip the non-configured controllers

* remove logging for every command interface

* Improve the complex chain test case checking

* added a test case to sort independent chains

* Added fixes for the independent chains sorting

* better randomization in independent chains testing

* Fix minor logic issues

* Add 3rd chain for better complex testing

* Apply suggestions from code review - Denis

Co-authored-by: Dr. Denis <denis@stoglrobotics.de>

* address pull request review comments

---------

Co-authored-by: Christoph Fröhlich <christophfroehlich@users.noreply.github.com>
Co-authored-by: Dr. Denis <denis@stoglrobotics.de>
Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants