Skip to content
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

refactor(ekf_localizer): rework parameters #6196

Conversation

Motsu-san
Copy link
Contributor

@Motsu-san Motsu-san commented Jan 29, 2024

Description

This pull request must be merged with the autoware_launch pull request.
Please see both pull request.

Implement the ROS Node configuration layout described in https://github.com/orgs/autowarefoundation/discussions/3371.

  • Remove the default value from the source code in order to ensure all parameter values are passed from the parameter files.
    • already completed.
  • Create the yaml configs in this package and the autoware_launch repository.
    • already completed.
  • Create the schema.
    • This PR.
  • Update Parameters chapter in the readme file.
    • This PR.

Tests performed

confirm parameters

  1. run autoware & replay rosbag that can start localization(Autoware sample rosbag is recommended).
  2. command the following
ros2 param dump /localization/pose_twist_fusion_filter/ekf_localizer 

expected result

/localization/pose_twist_fusion_filter/ekf_localizer:
  ros__parameters:
    diagnostics:
      pose_no_update_count_threshold_error: 100
      pose_no_update_count_threshold_warn: 50
      twist_no_update_count_threshold_error: 100
      twist_no_update_count_threshold_warn: 50
    misc:
      threshold_observable_velocity_mps: 0.0
    node:
      enable_yaw_bias_estimation: true
      extend_state_step: 50
      predict_frequency: 50.0
      publish_tf: true
      show_debug_info: false
      tf_rate: 50.0
    pose_frame_id: map
    pose_measurement:
      pose_additional_delay: 0.0
      pose_gate_dist: 10000.0
      pose_smoothing_steps: 5
    process_noise:
      proc_stddev_vx_c: 10.0
      proc_stddev_wz_c: 5.0
      proc_stddev_yaw_c: 0.005
    qos_overrides:
      /clock:
        subscription:
          depth: 1
          durability: volatile
          history: keep_last
          reliability: best_effort
      /parameter_events:
        publisher:
          depth: 1000
          durability: volatile
          history: keep_last
          reliability: reliable
      /tf:
        publisher:
          depth: 100
          durability: volatile
          history: keep_last
          reliability: reliable
    simple_1d_filter_parameters:
      pitch_filter_proc_dev: 0.01
      roll_filter_proc_dev: 0.01
      z_filter_proc_dev: 1.0
    twist_measurement:
      twist_additional_delay: 0.0
      twist_gate_dist: 10000.0
      twist_smoothing_steps: 2
    use_sim_time: true

Confirm json schema

  1. Confirm if check-jsonschema is installed.
pip3 install -U check-jsonschema
  1. Command the following
cd [Your Autoware.universe dir]/localization/ekf_localizer
find -wholename '*/schema/*.schema.json' -printf '%p: ' -execdir bash -c 'check-jsonschema -v --schemafile "$1" ../config/"${1:2:-12}"*.param.yaml' bash '{}' +

result

localization/ekf_localizer$ find -wholename '*/schema/*.schema.json' -printf '%p: ' -execdir bash -c 'check-jsonschema -v --schemafile "$1" ../config/"${1:2:-12}"*.param.yaml' bash '{}' +
./schema/ekf_localizer.schema.json: ok -- validation done
The following files were checked:
  ../config/ekf_localizer.param.yaml

Confirm README display

  1. Ready to use mkdocs serve with this instruction.
  2. Command the following
cd [Your Autoware.universe dir]
mkdocs serve

Effects on system behavior

Not applicable.

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.

  • There are no open discussions or they are tracked via tickets.

After all checkboxes are checked, anyone who has write access can merge the PR.

@github-actions github-actions bot added type:documentation Creating or refining documentation. (auto-assigned) component:localization Vehicle's position determination in its environment. (auto-assigned) labels Jan 29, 2024
@Motsu-san Motsu-san force-pushed the refactor/rework_parameter_ekf_localizer branch from 99207d8 to 88f4f81 Compare January 29, 2024 01:58
@Motsu-san Motsu-san changed the title Refactor/rework parameter ekf localizer refactor(ekf_localizer): rework parameters Jan 29, 2024
@Motsu-san Motsu-san marked this pull request as ready for review January 29, 2024 05:00
@Motsu-san Motsu-san self-assigned this Jan 29, 2024
@Motsu-san Motsu-san force-pushed the refactor/rework_parameter_ekf_localizer branch from 3ce3148 to a41202c Compare January 29, 2024 10:56
@Motsu-san Motsu-san marked this pull request as draft January 29, 2024 10:57
@Motsu-san Motsu-san force-pushed the refactor/rework_parameter_ekf_localizer branch 8 times, most recently from fc30c3e to 15dacad Compare February 2, 2024 05:24
@Motsu-san Motsu-san marked this pull request as ready for review February 2, 2024 05:58
@Motsu-san Motsu-san requested a review from anhnv3991 as a code owner February 2, 2024 05:58
@Motsu-san Motsu-san added the tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Feb 2, 2024
Copy link

codecov bot commented Feb 2, 2024

Codecov Report

Attention: 29 lines in your changes are missing coverage. Please review.

Comparison is base (9b4d7e4) 14.91% compared to head (1602f56) 14.36%.
Report is 4 commits behind head on main.

❗ Current head 1602f56 differs from pull request most recent head a363f86. Consider uploading reports for the commit a363f86 to get more accurate results

Files Patch % Lines
...calizer/include/ekf_localizer/hyper_parameters.hpp 0.00% 0 Missing and 29 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6196      +/-   ##
==========================================
- Coverage   14.91%   14.36%   -0.55%     
==========================================
  Files        1817     1907      +90     
  Lines      125357   130137    +4780     
  Branches    37640    37641       +1     
==========================================
+ Hits        18693    18697       +4     
- Misses      85652    90426    +4774     
- Partials    21012    21014       +2     
Flag Coverage Δ *Carryforward flag
differential 43.79% <0.00%> (?)
total 14.37% <ø> (-0.55%) ⬇️ Carriedforward from bd4b5ca

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Motsu-san Motsu-san force-pushed the refactor/rework_parameter_ekf_localizer branch from dda33fd to 7bc163b Compare February 2, 2024 07:41
@Motsu-san Motsu-san force-pushed the refactor/rework_parameter_ekf_localizer branch from a3be667 to 6dee595 Compare February 2, 2024 07:46
@KYabuuchi
Copy link
Contributor

I will review soon. Please wait 🙏

@KYabuuchi KYabuuchi added the run:deploy-docs Mark for deploy-docs action generation. (used-by-ci) label Feb 2, 2024
Copy link
Contributor

@KYabuuchi KYabuuchi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pose_frame_id parameter has a default value so please modify it. 🙏
Except for that, there is no problem

@Motsu-san Motsu-san force-pushed the refactor/rework_parameter_ekf_localizer branch from 347e5fb to 48109f1 Compare February 5, 2024 01:05
@Motsu-san Motsu-san requested a review from KYabuuchi February 5, 2024 01:08
@Motsu-san Motsu-san force-pushed the refactor/rework_parameter_ekf_localizer branch 2 times, most recently from 0c7f674 to 76d11bb Compare February 5, 2024 02:05
Copy link
Contributor

@KYabuuchi KYabuuchi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🙆

@Motsu-san Motsu-san force-pushed the refactor/rework_parameter_ekf_localizer branch from f43c38f to 1602f56 Compare February 5, 2024 04:09
@Motsu-san Motsu-san force-pushed the refactor/rework_parameter_ekf_localizer branch from 1602f56 to a363f86 Compare February 5, 2024 10:10
@Motsu-san Motsu-san merged commit 7286437 into autowarefoundation:main Feb 6, 2024
16 of 18 checks passed
@Motsu-san Motsu-san deleted the refactor/rework_parameter_ekf_localizer branch February 6, 2024 00:21
zulfaqar-azmi-t4 pushed a commit to zulfaqar-azmi-t4/autoware.universe that referenced this pull request Feb 6, 2024
* refactor: Create JSON Schema files

Signed-off-by: Motsu-san <[email protected]>

* Fix: Modify the descriptions of parameters

Signed-off-by: Motsu-san <[email protected]>

* fix: Redo modification of the descriptions

Signed-off-by: Motsu-san <[email protected]>

* doc: Replace parameter tables to JSON Schema ones in README

Signed-off-by: Motsu-san <[email protected]>

* refactor: Remove default value from source code and launch.xml
with arrangement of param .yaml

Signed-off-by: Motsu-san <[email protected]>

* style(pre-commit): autofix

---------

Signed-off-by: Motsu-san <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
anhnv3991 pushed a commit to anhnv3991/autoware.universe that referenced this pull request Feb 13, 2024
* refactor: Create JSON Schema files

Signed-off-by: Motsu-san <[email protected]>

* Fix: Modify the descriptions of parameters

Signed-off-by: Motsu-san <[email protected]>

* fix: Redo modification of the descriptions

Signed-off-by: Motsu-san <[email protected]>

* doc: Replace parameter tables to JSON Schema ones in README

Signed-off-by: Motsu-san <[email protected]>

* refactor: Remove default value from source code and launch.xml
with arrangement of param .yaml

Signed-off-by: Motsu-san <[email protected]>

* style(pre-commit): autofix

---------

Signed-off-by: Motsu-san <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
karishma1911 pushed a commit to Interplai/autoware.universe that referenced this pull request Jun 3, 2024
* refactor: Create JSON Schema files

Signed-off-by: Motsu-san <[email protected]>

* Fix: Modify the descriptions of parameters

Signed-off-by: Motsu-san <[email protected]>

* fix: Redo modification of the descriptions

Signed-off-by: Motsu-san <[email protected]>

* doc: Replace parameter tables to JSON Schema ones in README

Signed-off-by: Motsu-san <[email protected]>

* refactor: Remove default value from source code and launch.xml
with arrangement of param .yaml

Signed-off-by: Motsu-san <[email protected]>

* style(pre-commit): autofix

---------

Signed-off-by: Motsu-san <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:localization Vehicle's position determination in its environment. (auto-assigned) run:deploy-docs Mark for deploy-docs action generation. (used-by-ci) tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) type:documentation Creating or refining documentation. (auto-assigned)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants