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

[ros2] Camera and triggered camera #827

Merged
merged 15 commits into from
Dec 11, 2018
Merged

[ros2] Camera and triggered camera #827

merged 15 commits into from
Dec 11, 2018

Conversation

chapulina
Copy link
Contributor

@chapulina chapulina commented Sep 18, 2018

See ROS 2 Migration: Camera.

This needs ros-perception/image_common#95 .

TODOs:

  • This PR uses features not available on Bouncy, so all CI will fail. FYI @j-rivero

  • I didn't have time to port dynamic reconfigure. I left the ROS1 code commented out. Feel free to implement it if you feel inspired or I can take it when I get back.

  • The gazebo_ros_camera_distortion test fails on teardown because the ROS node's destructor hangs forever. Not sure yet if the problem is coming from rclcpp, gazebo_ros or image_transport.


Demos

All instructions to run the demos can be found on their world files.

gazebo_ros_camera_demo

gazebo_ros_camera_demo

gazebo_ros_triggered_camera_demo

gazebo_ros_camera_triggered_demo

gazebo_ros_camera_distortion_barrel_demo

gazebo_ros_camera_distortion_barrel_demo

gazebo_ros_camera_distortion_pincushion_demo

gazebo_ros_camera_distortion_pincushion_demo


@tfoote tfoote added the ros2 label Sep 18, 2018
Copy link

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

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

Generally looks good to me.

I'm interested to know why the ros_node_.reset() causes issues here. May be worth adding an issue to track it if you haven't already.

@chapulina
Copy link
Contributor Author

chapulina commented Oct 16, 2018

Thanks for the review, @mjcarroll ! I'll take another look at the node reset issue this week and ticket an issue if I can't make progress.

We should only merge this PR once we have CI working for it. I think this means we need:

@mjcarroll
Copy link

message_filters: ros2/ros2#588

@chapulina
Copy link
Contributor Author

@chapulina
Copy link
Contributor Author

Updated this PR on da044a5 to use image_transport's topic remapping: ros-perception/image_common#97

@chapulina
Copy link
Contributor Author

So after today's meeting, it was decided that this PR will wait until there are Crystal debs available, so we can use those on the OSRF buildfarm for CI. Until then, gazebo_ros_pkgs's ros2 branch should keep working against both Bouncy debs and the master ros2.repos built from source.

@chapulina
Copy link
Contributor Author

Thanks @mjcarroll for handling the release of image_common. I just tested this branch against crystal debs and everything works fine 🎉

I'm trying to adapt the OSRF build farm CI to use crystal here. Will merge this and the other 2 PRs targeted at ros2 before making the Crystal release.

@chapulina
Copy link
Contributor Author

OSRF build farm CI: Build Status

  • the sim time tests are also failing in the ros2 branch
  • not sure yet why the camera tests are failing in Jenkins, they pass for me locally
  • not sure what's going on with uncrustify either

@chapulina
Copy link
Contributor Author

@ros-pull-request-builder retest this please

@j-rivero
Copy link
Contributor

@osrf-jenkins run tests please

@chapulina
Copy link
Contributor Author

Not going to delay this further, merging now even though there's no CI to test it. All pass locally, we should revisit once we have GPU-enabled CI working.

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