ros-drivers / openni2_camera

ROS wrapper for openni 2.0
http://wiki.ros.org/openni2_camera
BSD 3-Clause "New" or "Revised" License
56 stars 95 forks source link

Remove unused ROS Parameters. #76

Closed 130s closed 5 years ago

130s commented 6 years ago

DONOT merge yet but thoughts are welcomed.

auto_exposure is defined as both ROS Parameter and Dynamic Reconfigure entry as you see in the following search result, and in the code only those dynamic reconf version is used. In this PR Param versions are removed except args in launch file to retain backward compatibility of launch files.

There seems to be other Parameters in the same situation. I'll add those params too. -> UPDATE 20181019 I can't find them.

CC @MauleshSTrivedi who hinted for this finding.

$ git log -1
commit 21ea067c02dbb2f576410208fcb45b79e6fb1521
Author: Isaac I.Y. Saito <130s@2000.jukuin.keio.ac.jp>
Date:   Fri Nov 3 20:09:21 2017 -0700

    0.3.0

$ ack-grep -B 1 -A 1 -i auto_exposure .
openni2_camera/include/openni2_camera/openni2_driver.h
177-
178:  bool auto_exposure_;
179-  bool auto_white_balance_;

openni2_camera/cfg/OpenNI2.cfg
30-gen.add("color_depth_synchronization", bool_t, 0, "Synchronization of color and depth camera", False)
31:gen.add("auto_exposure", bool_t, 0, "Auto-Exposure", True)
32-gen.add("auto_white_balance", bool_t, 0, "Auto-White-Balance", True)

openni2_camera/src/openni2_driver.cpp
203-
204:  auto_exposure_ = config.auto_exposure;
205-  auto_white_balance_ = config.auto_white_balance;
--
298-  {
299:    if (!config_init_ || (old_config_.auto_exposure != auto_exposure_))
300:      device_->setAutoExposure(auto_exposure_);
301-  }
--
320-  // this check is always performed and exposure set.
321:  if( (!auto_exposure_ && !auto_white_balance_) && exposure_ != 0 )
322-  {
--
931-        // white balance are disabled, and FIXED exposure is used instead.
932:        if((!auto_exposure_ && !auto_white_balance_ ) && exposure_ == 0)
933-        {
--
944-          ROS_WARN_STREAM("Resetting auto exposure and white balance to previous values");
945:          device_->setAutoExposure(auto_exposure_);
946-          device_->setAutoWhiteBalance(auto_white_balance_);

openni2_launch/launch/includes/device.launch
11-  <arg name="color_depth_synchronization" default="false" />
12:  <arg name="auto_exposure" default="true" />
13-  <arg name="auto_white_balance" default="true" />

openni2_launch/launch/includes/device.launch.xml
18-  <arg name="color_depth_synchronization" default="false" />
19:  <arg name="auto_exposure" default="true" />
20-  <arg name="auto_white_balance" default="true" />
--
47-    <param name="color_depth_synchronization" value="$(arg color_depth_synchronization)" />
48:    <param name="auto_exposure" value="$(arg auto_exposure)" />
49-    <param name="auto_white_balance" value="$(arg auto_white_balance)" />

openni2_launch/launch/openni2_tf_prefix.launch
26-  <arg name="color_depth_synchronization"     default="false" />
27:  <arg name="auto_exposure"                   default="true" />
28-  <arg name="auto_white_balance"              default="true" />
--
85-      <arg name="color_depth_synchronization"     value="$(arg color_depth_synchronization)" />
86:      <arg name="auto_exposure"                   value="$(arg auto_exposure)" />
87-      <arg name="auto_white_balance"              value="$(arg auto_white_balance)" />

openni2_launch/launch/openni2.launch
25-  <arg name="color_depth_synchronization"     default="false" />
26:  <arg name="auto_exposure"                   default="true" />
27-  <arg name="auto_white_balance"              default="true" />
--
84-      <arg name="color_depth_synchronization"     value="$(arg color_depth_synchronization)" />
85:      <arg name="auto_exposure"                   value="$(arg auto_exposure)" />
86-      <arg name="auto_white_balance"              value="$(arg auto_white_balance)" />
130s commented 5 years ago

There seems to be other Parameters in the same situation. I'll add those params too.

I can't find them. Merging.

mpetersen94 commented 4 years ago

What is the right way to turn off white balance now? I recently updated from a rather old compiled version of this repo where this parameter turned white balance off.