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

Tools: add ROS 2 launch tests #23429

Merged
merged 1 commit into from
Apr 22, 2023

Conversation

srmainwaring
Copy link
Contributor

@srmainwaring srmainwaring commented Apr 7, 2023

This PR adds two colcon packages that enable colcon to build and test the AP_DDS library in SITL.

There is a brief README in ./Tools/ros2 which includes instructions for running the examples in the Dockerfile_dev-ros Docker image.

Related issues

Motivation

The objective is to be able to build and test ArduPilot / ROS 2 integration using colcon with commands such as:

# build
colcon build --packages-select ardupilot_dds_tests

# test
colcon test --packages-select ardupilot_dds_tests

# test results
colcon test-result --all --verbose

We'd eventually like to be able to launch a simulation example as

ros2 launch ardupilot_sitl_gz copter.launch.py

A key requirement is that it should be easy for users familiar with ROS to install and run.

Some non-functional requirements are that the code required to support this be modular and reusable. In particular we'd like to reuse the code for launching in the tests.

Tasks

Details

Two packages are added to ./Tools/ros2.

ardupilot_sitl

A cmake project with a colcon package XML that presents the ArduPilot waf build as a cmake project, installing targets into the usual workspace folders expected by a ROS 2 colcon build. There are a couple of example launch files that will form component parts of the larger test suite for AP_DDS. These allow SITL and a non-interactive mavproxy session to be run from the ROS 2 CLI. For example:

ros2 launch ardupilot_sitl sitl.launch.py
ros2 launch ardupilot_sitl mavproxy.launch.py

The launch files support substitution actions and have launch arguments exposed, so that:

ros2 launch ardupilot_sitl sitl.launch.py --show-args
Arguments (pass arguments as '<name>:=<value>'):

    'command':
        Run ArduPilot SITL.
        (default: 'arducopter')

    'model':
        Set simulation model.
        (default: 'quad')

    'slave':
        Set the number of JSON slaves.
        (default: '0')

    'sim_address':
        Set address string for simulator.
        (default: '127.0.0.1')

    'speedup':
        Set simulation speedup.
        (default: '1')

    'instance':
        Set instance of SITL (adds 10*instance to all port numbers).
        (default: '0')

    'defaults':
        Set path to defaults file.
        (default: '/Users/rhys/Code/ros2/xrce-dds/ardupilot_ros2_ws/install/ardupilot_sitl/share/ardupilot_sitl/config/default_params/copter.parm')

Executables and default parameters are resolved in the launch scripts using the usual ROS 2 package tools.

ardupilot_dds_tests

This package will contain the test suite for the AP_DDS library. At present it has a launch file for creating the virtual ports needed for testing SITL:

ros2 launch ardupilot_dds_tests virtual_ports.launch.py tty0:=./dev/tty0 tty1:=./dev/tty1
[INFO] [launch]: All log files can be found below /Users/rhys/.ros/log/2023-04-07-14-14-46-569511-MacPro2-2.local-9436
[INFO] [launch]: Default logging verbosity is set to INFO
command:      socat
tty0:         ./dev/tty0
tty1:         ./dev/tty1
[INFO] [tty1 -1]: process started with pid [9442]
[tty1 -1] 2023/04/07 14:14:46 socat[9442] N PTY is /dev/ttys024
[tty1 -1] 2023/04/07 14:14:46 socat[9442] N PTY is /dev/ttys025
[tty1 -1] 2023/04/07 14:14:46 socat[9442] N starting data transfer loop with FDs [5,5] and [7,7]

There is an example test checking the time message published to the topic /ROS2_Time is received.

colcon test --packages-select ardupilot_sitl  ardupilot_dds_tests
Starting >>> ardupilot_dds_tests
Starting >>> ardupilot_sitl
Finished <<< ardupilot_sitl [0.54s]                               
--- stderr: ardupilot_dds_tests                     

=============================== warnings summary ===============================
test/test_time_msg_received.py::test_time_msgs_received
test/test_virtual_ports.py::test_read_stdout
  Warning: There is no current event loop

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
---
Finished <<< ardupilot_dds_tests [13.1s]

Summary: 2 packages finished [15.5s]
  1 package had stderr output: ardupilot_dds_tests

@srmainwaring srmainwaring marked this pull request as draft April 7, 2023 13:29
@srmainwaring srmainwaring force-pushed the pr/pr-dds-launch-tests branch 5 times, most recently from 972a926 to fce1216 Compare April 11, 2023 21:20
@srmainwaring srmainwaring marked this pull request as ready for review April 11, 2023 21:31
Copy link
Collaborator

@Ryanf55 Ryanf55 left a comment

Choose a reason for hiding this comment

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

Great work so far. Going to go ahead and do some testing to validate. A few minor points here and there.

Tools/ros2/README.md Show resolved Hide resolved
Tools/ros2/ardupilot_dds_tests/setup.py Show resolved Hide resolved
Tools/ros2/ardupilot_sitl/CMakeLists.txt Outdated Show resolved Hide resolved
Tools/ros2/ardupilot_sitl/package.xml Outdated Show resolved Hide resolved
Tools/ros2/ardupilot_sitl/tests/test_a.py Outdated Show resolved Hide resolved
Tools/ros2/ros2.repos Outdated Show resolved Hide resolved
Add ros2.repo for installing packages with vcstool.

Add cmake custom targets to run waf configure and build.

Update ros2.repo.
- Add dependency on arshPratap/ardupilot_ros2.git.
- Update version used for ardupilot_ros2.git.
- Update ardupilot branch.
- Reduce to minimum required.

Add ROS 2 launch tests.
- Add ROS 2 Python package for testing the AP_DDS library.
- Initial version including example Python test.
- Move the CMakeLists.txt to ./Tools/scripts/ros2/ardupilot_sitl.
- Add virtual port test.

Update README.
- Add instructions for using the docker image.

Disable socat tests return code.
- Not portable across platforms?

Update ros2.repo
- Reduce to minimum required.
- Update README.
- Update ardupilot_dds_tests packahe.xml

Use pytest tmp_path_factory.
- Use pytest built-in fixture for tmp directories.

Update test README.
- Update instructions for running test in docker.

Add test config and param files.

Add subscriber to Time messages.

Clean up virtual port test.
- Remove unused code.

Test time message is published.
- Copied from ardupilot_ros2/pr-ci-test-package branch.

Update time msg test
- Update workspace relative path.
- Remove sleep in main test.

Add original time message test.
- Add original version of time message test to help resolve failure.

Use separate processes for sitl and mavproxy.

Update original time message test.

Add Python testing to the ardupilot_sitl package.
- Add support for Python testing in the ardupilot_sitl package.

Add install section to CMakeLists.txt.
- Install executables and libs.
- Install default params and launch files.

Add launch for SITL.
- Add example launch file for SITL.
- Add local param file in config (test install).

Update ROS time message test.

Update CMakeLists.txt
- Set executable bit when installing programs.

Add example launch file for mavproxy.

Fix param path in sitl.launch.

Fix sitl address and port in example dds test.

Rename ardupilot dds yaml config file.

Rework sitl.launch to support launch arguments.
- Add launch arguments.
- Prep work for composing launch files.

Rework mavproxy.launch to support launch arguments.
- Add launch arguments.
- Fix default instance in sitl.launch.
- Run as MAVProxy in non-interactive mode.

Add launch file for socat virtual port process.
- Add separate launch file for process creating virtual ports.

Rename launch file for creating virtual ports.
- Remove unused import.

Add sample config yaml for sitl.launch.

Update ros2.repos.
- Remove ardupilot_ros2 and micro_ros_setup.
- Rename branch.

Move ROS 2 packages up a level.

Update path to ArduPilot root directory in CMakeLists.txt.

Update paths in ros2 dds time message tests

Update ros2 README and provide separate ros2.repo for macOS.
- Add build instructions for each platform.
- Provide separate ros2.repos for macOS which has additional dependencies to build from source.

Add composite launch for sitl and mavproxy.
-Provide example of composite launch that reuses existing launch files.

Add uart and serial port arguments to sitl.launch
- Add extra (optional) arguments for ports.
- Handle default arguments (e.g. wipe and synthetic clock).
- Remove use of TextSubstitution which seems redundant.

Simplify and update formatting in mavproxy and virtual port launches.
- Update print formatting.
- Remove use of TextSubstitution.

Add launch file for micro_ros_agent.
- The launch file in the micro_ros_agent does not have launch arguments.
- Replace hardcoded transport.

Correct install path for launch files in setup.py.
- Correct install path for launch files.
- Format line length.

Update micro_ros_agent launch.
- Do not use None for launch argument default value.

Add composite launch file for the time message test.
- Compose launch from four simpler launch files.

Comment unused variables for linting.

Install dds profile.
- Update CMakeLists.txt to install the dds_xrceprofile.
- Move install location of dds.parm to config/default_params.
- Update README with notes on equivalent command line calls.

Correct launch for micro_ros_agent.
- Remove extra space prefixing device field.
- Update README with example launch commands.

Update launch examples in README
- Update README with example launch commands.

Update combined launch for DDS time message test.
- Add events to combined launch to control launch sequence.
- Update README with example command for combined launch.

Remove dds.parm from ardupilot_dds_tests.
- File moved to ardupilot_sitl.

Update combined launch for DDS time message test.
- Disable events as these will not work with a launch description as the target_action.

Rename launch file for bringing up sitl with dds.

Rename virtual ports launch test.

Use PathJoinSubstitution and FindPackageShare for package resolution.
- Use substitutions for package and launch path resolution.

Update launch example in readme.
- Fix typo in combined launch.

Update virtual ports test case.

Rename virtual ports test case.

Rename time message test case.

Rework the time msg test case to use previously defined launch files.

Add time message check node.

Clean up test cases.

Move bringup fixture into conftest.py.
- Factor out bringup fixture.

Remove unused code and imports.

Add test for navsatfix.
- Update qos profile for navsatfix test.

Update test case names.

Use pathlib instead of os.path.

Set speedup to 10, reduce test timeout.

Update CMakeLists.txt
- Remove --debug.
- Remove commented code.

Update sitl_launch.py.
- Use max_serial_ports instead of hardcoded number.

Remove sample python test.

Update maintainer for ros2 packages.

Update ros2.repos.
- Point to ardupilot master instead of fork and PR branch.

Update CMakeLists.txt.
- Fix format (indent) in build test section.

Enable ament linters and use black formatter.
- Enable ament_lint_auto in CMakeLists.txt.
- Override default flake8 config to prevent conflicts with black formatter.
- Update README.
- Update files to satisfy linters.

More PEP 257 compliance.
- Adopt recommended style for comments.

Apply linters and formatter.
- Apply linter and black formatter to ardupilot_dds_tests.
- Move package tests under folder.
- Reinstate copyright, flake8 and pep257 tests.

Reorganise ros2 launch files
- Move launch files for SITL from ardupilot_dds_tests to ardupilit_sitl.
- Update docs.

Signed-off-by: Rhys Mainwaring <rhys.mainwaring@me.com>
@srmainwaring
Copy link
Contributor Author

srmainwaring commented Apr 18, 2023

Update

  • Moved launch files in ardupilot_dds_tests to ardupilot_sitl. Launch files being used downstream should not be in a project named *_test.
  • Renamed the launch file for sitl_dds.
  • Update README and refactor tests to accommodate launch file changes.

@tridge tridge requested a review from Ryanf55 April 19, 2023 08:23
@tridge
Copy link
Contributor

tridge commented Apr 19, 2023

@Ryanf55 are you ok to approve this?

@tridge tridge removed the DevCallEU label Apr 19, 2023
Copy link
Collaborator

@Ryanf55 Ryanf55 left a comment

Choose a reason for hiding this comment

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

LGTM. Minor updates can be done after it bootstraps.

Tools/ros2/README.md Show resolved Hide resolved
Tools/ros2/README.md Show resolved Hide resolved
@davidbuzz
Copy link
Collaborator

it does seem a bit weird to me to use colcon to execute cmake to execute waf ... when all three of them are build tools with similar capabilities.

@Ryanf55
Copy link
Collaborator

Ryanf55 commented Apr 20, 2023

it does seem a bit weird to me to use colcon to execute cmake to execute waf ... when all three of them are build tools with similar capabilities.

If ArduPilot supported CMake, then it would only be two, but we're just using what's available. It's a bit of an odd marriage, but that's the best idea we had for this initial version.

@srmainwaring
Copy link
Contributor Author

@davidbuzz it seems a bit strange at first, but it's a mechanism to translate ArduPilot's core repo into the ROS world without making any intrusive changes.

Here's some of the reasoning behind it: colcon is more of a package management system / build workflow coordinator than a build system like Python setuptools or cmake. ROS 2 ament, which extends colcon, dictates the structure of ROS 2 packages so the tooling can locate and find libraries, binaries and other resources. In order for those to work we need to make ArduPilot look and feel like a set of ROS 2 packages and follow their rules. They have no rules for waf, so the package has to be either cmake or Python. The Python packages have a number of limitations - very stringent directory structure and no symlink installs - so cmake was the better option. We also want to make ArduPilot appear as a number of ROS packages - one for core library, one for tests, perhaps others to come. This precludes setting the package structure at the ArduPilot root (which is also more intrusive). Finally, if we want to eventually distribute built packages as .debs or the widows equivalents, so ROS users can download pre-built versions of ArduPilot for ROS (which would make setting up new users that already have a working ROS much easier) then I think we need to go all in with the ROS build / install process. The resulting installation from the colcon build is completely divorced from the ArduPilot source structure and will run independently of that. I don't believe that is true for things like sim_vehicle.py and autotools which rely on things being in certain places in the ArduPilot source tree.

@tridge tridge merged commit 96d7265 into ArduPilot:master Apr 22, 2023
@srmainwaring srmainwaring deleted the pr/pr-dds-launch-tests branch May 1, 2023 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants