rclpy
rclpy copied to clipboard
Composition API
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
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
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
@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
Any update on this? We have the need for launching python components and having that integrated would be very neat.
@buschbapti Thanks for the attention. I'll make a recheck on this and see if it can be merged ASAP.
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.
This seems to be an important feature and it is almost done! Any update on this?
+1
@crystaldust Are you still working on this? If not I'd be take over and help push this PR through.
Hello any updates on this? Will it be integrated?
@mjcarroll @sloretz @crystaldust just trying to bump this again, we've been using it for 2 years and works as expected
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 theLoadNode.package_name
parameter. The entrypoint specification is stillpackage_name::PluginName
, but now the component manager looks for an entry point named that way instead of using onlyplugin_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.
CI (repos file build: --packages-up-to rclpy_components
test: --packages-select rclpy_components
)
EDIT: the tests on Windows got stuck. I don't have access to a Windows machine at the moment to debug this :(
Any chance this could get merged soon? I'd also be interested in a backport at least through Humble.
Ditto to the value of a humble
backport soon. Thanks all for your hard work!