-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Add missing sitl setup and packages up to #5524
Add missing sitl setup and packages up to #5524
Conversation
if this is ready to merge let me know |
dev/source/docs/ros2-sitl.rst
Outdated
@@ -4,7 +4,7 @@ | |||
ROS 2 with SITL | |||
=============== | |||
|
|||
Once ROS2 is correctly :ref:`installed <ros2>`, source your workspace and launch Ardupilot SITL with ROS 2! | |||
Once ROS2 is correctly :ref:`installed <ros2>`, and SITL is also :ref:`installed <sitl-simulator-software-in-the-loop>`, source your workspace and launch Ardupilot SITL with ROS 2! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looks fine. From the discord discussions with users the main thing I'd want to emphasise is that users should have a working ArduPilot SITL environment (which requires all deps be installed) before attempting to build and run with ROS 2 integration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I change the wording here to be installed and working
instead of installed
?
I'm running the wiki build now to confirm rendering looks good. I'll tag you when everything is good. |
Sorry, after running through all the steps on a fresh machine, the build failed. I'll have to debug this next week at work. We might be missing a step to explicitely say to source the ArduPilot virtual environment at some point.
|
I have it figured out now; I needed to install the ArduPilot toolchains and have added that as a pre-req. The next error is unrelated, and is a problem in the repo, not the wiki.
|
* SITL is required to run ROS2 with SITL Signed-off-by: Ryan Friedman <[email protected]>
Signed-off-by: Ryan Friedman <[email protected]>
* We never told the user to build it * We also should keep reminding people to source their ros environment Signed-off-by: Ryan Friedman <[email protected]>
* Don't forget to source everything all the time * Don't build the world unless you need to * Unify rosdep commands with other instructions to ignore the source directory Signed-off-by: Ryan Friedman <[email protected]>
6752e2e
to
af100f4
Compare
* Add missing periods
Signed-off-by: Ryan Friedman <[email protected]>
e6f56fc
to
095193f
Compare
Signed-off-by: Ryan Friedman <[email protected]>
Missing a We also use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good thanks @Ryanf55.
Ready for merge! |
This improves instructions to make sure users don't miss steps for SITL.
It also improves the build workflow by only building what's needed, instead of everything, which is good when someone's ROS workspace is huge.
This work was sponsored by AeroVironment.