search

ros2 / rclcpp

rclcpp (ROS Client Library for C++)
Apache License 2.0
513 stars 412 forks source link

Use the same context for the specified node in rclcpp::spin functions #2433

Closed HansRobo closed 3 months ago

HansRobo commented 4 months ago

Currently, rclcpp::spin functions, rclcpp::spin, rclcpp::spin_some, and so on, use the default context. However they may occur errors when you use the custom context for the specified node of argument of spin functions.

In this pull-request, I set in-place executor to use context of node via executor options.

This is the only fix I found, but there may be other similar fixes.

alsora commented 4 months ago

The approach in the PR looks good to me.

There's several test failures. Can you take a look?

We should also add a unit-test to verify that the executor is using the correct context

alsora commented 3 months ago

@HansRobo FYI, the feature freeze for Jazzy will be in ~2 weeks, would you be able to add tests before that date? Otherwise this PR will need to go in the next release.

fujitatomoya commented 3 months ago

@HansRobo thanks for adding test, i have a question for test.

fujitatomoya commented 3 months ago

@alsora since you are interested in this PR. can you take a look at this? i will leave this to you.

fujitatomoya commented 3 months ago

https://build.ros2.org/job/Rpr__rclcpp__ubuntu_noble_amd64/106/ is unrelated failure.

alsora commented 3 months ago
alsora commented 3 months ago

Merging with green full CI (the github action failure is unrelated)