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

Deprecate openni_kinect in favor of depth_camera #757

Open
wants to merge 2 commits into
base: melodic-devel
Choose a base branch
from

Conversation

kev-the-dev
Copy link
Collaborator

@kev-the-dev kev-the-dev commented Jun 29, 2018

For several years there has been two nearly identical plugins, openni_kinnect and depth_camera. This is confusing for users and had lead to several bugs from not porting changes in one to the other.

This PR makes openni_kinect inherit from depth_camera and print a deprecation warning on load so users will switch to depth_camera. Some fixes/features from openni_kinnect are also ported into depth_camera.

The API for using both plugins should remain the same, but ABI for both had to be broken (this isn't very important as they are dynamically loaded anyways).

Also added a test for openni_kinnect to ensure it still works

Implements #127
Replaces #749
Fixes #569
Replaces #570

//ROS_ERROR_NAMED("depth_camera", "camera_ new frame %s %s",this->parentSensor_->GetName().c_str(),this->frame_name_.c_str());
# if GAZEBO_MAJOR_VERSION >= 7
=======
>>>>>>> abf673f... GAZEBO_PLUGINS: Deprecate openni_kinect
Copy link
Contributor

Choose a reason for hiding this comment

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

Some merge leftovers :)

this->point_cloud_cutoff_ = 0.4;
ROS_INFO_NAMED("depth_camera", "Tag <pointCloudCutoff> not set, defaulting to %f", this->point_cloud_cutoff_);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe use plugin name from from sdf instead of depth_camera for multiple camera support

@marting87
Copy link
Contributor

Related review: #759

@kev-the-dev kev-the-dev force-pushed the issue-127 branch 4 times, most recently from 89d8ad4 to 4c56d70 Compare July 7, 2018 00:15
@hvpandya-TRI
Copy link

Any updates on this?


namespace gazebo
{
class GazeboRosOpenniKinect : public DepthCameraPlugin, GazeboRosCameraUtils
class GazeboRosOpenniKinect : public GazeboRosDepthCamera
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a big change to make in a release branch since it should break API.

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.

5 participants