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

Added point_cloud_transport #1008

Merged
merged 6 commits into from Mar 15, 2024
Merged

Added point_cloud_transport #1008

merged 6 commits into from Mar 15, 2024

Conversation

ahcorde
Copy link
Contributor

@ahcorde ahcorde commented Jul 11, 2023

We started to port point_cloud_transport from ROS 1 to ROS 2. This issue track the progress ros-perception/point_cloud_transport#1

I will open this PR as a draft because the package is not released yet, we need to discuss it makes or not to include this dependency.

It helps me a lot to debug issue in the point_cloud_transport.

Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
subscription_ = std::make_shared<point_cloud_transport::SubscriberFilter>();
subscription_->subscribe(
rviz_ros_node_.lock()->get_raw_node(),
getPointCloud2BaseTopicFromTopic(topic_property_->getTopicStd()),

Choose a reason for hiding this comment

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

These helpers seem useful to other users, not just rviz. Should we move these helpers inside pointcloud transport (they might already exist)? I can make a PR for that if you agree

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This might add some knowledge about the point cloud plugins that are installed. I prefer to keep point_cloud_transport agnostic

namespace displays
{

template<class MessageType>

Choose a reason for hiding this comment

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

Since this function is already explicitly a specialized sensor_msgs::msg::Poincloud2 class, does it need to be a template?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ImageTransportDisplay class has the same structure. I would like to keep it the same for consistency.

Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
@FranzAlbers
Copy link

Thanks for working on point_cloud_transport and the rviz2 plugin, @ahcorde!

I've noted that it's currently not possible to subscribe to two Draco compressed point clouds at the same time.
RViz2 crashes when two topics ending with /draco are subscribed with Pointcloud2 displays:

terminate called after throwing an instance of 'rclcpp::exceptions::ParameterAlreadyDeclaredException'
  what():  parameter 'SkipDequantizationPOSITION' has already been declared
Aborted

The topics don't even need to be published, just typing in the second /draco topic in the topic field lets RViz2 crash.

@ahcorde
Copy link
Contributor Author

ahcorde commented Oct 25, 2023

I recently added this commit ros-perception/point_cloud_transport_plugins@45c42b0 fixing the parameter names which was backported to iron and humble.

can you check if you are using these changes too ?

@FranzAlbers
Copy link

Sorry for the late reply, I had some days off.
I am already using that commit, but the crash occurs nevertheless.

It seems like SkipDequantizationPOSITION and the other SkipDequantization parameters are declared in draco_subscriber.cpp, which was not touched by that commit.
Currently, there is only one parameter SkipDequantizationPOSITION parameter for all Draco subscribers in rviz2. If a second Draco subscriber is initialized, the same parameter is declared again, and rviz2 crashes.

The crash does not occur if I comment the content of the DracoSubscriber::declareParameters() function, but in this case, these parameters cannot be changed at all.

@sinamoghimi73
Copy link

Hello, I'm trying to visualize the point cloud published by point_cloud_transport::Publisher. I get the data with ros2 topic echo ... but when trying to load in rviz2 I get the following warning:
requesting incompatible QoS. No messages will be sent to it. Last incompatible policy: RELIABILITY_QOS_POLICY

and nothing appears in rviz2. I would be grateful if you could guide me.

@ahcorde ahcorde marked this pull request as ready for review February 21, 2024 08:53
@ahcorde
Copy link
Contributor Author

ahcorde commented Feb 21, 2024

Hello, I'm trying to visualize the point cloud published by point_cloud_transport::Publisher. I get the data with ros2 topic echo ... but when trying to load in rviz2 I get the following warning: requesting incompatible QoS. No messages will be sent to it. Last incompatible policy: RELIABILITY_QOS_POLICY

and nothing appears in rviz2. I would be grateful if you could guide me.

@sinamoghimi73 you should be able to change the QoS, check the following image:

rviz_qos

@ahcorde ahcorde requested review from john-maidbot, clalancette and adityapande-1995 and removed request for john-maidbot February 21, 2024 09:41
Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
Copy link

@adityapande-1995 adityapande-1995 left a comment

Choose a reason for hiding this comment

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

Changes look good to me, will try this out locally once, and then approve !

@ahcorde
Copy link
Contributor Author

ahcorde commented Feb 29, 2024

friendly ping @adityapande-1995

Copy link

@adityapande-1995 adityapande-1995 left a comment

Choose a reason for hiding this comment

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

🚀

@ahcorde
Copy link
Contributor Author

ahcorde commented Mar 6, 2024

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@ahcorde
Copy link
Contributor Author

ahcorde commented Mar 8, 2024

this PR ros-perception/point_cloud_transport#64 on point_cloud_transport should fix the issue on the Windows CI

@ahcorde
Copy link
Contributor Author

ahcorde commented Mar 14, 2024

  • Windows Build Status

@ahcorde
Copy link
Contributor Author

ahcorde commented Mar 15, 2024

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@ahcorde ahcorde merged commit 5a0bde5 into rolling Mar 15, 2024
2 of 3 checks passed
@delete-merged-branch delete-merged-branch bot deleted the ahcorde/point_cloud_transport branch March 15, 2024 14:30
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.

None yet

5 participants