Closed srmainwaring closed 4 months ago
There seems to be some style changes, but the styles were formatted with clang-format-6.0 for ROS1. Could you maybe share the exact style format you are using?
Style changes are probably not intended. Some change, such as comments around unused arguments would be for clang. Line length is probably my changing from habit - working on Gazebo where there is a strict line length policy. If there is a formatter you like run I'd be happy to - @Ryanf55 do you know what is standard for ROS 2 these days?
Some of the inline implementations seems to have now gotten a cpp file. Could you maybe clarify why this is more preferred? I am fine with the changes, was just wondering why this would be better
A couple of reasons. The main one is that the header only versions lead to duplicate symbols in clang. The other reason is that inline code with branch conditions seldom gets compiled inline. Same if the functions are virtual. If the section is critical for performance then inlining may make sense, but most of the time placing the code in a translation unit is better and allows a smaller set of includes to be used in the header (which propagate) with most of the details in the cpp.
There seems to be some style changes, but the styles were formatted with clang-format-6.0 for ROS1. Could you maybe share the exact style format you are using?
Style changes are probably not intended. Some change, such as comments around unused arguments would be for clang. Line length is probably my changing from habit - working on Gazebo where there is a strict line length policy. If there is a formatter you like run I'd be happy to - @Ryanf55 do you know what is standard for ROS 2 these days?
Some of the inline implementations seems to have now gotten a cpp file. Could you maybe clarify why this is more preferred? I am fine with the changes, was just wondering why this would be better
A couple of reasons. The main one is that the header only versions lead to duplicate symbols in clang. The other reason is that inline code with branch conditions seldom gets compiled inline. Same if the functions are virtual. If the section is critical for performance then inlining may make sense, but most of the time placing the code in a translation unit is better and allows a smaller set of includes to be used in the header (which propagate) with most of the details in the cpp.
clang-format with google style guide. https://github.com/ethz-asl/grid_map_geo/pull/45
I have no perference if you want to fix formatting now or later; depends if it's going to create a huge diff or not. If the old code was formatted with version 6 or something, it would be best to preserve the old style in this PR, and then migrate versions in a follow up PR once we can enforce it in CI and ignore a single reformat commit to reduce code churn noise.
I got up to the part of launching terrain navigation but there is a problem with getting the origin. mavros log
[mavros_node-1] [WARN] [1705699705.649010553] [mavros.guided_target]: PositionTargetGlobal failed because no origin
planner log
[terrain_planner_node-1] [WARN] [1705699248.629211012] [terrain_planner]: Service [/mavros/cmd/command] not available.
[terrain_planner_node-1] Requesting global origin messages
@Jaeyoung-Lim and @Ryanf55, do you require any further changes to get this PR merged?
I've run
./Tools/fix_code_style.sh ..
on the repo, and it runs clean, although for clang-format 17.0.6
on macOS and 14.0.0-1ubuntu1.1
on Ubuntu 22.04 as these are the default versions for those platforms.
@Ryanf55 I'd prefer to get this PR merged and then apply your two changes as a follow up, as they need a rebase and this ros2
branch has been something of a WIP up until now in any case.
There seems to be some style changes, but the styles were formatted with clang-format-6.0 for ROS1. Could you maybe share the exact style format you are using?
Style changes are probably not intended. Some change, such as comments around unused arguments would be for clang. Line length is probably my changing from habit - working on Gazebo where there is a strict line length policy. If there is a formatter you like run I'd be happy to - @Ryanf55 do you know what is standard for ROS 2 these days?
Some of the inline implementations seems to have now gotten a cpp file. Could you maybe clarify why this is more preferred? I am fine with the changes, was just wondering why this would be better
A couple of reasons. The main one is that the header only versions lead to duplicate symbols in clang. The other reason is that inline code with branch conditions seldom gets compiled inline. Same if the functions are virtual. If the section is critical for performance then inlining may make sense, but most of the time placing the code in a translation unit is better and allows a smaller set of includes to be used in the header (which propagate) with most of the details in the cpp.
We can select clang-format likely - if it minimizes code churn.
Yea, feel free to merge this. I wasn't able to get it running all the way, but so far it's a big improvement.
@srmainwaring Unfortunately on myside it doesn't seem to progress after requesting for origin messages:
[terrain_planner_node-1] [INFO] [1708261393.675265756] [terrain_planner]: resource_path: /home/jaeyoung/ros2_ws/install/terrain_navigation_ros/share/terrain_navigation_ros/resources
[terrain_planner_node-1] [INFO] [1708261393.675368422] [terrain_planner]: map_path_: /home/jaeyoung/ros2_ws/install/terrain_navigation_ros/share/terrain_navigation_ros/resources/davosdorf.tif
[terrain_planner_node-1] [INFO] [1708261393.675374867] [terrain_planner]: map_color_path_: /home/jaeyoung/ros2_ws/install/terrain_navigation_ros/share/terrain_navigation_ros/resources/davosdorf_color.tif
[terrain_planner_node-1] [INFO] [1708261393.675379037] [terrain_planner]: mesh_resource_path_: /home/jaeyoung/ros2_ws/install/terrain_navigation_ros/share/terrain_navigation_ros/resources/believer.dae
[terrain_planner_node-1] [INFO] [1708261393.675382954] [terrain_planner]: avalanche_map_path: /home/jaeyoung/ros2_ws/install/terrain_navigation_ros/share/terrain_navigation_ros/resources/avalanche.tif
[terrain_planner_node-1] [INFO] [1708261393.675401365] [terrain_planner]: goal_radius_: 80
[terrain_planner_node-1] [INFO] [1708261393.675422066] [terrain_planner]: alt_control_p: 0.5
[terrain_planner_node-1] [INFO] [1708261393.675426568] [terrain_planner]: alt_control_max_climb_rate: 3
[terrain_planner_node-1] [INFO] [1708261393.675430631] [terrain_planner]: cruise_speed: 15
[terrain_planner_node-1] Requesting global origin messages
However, probably we should address this in following PRs! Thanks for all the work!
This PR updates services and launch files in the terrain planner node and rviz plugin to allow the ROS 2 port to run a PX4 example.
Figure: planned paths and the planning tree are now visible![ros2_terrain_planner_nav_1](https://github.com/ethz-asl/terrain-navigation/assets/24916364/11a5f818-845e-4cc1-bc23-8627da831ade)
Details
Issues
Other changes
GeographicLib
If you do not have the geoids for GeographicLib install the following:
Usage PX4
The testing uses this branch of PX4 : https://github.com/srmainwaring/PX4-AutoPilot/tree/prs/pr-hinwil-testing-rebased as it contains a small fix to the height rate setpoint forward ported to main, and a DEM for Gazebo of the region used in the terrain planner example.
Start QGC:
Start a PX4 SITL session with Gazebo:
standard_vtol_0
and select 'Move To' to centre the camera on the plane.Load the example mission:
terrain_navigation_ros/config/davosdorf_mission.plan
Figure: Gazebo and QGC in loiter
![px4_terrain_qgc_start](https://github.com/ethz-asl/terrain-navigation/assets/24916364/01be900a-25c4-4c9c-8dca-12419b24dbc1)
Launch the terrain planner, mavproxy and rviz nodes in separate terminals. These can be combined but it helps with debugging to run them separately:
Set the start and goal positions. If the interactive marker server is not updating correctly this may done using service calls:
Trigger the planner and set the planner to navigate. The equivalent ROS 2 service calls are:
Figure: rviz after planning![px4_terrain_rviz_plan](https://github.com/ethz-asl/terrain-navigation/assets/24916364/a7a37c39-edeb-42ce-9d6f-535360dc1027)
Finally engage the planner by switching the plane to OFFBOARD mode. Using mavros directly this is:
Figure: Gazebo and QGC at the goal
![px4_terrain_qgc_goal](https://github.com/ethz-asl/terrain-navigation/assets/24916364/0b46c0e5-1866-48a8-92d4-a2eedf64e998)
Usage ArduPilot (partial support)
There is work in progress to support ArduPilot, however the ArduPilot plane navigation code does not currently support the an external control mode that works with the global position set points emitted by the planner
Dependencies
There is currently a dependency on the launch scripts for ArduPilot DDS. These should be installed following the instructions here: https://github.com/ArduPilot/ardupilot/blob/master/Tools/ros2/README.md.
The ArduPilot dependencies may be installed in the same workspace as terrain navigation or a separate one. If the latter the ArduPilot workspace should be sourced in addition to the terrain navigation workspace when running examples.
Running
Launch files for each node have been provided to aid debugging. These need to be started in separate terminals.
Launch the
ardupilot_sitl
node:Launch the terrain planner node:
Launch the
mavros
node:Launch
rviz
:Start an interactive
mavproxy
session:Use the mavproxy mission editor to load the example mission from
./src/terrain_navigation_ros/config/davosdorf_mission.txt
. The mission comprises a VTOL takeoff, a waypoint and an unlimited loiter.In mavproxy, switch to
AUTO
and arm:Wait for the map to load in rviz. If it does not appear use the
Load Terrain
button. The console should report details such as:In another terminal call services to set the start and goal:
A green circle near the vehicles loiter position should appear.
A green circle on the mountain near the top of the snow should appear.
The terrain planner console should show:
Call the loiter service
Put the planner in its HOLD state:
Switch the flight controller to GUIDED:
Trigger the planner (the z-component of the vector3 is the planning budget time in seconds):
Console output:
Put the planner in its NAVIGATE state:
Verify the setpoints are being published: