Convert Guardcondition to use C++ classes#772
Merged
ahcorde merged 2 commits intopost_galactic_freezefrom Apr 16, 2021
Merged
Conversation
Signed-off-by: ahcorde <ahcorde@gmail.com>
34 tasks
hidmic
approved these changes
Apr 15, 2021
rclpy/src/rclpy/guard_condition.hpp
Outdated
| _rclpy_destroy_guard_condition(void * p) | ||
| { | ||
| (void)p; | ||
| // Empty destructor, the class should take care of the lifecycle. |
Contributor
There was a problem hiding this comment.
@ahcorde what ensures this object won't get trashed before the capsule does?
Contributor
Author
There was a problem hiding this comment.
The Python class SignalHandlerGuardCondition(GuardCondition) is the only one in the code making use of this reference. In the destructor I can see how the capsule is destroyed first and then the object.
Lines 38 to 42 in 7dff0ee
or should we include some kind of logic ?
Contributor
There was a problem hiding this comment.
I see. This is arguably a (temporary?) implementation detail. No need to change it.
Signed-off-by: ahcorde <ahcorde@gmail.com>
Contributor
Author
|
hidmic
approved these changes
Apr 16, 2021
sloretz
added a commit
that referenced
this pull request
Apr 28, 2021
* Convert Publisher and Subscription to use C++ Classes (#756) * Convert Publisher and Subscription to C++ Classes Signed-off-by: ahcorde <ahcorde@gmail.com> * Fixed destructor issues Signed-off-by: ahcorde <ahcorde@gmail.com> * [pub/sub 756 addition] use std::shared_ptr holder type and shared_from_this (#757) * Use std::shared_ptr holder type Signed-off-by: Shane Loretz <sloretz@openrobotics.org> * Use shared_from_this for pub/sub Signed-off-by: Shane Loretz <sloretz@openrobotics.org> * Make Timer keep Clock wrapper Signed-off-by: Shane Loretz <sloretz@openrobotics.org> * Fixed publisher and subscription destructors Signed-off-by: ahcorde <ahcorde@gmail.com> * Added feedback Signed-off-by: ahcorde <ahcorde@gmail.com> * Fixed docs Signed-off-by: ahcorde <ahcorde@gmail.com> Co-authored-by: Shane Loretz <sloretz@openrobotics.org> * Use py::class_ for rcl_action_goal_handle_t (#751) * Class object Signed-off-by: Greg Balke <greg@openrobotics.org> * Python adjustments Signed-off-by: Greg Balke <greg@openrobotics.org> * Adjustments to server and enum Signed-off-by: Greg Balke <greg@openrobotics.org> * Adjust destructor calls Signed-off-by: Greg Balke <greg@openrobotics.org> * Clear out destructor Signed-off-by: Greg Balke <greg@openrobotics.org> * Do what rclcpp did Signed-off-by: Greg Balke <greg@openrobotics.org> * Remove iostream (used for debugging) Signed-off-by: Greg Balke <greg@openrobotics.org> * Matching upstream changes Signed-off-by: Greg Balke <greg@openrobotics.org> * Convert ActionClient to use C++ classes (#759) * Convert ActionClient to use C++ classes Signed-off-by: ahcorde <ahcorde@gmail.com> * Added feedback Signed-off-by: ahcorde <ahcorde@gmail.com> * Added convert_from_py to utils Signed-off-by: ahcorde <ahcorde@gmail.com> * improved docs Signed-off-by: ahcorde <ahcorde@gmail.com> * nits to adapt the code to pybind11 Signed-off-by: ahcorde <ahcorde@gmail.com> * throw a exception in convert_from_py Signed-off-by: ahcorde <ahcorde@gmail.com> * Fixed docs Signed-off-by: ahcorde <ahcorde@gmail.com> * Convert ActionServer to use C++ Classes (#766) * Convert ActionServer to use C++ Classes Signed-off-by: ahcorde <ahcorde@gmail.com> * Fixed Test_action_server Signed-off-by: ahcorde <ahcorde@gmail.com> * Improved docs Signed-off-by: ahcorde <ahcorde@gmail.com> * Improved docs and minor fixes Signed-off-by: ahcorde <ahcorde@gmail.com> * Convert WaitSet to use C++ Classes (#769) * Convert WaitSet to use C++ Classes Signed-off-by: ahcorde <ahcorde@gmail.com> * added feedback Signed-off-by: ahcorde <ahcorde@gmail.com> * Removed unused structs (#770) Signed-off-by: ahcorde <ahcorde@gmail.com> * Convert Guardcondition to use C++ classes (#772) * Convert Guardcondition to use C++ classes Signed-off-by: ahcorde <ahcorde@gmail.com> * make destroy guard condition capsule function private Signed-off-by: ahcorde <ahcorde@gmail.com> * Pybind11 action goal handle nitpicks (#767) * Add GoalEvent back to module Signed-off-by: Shane Loretz <sloretz@osrfoundation.org> * self._goal -> self._goal_handle Signed-off-by: Shane Loretz <sloretz@osrfoundation.org> * Remove unused API Signed-off-by: Shane Loretz <sloretz@osrfoundation.org> Co-authored-by: ahcorde <ahcorde@gmail.com> * Pybind11 actionserver nitpicks and docblock improvements (#774) Signed-off-by: ahcorde <ahcorde@gmail.com> * Convert Node and Context to use C++ Classes (#771) * convert Node and Context to use C++ Classes Signed-off-by: ahcorde <ahcorde@gmail.com> * Added feedback Signed-off-by: ahcorde <ahcorde@gmail.com> * Added feedback Signed-off-by: ahcorde <ahcorde@gmail.com> * Implement solution 1 Signed-off-by: ahcorde <ahcorde@gmail.com> * Prevent double early destruction of destroyable This along with an extra call to `destroy_node()` in `Node.__del__` was causing a segfault in the test_publisher test. The problem is `destroy_when_not_in_use()` was being called twice, and the second time is now a problem because rcl_ptrs_ is dereferenced in the qos_event destroy() method. Signed-off-by: Shane Loretz <sloretz@osrfoundation.org> * Remove unnecessary Node.__del__ This should not be required. If it is, it means there's a bug that shows when the Node is garbage collected. Signed-off-by: Shane Loretz <sloretz@osrfoundation.org> * Fix order of pointers in RclPtrs structs The order of the pointers in the RclPtrs structs is important. Things at the bottom are destroyed first, so children must be placed underneath the parent's that must be kept alive. Signed-off-by: Shane Loretz <sloretz@osrfoundation.org> * Intentionally copy rclpy:: objects instead of having RclPtrs structs (#773) * Rough pass converting everything Signed-off-by: Shane Loretz <sloretz@osrfoundation.org> * Fixes to get copy destruction order working Signed-off-by: Shane Loretz <sloretz@osrfoundation.org> * Fix qos_event test flakiness Signed-off-by: Shane Loretz <sloretz@osrfoundation.org> Signed-off-by: ahcorde <ahcorde@gmail.com> * MacOS warning Signed-off-by: ahcorde <ahcorde@gmail.com> * added feedback and removed some files Signed-off-by: ahcorde <ahcorde@gmail.com> * Create only one context otherwise throw a exception Signed-off-by: ahcorde <ahcorde@gmail.com> Co-authored-by: Shane Loretz <sloretz@osrfoundation.org> Co-authored-by: Shane Loretz <sloretz@openrobotics.org> Co-authored-by: Alejandro Hernández Cordero <alejandro@openrobotics.org> Co-authored-by: Greg Balke <11024792+gbalke@users.noreply.github.com> Co-authored-by: ahcorde <ahcorde@gmail.com>
sloretz
pushed a commit
that referenced
this pull request
Apr 28, 2021
* Convert Guardcondition to use C++ classes Signed-off-by: ahcorde <ahcorde@gmail.com> * make destroy guard condition capsule function private Signed-off-by: ahcorde <ahcorde@gmail.com>
sloretz
pushed a commit
that referenced
this pull request
Jun 23, 2021
* Convert Guardcondition to use C++ classes Signed-off-by: ahcorde <ahcorde@gmail.com> * make destroy guard condition capsule function private Signed-off-by: ahcorde <ahcorde@gmail.com> Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
sloretz
pushed a commit
that referenced
this pull request
Aug 11, 2021
* Convert Guardcondition to use C++ classes Signed-off-by: ahcorde <ahcorde@gmail.com> * make destroy guard condition capsule function private Signed-off-by: ahcorde <ahcorde@gmail.com> Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Related to #665
This converts the GuardCondition functions to a C++ class.
Signed-off-by: ahcorde ahcorde@gmail.com