Add in support for the version tag. #52
Conversation
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
test/test_urdf.py
Outdated
| xml = '''<?xml version="1.0"?> | ||
| <robot name="test" version="foo"> | ||
| </robot>''' | ||
| #self.assertRaises(xmlr.core.ParseError, self.parse, xml) |
There was a problem hiding this comment.
Yeah, definitely, just a leftover from testing. Done in 9f78b7f
|
this also looks good; just one more minor comment |
|
The concerns about parsing floats in ros/urdfdom#133 (comment) are relevant here too |
|
I think we need to update the version parsing here too |
Yeah, definitely, I just ran out of time yesterday. I'll get to this early next week. |
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
OK, late this week, but I did finally get it done. I switched this to a string now, and updated the tests to match the C++ ones in 9f78b7f. This is ready for another review. |
test/test_urdf.py
Outdated
|
|
||
| def test_version_attribute_no_major_number(self): | ||
| xml = '''<?xml version="1.0"?> | ||
| <robot name="test" version="1."> |
There was a problem hiding this comment.
for no_major_number, should this be version=".0"?
There was a problem hiding this comment.
Yeah, definitely. That was a mistake, and it actually showed a bug. Will fix.
test/test_urdf.py
Outdated
|
|
||
| def test_version_attribute_negative_minor_number(self): | ||
| xml = '''<?xml version="1.0"?> | ||
| <robot name="test" version="-1.0"> |
src/urdf_parser_py/urdf.py
Outdated
|
|
||
| split = self.version.split(".") | ||
| if len(split) != 2: | ||
| raise Exception("The version attribute should be in the form 'x.y'") |
There was a problem hiding this comment.
Recommend a custom exception, but ValueError would fit too. Same below.
There was a problem hiding this comment.
Yeah, I went back and forth on making a custom exception. I think I'll go with "ValueError", as that seems to make the most sense to me.
| @@ -533,6 +533,20 @@ def get_root(self): | |||
| assert root is not None, "No roots detected, invalid URDF." | |||
| return root | |||
|
|
|||
| def post_read_xml(self): | |||
There was a problem hiding this comment.
What calls this function? I can't find any uses of it.
There was a problem hiding this comment.
It's magic :). The Robot class is a sub-class of xmlr.Object, which defines an empty version of post_read_xml. In turn, this method is called directly after reading in the XML (see here). So I just overrode it in the Robot sub-class.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
|
Thanks! Merging. |
* Add in support for the version tag. Cherry-picked from the melodic-devel branch commit c0a75a2 Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Chris Lalancette clalancette@openrobotics.org
Just like in urdfdom, add support for the version tag in the URDF XML. Note that I ended up adding the
versiontag by hand to all of the round-trip tests so that they pass; there are other tests that test the lack of aversiontag, so I think this is fine. @sloretz @scpeters FYI.