remove respawn in fetch_bringup/launch/include/teleop.launch.xml

[knorth55]

We want to remove respawn param for some teleop nodes

[cjds]

@knorth55 could you tell us why?

[moriarty]

I'm not sure why we would want to disable the respawn=true on this for all Fetch robots.

If @knorth55 can give us a compelling reason, then maybe we can make it an optional thing that some users could disable if they want to.

[knorth55]

other nodes related to teleop are running as respawn=False, but these two are running with respawn=True.
I want to set the same respawn parameter (respawn=False) to all teleop nodes.
But another option is that I can modify this PR to give <args name="respawn" default="False"> to
teleop launch.

[knorth55]

@moriarty kindly ping. could you give me some feedback?

[erelson]

@knorth55 your above explanation still doesn't describe why this change should be made. What undesired effect is the respawn parameter having?

Having respawn be an arg that defaults to true, would probably be our preference.

I changed the PR to merge into melodic-devel, so you will need to rebase or cherry-pick your commit accordingly. I don't think it makes sense to update indigo-devel at this point.

[knorth55]

@erelson We want to set same value for respawn parameter because we want to run same nodes by another launch.
Now those two nodes are set as respawn:=true so that only those two nodes are trying to respawn.
But other nodes are set as respawn:=false, so respawned two nodes mean nothing.

[knorth55]

Also, we are using fetch in Indigo, so I want to backport this PR to indigo branch, too.
We want to use melodic in fetch, but we are still waiting for release of some ros packages which are not supported for melodic.

[erelson]

It sounds like you want the teleop node to not respawn if it exits on startup due to the existence of the same node already. Since the teleop node is launched via /etc/ros/$distro/robot.launch, for your usecase would it make sense to comment out the line in that file? (This file is intended to be user-modified, and doesn't get overwritten by updates)

Also, note that there will not be any further releases of fetch ROS debians for ROS Indigo.

[knorth55]

My use case is like this.

1. launch fetch_bringup/launch/fetch.launch in /etc/ros/indigo/robot.launch
2. run same nodes in other launch

In this case I want to set respawn=false in fetch_bringup/launch/fetch.launch

I mean there is no reason to set respawn:=true for only those two nodes.
I want to set same respawn value parameter for this file.
I thought you want to set respawn=false to all nodes, so I modify to remove it,
but if you want to set respawn=true to all nodes, please tell me.
I can set <arg name="respawn" default="true"> to all nodes in fetch_bringup/launch/fetch.launch

Indigo is EOL, so I need to switch to use this package from source :(
Well, we have to release all our package in melodic quickly...

[moriarty]

The two nodes which are re-spawned are the topic_tools mux, and the tuck arm script.

This is odd. I would have expected us to respawn the core required components... But I can't think of why we would respawn only the tuck_arm_script unless it's known to crash frequently, or the topic mux.

[cjds]

The topic mux should not respawn. In fact it should probably be listed as required. Tuck arm should also not respawn. It doesn't crash

[moriarty]

@cjds yes I think if we wanted to enforce some nodes were always running the better way would be to use required.

If we were to change these all to respawn=true that would be a bigger change.

I think we can accept his PR.

[moriarty]

Seems Reasonable

[erelson]

I'm fine with merging at this point. It looks like the respawns were not added in response to issues with the nodes crashing, so the consensus is they're safe to remove.

launch fetch_bringup/launch/fetch.launch in /etc/ros/indigo/robot.launch

This may be a mis-statement, as it doesn't make sense to me? robot.launch is initially a copy of fetch.launch, with (minimal) modifications such as pointing to a calibrated URDF.

[knorth55]

launch fetch_bringup/launch/fetch.launch in /etc/ros/indigo/robot.launch
This may be a mis-statement, as it doesn't make sense to me? robot.launch is initially a copy of fetch.launch, with (minimal) modifications such as pointing to a calibrated URDF.

Yes, you are right.
correct statement is teleop.launch.xml is included in /etc/ros/indigo/robot.launch