-
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(lidar_centerpoint): updated the config file for the centerpoint tiny v2 #3096
fix(lidar_centerpoint): updated the config file for the centerpoint tiny v2 #3096
Conversation
Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp>
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.
I tested this more than 3 times in logging simulator and AWSIM simulator.
Without this PR, my centerpoint tiny v2 died.
With this PR, I confirmed everything works fine!
@yukke42 |
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
@shmpwk Thank you for cheking it to work. |
Hello,
I've started autoware loggin simulator with: And with this command I've started playing ros2 bag: It happens on the main branch of autoware.universe. Here is the video. If I change the config file to its previous stage it works fine and produce bounding boxes. Here is the video. Actually centerpoint still throws this warning as you can see in the end of the video but not so frequently. |
Also I dug a little bit deeper and this change cause it |
@lexavtanke Thank you for the feedback. Cloud you try again after removing |
@yukke42 Yes, sure. It works strange, very unstable. Here is the video. centerpoint_rebuild.mp4 |
here is the video with old config file without rebuilding package. centerpoint_old_config.mp4 |
@lexavtanke I have checked only lidar_centerpoint node and there is no issue in my environment. |
Or it might be an issue in another package. Please give me more time to check the behavior. |
@lexavtanke |
@lexavtanke It works without any issues here.
|
…iny v2 (#3096) Fixed the cfg file regarding centerpoint tiny Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp>
* fix(lidar_centerpoint): updated the config file for the centerpoint tiny v2 (#3096) Fixed the cfg file regarding centerpoint tiny Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * feat(traffic_light_map_based_detector): change the vibration parameter of the Traffic Light Detector (#3189) Change the vibration parameter of the Traffic Light Detector. Improve the accuracy of yellow signal recognition in AWSIM. * fix: use centerpoint_tiny to centerpoint Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp> * revert: "fix: use centerpoint_tiny to centerpoint" This reverts commit efcf726. --------- Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp> Co-authored-by: Kenzo Lobos Tsunekawa <kenzo.lobos@tier4.jp> Co-authored-by: mackierx111 <mackierx111@gmail.com>
Description
When I uploaded the centerpoint tiny v2 I missed uploading the updated config file, which caused errors and an unnecessary bad performance.
The new file was tested in the environments in which the error arose
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.