-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Revise timestamping #18
Conversation
Introduce the TimeStampsHandler moduleI now introduce the
In both cases, the end stamp of one odometry query will become the begin stamp of the following query. The only breaking point I see until now is if the scan is stamped at the end and the relative timestamps in the |
The current implementation misses the combination of stamping at the beginning of the scan and having relative timestamps. Right now, we assume you stamp at the end, even though you can get the duration from the timestamps. Also, having negative timestamps for de-skewing does not work when using the KISS deskewing because we subtract the |
* Query from last stamp in case of frame drops * Rename * WIP * Fix conversions * Fix build * Make CI happy * Add TimeStampHandler module to handle this madness * Minor renaming * Try to take care of absolute timestamping of points + scan stamped at the end * Comments * Should fix all cases that came to my mind at this time * Some comments to make stuff more clear * Remove useless case, we need to fix deskewing * Fix condition for clarity --------- Co-authored-by: tizianoGuadagnino <[email protected]>
* Query from last stamp in case of frame drops * Rename * WIP * Fix conversions * Fix build * Make CI happy * Add TimeStampHandler module to handle this madness * Minor renaming * Try to take care of absolute timestamping of points + scan stamped at the end * Comments * Should fix all cases that came to my mind at this time * Some comments to make stuff more clear * Remove useless case, we need to fix deskewing * Fix condition for clarity --------- Co-authored-by: tizianoGuadagnino <[email protected]>
* We didnt have proper flagging in the online node * Clean launch common args * Add new threshold * Update CMakeLists.txt (#7) Make *USE_SYSTEM_SOPHUS* consistent with the rest of the options. * Update README.md (#6) More accurate statement in the readme, the system does not need to follow a unicycle motion model, just the pose correction needs to do that, which applies to much more robotics platforms. * Add python checks to pre-commit (#9) * Add black and isort * Format * use black for isort * Fixing ROS launch system (#8) * We didnt have proper flagging in the online node * Clean launch common args * Fix formatting python * Tiny modification, but makes sense * Renaming and add configuration for the correspondence threshold * Fix pipeline * Consistency in the CMakeLists * Remove ternary operator and make the two functions consistent * Add 2d lidar support, need testing * Restore ros utils to master * Revise timestamping (#18) * Query from last stamp in case of frame drops * Rename * WIP * Fix conversions * Fix build * Make CI happy * Add TimeStampHandler module to handle this madness * Minor renaming * Try to take care of absolute timestamping of points + scan stamped at the end * Comments * Should fix all cases that came to my mind at this time * Some comments to make stuff more clear * Remove useless case, we need to fix deskewing * Fix condition for clarity --------- Co-authored-by: tizianoGuadagnino <[email protected]> * Kinematic ICP Threshold (#15) * We didnt have proper flagging in the online node * Clean launch common args * Add new threshold * Update CMakeLists.txt (#7) Make *USE_SYSTEM_SOPHUS* consistent with the rest of the options. * Update README.md (#6) More accurate statement in the readme, the system does not need to follow a unicycle motion model, just the pose correction needs to do that, which applies to much more robotics platforms. * Add python checks to pre-commit (#9) * Add black and isort * Format * use black for isort * Fixing ROS launch system (#8) * We didnt have proper flagging in the online node * Clean launch common args * Fix formatting python * Tiny modification, but makes sense * Renaming and add configuration for the correspondence threshold * Fix pipeline * Consistency in the CMakeLists * Remove ternary operator and make the two functions consistent * Update cpp/kinematic_icp/correspondence_threshold/CorrespondenceThreshold.hpp Co-authored-by: Benedikt Mersch <[email protected]> * Renaming according to Ben's review * Why a ref * Odometry Regularization (#19) * Natural extension to make also the odometry regularization optional * Consistency in the CMakeLists * Remove ternary operator and make the two functions consistent * Update cpp/kinematic_icp/correspondence_threshold/CorrespondenceThreshold.hpp Co-authored-by: Benedikt Mersch <[email protected]> * Renaming according to Ben's review * Why a ref * Use the same default --------- Co-authored-by: Benedikt Mersch <[email protected]> Co-authored-by: Benedikt Mersch <[email protected]> --------- Co-authored-by: Benedikt Mersch <[email protected]> Co-authored-by: Benedikt Mersch <[email protected]> * We didnt have proper flagging in the online node * Add new threshold * Renaming and add configuration for the correspondence threshold * Fix pipeline * fixx * Include timestamps from the projected laser --------- Co-authored-by: Benedikt Mersch <[email protected]> Co-authored-by: Benedikt Mersch <[email protected]>
There are some inconsistencies in our timestamping that this PR addresses to fix.
First, we currently query the TF tree for the odometry from the beginning to the end of the current scan. This is incorrect because the sensor might drop frames, and we, in that case, ignore the odometry during that time.
Second, we assume that the timestamps of the points are absolute, which is not necessarily the case.
We can still not cover one case: if the timestamp of the ROS message is the beginning of the scan and no timestamps are provided for the points. In such a case, there is no way to query the correct odometry from TF because the exact timestamp at the scan's end is unknown.