-
Notifications
You must be signed in to change notification settings - Fork 435
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
Pass callbacks as universal references #147
Conversation
Again, I appreciate it if this could be deferred until #140 is merged. |
50f2f2c
to
bdd1fd0
Compare
Sure, no problem, it's not marked for review yet, though. |
I've cancelled the CI jobs. |
bdd1fd0
to
8c47f61
Compare
The Windows build will fail since you can only build with a single rmw impl. enabled. |
I triggered two more jobs, one for OpenSplice and another for Connext: http://ci.ros2.org/job/ros2_batch_ci_windows/590/ |
The "Connext" job will fail too since you must select a single rmw implementation. |
http://ci.ros2.org/job/ros2_batch_ci_windows/591/ for Connext static |
Can you give a summary of why a universal reference is needed, instead of the current behavior, or instead of an lvalue reference? |
This way we avoid extra copies and passing normal references breaks lambdas. |
so you can't pass a lambda to a function by lvalue reference? I couldn't find any documentation on the matter. I recall you made this PR because of that problem: #143 was it because somewhere the lambdas were being passed as temporaries? Then I can see why perfect forwarding would be needed. Anyway, regardless of the details, the change is a good idea; I'm just trying to learn more about C++11... |
Yes, the result of a lambda expression is a temporary, which can't be bound to lvalue references, hence the need for universal ones. |
In general, any function that takes a template argument should use a universal reference, unless we really need copies. |
+1 |
Pass callbacks as universal references
* Check if goal was sent through IPC before send responses * Add intra_process_action_server_is_available API to intra-process Client --------- Co-authored-by: Mauro Passerino <mpasserino@irobot.com>
* Check if goal was sent through IPC before send responses * Add intra_process_action_server_is_available API to intra-process Client --------- Co-authored-by: Mauro Passerino <mpasserino@irobot.com>
* Check if goal was sent through IPC before send responses * Add intra_process_action_server_is_available API to intra-process Client --------- Co-authored-by: Mauro Passerino <mpasserino@irobot.com>
* Fixes for intra-process actions (ros2#144) * Fixes for intra-process Actions * Fixes for Clang builds * Fix deadlock * Server to store results until client requests them * Fix feedback/result data race See ros2#2451 * Add missing mutex * Check return value of intra_process_action_send --------- Co-authored-by: Mauro Passerino <mpasserino@irobot.com> * Fix IPC Actions data race (ros2#147) * Check if goal was sent through IPC before send responses * Add intra_process_action_server_is_available API to intra-process Client --------- Co-authored-by: Mauro Passerino <mpasserino@irobot.com> * Fix data race in Actions: Part 2 (ros2#148) * Fix data race in Actions: Part 2 * Fix warning - copy elision --------- Co-authored-by: Mauro Passerino <mpasserino@irobot.com> * fix: Fixed race condition in action server between is_ready and take"… (ros2#2531) * fix: Fixed race condition in action server between is_ready and take" (ros2#2495) Some background information: is_ready, take_data and execute data may be called from different threads in any order. The code in the old state expected them to be called in series, without interruption. This lead to multiple race conditions, as the state of the pimpl objects was altered by the three functions in a non thread safe way. Co-authored-by: William Woodall <william@osrfoundation.org> Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com> * fix: added workaround for call to double calls to take_data This adds a workaround for a known bug in the executor in iron. Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com> --------- Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com> Co-authored-by: Janosch Machowinski <J.Machowinski@cellumation.com> Co-authored-by: William Woodall <william@osrfoundation.org> --------- Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com> Co-authored-by: Mauro Passerino <mpasserino@irobot.com> Co-authored-by: jmachowinski <jmachowinski@users.noreply.github.com> Co-authored-by: Janosch Machowinski <J.Machowinski@cellumation.com> Co-authored-by: William Woodall <william@osrfoundation.org>
This PR uses universal references for callbacks.