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.

Sign up for GitHub

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

Jump to bottom

ROS2 - ardupilot_sitl launch file to include a "out" parameter for Mavproxy #25724

Merged
merged 1 commit into from
Feb 13, 2024
Merged

ROS2 - ardupilot_sitl launch file to include a "out" parameter for Mavproxy #25724

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Tools/ros2/ardupilot_sitl/src/ardupilot_sitl/launch.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,21 +284,21 @@ def generate_action(context: LaunchContext, *args, **kwargs) -> ExecuteProcess:

# Retrieve launch arguments.
master = LaunchConfiguration("master").perform(context)
# out = LaunchConfiguration("out").perform(context)
out = LaunchConfiguration("out").perform(context)
sitl = LaunchConfiguration("sitl").perform(context)

# Display launch arguments.
print(f"command: {command}")
print(f"master: {master}")
print(f"sitl: {sitl}")
print(f"out: {out}")

# Create action.
mavproxy_process = ExecuteProcess(
cmd=[
[
f"{command} ",
"--out ",
"127.0.0.1:14550 ",
f"--out {out} ",
Copy link
Contributor

Choose a reason for hiding this comment

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

should use a list as argument so you can add as much as you want .
That will be handly to remove the second --out too.

And it probably need default value to "127.0.0.1:14550 " in case nothing is passed to retain the current behavior

Copy link
Contributor

Choose a reason for hiding this comment

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

And it probably need default value to "127.0.0.1:14550 " in case nothing is passed to retain the current behavior

That's already handled by the generate_launch_arguments method which supplies a default for a single --out param.

should use a list as argument so you can add as much as you want .

Yep. Could revisit the current launch methods in a follow up PR and apply this consistently. Will prob need to handle both list and single value as fallback to support existing usage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with the list of --out, in general you may want to pass more than one argument at a time. And yes, the localhost:14550 is default, so you don't need to add it. One option could be to have localhost anyway and add an external --out argument in case it was different

"--out ",
"127.0.0.1:14551 ",
f"--master {master} ",
Expand Down
Loading