Skip to content

Enable filters in humble #501

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

Open
wants to merge 21 commits into
base: humble
Choose a base branch
from

Conversation

barryridge
Copy link

Related thread: #423

I don't know if you folks accept PRs for previously released distros, i.e. humble in this case?

We've been doing some work on Jetson Orin Nano boards, which have not yet been bumped to noble/jazzy yet AFAIK, and needed to use the PCL filters, so I thought it would be a good idea to try to enable them in the pcl_ros humble build.

The general strategy I used was to cherry-pick the requisite filter updates from jazzy and merge them into humble. This required some slight modifications detailed as follows.

I modified pcl_ros::Filter::subscribe() to use humble-compatible message_filters::Subscriber::subscribe() calls, changing this from jazzy:

    // Subscribe to the input using a filter
    auto sensor_qos_profile =
      rclcpp::SensorDataQoS().keep_last(max_queue_size_).get_rmw_qos_profile();
    sub_input_filter_.subscribe(this, "input", sensor_qos_profile);
    sub_indices_filter_.subscribe(this, "indices", sensor_qos_profile);

to this:

    // Subscribe to the input using a filter
    auto sensor_qos_profile = rclcpp::SensorDataQoS().keep_last(max_queue_size_);
    sub_input_filter_.subscribe(this, "input", sensor_qos_profile.get_rmw_qos_profile());
    sub_indices_filter_.subscribe(this, "indices", sensor_qos_profile.get_rmw_qos_profile());

I also modified pcl_ros::Filter::createPublishers() to avoid the use of missing rclcpp::PublisherOptions.event_callbacks.matched_callback, swapping this in jazzy:

void
pcl_ros::Filter::createPublishers()
{
  auto pub_options = rclcpp::PublisherOptions();
  pub_options.event_callbacks.matched_callback = [this](rclcpp::MatchedInfo & /*info*/) {
      if (pub_output_->get_subscription_count() == 0) {
        unsubscribe();
      } else {
        if (use_indices_) {
          if (!sub_input_filter_.getSubscriber() || !sub_indices_filter_.getSubscriber()) {
            subscribe();
          }
        } else {
          if (!sub_input_) {
            subscribe();
          }
        }
      }
    };
  pub_output_ = create_publisher<PointCloud2>("output", max_queue_size_, pub_options);
}

for this:

void
pcl_ros::Filter::createPublishers()
{
  auto pub_options = rclcpp::PublisherOptions();
  if (use_indices_) {
    if (!sub_input_filter_.getSubscriber() || !sub_indices_filter_.getSubscriber()) {
      subscribe();
    }
  } else {
    if (!sub_input_) {
      subscribe();
    }
  }
  pub_output_ = create_publisher<PointCloud2>("output", max_queue_size_, pub_options);
}

I've tested all of the filters manually and they all build and appear to be working. I had some struggles getting pcl_ros::RadiusOutlierRemoval to work, but I suspect that may be due to a misconfiguration on my end rather than a problem with the filter itself.

I've also re-enabled the filter tests.

Let me know if you need any updates or if this is an inappropriate PR given that humble has already been released.

@Rayman
Copy link
Contributor

Rayman commented Jun 5, 2025

humble is still a supported distro, so features could be backported to it. However I have to make sure that we don't break the ABI and break compilation for downstream users. Are you sure backporting is safe?

In your PR you've copied a lot of files. This means I can't easily review this PR. It would be easier if you cherry-picked commits. After that I'd expect two small commits to fix the compilation issues you've encountered.

@barryridge
Copy link
Author

Thanks for the feedback @Rayman. I can have a go at doing what you suggest. I suppose it would be best to create a separate PR if taking that approach?

@Rayman
Copy link
Contributor

Rayman commented Jun 6, 2025

Yes you can create a new PR or force push on your old one. Either will work. A short summary:

  • branch off from the humble branch
  • git cherry-pick all relevant commits from the ros2 branch that you need
  • Finally put one or two commits on top that fix compilation for humble

@TomasCorral
Copy link

I also need it

@barryridge
Copy link
Author

@TomasCorral I'm afraid I haven't gotten around to following through on @Rayman's suggestion as of yet. In the meantime, you should be able to readily use this branch if you need the filters urgently.

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.

3 participants