RoboSense-LiDAR / rslidar_sdk

RoboSense LiDAR SDK for ROS & ROS2
Other
458 stars 219 forks source link

ROS2 Perfomance improvement #89

Open simulacrus opened 2 years ago

simulacrus commented 2 years ago

I found several places for improvement.

First, function toRosMsg. It's better to publish a unique_ptr instead of an object, avoiding unnecessary copying when publishing a message

Second, the implementation of the driver as a component

ZhenshengLee commented 2 years ago

First, function toRosMsg. It's better to publish a unique_ptr instead of an object, avoiding unnecessary copying when publishing a message

using std::move would be better?

https://github.com/RoboSense-LiDAR/rslidar_sdk/blob/34ac46b398add4e86c81d4a19d764f24afd8054d/src/adapter/point_cloud_ros_adapter.hpp#L71

point_cloud_pub_.publish(std::move(toRosMsg(msg))); 
ZhenshengLee commented 2 years ago

the implementation of the driver as a component

It should be hard for the current rslidar_sdk arch, because PointCloudRosAdapter inherits from AdapterBase.

simulacrus commented 2 years ago

First, function toRosMsg. It's better to publish a unique_ptr instead of an object, avoiding unnecessary copying when publishing a message

using std::move would be better?

https://github.com/RoboSense-LiDAR/rslidar_sdk/blob/34ac46b398add4e86c81d4a19d764f24afd8054d/src/adapter/point_cloud_ros_adapter.hpp#L71

point_cloud_pub_.publish(std::move(toRosMsg(msg))); 

There is some difference between publish methods(publish(const MessageT & msg) and publish(std::unique_ptr<MessageT, MessageDeleter> msg)) in rclcpp here. When intra_process_isenabled flag is enabled(only for component) then one copy operation will not be processed.

simulacrus commented 2 years ago

the implementation of the driver as a component

It should be hard for the current rslidar_sdk arch, because PointCloudRosAdapter inherits from AdapterBase.

Maybe need to implement init method like this void PointCloudRosAdapter::init(const YAML::Node& config, rclcpp::Node* node=nullptr) and create node in int main(int argc, char** argv) function?

boxanm commented 1 month ago

Hi, is there any update on implementing a component version of the driver?