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(tier4_rtc_msgs): add new field to rtc cooperate status #119

Merged

Conversation

satoshi-ota
Copy link
Contributor

@satoshi-ota satoshi-ota commented May 8, 2024

Related Links

TIER IV INERNAL LINK

Description

Add new field tier4_rtc_msgs/State state so that external tools (e.g. FOA, MOT) can detect canceled behavior.

image

uint8 type

uint8 WAITING_FOR_EXECUTION = 0
uint8 RUNNING = 1
uint8 ABORTING = 2
uint8 SUCCEEDED = 3
uint8 FAILED = 4

Example Usecase

situation state in cooperate status FOA
Detect obstacle on the lane. Avoidance module starts running and publishing cooperate status. WAITING_FOR_EXECUTION
Operator approves avoidance RTC request. RUNNING
Obstacle has gone. Avoidance maneuver is no longer needed. RUNNING
Avoidance module reverts avoidance maneuver and cooperate request. FAILED Detect state transition to FAILED from RUNNING. Notify operator of canceling.
Avoidance module stops running. FAILED

Remarks

Pre-Review Checklist for the PR Author

PR Author should check the checkboxes below when creating the PR.

  • Code is properly formatted
  • Assign PR to reviewer

Checklist for the PR Reviewer

Reviewers should check the checkboxes below before approval.

  • Commits are properly organized and messages are according to the guideline
  • Code is properly formatted
  • PR title describes the changes

Post-Review Checklist for the PR Author

PR Author should check the checkboxes below before merging.

  • All open points are addressed and tracked via issues or tickets
  • Write release notes

CI Checks

  • Build and test for PR: Required to pass before the merge.
  • Check spelling: NOT required to pass before the merge. It is up to the reviewer(s). See here if you want to add some words to the spell check dictionary.

@yukkysaito
Copy link
Collaborator

Can you draw a diagram of the state transitions, I'd be happy to ask for it in the README or something.

@satoshi-ota
Copy link
Contributor Author

@yukkysaito I adde diagram and example usage. Is it okay for you...?

@mitsudome-r
Copy link
Collaborator

@satoshi-ota Is the information going to be available in some documentation? No one would want to look for this PR to know the state transition. If its going to be described in FOA/MOT app's README, that's also okay, but I want to put it somewhere more visible.

@mitsudome-r
Copy link
Collaborator

Also, I thought it might be better to add some comments to explain what each field is used for, just like in sensor_msgs/Image message for example because the message is starting to get big and hard to understand the purpose of each fields just from the variable's name.

Something like the following might work (please fix the description since I'm not sure about some of the fields)

# module type
tier4_rtc_msgs/Module module

# received command from human operator
tier4_rtc_msgs/Command command_status

# current execution status of RTC module
tier4_rtc_msgs/State state

# Indicates whether Autoware thinks it's safe to execute RTC
bool safe

# Indicates if the module is in auto mode. 
# If true, Autoware will automatically execute RTC behavior without 
# waiting for human operator's command. (i.e., when "safe" field is true)
bool auto_mode

# distances to where RTC will start/finish it's execution
float32 start_distance
float32 finish_distance

This is just a suggestion and I'm open to other opinions about how we do this(e.g., explain in a separate README file, do it in a separate PR, etc).

@satoshi-ota
Copy link
Contributor Author

@satoshi-ota Is the information going to be available in some documentation? No one would want to look for this PR to know the state transition. If its going to be described in FOA/MOT app's README, that's also okay, but I want to put it somewhere more visible.

I'm thinkind I'll put documentation in https://autowarefoundation.github.io/autoware.universe/main/planning/rtc_interface/#rtc-interface.

@satoshi-ota
Copy link
Contributor Author

Also, I thought it might be better to add some comments to explain what each field is used for, just like in sensor_msgs/Image message for example because the message is starting to get big and hard to understand the purpose of each fields just from the variable's name.

Something like the following might work (please fix the description since I'm not sure about some of the fields)

# module type
tier4_rtc_msgs/Module module

# received command from human operator
tier4_rtc_msgs/Command command_status

# current execution status of RTC module
tier4_rtc_msgs/State state

# Indicates whether Autoware thinks it's safe to execute RTC
bool safe

# Indicates if the module is in auto mode. 
# If true, Autoware will automatically execute RTC behavior without 
# waiting for human operator's command. (i.e., when "safe" field is true)
bool auto_mode

# distances to where RTC will start/finish it's execution
float32 start_distance
float32 finish_distance

This is just a suggestion and I'm open to other opinions about how we do this(e.g., explain in a separate README file, do it in a separate PR, etc).

Thank you for your suggestion. I'll add explanation.

@satoshi-ota
Copy link
Contributor Author

@mitsudome-r I added variable explanation in 9e887f0.

Copy link
Collaborator

@yukkysaito yukkysaito left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@mitsudome-r mitsudome-r left a comment

Choose a reason for hiding this comment

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

LGTM

@mitsudome-r mitsudome-r merged commit cc93262 into tier4:tier4/universe May 9, 2024
9 checks passed
@satoshi-ota satoshi-ota deleted the feat/rtc-add-new-field branch May 9, 2024 06:44
go-sakayori pushed a commit that referenced this pull request Aug 1, 2024
* feat(tier4_rtc_msgs): add new field

Signed-off-by: satoshi-ota <[email protected]>

* chore(tier4_rtc_request): add description to msg file

Signed-off-by: satoshi-ota <[email protected]>

---------

Signed-off-by: satoshi-ota <[email protected]>
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.

3 participants