fix(sensor_calibration_manager): cache the constant tfs used by the tag-based SfM post_process - #279
Open
abailinrun wants to merge 1 commit into
Conversation
…ag-based SfM post_process The tag_based_sfm_calibrator broadcasts the optimized sensor poses (main sensor -> calibration frames) once the calibration finishes, for visualization purposes. Since tf2 only allows a single parent per frame, these broadcasts re-parent the calibration frames and shadow the constant transforms defined in the robot description. The post_process implementations of the rdv, x2 and xx1_15 base_lidars_cameras calibrators query those constant transforms after the calibration has finished, so the queries resolve through the broadcast tfs and return values containing the inverse of the optimized poses. When composed with the calibration results, the optimized poses cancel out exactly and the outputs degenerate to a rigid carry-over of the initial (robot description) values, silently discarding the BA solutions for the cameras (and for the front/rear kits on x2 and the lidar base links on rdv). Cache the constant transforms as soon as the required tfs become available (before any calibration result can be broadcast) and use the cached values in post_process. Signed-off-by: abailinrun <abailinrun@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #278 — see the issue for the full analysis and the discovery story.
This PR fixes a subtle bug in the
tag_based_sfm_calibratormanager integration that silently discards the camera BA results (and some lidar/kit results) from the final calibration output, replacing them with a rigid carry-over of the initial robot-description values — regardless of whether the BA converged.Root cause
main sensor -> calibration frames) for visualization purposes (publish_tfs, enabled by default).cameraX/camera_link -> cameraX/camera_optical_linkjoints).post_processimplementations of therdv,x2, andxx1_15base_lidars_camerascalibrators query those constant transforms after the calibration has finished. The query path is forced through the broadcast tfs, so the returned value contains the inverse of the optimized pose:post_processcomposes it with the calibration result, the optimized pose cancels out exactly:The output row degenerates to the initial value, bit-exact up to floating point noise, no matter what the BA estimated.
Affected outputs
xx1_15optical_link -> camera_linkquery)rdv*_base_linkrows (lidar -> lidar_basequery)x2base -> front/rear kitrows (kit -> pandar_40p_*queries)kit -> mainpath never traverses a broadcast frame)default_projectpost_process, raw BA results are returned as-is)The general rule: any constant-tf query in
post_processwhose tf path traverses a frame that is broadcast as a child by the calibrator returns a contaminated value and cancels the corresponding BA factor.Why it is hard to notice
base_linkcorrection), so nothing fails and no warning is printed.Fix
Cache the constant transforms as soon as the required tfs become available (
on_check_tf_timer, i.e. before the UI becomes ready and therefore before any calibration result can exist), and use the cached values inpost_process. A fallback with a warning is kept in casepost_processis ever reached without the cache being populated.This keeps
publish_tfsfully functional — the visualization behavior is unchanged.Related links
Tests performed
post_processpattern asxx1_15):ast), and passblack/isortwith the repository settings. I do not have access to rdv/x2/xx1_15 hardware, so the upstream files could not be tested end-to-end — the x2/rdv lidar/kit cancellation paths are derived algebraically from the same identity.Notes for reviewers
on_check_tf_timerstops as soon as the tfs become ready, which happens while the UI is still initializing, anddata_->optimized_sensor_poses_mapis necessarily empty at that point — so the cached values can never be contaminated by a previous run of the same node.CalibratorBasewould fix all projects at once, but touches every calibrator; the per-calibrator caching keeps the diff minimal and local to the three affected files.default_projectis intentionally untouched (it has nopost_processand returns the raw BA results, which are correct).Pre-review checklist for the PR author
The PR author must check the checkboxes below when creating the PR.
In-review checklist for the PR reviewers
The PR reviewers must check the checkboxes below before approval.
Post-review checklist for the PR author
The PR author must check the checkboxes below before merging.
After all checkboxes are checked, anyone who has write access can merge the PR.