search

ros2 / ros_astra_camera

ROS wrapper for Astra camera
9 stars 12 forks source link

Disable various channels with params #18

Closed Kukanani closed 7 years ago

Kukanani commented 7 years ago

Allow disabling the use of RGB, IR, and/or depth by using the parameters passed to the Astra driver. The astra_camera_node also now supports toggling these parameters using the flags -I, -C, and -D.

Split the IR streaming code into another function to avoid code duplication.

clalancette commented 7 years ago

@Kukanani and I discussed this, and given that this isn't necessary right now, and given the current schedule, we decided to wait on this until after beta2. After that, I think it is a good change so we should review it and merge. Thus, I've moved this back to in progress for now.

mikaelarguedas commented 7 years ago

Just to confirm my understanding, the decision is to keep all these streams on for the beta and make them configurable with this PR after the release, is that correct?

clalancette commented 7 years ago

Yeah, exactly.

Kukanani commented 7 years ago

Testing should be as simple as:

# no IR
astra_camera_node -I

# no RGB (Color)
astra_camera_node -C

# no depth
astra_camera_node -D

# multiple
astra_camera_node -D -C -I

These should all produce "Astra camera disabled" output

Kukanani commented 7 years ago

I wasn't sure if I should set the parameters on the public or private node handle; technically these parameters are private to the node (thinking in ROS 1 terms), but I don't think this applies anymore.

On Tue, Jun 13, 2017 at 4:09 PM, Chris Lalancette notifications@github.com wrote:

@clalancette commented on this pull request.

In ros/astra_camera_node.cpp https://github.com/ros2/ros_astra_camera/pull/18#discussion_r121819455:

// TODO(clalancette): parsing the command-line options here is temporary until // we get parameters working in ROS2.

  • if (!parse_command_options(argc, argv, &width, &height, &framerate, &dwidth, &dheight, &dframerate, &dformat)) {
  • if (!parse_command_options(argc, argv, &width, &height, &framerate, &dwidth, &dheight, &dframerate, &dformat, &use_ir, &use_color, &use_depth)) { return 1; }

rclcpp::init(argc, argv); rclcpp::node::Node::SharedPtr n = rclcpp::node::Node::make_shared("astra_camera"); rclcpp::node::Node::SharedPtr pnh = rclcpp::node::Node::make_shared("astracamera");

Yeah, I totally agree. That's one of the things that really needs to be cleaned up in this node, but I ran out of time to do it. I think we should address it after beta2.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ros2/ros_astra_camera/pull/18#discussion_r121819455, or mute the thread https://github.com/notifications/unsubscribe-auth/AGkvH9khYsga6RPUG79sJyVYmkv7k_glks5sDxbAgaJpZM4N3kG3 .

-- Adam