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(default_ad_api): add operation mode api #1569

Merged

Conversation

isamu-takagi
Copy link
Contributor

@isamu-takagi isamu-takagi commented Aug 10, 2022

Signed-off-by: Takagi, Isamu isamu.takagi@tier4.jp

Description

This is an implementation of operation mode API. For details, see the discussion page.

Related links

#1535
#1586
tier4/tier4_autoware_msgs#46

Tests performed

  1. Launch the planning_simulator.launch.xml, initialize pose and set route.
  2. Call change_to_autonomous service, and check if the vehicle starts.
  3. Call change_to_stop service, and check if the vehicle stops.
  4. Call change_to_local and change_to_remote services, and check if gate_mode and selector_mode are changed (AutowareStatePanel is useful).

Notes for reviewers

Commands
ros2 service call /api/operation_mode/change_to_stop autoware_adapi_v1_msgs/srv/ChangeOperationMode
ros2 service call /api/operation_mode/change_to_autonomous autoware_adapi_v1_msgs/srv/ChangeOperationMode
ros2 service call /api/operation_mode/change_to_local autoware_adapi_v1_msgs/srv/ChangeOperationMode
ros2 service call /api/operation_mode/change_to_remote autoware_adapi_v1_msgs/srv/ChangeOperationMode

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.

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
@isamu-takagi isamu-takagi self-assigned this Aug 10, 2022
@isamu-takagi isamu-takagi marked this pull request as draft August 10, 2022 14:51
@isamu-takagi isamu-takagi changed the title Feature/ad api/operation mode feat(default_ad_api): add operation mode api Aug 10, 2022
isamu-takagi and others added 10 commits August 10, 2022 23:57
Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
…ate.msg

Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>
…ate.msg

Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>
Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
@codecov
Copy link

codecov bot commented Nov 10, 2022

Codecov Report

Base: 11.09% // Head: 11.14% // Increases project coverage by +0.04% 🎉

Coverage data is based on head (4064b33) compared to base (a40d23e).
Patch coverage: 24.09% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1569      +/-   ##
==========================================
+ Coverage   11.09%   11.14%   +0.04%     
==========================================
  Files        1213     1205       -8     
  Lines       86618    86380     -238     
  Branches    20787    20866      +79     
==========================================
+ Hits         9612     9626      +14     
+ Misses      66847    66571     -276     
- Partials    10159    10183      +24     
Flag Coverage Δ *Carryforward flag
differential 17.25% <24.09%> (?)
total 11.10% <ø> (+0.03%) ⬆️ Carriedforward from 0afa8ad

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

Impacted Files Coverage Δ
system/default_ad_api/src/operation_mode.cpp 24.09% <24.09%> (ø)
...vehicle_model/sim_model_ideal_steer_acc_geared.cpp 73.58% <0.00%> (-9.44%) ⬇️
...planning_evaluator/src/planning_evaluator_node.cpp 37.11% <0.00%> (-1.04%) ⬇️
control/vehicle_cmd_gate/src/vehicle_cmd_gate.hpp 0.00% <0.00%> (ø)
planning/motion_velocity_smoother/src/resample.cpp 0.00% <0.00%> (ø)
...rol/operation_mode_transition_manager/src/data.cpp 0.00% <0.00%> (ø)
...ol/operation_mode_transition_manager/src/state.cpp 0.00% <0.00%> (ø)
.../pointcloud_preprocessor/src/utility/utilities.cpp 0.00% <0.00%> (ø)
...ehavior_path_planner/src/behavior_tree_manager.cpp 0.00% <0.00%> (ø)
...n_velocity_smoother/src/smoother/smoother_base.cpp 0.00% <0.00%> (ø)
... and 45 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.

@isamu-takagi isamu-takagi marked this pull request as ready for review November 10, 2022 13:35
Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
@isamu-takagi isamu-takagi requested review from yabuta and tkhmy November 11, 2022 04:01
@tkhmy
Copy link
Contributor

tkhmy commented Nov 14, 2022

@isamu-takagi cc: @yabuta
Here are several commend for this PR

  1. I think the /api/operation_mode/state should be change to publish in 10Hz, as now it only publish when it receive mode change. This might cause the data loss if the connection is not stable, so I think is better to publish it in a constant frequency.
  2. The tools will required to base on the mode and choose the mode change, but currently I cannot get the mode in the initial, so the tools cannot know what is the current mode and cannot call the correct service.
  3. is_autonomous_mode_available is always false, cannot change to auto mode

@github-actions github-actions bot added document component:system System design and integration. (auto-assigned) labels Nov 15, 2022
@isamu-takagi
Copy link
Contributor Author

isamu-takagi commented Nov 15, 2022

@tkhmy Thank you for the comments.

  1. AD API avoid periodically publishing topics because it increases the amount of communication. The data loss should be guaranteed by DDS QoS.
  2. Since the initial state depends on the vehicle interface, the application need to wait until the first state
  3. Will it be true if you set initial pose and route?

@tkhmy
Copy link
Contributor

tkhmy commented Nov 15, 2022

@tkhmy Thank you for the comments.

  1. AD API avoid periodically publishing topics because it increases the amount of communication. The data loss should be guaranteed by DDS QoS.
  2. Since the initial state depends on the vehicle interface, the application need to wait until the first state
  3. Will it be true if you set initial pose and route?
  1. I think if AD API do not plan to periodically publish topic, it should be change to services instead of topic, because the app will not able to get the data is the change occur before the app launch.
  2. Which part will trigger the first state? As some of the application will need to use the api to trigger the mode change, currently in psim, we will not able to get any info.
  3. It still false even I set the initial pose and route

@isamu-takagi
Copy link
Contributor Author

I think if AD API do not plan to periodically publish topic, it should be change to services instead of topic, because the app will not able to get the data is the change occur before the app launch.

QoS of this topic is reliable/transient_local, so the application can read the message even if it is started later.

  1. Which part will trigger the first state? As some of the application will need to use the api to trigger the mode change, currently in psim, we will not able to get any info.

The state needed to set the initial pose.

  1. It still false even I set the initial pose and route

tier4/autoware_launch#561

@shmpwk shmpwk added type:documentation Creating or refining documentation. (auto-assigned) and removed document labels Nov 16, 2022
Copy link
Contributor

@tkhmy tkhmy left a comment

Choose a reason for hiding this comment

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

LGTM
Tested the new api

Copy link
Contributor

@yabuta yabuta left a comment

Choose a reason for hiding this comment

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

LGTM

@isamu-takagi isamu-takagi merged commit 29e8b43 into autowarefoundation:main Nov 17, 2022
@isamu-takagi isamu-takagi deleted the feature/ad-api/operation-mode branch November 17, 2022 06:54
HansRobo pushed a commit to HansRobo/autoware.universe that referenced this pull request Dec 16, 2022
* feat(autoware_ad_api_msgs): define operation mode interface

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat(default_ad_api): add operation mode api

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* fix: add message

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* Update common/autoware_ad_api_msgs/operation_mode/msg/OperationModeState.msg

Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>

* Update common/autoware_ad_api_msgs/operation_mode/msg/OperationModeState.msg

Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>

* fix: add message callback

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: add topic monitoring

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: use topic monitoring

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: modify topic monitoring config

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* fix: config name

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: modify diag name

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: move adapi message

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: change message type

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* fix: merge

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* WIP

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* fix: fix build error

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: move diagnostics

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: remove diagnostics

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: modify error message

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: remove unused code

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>
Signed-off-by: Kotaro Yoshimoto <pythagora.yoshimoto@gmail.com>
kminoda pushed a commit to kminoda/autoware.universe that referenced this pull request Jan 6, 2023
* feat(autoware_ad_api_msgs): define operation mode interface

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat(default_ad_api): add operation mode api

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* fix: add message

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* Update common/autoware_ad_api_msgs/operation_mode/msg/OperationModeState.msg

Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>

* Update common/autoware_ad_api_msgs/operation_mode/msg/OperationModeState.msg

Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>

* fix: add message callback

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: add topic monitoring

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: use topic monitoring

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: modify topic monitoring config

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* fix: config name

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: modify diag name

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: move adapi message

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: change message type

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* fix: merge

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* WIP

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* fix: fix build error

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: move diagnostics

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: remove diagnostics

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: modify error message

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: remove unused code

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>
Signed-off-by: kminoda <koji.minoda@tier4.jp>
YoshiRi pushed a commit to YoshiRi/autoware.universe that referenced this pull request Jan 11, 2023
* feat(autoware_ad_api_msgs): define operation mode interface

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat(default_ad_api): add operation mode api

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* fix: add message

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* Update common/autoware_ad_api_msgs/operation_mode/msg/OperationModeState.msg

Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>

* Update common/autoware_ad_api_msgs/operation_mode/msg/OperationModeState.msg

Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>

* fix: add message callback

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: add topic monitoring

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: use topic monitoring

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: modify topic monitoring config

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* fix: config name

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: modify diag name

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: move adapi message

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: change message type

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* fix: merge

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* WIP

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* fix: fix build error

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: move diagnostics

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: remove diagnostics

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: modify error message

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

* feat: remove unused code

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>

Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>
Signed-off-by: yoshiri <yoshiyoshidetteiu@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:system System design and integration. (auto-assigned) type:documentation Creating or refining documentation. (auto-assigned)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants