fix(ring_outlier_filter): change a parameter to be less misleading #4981
Conversation
github-actions
bot
added
type:documentation
Signed-off-by: Takahiro Ishikawa <[email protected]>
| | `distance_ratio` | double | 1.03 | | | ||
| | `object_length_threshold` | double | 0.1 | | | ||
| | `num_points_threshold` | int | 4 | | | ||
| | `max_ring_index` | uint_16 | 128 | `128` corresponds to vls128. | |
There was a problem hiding this comment.
For index, I am not sure if it is 127 or 128 when using vls128. max_rings_num would be easier to understand if it is 128.
There was a problem hiding this comment.
related comment: #4981 (comment)
cc @taikitanaka3
There was a problem hiding this comment.
In a project using Autoware, max_rings_num was set to 128 (i.e. the range of ring index was set to be 0~127), but actually the maximum value of ring index was 128.
To prevent this kind of thing, the parameter name should be changed to be less misleading.
Based on this explanation, wouldn't it be more appropriate to use 'max_rings_index' rather than 'max_rings_num'?
There was a problem hiding this comment.
@tkimura4 If it weren't for the description, I would think that vls128 would be correct to specify 127 based on the name max_ring_index.
Wouldn't it be better to fix the index set to 128?
|
This pull request has been automatically marked as stale because it has not had recent activity. |
|
We are closing this PR as there are no response for months. Please re-open if the issue still persists. |
Description
The name of a ring_outlier_filter parameter (
max_rings_num) has been changed to be less misleading (max_ring_index), and the source code has been modified accordingly.In a project using Autoware,
max_rings_numwas set to128(i.e. the range of ring index was set to be0~127), but actually the maximum value of ring index was128.To prevent this kind of thing, the parameter name should be changed to be less misleading.
TIER IV internal link
Tests performed
Even in projects with a maximum ring index of 128, it worked without error messages.
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.