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

add cpp version for urdf_tutorial #4926

Open
wants to merge 12 commits into
base: rolling
Choose a base branch
from

Conversation

Demonmasterlqx
Copy link
Contributor

No description provided.

some command to launch debug demo is wrong and correct it

Signed-off-by: Demonmasterlqx <[email protected]>
…obot-State-Publisher-py.rst

change name

Signed-off-by: Demonmasterlqx <[email protected]>
add cpp version

Signed-off-by: Demonmasterlqx <[email protected]>
add 
.. redirect-from::

    Tutorials/URDF/Using-URDF-with-Robot-State-Publisher

Signed-off-by: Demonmasterlqx <[email protected]>
Copy link
Collaborator

@kscottz kscottz left a comment

Choose a reason for hiding this comment

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

Hi @Demonmasterlqx, thank you so much for your contribution! We really appreciate it.

I sent over a bunch of suggestions. Before I approve this I would like to see more comments in main C++ source file. It really helps users when we document the example code in excruciating detail.

Other than that I think we have to discuss the copyright of the code cited in pull request. I don't think it is a problem, but we need some clarity on how to handle it appropriately.

-------

You created a ``JointState`` publisher node and coupled it with ``robot_state_publisher`` to simulate a walking robot.
The code used in these examples is originally from `here <https://github.com/benbongalon/ros2-migration/tree/master/urdf_tutorial>`__.
Copy link
Collaborator

Choose a reason for hiding this comment

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

@fujitatomoya how should we handle the copyright on this kind of stuff?

The code used in these examples is originally from `here <https://github.com/benbongalon/ros2-migration/tree/master/urdf_tutorial>`__.

Credit is given to the authors of this
`ROS 1 tutorial <http://wiki.ros.org/urdf/Tutorials/Using%20urdf%20with%20robot_state_publisher>`__
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know if we need these lines.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I just copy these form python version. So I tend to delete them. QWQ

@Demonmasterlqx
Copy link
Contributor Author

Hi @kscottz ! Thank you so much for helping me to polish my words and give some suggestion. I have add some code comment into code blocks and polish my words


To do this, we must specify all three joints and the overall robot geometry.

Fire up your favorite editor and paste the following code into
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Fire up your favorite editor and paste the following code into
Fire up your favorite editor and paste the following code into

#include <cmath>
#include <thread>
#include <chrono>

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change

Copy link
Collaborator

@kscottz kscottz left a comment

Choose a reason for hiding this comment

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

I went through and added a few suggestions. I'll approve the PR under the assumption that you'll approve those and that you pass the linter.

You had a long list of linter whitespace errors that needed to be fixed. I think I fixed them all for you but there might be a few stragglers.

You need one more review before we can merge this in.

#include <chrono>

using namespace std::chrono;

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change


class StatePublisher : public rclcpp::Node{
public:

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change

// create a broadcaster to tell the tf2 state information
// this broadcaster will determine the position of coordinate system 'asix' in coordinate system 'odom'
RCLCPP_INFO(this->get_logger(),"Starting state publisher");

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change

RCLCPP_INFO(this->get_logger(),"Starting state publisher");

loop_rate_=std::make_shared<rclcpp::Rate>(33ms);

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change

return 0;
}

This file will send the ``joint_state`` to ``robot_state_publisher`` and ``robot_state_publisher`` will tell tf2 how to place model.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
This file will send the ``joint_state`` to ``robot_state_publisher`` and ``robot_state_publisher`` will tell tf2 how to place model.
This file will send ``joint_state`` values to ``robot_state_publisher`` which in turn will tell tf2 how to place model.


This file will send the ``joint_state`` to ``robot_state_publisher`` and ``robot_state_publisher`` will tell tf2 how to place model.

This file will also tell ``tf2`` how to place the whole model by ``broadcaster``
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
This file will also tell ``tf2`` how to place the whole model by ``broadcaster``
The code file will also tell ``tf2`` how to place the whole model using the ``transform_broadcaster``

rclcpp::TimerBase::SharedPtr timer_;

//Robot state variables

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change

};

void StatePublisher::publish(){
// create msg subject
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// create msg subject
// create the necessary messages


ros2 launch urdf_tutorial_cpp launch.py

To visualize your results you will need to open a new terminal and run Rviz using your rviz configuration file.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
To visualize your results you will need to open a new terminal and run Rviz using your rviz configuration file.
To visualize your results you will need to open a new terminal and run RViz using your RViz configuration file.

6 Install the package
^^^^^^^^^^^^^^^^^^^^^

To visualize the results you will need to open a new terminal and run RViz using your RViz configuration file.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
To visualize the results you will need to open a new terminal and run RViz using your RViz configuration file.
To visualize the results you will need to open a new terminal and run RViz using your RViz configuration file.

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.

2 participants