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

define initial ROS 2 variants #1

Merged
merged 8 commits into from
Jul 17, 2018
Merged

define initial ROS 2 variants #1

merged 8 commits into from
Jul 17, 2018

Conversation

mikaelarguedas
Copy link
Member

2 outstanding questions still:

  • Are we fine dropping the ros_ prefix for the variants?
  • Should rcl(cpp)_lifecycle live in core or base? (currently in base via lifecycle demo but I;d like them to be explicitly listed in a package.xml)

@mikaelarguedas mikaelarguedas added in progress Actively being worked on (Kanban column) ready Work is about to start (Kanban column) in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) ready Work is about to start (Kanban column) in review Waiting for review (Kanban column) labels Jul 12, 2018
@dirk-thomas
Copy link
Member

Are we fine dropping the ros_ prefix for the variants?

If there is no strong reason to do so I would lean to keep the names the same as in ROS 1.

Should rcl(cpp)_lifecycle live in core or base?

I would lean to base. As an analogy it is similar to actions in ROS 1.

@mikaelarguedas
Copy link
Member Author

Addressed the comments in the following commits:
add ros_ prefix b86c82d
add rcl(cpp)_lifecycle to ros_base 279a637
add pluginlib to ros_base to match ROS1 7a4e86a

@dirk-thomas
Copy link
Member

LGTM - I haven't checked the exact lists of dependencies though. the highlevel grouping should be documented somewhere (if not in an REP as in ROS 1 then somewhere else).

@wjwwood
Copy link
Member

wjwwood commented Jul 12, 2018

+1 to documenting it somewhere, but I'm not sure where. I suppose a new REP in the 2xxx range is the most proper, but I'd also be ok with somewhere else that's less tedious for now.

@mikaelarguedas
Copy link
Member Author

I haven't checked the exact lists of dependencies though

Yeah I was about to provide the breakdown

ros_core packages:

$ colcon list -n --packages-up-to ros_core
ament_cmake
ament_cmake_copyright
ament_cmake_core
ament_cmake_cppcheck
ament_cmake_cpplint
ament_cmake_export_definitions
ament_cmake_export_dependencies
ament_cmake_export_include_directories
ament_cmake_export_interfaces
ament_cmake_export_libraries
ament_cmake_export_link_flags
ament_cmake_flake8
ament_cmake_gmock
ament_cmake_gtest
ament_cmake_include_directories
ament_cmake_libraries
ament_cmake_lint_cmake
ament_cmake_pep257
ament_cmake_pytest
ament_cmake_python
ament_cmake_ros
ament_cmake_target_dependencies
ament_cmake_test
ament_cmake_uncrustify
ament_copyright
ament_cppcheck
ament_cpplint
ament_flake8
ament_index_python
ament_lint_auto
ament_lint_cmake
ament_lint_common
ament_package
ament_pep257
ament_uncrustify
builtin_interfaces
connext_cmake_module
fastcdr
fastrtps
fastrtps_cmake_module
gmock_vendor
gtest_vendor
launch
launch_ros
launch_testing
libyaml_vendor
lifecycle_msgs
opensplice_cmake_module
osrf_pycommon
osrf_testing_tools_cpp
poco_vendor
python_cmake_module
rcl
rcl_interfaces
rcl_yaml_param_parser
rclcpp
rclpy
rcutils
rmw
rmw_connext_cpp
rmw_connext_shared_cpp
rmw_fastrtps_cpp
rmw_implementation
rmw_implementation_cmake
rmw_opensplice_cpp
ros_core
ros_environment
rosgraph_msgs
rosidl_cmake
rosidl_default_generators
rosidl_default_runtime
rosidl_generator_c
rosidl_generator_cpp
rosidl_generator_dds_idl
rosidl_generator_py
rosidl_parser
rosidl_typesupport_c
rosidl_typesupport_connext_c
rosidl_typesupport_connext_cpp
rosidl_typesupport_cpp
rosidl_typesupport_interface
rosidl_typesupport_introspection_c
rosidl_typesupport_introspection_cpp
rosidl_typesupport_opensplice_c
rosidl_typesupport_opensplice_cpp
std_msgs
test_msgs
uncrustify_vendor

diff ros_core ros_base:

``` $ diff ros_core.packages ros_base.packages 0a1 > actionlib_msgs 1a3 > ament_cmake_auto 28a31 > ament_index_cpp 36a40,42 > class_loader > common_interfaces > composition 37a44,60 > console_bridge_vendor > demo_nodes_cpp > demo_nodes_cpp_native > demo_nodes_py > diagnostic_msgs > example_interfaces > examples_rclcpp_minimal_client > examples_rclcpp_minimal_composition > examples_rclcpp_minimal_publisher > examples_rclcpp_minimal_service > examples_rclcpp_minimal_subscriber > examples_rclcpp_minimal_timer > examples_rclpy_executors > examples_rclpy_minimal_client > examples_rclpy_minimal_publisher > examples_rclpy_minimal_service > examples_rclpy_minimal_subscriber 40a64 > geometry_msgs 46a71 > lifecycle 47a73,74 > logging_demo > nav_msgs 48a76 > orocos_kdl 50a79 > pluginlib 54a84 > rcl_lifecycle 56a87 > rclcpp_lifecycle 65a97,108 > ros2cli > ros2launch > ros2lifecycle > ros2msg > ros2node > ros2param > ros2pkg > ros2run > ros2service > ros2srv > ros2topic > ros_base 85a129,131 > sensor_msgs > shape_msgs > sros2 86a133,134 > std_srvs > stereo_msgs 87a136,145 > tf2 > tf2_eigen > tf2_geometry_msgs > tf2_msgs > tf2_ros > tinyxml2_vendor > tlsf > tlsf_cpp > topic_monitor > trajectory_msgs 88a147 > visualization_msgs ```

diff ros_base desktop

$ diff ros_base.packages ros_desktop.packages 
38a39
> angles
48a50,52
> dummy_map_server
> dummy_robot_bringup
> dummy_sensors
66a71,74
> image_tools
> intra_process_demo
> kdl_parser
> laser_geometry
69a78
> libcurl_vendor
73a83
> map_msgs
78a89,90
> pendulum_control
> pendulum_msgs
89a102
> resource_retriever
96a110
> robot_state_publisher
109a124
> ros_desktop
128a144,153
> rttest
> rviz2
> rviz_assimp_vendor
> rviz_common
> rviz_default_plugins
> rviz_ogre_vendor
> rviz_rendering
> rviz_rendering_tests
> rviz_visual_testing_framework
> rviz_yaml_cpp_vendor
141a167
> tinyxml_vendor
146a173,175
> urdf
> urdfdom
> urdfdom_headers

diff ALL desktop:

$ diff all.packages ros_desktop.packages 
2d1
< ament_clang_format
5d3
< ament_cmake_clang_format
22,23d19
< ament_cmake_nose
< ament_cmake_pclint
25,26d20
< ament_cmake_pep8
< ament_cmake_pyflakes
43d36
< ament_pclint
45,47d37
< ament_pep8
< ament_pyflakes
< ament_tools
121d110
< ros1_bridge
171,173d159
< test_cli
< test_cli_remapping
< test_communication
175,177d160
< test_osrf_testing_tools_cpp
< test_rclcpp
< test_security

the highlevel grouping should be documented somewhere (if not in an REP as in ROS 1 then somewhere else).

+1 to documenting it somewhere, but I'm not sure where. I suppose a new REP in the 2xxx range is the most proper, but I'd also be ok with somewhere else that's less tedious for now.

I'll open a draft REP later today.

@dirk-thomas
Copy link
Member

Please break down the list by repositories rather than by packages.

@mikaelarguedas
Copy link
Member Author

Please break down the list by repositories rather than by packages.

Is the goal to not have packages from the same repository in different variants ?
Or just to make it easier to read?

@dirk-thomas
Copy link
Member

Both, and this is how it is in the ROS 1 REP.

@mikaelarguedas
Copy link
Member Author

Both

The list needs to be changes then.
This was not a listed requirement originally. And this will be violated in multiple places.
If this is a hard requirement the variants need to be revisited and some repositories will need to be split

@mikaelarguedas
Copy link
Member Author

mikaelarguedas commented Jul 13, 2018

Looking more into this. The requirement of not having any overlap on the repos seems unfeasible for the Bouncy patch release (as it will require splitting multiple repositories or a very big set of packages in ros_core).
The main blockers are:

  • many packages use packages from the launch repo for testing, down to rcutils, launch repo contains ros2launch requiring all ros2cli to be in ros_core
  • several packages use msgs from common_interfaces for testing requiring common_interfaces to be in core
  • some demos require many things and dependencies while others are very basic, we would need to remove all demos from ros_base and put them in ros_desktop

As all bouncy packages have been released already, (and that I don't see why we would release the variants one by one anyway) I'm going to update this to reduce the amount of overlap when trivial but not push much more things in the core and not split repositories for the patch release.

If we want to respect that requirement for Crystal, we will need to audit all repository dependencies and split them as needed. Then refedine the variants accordingly

@dirk-thomas
Copy link
Member

dirk-thomas commented Jul 13, 2018

many packages use packages from the launch repo for testing, down to rcutils, launch repo contains ros2launch requiring all ros2cli to be in ros_core

roslaunch as well as the command line tools like rosmsg, rosrun etc. in ROS 1 are all in the ROS core metapackage. Maybe that isn't such a bad thing?

several packages use msgs from common_interfaces for testing requiring common_interfaces to be in core

The common_msgs repo in ROS 1 is also part of ROS core. Maybe that isn't such a bad thing either?

some demos require many things and dependencies while others are very basic, we would need to remove all demos from ros_base and put them in ros_desktop

In ROS 1 the goal of core and base is to be commonly installed on a robot where as desktop is meant to be for the developer machine. Having the demos in desktop doesn't seem like a bad choice?

@mikaelarguedas
Copy link
Member Author

mikaelarguedas commented Jul 14, 2018

Pushed a new version with a split closer to ROS 1. I am not planning on iterating more on it:

ros_core:

# ament org
ament_cmake
ament_index
ament_lint
ament_package
googletest
uncrustify
# eProsima
Fast-CDR
Fast-RTPS
# osrf
osrf_testing_tools_cpp
osrf_pycommon

# ros
class_loader
console_bridge
pluginlib
ros_environment

# ros2
ament_cmake_ros
common_interfaces
launch
libyaml_vendor
poco_vendor
rcl
rcl_interfaces
rclcpp
rclpy
rcutils
rmw
rmw_connext
rmw_fastrtps
rmw_implementation
rmw_opensplice
ros2cli
rosidl
rosidl_dds
rosidl_defaults
rosidl_python
rosidl_typesupport
rosidl_typesupport_connext
rosidl_typesupport_opensplice
tinyxml2_vendor

ros_base

# ros
urdfdom_headers
# ros2
geometry2
kdl_parser
orocos_kinematics_dynamics
robot_state_publisher
tinyxml_vendor
urdf
urdfdom

ros_desktop

cartographer
cartographer_ros
demos
depthimage_to_laserscan
example_interfaces
examples
joystick_drivers
laser_geometry
navigation
navigation_msgs
pcl_conversions
realtime_support
resource_retriever
ros_astra_camera
rviz
sros2
teleop_twist_joy
teleop_twist_keyboard
tlsf
# ros-perception
vision_opencv

none of the variants:

# ament
ament_tools
# ros2
ros1_bridge
system_tests
turtlebot2_demo

@mikaelarguedas mikaelarguedas added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Jul 16, 2018
@mikaelarguedas
Copy link
Member Author

planning on merging this shortly and to make a bouncy branch right away as this set of variants is specific to bouncy. The variants will need to me modified as master moves forward.

Last commits remove cartographer, astra driver and navigation packages from desktop as they're not in desktop in ROS 1.

Feel free to comment post-merge / open follow-up PRs

@mikaelarguedas mikaelarguedas merged commit 5068a1c into master Jul 17, 2018
@mikaelarguedas mikaelarguedas deleted the initial_variants branch July 17, 2018 00:13
@mikaelarguedas mikaelarguedas removed the in review Waiting for review (Kanban column) label Jul 17, 2018
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.

None yet

3 participants