Skip to content

Add tests for multiple controller ros args #2155

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

Conversation

saikishor
Copy link
Member

Added tests for multiple --controller-ros-args and also test parsing params and also check that we are able to parse the parameters in the same way

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.

thx <3

"ctrl_2 -c test_controller_manager --controller-ros-args '-r /ctrl_2/set_bool:=/set_bool'"),
call_spawner("ctrl_2 -c test_controller_manager --controller-ros-args '-r "
"/ctrl_2/set_bool:=/set_bool -p run_cycle:=20' --controller-ros-args '-r "
"/ctrl_1/set_value:=/set_value'"),
Copy link
Contributor

Choose a reason for hiding this comment

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

you want to check for the /set_value as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really. There is nothing related. This is just to test that it doesn't break at spawner level

Copy link
Member Author

Choose a reason for hiding this comment

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

I can move the parameter part to the second part. May be that's more robust way

Copy link
Contributor

Choose a reason for hiding this comment

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

I see 3 --ros-args, two remap rules and one parameter. Why just don't check for all the three of them?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because the second remapping doesn't exist. I just added casually. May be I can remove it

Copy link
Contributor

Choose a reason for hiding this comment

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

ok sorry, I haven't checked that carefully. If we want to support space-separated rules with a single --controller-ros-args, we should somewhen also test it.

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.

Perfect!

@christophfroehlich christophfroehlich merged commit 7fba025 into ros-controls:master Apr 1, 2025
14 of 24 checks passed
@christophfroehlich christophfroehlich deleted the add/tests/spawner/multi_controller_ros_args branch April 1, 2025 19:15
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.

2 participants