Update for Exporting Keyframes #1
Conversation
There was a problem hiding this comment.
Thanks @WANGYINGYU for the PR!
I've left comments. In general, I think it'd be great if you could expand on how these modules interact with each other and with the original lio_sam nodes. Also, a bit of explanation on why these nodes are needed in the Pull request and the README_Export_keyframe.md. The way to look at it is to describe it for someone who was not part of our conversation and wants to know why we are introducing these changes 🙂
Cheers!
| add_executable(export_keyframes src/export_keyframes.cpp) | ||
| ament_target_dependencies(export_keyframes | ||
| rclcpp | ||
| rmw # <-- add this |
There was a problem hiding this comment.
Do we need to import this rmw lib? Looking at the export_keyframe.cpp, it doesn't seem to need it 🤔
There was a problem hiding this comment.
Could you add a system diagram to depict how these new nodes interact with lio_sam?
This could explain faster the other nodes and their outputs without looking at the code.
| The export functionality consists of three main components: | ||
| 1. `aligned_saver`: Exports undistorted point clouds of keyframes to PCD files | ||
| 2. `export_keyframes`: Exports keyframe poses to a CSV file | ||
| 3. `export gps/odometry topic`: A script written by python to export gps/odometry topic |
There was a problem hiding this comment.
Could you elaborate a bit more on each module's functionality/scope?
For instance, "It subscribes to X and Y topics and produces P output", etc
There was a problem hiding this comment.
Also, the explanation/rationale of save_map.cpp is missing
| sub_ = this->create_subscription<CloudInfo>( | ||
| "/lio_sam/deskew/cloud_info", | ||
| rclcpp::SensorDataQoS(), | ||
| [this](CloudInfo::SharedPtr msg) { | ||
| RCLCPP_INFO(this->get_logger(), "Received CloudInfo, saving deskewed.pcd …"); | ||
| save_pcd(msg->cloud_deskewed); | ||
| rclcpp::shutdown(); | ||
| }); |
There was a problem hiding this comment.
Even though I love lambdas, I think it looks more readable/cleaner if we define a separate on*() functions for subscribers
| sub_ = this->create_subscription<PathMsg>( | ||
| "/lio_sam/mapping/path", // correct topic | ||
| rclcpp::QoS(1), // use SharedPtr API | ||
| [this](PathShared msg) { | ||
| if (msg->poses.empty()) { | ||
| RCLCPP_WARN(this->get_logger(), "waiting for non‐empty path"); | ||
| return; | ||
| } | ||
| std::ofstream fs("keyframes.csv", std::ios::trunc); | ||
| fs << "idx,sec,nsec,x,y,z,qx,qy,qz,qw\n"; | ||
| for (size_t i = 0; i < msg->poses.size(); ++i) { | ||
| auto &p = msg->poses[i]; | ||
| auto &t = p.header.stamp; | ||
| auto &o = p.pose.orientation; | ||
| auto &pos = p.pose.position; | ||
| fs << i << ',' | ||
| << t.sec << ',' << t.nanosec << ',' | ||
| << pos.x << ',' << pos.y << ',' << pos.z << ',' | ||
| << o.x << ',' << o.y << ',' << o.z << ',' << o.w | ||
| << '\n'; | ||
| } | ||
| RCLCPP_INFO(this->get_logger(), | ||
| "Wrote %zu keyframes to keyframes.csv", | ||
| msg->poses.size()); | ||
| rclcpp::shutdown(); | ||
| }); |
There was a problem hiding this comment.
As previous comment, let's not use lambdas for defining subscriptions
There was a problem hiding this comment.
Please see featureExtraction.cpp comment
There was a problem hiding this comment.
Please see featureExtraction.cpp comment
There was a problem hiding this comment.
Please see featureExtraction.cpp comment
There was a problem hiding this comment.
I think this is VSCode macOS cache data. In general, we should avoid pushing this kind of file to a repository.
Could you please remove it?
| @@ -33,8 +34,15 @@ find_package(OpenMP REQUIRED) | |||
|
|
|||
| include_directories( | |||
| include/lio_sam | |||
| ${rclcpp_INCLUDE_DIRS} | |||
| ${rmw_INCLUDE_DIRS} | |||
| ${CMAKE_CURRENT_BINARY_DIR} # so you can #include "lio_sam/msg/cloud_info.hpp" | |||
There was a problem hiding this comment.
Is this necessary?
How do the original executables include lio_sam/msg/cloud_info.hpp? 🤔
Adding ros nodes to export point clouds and associated poses of keyframes;
Adding python script to export gps/odometry topic