Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better support running from a local catkin workspace #755

Merged
merged 1 commit into from
Jan 20, 2021

Conversation

peci1
Copy link
Collaborator

@peci1 peci1 commented Jan 19, 2021

Currently, running a multirobot simulation (or multiple simulations) directly from local catkin workspace has its quirks. This PR addresses them.

  1. robot_data bag files need to have a different name for different robots (and simulation partitions)... otherwise, all robots try to record to ~/.ros/robot_data_0.bag, and only one of them obviously succeeds.
  2. ignition log files (currently hardcoded to /tmp/ign/logs) need a configurable location, so that multiple simultaneous simulations do not write into the same folder.
  3. The "simulator rosbag" needs to respect the path set in point 2. (currently, it hard-codes /tmp/ign/logs regardless of the <path> setting of GameLogicPlugin)

I think I saw some initiative towards 2. somewhere (passing a CLI arg with the path instead of having it hardcoded in the launch file), but I can't remember where it was.

Also, the parametrization of ign launch files with the log path would need to modify all of the provided files. I modified just cloudsim_sim.ign, and when we agree whether it's a good approach, I'd modify all of them.

The renaming of robot rosbags might break backwards compatibility for Docker users - the names of the files would change. I'm not sure if this is acceptable or not, but I can imagine some workarounds that would keep the original name if Docker environment is detected (e.g. matching $USERNAME == developer, or checking for nonempty IGN_PARTITION). Local catkin workspace users would also be affected, but as they were using broken logic anyways, it shouldn't actually hurt them.

@osrf-jenkins
Copy link

Build finished. 15 tests run, 0 skipped, 0 failed.

@mjcarroll mjcarroll self-requested a review January 20, 2021 20:15
@nkoenig nkoenig merged commit fbbb512 into osrf:master Jan 20, 2021
@peci1
Copy link
Collaborator Author

peci1 commented Jan 21, 2021

This will need to be mentioned in the new release announcement. I suggest the following:

  • cloudsim_sim.ign got a new parameter logPath which specifies where to record the simulator logs. Previously, it was hardcoded to /tmp/ign/logs.
  • BAG files with robot data are named differently for each robot (and for each IGN_PARTITION). This allows running multirobot simulations from a local catkin workspace. For a robot with name ROBOT, the new name of the rosbags is as follows:
    • if IGN_PARTITION is unset or empty, the files will be named robot_data_${ROBOT}_${i}.bag (previously it was robot_data_${i}.bag).
    • If IGN_PARTITION is non-empty, the files will be named robot_data_${IGN_PARTITION}_${ROBOT}_${i}.bag (previously it was robot_data_${i}.bag).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants