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

fix(dummy diag publisher): change diag name specification method to YAML #2745

Closed

Conversation

asana17
Copy link
Contributor

@asana17 asana17 commented Jan 25, 2023

Signed-off-by: asana17 [email protected]

Description

diag_name parameter of the ROS node dummy_diag_publisher is configured in the node launch file. dummy_diag_publisher can take only one diag name parameter and the ROS node is launched for each diagnostic. This causes differences in the launch files among projects.
I changed to publish diagnostic messages in one ROS node and multiple diag_name parameters can be configured in the parameter YAML file. status and is_active parameters of each diagnostic can be reconfigured from the command line.

I also changed the launch files to update tier4/autoware_launch. Please see the related links below for more details about the launch files.

Changes

  1. Use parameters in the YAML file as the name of diagnostics instead of the parameter in the launch file.
  2. Use new parameters to read from the YAML file and enable parameter reconfiguration for each diagnostic.
  3. Multiple diagnostics messages are published from one ROS node.

Related links

TIER IV INTERNAL LINK TO SLACK
https://github.com/tier4/autoware_launch/pull/762
tier4/aip_launcher#101

Tests performed

dummy_diag_publisher

  1. Run dummy_diag_pubilsher and rqt_runtime_monitor.
    The config YAML file is system/dummy_diag_publisher/config/dummy_diag_publisher.param.yaml
ros2 launch dummy_diag_publisher dummy_diag_publisher.launch.xml

Screenshot from 2023-01-25 15-24-23
2. Change the parameter from the command line.

ros2 param set /dummy_diag_publisher velodyne_temperature.status "Error"

Screenshot from 2023-01-25 15-25-01

ros2 param set /dummy_diag_publisher obstacle_crash.is_active false

Screenshot from 2023-01-25 15-26-05

launch files

I used tier4/pilot-auto for the tests. I replaced universe , launcher, and aip_launcher.

  1. Launch planning simulation. (x1, xx1)

terminal 1

source ~/pilot-auto/install/setup.zsh
ros2 launch autoware_launch planning_simulator.launch.xml map_path:=/home/asana/autoware_map/sample-map-planning 
vehicle_model:=lexus sensor_model:=aip_xx1

terminal 2

ros2 run rqt_runtime_monitor rqt_runtime_monitor
ros2 node list
ros2 topic list
  1. Launch system component (x1, xx1, x2)

terminal 1

ros2 launch autoware_launch tier4_system_component.launch.xml run_mode:=online launch_system_monitor:=true sensor_model:=aip_x2
ros2 launch autoware_launch tier4_system_component.launch.xml run_mode:=planning_simulation launch_system_monitor:=false sensor_model:=aip_x2

terminal 2

ros2 run rqt_runtime_monitor rqt_runtime_monitor
ros2 node list
ros2 topic list

In each case, I compared the rqt_runtime_monitor results, the node list results, and the topic list results and confirmed no difference other than points below.

  • service_log_checker is launched in tier4_system_launch, but is not launched in the previous system_launch.
  • The changes of dummy_diag_publisher nodes: all dummy diags are published by one dummy_diag_publisher node, and the diag names, Hardware IDs are changed as the screenshots.
    pilot-auto-xx1-xml-runtimemonitor
    pilot-auto-xx1-yaml-runtimemonitor

Notes for reviewers

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.

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

@asana17 asana17 added type:documentation Creating or refining documentation. (auto-assigned) component:system System design and integration. (auto-assigned) labels Jan 25, 2023
@asana17 asana17 requested a review from kminoda January 25, 2023 06:36
@asana17 asana17 self-assigned this Jan 25, 2023
@asana17 asana17 requested a review from a team as a code owner January 25, 2023 06:36
@@ -0,0 +1,32 @@
# Description:
Copy link
Contributor

@kminoda kminoda Jan 26, 2023

Choose a reason for hiding this comment

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

@asana17 Thank you for the PR!
In addition to this PR, we also need to update tier4/autoware_launch. Would you do that to see if this new dummy_diag_publisher actually works?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kminoda I made new PRs updating tier4/autoware_launch and tier4/aip_launcher. I add test details to this PR (#2745 (comment)).

@codecov
Copy link

codecov bot commented Jan 26, 2023

Codecov Report

Base: 11.52% // Head: 12.38% // Increases project coverage by +0.86% 🎉

Coverage data is based on head (f96d5e5) compared to base (b7b0188).
Patch coverage: 9.42% of modified lines in pull request are covered.

❗ Current head f96d5e5 differs from pull request most recent head 78c9f59. Consider uploading reports for the commit 78c9f59 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2745      +/-   ##
==========================================
+ Coverage   11.52%   12.38%   +0.86%     
==========================================
  Files        1305     1220      -85     
  Lines       91225    86213    -5012     
  Branches    24172    24382     +210     
==========================================
+ Hits        10510    10677     +167     
+ Misses      69720    64385    -5335     
- Partials    10995    11151     +156     
Flag Coverage Δ *Carryforward flag
differential 0.00% <0.00%> (?)
total 12.40% <10.35%> (+0.88%) ⬆️ Carriedforward from 73c36fe

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

Impacted Files Coverage Δ
...include/object_detection/object_polygon_detail.hpp 0.00% <ø> (ø)
...e/object_detection/object_polygon_display_base.hpp 0.00% <0.00%> (ø)
...gin/src/object_detection/object_polygon_detail.cpp 0.00% <0.00%> (ø)
...manager_rviz_plugin/src/bag_time_manager_panel.cpp 0.00% <0.00%> (ø)
...ion/include/interpolation/spline_interpolation.hpp 28.57% <ø> (ø)
...e/interpolation/spline_interpolation_points_2d.hpp 83.33% <ø> (ø)
...terpolation/test/src/test_spline_interpolation.cpp 28.37% <0.00%> (ø)
...ils/include/motion_utils/trajectory/trajectory.hpp 75.12% <ø> (ø)
common/motion_utils/src/marker/marker_helper.cpp 15.00% <0.00%> (-9.33%) ⬇️
.../rtc_manager_rviz_plugin/src/rtc_manager_panel.cpp 0.00% <0.00%> (ø)
... and 274 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.

@github-actions github-actions bot added type:ci Continuous Integration (CI) processes and testing. (auto-assigned) component:common Common packages from the autoware-common repository. (auto-assigned) component:control Vehicle control algorithms and mechanisms. (auto-assigned) component:localization Vehicle's position determination in its environment. (auto-assigned) component:map Map creation, storage, and loading. (auto-assigned) component:perception Advanced sensor data processing and environment understanding. (auto-assigned) component:planning Route planning, decision-making, and navigation. (auto-assigned) component:sensing Data acquisition from sensors, drivers, preprocessing. (auto-assigned) component:vehicle Vehicle-specific implementations, drivers, packages. (auto-assigned) labels Feb 8, 2023
@asana17 asana17 closed this Feb 8, 2023
@github-actions github-actions bot removed component:system System design and integration. (auto-assigned) component:map Map creation, storage, and loading. (auto-assigned) component:launch Launch files, scripts and initialization tools. (auto-assigned) component:planning Route planning, decision-making, and navigation. (auto-assigned) component:perception Advanced sensor data processing and environment understanding. (auto-assigned) component:vehicle Vehicle-specific implementations, drivers, packages. (auto-assigned) type:documentation Creating or refining documentation. (auto-assigned) component:control Vehicle control algorithms and mechanisms. (auto-assigned) component:localization Vehicle's position determination in its environment. (auto-assigned) component:common Common packages from the autoware-common repository. (auto-assigned) component:sensing Data acquisition from sensors, drivers, preprocessing. (auto-assigned) type:ci Continuous Integration (CI) processes and testing. (auto-assigned) labels Feb 8, 2023
@asana17
Copy link
Contributor Author

asana17 commented Feb 8, 2023

I sign-offed by mistake, so I closed this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants