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.
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
Use the same context for the specified node in rclcpp::spin functions #2433
Conversation
Signed-off-by: GitHub <noreply@github.com>
7cf56a9
to
d56c99f
Compare
The approach in the PR looks good to me. There's several test failures. We should also add a unit-test to verify that the executor is using the correct context |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, test is ideal.
@HansRobo FYI, the feature freeze for Jazzy will be in ~2 weeks, would you be able to add tests before that date? |
Signed-off-by: Kotaro Yoshimoto <pythagora.yoshimoto@gmail.com>
Signed-off-by: Kotaro Yoshimoto <pythagora.yoshimoto@gmail.com>
@HansRobo thanks for adding test, i have a question for test. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@alsora since you are interested in this PR. can you take a look at this? i will leave this to you. |
https://build.ros2.org/job/Rpr__rclcpp__ubuntu_noble_amd64/106/ is unrelated failure. |
Merging with green full CI (the github action failure is unrelated) |
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.