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

Triggered camera / multicamera plugins #687

Merged

Conversation

scpeters
Copy link
Member

This adds a plugin for triggered cameras and multicameras. These sensors do not publish unless triggered. They have an additional topic (default name image_trigger) that subscribes to std_msgs/Empty messages and will publish a single update after being triggered.

Its maximum update rate is currently set by the <update_rate> sdf tag in the <sensor> block. Under the hood. If you publish two trigger messages before the camera has a chance to publish, it will only publish once, since a bool is used to keep track of the trigger status. The test reflects this behavior.

@scpeters
Copy link
Member Author

I filed an issue with gazebo because the first triggered image looks different (and wrong) compared to subsequent images:

double _hack_baseline)
{
GazeboRosCameraUtils::Load(_parent, _sdf, _camera_name_suffix, _hack_baseline);

Copy link
Contributor

Choose a reason for hiding this comment

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

add another this->SetCameraEnabled(false); here in this overloaded Load function?

Copy link
Member Author

Choose a reason for hiding this comment

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

good call

Copy link
Member Author

Choose a reason for hiding this comment

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

squashed into 7875cbd

ros::spinOnce();
ros::Duration(0.1).sleep();
}
EXPECT_EQ(images_received_, 2);
Copy link
Contributor

Choose a reason for hiding this comment

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

is there logic to filter out mgs close together? I'm getting an error:

[gazebo_plugins.rosunit-triggered_camera/cameraSubscribeTest][FAILURE]----------
/home/osrf/code/ian/gz_ws/src/gazebo_ros_pkgs/gazebo_plugins/test/camera/triggered_camera.cpp:103
Value of: 2
Expected: images_received_
Which is: 3

but the tests are passing on jenkins..

Copy link
Member Author

Choose a reason for hiding this comment

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

hm...it is kind of a race condition, but it was pretty consistent on my machine

are you consistently getting this result?

Copy link
Contributor

@iche033 iche033 Mar 16, 2018

Choose a reason for hiding this comment

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

It was pretty consistent when I was testing it before but I just ran the test 10 more times and it only failed once so likely a race condition.

Copy link
Member Author

Choose a reason for hiding this comment

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

an alternative is to try to change the behavior so that each trigger signal leads to a new image. this would require a slight refactoring (use int instead of bool in a few places) but would be more predictable

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I've fixed the race condition in cb9f299. Now the plugin uses an int to keep track of the number of triggers received from the topic.

@scpeters
Copy link
Member Author

I just rebased on kinetic-devel in order to get the renamed distortion tests.

@iche033
Copy link
Contributor

iche033 commented Apr 6, 2018

mmm the new builds are failing because cv_bridge is not found

@j-rivero
Copy link
Contributor

j-rivero commented Apr 6, 2018

@scpeters would you mind fixing the conflicts when you have some free time? Thanks.

@scpeters
Copy link
Member Author

@j-rivero sorry for the delay; I just resolved the conflicts

@scpeters
Copy link
Member Author

I tested locally with gazebo9 and the tests pass except for the barrel distortion test (as expected due to #681 )

@scpeters
Copy link
Member Author

mmm the new builds are failing because cv_bridge is not found

maybe we aren't declaring an explicit dependency on it; let me check

@scpeters
Copy link
Member Author

Someone else had the cv_bridge problem too ( ros-perception/vision_opencv#209 ), and it's been fixed and released in ros-kinetic-cv-bridge 1.12.8, which is currently in shadow-fixed. I'll check to see when that will sync.

@scpeters
Copy link
Member Author

@ros-pull-request-builder retest this please

1 similar comment
@scpeters
Copy link
Member Author

@ros-pull-request-builder retest this please

@scpeters
Copy link
Member Author

@ros-pull-request-builder retest this please

@scpeters
Copy link
Member Author

The builds are now unstable, with just the distortion camera test failures

@iche033
Copy link
Contributor

iche033 commented May 3, 2018

just tested everything again, works as expected.

@scpeters scpeters merged commit af49013 into ros-simulation:kinetic-devel May 7, 2018
@scpeters scpeters deleted the triggered_cameras_kinetic branch May 7, 2018 07:34
@j-rivero
Copy link
Contributor

I've added the missing entry to SENSORS.md in f1b28c9

Roboterbastler pushed a commit to Roboterbastler/gazebo_ros_pkgs that referenced this pull request Jul 6, 2018
* adds triggered cameras and multicameras

* fix threaded event connection

* use correct timestamp for images

* remove compiler directives for old gazebo versions

* update copyright dates and remove copied comments

* test for triggered_camera

* expect cameras don't subscribe to trigger topic

* subscribe to trigger if CanTrigger is true

* fix untriggered first image

* fix race condition

* change triggered camera test name

* fix test

* fix 16bit test name
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