Skip to content

Refactoring Launch FIles#51

Open
vincentfborello wants to merge 101 commits into
developfrom
feature/Refractor-Launch-Files
Open

Refactoring Launch FIles#51
vincentfborello wants to merge 101 commits into
developfrom
feature/Refractor-Launch-Files

Conversation

@vincentfborello

Copy link
Copy Markdown
Contributor

No description provided.

@vincentfborello vincentfborello marked this pull request as ready for review March 10, 2026 20:50

@zbrotherton zbrotherton left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I need to look into this deeper, but at first glance it looks like you're deleting vital launch components

Pull from develop so we're sure I'm only looking at your changes?

Comment thread src/igvc_nav/launch/igvc_nav.launch.py Outdated
Node = [
]

return LaunchDescription(declared_arguments + Node)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This won't work. We need nav2 to be part of our launch description, otherwise this launch file will not function

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You added the description to our arguments? I'm confused

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The commit 301767d should have solved the issue.

Comment thread src/igvc_nav/launch/igvc_nav.launch.py Outdated
Comment on lines 27 to 32
nav2 = IncludeLaunchDescription(
PythonLaunchDescriptionSource(nav2_launch_path),
launch_arguments={
'params_file' : config_path
}.items()
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove all commented out code blocks - this goes against the point of using git

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The commit 301767d should have solved the issue.

Comment on lines -16 to -25
declared_arguments.append(
DeclareLaunchArgument(
name="use_sim",
default_value="false",
description="Wether or not the robot is launching in simulation"
)
)

use_sim = LaunchConfiguration("use_sim")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Confused why you're removing these.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 317bf0c

Comment on lines +63 to +66
#TODO
linear.x.has_jerk_limits: false
linear.x.max_jerk: 0.0
linear.x.min_jerk: 0.0
# #TODO
# linear.x.has_jerk_limits: false
# linear.x.max_jerk: 0.0
# linear.x.min_jerk: 0.0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why did you comment these out?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i didn't do that though ;(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 40ad9a1

@rlj5 rlj5 self-assigned this Apr 7, 2026
@zbrotherton

Copy link
Copy Markdown
Contributor

Non-critical for competition

@zbrotherton zbrotherton added the wontfix This will not be worked on label May 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wontfix This will not be worked on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor all launch files to the same standard.

5 participants