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.

Sign up for GitHub

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

Jump to bottom

feat!: replace autoware_auto_msgs with autoware_msgs for perception modules #7245

Merged
merged 4 commits into from
Jun 4, 2024
Merged

feat!: replace autoware_auto_msgs with autoware_msgs for perception modules #7245

merged 4 commits into from
Jun 4, 2024

Conversation

mitsudome-r
Copy link
Member

Description

This is subset of #6893 to make it easier to review.
This includes all the modification for perception directory in the original PR.

Related links

#6893
https://github.com/orgs/autowarefoundation/discussions/3862

Tests performed

Notes for reviewers

Interface changes

Message types are modified according to the table in this comment.

Effects on system behavior

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 type:documentation Creating or refining documentation. (auto-assigned) component:perception Advanced sensor data processing and environment understanding. (auto-assigned) labels Jun 3, 2024
yukkysaito
yukkysaito reviewed Jun 4, 2024
View reviewed changes
perception/map_based_prediction/README.md
| -------------------------------------------------------- | ------------------------------------------------------- | ---------------------------------------------------------- |
| `~/perception/object_recognition/tracking/objects` | `autoware_perception_msgs::msg::TrackedObjects` | tracking objects without predicted path. |
| `~/vector_map` | `autoware_map_msgs::msg::LaneletMapBin` | binary data of Lanelet2 Map. |
| `~/perception/traffic_light_recognition/traffic_signals` | `autoware_perception_msgs::msg::TrafficLightGroupArray` | rearranged information on the corresponding traffic lights |
Copy link
Contributor

Choose a reason for hiding this comment

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

Note : It may be necessary to change the topic name to traffic_light_group, but first complete the transition to autoware_auto_msgs and then use a separate PR.

yukkysaito
yukkysaito reviewed Jun 4, 2024
View reviewed changes
perception/map_based_prediction/src/map_based_prediction_node.cpp Outdated
Comment on lines 914 to 919
autoware_perception_msgs::msg::ObjectClassification classification;
for (size_t i = 0; i < tracked_object.classification.size(); i++) {
classification.label = tracked_object.classification[i].label;
classification.probability = tracked_object.classification[i].probability;
predicted_object.classification.push_back(classification);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: Need to confirm why the writing style was changed.

Copy link
Contributor

Choose a reason for hiding this comment

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

  predicted_object.classification = tracked_object.classification;

I have confirmed that it works correctly with the original code here with planning simulator.

Copy link
Contributor

Choose a reason for hiding this comment

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

d258b4f

yukkysaito
yukkysaito approved these changes Jun 4, 2024
View reviewed changes
Copy link
Contributor

@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

@xmfcx xmfcx force-pushed the autoware_msg_perception branch from 8ed33d3 to 2048cd5 Compare June 4, 2024 12:56
@xmfcx
Copy link
Contributor

xmfcx commented Jun 4, 2024

Added 3d05d58 or it won't compile.

@xmfcx
Copy link
Contributor

xmfcx commented Jun 4, 2024 •
edited
Loading

This depends on:

@xmfcx xmfcx added the run:deploy-docs Mark for deploy-docs action generation. (used-by-ci) label Jun 4, 2024
@xmfcx xmfcx added the tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Jun 4, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 4, 2024

Documentation URL: https://autowarefoundation.github.io/autoware.universe/pr-7245/
Modified URLs:

@yukkysaito
Copy link
Contributor

I'm working now in this branch

@xmfcx
Copy link
Contributor

xmfcx commented Jun 4, 2024

I'm working now in this branch

@yukkysaito san, you have approved, I've assumed you were done with this PR.

@yukkysaito
Copy link
Contributor

@xmfcx Sorry. I thought this was for review, and the main content was in this PR. I've made the necessary changes in this PR, so please wait as you will need to cherry-pick them.

@yukkysaito
Copy link
Contributor

@xmfcx This is ready for merge.

@yukkysaito yukkysaito enabled auto-merge (squash) June 4, 2024 14:24
@yukkysaito yukkysaito disabled auto-merge June 4, 2024 14:26
@xmfcx
Copy link
Contributor

xmfcx commented Jun 4, 2024

Since this depends on:

I will merge that first 👍

@xmfcx xmfcx force-pushed the autoware_msg_perception branch from a0dcc31 to c3c1e93 Compare June 4, 2024 14:56
@xmfcx
Copy link
Contributor

xmfcx commented Jun 4, 2024

Also depends on:

Checking that now.

mitsudome-r and others added 4 commits June 4, 2024 18:04
@mitsudome-r @cyn-liu
@NorahXiong @beginningfan @xmfcx
feat!: replace autoware_auto_msgs with autoware_msgs for perception m…
e4db7e9 
…odules

Signed-off-by: Ryohsuke Mitsudome <[email protected]>
Co-authored-by: Cynthia Liu <[email protected]>
Co-authored-by: NorahXiong <[email protected]>
Co-authored-by: beginningfan <[email protected]>
@pre-commit-ci @xmfcx
style(pre-commit): autofix
018309c
@xmfcx
add autoware_perception_msgs dependency
7c04fda 
Signed-off-by: M. Fatih Cırıt <[email protected]>
@yukkysaito @xmfcx
cosmetic change
125274b 
Signed-off-by: Yukihito Saito <[email protected]>
@xmfcx xmfcx force-pushed the autoware_msg_perception branch from c3c1e93 to 125274b Compare June 4, 2024 15:04
@yukkysaito
Copy link
Contributor

No logic change and confirmed that it is a replacement (no _auto_msgs).

@xmfcx
Copy link
Contributor

xmfcx commented Jun 4, 2024

colcon build --symlink-install --cmake-args -DCMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_EXPORT_COMPILE_COMMANDS=1 --packages-up-to bytetrack cluster_merger compare_map_segmentation crosswalk_traffic_light_estimator detected_object_feature_remover detected_object_validation detection_by_tracker elevation_map_loader euclidean_cluster ground_segmentation image_projection_based_fusion lidar_apollo_instance_segmentation lidar_apollo_segmentation_tvm lidar_apollo_segmentation_tvm_nodes lidar_centerpoint lidar_centerpoint_tvm map_based_prediction multi_object_tracker object_merger object_range_splitter object_velocity_splitter occupancy_grid_map_outlier_filter probabilistic_occupancy_grid_map radar_crossing_objects_noise_filter radar_fusion_to_detected_object radar_object_clustering radar_object_tracker radar_tracks_msgs_converter raindrop_cluster_filter shape_estimation simple_object_merger tensorrt_classifier tensorrt_yolox tracking_object_merger traffic_light_arbiter traffic_light_classifier traffic_light_fine_detector traffic_light_map_based_detector traffic_light_multi_camera_fusion traffic_light_occlusion_predictor traffic_light_visualization
was successful.

There was no _auto_msgs references left in the perception folder.

I think we can merge.

@yukkysaito yukkysaito enabled auto-merge (squash) June 4, 2024 15:10
@yukkysaito yukkysaito disabled auto-merge June 4, 2024 15:10
@xmfcx xmfcx merged commit 09b6899 into main Jun 4, 2024
18 of 22 checks passed
@xmfcx xmfcx deleted the autoware_msg_perception branch June 4, 2024 15:10
a-maumau pushed a commit to a-maumau/autoware.universe that referenced this pull request Jun 7, 2024
@mitsudome-r @cyn-liu
@NorahXiong @beginningfan @yukkysaito @a-maumau
feat!: replace autoware_auto_msgs with autoware_msgs for perception m…
8049571 
…odules (autowarefoundation#7245)

Signed-off-by: Ryohsuke Mitsudome <[email protected]>
Co-authored-by: Cynthia Liu <[email protected]>
Co-authored-by: NorahXiong <[email protected]>
Co-authored-by: beginningfan <[email protected]>
Co-authored-by: M. Fatih Cırıt <[email protected]>
Co-authored-by: Yukihito Saito <[email protected]>
KhalilSelyan pushed a commit that referenced this pull request Jul 22, 2024
@mitsudome-r @cyn-liu
@NorahXiong @beginningfan @yukkysaito @KhalilSelyan
feat!: replace autoware_auto_msgs with autoware_msgs for perception m…
ca0ccc6 
…odules (#7245)

Signed-off-by: Ryohsuke Mitsudome <[email protected]>
Co-authored-by: Cynthia Liu <[email protected]>
Co-authored-by: NorahXiong <[email protected]>
Co-authored-by: beginningfan <[email protected]>
Co-authored-by: M. Fatih Cırıt <[email protected]>
Co-authored-by: Yukihito Saito <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xmfcx xmfcx xmfcx approved these changes

@yukkysaito yukkysaito yukkysaito approved these changes

@tzhong518 tzhong518 Awaiting requested review from tzhong518

@miursh miursh Awaiting requested review from miursh

@knzo25 knzo25 Awaiting requested review from knzo25

@technolojin technolojin Awaiting requested review from technolojin

@YoshiRi YoshiRi Awaiting requested review from YoshiRi

@dan-dnn dan-dnn Awaiting requested review from dan-dnn

@manato manato Awaiting requested review from manato

@scepter914 scepter914 Awaiting requested review from scepter914

@amc-nu amc-nu Awaiting requested review from amc-nu

@ktro2828 ktro2828 Awaiting requested review from ktro2828

@kyoichi-sugahara kyoichi-sugahara Awaiting requested review from kyoichi-sugahara

@takayuki5168 takayuki5168 Awaiting requested review from takayuki5168

@tkimura4 tkimura4 Awaiting requested review from tkimura4

@angry-crab angry-crab Awaiting requested review from angry-crab

@kminoda kminoda Awaiting requested review from kminoda

@ambroise-arm ambroise-arm Awaiting requested review from ambroise-arm

@badai-nguyen badai-nguyen Awaiting requested review from badai-nguyen

@kosuke55 kosuke55 Awaiting requested review from kosuke55

@Shin-kyoto Shin-kyoto Awaiting requested review from Shin-kyoto

@satoshi-ota satoshi-ota Awaiting requested review from satoshi-ota

Assignees

@mitsudome-r mitsudome-r

Labels
component:perception Advanced sensor data processing and environment understanding. (auto-assigned) run:deploy-docs Mark for deploy-docs action generation. (used-by-ci) tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) type:documentation Creating or refining documentation. (auto-assigned)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mitsudome-r @xmfcx @yukkysaito