You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
During a static code review, I observed that different naming conventions have been followed for the variables/names representing transform:
(1) A_to_B: where, A is frame_id and B is child_frame_id
For example: robot1_baselink_to_laserscan1 in file real_single_truck_rob_lab_iliad_smp.launch (line #40) <node pkg="tf" type="static_transform_publisher" name="robot1_baselink_to_laserscan1" args="1.44 0.07 0 0 0 0 /robot1/base_link /laserscan0 100"/>
(2) A_to_B: where, A is child_frame_id and B is frame_id
For example: robot1_baselink_to_baselink in file real_single_truck_rob_lab_iliad_smp.launch (line #36) <node pkg="tf" type="static_transform_publisher" name="robot1_baselink_to_baselink" args="0 0 0 0 0 0 /base_link /robot1/base_link 100"/>
and base_footprint_to_odom in file gazebo_ros_steer_drive.cpp (line #390) transform_broadcaster_->sendTransform ( tf::StampedTransform ( base_footprint_to_odom, current_time, odom_frame, base_footprint_frame ) );
The transform object/transform name represents the transform from parent frame (i.e., frame_id) to child frame (i.e., child_frame_id). Therefore, I think the transform variable name convention in case(1) above gives the clear meaning of that transform.
(3) A_broadcaster: this is a bit different than above two cases. A static transform publisher broadcasts a child frame (that is defined with respect to a parent frame). Therefore, A_broadcaster name with child_frame_id as A is an appropriate name. However, for example, robot1_kinect_link_broadcaster in file cititruck_empty.launch (line #17) suggests that this publisher is broadcasting a parent frame: <node pkg="tf" type="static_transform_publisher" name="robot1_kinect_link_broadcaster" args="0 0 0 0 1.57 0 robot1/kinect_link robot1/kinect_link_tmp 100" />
A third user may just assume that the meaningful name A_to_B represents the transform from frame A to frame B, and A_broadcaster is publishing frame A. Further, following different naming conventions may lead to confusions or issues during code maintenance/reuse in future.
Would really appreciate if you could please describe your perspective behind using two different naming conventions, and your views on its effects on code maintainability/reuse in future.
Thank you.
The text was updated successfully, but these errors were encountered:
During a static code review, I observed that different naming conventions have been followed for the variables/names representing transform:
(1) A_to_B: where, A is frame_id and B is child_frame_id
For example: robot1_baselink_to_laserscan1 in file real_single_truck_rob_lab_iliad_smp.launch (line #40)
<node pkg="tf" type="static_transform_publisher" name="robot1_baselink_to_laserscan1" args="1.44 0.07 0 0 0 0 /robot1/base_link /laserscan0 100"/>
(2) A_to_B: where, A is child_frame_id and B is frame_id
For example: robot1_baselink_to_baselink in file real_single_truck_rob_lab_iliad_smp.launch (line #36)
<node pkg="tf" type="static_transform_publisher" name="robot1_baselink_to_baselink" args="0 0 0 0 0 0 /base_link /robot1/base_link 100"/>
and
base_footprint_to_odom in file gazebo_ros_steer_drive.cpp (line #390)
transform_broadcaster_->sendTransform ( tf::StampedTransform ( base_footprint_to_odom, current_time, odom_frame, base_footprint_frame ) );
The transform object/transform name represents the transform from parent frame (i.e., frame_id) to child frame (i.e., child_frame_id). Therefore, I think the transform variable name convention in case(1) above gives the clear meaning of that transform.
(3) A_broadcaster: this is a bit different than above two cases. A static transform publisher broadcasts a child frame (that is defined with respect to a parent frame). Therefore, A_broadcaster name with child_frame_id as A is an appropriate name. However, for example, robot1_kinect_link_broadcaster in file cititruck_empty.launch (line #17) suggests that this publisher is broadcasting a parent frame:
<node pkg="tf" type="static_transform_publisher" name="robot1_kinect_link_broadcaster" args="0 0 0 0 1.57 0 robot1/kinect_link robot1/kinect_link_tmp 100" />
A third user may just assume that the meaningful name A_to_B represents the transform from frame A to frame B, and A_broadcaster is publishing frame A. Further, following different naming conventions may lead to confusions or issues during code maintenance/reuse in future.
Would really appreciate if you could please describe your perspective behind using two different naming conventions, and your views on its effects on code maintainability/reuse in future.
Thank you.
The text was updated successfully, but these errors were encountered: