This works better if you have #309 to detect when a command failed.

And also ros2/ros2cli#590 for the param load functionality.

Request for 2 small changes.

Were spawner and unspawner nodes in ros1. Does it make sense to have them as nodes or just as scripts as you implemented?

That's up for debate. I started as a node, but I would need to duplicate much code from ros2controlcli and from the ros2param load. I opened ros2/rclpy#671 and went to work as a script to above duplication.

If we want to add more features like listening to a topic as in ROS1, it makes more sense as a node, hopefully there's a way of avoiding duplicating everything.

I'd say go with the current version so we have something, and later when we can avoid code duplication, we rewrite it to use services directly.

One thing we could do now as a middleground solution, is implement it as a Node that calls the subprocess.
The reasoning being that if we want to write launch files using it, right now they would be written with ExecuteProcess Action and if it was a Node with the Node Action..

If we want launch files to remain unchanged, we should use a Node right now, even if it's implemented as a script.

I'd say go with the current version so we have something, and later when we can avoid code duplication, we rewrite it to use services directly.

OK, if you correct flake8 I agree :)

Damn it, yeah I will.

Let me change it to node first (albeit using subprocess), I'll do it Friday morning.

I will, sorry about that.

Do you know why my colcon test and ament_flake8 won't detect that stuff?

$ dpkg -l | grep flake8
ii  ros-foxy-ament-flake8                           0.9.6-1focal.20201210.043943         amd64        The ability to check code for style and syntax conventions with flake8.

$ ament_flake8  
2 files checked
No problems found

Checked files:

* ./spawner.py
* ./unspawner.py

I'm happy to merge this if you pinky promise a follow-up PR with some tests?

I will add the tests, but we may want to hold the merge until ros2/ros2cli#590 is passed. Otherwise the part about loading parameters is going to fail because ros2 param load will not exist.

Is that going into Foxy or only Galactic?

I'm gonna request for a cherry-pick to foxy since it's 100% backwards compatible and useful.

I added the tests but they seem to fail on the buildfarm, I'll check them next week.

Could you add an example of using spawners in launch files? Maybe on ros2_control_demos repo?

I'll add the examples.

Do you have any idea why is it failing?
I see some "No executable found" messages as if the spawner.py was not in the ros2 run path, but if I run the tests with colcon test it works for me.

Also I expect that they will fail until ros2/ros2cli#596 is accepted, but they should fail differently complaining with:

usage: ros2 param [-h] Call ros2 param <command> -h for more detailed usage. ...
ros2 param: error: argument Call ros2 param <command> -h for more detailed usage.: invalid choice: 'load' (choose from 'delete', 'describe', 'dump', 'get', 'list', 'set')

@destogl @bmagyar to add the ros2_control_demo example I need to solve #320 first. To put the reusable code there. It's 70 lines of python launch nonsense that I don't want to replicate.

@destogl @bmagyar to add the ros2_control_demo example I need to solve #320 first. To put the reusable code there. It's 70 lines of python launch nonsense that I don't want to replicate.

fully agree with you!

@Karsten1987 This is the PR we were talking about. Waiting on ros2cli to be released.

I have rebased the branch to resolve the conflict. We are still blocked waiting for a release of ros2/ros2cli#596 , ros2cli to Foxy.

Asked for a release here: ros2/ros2cli#612