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

description: spec limits in degrees. #533

Conversation

gavanderhoorn
Copy link
Member

Both rosparam (wiki page) and xacro (with ros/xacro#252 merged) support special yaml constructors which allow conversion of numeric values to radians on-the-fly.

This makes it possible for us to specify joint limits in the config/urXX/joint_limits.yaml files in degrees, which are much more human readable and easier to compare to the spec sheets.

Use rosparam's and xacro's support for these special constructors to convert to radians on-the-fly.
@gavanderhoorn gavanderhoorn added this to the melodic-devel-staging milestone Aug 18, 2020
@gavanderhoorn
Copy link
Member Author

This PR is expected to fail until ros/xacro#252 gets merged.

Copy link
Member

@ipa-nhg ipa-nhg left a comment

Choose a reason for hiding this comment

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

LGTM 👍 thanks!

But I suggest to wait until the latest xacro package version is released

@gavanderhoorn
Copy link
Member Author

But I suggest to wait until the latest xacro package version is released

Yes, that's my intention.

CI is failing for a different reason though: somehow it can't find ros-*-moveit-core, which is rather curious ..

@gavanderhoorn gavanderhoorn force-pushed the yaml_ctors_rosparam_joint_limits branch from b385f57 to c31636b Compare August 30, 2020 11:31
@gavanderhoorn
Copy link
Member Author

xacro was synced into Melodic, so retriggering CI.

@ipa-nhg
Copy link
Member

ipa-nhg commented Oct 8, 2020

I tested the simulation locally with the updated release version of the xacro package and it works as expected.
Travis is also happy 😃
--> MERGE PR

@ipa-nhg ipa-nhg merged commit b09afc1 into ros-industrial:melodic-devel-staging Oct 8, 2020
@gavanderhoorn
Copy link
Member Author

Thanks for the enthusiasm, but c31636b was actually not supposed to end up in the merge :)

I'll fix it later.

@ipa-nhg
Copy link
Member

ipa-nhg commented Oct 8, 2020

Ups sorry, I just showed that I had previously reviewed and approved the PR. I didn't scroll again the changed files.

Let me know if you don't have time and I should delete the commit and rebase the branch.

gavanderhoorn pushed a commit that referenced this pull request Oct 8, 2020
…limits

description: spec limits in degrees.
gavanderhoorn pushed a commit that referenced this pull request Oct 8, 2020
…limits

description: spec limits in degrees.
@gavanderhoorn gavanderhoorn deleted the yaml_ctors_rosparam_joint_limits branch October 8, 2020 09:38
@gavanderhoorn
Copy link
Member Author

I've dropped the commit.

Now that it's a 1 commit PR, it should've been squash-merged, but I've left your merge commit as-is.

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.

2 participants