strands-project / strands_systems

all system configs
0 stars 16 forks source link

Make robot-independent launch files #74

Closed hawesie closed 9 years ago

hawesie commented 10 years ago

We should remove the strands_ROBOT directories from this repo and replace them with a set of launch files which are site independent. Local configuration should be stored in a site-specific repo, and should ideally be done via environment variables. @marc-hanheide volunteered to push this. It would be great if it could be done soon so that @ferdianjovan can get the NHM running on a generic way this week.

marc-hanheide commented 9 years ago

we also need to get rid of the site/machine-specifics in the launch files in general (across repos). @Jailander was to look into this and then we can also get rid of the strands_ROBOT things or at least put them into some very-site specific repo. I agree they have to be removed from this. With regard to the machine tags, I suggest to take inspiration from PR2's solution once again. I reassign this to @Jailander as he volunteered to have a look at this.

Currently, we always have lines like this in our launch files:

<machine name="$(arg machine)" 
  address="$(arg machine)" 
  env-loader="/opt/strands/strands_catkin_ws/devel/env.sh" 
  user="$(arg user)" default="true"
/>

It is suggest to use this instead:

<machine name="$(arg machine)" address="$(arg machine)" 
  ros-root="$(env ROS_ROOT)" 
  ros-package-path="$(env ROS_PACKAGE_PATH)" 
  default="true" 
/>

Then the same setting will be used on the remote hosts than on the launching one. Getting rid of the hard-coded path is quite urgent, indeed. To make live easy and search for all of these, @Jailander might want to use the automatically created https://lcas.lincoln.ac.uk/jenkins/job/generate_report/lastSuccessfulBuild/artifact/rosinstall.yaml file to checkout the whole lot of STRANDS packages and do a search and replace in all the launch files needed. But also test the proposed solution first, obviously.

I'm including @cburbridge @hawesie @cdondrup @lucasb-eyer and @ToMadoRe in this discussion.

lucasb-eyer commented 9 years ago

Maybe noob question.

I've read the PR2 wiki you linked, but I'm still not sure I understand the value of the <machine> tag in our context. Right now, we're all very heterogenous, afaik many haven't even setup both side-pcs. So I think different sites will want to run different things on different locations.

Why are site-and-scenario-specific tmux startup scripts setting ROS_MASTER_URI + ROS_HOSTNAME and running various roslaunchs on the pcs not enough? (Or even better since we can start/stop CTRL+C launchfiles independently?) Shouldn't the launchfiles themselves not contain any machine-setup but just start nodes with parameters?

Jailander commented 9 years ago

I don't understand what would be the advantage of that, right now the machine tags are not global for all the robots you can set up your own machine names as they are. The machine tags as we use them right now are parameters that can be set on your local launch files not network settings.

The advantage of this is that you can launch nodes on different computers from the same computer on the same launch file without setting ROS_MASTER_URI + ROS_HOSTNAME, so you can have launch files per application like starting the robot or launching the navigation system instead of one launch file per package that has to be launch ssh into the right machine and setting the environment.

As soon as we get Linda running again I'll fix this issue.

lucasb-eyer commented 9 years ago

Ok, I see thanks.

Jailander commented 9 years ago

No problem, I will try to set it up this time so it is easy to set all the machine tags in just one step

marc-hanheide commented 9 years ago

actually, looking at http://wiki.ros.org/roslaunch/XML/machine my proposed solution doesn't work in post-electric ROS and indeed we need to use env-loader as before. But we can still use the $ROS_ROOT variable to make it general. And then we should in my view only include the machine tag IF an arg explicitly asks for it, i.e. include if="$(arg remote)" in the <machine> tag, with 'remote' being 'false' by default. In addition we should get rid of any 'machine' attributes in the 'specifications but rather usedefaultin thetags. I think it's a reasonable decision to launch the whole of a launch file on one node. Otherwise, use`. So, for example (not tested!):

<launch>
    <arg name="machine" default="localhost" />
    <arg name="user" default="" />
    <arg name="remote" default="false"/>

    <!-- now has an IF checking for the arg "remote" -->
    <machine name="$(arg machine)" address="$(arg machine)" env-loader="$(env ROS_ROOT)/env.sh" user="$(arg user)" default="true" if="$(arg remote)" />
    <!-- NO machine attribute here! -->
    <node pkg="whatever" type="whatever" name="whatever" output="screen">
</launch>

What do people think? Then by default it wouldn't use ssh at all which is what we want/need. Just using localhost by default still requires people to first set up shared-key ssh for localhost, so we want to avoid that and just make running remotely optionally.

Jailander commented 9 years ago

Ok that seems like a good idea I'll do that :+1:

marc-hanheide commented 9 years ago

last(?) correction: env-loader="$(env ROS_ENV_LOADER)" (taken from https://github.com/PR2/pr2_common/blob/hydro-devel/pr2_machine/pr2.machine)

Jailander commented 9 years ago

One comment for this to work you have to set ROS_ENV_LOADER for now I added export ROS_ENV_LOADER=/opt/ros/hydro/env.sh in Linda's .bashrc but I am not sure if this is the desired solution @marc-hanheide

(taken form http://wiki.ros.org/pr2_robot/Tutorials/Setting%20up%20the%20ROS_ENV_LOADER)

hawesie commented 9 years ago

Do we need the separate remote arg in @marc-hanheide's solution? Won't the machine arg enough to check somehow?

hawesie commented 9 years ago

I'm on my phone so can't actually sanity check that!

Jailander commented 9 years ago

yes you actually need a variable set to true for the if to work, but in this solution you only need to set it if it is going to be launched remotely

marc-hanheide commented 9 years ago

this is achieved