Ankitvm / ipc_slam_cnn

5 stars 2 forks source link

static transform frame order in offline_slam/launch/start_offline_slam.launch #1

Open sayali-purdue opened 4 years ago

sayali-purdue commented 4 years ago

As per ROS convention document REP-105, odom should be a parent of base_link frame.

However, the launch file 'offline_slam/launch/start_offline_slam.launch' contains a static transform with a reversed order of frames: base_link to odom. Can you please explain why the order of frames is changed? I think to avoid issues during code reuse, it is good to follow ROS conventions.

I would really appreciate if you could provide your insights on the above point.

Thank you!

Ankitvm commented 4 years ago

Hi. I appreciate your feedback! Typically, the primary reason for the odom frame to be set as a parent frame is that in an online operation, the physical robot publishes the odometry data as the source which is then transformed to the rest of the frames. But since this code performs offline in a simulation, the robot motion can be directly manipulated from the base_link or the odom frame - the order of the static transform then does not have much effect on the operation or the data flow. Nevertheless, I have updated the scripts accordingly.

Thanks!

sayali-purdue commented 4 years ago

Thank you for the inputs. I would like to make one more suggestion.

During static review/analysis of the code, we observed that names of static transforms in the same launch file do not match the actual transforms they represent: sick_frame_tf and scan_frame_tf names should be swapped.

Again, this does not affect the correctness of the code, but it is a good practice to follow naming conventions for code reusability and maintenance. Can you please provide your thoughts on this?