rclpy icon indicating copy to clipboard operation
rclpy copied to clipboard

Composition API

Open crystaldust opened this issue 4 years ago • 13 comments

The composition API for rclpy. Add a new rclpy_components sub project just like what rclcpp_components does. The project provides the component manager(essentially a wrapper of Node and executor), which provides the required services described in the composition design doc for the ros2cli to load/unload/list components

Related PR: https://github.com/ros2/ros2cli/pull/554

crystaldust avatar Jul 14 '20 16:07 crystaldust

Missing features/TODO list records:

  • [ ] The supported_types service(discussed here), need a discussion on where to put this service, put it in the composition_interfaces package, or just the rclpy package.
  • [x] Need some essential test cases

crystaldust avatar Jul 14 '20 16:07 crystaldust

Thanks for the review! Just get myself back to work after a LONG business travel. I'll look into the code and make some fixes/improvments

crystaldust avatar Aug 30 '20 10:08 crystaldust

@jacobperron Hi, I've made some changes following the reviews, make a list here for better track:

  • [x] Remove the unnecessary spin in ComponentManager's __init__ method
  • [x] Make up the meta data in package.xml and setup.py, like author info, desc, version, license etc
  • [x] Surround spin with try-catch, instead of creating a shadow _main function
  • [x] Call component manager's destroy node when exiting program
  • [x] Use the node's logger instead of creating a new global one
  • [x] Use f string for logging
  • [x] Return error message in response instead of logging it
  • [x] Use request.id(type integer) as ComponentManager's components member as key

Also with the basic unit/integrated test code added. Let's first make this PR merged and then move to next PRs related to ros2cli and examples

crystaldust avatar Oct 19 '20 10:10 crystaldust

Any update on this? We have the need for launching python components and having that integrated would be very neat.

buschbapti avatar Jul 21 '21 14:07 buschbapti

@buschbapti Thanks for the attention. I'll make a recheck on this and see if it can be merged ASAP.

crystaldust avatar Jul 27 '21 01:07 crystaldust

I copied your rclpy_components to make this work in our application. Everything runs smoothly but I needed to change the get_fully_qualified_node_name line. Either this function has been deleted in the latest version (galactic) or is not integrated yet.

buschbapti avatar Jul 27 '21 07:07 buschbapti

This seems to be an important feature and it is almost done! Any update on this?

alpylmz avatar Apr 29 '22 15:04 alpylmz

+1

SteveMacenski avatar May 24 '22 00:05 SteveMacenski

@crystaldust Are you still working on this? If not I'd be take over and help push this PR through.

ihasdapie avatar Jun 02 '22 18:06 ihasdapie

Hello any updates on this? Will it be integrated?

MathieuxHugo avatar Jul 19 '23 14:07 MathieuxHugo

@mjcarroll @sloretz @crystaldust just trying to bump this again, we've been using it for 2 years and works as expected

domire8 avatar Feb 16 '24 11:02 domire8

Since it had been a while I rebased this onto rolling and made some changes

  • Resolved all flake8 linter issues
  • Updated setup.py and package.xml to list Zhen Ju as the author with @adityapande-1995 and I as the maintainers
  • Rewrote tests to use pytest and made each test independent. Previously the tests were dependent and relied on the tests being executed in alphabetical order, but that must have changed (or one of my installed pytest plugins changed it) because the tests weren't passing on my machine.
  • Fixed code that assumed .keys() and .values() from a dict would always be the same order to use .items()
  • Made the component_manager use the LoadNode.package_name parameter. The entrypoint specification is still package_name::PluginName, but now the component manager looks for an entry point named that way instead of using only plugin_name.
  • Removed tests_components package marker file and put the test node into the rclpy_components.test subpackage
  • Renamed the test node and file so that pytest didn't think it was a test
  • Added a check to make sure the fully qualified name of the loaded node is unique in the component manager to satisfy requirement in this design doc
  • Protected component_manager from crashing in case the entrypoint or class itself fail to load or be instantiated
  • Made node name and namespace get passed in via remap arguments instead of direct arguments to match rclcpp. The remap rules get evaluated with a different priority from the passed in node name so this makes that behavior consistent between the two
  • Added more tests (failure handling cases + making sure name and namespace remapping worked)
  • Use non-deprecated entry_points API
  • Removed importlib_metata as we don't need to support Python less than 3.8 on rolling
  • Use non-deprecated setup.cfg types.

And with that, this PR LGTM, but I've made so many changes I think it needs another reviewer.

sloretz avatar Mar 31 '24 16:03 sloretz

CI (repos file build: --packages-up-to rclpy_components test: --packages-select rclpy_components)

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

EDIT: the tests on Windows got stuck. I don't have access to a Windows machine at the moment to debug this :(

sloretz avatar Mar 31 '24 16:03 sloretz