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

Added option to specify x and y components of camera focal length. #374

Closed

Conversation

carlosmccosta
Copy link

No description provided.

this->focal_length_y_ = this->focal_length_x_;
}

if (!this->sdf->HasElement("focalLength_x"))
Copy link

Choose a reason for hiding this comment

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

be consistent with the naming focalLength_x vs focalLengthX and make sure you only output debug messages if none of the parameters are set.

Copy link
Author

Choose a reason for hiding this comment

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

Corrected sdf parameters names.
In relation to the warnings, it only outputs msgs if the focalLength and focalLengthX / focalLengthY aren't specified.

Thanks for the feedback :)

@hsu
Copy link
Collaborator

hsu commented Jan 21, 2016

@carlosmccosta Thank you, PR looks good. Do you have an example or test scenario for this PR?

@carlosmccosta
Copy link
Author

I added a testing world with a chess board in:
https://github.com/inesc-tec-robotics/gazebo_projection_mapping/blob/hydro-devel/worlds/camera_intrinsics.world

Some images showing the affect of increasing and decreasing each camera intrinsic parameters are available at:
https://github.com/inesc-tec-robotics/gazebo_projection_mapping/tree/hydro-devel/docs/camera_intrinsics

If you need further improvements let me known :)
Have a nice day :)

@scpeters scpeters added this to the untargeted milestone Apr 25, 2016
@j-rivero
Copy link
Contributor

j-rivero commented Jul 6, 2016

@osrf-jenkins run test please

@carlosmccosta
Copy link
Author

Good afternoon,

The error reported is due to this latter commit:
dacd872#diff-54a33e23f1a882810606eba53a485312R497

That overwrote this change:
35d585f#diff-54a33e23f1a882810606eba53a485312R512

And reverted the update of the field name from this->focal_length_x_ to this->focal_length_

To fix this, either

  • The pull request is merged and a new commit is made to remove the changes that reverted the this->focal_length_x_ member name
  • I apply the changes of this pull request into the current head of the jade-devel branch and create another pull request or overwrite the history of this pull request

How do you want to proceed?

Have a nice day :)

@j-rivero
Copy link
Contributor

j-rivero commented Jul 6, 2016

How do you want to proceed?

Thanks for the quick answer Carlos. I would prefer not to break the gazebo7 build in any moment in the current -devel branches, so I think that the best option is the second one that you proposed: update this PR with latest jade-devel changes. I really don't mind about creating a new pull request or overwrite this one, the one that works better for you.

Hace a good day you too :)

@carlosmccosta
Copy link
Author

Good morning,

I rebased this PR commit on top of the current jade-devel branch and rewrote the carlosmccosta:jade-devel git history.
This way there is a clean merge of a single commit on top of ros-simulation:jade-devel.
All CI tests passed. If you need further corrections / improvements let me know :)

Have a nice day :)

@carlosmccosta
Copy link
Author

Rebase of this PR for melodic-devel in #767

@ahcorde
Copy link
Contributor

ahcorde commented Nov 22, 2022

No activity since 2018 and targeting and unsupported ROS version

@ahcorde ahcorde closed this Nov 22, 2022
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.

None yet

6 participants