Skip to content

Bug fix/erroneous elevator joint#191

Open
reesep5 wants to merge 4 commits into
developfrom
bug-fix/erroneous-elevator-joint
Open

Bug fix/erroneous elevator joint#191
reesep5 wants to merge 4 commits into
developfrom
bug-fix/erroneous-elevator-joint

Conversation

@reesep5
Copy link
Copy Markdown

@reesep5 reesep5 commented Sep 18, 2024

Fixes issue where an erroneous elevator joint gets added when changing offsets within a model, causing all of the reference frames to be incorrect.

Comment thread README.md
Comment on lines +1 to +2
# __IMPORTANT NOTE: READ-ONLY REPO, SCS2 GOT MOVED TO GITHUB: https://github.com/ihmcrobotics/simulation-construction-set-2 __

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is this addition to the README?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think this was a Sylvain addition, may not be needed

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you undo all the formatting changes to this file so that the only changes are relevant to the bug fix?

@ds58
Copy link
Copy Markdown
Contributor

ds58 commented Sep 19, 2024

You have a failing test

SimpleCrossFourBarRobotTest > testLoadingURDF() FAILED
    java.lang.NullPointerException: Cannot invoke "us.ihmc.scs2.definition.robot.SixDoFJointDefinition.getSuccessor()" because the return value of "us.ihmc.scs2.definition.robot.RobotDefinition.getFloatingRootJointDefinition()" is null
        at us.ihmc.scs2.definition.robot.urdf.URDFTools.toURDFModel(URDFTools.java:1831)
        at us.ihmc.scs2.definition.robot.urdf.URDFTools.toURDFModel(URDFTools.java:1816)
        at us.ihmc.scs2.examples.urdf.SimpleCrossFourBarRobotTest.testLoadingURDF(SimpleCrossFourBarRobotTest.java:50)

Comment on lines +1823 to +1829
List<JointDefinition> allJointsMinusRootJoint = new ArrayList<>(robotDefinition.getAllJoints());
for (int i = 0; i < allJointsMinusRootJoint.size(); i++)
{
if(allJointsMinusRootJoint.get(i).equals(robotDefinition.getFloatingRootJointDefinition()))
allJointsMinusRootJoint.remove(i);
}

Copy link
Copy Markdown
Contributor

@ds58 ds58 Sep 19, 2024

Choose a reason for hiding this comment

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

You can replace this with

List<JointDefinition> allJointsMinusRootJoint = robotDefinition.getAllJoints();
allJointsMinusRootJoin.remove(robotDefinition.getFloatingRootJointDefinition());

as robotDefinition.getAllJoints() always returns a new ArrayList

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.

4 participants