Backport ign sdf --inertial-stats
#958
Conversation
Signed-off-by: Aditya <aditya050995@gmail.com>
Signed-off-by: Aditya <aditya050995@gmail.com>
adityapande-1995
requested review from
jennuine and
scpeters
and removed request for
scpeters
Signed-off-by: Aditya <aditya050995@gmail.com>
Codecov Report
@@ Coverage Diff @@
## sdf9 #958 +/- ##
==========================================
+ Coverage 87.55% 87.58% +0.02%
==========================================
Files 63 63
Lines 9903 9963 +60
==========================================
+ Hits 8671 8726 +55
- Misses 1232 1237 +5
Continue to review full report at Codecov.
|
src/ign.cc
Outdated
| { | ||
| std::cerr << "Error: " << error.Message() << std::endl; | ||
| } | ||
| return -1; |
There was a problem hiding this comment.
I think we don't need to return here. There can be minor errors that wouldn't affect the inertial calculations. It's nice to print the errors, but just skip the return and that should be fine
There was a problem hiding this comment.
Wouldn't it be safer to return in case the error isn't minor? For example, it won't catch inertial_invalid.sdf
There was a problem hiding this comment.
Yeah that just broke a test case. ign sdf --inertial-stats should inherently check for sdf errors I think, and if those pass only then we should return the statistics. Maybe that is safer for users.
There was a problem hiding this comment.
it currently doesn't work for the VIPER urdf. That might be fixed by #959, but it's not very helpful for debugging if it doesn't at least try to give you some inertial information if there's anything at all wrong with the model. We could add a --strict flag to make this configurable if we can't agree, but I think printing the warning messages should be enough of a clue that there's something wrong
There was a problem hiding this comment.
Updated the test case. On the other hand maybe printing out the false inertial stats can help with debugging.
Signed-off-by: Aditya <aditya050995@gmail.com>
Signed-off-by: Aditya <aditya050995@gmail.com>
Signed-off-by: Aditya <aditya050995@gmail.com>
adityapande-1995
force-pushed
the
aditya/inertial_stats_backport
branch
from
7d2fd04
to
6b532e3
Compare
Signed-off-by: Aditya <aditya050995@gmail.com>
|
The ABI checker failed weirdly. Seems unrelated. |
yes, it's unrelated. I'll merge this now |
🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸
🎉 New feature
Summary
Backport of #936
Test it
Added a test case to
ign_TEST.ccand an example model sdf file.Checklist
codecheckpassed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-bymessages.🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸