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

fix(nebula_launch.py): make the generic launch file hand off to the correct vendor's launch file #191

Merged
merged 1 commit into from
Sep 6, 2024

Conversation

mojomex
Copy link
Collaborator

@mojomex mojomex commented Sep 4, 2024

PR Type

  • Improvement
  • Bug Fix

Description

The nebula_launch.py file was long deprecated but also had incorrect/incomplete launch arguments for some vendors/sensors.
This PR re-writes this file such that it only explicitly handles the sensor_model argument, finds the corresponding vendor-specific launch file and hands off all remaining arguments to that file.
This way, backwards-compatibility is still ensured and future changes to vendor launch files will not necessitate updates to nebula_launch.py.

Support for containers is purpusefully dropped since the vendor launch files also do not support it anymore. This could be added back in in the future.

Review Procedure

  • Try launching nebula_launch.py with only sensor_model as an argument.
  • Try launching with vendor-specific arguments
  • Try launching with a custom config_file in a different location than the default one (in nebula_ros/config)

Pre-Review Checklist for the PR Author

PR Author should check the checkboxes below when creating the PR.

  • Assign PR to reviewer

Checklist for the PR Reviewer

Reviewers should check the checkboxes below before approval.

  • Commits are properly organized and messages are according to the guideline
  • (Optional) Unit tests have been written for new behavior
  • PR title describes the changes

Post-Review Checklist for the PR Author

PR Author should check the checkboxes below before merging.

  • All open points are addressed and tracked via issues or tickets

CI Checks

  • Build and test for PR: Required to pass before the merge.

@mojomex mojomex requested review from knzo25 and drwnz September 4, 2024 12:06
@mojomex mojomex self-assigned this Sep 4, 2024
@mojomex mojomex mentioned this pull request Sep 4, 2024
5 tasks
…arguments to the correct vendor's launch file
Copy link
Collaborator

@knzo25 knzo25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loved this !

@mojomex mojomex merged commit 723a853 into tier4:develop Sep 6, 2024
10 checks passed
@mojomex mojomex deleted the launch-fix-nebula-launch branch September 6, 2024 15:38
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