Skip to content

Fix ros2 compilation warnings#4532

Merged
zimmy87 merged 2 commits intomicrosoft:masterfrom
alonfaraj:fix-ros2-warnings
Jun 1, 2022
Merged

Fix ros2 compilation warnings#4532
zimmy87 merged 2 commits intomicrosoft:masterfrom
alonfaraj:fix-ros2-warnings

Conversation

@alonfaraj
Copy link
Contributor

About

Fix some -Wformat and -Wdeprecated warnings for ros2 wrapper.

How Has This Been Tested?

Build the wrapper and make sure warnings are gone.

Screenshots (if appropriate):

@alonfaraj alonfaraj changed the title Fix ros2 warnings Fix ros2 compilation warnings May 25, 2022
@zimmy87
Copy link
Contributor

zimmy87 commented Jun 1, 2022

Thanks for submitting this PR @alonfaraj! I tested this locally and it's working for me, so I am moving ahead with merging.

@zimmy87 zimmy87 merged commit caa1318 into microsoft:master Jun 1, 2022
@nikola-j
Copy link

nikola-j commented Jun 6, 2022

Is using ros2 foxy supported? This breaks compatibility by causing the following error:

AirSim/ros2/src/airsim_ros_pkgs/src/airsim_ros_wrapper.cpp: In member function ‘sensor_msgs::msg::PointCloud2 AirsimROSWrapper::get_lidar_msg_from_airsim(const msr::airlib::LidarData&, const string&, const string&) const’:
AirSim/ros2/src/airsim_ros_pkgs/src/airsim_ros_wrapper.cpp:685:138: error: ‘from_nanoseconds’ is not a member of ‘rclcpp::Duration’
  685 |                 auto transformStampedENU = tf_buffer_->lookupTransform(AIRSIM_FRAME_ID, vehicle_name, rclcpp::Time(0), rclcpp::Duration::from_nanoseconds(1));
      |                                                                                                                                          ^~~~~~~~~~~~~~~~

It can be fixed by reverting that line.

@alonfaraj
Copy link
Contributor Author

@nikola-j sorry about that.
airsim CI is running on ros2 galactic so this error wasn't caught.
The maintainer should decide whether the airsim should support foxy/galactic/humble, or just one, and modify the CI accordingly.
As an example, the old code compiles on foxy but doesn't on humble, AFAIK.
@zimmy87 what do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants