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 lidar resolution description about interpolation #506

Merged

Conversation

mabelzhang
Copy link
Collaborator

🦟 Bug fix

Fixes description about when interpolation is done in <lidar><scan><horizontal | vertical><resolution>.

Summary

The current description says "If resolution is less than one, range data is interpolated. If resolution is greater than one, range data is averaged."

But in the actual RaySensor implementation in both Gazebo 9 and 11, interpolation is done whenever resolution != 1. interp is set to true whenever rayCount != rangeCount (in SDF parameters' terms, that is samples != samples * resolution):
https://github.com/osrf/gazebo/blob/d94e2e0b6754a929c11dec4dff2131cfd291478e/gazebo/sensors/RaySensor.cc#L412-L413

This PR updates the description to match the implementation.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Code check passed (In source directory, run sh tools/code_check.sh)
  • All tests passed (See
    test coverage)
  • While waiting for a review on your PR, please help review
    another open pull request
    to support the maintainers

@iche033

Note to maintainers: Remember to use Squash-Merge

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
@github-actions github-actions bot added the 🏢 edifice Ignition Edifice label Mar 4, 2021
@mabelzhang
Copy link
Collaborator Author

Thanks for the review! I don't have permission in this repo to merge, so anyone else feel free to merge :)

@ahcorde ahcorde merged commit 9162e47 into gazebosim:master Mar 5, 2021
@mabelzhang mabelzhang deleted the mabelzhang/fix_lidar_resolution branch March 5, 2021 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏢 edifice Ignition Edifice
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants