Replace deprecated spin_until_future_complete with spin_until_complete

ros2 / ros1_bridge

ROS 2 package that provides bidirectional communication between ROS 1 and ROS 2

Apache License 2.0

425 stars 275 forks source link

Replace deprecated spin_until_future_complete with spin_until_complete #350

Open hliberacki opened 2 years ago

hliberacki commented 2 years ago

Signed-off-by: Hubert Liberacki hliberacki@gmail.com

hliberacki commented 2 years ago

Due to change in RCLCPP - https://github.com/ros2/rclcpp/pull/1874 Pull request

gbiggs commented 2 years ago

Running CI:

gbiggs commented 2 years ago

It looks like you missed one.

hliberacki commented 2 years ago

It looks like you missed one.

Sorry, but which one do you mean? I've checked (hopefully correctly) and there is only a single call to spin_until_future_complete in this repository ;). Unless you meant something else

gbiggs commented 2 years ago

The offending function is at ros1_bridge/test/test_ros2_client.cpp:36. And I made a mistake, it's not one you missed, it appears to be a missing include. Check the bottom of this output log.

hliberacki commented 2 years ago

The offending function is at ros1_bridge/test/test_ros2_client.cpp:36. And I made a mistake, it's not one you missed, it appears to be a missing include. Check the bottom of this output log.

yes, that was because my changes to rclcpp are still not merged yet. Since the old method needs to be deprecated when new is introduced - merging it first would cause a warning which would break the ci.

the main PR has CI with all of my changes :)

hliberacki commented 2 years ago

https://github.com/ros2/rclcpp/pull/1874#issuecomment-1086906375 Passing CI with all related PRs linked and build together.

methylDragon commented 2 years ago

Just noting here that this is still pending the merge of:

https://github.com/ros2/rclcpp/pull/1874

gbiggs commented 2 years ago

Since rclcpp#1874 has been merged, let's try a CI run.

hliberacki commented 2 years ago

Since rclcpp#1874 has been merged, let's try a CI run.

Sorry for the confusion, unfortunately there were dependencies which should've been merged together, and due to the fact that there are review findings in rclpy - this PR was reverted until all PRs are in correct shape. rclcpp#1874 - has all desprition about it.

gbiggs commented 2 years ago

That explains the failing CI then. Ping this PR when it's ready to go, then, so I don't forget about it.