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

Ported to ROS2 Dashing #210

Closed
wants to merge 9 commits into from
Closed

Ported to ROS2 Dashing #210

wants to merge 9 commits into from

Conversation

kazuki0824
Copy link

Same as the title. This PR is based on @vandanamandlik 's #203. Thanks very much.

- Moved contents from "src/xacro/" folder to "xacro/" folder.
- Renamed scripts folder to resource folder.
- Modifed package.xml and setup.py according to ROS2 rules.
- Deleted CMakeLists.txt
- Modified all python scripts as per ros2 coding guidelines.
- Added substitution_args.py file, replacement for roslaunch's resolve_args() method,
  as it is not available in ROS2's ros2launch package.
  - Updated shebang line in all python scripts to use python3.
  - Resolved errors where some test cases were failing to find out some .xml files. (eg. include1.xml)
  - Added temporary workaround for rosgraph.names.load_mappings because rosgraph is not migrated to ROS2.
- Added some more details in README.md file.
@rhaschke
Copy link
Contributor

rhaschke commented Jul 5, 2019

@kazuki0824, could you please describe in which respect this is different to #203?

@kazuki0824
Copy link
Author

@rhaschke Completely the same for now. This is currently still in progress and set as draft. Further changes are yet to be pushed.

@rhaschke
Copy link
Contributor

rhaschke commented Jul 5, 2019

So, I guess this is simply a rebase of #203 to the latest master so far?

@kazuki0824
Copy link
Author

@rhaschke Yes, it is.

@allenh1
Copy link

allenh1 commented Jul 25, 2019

@kazuki0824 What's the status of this PR? Do you need help with anything?

@ross-desmond
Copy link

ross-desmond commented Jul 31, 2019

@kazuki0824 It seems like there is some unfinished work with _find in substitution_args.py, do you need help switching the find statements to use ament_index_python.packages? It does not seem _get_rospack() is completely transferable to ROS2 and may require some refactoring.

Please let me know!

@jacobperron
Copy link

FWIW, I briefly messed around to get find substitution working using ament_index_python: https://github.com/jacobperron/xacro/commit/837e862979616c655d60b57ec24e80d668b00d2b

@kazuki0824
Copy link
Author

kazuki0824 commented Aug 23, 2019

@jacobperron I looked through the commit(https://github.com/jacobperron/xacro/commit/837e862979616c655d60b57ec24e80d668b00d2b). Your substitution_args.py looks great to me. I would like you to send PR to my project. Thank you.

@jacobperron
Copy link

@kazuki0824 See kazuki0824#1

@AAlon
Copy link

AAlon commented Aug 28, 2019

@kazuki0824 Can we get that merged in?

@gonzodepedro
Copy link

Changes on #211

@rhaschke rhaschke mentioned this pull request Sep 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants