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(intrinsic_camera_calibrator): eval mode and specific camera profiles #220

Merged

Conversation

SergioReyesSan
Copy link
Contributor

Description

This PR contains:

-The result of merging Ceres method with evaluation mode.
-Added a selector for a predefined configuration from a yaml file when calibrating cameras.
-Fix for set parameters: The Chessbboard detection file lacked of a set_parameter function and was not possible to set parameters from a file.
-Fixed the implementation of Aspect ratio with the new camera model selector.

Related links

Tests performed

-Performed test with rosbag and topic mode (playing a rosbag in the terminal and subscribing to the node).
-Tested the C2 and Ceres configuration with rosbag and topic mode.

Notes for reviewers

Parameters on the YAML have not been selected as the best, but are providing somehow stable results.

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.

yabuta and others added 29 commits July 6, 2022 16:18
copy calibration tools from private repository
merge main for galactic release
…and calibration parameters into a file, change ui elements in eval mode.
…rinsics_evaluation_mode

Signed-off-by: Kenzo Lobos-Tsunekawa <[email protected]>
@SergioReyesSan SergioReyesSan requested a review from knzo25 December 3, 2024 07:04
Copy link
Collaborator

@knzo25 knzo25 left a comment

Choose a reason for hiding this comment

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

Some of the comments were not addressed in the last round and there were some over language-related comments. For the code suggestions, you can just click commit comment. For the others, please paste the commit that addresses the comments

SergioReyesSan and others added 9 commits December 25, 2024 09:04
…rator/intrinsic_camera_calibrator/board_detections/board_detection.py


accepting the suggestion

Co-authored-by: Kenzo Lobos Tsunekawa <[email protected]>
…rator/intrinsic_camera_calibrator/board_detections/board_detection.py


accepting the suggestion

Co-authored-by: Kenzo Lobos Tsunekawa <[email protected]>
…rator/intrinsic_camera_calibrator/board_detections/board_detection.py


accepting the suggestion of changing the grammar

Co-authored-by: Kenzo Lobos Tsunekawa <[email protected]>
…rator/intrinsic_camera_calibrator/data_collector.py


accepting the suggestion of changing the grammar

Co-authored-by: Kenzo Lobos Tsunekawa <[email protected]>
…rator/intrinsic_camera_calibrator/data_collector.py


accepting the suggestion of changing the grammar

Co-authored-by: Kenzo Lobos Tsunekawa <[email protected]>
…rator/intrinsic_camera_calibrator/views/image_view.py


accepting the suggestion of changing the grammar

Co-authored-by: Kenzo Lobos Tsunekawa <[email protected]>
…rator/intrinsic_camera_calibrator/views/image_view.py


accepting the suggestion

Co-authored-by: Kenzo Lobos Tsunekawa <[email protected]>
…rator/intrinsic_camera_calibrator/data_collector.py


accepting suggestion

Co-authored-by: Kenzo Lobos Tsunekawa <[email protected]>
@knzo25
Copy link
Collaborator

knzo25 commented Dec 26, 2024

@SergioReyesSan
There are a few comments that are left unaddressed. Probably you missed them because they are folded (you need to click Load more)

Copy link
Collaborator

@knzo25 knzo25 left a comment

Choose a reason for hiding this comment

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

The code is Ok now, but the parameters for the c2_intrinsics_calibrator.yaml do not allow us to calibrate correctly the c2-120.
For now, its contents should be the ones we use to calibrate those cameras (the same ones Yabuta-san is using)

Copy link
Collaborator

@knzo25 knzo25 left a comment

Choose a reason for hiding this comment

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

LGTM !

@knzo25
Copy link
Collaborator

knzo25 commented Jan 10, 2025

@SergioReyesSan
The PR was approved. Usually the person who sends the PR gets to merge it

@SergioReyesSan SergioReyesSan merged commit 116ada0 into tier4:tier4/universe Jan 10, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants