Skip to content
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

Add support for Cartesian paths #8

Merged
merged 3 commits into from
Apr 8, 2022
Merged

Conversation

Schnilz
Copy link
Contributor

@Schnilz Schnilz commented Apr 4, 2022

Hi,
thank you for writing this, especially with the good examples :)
I missed the cartesian planner, so i added it here.
I don't know why the moveit2 interface is so weird, but the code works (and calls the same service as the GUI in rviz does i think).

closes #6

@AndrejOrsula
Copy link
Owner

Hello @Schnilz,
Great! Thank you for your contribution.
I will test, review and merge it in the coming days. 😄


I don't know why the moveit2 interface is so weird, but the code works (and calls the same service as the GUI in rviz does i think).

I have to agree that the generic ROS 2 interface of MoveIt 2 can be a bit difficult to follow. Maybe there is a much simpler way, but I have not found it yet. The code in this repo can be quite ugly, especially when trying to reduce the number of memory allocation calls and create only clients that are actually used.

I also noticed a difference in performance between services and actions that achieve the same goal. That's why both options are currently available, despite adding yet another level of ugliness to the code.

@AndrejOrsula AndrejOrsula self-requested a review April 4, 2022 15:35
Copy link
Owner

@AndrejOrsula AndrejOrsula left a comment

Choose a reason for hiding this comment

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

Thank you again for this PR! It works and looks good to me. 😄 I added some cosmetic comments.

Could you run the pre-commit hooks (it will fix formatting and sort imports)? There is a small setup script under .git_hooks/setup.bash that would install it on your system and setup the repository. If not, I will run them before merge.

pymoveit2/moveit2.py Outdated Show resolved Hide resolved
pymoveit2/moveit2.py Outdated Show resolved Hide resolved
pymoveit2/moveit2.py Outdated Show resolved Hide resolved
pymoveit2/moveit2.py Outdated Show resolved Hide resolved
pymoveit2/moveit2.py Outdated Show resolved Hide resolved
pymoveit2/moveit2.py Outdated Show resolved Hide resolved
pymoveit2/moveit2.py Show resolved Hide resolved
@AndrejOrsula AndrejOrsula changed the base branch from master to devel April 7, 2022 15:03
@AndrejOrsula AndrejOrsula changed the title added cartesian planning Add support for Cartesian paths Apr 8, 2022
@AndrejOrsula AndrejOrsula merged commit f2574a8 into AndrejOrsula:devel Apr 8, 2022
@Schnilz Schnilz deleted the master branch April 29, 2022 09:50
AndrejOrsula added a commit that referenced this pull request May 2, 2022
* Add support for Cartesian paths (#8)

* Add support for collision objects (#9)

* Remove trimesh from ROS 2 dependencies

Signed-off-by: Andrej Orsula <orsula.andrej@gmail.com>

* Expose cartesian planning as a parameter in the pose goal example

Signed-off-by: Andrej Orsula <orsula.andrej@gmail.com>

* Add default test mesh for collision object example

Signed-off-by: Andrej Orsula <orsula.andrej@gmail.com>

* Move public functions before private

Signed-off-by: Andrej Orsula <orsula.andrej@gmail.com>

Co-authored-by: Nils Schulte <47043622+Schnilz@users.noreply.github.com>
AndrejOrsula pushed a commit that referenced this pull request Aug 17, 2023
Merge upstream into this repo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cartesian Path for reaching a Pose Goal
2 participants