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
adapt component parser to new xml schema #209
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor comment, otherwise looks good to me!
Codecov Report
@@ Coverage Diff @@
## master #209 +/- ##
===========================================
- Coverage 36.61% 15.72% -20.89%
===========================================
Files 42 116 +74
Lines 1726 9066 +7340
Branches 1080 1880 +800
===========================================
+ Hits 632 1426 +794
- Misses 100 5973 +5873
- Partials 994 1667 +673
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you updated the urdfs by hand? Please update the example URDFs from the example URDFs page we have here: https://github.com/ros-controls/roadmap/blob/master/design_drafts/components_architecture_and_urdf_examples.md
Using min_position_value
has been gone for a while and should be updated.
I saw that some asserts were removed due to this, please re-add them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Karsten1987 happy to go ahead with this once the CI is happy. I can do a follow up update of the example urdfs provided there is an issue for it.
Signed-off-by: Karsten Knese <Karsten1987@users.noreply.github.com>
Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com>
c1125e6
to
46d18bf
Compare
As per #214 I'm ignoring the foxy source job on this one. @Karsten1987 let me know if you are happy with the changes I put on top and if yes, feel free to merge this |
addresses the concerns raised in ros-controls/roadmap#28 (comment)