Support Depth Mapping Streams #2757
Conversation
| sensor_msgs::PointCloud2Iterator<float> iter_label(*msg_pointcloud, "label"); | ||
| const rs2::vertex* vertex = pc.get_vertices(); | ||
| const uint8_t* label = pc.get_labels(); | ||
| msg_pointcloud->width = 320; |
There was a problem hiding this comment.
@remibettan
should we add get_width / get_height for rs2::labeleld_points?
because in the future, labeled point cloud might support different heights/widths/resolutions
There was a problem hiding this comment.
The API size() should be used instead of width and height
There was a problem hiding this comment.
but I used size to get the size of the whole PC.
what if I want to get width/height to fill the structure of point cloud message ?
or should I assume this is unordered Point Cloud and set the height 1 and width = frame.size() ?
Reference: http://docs.ros.org/en/melodic/api/sensor_msgs/html/msg/PointCloud2.html
2D structure of the point cloud. If the cloud is unordered, height is 1 and width is the length of the point cloud.
There was a problem hiding this comment.
Well, you may this. Because, as for point cloud, there is no meaning to height and width there, since vertices are passed, and not a frame
There was a problem hiding this comment.
I understand we are getting unordered vertices, not an ordered ones.
But, its better to wait for @benlev inputs here, since he might want this meta info in the msg itself.
There was a problem hiding this comment.
This was a valid question :)
LPC resolution is now changed and I am phasing the same question..
SamerKhshiboun
force-pushed
the
hkr-occupancy
branch
from
78f72a0 to
d694b34
Compare
| @@ -271,7 +273,7 @@ namespace realsense2_camera | |||
|
|
|||
| bool _use_intra_process; | |||
| std::map<stream_index_pair, std::shared_ptr<image_publisher>> _image_publishers; | |||
|
|
|||
| rclcpp::Publisher<sensor_msgs::msg::PointCloud2>::SharedPtr _labeled_pointcloud_publisher; | |||
There was a problem hiding this comment.
Can you explain why you have a specific publisher for lpc and not for pc?
There was a problem hiding this comment.
Because we can turn on both Depth point cloud and Labeled Point cloud at the same time
These are two different publishers
| image_publishers.find(stream) == image_publishers.end()) | ||
| bool is_sc_stream = isSCStream(stream); | ||
|
|
||
| // if stream is on, and is not a SC stream, publish camereaInfo |
| bool is_sc_stream = isSCStream(stream); | ||
|
|
||
| // if stream is on, and is not a SC stream, publish camereaInfo | ||
| if(!is_sc_stream && info_publishers.find(stream) != info_publishers.end()) |
There was a problem hiding this comment.
I would have renamed the is_sc_stream to should_publish_camera_info, since this is the aim of it
There was a problem hiding this comment.
changed the function name from isSCStream to shouldPublishCameraInfo, and changed the implementation inside it.
| @@ -188,6 +189,11 @@ void BaseRealSenseNode::stopPublishers(const std::vector<stream_profile>& profil | |||
| _info_publisher.erase(sip); | |||
| _depth_aligned_image_publishers.erase(sip); | |||
| _depth_aligned_info_publisher.erase(sip); | |||
|
|
|||
| if(_labeled_pointcloud_publisher) | |||
There was a problem hiding this comment.
In which case would this condition be false?
There was a problem hiding this comment.
if it was not initialized at all (e.g., the camera doesn't support Depth Mapping Sensor)
| if (profile.is<rs2::video_stream_profile>() && profile.stream_type() == RS2_STREAM_LABELED_POINT_CLOUD) | ||
| { | ||
| _labeled_pointcloud_publisher = _node.create_publisher<sensor_msgs::msg::PointCloud2>("labeled_point_cloud/points", | ||
| rclcpp::QoS(rclcpp::QoSInitialization::from_rmw(qos),qos)); |
There was a problem hiding this comment.
Please explain what is QoS?
There was a problem hiding this comment.
Quality Of Service.
Each publisher should define quality of service, regarding its Durability, Reliability, Depth, History, Queue Size, etc..
Reference: https://docs.ros.org/en/rolling/Concepts/About-Quality-of-Service-Settings.html
Read both parts:
1- https://docs.ros.org/en/rolling/Concepts/About-Quality-of-Service-Settings.html#qos-policies
2- https://docs.ros.org/en/rolling/Concepts/About-Quality-of-Service-Settings.html#qos-profiles
| _labeled_pointcloud_publisher = _node.create_publisher<sensor_msgs::msg::PointCloud2>("labeled_point_cloud/points", | ||
| rclcpp::QoS(rclcpp::QoSInitialization::from_rmw(qos),qos)); | ||
| } | ||
| else if (profile.is<rs2::video_stream_profile>()) |
There was a problem hiding this comment.
Since the video_stream_profile happens most of the time, it should be the first condition in the code.
Please pass it back to the if condition, and move the lpc condition to some else if after it (even maybe last one)
There was a problem hiding this comment.
Changed the flow.
Combined it to one "if (profile.isrs2::video_stream_profile()) "
then asked about Labeled PC stream inside it.
SamerKhshiboun
force-pushed
the
hkr-occupancy
branch
2 times, most recently
from
f54a018 to
d3c09cb
Compare
|
1 side comment, we did some work with the community on supporting zero copy for PC, please make sure you are aligning L-PC to that. |
Already aligned in the current implementation |
SamerKhshiboun
force-pushed
the
hkr-occupancy
branch
from
d3c09cb to
b9b32f9
Compare
| @@ -520,6 +522,12 @@ void BaseRealSenseNode::frame_callback(rs2::frame frame) | |||
| if (f.is<rs2::video_frame>()) | |||
| ROS_DEBUG_STREAM("frame: " << f.as<rs2::video_frame>().get_width() << " x " << f.as<rs2::video_frame>().get_height()); | |||
|
|
|||
| if (f.is<rs2::labeled_points>()) | |||
There was a problem hiding this comment.
better change this if statement to else if - do you agree?
No description provided.