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

fix(yabloc_pose_initializer): disable downloading artifacts by default #4110

Merged
merged 1 commit into from
Jun 30, 2023

Conversation

esteve
Copy link
Contributor

@esteve esteve commented Jun 28, 2023

Description

Other packages that use artifacts, have a DOWNLOAD_ARTIFACTS option, so that they can be built without access to the internet (e.g. when built with Debian's sbuild). This PR adds such option and defaults it to off.

See autowarefoundation/autoware-deb-packages#18

Tests performed

Not applicable.

Effects on system behavior

Not applicable.

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.

Signed-off-by: Esteve Fernandez <esteve.fernandez@tier4.jp>
@esteve esteve force-pushed the disable-artifacts-download-yabloc-pose-initializer branch from 3d8f47d to 2fa6d6d Compare June 28, 2023 15:13
@codecov
Copy link

codecov bot commented Jun 28, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.01 ⚠️

Comparison is base (1b69779) 14.35% compared to head (2fa6d6d) 14.34%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4110      +/-   ##
==========================================
- Coverage   14.35%   14.34%   -0.01%     
==========================================
  Files        1569     1569              
  Lines      107991   107988       -3     
  Branches    31249    31248       -1     
==========================================
- Hits        15498    15496       -2     
  Misses      75639    75639              
+ Partials    16854    16853       -1     
Flag Coverage Δ *Carryforward flag
differential 0.00% <ø> (?)
total 14.34% <ø> (-0.01%) ⬇️ Carriedforward from 1b69779

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

Impacted Files Coverage Δ
...nner/include/behavior_path_planner/utils/utils.hpp 45.45% <ø> (ø)
planning/behavior_path_planner/src/utils/utils.cpp 15.17% <ø> (-0.12%) ⬇️

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

@KYabuuchi
Copy link
Contributor

@esteve Hi! I think this PR looks good.
However, I noticed that lidar_apollo_instance_segmentation also downloads files using CMake, but I'm curious why it doesn't result in any errors. If you don't mind, could you please provide an explanation? I would appreciate it.

@esteve
Copy link
Contributor Author

esteve commented Jun 29, 2023

@KYabuuchi thanks for the feedback. We're currently not building lidar_apollo_instance_segmentation as part of https://github.com/autowarefoundation/autoware-deb-packages, because it depends on TensorRT (https://github.com/autowarefoundation/autoware.universe/blob/main/perception/lidar_apollo_instance_segmentation/CMakeLists.txt#L15).

However, if we ever include it as part of the autoware-deb-packages repository, we'll have to disable automatic downloading of the artifacts. Moreover, we have #3137 which does disable downloading artifacts in lidar_apollo_instance_segmentation and other packages.

@KYabuuchi
Copy link
Contributor

I understand. It makes sense now. 👍

@esteve
Copy link
Contributor Author

esteve commented Jun 30, 2023

@KYabuuchi thanks for approving this PR, I will merge it and add it to the list of PRs that disable downloading artifacts by default.

@esteve esteve merged commit da9c9c5 into main Jun 30, 2023
22 checks passed
@esteve esteve deleted the disable-artifacts-download-yabloc-pose-initializer branch June 30, 2023 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:localization Vehicle's position determination in its environment. (auto-assigned)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants