-
Notifications
You must be signed in to change notification settings - Fork 315
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
Demo for callback-group-level executor concept. #302
Demo for callback-group-level executor concept. #302
Conversation
Signed-off-by: Ralph Lange <[email protected]>
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.
rclcpp/executors/cbg_executor/src/examples_rclcpp_cbg_executor/ping_node.cpp
Outdated
Show resolved
Hide resolved
rclcpp/executors/cbg_executor/src/examples_rclcpp_cbg_executor/ping_node.cpp
Outdated
Show resolved
Hide resolved
rclcpp/executors/cbg_executor/include/examples_rclcpp_cbg_executor/ping_node.hpp
Outdated
Show resolved
Hide resolved
rclcpp/executors/cbg_executor/src/examples_rclcpp_cbg_executor/ping_node.cpp
Outdated
Show resolved
Hide resolved
rclcpp/executors/cbg_executor/src/examples_rclcpp_cbg_executor/ping_node.cpp
Outdated
Show resolved
Hide resolved
rclcpp/executors/cbg_executor/src/examples_rclcpp_cbg_executor/pong_node.cpp
Outdated
Show resolved
Hide resolved
rclcpp/executors/cbg_executor/src/examples_rclcpp_cbg_executor/pong_node.cpp
Outdated
Show resolved
Hide resolved
Co-authored-by: Karsten Knese <[email protected]> Signed-off-by: Ralph Lange <[email protected]>
Signed-off-by: Ralph Lange <[email protected]>
Hi @Karsten1987, I fixed the warnings caused by the line lengths in the CMakeLists.txt. Could you please re-trigger the job on ci.ros2.org? |
Converted it to a draft until macOS support is implemented. |
Signed-off-by: Ralph Lange <[email protected]>
Signed-off-by: Ralph Lange <[email protected]>
Hi @Karsten1987, I tried to add support for macOS - without having any practical C++ experience on this platform. Could you please test and my implementation? |
Signed-off-by: Ralph Lange <[email protected]>
Signed-off-by: Karsten Knese <[email protected]>
…nt64 in logging. Signed-off-by: Ralph Lange <[email protected]>
Signed-off-by: Karsten Knese <[email protected]>
@ralph-lange the CPU affinity indeed required the threads to the suspended while being configured. The condition variable seemed to have work according to the following output:
I could do this without requiring root rights, so maybe that'll also reduce the need for it on Linux. I further went ahead and refactored the nodes from being compile dependent on the compiler options ( CI: |
Signed-off-by: Karsten Knese <[email protected]>
Signed-off-by: Karsten Knese <[email protected]>
Great that you got the core pinning for macOS working. However, we have to use separate mutexes for each executor instance. Otherwise, with the current lock mechanism, only one of the two executor instances will eventually run. I'll fix this in the evening. |
…iguration. Signed-off-by: Ralph Lange <[email protected]>
Signed-off-by: Karsten Knese <[email protected]>
Signed-off-by: Ralph Lange <[email protected]>
Signed-off-by: Ralph Lange <[email protected]>
Signed-off-by: Ralph Lange <[email protected]>
@Karsten1987, could you please re-trigger the job on ci.ros2.org? |
Signed-off-by: Ralph Lange <[email protected]>
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.
this looks good to me with green CI.
However, @clalancette if you could give it a pair of eyes as well. I've ended up contributing to the PR quite a bit.
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.
Seems reasonable enough to me. @Karsten1987 I'll let you merge.
Moved PR from ros2/demos to this repository as discussed in ros2/demos#485.