-
Notifications
You must be signed in to change notification settings - Fork 52
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_v2x_msgs): add TrafficLightInfo.msg #160
base: tier4/universe
Are you sure you want to change the base?
Conversation
Signed-off-by: Y.Hisaki <[email protected]>
Are there any resources that explains the design of this message? If there are no existing discussion threads, I would at least would like to have a README added to the package explaining how the messages are expected to be used. |
@mitsudome-r |
It's okay to have internal discussion in slack, but I would like to make sure that the final design and the usage are visible somewhere in GitHub so it's more obvious when we look back. |
bool has_min_rest_time | ||
float32 min_rest_time | ||
float32 min_rest_time_to_red | ||
bool has_max_rest_time | ||
float32 max_rest_time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- What is the difference between min_rest_time and min_rest_time_to_red?
- Why max_rest_time does not have max_rest_time_to_red?
Also, it sounds more natural if we name it remaning_time instead of rest_time.
@@ -0,0 +1,7 @@ | |||
std_msgs/Header header | |||
autoware_perception_msgs/TrafficLightGroup[] states |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this message is for describing the remaining time for traffic light state, it might be better to explicitly name the variable name as current_state
or next_state
since it is confusing for the users.
unique_identifier_msgs/UUID vehicle_id | ||
uint64 target_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are these IDs for?
Related Links
Description
Added a messages to read the traffic light in v2x.
Remarks
Pre-Review Checklist for the PR Author
PR Author should check the checkboxes below when creating the PR.
Checklist for the PR Reviewer
Reviewers should check the checkboxes below before approval.
Post-Review Checklist for the PR Author
PR Author should check the checkboxes below before merging.
CI Checks