-
Notifications
You must be signed in to change notification settings - Fork 478
avoid adding notify waitable twice to events-executor collection #2564
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
Merged
+64
−6
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…tion Signed-off-by: Alberto Soragna <[email protected]>
mjcarroll
approved these changes
Jun 20, 2024
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 with green CI
rclcpp/src/rclcpp/experimental/executors/events_executor/events_executor.cpp
Outdated
Show resolved
Hide resolved
Signed-off-by: Alberto Soragna <[email protected]>
fujitatomoya
approved these changes
Jun 22, 2024
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 with green CI
wjwwood
approved these changes
Jun 25, 2024
|
Merging with green CI. |
apojomovsky
pushed a commit
to apojomovsky/rclcpp
that referenced
this pull request
Jul 8, 2024
…2#2564) * avoid adding notify waitable twice to events-executor entities collection Signed-off-by: Alberto Soragna <[email protected]> * remove redundant mutex lock Signed-off-by: Alberto Soragna <[email protected]> --------- Signed-off-by: Alberto Soragna <[email protected]>
skyegalaxy
pushed a commit
to irobot-ros/rclcpp
that referenced
this pull request
Mar 29, 2025
…2#2564) * avoid adding notify waitable twice to events-executor entities collection Signed-off-by: Alberto Soragna <[email protected]> * remove redundant mutex lock Signed-off-by: Alberto Soragna <[email protected]> --------- Signed-off-by: Alberto Soragna <[email protected]> (cherry picked from commit f27bdbf)
skyegalaxy
pushed a commit
to irobot-ros/rclcpp
that referenced
this pull request
Apr 3, 2025
…2#2564) * avoid adding notify waitable twice to events-executor entities collection Signed-off-by: Alberto Soragna <[email protected]> * remove redundant mutex lock Signed-off-by: Alberto Soragna <[email protected]> --------- Signed-off-by: Alberto Soragna <[email protected]> (cherry picked from commit f27bdbf)
skyegalaxy
pushed a commit
to irobot-ros/rclcpp
that referenced
this pull request
May 14, 2025
…2#2564) * avoid adding notify waitable twice to events-executor entities collection Signed-off-by: Alberto Soragna <[email protected]> * remove redundant mutex lock Signed-off-by: Alberto Soragna <[email protected]> --------- Signed-off-by: Alberto Soragna <[email protected]> (cherry picked from commit f27bdbf)
skyegalaxy
pushed a commit
to irobot-ros/rclcpp
that referenced
this pull request
May 19, 2025
…2#2564) * avoid adding notify waitable twice to events-executor entities collection Signed-off-by: Alberto Soragna <[email protected]> * remove redundant mutex lock Signed-off-by: Alberto Soragna <[email protected]> --------- Signed-off-by: Alberto Soragna <[email protected]> (cherry picked from commit f27bdbf)
skyegalaxy
added a commit
to irobot-ros/rclcpp
that referenced
this pull request
May 20, 2025
* Add logs on failed take response/request (#107) * Ignore local endpoints (#131) * Refs #18846: PoC ignore local endpoints: extend RCLCPP API Signed-off-by: JLBuenoLopez-eProsima <[email protected]> * Refs #18846: PoC ignore local endpoints: modify RCLCPP publish logic Signed-off-by: JLBuenoLopez-eProsima <[email protected]> --------- Signed-off-by: JLBuenoLopez-eProsima <[email protected]> Co-authored-by: JLBuenoLopez-eProsima <[email protected]> (cherry picked from commit 106c03a) style * Support intra-process communication: Clients & Services (ros2#1847) Signed-off-by: Mauro Passerino <[email protected]> (cherry picked from commit 58d2a04) Remove redundant `add_subscription` cherry pick artifact use const ref instead of ptr, add missing capacity fn more style * Jazzy recreation of "Add action client/server IPC support" (aeacde9) * add override attribute to some ipc methods Signed-off-by: Alberto Soragna <[email protected]> * clear intra-process manager on client/server destructors (#94) (cherry picked from commit 378223d) * move ipc lock to appropriate position in client.hpp (cherry picked from commit 9de603e) * Actions: Use ipc_setting = rclcpp::IntraProcessSetting::NodeDefault (#133) Co-authored-by: Mauro Passerino <[email protected]> * Allow for deferred responses with ipc (#135) * allow for deferred responses with ipc * fix send response * fix use member service ipc process * add map to store CallbackInfoVariant * add send_response to base class, set function ptr to get handle * move service intra process outside base * copy response into shared pointer for ipc * add typename for service template * remove ref signature * try without std::ref * try without ref wrapper * try emplace * make pair with variant * some cleanup * erase callback info if client invalid * add post_init_setup for services * fix extra comma * add post init setup after lifecycle node services * add documentation for ServiceIntraProcess template change * use weak ptr to service in ServiceIntraProcess * move check for valid service handle to beginning of function * add comment for callback info map (cherry picked from commit aa95a48) * check request header for intraprocess (#139) * check request header for intraprocess * set request header intraprocess to false in execute_service (cherry picked from commit a617f93) * Include namespaces in service names (#140) * Include node namespace in IPC Action service name * Include node namespace in IPC Client/Service service name --------- Co-authored-by: Mauro Passerino <[email protected]> (cherry picked from commit f5b2001) remove whitespace in service.hpp * Fix mutltiple client requests (#142) * store map of unique request id to client id and callback info pair * fix map end check * fix undefined reference * remove unnecessary request id erase, remove/fix unique id comment * improve unique id comment (cherry picked from commit b865383) * RECREATION OF Fixes for intra-process actions (#144) action client / server ipc decrustification whitespace / line length / uncrustify - service_intra_process const correctness * add logs and minor fixes (#146) * add logs and minor fixes Signed-off-by: Alberto Soragna <[email protected]> * use >0 rather than ==1 in comparison Signed-off-by: Alberto Soragna <[email protected]> --------- Signed-off-by: Alberto Soragna <[email protected]> * correct template syntax Signed-off-by: Alberto Soragna <[email protected]> (cherry picked from commit d4dd4e4) * avoid adding notify waitable twice to events-executor collection (ros2#2564) * avoid adding notify waitable twice to events-executor entities collection Signed-off-by: Alberto Soragna <[email protected]> * remove redundant mutex lock Signed-off-by: Alberto Soragna <[email protected]> --------- Signed-off-by: Alberto Soragna <[email protected]> (cherry picked from commit f27bdbf) * Fix bug in timers lifecycle for events executor (ros2#2586) * Remove expired timers before updating the collection Signed-off-by: Alexis Pojomovsky <[email protected]> * Add regression test for reinitialized timers bug Signed-off-by: Alexis Pojomovsky <[email protected]> * Add missing includes Signed-off-by: Alexis Pojomovsky <[email protected]> * Relocate test under the executors directory Signed-off-by: Alexis Pojomovsky <[email protected]> * Extend test to run with all supported executors Signed-off-by: Alexis Pojomovsky <[email protected]> * Adjust comment in fix to make it more generic Signed-off-by: Alexis Pojomovsky <[email protected]> * Apply ament clang format to test Signed-off-by: Alexis Pojomovsky <[email protected]> * Fix uncrustify findings Signed-off-by: Alexis Pojomovsky <[email protected]> --------- Signed-off-by: Alexis Pojomovsky <[email protected]> Co-authored-by: Alexis Pojomovsky <[email protected]> (cherry picked from commit 9ef9646) * Bring lock_free_events_queue to rclcpp from the events_executor repo (#149) Co-authored-by: Alexis Pojomovsky <[email protected]> (cherry picked from commit 30050c1) exclude lock free queue from linting actually lint lock_free_events_queue lock_free_events_queue copyright whitespace / line length / uncrustify - lock_free_events_queue * Add test_actions (#150) Co-authored-by: Alexis Pojomovsky <[email protected]> (cherry picked from commit aef928d) * Jazzy - Action IPC Fixes (See 7a51f00) whitespace / line length / uncrustify - rclcpp_action client whitespace / line length / uncrustify - rclcpp_action server fix duplicate member definitions in rclcpp_action server.hpp fixes for IPC action server delete log whitespace / line length / uncrustify - rclcpp action server again minor action client fixes whitespace / line length / uncrustify - rclcpp action client again * Always publish inter-process on TRANSIENT_LOCAL pubs (#152) * Mauro/irobot iron fixes (#155) whitespace / line length / uncrustify - publisher proper inter_process_publish_needed whitespace / line length / uncrustify - rclcpp publisher * Add test_actions & test_transient_local (#157) Co-authored-by: Mauro Passerino <[email protected]> (cherry picked from commit cf182e0) whitespace / line length / uncrustify - test_actions and test_transient_local whitespace / line length / uncrustify - test actions and transient_local again remove redundant test_actions * Call service post_init_setup in test_service.cpp --------- Signed-off-by: Alberto Soragna <[email protected]> Signed-off-by: Alexis Pojomovsky <[email protected]> Co-authored-by: Mauro Passerino <[email protected]> Co-authored-by: mauropasse <[email protected]> Co-authored-by: Alberto Soragna <[email protected]> Co-authored-by: Jeffery Hsu <[email protected]> Co-authored-by: bpwilcox <[email protected]> Co-authored-by: Alexis Pojomovsky <[email protected]> Co-authored-by: Alexis Pojomovsky <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes a bug noticed by @wjwwood as part of his PR here
We don't need to add the notify waitable to the current entities collection list there, because it has already been added in the constructor.
The PR includes a unit-test that would detect bugs if the constructor insertion was removed.