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

IRB 1200 support pkg updates & MoveIt pkgs #85

Merged

Conversation

gavanderhoorn
Copy link
Member

@gavanderhoorn gavanderhoorn commented Jan 3, 2018

This includes the changes contributed by @ajshort (in #77) and by @stwirth (in #81), plus a nr of smaller commits by me.

We could also opt to just merge #77 or #81 and fixup things in a subsequent PR, but the reason it might make sense not to do that is that the 7/0.7 variant can share many of its meshes with the 5/0.9. As the 7/0.7 variant is introduced in #77 (but doesn't share any meshes with the 5/0.9 there), merging that PR would introduce commits that add (relatively) large files to the repository that would then later be removed again.

Doing it all in one PR would avoid that (as we can squash merge).

@gavanderhoorn
Copy link
Member Author

Opening a PR so Travis can take a look and to (if needed) discuss the proposed changes.

@gavanderhoorn
Copy link
Member Author

I just realised that sharing meshes between the 7/0.7 and 5/0.9 variants is difficult as one uses Collada for the visual meshes and the other STL.

Technically no reason those can't be mixed, but aesthetically it's a bit of a no-no.

@stwirth
Copy link

stwirth commented Feb 19, 2018

Ping! I'd like to see this merged...

@gavanderhoorn
Copy link
Member Author

To do this properly I'll need to go and convert meshes to Collada from the meshes that ABB provides. I won't have time for that this week.

If you can provide the meshes we are missing, that would expedite things.

@bhomberg
Copy link

Hi! I'm working with @stwirth on this project.

I'm trying to figure out what exactly you need to expedite this. As far as I can tell:
-for the 1200_5_90 arm, abb_experimental has .stl's for collision meshes
-for the 1200_5_90 arm, abb_experimental has .dae's for visual meshes
-this PR includes collision meshes and visual meshes as .stl's

Do you just need the 1200_7_70 visual meshes converted from .stl to .dae format, or do you also need some comparison of which ones are equivalent / change in file setup so they can be used together more easily?

@ajshort
Copy link

ajshort commented Mar 10, 2018

If you're trying to get collada meshes, you can export them directly from robotstudio rather than converting them. Just right click on each link and go "Export Geometry".

@gavanderhoorn
Copy link
Member Author

@bhomberg wrote:

I'm trying to figure out what exactly you need to expedite this

It would be nice if we could share as many meshes as possible between the two variants supported by the irb1200 package. Both packages have STL meshes for the collision quality meshes, but the 7/0.7 variant uses STL for the visual meshes and the 5/0.9 uses Collada.

Technically there is nothing preventing us from mixing the two, it just won't look very nice.

Summarising: the 7/0.7 meshes should be Collada, the overlap between the two variants determined and only a single set of meshes should be stored. See fanuc_lrmate200ic_support for an example (note the lrmate200ic5l directory which only contains meshes for links 2 and 4).

bhomberg and others added 3 commits April 13, 2018 21:03
Updated collision meshes to match new visuals.

Reuse link_4 from 5_90 with a small (0.1m) offset in x.
More in-line with other support packages.
@gavanderhoorn gavanderhoorn changed the title IRB 1200 support pkg updates & MoveIt pkgs [WIP] IRB 1200 support pkg updates & MoveIt pkgs Apr 14, 2018
@gavanderhoorn
Copy link
Member Author

I believe this is good to go now.

I've requested a review from @Levi-Armstrong.

I'll merge after he gives his 👍

@gavanderhoorn gavanderhoorn merged commit f749600 into ros-industrial:kinetic-devel Apr 14, 2018
@gavanderhoorn gavanderhoorn deleted the irb1200_updates branch April 14, 2018 13:41
@gavanderhoorn
Copy link
Member Author

Thanks @Levi-Armstrong for the review.

And thanks @stwirth, @ajshort, @bhomberg and @jonbinney.

🍻 🍔 👍

@gavanderhoorn
Copy link
Member Author

@stwirth, @ajshort, @bhomberg and @jonbinney: I've done a squash-merge in order to avoid repository bloat due to the changes to the various meshes that were tried/created.

I've made sure to retain attribution of your contributions in the commit msg of the squash commit.

Thanks again.

@jonbinney
Copy link

Thanks @gavanderhoorn !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants