Change imu_alignment parameter to imu_correction. #2
Conversation
…d from static transform and can be specified in the robot's URDF.
src/imu_and_wheel_odom_node.cpp
Outdated
| tf::StampedTransform tf_imu_bl; | ||
|
|
||
| try { | ||
| tf_listener.waitForTransform(p_base_frame_, p_imu_frame_, ros::Time(0), ros::Duration(0.1)); |
There was a problem hiding this comment.
Hi, I'm Martin from CTU in Prague. We're using your package so I've set up a watch to it to get notifications about changes :) Thanks for the PR, it looks good! Just the timeout of this wait seems too short. You set up the TF listener just 3 lines above and it might take some time to do all the talking to ROS master and all static transform publishers and get their data. At least 1 second is needed here, probably even more.
It can also happen that the robot model publisher will start later than this IMU node. In such case, even the 1 second wait could be too short. The question is what to do in such case. One option would be to put this lookup into an endless loop and keep trying (and writing red messages to console). The other option would be leave it as it is (i.e. exit if TF is not found) and expect that the node is launched with respawn="true" (which is basically a good idea). In case the TF would not be found, this node would die and roslaunch would re-launch it again until the TF is ok. In case you choose this approach, please, return anything else than 0 from the program if it exits because of failed TF lookup. 0 means success and clean exit.
Last, I'm not sure how much you care about keeping backwards compatibility. It is usually a nice thing to do because it gives people more time to adapt to the changes. If you'd be interested in not breaking old code, I'd suggest looking for the imu_alignment_rpy argument and if it is found, skip the TF lookup and use the old method (and print a warning about using a deprecated feature). Or you could even treat this as a supported feature - if the user sets imu_alignment_rpy, then he says that the IMU should not be searched in TF and that he is providing the transform manually.
There was a problem hiding this comment.
@boxanm Both suggestions are reasonable, could you please modify the code accordingly?
src/imu_and_wheel_odom_node.cpp
Outdated
| imu_alignment_rpy_[1], | ||
| imu_alignment_rpy_[2]); | ||
| // Get IMU->base_link tf | ||
| tf::TransformListener tf_listener; |
There was a problem hiding this comment.
Pass n (the public nodehandle) to the constructor. This way, parameter and topic remappings are easier transferred also to the TF listener.
| tf::TransformListener tf_listener; | |
| tf::TransformListener tf_listener(n); |
…public nodehandle passed to TF listener constructor.
|
Great job @boxanm ! |
|
@boxanm please put this branch on the marmotte and warthog robot and keep testing it for a week, if everything works, we will merge next Friday |
The imu to base_link orientation is now loaded from static transform and can be specified in the robot's URDF.