strands-project / mongodb_store

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

add switch option to use machine tag #119

Closed furushchev closed 9 years ago

furushchev commented 9 years ago

I'd like to choose to use machine tag or not. If this is included by another launch file and it also has machine tag ( 2 machine tag ), launch fails.

strands-jenkins commented 9 years ago

Can one of the STRANDS admins verify this patch? Post a comment containing "ok to test" to enable Jenkins builds for this pull request.

marc-hanheide commented 9 years ago

The handling of machine tags is indeed nasty in ROS (or we didn't find the correct solution yet). Yes, you're right, in general, we should probably not have the machine tags in the usual launch files at all. @hawesie?

marc-hanheide commented 9 years ago

ok to test

Jailander commented 9 years ago

I don't know which problem was @furushchev having, if he was using another launch file with machine tags it could have failed because he defined the same machine diferently twice, e.g, defining localhost with different users. This being said I like this solution, however, I don't think that removing machine tags in the launch files is viable, instead we can have strands launch files and general launch files without machine tags, does this sound logical? @marc-hanheide @hawesie

marc-hanheide commented 9 years ago

@Jailander yes, that's what I kind of meant... We could have a "wrapper" launch file for our purposes, but there should also be a "vanilla" one.

Jailander commented 9 years ago

Ok just making sure ;-)

furushchev commented 9 years ago

So should I make a wrapper launch file and send another pull request?

Jailander commented 9 years ago

Hi, don't worry I'll do that

marc-hanheide commented 9 years ago

I have accepted this for now. Obviously the default should then in the future be that machine tags are not being used (use_machine=false by default) and only in our own setting, a wrapper should set it to true... I still don't like this whole machine tag business. I wonder if we should initiate a discussion on this with the ROS people or on answers.ros.org to find the "right" way of doing this.