-
Notifications
You must be signed in to change notification settings - Fork 668
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
fix(gnss_poser): fix_transform_direction_problem #5033
fix(gnss_poser): fix_transform_direction_problem #5033
Conversation
Signed-off-by: meliketanrikulu <[email protected]>
7f44aca
to
4540b7c
Compare
@kminoda Could you review this pr? |
@meliketanrikulu Hi, sorry for being late, and thank you for your contribution 🙏 |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #5033 +/- ##
=======================================
Coverage 15.61% 15.61%
=======================================
Files 1589 1589
Lines 110032 110032
Branches 34103 34103
=======================================
Hits 17182 17182
Misses 73943 73943
Partials 18907 18907
*This pull request uses carry forward flags. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Signed-off-by: meliketanrikulu <[email protected]>
Hello @kminoda I added detailed description and fix the CI issue which is related me. Could you check again. Thank you |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Apologies for the late review 🙏
264accb
into
autowarefoundation:main
Description
tf_gnss_antenna2base_link_msg_ptr names implies "gnss_link -> base_link" transform and is used that way, but getStaticTransform(gnss_frame_, base_frame_, ... returns the transform "base_link -> gnss_link". The transform direction is not true.
Here the function to make transforms is called. And this line contains transform from base_link to gnss using lookup transform. But I think the exact source and target frame should be swapped. Source frame should be gnss target frame base_link
Expected Functionality:
How does it work now:
The expected output is here-->tf_gnss_antenna2base_link_msg_ptr butthe function calculates base_link2gnss_antenna transformations because target frame is gnss. It sould be base_link.
Related links
Related Issue: #3640
Tests performed
Since roll pitch yaw values for gnss in sample sensor kit are 0 0 0, we do not encounter this error in autoware demos.
Also, I do not have a sensor setup with the sensor configuration to test this, but when you look at the function descriptions, you can see that it is mathematically incorrect.
Notes for reviewers
Interface changes
Effects on system behavior
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.