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

Refactor Ignition launch files and spawners to share code #786

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

peci1
Copy link
Collaborator

@peci1 peci1 commented Feb 10, 2021

This is a continuation of #759. In that PR I noticed how cumbersome and unreliable it is to add a change affecting all robots, e.g. a new plugin shared by all robots.

I was seeking for some kind of <include> tag for ign-launch, which isn't yet supported for anything else than whole models. After discussion with @adlarkin, we concluded that it could be tackled from Ruby.

Here's a partial PR on which I mainly want to agree whether the proposed changes are okay, and once they are approved, I can "spread them" to all files that could potentially benefit from them (all robot spawn scripts and some world launch files).

Basically, I created three Ruby library files which contain most of the logic that should rather be shared than copied all over the repository. My ruby-fu is not yet very mature, so please pay special attention to the library code (I learned something during preparation of the PR, but it was mainly bits and pieces).

I based this PR on top of #759 because it wouldn't make sense to start it off of master. So the changes as viewed in this PR are a bit mixed, and it's better to view the changelog at peci1/subt@peci1:dynamic_ground_truth_tf...peci1:ruby-refactor . That are just the changes proposed in this PR (7 changed files ATM).

# Conflicts:
#	subt_ign/launch/cave_circuit.ign
#	subt_ign/launch/tunnel_circuit_practice.ign
#	subt_ign/launch/urban_circuit.ign
@osrf-jenkins
Copy link

Build finished. 21 tests run, 0 skipped, 1 failed.

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.

2 participants