LCAS / topological_navigation

The topological navigation framework
Apache License 2.0
31 stars 36 forks source link

ROS 2 / Humble - porting #160

Closed Iranaphor closed 1 year ago

Iranaphor commented 1 year ago

Includes:

Much is left to port, however I thought it best to open a (WIP) PR now.

Currently I have not yet addressed topological_utils meaning topological_rviz_tools is also uncompilable. By skipping topological_rviz_tools topological_utils, the remaining 3 packages will build successfully. However containing code will still require further porting.

colcon build --symlink-install --packages-skip topological_rviz_tools topological_utils

A noteworthy change which was required, is to some msg files which utilised the field name "namespace". The rosidl_generator_cpp is not able to compile msg files which utilise keywords from c. I have renamed these instances instead, to "namespace_name".

Iranaphor commented 1 year ago

The build process should work now, however note that none of the ROS1 CPP files have been ported yet, (and they are commented out in their respective CMakeLists.txt file to avoid a build crash). The ROS1 Python files have also not been ported yet, nor have any ROS1 Launch files.

The Service files from topological_utils have been moved into topological_navigation_msgs due to some conflicts between rosidl_generate_interfaces and ament_python_install_package which I was not able to fully solve.It seemed to make sense given we have a msgs package already available and included, so now topological_utils, is now looking to be a full python package rather than a combined one. Depending on feedback from @marc-hanheide (+others?), I might recommend also moving messages from bayesian_topological_navigation to topological_navigation_msgs.

Due to my unfamiliarity with ROSCPP and RCLCPP, I am going to avoid updating the topological_rviz_tools package myself, and leave this for someone more experienced.

marc-hanheide commented 1 year ago

Awesome stuff, @Iranaphor !

I fully agree that the messages should all be moved.

Great you took this on and made it work. I’m also not familiar with CPP in ROS2 yet, so we need to see how we migrate the topological_rviz_tools.

Iranaphor commented 1 year ago

Combining the message files, there were two srv files by the same name AddEdge, the instance in the topological_rviz_tools package, I have renamed (along with its uses within the stack) to AddEdgeRviz during the centralising to topological_navigation_msgs.

I have fixed the authorships to reflect the ROS1 HEAD, and have now synced the packages to all be version 3.0.0 to coincide with the major update to ROS2. Including myself as a maintainer on all packages and Marc on topological_navigation. The setup.py files have also had their maintainers updated to reflect the first maintainer in their respective package.xml files.

I have noticed that, unlike the other packages, bayesian_topological_localisation's lisence is BSD, rather than MIT. Is this intentional of should it coincide with the rest?