-
Notifications
You must be signed in to change notification settings - Fork 673
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
feat(autoware_detected_object_validation): add height filter in lanelet filtering #10003
Conversation
Thank you for contributing to the Autoware project! 🚧 If your pull request is in progress, switch it to draft mode. Please ensure:
|
"max_height_threshold": { | ||
"type": "number", | ||
"default": 10.0, | ||
"description": "Maximum height threshold for filtering objects (in meters)." | ||
}, | ||
"min_height_threshold": { | ||
"type": "number", | ||
"default": -1.0, | ||
"description": "Minimum height threshold for filtering objects (in meters)." | ||
} |
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.
@YoshiRi
Please add description which frame_id is based for these parameters?
I think it should be map/lanelet_frame_id_
as there is a transformation to lanelet_frame_id_
at
Lines 149 to 154 in f83aa01
autoware_perception_msgs::msg::DetectedObjects transformed_objects; | |
if (!autoware::object_recognition_utils::transformObjects( | |
*input_msg, lanelet_frame_id_, tf_buffer_, transformed_objects)) { | |
RCLCPP_ERROR(get_logger(), "Failed transform to %s.", lanelet_frame_id_.c_str()); | |
return; | |
} |
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.
Thanks! Fixed in the latest test.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10003 +/- ##
==========================================
+ Coverage 29.41% 29.52% +0.10%
==========================================
Files 1419 1419
Lines 107507 107535 +28
Branches 42545 42561 +16
==========================================
+ Hits 31624 31749 +125
+ Misses 72513 72401 -112
- Partials 3370 3385 +15
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: yoshiri <[email protected]>
Signed-off-by: yoshiri <[email protected]>
Signed-off-by: yoshiri <[email protected]>
Signed-off-by: yoshiri <[email protected]>
Signed-off-by: yoshiri <[email protected]>
Signed-off-by: yoshiri <[email protected]>
1936d8a
to
f94bdcf
Compare
Test run result:
|
// vehicle base pose :map -> base_link | ||
try { | ||
ego_base_height_ = tf_buffer_ | ||
.lookupTransform( | ||
lanelet_frame_id_, "base_link", transformed_objects.header.stamp, | ||
rclcpp::Duration::from_seconds(0.5)) | ||
.transform.translation.z; | ||
} catch (const tf2::TransformException & ex) { | ||
RCLCPP_ERROR_STREAM(get_logger(), "Failed to get transform: " << ex.what()); | ||
return; | ||
} |
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.
@YoshiRi
IMO, it is better to add condition of use_height_threshold
here also
// vehicle base pose :map -> base_link | |
try { | |
ego_base_height_ = tf_buffer_ | |
.lookupTransform( | |
lanelet_frame_id_, "base_link", transformed_objects.header.stamp, | |
rclcpp::Duration::from_seconds(0.5)) | |
.transform.translation.z; | |
} catch (const tf2::TransformException & ex) { | |
RCLCPP_ERROR_STREAM(get_logger(), "Failed to get transform: " << ex.what()); | |
return; | |
} | |
// vehicle base pose :map -> base_link | |
if (filter_settings_.use_height_threshold) { | |
try { | |
ego_base_height_ = tf_buffer_ | |
.lookupTransform( | |
lanelet_frame_id_, "base_link", transformed_objects.header.stamp, | |
rclcpp::Duration::from_seconds(0.5)) | |
.transform.translation.z; | |
} catch (const tf2::TransformException & ex) { | |
RCLCPP_ERROR_STREAM(get_logger(), "Failed to get transform: " << ex.what()); | |
return; | |
} | |
} |
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.
Thanks! Fixed in 7206f1d.
Signed-off-by: yoshiri <[email protected]>
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
Description
Add height filter in lanelet filter.
This filter is based on z axis comparison between ego base_link and not considering lanelet slope.
I think we should use lane information for future, but currently it will be helpful in some feature such as radar filter.
Related links
Parent Issue:
How was this PR tested?
Add node test. You can test it with
Test case is like following:
sorry I had typo:
filtered
means the objects will be remainedremained
means the objects will be filtered outNotes for reviewers
None.
Interface changes
None.
Effects on system behavior
None.