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

Add error handling for JointAxis::SetXyz and remove use of use_parent_model_frame #288

Merged
merged 4 commits into from
Sep 2, 2020
Merged

Add error handling for JointAxis::SetXyz and remove use of use_parent_model_frame #288

merged 4 commits into from
Sep 2, 2020

Conversation

azeey
Copy link
Contributor

@azeey azeey commented Aug 12, 2020

As of gazebosim/sdformat#312, JointAxis::SetXyz returns an sdf::Errors. A warning is emitted if the return is ignored, so this PR adds error handling for it. In addition, gazebosim/sdformat#312 enforces normalization of the input XYZ vector, so I have updated the test expectations in this PR.

This PR also removes the use of use_parent_model_frame since it was deprecated in Citadel and should be removed in Dome. I will create a followup PR in ign-msgs to remove it, but that's not required for this PR.

@azeey
Add error handling for JointAxis::SetXyz and remove use of use_parent…
28929ae 
…_model_frame

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@azeey azeey added the 🔮 dome Ignition Dome label Aug 12, 2020
@azeey azeey requested a review from scpeters August 12, 2020 17:25
@azeey azeey requested a review from chapulina as a code owner August 12, 2020 17:25
@azeey azeey self-assigned this Aug 12, 2020
@chapulina
Copy link
Contributor

@osrf-jenkins run tests please

@scpeters
Copy link
Member

the changes look good to me, but I'm not sure about the CI failures

@chapulina
Copy link
Contributor

@osrf-jenkins run tests please

@azeey
Copy link
Contributor Author

azeey commented Aug 17, 2020

I think we need a pre-release of sdformat to fix the homebrew CI failures. The Ubuntu CI failures have been fixed by #271, but it hasn't made its way to master yet.

@scpeters
Copy link
Member

I think we need a pre-release of sdformat to fix the homebrew CI failures. The Ubuntu CI failures have been fixed by #271, but it hasn't made its way to master yet.

the prerelease has been made

@osrf-jenkins run tests please

@azeey
Copy link
Contributor Author

azeey commented Aug 18, 2020

Sorry @scpeters, I should have been more specfic. We actually need a prerelease of sdf10 for this since it needs gazebosim/sdformat#312.

@scpeters
Copy link
Member

actually we need another sdformat10 prerelease for this. I'll work on that too

@scpeters scpeters mentioned this pull request Aug 18, 2020
Merged
@scpeters
Copy link
Member

Sorry @scpeters, I should have been more specfic. We actually need a prerelease of sdf10 for this since it needs osrf/sdformat#312.

the prerelease is building; I'll restart the jenkins jobs once it is finished

@scpeters
Copy link
Member

@osrf-jenkins run tests please

@azeey
Copy link
Contributor Author

azeey commented Aug 19, 2020

New builds

  • Ubuntu: Build Status
  • Homebrew: Build Status

Failing tests don't appear to be caused by this PR.

@scpeters scpeters mentioned this pull request Aug 24, 2020
Merged
@chapulina
Copy link
Contributor

@osrf-jenkins run tests please

1 similar comment
@scpeters
Copy link
Member

@osrf-jenkins run tests please

@chapulina
Copy link
Contributor

I'll close and reopen to trigger CI, not sure why the ubuntu actions are not coming up

@chapulina chapulina closed this Sep 1, 2020
@chapulina chapulina reopened this Sep 1, 2020
@chapulina
actions CI for forks
278cde4 
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina
Copy link
Contributor

not sure why the ubuntu actions are not coming up

It's because the PR is from a fork, now it will be triggered: 278cde4

@chapulina chapulina mentioned this pull request Sep 1, 2020
Merged
@chapulina chapulina merged commit e06ad9e into gazebosim:master Sep 2, 2020
@chapulina chapulina mentioned this pull request Sep 2, 2020
Merged
@azeey azeey deleted the joint_axis_xyz_warning branch September 2, 2020 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chapulina chapulina chapulina approved these changes

@scpeters scpeters Awaiting requested review from scpeters

Assignees

@azeey azeey

Labels
🔮 dome Ignition Dome
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@azeey @chapulina @scpeters