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

Added motoman_resources package. #378

Closed
wants to merge 1 commit into from
Closed

Added motoman_resources package. #378

wants to merge 1 commit into from

Conversation

Doomerdinger
Copy link
Contributor

@Doomerdinger Doomerdinger commented Dec 7, 2020 •
edited
Loading

Made robots use materials from that package so they do not conflict when using multiple arms.

Also added a common macro file that can be used to load kinematic parameters and other values.

Related to #314

@Doomerdinger Doomerdinger force-pushed the add-motoman-resources branch 2 times, most recently from 5e73f12 to eb6e591 Compare February 2, 2021 20:43
@gavanderhoorn
Copy link
Member

I'll do a proper review later, some high-level comments now:

  • please don't manually bump versions of packages. This will all be done automatically by the release pipeline. Please revert all those changes to the manifests
  • same goes for the CHANGELOG file: it's added automatically, so please revert the addition
  • please add at least one ROS-I maintainer (me). See the other packages for the metadata to add to the manifest
  • and it's a minor point, but I believe you've copied the structure/approach from fanuc_resources or one of the other similar packages. I realise we're in the same 'context' here (ie: ROS-Industrial), but technically, I believe it would've been good to at least mention that somewhere, as the license of those packages requires

@Doomerdinger
Copy link
Contributor Author

I'll do a proper review later, some high-level comments now:

  • please don't manually bump versions of packages. This will all be done automatically by the release pipeline. Please revert all those changes to the manifests
  • same goes for the CHANGELOG file: it's added automatically, so please revert the addition
  • please add at least one ROS-I maintainer (me). See the other packages for the metadata to add to the manifest
  • and it's a minor point, but I believe you've copied the structure/approach from fanuc_resources or one of the other similar packages. I realise we're in the same 'context' here (ie: ROS-Industrial), but technically, I believe it would've been good to at least mention that somewhere, as the license of those packages requires

Didn't realize the release pipeline took care of versions/changelog, I'll be sure to not do that moving forward.
I'll add you to the packages, and mention the fanuc package.

@Doomerdinger Doomerdinger mentioned this pull request Feb 18, 2021
Closed
@Doomerdinger Doomerdinger mentioned this pull request Mar 4, 2021
Closed
Added motoman_resources package.
3056cde 
Made robots use materials from that package so they do not conflict when using multiple arms.
This was referenced Mar 5, 2021
Closed
Open
@gavanderhoorn gavanderhoorn mentioned this pull request May 15, 2021
Merged
gavanderhoorn added a commit that referenced this pull request May 15, 2021
@gavanderhoorn
Merge pull request #405 from gavanderhoorn/add-motoman-resources-rebased
9ade90a 
Add motoman resources (#378 rebased)
@gavanderhoorn
Copy link
Member

Replacement in #405 (with additional commits migrating more support packages to use the resources package).

Thanks for the initial PR @Doomerdinger 👍

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

@gavanderhoorn gavanderhoorn Awaiting requested review from gavanderhoorn

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Doomerdinger @gavanderhoorn