strands-project / mongodb_store

MongoDB tools for storing and analysing runs of ROS systems.
BSD 3-Clause "New" or "Revised" License
49 stars 73 forks source link

Changing launch file to adjust to new machine tag type #107

Closed Jailander closed 10 years ago

marc-hanheide commented 10 years ago

@hawesie before we merge this, can you approve the new way we use machine tags? As discussed in length in https://github.com/strands-project/strands_systems/issues/74

Jailander commented 10 years ago

@hawesie have you had time to check the new machine tag format?

hawesie commented 10 years ago

Yes, and I approve. I actually thought I'd merged this, but clearly didn't. Please rebase on hydro-devel and then I will merge.

marc-hanheide commented 10 years ago

may I suggest one last possible improvement? If you'd use optenv rather than env with the default set to $(env ROS_ROOT)/../../env.sh then one would only have to define the env variable IF any special setting is needed. With the released stuff, it would work out of the box... Good or stupid? see http://wiki.ros.org/roslaunch/XML#substitution_args

Jailander commented 10 years ago

just to be sure you are suggesting this $(optenv ROS_ENV_LOADER /opt/ros/hydro/env.sh), remember that ROS_ROOT points to /opt/ros/hydro/share/ros, otherwise I could do that no problem, I think is logical.

marc-hanheide commented 10 years ago

no, I meant $(optenv ROS_ENV_LOADER $(env ROS_ROOT)/../../env.sh) (i.e. go two directories up from $ROS_ROOT to find the env.sh script). The version you have now again requires a different solution for indigo and any coming ROS version to be maintained. We want the source to be as distribution-independent as possible. If my solution about doesn't work, then forget about it, but don't put anything distribution-specific in here, please.

marc-hanheide commented 10 years ago

also, for future reference, a git rebase is better to pull in the upstream changes in these cases as it "stacks" your own changes to the end (i.e. makes them the last commits based on the upstream). Much easier to maintain and manage.

Jailander commented 10 years ago

ah ok got it, sorry I'll fix it now

marc-hanheide commented 10 years ago

no worries ;-)

Jailander commented 10 years ago

Hi @marc-hanheide this is not possible since substitution arguments cannot go inside substitution arguments, I also tried setting ROS_ENV_LOADER using the <env> tag when ROS_ENV_LOADER was not set, but sadly this is not possible either as the $(env ) substitution arguments doesn't pick environment variables set using <env> tag. Any idea?

For now I will leave it as it was originally and afterwards I'll fix it, when I find a suitable solution