Skip to content

Convert ActionServer to use C++ Classes#766

Merged
ahcorde merged 5 commits intopost_galactic_freezefrom
ahcorde/pybind11-actionserver
Apr 14, 2021
Merged

Convert ActionServer to use C++ Classes#766
ahcorde merged 5 commits intopost_galactic_freezefrom
ahcorde/pybind11-actionserver

Conversation

@ahcorde
Copy link
Copy Markdown
Contributor

@ahcorde ahcorde commented Apr 9, 2021

Related to #665

This converts the Action Server functions to a C++ class.

Signed-off-by: ahcorde ahcorde@gmail.com

Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde added the enhancement New feature or request label Apr 9, 2021
@ahcorde ahcorde requested a review from hidmic April 9, 2021 09:31
@ahcorde ahcorde self-assigned this Apr 9, 2021
@ahcorde
Copy link
Copy Markdown
Contributor Author

ahcorde commented Apr 9, 2021

I created this as a Draft PR because I have some issues.

Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde force-pushed the ahcorde/pybind11-actionserver branch from a9b5414 to c726671 Compare April 9, 2021 12:17
@ahcorde
Copy link
Copy Markdown
Contributor Author

ahcorde commented Apr 9, 2021

ActionServer tests are fixed, but I still have the issue with test_action_client

ahcorde added 2 commits April 12, 2021 17:26
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde marked this pull request as ready for review April 12, 2021 17:37
@ahcorde
Copy link
Copy Markdown
Contributor Author

ahcorde commented Apr 12, 2021

@hidmic This is ready for review

@ahcorde ahcorde mentioned this pull request Apr 13, 2021
34 tasks
Copy link
Copy Markdown
Contributor

@hidmic hidmic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM, pending green CI

Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Copy link
Copy Markdown
Contributor Author

ahcorde commented Apr 14, 2021

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@ahcorde ahcorde merged commit 9e4ba03 into post_galactic_freeze Apr 14, 2021
@delete-merged-branch delete-merged-branch bot deleted the ahcorde/pybind11-actionserver branch April 14, 2021 15:22
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 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>
sloretz pushed a commit that referenced this pull request Jun 23, 2021
* 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>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
sloretz pushed a commit that referenced this pull request Aug 11, 2021
* 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>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants