rclpy icon indicating copy to clipboard operation
rclpy copied to clipboard

rclpy composition API

Open crystaldust opened this issue 4 years ago • 20 comments

Feature request

Feature description

The composition API allows developer to manage the launch of node at runtime as described in the design: process-with-multiple-nodes

The rclcpp library has implemented the composition API and has a few samples: composition, minimal_composition

Implementation considerations

I don't have a clear implementation for the API yet, but the rclcpp implementation should be considered as a reference: register node macro

Related Refs:

crystaldust avatar Jun 16 '20 00:06 crystaldust

I now have a rough understanding of the composition, according to the design and rclcpp implementation, the rclpy should implement:

component container

  • The container itself is a node
  • The container binds an executor(single/multi threaded)
  • The container provides 3 services: ~/_container/list_nodes, ~/_container/load_node, ~/_container/unload_node
  • The list node service should return the name and id of the nodes which are registered as Component, ~~the rclpy can refer to the ros2cli component verb's implementation(won't work, since the verb's list command just call the service)~~
  • The load/unload node service call the executor's add_node/remove_node method

component The rclcpp now uses a macro RCLCPP_COMPONENTS_REGISTER_NODE to regsiter node as Component, it registers node with class_loader, something like entrypoint. According to the design docs, Python's entry points should be able to do this.

crystaldust avatar Jun 17 '20 01:06 crystaldust

The composition mechanism relies on ament_resource_indexer, basically the resources are stored in path <prefix>/share/ament_index/resource_index/<resource_type>/<resource_name>.

The ros2cli component verb's types command will only try to find resource_type 'rclcpp_components' as the code shows here. The rclcpp's implementation uses ros/class_loader to load cpp classes from the compiled .so files. The .so files are compiled following the cmake rules described in the application level codes(like the composition demo), I'm not a C++ expert, but the basic logic should be like this.

So a few things should be considered:

  • Move the rclcpp_components resource type to ros(2)_components to make it language irrelevant.

  • Different language client libs take advantage of the language speicification to "load class dynamically" from the ros(2)_components resource, which might be mapped to a .so file, .dll file(for Windows), or source code(for scripting languages like NodeJS, Python).

  • For the Python client lib rclpy, classes can be compiled to / loaded from a .so file with Cython, or just python source code files with importlib. We need some discussion on this, the former encapsulate the code but import complexity, the latter looks more 'Pythonic' but the node classes' source code is accessible.

crystaldust avatar Jun 17 '20 10:06 crystaldust

Move the rclcpp_components resource type to ros(2)_components to make it language irrelevant.

I am not sure how this would be beneficial. In each language the logic how to load a component is specific to that language. A container written in one language can't load components written in a different language.

For the Python client lib rclpy, classes can be compiled to / loaded from a .so file with Cython, or just python source code files with importlib. We need some discussion on this, the former encapsulate the code but import complexity, the latter looks more 'Pythonic' but the node classes' source code is accessible.

I would propose to just use a Python API / entry point for Python components. The component can then decide internally if it wants to use a Cython library or not.

dirk-thomas avatar Jun 17 '20 20:06 dirk-thomas

I am not sure how this would be beneficial. In each language the logic how to load a component is specific to that language. A container written in one language can't load components written in a different language.

That's true, so maybe we should add more resource types like rclpy_components and ros2cli component verb would also obtain these types when listing components.

And when load/unload is called, the client lib should be able to distinguish in which language the component is written, if the user try to load a cpp component into a python container, an exception should be raised.

crystaldust avatar Jun 18 '20 01:06 crystaldust

And when load/unload is called, the client lib should be able to distinguish in which language the component is written, if the user try to load a cpp component into a python container, an exception should be raised.

The container could also offer a service to provide a list of supported component types. That way the CLI can offer to only complete matching ones and already error out before calling the container.

dirk-thomas avatar Jun 18 '20 01:06 dirk-thomas

That's a good idea, I see the CLI uses argcomplete to match arguments, should I provide a service whose name is in a specific pattern to make argcomplete recognize the matching rules? I see your answer in a similar question here, the argcomplete should rely on the ament index resource.

For now, a container written in a specific language could only support components written in the same language(I guess there are tricks to make some lower-level language able to load code in other languages, but it might make things too complex), so a python container would always say "Hi, I support ['rclpy_component']".

crystaldust avatar Jun 18 '20 03:06 crystaldust

I would propose to just use a Python API / entry point for Python components. The component can then decide internally if it wants to use a Cython library or not.

I tried to make a basic implementation with entry point, it's a brilliant idea.

Referring to the implementation of rclcpp_component, which introduces several cmake macros in the CMakeList.txt, similarly, the python version just need to make some command override in the setup.py file. I tried this:

from setuptools.command.develop import develop
from rclpy.component import rclpy_register_component

rclpy_components = [
  'composition::Talker=my_robot_app.components:Talker',
  'composition::Listener=my_robot_app.components:Listener',
]

class develop_cmd_with_component_registration(develop):
  def run(self):
    develop.run(self)
    # It's just like a post install hook
    rclpy_register_component(rclpy_components)

setup(
  name='my_robot_app',
  entry_points={
    'rclpy_components': rclpy_components
  },
  cmdclass={'develop': develop_cmd_with_component_registration}
)

We can definitely make the customized develop command with a wrapper which is provided in rclpy.component module. The helper rclpy_register_component will call ament_index_python's functions to store the entry_points in the path {PROJECT_FOLDER}/install/my_robot_app/ament_index/resource_index/rclpy_components/composition, when the python container load node, the project's setup.bash should first be sourced or the container won't find the components. When the container got the content 'my_robot_app.components:Talker', it will load the module my_robot_app.components and read the class from it, create an instance, and run it with executor.add_node.

That's what I come up with for now, any ideas?

crystaldust avatar Jun 20 '20 10:06 crystaldust

Or, maybe it would be better to skip the command override, just provide the rclpy_register_component helper, and put it in the setup.py:

...
setup(name='my_robot_app')
# Pass a list like it does above
rclpy_register_component(rclpy_components)
# Or pass a string if the developer want to make the registration explicit
rclpy_register_component('composition::Talker=my_robot_app.component:Talker')

crystaldust avatar Jun 20 '20 10:06 crystaldust

the python version just need to make some command override in the setup.py file

A package can't rely on dependencies being available when the setup.py file is being processed. Simply because Python needs to parse that file in order to extract the declared dependencies. So you can't use any external modules like from rclpy.component import rclpy_register_component.

At least until PEP 517 is supported.

dirk-thomas avatar Jun 22 '20 19:06 dirk-thomas

Simply because Python needs to parse that file in order to extract the declared dependencies.

I'm interested in this, is there any more detail about the mechanism? I would like to look into this and see if we can do something around it. Since I see some talks about 3rd party modules in setup.py in stack overflow here, but it seems like they don't consider the dependency availability problem.

If we can't use external modules inside the setup.py for now, is there some recommended way to register tye python component? The colcon seems only support some environment script hooks, which might not be the proper place to do it. I also considered a CMakeList file, but the build type of a ros2 package can only be ONE of cmake, ament_cmake and ament_python.

crystaldust avatar Jun 23 '20 08:06 crystaldust

I'm interested in this, is there any more detail about the mechanism?

The Python documentation, setuptools and the mentioned PEP should provide the necessary context.

is there some recommended way to register tye python component?

This has already been answered a couple of times above: by using Python entry points.

dirk-thomas avatar Jun 23 '20 17:06 dirk-thomas

Thanks. Maybe I didn't make it clear. When saying 'register the python component', besides passing the entry_points parameter in setup.py, the component class's path also needs to be stored in the ament resource index path so ros2 component types command will find it. So now I can't find a good way to create the file under ament resource index path.

But for python, searching component types under the ament resource index file is not necessary, as you mentioned, the python entry point can do the job perfectly. Just let ros2 component types command search the rclpy_components entry points, we can refer to the actual component classes. It will also reduce complexity. In ros2cli component verb, we just need to add a different logic to search Python components, which is different from searching C++ components. Does it make sense?

crystaldust avatar Jun 24 '20 00:06 crystaldust

the component class's path also needs to be stored in the ament resource index path so ros2 component types command will find it.

You can use the ament resource index if you want to (or you could just use the entry points to look up available components). Using the index is the more generic / language independent approach - entry points are Python-specific.

I can't find a good way to create the file under ament resource index path.

Installing a file from the setup.py logic is suffcient to add information to the ament resource index: see e.g. https://github.com/ament/ament_index/blob/e58791231898d575a48455f23f7980820a9356b7/ament_index_python/setup.py#L16-L17 which registers the package name.

Just let ros2 component types command search the rclpy_components entry points, we can refer to the actual component classes.

Yes, that would be fine. Storing the actual component class directly or together with some metadata would be a design decision to figure out. Often you want a way to determine what version of the interface contract a component implements / supports to allow iterating it i the future.

In ros2cli component verb, we just need to add a different logic to search Python components, which is different from searching C++ components.

Yes. Beside that I think the component container needs to expose an API what type of components supports (as mentioned before).

dirk-thomas avatar Jun 24 '20 04:06 dirk-thomas

Installing a file from the setup.py logic is sufficient to add information to the ament resource index: see e.g. https://github.com/ament/ament_index/blob/e58791231898d575a48455f23f7980820a9356b7/ament_index_python/setup.py#L16-L17 which registers the package name.

That's a good approach, the only difference is to generate the file by code or user. By using data_files, develper will maintain the content manually, and a little extra work will be considered: the content is composition::Talker;my_components.Talker while the entry point value is composition::Talker=my_components.Talker.

That would introduce a little complexity, especially compared with just using the entry point. We have to choose between complex/generic and simple/specific, how do you think on this point?

crystaldust avatar Jun 24 '20 05:06 crystaldust

From tooling or framework perspective, we should leave the simplicity to developers outside and solve the complexity inside, so providing Python Entry Point should be a preferable way. But after looking into deeper of the code, the ros2cli component verb relies on ros2 pkg verb's APIs:

https://github.com/ros2/ros2cli/blob/dcc8f5d19f27e684af14815c272d9f77ede7ac87/ros2component/ros2component/verb/types.py#L20-L21

Which will also call ament index APIs. So adding an extra Python Entry Point loading requires a greater change for the whole cli architecture.

However, if we require developers to maintain the files in data_files, according to the design of package resource indexer, and the component mechanism, we will need two files at the locations:

<prefix>
    `-- share
        `-- ament_index
            `-- resource_index
                `-- packages
                    `-- my_composition # empty file
                `-- rclpy_components
                    `-- my_composition # with content to refer the python classes

So we need two files with the same name but different content, that's pretty confusing for the developers. I would come up with the solution to store two files with different names my_composition_empty and my_composition, then specify the different paths to install them.

But according to the setupscript docs installing additional files, setup.py asks to specify (directory, installed_files) in data_files parameters, this would make maintaining the files more complex. Developers will have to store the two my_composition files in different local paths, well, maybe we are asking too much for the developers to do.

So it becomes even harder to decide, maybe we need a more thorough discussion on this?

crystaldust avatar Jun 29 '20 04:06 crystaldust

The question is what is more efficient and scales better. I don't think requiring to load entry points when trying to only collect available components would be the right approach. With the number of packages and component increasing I would expect the time to grow until this would be a problem. So either the information can be gathered from the entry point alone (without the need to load them) or from the ament resource index.

dirk-thomas avatar Jun 30 '20 21:06 dirk-thomas

Agree, to fetch component info through entry point API, just call importlib.metadata.entry_points and pass 'rclpy_components' as group name. This should just search the python paths and get the refs to EntryPoint objects(no loading until EntryPoint object's load method is called): https://github.com/crystaldust/ros2cli/blob/994670497f32728e971e95fd8eeb12dc3864bd16/ros2component/ros2component/api/component_loaders.py#L75

Then I would prefer to ask developer to install the empty component file to <prefix>/share/ament_index/resource_index/packages, so the argcomplete and get_package_names API would be supported. And when we try to instantiate the component, use the entry point API to load the real classes.

So developers would do this:

  • specify the 'rclpy_components' entry points in setup.py(this is required no matter what)
  • install the empty component files to amend resource index folder, keep the file name same with entry point name

This doesn't look that complex as above, acceptable for me as a developer who uses py composition.

crystaldust avatar Jul 01 '20 15:07 crystaldust

I've made an example according to the solution above: https://github.com/crystaldust/rclpy_composition_example/tree/composition-api

Please try and see if it makes sense :)

crystaldust avatar Jul 03 '20 00:07 crystaldust

The direction looks good to me.

Without actual PRs for the implemented code it is difficult to comment on specifics though. E.g. the branches contain commented code, unrelated changes, etc. which should be cleaned up before making pull requests. Also instead of hard coding the possible extension types and their loader I would suggest to make it extensible too, e.g. by making the component type an entry point itself.

dirk-thomas avatar Jul 06 '20 18:07 dirk-thomas

Making the extension types and loaders entry points is a good idea! I'll restructure the code and make a PR to comment on.

crystaldust avatar Jul 07 '20 02:07 crystaldust