-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
fixed mIsOmnidirectional in WbObjectDetection #6396
fixed mIsOmnidirectional in WbObjectDetection #6396
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
May I kindly ask you to add a line in the changelog under "Bug fixes"?
Does this work for you? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good to me, but I believe this test could be also simplified now.
…tadt-ros-pkg/webots into fix-spherical-object-projection
That seems to be the case. I adapted it and used M_PI_2 instead of M_PI / 2.0 in the initial change now to remove the additional computation and stick to the use of M_PI_2. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks very good to me now.
Thank you!
@TheMangalex: This PR apparently broke the camera_recognition test, see: https://github.com/cyberbotics/webots/actions/runs/6386382958/job/17333860294#step:7:1254 |
I'll try to look into it in the following days |
Thank you for investigating this. |
I created #6403 as a draft which seems to fix the test and #6395 I want to add another test which specifically checks for the error which was fixed in #6395 , so I'll come back to this tomorrow. |
Description
Changes the calculation of mIsOmniDirectional to horizontalFov > pi / 2.
Related Issues
This pull-request fixes issue #6395
Additional context
Details on how the spherical projections were wrong before can be seen in #6395