Add downsampleCloudMsg function to noetic-devel

[h-wata]

PR Type

• Feature
• Bug fix
• Refactor
• Documentation
• Other

Overview

@Alpaca-zip mentioned that there is a plan to add the downsampleCloudMsg feature to noetic-devel, as mentioned in the comment below, which is why this function is being added.

• Adding excluded points publisher in track_with_cloud_node. #57 (comment)

Detail

Test

• I have tested this change with gazebo

Attention

• There's no rush, so please review when you have time. Feel free to point out any issues with the implementation.

[Alpaca-zip]

Thank you for submitting a Pull Request for my upcoming tasks. I appreciate your contribution and will proceed with a code review for this PR.

• The function msg2TransformedCloud() appears to be unused and therefore seems unnecessary. You can find it here:
  Link to code snippet

• The usage of cloud2TransformedCloud() is correct in the following part of the code:
  Link to code snippet
  However, the way cloud2TransformedCloud() is utilized here seems to be incorrect:
  Link to code snippet
  It should actually contain the following transformation:

  geometry_msgs::TransformStamped tf =
          tf_buffer_->lookupTransform(cam_model_.tfFrame(), header.frame_id, header.stamp);

  Therefore, it would be beneficial to modify cloud2TransformedCloud() to take the source and target frames as arguments to increase its versatility.

• Regarding the GitHub Actions, all checks are failing due to an oversight on my part. It seems that removing the following lines should fix the issue:
  Link to CI configuration
  This fix is beyond the scope of this PR, so I will address it in a separate PR. However, if it's of concern to you, feel free to make the correction within this PR.

[Alpaca-zip]

I have checked the changes made so far. I would appreciate it if you could confirm the following points.

• In the cloud2TransformedCloud() method described here:

  ultralytics_ros/src/tracker_with_cloud_node.cpp

  Line 57 in 41753e2

  transformed_cloud = cloud2TransformedCloud(downsampled_cloud, cam_model_.tfFrame(), cloud_msg->header);

  it is necessary to transform the point cloud from header.frame_id (the LiDAR frame) to cam_model_.tfFrame() (the camera frame).

  On the other hand, in the cloud2TransformedCloud() method described here:

  ultralytics_ros/src/tracker_with_cloud_node.cpp

  Line 96 in 41753e2

  cloud2TransformedCloud(detection_cloud_raw, cam_model_.tfFrame(), header);

  it is necessary to transform the point cloud from cam_model_.tfFrame() (the camera frame) to header.frame_id (the LiDAR frame).

  Currently, the implementation of cloud2TransformedCloud() in both cases is transforming the pointcloud from cam_model_.tfFrame() (the camera frame) to header.frame_id (the LiDAR frame).

  Here is an example implementation of cloud2TransformedCloud() that you might find useful:

  ultralytics_ros/src/tracker_with_cloud_node.cpp

  Lines 245 to 263 in a8a6a7e

  	TrackerWithCloudNode::cloud2TransformedCloud(const pcl::PointCloud<pcl::PointXYZ>::Ptr& cloud,
  	const std::string& source_frame, const std::string& target_frame,
  	const rclcpp::Time& stamp)
  	{
  	try
  	{
  	geometry_msgs::msg::TransformStamped tf_stamped = tf_buffer_->lookupTransform(target_frame, source_frame, stamp);
  	Eigen::Affine3f eigen_transform = tf2::transformToEigen(tf_stamped.transform).cast<float>();
  	pcl::PointCloud<pcl::PointXYZ>::Ptr transformed_cloud(new pcl::PointCloud<pcl::PointXYZ>());
  	transformPointCloud(cloud, transformed_cloud, eigen_transform);
  	return transformed_cloud;
  	}
  	catch (tf2::TransformException& e)
  	{
  	RCLCPP_WARN(this->get_logger(), "%s", e.what());
  	return cloud;
  	}
  	}

  Notice
  Please be aware that the first argument of lookupTransform() is target_frame, and the second argument is source_frame.
  Link to reference

• The following section means explicitly specifying the source branch of the pull request for checkout, but it seems that this method cannot find branches from forks. It appears that actions/checkout@v2 can automatically checkout?(*) the source branch of the pull request on GitHub Actions, so it is necessary to delete the following sections that are being used:

  with:
    ref: ${{ github.head_ref }}

  Specifically, these are:

  • ultralytics_ros/.github/workflows/noetic-ci.yml

    Lines 23 to 24 in 41753e2

    	with:
    	ref: ${{ github.head_ref }}

  • ultralytics_ros/.github/workflows/noetic-ci.yml

    Lines 37 to 38 in 41753e2

    	with:
    	ref: ${{ github.head_ref }}

  • ultralytics_ros/.github/workflows/noetic-ci.yml

    Lines 50 to 51 in 41753e2

    	with:
    	ref: ${{ github.head_ref }}

  • ultralytics_ros/.github/workflows/noetic-ci.yml

    Lines 65 to 66 in 41753e2

    	with:
    	ref: ${{ github.head_ref }}

  • ultralytics_ros/.github/workflows/noetic-ci.yml

    Lines 93 to 94 in 41753e2

    	with:
    	ref: ${{ github.head_ref }}

  • ultralytics_ros/.github/workflows/noetic-docker-build-check.yml

    Lines 16 to 17 in 41753e2

    	with:
    	ref: ${{ github.head_ref }}

---

英語があまり得意な方ではないので、ミスリーディングを減らすために日本語も残しておきます。
以下の点ご確認いただけますと幸いです。

• 以下のcloud2TransformedCloud()では、header.frame_id(LiDARのframe)からcam_model_.tfFrame()(カメラのframe)に点群を変換する必要があります。

  ultralytics_ros/src/tracker_with_cloud_node.cpp

  Line 57 in 41753e2

  transformed_cloud = cloud2TransformedCloud(downsampled_cloud, cam_model_.tfFrame(), cloud_msg->header);

  一方で以下のcloud2TransformedCloud()では、cam_model_.tfFrame()(カメラのframe)からheader.frame_id(LiDARのframe)に点群を変換する必要があります。

  ultralytics_ros/src/tracker_with_cloud_node.cpp

  Line 96 in 41753e2

  cloud2TransformedCloud(detection_cloud_raw, cam_model_.tfFrame(), header);

  しかしながら現在の実装では、上記の両方においてcloud2TransformedCloud()がcam_model_.tfFrame()(カメラのframe)からheader.frame_id(LiDARのframe)に点群を変換するようになってしまっています。

  ROS 2版ですがcloud2TransformedCloud()の実装例が以下にございますのでこちらを是非参考にしてください。

  ultralytics_ros/src/tracker_with_cloud_node.cpp

  Lines 245 to 263 in a8a6a7e

  	TrackerWithCloudNode::cloud2TransformedCloud(const pcl::PointCloud<pcl::PointXYZ>::Ptr& cloud,
  	const std::string& source_frame, const std::string& target_frame,
  	const rclcpp::Time& stamp)
  	{
  	try
  	{
  	geometry_msgs::msg::TransformStamped tf_stamped = tf_buffer_->lookupTransform(target_frame, source_frame, stamp);
  	Eigen::Affine3f eigen_transform = tf2::transformToEigen(tf_stamped.transform).cast<float>();
  	pcl::PointCloud<pcl::PointXYZ>::Ptr transformed_cloud(new pcl::PointCloud<pcl::PointXYZ>());
  	transformPointCloud(cloud, transformed_cloud, eigen_transform);
  	return transformed_cloud;
  	}
  	catch (tf2::TransformException& e)
  	{
  	RCLCPP_WARN(this->get_logger(), "%s", e.what());
  	return cloud;
  	}
  	}

  Notice
  lookupTransform()の第一引数はtarget_frame、第二引数はsource_frameであることにご注意ください。
  Link to reference

• 以下の箇所はプルリクエストのソースブランチを明示的に指定してチェックアウトすることを意味しますが、どうやらこの方法ではフォーク先のブランチは見つけられないようです。actions/checkout@v2はそれ単体でも自動でプルリクエストのソースブランチにチェックアウトをしてくれるらしい？(*)のでGithub Action上で利用している以下の箇所はすべて削除する必要があります。

  with:
    ref: ${{ github.head_ref }}

  削除の対象は以下の全てです。

  • ultralytics_ros/.github/workflows/noetic-ci.yml

    Lines 23 to 24 in 41753e2

    	with:
    	ref: ${{ github.head_ref }}

  • ultralytics_ros/.github/workflows/noetic-ci.yml

    Lines 37 to 38 in 41753e2

    	with:
    	ref: ${{ github.head_ref }}

  • ultralytics_ros/.github/workflows/noetic-ci.yml

    Lines 50 to 51 in 41753e2

    	with:
    	ref: ${{ github.head_ref }}

  • ultralytics_ros/.github/workflows/noetic-ci.yml

    Lines 65 to 66 in 41753e2

    	with:
    	ref: ${{ github.head_ref }}

  • ultralytics_ros/.github/workflows/noetic-ci.yml

    Lines 93 to 94 in 41753e2

    	with:
    	ref: ${{ github.head_ref }}

  • ultralytics_ros/.github/workflows/noetic-docker-build-check.yml

    Lines 16 to 17 in 41753e2

    	with:
    	ref: ${{ github.head_ref }}

[h-wata]

@Alpaca-zip
Thank you your replay and apologize my misunderstandings.
Made the fixes from the review.
Could you please review the changes when you have a moment? Thanks.

[Alpaca-zip]

https://github.com/Alpaca-zip/ultralytics_ros/actions/runs/7881844141/job/21508091103?pr=58

Please delete the following section.

ultralytics_ros/.github/workflows/noetic-docker-build-check.yml

Lines 16 to 17 in c23d77f

	with:
	ref: ${{ github.head_ref }}

---

https://github.com/Alpaca-zip/ultralytics_ros/actions/runs/7881844137/job/21508091095?pr=58

https://github.com/Alpaca-zip/ultralytics_ros/actions/runs/7881844137/job/21508091683?pr=58

https://github.com/Alpaca-zip/ultralytics_ros/actions/runs/7881844137/job/21508091421?pr=58

https://github.com/Alpaca-zip/ultralytics_ros/actions/runs/7881844137/job/21508091860?pr=58

https://github.com/Alpaca-zip/ultralytics_ros/actions/runs/7881844137/job/21508091994?pr=58

I haven't encountered above issues with the GitHub Action before(*), and since it's not clear what's causing it, I'll handle it separately and keep an eye on things.

[Alpaca-zip]

https://github.com/Alpaca-zip/ultralytics_ros/actions/runs/7883310972/job/21510311360?pr=58

Sorry for the inconvenience, it seems that the GitHub Action failed because the repository you forked to did not have access to Secrets. The Docker login is not required just to build the image, so could you please remove the code in the following section?

ultralytics_ros/.github/workflows/noetic-docker-build-check.yml

Lines 20 to 24 in dfbdb23

	- name: login to DockerHub
	uses: docker/login-action@v1
	with:
	username: ${{ secrets.DOCKERHUB_USERNAME }}
	password: ${{ secrets.DOCKERHUB_TOKEN }}

I'd like to proceed with the merge after making this adjustment. Thank you for your cooperation.

[Alpaca-zip]

• I have tested this change with KITTI data set.