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.

Sign up for GitHub

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

Jump to bottom

REP-2001: Propose c++-only minimal ros2 variant #231

Closed
wants to merge 3 commits into from
Closed

REP-2001: Propose c++-only minimal ros2 variant #231

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
181dbd6
First take at a c++-only minimal ros variant proposal
emersonknapp Feb 4, 2020
09b1494
Change to ros_cpp_core, explicitly mention all packages
emersonknapp Feb 5, 2020
d643255
Revert changes to ros_core for now
emersonknapp Feb 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 27 additions & 8 deletions rep-2001.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,26 +38,45 @@ We define three main entry points for ROS users.
* desktop (recommended)
* ros_base
* ros_core
* ros_minimal
emersonknapp marked this conversation as resolved.
Show resolved Hide resolved

Variants
--------

ROS Minimal
'''''''''''

The `ros_minimal` variant is a highly restricted set of ROS2 functionality.
It targets resource- and tool-constrained environments where package managers may be limited or unavailable.
The intention is that this variant can target cross-compiling for custom OS sysroots that may not have build tools or Python available that run on the target platform.
Copy link
Contributor

Choose a reason for hiding this comment

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

If it is an explicit goal for this variant to be able to be used on a target system that doesn't have Python available, a different building + environment setup process should be highlighted as the currently recommended one (colcon build + source a setup file) does not fulfill this requirement (colcon/colcon-core#73)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The goal isn't to have this be built on a target system that doesn't have Python available. The intention is to be built for a target system that doesn't have Python available. I want to make a strong distinction between the build-time environment and the run-time environment. While these are often the same in classic ROS workflows, It shouldn't be the assumption. Do you think I should add language around that here? I fully expect this to be built with colcon and linted with cpplint, for example.

Copy link
Member

Choose a reason for hiding this comment

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

The build also needs Python to run the code generators for messages / services / actions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But that's not needed at runtime, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

The intention is to be built for a target system that doesn't have Python available.

That's what I meant as well but it was not clear.
Currently AFAICT it is not possible to source a colcon setup file (top-level setup.bash file generated by colcon) without Python: colcon/colcon-core#73.
So for such a variant to be usable on a system that doesn't have Python available, we need a different workflow that allow to setup the environment on the target even if the target doesn't have Python. It can be modifying colcon to generate Python-agnostic setup files, or a different mechanism.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh - I see what you mean. The way I have used it is to

  • use --merged-install
  • export LD_LIBRARY_PATH=install/lib
  • invoke binaries by full path

Now that I'm thinking about that, yes, it seems we'll need a non-python way to set up the proper environment variables. Would it be possible to get away with creating a fairly simple pure-shell utiility? Something that would

  • Set LD_LIBRARY_PATH
  • Add all package lib folders to PATH

Copy link
Member

Choose a reason for hiding this comment

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

That simple approach will only work for a subset. A complete shell-based solution must replicate the same logic as it is implemented in Python. That includes collecting all packages in the prefix, getting their dependencies (available in the filesystem, doesn't need to parse the manifests), order them topologically, and then source the setup files of each package.

If the shell-based solution doesn't implement the logic around .dsv files that would also imply a much longer time to setup the environment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are these details just necessary to handle the different types of installs, or is it deeper than that? Perhaps we could impose limitations on the usage of this variant so as to avoid more complexity in the environment setup.

source the setup files of each package.

Do these require python, or just the top level script does?

Copy link
Member

Choose a reason for hiding this comment

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

Are these details just necessary to handle the different types of installs, or is it deeper than that? Perhaps we could impose limitations on the usage of this variant so as to avoid more complexity in the environment setup.

That has nothing do to with the different types of installs.

The importance is the order. One package might rely on an environment change caused by another package it depends on.

source the setup files of each package.

Do these require python, or just the top level script does?

No, they don't.

It may not contain any GUI or Python dependencies.
It may not have any dependencies outside a source workspace of its members with an exception of ubiquitous Linux libraries.
emersonknapp marked this conversation as resolved.
Show resolved Hide resolved
Note that this variant explicitly does not include an implementation of either RMW or rcl_logging, users will need to choose their own that handle the dependencies that come with them.
Copy link
Member

Choose a reason for hiding this comment

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

I think we need to at least have one RMW in order to be able to build this variant - it can be just the default (FastRTPS) - or if that requires undesired dependencies any other RMW impl.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The attempt to avoid FastRTPS was because of its dependency on libasio-dev. I'm more inclined toward Cyclone because it has no external rosdeps, but I know it's not the default RMW - what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I agree that I think that it should stand alone and be buildable in the default configuration without modifications. Otherwise the out of the box experience will be quite poor. And we won't be able to test it easily as all tests would need to have logic to extend it with an implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I included rcl_spdlog and rmw_cyclonedds in the latest revision

Copy link
Member

Choose a reason for hiding this comment

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

I think there needs to be a paragraph describing the rational why CycloneDDS was chosen over the default RMW FastRTPS.

Copy link

Choose a reason for hiding this comment

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

FWIW, and I'm not advocating for FastRTPS, I've been able to cross compile to an embedded ARM target using FastRTPS via --cmake-flags -DTHIRDPARTY=ON which forces FastRTPS to source it's own copy of asio (among other things).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's good information - perhaps we can target this at "one of" like the current core does


::

- ros_minimal:
packages: [ament_cmake, ament_cmake_ros, ament_index,
class_loader, common_interfaces, console_bridge,
libyaml_vendor, poco_vendor, rcl, rcl_interfaces,
rcl_logging, rclcpp, rcpputils, rcutils, rmw,
rmw_implementation, ros_tracing, rosidl, rosidl_defaults,
rosidl_typesupport, spdlog_vendor, test_interface_files,
tinydir_vendor, unique_identifier_msgs]
Copy link
Contributor

Choose a reason for hiding this comment

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

historically despite the name "packages", this represent a list of repositories that one can clone and build as is (without the REP needing to explicitly list all packages in all the listed repos).
However some of these repositories contain python packages (e.g. ament_index repository includes ament_index_cpp and ament_index_py or ament_cmake includes many python packages). Should they be split out to keep this list "without any Python dependency"? or switch to listing explicitly packages and a recommended way to clone only those packages?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point - I am definitely hoping to build a subset of packages within the involved repositories. I'll add a list of repos and the packages from them that will be built. Since the variants are described by a meta-package, though, if you build --packages-up-to VARIANT that's probably more correct than checking things out and building everything.



ROS Core
''''''''

The `ros_core` variants composes the core communication protocols.
The `ros_core` variant composes the core communication protocols.
It may not contain any GUI dependencies.

::

- ros_core:
packages: [ament_cmake, ament_cmake_ros, ament_index, ament_lint,
ament_package, class_loader, common_interfaces,
console_bridge, googletest, launch, libyaml_vendor,
osrf_pycommon, osrf_testing_tools_cpp, pluginlib,
poco_vendor, rcl, rcl_interfaces, rclcpp, rclpy,
rcutils, rmw, rmw_implementation, ros2cli, rosidl,
rosidl_dds, rosidl_defaults, rosidl_python, rosidl_typesupport,
extends: [ros_minimal]
packages: [ament_lint, googletest, osrf_pycommon, osrf_testing_tools_cpp,
pluginlib, rclpy, ros2cli, rosidl_dds, rosidl_python,
ros_environment, sros2, tinyxml2_vendor, uncrustify]
And at least one of the following rmw_implementation:
- Fast-RTPS: [Fast-CDR, Fast-RTPS, rmw_fastrtps]
Expand Down