refactor(radar_fusion_to_detected_object): rework parameters #4663
There are no files selected for viewing
This file contains 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
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| { | ||
| "$schema": "http://json-schema.org/draft-07/schema#", | ||
| "title": "Parameters for Radar Fusion to Detected Object Node", | ||
| "type": "object", | ||
| "definitions": { | ||
| "radar_fusion_to_detected_object": { | ||
| "type": "object", | ||
| "properties": { | ||
| "node_params": { | ||
| "type": "object", | ||
| "properties": { | ||
| "update_rate_hz": { | ||
| "type": "number", | ||
| "description": "Update rate for parameters. [hz]", | ||
| "default": "10.0", | ||
| "minimum": 0.0 | ||
| } | ||
| }, | ||
| "required": ["update_rate_hz"] | ||
| }, | ||
| "core_params": { | ||
| "type": "object", | ||
| "properties": { | ||
| "bounding_box_margin": { | ||
| "type": "number", | ||
| "description": "The distance to extend the 2D bird's-eye view Bounding Box on each side. This distance is used as a threshold to find radar centroids falling inside the extended box. [m]", | ||
| "default": "2.0", | ||
| "minimum": 0.0 | ||
| }, | ||
| "split_threshold_velocity": { | ||
| "type": "number", | ||
| "description": "The object's velocity threshold to decide to split for two objects from radar information (currently not implemented) [m/s]", | ||
| "default": "5.0", | ||
| "minimum": 0.0 | ||
| }, | ||
| "threshold_yaw_diff": { | ||
| "type": "number", | ||
| "description": "The yaw orientation threshold. If ∣θob−θra∣<threshold×yaw_diff attached to radar information include estimated velocity, where `θob` is yaw angle from 3d detected object, `θra` is yaw angle from radar object. [rad]", | ||
| "default": "0.35" | ||
| }, | ||
| "velocity_weight_min_distance": { | ||
| "type": "number", | ||
| "description": "The twist coefficient of radar data nearest to the center of bounding box in velocity estimation.", | ||
| "default": "1.0" | ||
| }, | ||
| "velocity_weight_average": { | ||
| "type": "number", | ||
| "description": "The twist coefficient of average twist of radar data in velocity estimation.", | ||
| "default": "0.0" | ||
| }, | ||
| "velocity_weight_median": { | ||
| "type": "number", | ||
| "description": "The twist coefficient of median twist of radar data in velocity estimation.", | ||
| "default": "0.0" | ||
| }, | ||
| "velocity_weight_target_value_average": { | ||
| "type": "number", | ||
| "description": "The twist coefficient of target value weighted average in velocity estimation. Target value is amplitude if using radar pointcloud. Target value is probability if using radar objects.", | ||
| "default": "0.0" | ||
| }, | ||
| "velocity_weight_target_value_top": { | ||
| "type": "number", | ||
| "description": "The twist coefficient of top target value radar data in velocity estimation. Target value is amplitude if using radar pointcloud. Target value is probability if using radar objects.", | ||
| "default": "0.0" | ||
| }, | ||
| "convert_doppler_to_twist": { | ||
| "type": "boolean", | ||
| "description": "Convert doppler velocity to twist using the yaw information of a detected object.", | ||
| "default": "false" | ||
| }, | ||
| "threshold_probability": { | ||
| "type": "number", | ||
| "description": "If the probability of an output object is lower than this parameter, and the output object does not have radar points/objects, then delete the object.", | ||
| "default": "0.4", | ||
| "minimum": 0.0, | ||
| "maximum": 1.0 | ||
| }, | ||
| "compensate_probability": { | ||
| "type": "boolean", | ||
| "description": "If this parameter is true, compensate probability of objects to threshold probability.", | ||
| "default": "false" | ||
| } | ||
| }, | ||
| "required": [ | ||
| "bounding_box_margin", | ||
| "split_threshold_velocity", | ||
| "threshold_yaw_diff", | ||
| "velocity_weight_min_distance", | ||
| "velocity_weight_average", | ||
| "velocity_weight_median", | ||
| "velocity_weight_target_value_average", | ||
| "velocity_weight_target_value_top", | ||
| "convert_doppler_to_twist", | ||
| "threshold_probability", | ||
| "compensate_probability" | ||
| ] | ||
| } | ||
| }, | ||
| "required": ["node_params", "core_params"] | ||
| } | ||
| }, | ||
| "properties": { | ||
| "/**": { | ||
| "type": "object", | ||
| "properties": { | ||
| "ros__parameters": { | ||
| "$ref": "#/definitions/radar_fusion_to_detected_object" | ||
| } | ||
| }, | ||
| "required": ["ros__parameters"] | ||
| } | ||
| }, | ||
| "required": ["/**"] | ||
| } |
This file contains 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
Oops, something went wrong.
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.
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.
[MUST] Probability value is defined by float in Autoware message. Please revert to float.
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.
Hi @scepter914 , it was "float" in my previous commit, but @ambroise-arm asked me to change to "double" because of the new Declare Parameter Function. ( see in #4537 (comment))
Please let me know which to follow. Thanks!
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.
@scepter914 Yes, as I wrote in the comment linked above, the return type of
declare_parameter<float>()is actuallydouble. So for clarity we should writedeclare_parameter<double>().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.
@PhoebeWu21 @ambroise-arm
Thank you for comment.
I agree with the reason changing from "float" to "double", but pre-commit.ci failed.
So I cannot approve until you debug this build fail.
I think two way to solve this problem.
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 am not sure how the change from float to double relates to the pre-commit.ci failure. It is not a build job but a linting job, and the failures it reports look unrelated to the changes in this PR.
Let's see what the
build-and-test*jobs result in for building the code.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.
There are no pre-commit.ci failures in other PR. So I guess build-and-test* jobs don't have problem and your change cause pre-commit.ci failures.
I recommend that you construct local environment and debug it.
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.
Hi @scepter914,
Finally, pre-commit.ci check has passed.
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.
@PhoebeWu21
[imo] Since
core_param_.threshold_probabilityis defined as float, this is implicit type conversion here.Is this meant?