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

Migrate turtle_tf2 tutorial package to ROS2 #34

Merged
merged 24 commits into from
Jun 9, 2021
Merged

Migrate turtle_tf2 tutorial package to ROS2 #34

merged 24 commits into from
Jun 9, 2021

Conversation

kurshakuz
Copy link

@kurshakuz kurshakuz commented May 26, 2021

Functionality is the same as presented in http://wiki.ros.org/tf2/Tutorials/Introduction%20to%20tf2.

The only difference is that it is needed to launch turtlesim world, broadcaster and twist publishers separately from the teleop node. Corresponding issue is available here: ros/ros_tutorials#125

To launch that example, run following commands:

$ ros2 launch turtle_tf2_py turtle_tf2_demo.launch.py
$ ros2 run turtlesim turtle_teleop_key

For Rviz part in the end, run (will need to update the path):

$ ros2 run rviz2 rviz2 -d $(ros2 pkg prefix --share turtle_tf2)/rviz/turtle_rviz.rviz

@ahcorde ahcorde self-requested a review May 26, 2021 17:16
turtle_tf2/package.xml Outdated Show resolved Hide resolved
turtle_tf2/setup.py Outdated Show resolved Hide resolved
turtle_tf2/package.xml Outdated Show resolved Hide resolved
turtle_tf2/setup.py Outdated Show resolved Hide resolved
turtle_tf2/setup.py Outdated Show resolved Hide resolved
turtle_tf2_launch/launch/turtle_tf2_demo.launch.py Outdated Show resolved Hide resolved
turtle_tf2_launch/package.xml Outdated Show resolved Hide resolved
turtle_tf2_launch/package.xml Outdated Show resolved Hide resolved
turtle_tf2_launch/package.xml Outdated Show resolved Hide resolved
turtle_tf2_launch/launch/turtle_tf2_demo.launch.py Outdated Show resolved Hide resolved
@kurshakuz
Copy link
Author

@ahcorde can you suggest what should I use for the version tag? Previously it was 0.2.3.
In addition, should I change files in the main geometry_tutorials folder? Its run_depend, license and version tags should probably be updated too.

@ahcorde
Copy link
Contributor

ahcorde commented May 27, 2021

Don't worry about the version, these packages will be release in ROS 2 for the first time. We can start with the version 0.0.0

@ahcorde
Copy link
Contributor

ahcorde commented May 27, 2021

In addition, should I change files in the main geometry_tutorials folder? Its run_depend, license and version tags should probably be updated too.

You should update both package.xml to include the <run_depend>s. for example:

  • turtle_tf2_launch should include: launch_ros and launch
  • turtle_tf2 should include: rclpy tf2_ros_py, geometry_msgs, turtlesim

Copy link

@audrow audrow left a comment

Choose a reason for hiding this comment

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

Looks good overall. I agree with @ahcorde's comments.

In your Python files, it would be great to add comments for what things do. This will help check your understanding and can be something you leverage when writing tutorials.

turtle_tf2/turtle_tf2/turtle_tf2_broadcaster.py Outdated Show resolved Hide resolved
turtle_tf2/turtle_tf2/turtle_tf2_broadcaster.py Outdated Show resolved Hide resolved
@kurshakuz kurshakuz requested a review from ahcorde May 29, 2021 06:50
Copy link

@audrow audrow left a comment

Choose a reason for hiding this comment

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

One quick comment.

turtle_tf2/turtle_tf2/turtle_tf2_broadcaster.py Outdated Show resolved Hide resolved
@ahcorde
Copy link
Contributor

ahcorde commented May 31, 2021

@kurshakuz
Copy link
Author

@kurshakuz you should also need to modify the metapackage https://github.com/kurshakuz/geometry_tutorials/blob/ros2/geometry_tutorials/package.xml

fixed now

geometry_tutorials/CHANGELOG.rst Show resolved Hide resolved
geometry_tutorials/package.xml Show resolved Hide resolved
geometry_tutorials/package.xml Show resolved Hide resolved
geometry_tutorials/package.xml Show resolved Hide resolved
geometry_tutorials/package.xml Show resolved Hide resolved
@kurshakuz kurshakuz requested a review from ahcorde June 1, 2021 03:25
geometry_tutorials/package.xml Outdated Show resolved Hide resolved
@kurshakuz kurshakuz requested a review from ahcorde June 1, 2021 10:06
Copy link

@audrow audrow left a comment

Choose a reason for hiding this comment

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

What you have here looks good to me.

I'm not sure what all should be included in this PR. If it's much more, we should retarget this to another branch (probably on your fork) and then make smaller PRs to that branch. Then when that's ready, we can make the PR targeted for ros:ros2.

It would also be nice to have a checklist for all of the files you plan to add; perhaps in a PR's description.

@kurshakuz
Copy link
Author

What you have here looks good to me.

I'm not sure what all should be included in this PR. If it's much more, we should retarget this to another branch (probably on your fork) and then make smaller PRs to that branch. Then when that's ready, we can make the PR targeted for ros:ros2.

It would also be nice to have a checklist for all of the files you plan to add; perhaps in a PR's description.

Understood. Let's then merge that PR and I will push further update PRs to my own branch.

This PR mainly include:

  • tf2 broadcaster
  • tf2 listener
  • static tf2 broadcaster

Broadcaster and listener nodes are used for the demonstration capabilities of tf2 as described in ros2/ros2_documentation#1628. More in-depth explanation of the code will be covered in further tutorials 1, 2. The static broadcaster will also be described in separate tutorial 3.

@kurshakuz kurshakuz requested a review from ahcorde June 4, 2021 14:01
turtle_tf2_py/package.xml Outdated Show resolved Hide resolved
@kurshakuz kurshakuz requested a review from ahcorde June 7, 2021 12:01
geometry_tutorials/package.xml Outdated Show resolved Hide resolved
turtle_tf2_py/package.xml Outdated Show resolved Hide resolved
kurshakuz and others added 3 commits June 7, 2021 19:54
Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
…_broadcaster.py

Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
Copy link

@audrow audrow left a comment

Choose a reason for hiding this comment

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

Looks good to me overall. I think it'd be nice to add some formatting tests.

turtle_tf2_py/turtle_tf2_py/turtle_tf2_listener.py Outdated Show resolved Hide resolved
turtle_tf2_py/setup.py Outdated Show resolved Hide resolved
Copy link

@audrow audrow left a comment

Choose a reason for hiding this comment

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

Looks good after a few copyright year changes.

turtle_tf2_py/test/test_copyright.py Outdated Show resolved Hide resolved
turtle_tf2_py/test/test_flake8.py Outdated Show resolved Hide resolved
turtle_tf2_py/test/test_pep257.py Outdated Show resolved Hide resolved
@kurshakuz
Copy link
Author

All fixed now.

@ahcorde ahcorde merged commit 0145301 into ros:ros2 Jun 9, 2021
@kurshakuz kurshakuz deleted the ros2 branch June 9, 2021 09:38
@kurshakuz kurshakuz mentioned this pull request Aug 9, 2021
23 tasks
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.

None yet

3 participants