Auto detecting IMU/Points frame IDs and acc_scale#67
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to reduce required manual configuration by auto-detecting IMU/LiDAR frame IDs at runtime and auto-detecting IMU acceleration unit scaling, and extends RViz outputs with scan-end pose/odometry topics.
Changes:
- Default
imu_frame_id/lidar_frame_idto empty and auto-detect them viaGlobalConfigmeta fields. - Add
~/odom_scanend,~/pose_scanend(and corrected variants) publishers in the RViz viewer. - Change
acc_scaledefault to0.0and infer scale from IMU acceleration norm on first use.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| src/glim_ros/rviz_viewer.cpp | Adds scan-end pose/odom publishing and runtime frame-id fallback/auto-detection. |
| src/glim_ros/glim_ros.cpp | Auto-detects IMU/LiDAR frame IDs into GlobalConfig meta and auto-detects acc_scale. |
| include/glim_ros/rviz_viewer.hpp | Declares new scan-end publishers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| odom_scan_end_pub->publish(odom); | ||
|
|
||
| logger->debug("published odom (stamp={})", new_frame->stamp); |
There was a problem hiding this comment.
This debug log says published odom and prints new_frame->stamp, but this message publishes the scan-end odometry using imu_end_stamp. Logging the published stamp/topic (e.g., imu_end_time / odom_scanend) would avoid confusion when debugging timing issues.
| logger->debug("published odom (stamp={})", new_frame->stamp); | |
| logger->debug("published odom_scanend (stamp={})", imu_end_stamp); |
| odom.twist.twist.linear.x = v_odom_imu.x(); | ||
| odom.twist.twist.linear.y = v_odom_imu.y(); | ||
| odom.twist.twist.linear.z = v_odom_imu.z(); |
There was a problem hiding this comment.
odom.header.stamp is set to the scan-end time, but the twist is copied from v_odom_imu (the begin/frame velocity). That makes the Odometry message internally inconsistent in time. Consider leaving twist unset/zero for scan-end messages or computing an end-of-scan velocity if available.
| odom.twist.twist.linear.x = v_odom_imu.x(); | |
| odom.twist.twist.linear.y = v_odom_imu.y(); | |
| odom.twist.twist.linear.z = v_odom_imu.z(); | |
| // Twist at scan end is not available; publish zero twist to avoid time inconsistency. | |
| odom.twist.twist.linear.x = 0.0; | |
| odom.twist.twist.linear.y = 0.0; | |
| odom.twist.twist.linear.z = 0.0; |
| if (std::abs(imu_end_time - new_frame->stamp) < 1e-3) { | ||
| logger->warn("Scan end time is too close to the frame time (imu_end_time={}, frame_time={})", imu_end_time, new_frame->stamp); | ||
| } |
There was a problem hiding this comment.
Same as the odom scan-end block: this warning will also trigger whenever imu_rate_trajectory is missing (end time defaults to frame time), and it can be emitted twice per frame if both scan-end topics are subscribed. Consider gating it on trajectory availability and/or warning once per condition.
| pose.pose.orientation.w = quat_world_imuend.w(); | ||
| pose_scan_end_pub->publish(pose); | ||
|
|
||
| logger->debug("published pose (stamp={})", new_frame->stamp); |
There was a problem hiding this comment.
This debug log prints new_frame->stamp, but this pose is published at imu_end_stamp. Logging the published stamp/topic (e.g., imu_end_time / pose_scanend) would make timing debugging clearer.
| logger->debug("published pose (stamp={})", new_frame->stamp); | |
| logger->debug("published pose_scanend (stamp={})", imu_end_time); |
| imu_time_offset = config_ros.param<double>("glim_ros", "imu_time_offset", 0.0); | ||
| points_time_offset = config_ros.param<double>("glim_ros", "points_time_offset", 0.0); | ||
| acc_scale = config_ros.param<double>("glim_ros", "acc_scale", 1.0); | ||
| acc_scale = config_ros.param<double>("glim_ros", "acc_scale", 0.0); |
There was a problem hiding this comment.
Changing the default acc_scale from 1.0 to 0.0 changes runtime behavior for users who rely on defaults (it now enables auto-detection). If auto-detection is intended to be opt-in, consider keeping the default at 1.0 and treating 0.0 as a sentinel value (or add an explicit auto_detect_acc_scale param).
| acc_scale = config_ros.param<double>("glim_ros", "acc_scale", 0.0); | |
| acc_scale = config_ros.param<double>("glim_ros", "acc_scale", 1.0); |
| if (std::abs(acc_scale) < 1e-6) { | ||
| const double norm = Eigen::Vector3d(msg->linear_acceleration.x, msg->linear_acceleration.y, msg->linear_acceleration.z).norm(); | ||
| if (norm > 7.0 && norm < 12.0) { | ||
| acc_scale = 1.0; | ||
| spdlog::debug("assuming [m/s^2] for acceleration unit (acc_scale={}, norm={})", acc_scale, norm); | ||
| } else if (norm > 0.8 && norm < 1.2) { | ||
| acc_scale = 9.80665; | ||
| spdlog::debug("assuming [g] for acceleration unit (acc_scale={}, norm={})", acc_scale, norm); | ||
| } else { | ||
| acc_scale = 1.0; | ||
| spdlog::warn("unexpected acceleration norm {}. assuming [m/s^2] for acceleration unit (acc_scale={})", norm, acc_scale); | ||
| } | ||
| } |
There was a problem hiding this comment.
The auto-detection locks acc_scale based on a single IMU message. If the first sample is during motion/vibration, the acceleration norm may fall outside the expected ranges and permanently select the wrong scale. Consider estimating over a short window (e.g., median norm over N samples) and/or only finalizing when the norm is stable and near gravity.
| if (std::abs(imu_end_time - new_frame->stamp) < 1e-3) { | ||
| logger->warn("Scan end time is too close to the frame time (imu_end_time={}, frame_time={})", imu_end_time, new_frame->stamp); |
There was a problem hiding this comment.
The scan-end warning condition will trigger every frame when imu_rate_trajectory is empty because imu_end_time defaults to new_frame->stamp (diff=0 < 1e-3). This can spam logs whenever there are subscribers. Consider only warning when imu_rate_trajectory is available (e.g., cols() > 0) and the end stamp is unexpectedly close, or throttle/warn-once when scan-end info is unavailable.
| if (std::abs(imu_end_time - new_frame->stamp) < 1e-3) { | |
| logger->warn("Scan end time is too close to the frame time (imu_end_time={}, frame_time={})", imu_end_time, new_frame->stamp); | |
| static bool scan_end_time_warned = false; | |
| if (!scan_end_time_warned && std::abs(imu_end_time - new_frame->stamp) < 1e-3) { | |
| logger->warn("Scan end time is too close to the frame time (imu_end_time={}, frame_time={})", imu_end_time, new_frame->stamp); | |
| scan_end_time_warned = true; |
No description provided.