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(lidar_apollo_segmentation_tvm)!: remove packages #5022

Merged
merged 5 commits into from
Jul 29, 2024

Conversation

kminoda
Copy link
Contributor

@kminoda kminoda commented Jul 23, 2024

Description

Solves autowarefoundation/autoware.universe#8175
See autowarefoundation/autoware.universe#7996

Tests performed

See autowarefoundation/autoware.universe#7996

Effects on system behavior

See autowarefoundation/autoware.universe#7996

Interface changes

See autowarefoundation/autoware.universe#7996

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.

@kminoda kminoda changed the title feat!: remove lidar_apollo_segmentation_tvm feat(lidar_apollo_segmentation_tvm)!: remove packages Jul 23, 2024
Copy link
Member

@youtalk youtalk left a comment

Choose a reason for hiding this comment

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

Once autowarefoundation/autoware.universe#8150 was merged, it would be time to review.

@kminoda
Copy link
Contributor Author

kminoda commented Jul 23, 2024

@youtalk Thanks. Let me convert this to a draft at this moment.

@kminoda kminoda marked this pull request as draft July 23, 2024 08:47
@ktro2828
Copy link

ktro2828 commented Jul 24, 2024

Both lidar_apollo_segmentation_tvm and lidar_apollo_segmentation_tvm_nodes will be removed by autowarefoundation/autoware.universe#7996.

@kminoda kminoda marked this pull request as ready for review July 24, 2024 10:27
@kminoda
Copy link
Contributor Author

kminoda commented Jul 24, 2024

@youtalk Now ready to get your review. Thanks! 🙏

Copy link
Member

@youtalk youtalk left a comment

Choose a reason for hiding this comment

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

LGTM!
It is also worth to reduce the container image sizes. Nice work.

@kminoda
Copy link
Contributor Author

kminoda commented Jul 24, 2024

@youtalk Thanks. Would you merge this? I don't have the permission to do so.

@kminoda
Copy link
Contributor Author

kminoda commented Jul 25, 2024

Memo:
Build failing

#45 437.7 /autoware/src/universe/autoware.universe/map/map_height_fitter/src/map_height_fitter.cpp:148:10: error: ‘using element_type = struct autoware_map_msgs::srv::GetPartialPointCloudMap_Response_<std::allocator<void> >’ {aka ‘struct autoware_map_msgs::srv::GetPartialPointCloudMap_Response_<std::allocator<void> >’} has no member named ‘new_pointcloud_cells’; did you mean ‘new_pointcloud_with_ids’?
#45 437.7   148 |     res->new_pointcloud_cells.size());
#45 437.7       |          ^~~~~~~~~~~~~~~~~~~~
#45 437.7 /autoware/src/universe/autoware.universe/map/map_height_fitter/src/map_height_fitter.cpp:151:37: error: ‘using element_type = struct autoware_map_msgs::srv::GetPartialPointCloudMap_Response_<std::allocator<void> >’ {aka ‘struct autoware_map_msgs::srv::GetPartialPointCloudMap_Response_<std::allocator<void> >’} has no member named ‘new_pointcloud_cells’; did you mean ‘new_pointcloud_with_ids’?
#45 437.7   151 |   for (const auto & pcd_cell : res->new_pointcloud_cells) {
#45 437.7       |                                     ^~~~~~~~~~~~~~~~~~~~
#45 437.7       |                                     new_pointcloud_with_ids

@xmfcx
Copy link
Contributor

xmfcx commented Jul 25, 2024

@kminoda san, it's not your fault,

Either we switch them to main branch or make a new release for autoware_msgs and update the tag here.

So updating something would normally require 2 PR (e.g. universe and msgs). Now it will require 3 or 4 PRs (for autoware.repos and maybe for version update for msgs)

cc. @youtalk @mitsudome-r

@kminoda
Copy link
Contributor Author

kminoda commented Jul 25, 2024

@xmfcx Thanks. So is there anything I can do? Or should I just wait until those tasks are done?

@xmfcx
Copy link
Contributor

xmfcx commented Jul 25, 2024

Yes, we need to wait for this task for now. It's approved and will be merged anyways.

@xmfcx xmfcx merged commit c3e8374 into autowarefoundation:main Jul 29, 2024
14 of 15 checks passed
@youtalk
Copy link
Member

youtalk commented Jul 29, 2024

@kminoda Congrats finally!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tag:run-health-check Run health-check
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants