-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
fix ROS topic names #3880
fix ROS topic names #3880
Conversation
update vel_cmd_world_frame and odom_local_ned topic names to include vehicle name
while(vehicle_name == "") { | ||
nh_private_.getParam("/vehicle_name", vehicle_name); | ||
ROS_INFO_STREAM("Waiting vehicle name"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is a loop needed here? Also, getParam returns a bool whether the param exists or not, that can also be used for error checking. If needed, the listVehicles
API could also be used for this ig
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think a better solution could be addressed, this works but I don't think that is the most elegant solution (I did it so I can ensure that hahaha). Two problems I can remember:
-
That nasty loop
-
Multiple vehicles (this only if we think in a future multi-vehicle implementation of pd position controller)
-
Can be addressed by creating a service with String[] msg with the name of the vehicles and then using ros::service::waitForService("/vehicle_name_srv", -1)) function. (maybe a timeout of 100 sec instead infinity). So we have a service that provides the name of vehicles. My start thought was that names are static so a rosparam could be the solution. The while waiting is because the rosparam is set in the settings.json parser.
-
I think pd position controller doesn't support multiple vehicles (am I right, no?) this could be addressed by other PR but with the solution in 1, it's possible because we can get all the names and create the publisher for each one. Then a few changes in the methods to support the multiple independent controllers.
David. can you solve the conflict, please? |
fix clang-format check
Thanks David! |
Fixes: #3805
Fixes: #2602
Fixes: #3878
About
This PR updates vel_cmd_world_frame and odom_local_ned topic names to include vehicle name so that communication between PIDPositionController and AirsimROSWrapper. The changes for updating the odom_local_ned topic comes from @leoborgnino's work on #2789
How Has This Been Tested?
ROS node was built with changes applied and the repro steps in #3805 were followed
Screenshots (if appropriate):