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.

Sign up for GitHub

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

Jump to bottom

Parse <collision_default> and <enable_collisions> tags #97

Merged
merged 6 commits into from
Jan 17, 2022
Merged

Parse <collision_default> and <enable_collisions> tags #97

merged 6 commits into from
Jan 17, 2022

Conversation

rhaschke
Copy link
Contributor

@rhaschke rhaschke commented Nov 2, 2021 •
edited by v4hn
Loading

This PR augments the SRDF XML with tags <collision_default> and <enable_collisions> to allow specifying ACM defaults for individual links. For example,

<collision_default link="panda_hand_coarse" allow="ALWAYS" />
<enable_collisions link1="panda_hand_coarse" link2="panda_link0" />

UPDATE: See below for the final xml tags.

will disable collision checking for panda_hand_coarse with all other links (particularly newly introduced objects) but enable collision between panda_hand_coarse and panda_link0.

This was referenced Nov 2, 2021
Merged
Closed
@rhaschke rhaschke force-pushed the rework-acm-defaults branch from 0434167 to c7741ca Compare November 2, 2021 16:01
rhaschke added a commit to ubi-agni/moveit that referenced this pull request Nov 7, 2021
@rhaschke
TEMP: configure required upstream and downstream dependencies
589050d 
- moveit/srdfdom#97
  extending the syntax of SRDF to specify these defaults (and exceptions)

- frankaemika/franka_ros#188
- moveit#2938
  allowing collision checking to be disabled by default for specific links
@rhaschke rhaschke force-pushed the rework-acm-defaults branch from c7741ca to 8efbf11 Compare November 7, 2021 20:37
rhaschke added a commit to ubi-agni/moveit that referenced this pull request Nov 8, 2021
@rhaschke
TEMP: configure required upstream and downstream dependencies
c898c9c 
- moveit/srdfdom#97
  extending the syntax of SRDF to specify these defaults (and exceptions)

- frankaemika/franka_ros#188
- moveit#2938
  allowing collision checking to be disabled by default for specific links
@rhaschke rhaschke mentioned this pull request Nov 8, 2021
Closed
11 tasks
rhaschke added a commit to ubi-agni/moveit that referenced this pull request Nov 8, 2021
@rhaschke
TEMP: configure required upstream and downstream dependencies, drop R…
1bc81a9 
…oboStack workflows

- moveit/srdfdom#97
  extending the syntax of SRDF to specify these defaults (and exceptions)

- frankaemika/franka_ros#188
- moveit#2938
  allowing collision checking to be disabled by default for specific links

- RoboStack workflow doesn't use upstream config and thus fails
rickstaa pushed a commit to rickstaa/moveit that referenced this pull request Nov 11, 2021
@rhaschke @rickstaa
TEMP: configure required upstream and downstream dependencies, drop R…
beba5a6 
…oboStack workflows

- moveit/srdfdom#97
  extending the syntax of SRDF to specify these defaults (and exceptions)

- frankaemika/franka_ros#188
- moveit#2938
  allowing collision checking to be disabled by default for specific links

- RoboStack workflow doesn't use upstream config and thus fails
v4hn
v4hn reviewed Dec 10, 2021
View reviewed changes
Copy link
Contributor

@v4hn v4hn left a comment

Choose a reason for hiding this comment

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

Thanks for working on this. It's quite some work for a (at the moment) edge-case feature 👍

rhaschke added a commit to ubi-agni/moveit that referenced this pull request Dec 20, 2021
@rhaschke
TEMP: configure required upstream and downstream dependencies, drop R…
ff9ac3c 
…oboStack workflows

- moveit/srdfdom#97
  extending the syntax of SRDF to specify these defaults (and exceptions)

- frankaemika/franka_ros#188
- moveit#2938
  allowing collision checking to be disabled by default for specific links

- RoboStack workflow doesn't use upstream config and thus fails
@rhaschke rhaschke force-pushed the rework-acm-defaults branch from 1546bb2 to 9f9e2d3 Compare December 22, 2021 15:16
JafarAbdi
JafarAbdi approved these changes Dec 22, 2021
View reviewed changes
Copy link
Member

@JafarAbdi JafarAbdi 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 for tackling this, is there something blocking merging this PR?

The example in the PR description is outdated, right? it should be something like

<disable_default_collisions link="link_name" />
<enable_collisions link1="link1_name" link2="link2_name" reason="optional-reason" />
<disable_collisions link1="link1_name" link2="link2_name" reason="optional-reason" />

@rhaschke
Copy link
Contributor Author

Yes, as Michael asked for an adaption of the XML tags, the example should be adapted correspondingly.
This PR should be merged in sync with moveit/moveit#2938. However, nobody reviewed that PR yet.

@rhaschke rhaschke force-pushed the rework-acm-defaults branch 3 times, most recently from ee97fe7 to 1c15e6c Compare December 23, 2021 13:01
@rhaschke rhaschke force-pushed the rework-acm-defaults branch from 1c15e6c to eb2989f Compare December 23, 2021 13:07
@v4hn
Copy link
Contributor

v4hn commented Dec 23, 2021 •
edited
Loading

This PR should be merged in sync with moveit/moveit#2938.

Why in sync? I was about to merge this now as the basis for the MoveIt patch. Feel free to merge this now if you agree.

However, nobody reviewed that PR yet.

Sorry, I did not have enough time for both yet, so I focused on the basis first.

@rhaschke
Copy link
Contributor Author

Why in sync?

This PR changes API and thus breaks MoveIt. Thus it should be merged in sync with moveit/moveit#2938, which is currently broken due to your requested API changes.

@v4hn
Copy link
Contributor

v4hn commented Dec 23, 2021 via email

rhaschke added a commit to ubi-agni/moveit that referenced this pull request Dec 28, 2021
@rhaschke
TEMP: configure required upstream and downstream dependencies, drop R…
34c3584 
…oboStack workflows

- moveit/srdfdom#97
  extending the syntax of SRDF to specify these defaults (and exceptions)

- frankaemika/franka_ros#188
- moveit#2938
  allowing collision checking to be disabled by default for specific links

- RoboStack workflow doesn't use upstream config and thus fails
rhaschke added a commit to ubi-agni/moveit that referenced this pull request Dec 28, 2021
@rhaschke
TEMP: configure required upstream and downstream dependencies, drop R…
112f1c5 
…oboStack workflows

- moveit/srdfdom#97
  extending the syntax of SRDF to specify these defaults (and exceptions)

- frankaemika/franka_ros#188
- moveit#2938
  allowing collision checking to be disabled by default for specific links

- RoboStack workflow doesn't use upstream config and thus fails
rhaschke added a commit to ubi-agni/moveit that referenced this pull request Dec 28, 2021
@rhaschke
TEMP: configure required upstream and downstream dependencies, drop R…
395bc3c 
…oboStack workflows

- moveit/srdfdom#97
  extending the syntax of SRDF to specify these defaults (and exceptions)

- frankaemika/franka_ros#188
- moveit#2938
  allowing collision checking to be disabled by default for specific links

- RoboStack workflow doesn't use upstream config and thus fails
rhaschke added a commit to ubi-agni/moveit that referenced this pull request Dec 28, 2021
@rhaschke
TEMP: configure required upstream and downstream dependencies, drop R…
0ea8cb1 
…oboStack workflows

- moveit/srdfdom#97
  extending the syntax of SRDF to specify these defaults (and exceptions)

- frankaemika/franka_ros#188
- moveit#2938
  allowing collision checking to be disabled by default for specific links

- RoboStack workflow doesn't use upstream config and thus fails
rhaschke added a commit to ubi-agni/moveit that referenced this pull request Dec 28, 2021
@rhaschke
TEMP: configure required upstream and downstream dependencies, drop R…
8839758 
…oboStack workflows

- moveit/srdfdom#97
  extending the syntax of SRDF to specify these defaults (and exceptions)

- frankaemika/franka_ros#188
- moveit#2938
  allowing collision checking to be disabled by default for specific links

- RoboStack workflow doesn't use upstream config and thus fails
rhaschke added a commit to ubi-agni/moveit that referenced this pull request Dec 28, 2021
@rhaschke
TEMP: configure required upstream and downstream dependencies, drop R…
7768592 
…oboStack workflows

- moveit/srdfdom#97
  extending the syntax of SRDF to specify these defaults (and exceptions)

- frankaemika/franka_ros#188
- moveit#2938
  allowing collision checking to be disabled by default for specific links

- RoboStack workflow doesn't use upstream config and thus fails
@rhaschke rhaschke merged commit a20c9e4 into moveit:noetic-devel Jan 17, 2022
@rhaschke rhaschke deleted the rework-acm-defaults branch January 17, 2022 08:34
rhaschke added a commit to moveit/moveit that referenced this pull request Jan 17, 2022
@rhaschke
Merge PR #2938: Rework ACM
424a5b7 
Implement ACM defaults as a fallback instead of an override.
Based on moveit/srdfdom#97, this allows disabling collisions for specific links/objects by default and re-enabling individual pairs if necessary.
henningkayser pushed a commit that referenced this pull request Mar 29, 2022
@Abishalini @rhaschke
Parse <disable_default_collisions> and <enable_collisions> tags (#101,…
567ac52 
… from #97)

Co-authored-by: Robert Haschke <[email protected]>
@kbrameld
Copy link

I'd like to ask for some clarification in regards to these changes.

If a robot has a link that has collision disabled by default (via disable_default_collisions), does that mean that link does not collide with other collision objects in the environment (eg. table, wall) and other robots? Or does it just disable collisions between links in the robot.

@rhaschke
Copy link
Contributor Author

It means disabling collisions with any robot link or world object.

@kbrameld
Copy link

Thanks @rhaschke

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@v4hn v4hn v4hn left review comments

@JafarAbdi JafarAbdi JafarAbdi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rhaschke @v4hn @kbrameld @JafarAbdi