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

Parse sdf's joint dynamics completely #1383

Merged
merged 6 commits into from
Aug 12, 2019
Merged

Parse sdf's joint dynamics completely #1383

merged 6 commits into from
Aug 12, 2019

Conversation

vinnamkim
Copy link
Contributor

@vinnamkim vinnamkim commented Aug 5, 2019 •
edited
Loading

  • Parse joint friction, spring_stiffness, spring_reference

Signed-off-by: vinnamkim vinnam.kim@gmail.com

Resolve #1382

Before creating a pull request

  • Document new methods and classes
  • Format new code files using clang-format

Before merging a pull request

  • Set version target by selecting a milestone on the right side
  • Summarize this change in CHANGELOG.md
  • Add unit test(s) for this change

@vinnamkim
Parse sdf's joint dynamics completely
8246beb 
 - Parse joint friction, spring_stiffness, spring_reference

Signed-off-by: vinnamkim <vinnam.kim@gmail.com>
@codecov
Copy link

codecov bot commented Aug 5, 2019 •
edited
Loading

Codecov Report

Merging #1383 into master will increase coverage by 0.3%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master    #1383     +/-   ##
=========================================
+ Coverage   56.97%   57.27%   +0.3%     
=========================================
  Files         366      366             
  Lines       27416    27432     +16     
=========================================
+ Hits        15619    15711     +92     
+ Misses      11797    11721     -76
Impacted Files Coverage Δ
dart/utils/sdf/SdfParser.cpp 76.73% <100%> (+10.55%) ⬆️
dart/dynamics/UniversalJoint.cpp 75.94% <0%> (+6.32%) ⬆️
dart/dynamics/ScrewJoint.cpp 70.51% <0%> (+6.41%) ⬆️
dart/dynamics/PrismaticJoint.cpp 69.35% <0%> (+8.06%) ⬆️
dart/dynamics/RevoluteJoint.cpp 72.58% <0%> (+8.06%) ⬆️
dart/dynamics/detail/ScrewJointAspect.hpp 100% <0%> (+40%) ⬆️

jslee02
jslee02 requested changes Aug 7, 2019
View reviewed changes
Copy link
Member

@jslee02 jslee02 left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution! One style change is suggested in the inline comment.

Would you be able to add a simple unit test for completion something like this to test_SdfParser.cpp?

dart/utils/sdf/SdfParser.cpp Outdated
@@ -1258,7 +1258,7 @@ static void readAxisElement(
const Eigen::Isometry3d& _parentModelFrame,
Eigen::Vector3d& axis,
double& lower, double& upper, double& initial, double& rest,
double& damping)
double& damping, double &friction, double &sprint_stiffness)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
double& damping, double &friction, double &sprint_stiffness)
double& damping, double& friction, double& sprint_stiffness)

Copy link
Contributor Author

@vinnamkim vinnamkim Aug 9, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Applied clang-format to overall SdfParser.cpp.

@vinnamkim
Apply clang-format to SdfParser.cpp
fdb7467 
Signed-off-by: vinnamkim <vinnam.kim@gmail.com>
@vinnamkim
Fix error on spring_stiffness
ee277bf 
Signed-off-by: vinnamkim <vinnam.kim@gmail.com>
@vinnamkim
Add unittest for joint_properties
6a03045 
Signed-off-by: vinnamkim <vinnam.kim@gmail.com>
@vinnamkim
Copy link
Contributor Author

Thank you for the contribution! One style change is suggested in the inline comment.

Would you be able to add a simple unit test for completion something like this to test_SdfParser.cpp?

An error on parsing spring stiffness and related unit tests are added.

@vinnamkim vinnamkim force-pushed the parse_sdf_joint_dynamics branch from f7c4659 to 6a03045 Compare August 9, 2019 07:22
jslee02
jslee02 requested changes Aug 9, 2019
View reviewed changes
Copy link
Member

@jslee02 jslee02 left a comment

Choose a reason for hiding this comment

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

I think we're almost there. Could you resolve the conflicts? I merged #1384 for formatting the utils code to make the changes or this PR clean (removing the code formatting changes from this PR).

data/sdf/test/test_skeleton_joint.world Outdated
@@ -0,0 +1,278 @@
<?xml version="1.0" ?>
<sdf version="1.4">
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<sdf version="1.4">
<sdf version="1.5">

Looking at the specification, the new joint dynamics properties were introduced in version 1.5.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jslee02 Resolved the conflicts and modified the sdf file version to 1.5

jslee02
jslee02 approved these changes Aug 12, 2019
View reviewed changes
Copy link
Member

@jslee02 jslee02 left a comment

Choose a reason for hiding this comment

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

Looks good to me! Let's merge this once Travis CI becomes happy.

@jslee02 jslee02 merged commit 775a7d3 into dartsim:master Aug 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jslee02 jslee02 jslee02 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

SdfParser doesn't parse joint dynamics completely
2 participants
@vinnamkim @jslee02