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

Ignore invalid joint commands #137

Merged
merged 4 commits into from
Oct 21, 2020

Conversation

azeey
Copy link
Contributor

@azeey azeey commented Oct 20, 2020

Robots falling through the floor were reported in osrf/subt#675. This was caused by an input joint velocity command of nan being passed to JointFeatures::SetJointVelocityCommand via ign-gazebo's DiffDrive system. In DART, Joint velocity commands are handled by the constraint solver, which also handles collisions, so it makes sense that the robot falls through when the solver fails. The solution here is to ignore nan values in JointFeatures::SetJointVelocityCommand.

I have taken the opportunity to ignore invalid values in other JointFeatures::SetJoint* commands as well.

azeey added 4 commits October 20, 2020 16:06
…commanded

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@azeey azeey requested a review from mxgrey as a code owner October 20, 2020 21:46
@azeey azeey self-assigned this Oct 20, 2020
@azeey azeey added the 📜 blueprint Ignition Blueprint label Oct 20, 2020
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

Wow some bugs are just amazing. The fix looks good 👍

@nkoenig
Copy link
Contributor

nkoenig commented Oct 21, 2020

Thanks for finding this!

Copy link
Contributor

@mxgrey mxgrey left a comment

Choose a reason for hiding this comment

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

This seems like a reasonable safety measure, although I'm alarmed that the DiffDrive system would be outputting NaNs. I hope that issue can be addressed as well.

@azeey
Copy link
Contributor Author

azeey commented Oct 21, 2020

This seems like a reasonable safety measure, although I'm alarmed that the DiffDrive system would be outputting NaNs. I hope that issue can be addressed as well.

Yeah, we should probably do a better job of validating user input in ign-gazebo.

@azeey
Copy link
Contributor Author

azeey commented Oct 21, 2020

Jenkins CI looks good, but the GitHub actions didn't run. @chapulina, do you know why? is there a way to start them manually?

@chapulina
Copy link
Contributor

the GitHub actions didn't run. @chapulina, do you know why?

I've only been fixing the actions for forks from Citadel onwards because Blueprint will go EOL soon and I wasn't expecting many fork PRs for it.

is there a way to start them manually?

I don't think so. You could open a PR from a branch... But I think this is safe to merge just with Jenkins CI, so I'll go ahead and do that.

@chapulina chapulina merged commit f1b9f77 into gazebosim:ign-physics1 Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📜 blueprint Ignition Blueprint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants