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 tianracer_core to ROS2 #4

Merged
merged 33 commits into from
Nov 6, 2021

Conversation

chargerKong
Copy link

@chargerKong chargerKong commented Nov 4, 2021

#3 This PR is a port of tianracer_core to ROS 2.

Although the code can run normally, there is still a problem. After starting the launch file, the log in serial.cpp will not be printed

BTW, cmd_vel_to_ackermann_drive.py is separated from package navigation, because I don’t know of any way to write both C++ nodes and python nodes in one package in ROS2.
This separation may also be inappropriate. We can readjust this file structure.

rclcpp::Publisher<geometry_msgs::msg::Pose2D>::SharedPtr uwb_pub_;
rclcpp::Publisher<sensor_msgs::msg::Imu>::SharedPtr imu_pub_;
rclcpp::Subscription<ackermann_msgs::msg::AckermannDrive>::SharedPtr ackermann_sub_;
std::shared_ptr<tf2_ros::TransformBroadcaster> tf_broadcaster_;
Copy link
Author

Choose a reason for hiding this comment

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

oh, I suddenly find this tf_broadcaster_ can be removed since there is no tf to be published 😅

Copy link
Member

Choose a reason for hiding this comment

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

The tf_broadcaster can be kept. Later we can fusion imu and encoder in tianracer_core.

# Whether to broadcast the transformation over the /tf topic. Defaults to true if unspecified.
publish_tf: true

predict_to_curent_time: true
Copy link
Member

Choose a reason for hiding this comment

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

The original repo has a typo.
This param should be predict_to_current_time

false, false, true,
false, false, false,
false, false, false,
false, false, false,]
Copy link
Member

Choose a reason for hiding this comment

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

Better remove the last comma.

Copy link
Member

@tianb03 tianb03 left a comment

Choose a reason for hiding this comment

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

The teleop ackman repo changes cmd_vel twist msg to ackermann_drive msg.
Racecar is not suitable for a keyboard teleop but only for joystick.
A better way is to map the joystick axes to steering and throttle respectively in ackerman_drive_msgs.
Let's leave this to the next PR.

@tianb03 tianb03 merged commit 6815d03 into tianbot:dashing-devel Nov 6, 2021
@chargerKong
Copy link
Author

The teleop ackman repo changes cmd_vel twist msg to ackermann_drive msg. Racecar is not suitable for a keyboard teleop but only for joystick. A better way is to map the joystick axes to steering and throttle respectively in ackerman_drive_msgs. Let's leave this to the next PR.

Maybe it's already done.
Do you mean this node @tianb03

@tianb03
Copy link
Member

tianb03 commented Nov 8, 2021

The teleop ackman repo changes cmd_vel twist msg to ackermann_drive msg. Racecar is not suitable for a keyboard teleop but only for joystick. A better way is to map the joystick axes to steering and throttle respectively in ackerman_drive_msgs. Let's leave this to the next PR.

Maybe it's already done. Do you mean this node @tianb03

I mean this may not be a proper way to teleop the racecar. Could you please take a look at https://github.com/tianbot/tianracer/blob/master/tianracer_teleop/script/tianracer_joy.py

@chargerKong
Copy link
Author

I mean this may not be a proper way to teleop the racecar. Could you please take a look at https://github.com/tianbot/tianracer/blob/master/tianracer_teleop/script/tianracer_joy.py

OK. Do we need to delete the teleop ackman repo later that I've created?

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.

4 participants