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

feat(lane_change_module): add update paramter function for non defined paramters #9887

Conversation

kyoichi-sugahara
Copy link
Contributor

@kyoichi-sugahara kyoichi-sugahara commented Jan 10, 2025

Description

In the original implementation, while some parameters could be updated through ros2 param set and affected the behavior, other parameters did not influence the behavior despite being updated.
However, since both the output and ros2 param dump results showed the changes, developers might have believed they had modified parameters when in fact the changes weren't affecting the system. This could cause issues during parameter tuning. This is caused by the parameter server that publishes misleading messages.

To address this, I've modified the implementation to ensure all parameters can affect the behavior through the updateParam function, making the runtime parameter updates consistent and effective.

  • This PR adds functionality to dynamically update additional parameters in the lane change module
  • Also fixed min_lane_changing_velocity parameter path

I removed the intermediate variables since they appeared redundant, but I don't fully understand why they were implemented in the original code.
If these intermediate variables served a specific purpose (e.g., validation, safety checks, or compatibility), I will revert these changes.

How was this PR tested?

set paramter with following command with attached script

ros2 param set /planning/scenario_planning/lane_driving/behavior_planning/behavior_path_planner lane_change.<newly_added_parameter_name> <parameter_value>

set_lane_change_param.zip

And got following output

output
$ ./set_lane_change_param.sh 
Setting lane_change.min_length_for_turn_signal_activation to 1.0
Set parameter successful: success
---
Setting lane_change.collision_check.enable_for_prepare_phase.general_lanes to true
Set parameter successful: success
---
Setting lane_change.collision_check.enable_for_prepare_phase.intersection to true
Set parameter successful: success
---
Setting lane_change.collision_check.enable_for_prepare_phase.turns to true
Set parameter successful: success
---
Setting lane_change.collision_check.check_current_lanes to true
Set parameter successful: success
---
Setting lane_change.collision_check.check_other_lanes to true
Set parameter successful: success
---
Setting lane_change.collision_check.use_all_predicted_paths to true
Set parameter successful: success
---
Setting lane_change.collision_check.prediction_time_resolution to 0.5
Set parameter successful: success
---
Setting lane_change.collision_check.yaw_diff_threshold to 0.785
Set parameter successful: success
---
Setting lane_change.safety_check.execution.longitudinal_distance_min_threshold to 3.0
Set parameter successful: success
---
Setting lane_change.safety_check.execution.longitudinal_velocity_delta_time to 0.5
Set parameter successful: success
---
Setting lane_change.safety_check.execution.expected_front_deceleration to 1.0
Set parameter successful: success
---
Setting lane_change.safety_check.execution.expected_rear_deceleration to 1.0
Set parameter successful: success
---
Setting lane_change.safety_check.execution.rear_vehicle_reaction_time to 1.0
Set parameter successful: success
---
Setting lane_change.safety_check.execution.rear_vehicle_safety_time_margin to 1.0
Set parameter successful: success
---
Setting lane_change.safety_check.execution.lateral_distance_max_threshold to 1.0
Set parameter successful: success
---
Setting lane_change.safety_check.parked.longitudinal_distance_min_threshold to 3.0
Set parameter successful: success
---
Setting lane_change.safety_check.parked.longitudinal_velocity_delta_time to 0.5
Set parameter successful: success
---
Setting lane_change.safety_check.parked.expected_front_deceleration to 1.0
Set parameter successful: success
---
Setting lane_change.safety_check.parked.expected_rear_deceleration to 1.0
Set parameter successful: success
---
Setting lane_change.safety_check.parked.rear_vehicle_reaction_time to 1.0
Set parameter successful: success
---
Setting lane_change.safety_check.parked.rear_vehicle_safety_time_margin to 1.0
Set parameter successful: success
---
Setting lane_change.safety_check.parked.lateral_distance_max_threshold to 1.0
Set parameter successful: success
---
Setting lane_change.safety_check.cancel.longitudinal_distance_min_threshold to 3.0
Set parameter successful: success
---
Setting lane_change.safety_check.cancel.longitudinal_velocity_delta_time to 0.5
Set parameter successful: success
---
Setting lane_change.safety_check.cancel.expected_front_deceleration to 1.0
Set parameter successful: success
---
Setting lane_change.safety_check.cancel.expected_rear_deceleration to 1.0
Set parameter successful: success
---
Setting lane_change.safety_check.cancel.rear_vehicle_reaction_time to 1.0
Set parameter successful: success
---
Setting lane_change.safety_check.cancel.rear_vehicle_safety_time_margin to 1.0
Set parameter successful: success
---
Setting lane_change.safety_check.cancel.lateral_distance_max_threshold to 1.0
Set parameter successful: success
---
Setting lane_change.safety_check.stuck.longitudinal_distance_min_threshold to 3.0
Set parameter successful: success
---
Setting lane_change.safety_check.stuck.longitudinal_velocity_delta_time to 0.5
Set parameter successful: success
---
Setting lane_change.safety_check.stuck.expected_front_deceleration to 1.0
Set parameter successful: success
---
Setting lane_change.safety_check.stuck.expected_rear_deceleration to 1.0
Set parameter successful: success
---
Setting lane_change.safety_check.stuck.rear_vehicle_reaction_time to 1.0
Set parameter successful: success
---
Setting lane_change.safety_check.stuck.rear_vehicle_safety_time_margin to 1.0
Set parameter successful: success
---
Setting lane_change.safety_check.stuck.lateral_distance_max_threshold to 1.0
Set parameter successful: success
---
Setting lane_change.delay_lane_change.enable to true
Set parameter successful: success
---
Setting lane_change.delay_lane_change.check_only_parked_vehicle to true
Set parameter successful: success
---
Setting lane_change.delay_lane_change.min_road_shoulder_width to 0.5
Set parameter successful: success
---
Setting lane_change.delay_lane_change.th_parked_vehicle_shift_ratio to 0.5
Set parameter successful: success
---
Setting lane_change.terminal_path.enable to true
Set parameter successful: success
---
Setting lane_change.terminal_path.disable_near_goal to true
Set parameter successful: success
---
Setting lane_change.terminal_path.stop_at_boundary to true
Set parameter successful: success
---
Parameter setting completed

run planning simulator and lane_change module executed.

Notes for reviewers

None.

Interface changes

None.

Effects on system behavior

None.

…delay lane change functionality

Signed-off-by: Kyoichi Sugahara <[email protected]>
@github-actions github-actions bot added the component:planning Route planning, decision-making, and navigation. (auto-assigned) label Jan 10, 2025
Copy link

github-actions bot commented Jan 10, 2025

Thank you for contributing to the Autoware project!

🚧 If your pull request is in progress, switch it to draft mode.

Please ensure:

@kyoichi-sugahara kyoichi-sugahara added the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Jan 10, 2025
Copy link

codecov bot commented Jan 14, 2025

Codecov Report

Attention: Patch coverage is 92.30769% with 6 lines in your changes missing coverage. Please review.

Project coverage is 29.48%. Comparing base (0715615) to head (83b1329).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
...e_behavior_path_lane_change_module/src/manager.cpp 92.30% 2 Missing and 4 partials ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main    #9887    +/-   ##
========================================
  Coverage   29.47%   29.48%            
========================================
  Files        1444     1446     +2     
  Lines      108232   108722   +490     
  Branches    42168    42441   +273     
========================================
+ Hits        31904    32056   +152     
- Misses      73249    73575   +326     
- Partials     3079     3091    +12     
Flag Coverage Δ *Carryforward flag
differential 21.06% <92.30%> (?)
total 29.49% <ø> (+0.01%) ⬆️ Carriedforward from 77b2493

*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.

@kyoichi-sugahara kyoichi-sugahara merged commit 9c0e183 into autowarefoundation:main Jan 14, 2025
32 of 34 checks passed
@kyoichi-sugahara kyoichi-sugahara deleted the refactor/lane_change/update_parameter branch January 14, 2025 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:planning Route planning, decision-making, and navigation. (auto-assigned) run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci)
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants