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.

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

Port to ROS 2 #41

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 0 additions & 16 deletions CMakeLists.txt

This file was deleted.

3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ Not yet sure how to get it to generate catkin-like development installs, which u
* Kelsey Hawkins - `urdf_parser_python` implementation, integration
* Antonio El Khoury - bugfixes
* Eric Cousineau - reflection (serialization?) changes
* Ioan Sucan
* Jackie Kay
* Víctor Mayoral Vilches - port to ROS 2

## Reflection (or just Serialization?)

Expand Down
18 changes: 10 additions & 8 deletions package.xml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?xml version="1.0"?>
<package format="2">
<?xml-model href="http://download.ros.org/schema/package_format3.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
<package format="3">
<name>urdfdom_py</name>
<version>0.3.3</version>
<description>Python implementation of the URDF parser.</description>
Expand All @@ -13,6 +14,7 @@
<author>Eric Cousineau</author>
<author email="isucan@gmail.com">Ioan Sucan</author>
<author email="jacquelinekay1@gmail.com">Jackie Kay</author>
<author email="vmayoral@acutronicrobotics.com">Víctor Mayoral Vilches</author>

<maintainer email="clalancette@osrfoundation.org">Chris Lalancette</maintainer>
<maintainer email="sloretz@osrfoundation.org">Shane Loretz</maintainer>
Expand All @@ -21,13 +23,13 @@
<url type="bugtracker">https://github.com/ros/urdf_parser_py/issues</url>
<url type="repository">https://github.com/ros/urdf_parser_py</url>

<buildtool_depend>catkin</buildtool_depend>
<buildtool_depend>ament_cmake</buildtool_depend>
Copy link
Contributor

Choose a reason for hiding this comment

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

From what I can tell looking elsewhere, with a pure python package we don't need any buildtool_depend line, so just remove this.

Copy link
Contributor

Choose a reason for hiding this comment

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

This comment is still open.


<depend>python</depend>

<exec_depend>python-lxml</exec_depend>
<exec_depend>python-yaml</exec_depend>

<test_depend>python-mock</test_depend>
<exec_depend>python3-lxml</exec_depend>
<exec_depend>python3-yaml</exec_depend>
<test_depend>python3-mock</test_depend>

<export>
<build_type>ament_python</build_type>
</export>
</package>
33 changes: 26 additions & 7 deletions setup.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,30 @@
#!/usr/bin/env python
from setuptools import setup

from distutils.core import setup
from catkin_pkg.python_setup import generate_distutils_setup
package_name = 'urdfdom_py'

d = generate_distutils_setup(
setup(
name=package_name,
version='0.3.3',
Copy link
Contributor

Choose a reason for hiding this comment

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

melodic-devel is currently on 0.4.0, but since this is going to be a new branch, we probably want to bump up to something like 1.0.0 here (and in the package.xml).

package_dir={'': 'src'},
packages=['urdf_parser_py', 'urdf_parser_py.xml_reflection'],
package_dir={'': 'src'}
data_files=[
('scripts/display_urdf',
['package.xml']),
],
install_requires=['setuptools'],
zip_safe=True,
author='Víctor Mayoral Vilches',
author_email='vmayoral@acutronicrobotics.com',
maintainer='Víctor Mayoral Vilches',
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't mind adding you as a maintainer, but if you want to be the maintainer I'd suggest that we add you to the package.xml as well. If you don't want to be a maintainer, you can just put my name/email here.

maintainer_email='vmayoral@acutronicrobotics.com',
keywords=['ROS2'],
classifiers=[
'Intended Audience :: Developers',
'License :: OSI Approved :: Apache Software License',
'Programming Language :: Python',
'Topic :: Software Development',
],
description='Python implementation of the URDF parser.',
license='Apache License, Version 2.0',
Copy link
Contributor

Choose a reason for hiding this comment

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

This has to stay BSD, since we aren't really changing the underlying code.

tests_require=['pytest'],
)

setup(**d)
4 changes: 2 additions & 2 deletions src/urdf_parser_py/xml_reflection/basics.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@

def xml_string(rootXml, addHeader=True):
# Meh
xmlString = etree.tostring(rootXml, pretty_print=True)
xmlString = etree.tostring(rootXml, pretty_print=True, encoding=str)
if addHeader:
xmlString = '<?xml version="1.0"?>\n' + xmlString
Copy link
Contributor

Choose a reason for hiding this comment

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

Any particular reason for this change? I think this can stay as-is.

xmlString = "<?xml version=\"1.0\"?>\n" + xmlString
return xmlString


Expand Down