feat(calibration_tools): new api and 2.0 release

knzo25 commented 11 months ago

Description

Tests performed

Notes for reviewers

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

[x] I've confirmed the contribution guidelines.
[x] The PR follows the pull request guidelines.

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.

[x] There are no open discussions or they are tracked via tickets.
[x] The PR is ready for merge.

After all checkboxes are checked, anyone who has write access can merge the PR.

knzo25 commented 10 months ago

@yabuta https://github.com/nedbat/coveragepy/issues/1392 https://github.com/pytest-dev/pytest-cov/issues/539

As in the previous links, CI/CD is failing regardless of the code (it looks for non-existing files). What happens in this case? Ignore CI/CD? Disable the offending ones? Will you look for a fix?

yabuta commented 10 months ago

@knzo25 Sorry, I couldn't figure out where in this PR CI/CD the above error is occurring. Is it happening in build-and-test-differential / build-and-test-differential (humble) (pull_request)?

knzo25 commented 10 months ago

@yabuta

The error is as follows:

2024-01-15T01:00:11.4597855Z Starting >>> new_extrinsic_calibration_manager 2024-01-15T01:00:13.0596455Z --- stderr: new_extrinsic_calibration_manager 2024-01-15T01:00:13.0597974Z --- output: new_extrinsic_calibration_manager 2024-01-15T01:00:13.0600676Z Warning: Couldn't parse '/__w/CalibrationTools/CalibrationTools/sensor/new_extrinsic_calibration_manager/signature_bootstrap.py': No source for code: '/__w/CalibrationTools/CalibrationTools/sensor/new_extrinsic_calibration_manager/signature_bootstrap.py'. (couldnt-parse) 2024-01-15T01:00:13.0603030Z --- 2024-01-15T01:00:13.0603624Z Failed <<< new_extrinsic_calibration_manager [1.60s, exited with code 2] 2024-01-15T01:00:13.0606444Z Warning: Couldn't parse '/__w/CalibrationTools/CalibrationTools/sensor/new_extrinsic_calibration_manager/signature_bootstrap.py': No source for code: '/__w/CalibrationTools/CalibrationTools/sensor/new_extrinsic_calibration_manager/signature_bootstrap.py'. (couldnt-parse)

Looking for the file signature_bootstrap.py on google led me to the links I posted before. Note that that does not happen at the build stage but the test one. Just in case, I commented out the test dependencies in the packages.xml to see it we could handle it that way temporarily but no luck

yabuta commented 10 months ago

@knzo25 I'll check, but if I can't resolve the cause, I'll try to disable CICD when you merge the PRs. Or does this error affect your development?

knzo25 commented 10 months ago

@yabuta It does not affect the development, but I don't really know who is in charge of CI/CD so in the meantime I just reported it here. When the time for the merge comes, if there has not been a solution, I will drop a mention so we can disable and merge

vividf commented 8 months ago

@knzo25 radar_lidar_calibrator also need to be added into default project

knzo25 commented 8 months ago

@vividf Added in 3f5684b

knzo25 commented 8 months ago

@vividf As a reminder:

As I mentioned a few weeks ago, the ball in on your court now
If you start a comment / review, if the answer is satisfactory, you need to click resolve. If it is not, you need to give the appropriate feedback.
As we saw during the taxi experiments, there are changes you did not comprehend. While there is no need to understand the whole codebase (unless you want to), you need to understand all the changes. If you need to study parts of the code to understand the changes, then that is also part of the code review. How much one needs to study the codebase to make a meaningful review depends on the person

codecov-commenter commented 8 months ago

Codecov Report

Attention: Patch coverage is 0% with 276 lines in your changes missing coverage. Please review.

Project coverage is 5.09%. Comparing base (bab5400) to head (45453cf). Report is 21 commits behind head on tier4/universe.

Files with missing lines	Patch %	Lines
...ier4_ground_plane_utils/src/ground_plane_utils.cpp	0.00%	122 Missing :warning:
common/tier4_tag_utils/src/lidartag_hypothesis.cpp	0.00%	101 Missing :warning:
common/tier4_tag_utils/src/apriltag_hypothesis.cpp	0.00%	32 Missing :warning:
common/tier4_tag_utils/src/apriltag_filter.cpp	0.00%	10 Missing :warning:
common/tier4_tag_utils/src/lidartag_filter.cpp	0.00%	10 Missing :warning:
...de/tier4_ground_plane_utils/ground_plane_utils.hpp	0.00%	1 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## tier4/universe #148 +/- ## ================================================= + Coverage 0.93% 5.09% +4.15% ================================================= Files 270 176 -94 Lines 21339 13055 -8284 Branches 383 1416 +1033 ================================================= + Hits 200 665 +465 + Misses 20982 12364 -8618 + Partials 157 26 -131 ``` | [Flag](https://app.codecov.io/gh/tier4/CalibrationTools/pull/148/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tier4) | Coverage Δ | | |---|---|---| | [differential](https://app.codecov.io/gh/tier4/CalibrationTools/pull/148/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tier4) | `5.09% <0.00%> (?)` | | | [total](https://app.codecov.io/gh/tier4/CalibrationTools/pull/148/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tier4) | `?` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tier4#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

knzo25 commented 8 months ago

@vividf Ok, we only need to fix the internal issues with the SfM calibrator and we are done :muscle:

knzo25 commented 8 months ago

Reminder to everyone seeing this to NOT merge until we finish the internal checks

tier4 / CalibrationTools