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

Add missing sitl setup and packages up to #5524

Merged

Conversation

Ryanf55
Copy link
Contributor

@Ryanf55 Ryanf55 commented Nov 3, 2023

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.

@Hwurzburg
Copy link
Contributor

if this is ready to merge let me know

@@ -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!
Copy link
Contributor

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.

Copy link
Contributor Author

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?

@Ryanf55 Ryanf55 marked this pull request as draft November 3, 2023 21:32
@Ryanf55
Copy link
Contributor Author

Ryanf55 commented Nov 3, 2023

if this is ready to merge let me know

I'm running the wiki build now to confirm rendering looks good. I'll tag you when everything is good.

@Ryanf55
Copy link
Contributor Author

Ryanf55 commented Nov 3, 2023

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.

rfriedman@dev-rfriedman01:~/Dev/ardu_ws$ colcon build --packages-up-to ardupilot_dds_tests --cmake-args -DBUILD_TESTING=ON
Starting >>> micro_ros_agent
Finished <<< micro_ros_agent [0.62s]                     
Starting >>> ardupilot_sitl
--- stderr: ardupilot_sitl                               
openjdk version "11.0.20.1" 2023-08-24
OpenJDK Runtime Environment (build 11.0.20.1+1-post-Ubuntu-0ubuntu122.04)
OpenJDK 64-Bit Server VM (build 11.0.20.1+1-post-Ubuntu-0ubuntu122.04, mixed mode, sharing)

openjdk version "11.0.20.1" 2023-08-24
OpenJDK Runtime Environment (build 11.0.20.1+1-post-Ubuntu-0ubuntu122.04)
OpenJDK 64-Bit Server VM (build 11.0.20.1+1-post-Ubuntu-0ubuntu122.04, mixed mode, sharing)

openjdk version "11.0.20.1" 2023-08-24
OpenJDK Runtime Environment (build 11.0.20.1+1-post-Ubuntu-0ubuntu122.04)
OpenJDK 64-Bit Server VM (build 11.0.20.1+1-post-Ubuntu-0ubuntu122.04, mixed mode, sharing)

openjdk version "11.0.20.1" 2023-08-24
OpenJDK Runtime Environment (build 11.0.20.1+1-post-Ubuntu-0ubuntu122.04)
OpenJDK 64-Bit Server VM (build 11.0.20.1+1-post-Ubuntu-0ubuntu122.04, mixed mode, sharing)

openjdk version "11.0.20.1" 2023-08-24
OpenJDK Runtime Environment (build 11.0.20.1+1-post-Ubuntu-0ubuntu122.04)
OpenJDK 64-Bit Server VM (build 11.0.20.1+1-post-Ubuntu-0ubuntu122.04, mixed mode, sharing)

Build failed
Traceback (most recent call last):
  File "/home/rfriedman/Dev/ardu_ws/src/ardupilot/modules/waf/waflib/Task.py", line 348, in process
    ret = self.run()
  File "/home/rfriedman/Dev/ardu_ws/src/ardupilot/Tools/ardupilotwaf/mavgen.py", line 54, in run
    from pymavlink.generator import mavgen
  File "/home/rfriedman/Dev/ardu_ws/src/ardupilot/modules/mavlink/pymavlink/generator/mavgen.py", line 26, in <module>
    from future import standard_library
ModuleNotFoundError: No module named 'future'

gmake[2]: *** [CMakeFiles/ardupilot_build.dir/build.make:70: CMakeFiles/ardupilot_build] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:168: CMakeFiles/ardupilot_build.dir/all] Error 2
gmake: *** [Makefile:146: all] Error 2
---
Failed   <<< ardupilot_sitl [18.5s, exited with code 2]

@Ryanf55
Copy link
Contributor Author

Ryanf55 commented Nov 6, 2023

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.

- ardupilot_dds_tests.test.ardupilot_dds_tests.test_virtual_ports test_virtual_ports
  <<< failure message
    AssertionError: Test process had no output.
    assert 'N starting data transfer loop' in '/bin/sh: 1: socat: not found\n'
  >>>

Ryan Friedman added 4 commits November 6, 2023 13:37
* 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]>
@Ryanf55 Ryanf55 force-pushed the add-missing-sitl-setup-and-packages-up-to branch from 6752e2e to af100f4 Compare November 6, 2023 20:38
@Ryanf55 Ryanf55 marked this pull request as ready for review November 6, 2023 20:39
Ryan Friedman added 2 commits November 6, 2023 13:41
@srmainwaring
Copy link
Contributor

srmainwaring commented Nov 6, 2023

socat: not found

Missing a sudo apt install socat?

We also use ros-humble-launch-pytest which is not installed as part of desktop-full (but is available for Humble).

Copy link
Contributor

@srmainwaring srmainwaring left a 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.

@Ryanf55
Copy link
Contributor Author

Ryanf55 commented Nov 6, 2023

if this is ready to merge let me know

Ready for merge!

@Hwurzburg Hwurzburg merged commit b78f81e into ArduPilot:master Nov 7, 2023
4 checks passed
@Ryanf55 Ryanf55 deleted the add-missing-sitl-setup-and-packages-up-to branch January 20, 2024 20:41
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.

3 participants