feat(obstacle_collision_checker): fix dynamic parameter update #2153

brkay54 · 2022-10-25T11:38:29Z

Signed-off-by: Berkay Karaman berkay@leodrive.ai

Description

While Bus ODD tests, we needed to use obstacle_collision_checker package to bring to vehicle collision awareness. If vehicle can not stop with nominal acceleration before collision, it should send emergency stop. However, obstacle_collision_checker failed in tests.
After little investigation, we realize the parameters are returning zero. Because search_radius is returning zero, node can not detect anything in the lane. After some changes and updating param call back function, node run successfully.

Tests performed

Notes for reviewers

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

I've confirmed the contribution guidelines.
The PR follows the pull request guidelines.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

The PR follows the pull request guidelines.
The PR has been properly tested.
The PR has been reviewed by the code owners.

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.
The PR is ready for merge.

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

codecov · 2022-10-25T11:51:01Z

Codecov Report

Base: 10.82% // Head: 10.80% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (006a742) compared to base (cf6dda5).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2153      +/-   ##
==========================================
- Coverage   10.82%   10.80%   -0.02%     
==========================================
  Files        1177     1177              
  Lines       84506    84647     +141     
  Branches    19898    19986      +88     
==========================================
+ Hits         9149     9150       +1     
- Misses      65666    65797     +131     
- Partials     9691     9700       +9

Flag	Coverage Δ		*Carryforward flag
differential	`7.74% <0.00%> (?)`
total	`10.80% <0.00%> (ø)`		Carriedforward from cf6dda5

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

Impacted Files	Coverage Δ
...lision_checker_node/obstacle_collision_checker.cpp	`5.00% <0.00%> (-2.53%)`	⬇️
...n_checker_node/obstacle_collision_checker_node.cpp	`0.86% <0.00%> (+0.13%)`	⬆️
...ner/scene_module/pull_over/geometric_pull_over.hpp	`0.00% <0.00%> (ø)`
...nner/scene_module/pull_over/goal_searcher_base.hpp	`0.00% <ø> (ø)`
...src/scene_module/pull_over/geometric_pull_over.cpp	`0.00% <0.00%> (ø)`
...anner/src/scene_module/pull_over/goal_searcher.cpp	`0.00% <0.00%> (ø)`
...er/src/scene_module/pull_over/pull_over_module.cpp	`0.00% <0.00%> (ø)`
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

maxime-clem · 2022-10-26T00:30:17Z

...cle_collision_checker/include/obstacle_collision_checker/obstacle_collision_checker_node.hpp

-  // Dynamic Reconfigure
-  OnSetParametersCallbackHandle::SharedPtr set_param_res_;
-  rcl_interfaces::msg::SetParametersResult paramCallback(
-    const std::vector<rclcpp::Parameter> & parameters);


This should not be removed and I do not understand why this fixes the issue.
Is another node setting the parameter search_radius to 0 ?

I have the same question. I'd like to fix it properly.

I added dynamic reconfiguration again, and I updated update_param function, then parameters were set successfully. Also, I added node to control.launch.py. I am gonna change the PR name.

…refoundation#2153 Signed-off-by: Berkay Karaman <berkay@leodrive.ai>

Signed-off-by: Berkay Karaman <berkay@leodrive.ai>

kenji-miyake · 2022-10-27T19:16:05Z

...bstacle_collision_checker/src/obstacle_collision_checker_node/obstacle_collision_checker.cpp

+  for (size_t i = 1; i < vehicle_footprints.size();
+       i++) {  // skip first footprint because surround obstacle checker handle it


Could you write the comment above the for-loop to fix layout?

Suggested change

for (size_t i = 1; i < vehicle_footprints.size();

i++) { // skip first footprint because surround obstacle checker handle it

// skip first footprint because surround obstacle checker handle it

for (size_t i = 1; i < vehicle_footprints.size(); i++) {

kenji-miyake · 2022-10-27T19:22:17Z

launch/tier4_control_launch/launch/control.launch.py

@@ -266,6 +297,7 @@ def launch_setup(context, *args, **kwargs):
            shift_decider_component,
            vehicle_cmd_gate_component,
            operation_mode_transition_manager_component,
+            obstacle_collision_checker_component,


@TakaHoribe @yukkysaito How should we handle this? Is it okay for TIER IV to add this here, or should we consider another way? Maybe adding a if-condition is good?

It depends on the product, so I want to add an if-condition.

BTW, I feel strange that obstacle_collision_checker is in the control component.
I think it would be better to put obstacle_collision_checker into diagnostic systems and make it configurable there in the future.

kenji-miyake · 2022-10-27T19:26:11Z

...le_collision_checker/src/obstacle_collision_checker_node/obstacle_collision_checker_node.cpp

-    [&name](const rclcpp::Parameter & parameter) { return parameter.get_name() == name; });
-  if (it != parameters.cend()) {
-    value = it->template get_value<T>();
+  const auto itr = std::find_if(


@brkay54 Since it seems that bug-fix and refactoring are mixed, could you split PRs into the appropriate sizes/types?

Thank you for your review. I am going to close this PR and create 3 PRs for bug-fix, refactoring, and for launch files (I gonna add flag for obstacle_collision_checker to enable).

brkay54 self-assigned this Oct 25, 2022

brkay54 requested a review from kenji-miyake as a code owner October 25, 2022 11:38

maxime-clem reviewed Oct 26, 2022

View reviewed changes

brkay54 requested a review from TakaHoribe as a code owner October 27, 2022 12:04

brkay54 changed the title ~~feat(obstacle_collision_checker): remove dynamic parameter update~~ feat(obstacle_collision_checker): fix dynamic parameter update Oct 27, 2022

brkay54 and others added 3 commits October 27, 2022 15:53

feat(obstacle_collision_checker): fix dynamic parameter update autowa…

d678077

…refoundation#2153 Signed-off-by: Berkay Karaman <berkay@leodrive.ai>

param update fixed and node added launch file

252cdd7

Signed-off-by: Berkay Karaman <berkay@leodrive.ai>

ci(pre-commit): autofix

4957f10

brkay54 force-pushed the obstacle-collision-checker branch from 9e555e0 to 4957f10 Compare October 27, 2022 12:53

brkay54 requested a review from maxime-clem October 27, 2022 12:55

fix redundant blank line

006a742

Signed-off-by: Berkay Karaman <berkay@leodrive.ai>

kenji-miyake reviewed Oct 27, 2022

View reviewed changes

brkay54 closed this Oct 31, 2022

This was referenced Oct 31, 2022

fix(obstacle_collision_checker): fix bug on dynamic parameter update #2183

Merged

refactor(obstacle_collision_checker): add some debugs, change QoS #2191

Merged

feat(tier4_control_launch): add obstacle_collision_checker in control.launch.py #2193

Merged

brkay54 deleted the obstacle-collision-checker branch April 27, 2023 22:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(obstacle_collision_checker): fix dynamic parameter update #2153

feat(obstacle_collision_checker): fix dynamic parameter update #2153

brkay54 commented Oct 25, 2022 •

edited

Loading

codecov bot commented Oct 25, 2022 •

edited

Loading

maxime-clem Oct 26, 2022

kenji-miyake Oct 26, 2022

brkay54 Oct 27, 2022 •

edited

Loading

kenji-miyake Oct 27, 2022

kenji-miyake Oct 27, 2022 •

edited

Loading

yukkysaito Oct 28, 2022

kenji-miyake Oct 27, 2022 •

edited

Loading

brkay54 Oct 31, 2022 •

edited

Loading

		for (size_t i = 1; i < vehicle_footprints.size();
		i++) { // skip first footprint because surround obstacle checker handle it

feat(obstacle_collision_checker): fix dynamic parameter update #2153

feat(obstacle_collision_checker): fix dynamic parameter update #2153

Conversation

brkay54 commented Oct 25, 2022 • edited Loading

Description

Related links

Tests performed

Notes for reviewers

Pre-review checklist for the PR author

In-review checklist for the PR reviewers

Post-review checklist for the PR author

codecov bot commented Oct 25, 2022 • edited Loading

Codecov Report

maxime-clem Oct 26, 2022

Choose a reason for hiding this comment

kenji-miyake Oct 26, 2022

Choose a reason for hiding this comment

brkay54 Oct 27, 2022 • edited Loading

Choose a reason for hiding this comment

kenji-miyake Oct 27, 2022

Choose a reason for hiding this comment

kenji-miyake Oct 27, 2022 • edited Loading

Choose a reason for hiding this comment

yukkysaito Oct 28, 2022

Choose a reason for hiding this comment

kenji-miyake Oct 27, 2022 • edited Loading

Choose a reason for hiding this comment

brkay54 Oct 31, 2022 • edited Loading

Choose a reason for hiding this comment

brkay54 commented Oct 25, 2022 •

edited

Loading

codecov bot commented Oct 25, 2022 •

edited

Loading

brkay54 Oct 27, 2022 •

edited

Loading

kenji-miyake Oct 27, 2022 •

edited

Loading

kenji-miyake Oct 27, 2022 •

edited

Loading

brkay54 Oct 31, 2022 •

edited

Loading