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: add vehicle status api #2930

Merged
merged 27 commits into from
Jul 3, 2023
Merged

Conversation

tkhmy
Copy link
Contributor

@tkhmy tkhmy commented Feb 22, 2023

Description

Related links

Tests performed

  • makesure all those api output the correct data
  1. /api/vehicle/kinematic

    • geographic_pose
      • use a map that contain correct MGRS or UTM
      • check whether the latitude and longitude output correctly
      • the altitude should be the same as the pose z value
    • make sure the following have the correct output with the related topic
      • pose = /localization/kinematic_state ->pose
      • twist = /localization/kinematic_state ->twist
      • accel = /localization/acceleration -> accel
  2. /api/vehicle/state

    • make sure the following have the correct output with the related topic
      • steering_tire_angle = /vehicle/status/steering_status ->steering_tire_angle
      • gear = /vehicle/status/gear_status ->report
      • turn_indicator = /vehicle/status/turn_indicators_status` -> report
      • hazard_light = /vehicle/status/hazard_lights_status` -> report
      • energy_level = /vehicle/status/battery_charge -> energy_level
        • can be test by using following command
        ros2 topic pub /vehicle/status/battery_charge tier4_vehicle_msgs/msg/BatteryStatus energy_level:\ 20

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.

@github-actions github-actions bot added component:common Common packages from the autoware-common repository. (auto-assigned) component:system System design and integration. (auto-assigned) labels Feb 22, 2023
Signed-off-by: tkhmy <[email protected]>
Signed-off-by: tkhmy <[email protected]>
Signed-off-by: tkhmy <[email protected]>
Signed-off-by: tkhmy <[email protected]>
@github-actions github-actions bot added the component:map Map creation, storage, and loading. (auto-assigned) label Mar 28, 2023
@tkhmy tkhmy marked this pull request as ready for review March 29, 2023 01:38
@tkhmy tkhmy requested a review from KYabuuchi March 29, 2023 01:39
@tkhmy
Copy link
Contributor Author

tkhmy commented Mar 29, 2023

@isamu-takagi @KYabuuchi
Do you think is better to separate the map_loader publish mgrs_grid function into another PR?

@KYabuuchi
Copy link
Contributor

@tkhmy Since those functions will be reviewed by different people, it would be better to separate them if you can.

@tkhmy
Copy link
Contributor Author

tkhmy commented Mar 29, 2023

@tkhmy Since those functions will be reviewed by different people, it would be better to separate them if you can.

@KYabuuchi
Thanks for your commend, I moved the change to this PR, please review
#3200

@github-actions github-actions bot removed the component:map Map creation, storage, and loading. (auto-assigned) label Mar 29, 2023
@github-actions github-actions bot added the component:map Map creation, storage, and loading. (auto-assigned) label Mar 29, 2023
Signed-off-by: tkhmy <[email protected]>
@stale stale bot added the status:stale Inactive or outdated issues. (auto-assigned) label Jun 10, 2023
@stale stale bot removed the status:stale Inactive or outdated issues. (auto-assigned) label Jun 21, 2023
Signed-off-by: tkhmy <[email protected]>
Signed-off-by: tkhmy <[email protected]>
@tkhmy
Copy link
Contributor Author

tkhmy commented Jun 21, 2023

@isamu-takagi I fixed the naming and it should be working now.
Can you have a look?

I think the autoware document currently do not have a link to the vehicle api page. can I create a PR for that?
About the door layout, will it be handle in vehicle interface and the api need to read the layout and door status from the vehicle interface?

@isamu-takagi
Copy link
Contributor

I think the autoware document currently do not have a link to the vehicle api page. can I create a PR for that?

@tkhmy Thank you for checking document. I updated pages in this PR autowarefoundation/autoware-documentation#406. Now the vehicle status page is available from side menu.

About the door layout, will it be handle in vehicle interface and the api need to read the layout and door status from the vehicle interface?

Yes. The vehicle driver needs to support a interface for multiple doors. So for now, how about returning an empty array for status and layout?

@tkhmy
Copy link
Contributor Author

tkhmy commented Jun 21, 2023

Thank you for checking document. I updated pages in this PR autowarefoundation/autoware-documentation#406. Now the vehicle status page is available from side menu.

@isamu-takagi Thank you, I just saw the update, I think it is good

Yes. The vehicle driver needs to support a interface for multiple doors. So for now, how about returning an empty array for status and layout?

Actually I notice that vehicle interface do not have the message type for publishing multiple message, the current available is this tier4_api_msgs/msg/DoorStatus.

I think we can ask define vehicle_interface to use the DoorStatusArray directly, then the api will just relay the topic.

How do you think?

Signed-off-by: tkhmy <[email protected]>
@isamu-takagi
Copy link
Contributor

I think we can ask define vehicle_interface to use the DoorStatusArray directly, then the api will just relay the topic.

@tkhmy It sounds good. So we need to define interface for vehicle driver. It may be better to separate the PR for the door.

@tkhmy
Copy link
Contributor Author

tkhmy commented Jun 21, 2023

It sounds good. So we need to define interface for vehicle driver. It may be better to separate the PR for the door.

@isamu-takagi Okay I will separate the door from this PR

Signed-off-by: tkhmy <[email protected]>
@codecov
Copy link

codecov bot commented Jun 21, 2023

Codecov Report

Patch coverage: 15.49% and project coverage change: +0.01 🎉

Comparison is base (0443a08) 14.36% compared to head (39bd585) 14.38%.

❗ Current head 39bd585 differs from pull request most recent head 9f12741. Consider uploading reports for the commit 9f12741 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2930      +/-   ##
==========================================
+ Coverage   14.36%   14.38%   +0.01%     
==========================================
  Files        1567     1568       +1     
  Lines      107877   111046    +3169     
  Branches    31249    33564    +2315     
==========================================
+ Hits        15498    15972     +474     
- Misses      75525    77664    +2139     
- Partials    16854    17410     +556     
Flag Coverage Δ *Carryforward flag
differential 15.14% <15.49%> (?)
total 14.36% <ø> (ø) Carriedforward from 2bb5e4f

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

Impacted Files Coverage Δ
system/default_ad_api/src/vehicle.cpp 15.49% <15.49%> (ø)

... and 22 files with indirect coverage changes

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

system/default_ad_api/src/vehicle.cpp Outdated Show resolved Hide resolved
system/default_ad_api/src/vehicle.hpp Outdated Show resolved Hide resolved
system/default_ad_api/src/vehicle.hpp Outdated Show resolved Hide resolved
system/default_ad_api/src/vehicle.hpp Outdated Show resolved Hide resolved
@tkhmy
Copy link
Contributor Author

tkhmy commented Jun 28, 2023

@isamu-takagi I changed the stuff that you request and also fixed the conflict with main, can you review again?

Copy link
Contributor

@isamu-takagi isamu-takagi left a comment

Choose a reason for hiding this comment

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

LGTM

@tkhmy tkhmy merged commit 0136613 into autowarefoundation:main Jul 3, 2023
@isamu-takagi isamu-takagi self-assigned this Jul 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:common Common packages from the autoware-common repository. (auto-assigned) component:system System design and integration. (auto-assigned)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants